| Message ID | 20221204061555.1355453-1-dmitry.baryshkov@linaro.org |
|---|---|
| Headers | show |
| Series | dt-bindings: add missing subdevices to qcom-pm8xxx schema | expand |
On Sun, 4 Dec 2022 at 17:24, Jonathan Cameron <jic23@kernel.org> wrote: > > On Sun, 4 Dec 2022 08:15:54 +0200 > Dmitry Baryshkov <dmitry.baryshkov@linaro.org> wrote: > > > Several ADC channels are bound to the Multi Purpose Pins (MPPs). Allow > > specifying such channels using the mppN device node (as used on apq8060 > > dragonboard). > > > > Signed-off-by: Dmitry Baryshkov <dmitry.baryshkov@linaro.org> > > So I understand this more, why do the node names have to have anything to > do with the particular pin? I'm assuming the reg value provides that > relationship. Yes, the reg provides this relationship. If I understand correctly, the dts authors (see arch/arm/boot/dts/qcom-apq8060-dragonboard.dts) wanted to point out that these channels are connected to MPP pins rather than raw adc channels (e.g. vcoin, vbat, etc). > > > --- > > Documentation/devicetree/bindings/iio/adc/qcom,pm8018-adc.yaml | 2 +- > > 1 file changed, 1 insertion(+), 1 deletion(-) > > > > diff --git a/Documentation/devicetree/bindings/iio/adc/qcom,pm8018-adc.yaml b/Documentation/devicetree/bindings/iio/adc/qcom,pm8018-adc.yaml > > index d186b713d6a7..fee30e6ddd62 100644 > > --- a/Documentation/devicetree/bindings/iio/adc/qcom,pm8018-adc.yaml > > +++ b/Documentation/devicetree/bindings/iio/adc/qcom,pm8018-adc.yaml > > @@ -64,7 +64,7 @@ required: > > - adc-channel@f > > > > patternProperties: > > - "^(adc-channel@)[0-9a-f]$": > > + "^(adc-channel|mpp[0-9]+)@[0-9a-f]$": > > type: object > > description: | > > ADC channel specific configuration. >
On Sun, Dec 04, 2022 at 08:15:52AM +0200, Dmitry Baryshkov wrote: > Convert the bindings for the keypad subdevices of Qualcomm PM8921 and > PM8058 PMICs from text to YAML format. > > While doing the conversion also change linux,keypad-no-autorepeat > property to linux,input-no-autorepeat. The former property was never > used by DT and was never handled by the driver. Changing from the documented one to one some drivers use. I guess that's a slight improvement. Please see this discussion[1]. Rob [1] https://lore.kernel.org/all/YowEgvwBOSEK+kd2@google.com/
6 декабря 2022 г. 00:04:33 GMT+02:00, Rob Herring <robh@kernel.org> пишет: >On Sun, Dec 04, 2022 at 08:15:52AM +0200, Dmitry Baryshkov wrote: >> Convert the bindings for the keypad subdevices of Qualcomm PM8921 and >> PM8058 PMICs from text to YAML format. >> >> While doing the conversion also change linux,keypad-no-autorepeat >> property to linux,input-no-autorepeat. The former property was never >> used by DT and was never handled by the driver. > >Changing from the documented one to one some drivers use. I guess >that's a slight improvement. Please see this discussion[1]. Well, the problem is that the documentation is misleading. The driver doesn't handle the documented property, so we should change either the driver, or the docs. Which change is the preferred one? > >Rob > >[1] https://lore.kernel.org/all/YowEgvwBOSEK+kd2@google.com/
On Tue, Dec 06, 2022 at 05:20:16AM +0200, Dmitry Baryshkov wrote: > 6 декабря 2022 г. 00:04:33 GMT+02:00, Rob Herring <robh@kernel.org> пишет: > >On Sun, Dec 04, 2022 at 08:15:52AM +0200, Dmitry Baryshkov wrote: > >> Convert the bindings for the keypad subdevices of Qualcomm PM8921 and > >> PM8058 PMICs from text to YAML format. > >> > >> While doing the conversion also change linux,keypad-no-autorepeat > >> property to linux,input-no-autorepeat. The former property was never > >> used by DT and was never handled by the driver. > > > >Changing from the documented one to one some drivers use. I guess > >that's a slight improvement. Please see this discussion[1]. > > Well, the problem is that the documentation is misleading. The driver > doesn't handle the documented property, so we should change either > the driver, or the docs. Which change is the preferred one? The preference is autorepeat is not the default and setting 'autorepeat' enables it. You can't really change that unless you don't really need autorepeat by default. I can't see why it would be needed for the power button, but I haven't looked what else you have. Of all the no autorepeat options, I prefer 'linux,no-autorepeat' as I find 'input' or 'keypad' redundant. But Dmitry T. didn't think it should be a common property at the time. Rob
On Wed, 7 Dec 2022 at 19:07, Rob Herring <robh@kernel.org> wrote: > > On Tue, Dec 06, 2022 at 05:20:16AM +0200, Dmitry Baryshkov wrote: > > 6 декабря 2022 г. 00:04:33 GMT+02:00, Rob Herring <robh@kernel.org> пишет: > > >On Sun, Dec 04, 2022 at 08:15:52AM +0200, Dmitry Baryshkov wrote: > > >> Convert the bindings for the keypad subdevices of Qualcomm PM8921 and > > >> PM8058 PMICs from text to YAML format. > > >> > > >> While doing the conversion also change linux,keypad-no-autorepeat > > >> property to linux,input-no-autorepeat. The former property was never > > >> used by DT and was never handled by the driver. > > > > > >Changing from the documented one to one some drivers use. I guess > > >that's a slight improvement. Please see this discussion[1]. > > > > Well, the problem is that the documentation is misleading. The driver > > doesn't handle the documented property, so we should change either > > the driver, or the docs. Which change is the preferred one? > > The preference is autorepeat is not the default and setting > 'autorepeat' enables it. You can't really change that unless you don't > really need autorepeat by default. I can't see why it would be > needed for the power button, but I haven't looked what else you have. It's not a pon/resin. this is a full-fledged keypad. For example for apq8060-dragonboard: linux,keymap = < MATRIX_KEY(0, 0, KEY_MENU) MATRIX_KEY(0, 2, KEY_1) MATRIX_KEY(0, 3, KEY_4) MATRIX_KEY(0, 4, KEY_7) MATRIX_KEY(1, 0, KEY_UP) MATRIX_KEY(1, 1, KEY_LEFT) MATRIX_KEY(1, 2, KEY_DOWN) MATRIX_KEY(1, 3, KEY_5) MATRIX_KEY(1, 3, KEY_8) MATRIX_KEY(2, 0, KEY_HOME) MATRIX_KEY(2, 1, KEY_REPLY) MATRIX_KEY(2, 2, KEY_2) MATRIX_KEY(2, 3, KEY_6) MATRIX_KEY(3, 0, KEY_VOLUMEUP) MATRIX_KEY(3, 1, KEY_RIGHT) MATRIX_KEY(3, 2, KEY_3) MATRIX_KEY(3, 3, KEY_9) MATRIX_KEY(3, 4, KEY_SWITCHVIDEOMODE) MATRIX_KEY(4, 0, KEY_VOLUMEDOWN) MATRIX_KEY(4, 1, KEY_BACK) MATRIX_KEY(4, 2, KEY_CAMERA) MATRIX_KEY(4, 3, KEY_KBDILLUMTOGGLE) >; > Of all the no autorepeat options, I prefer 'linux,no-autorepeat' as I > find 'input' or 'keypad' redundant. But Dmitry T. didn't think it should > be a common property at the time. We have not used any of the options in the in-kernel DTs. However the driver for the keypad has supported the 'linux,input-no-autorepeat' since March 2014. I'm just changing the docs to document the correct option. I can split the patch into two distinct patches (one for the bugfix, one for conversion), if you think that it would be better.
On Wed, Dec 07, 2022 at 11:07:53AM -0600, Rob Herring wrote: > On Tue, Dec 06, 2022 at 05:20:16AM +0200, Dmitry Baryshkov wrote: > > 6 декабря 2022 г. 00:04:33 GMT+02:00, Rob Herring <robh@kernel.org> пишет: > > >On Sun, Dec 04, 2022 at 08:15:52AM +0200, Dmitry Baryshkov wrote: > > >> Convert the bindings for the keypad subdevices of Qualcomm PM8921 and > > >> PM8058 PMICs from text to YAML format. > > >> > > >> While doing the conversion also change linux,keypad-no-autorepeat > > >> property to linux,input-no-autorepeat. The former property was never > > >> used by DT and was never handled by the driver. > > > > > >Changing from the documented one to one some drivers use. I guess > > >that's a slight improvement. Please see this discussion[1]. > > > > Well, the problem is that the documentation is misleading. The driver > > doesn't handle the documented property, so we should change either > > the driver, or the docs. Which change is the preferred one? > > The preference is autorepeat is not the default and setting > 'autorepeat' enables it. You can't really change that unless you don't > really need autorepeat by default. I can't see why it would be > needed for the power button, but I haven't looked what else you have. > > Of all the no autorepeat options, I prefer 'linux,no-autorepeat' as I > find 'input' or 'keypad' redundant. But Dmitry T. didn't think it should > be a common property at the time. Right, I would prefer for new bindings we used assertive "autorepeat", instead of negating "no-autorepeat". However here we are dealing with existing binding. The issue is complicated with the driver using one option, binding specifying another, and existing in-kernel DTSes not using any and thus activating autorepeat as the default driver behavior. Do we have an idea if there are out-of-tree users of this? If we are reasonable sure there are not we could converge on the standard "autorepeat" property.