| Message ID | 20220606184243.1057145-1-robh@kernel.org (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
| Series | dt-bindings: input: Convert adc-keys to DT schema | expand |
On 6/6/22 20:42, Rob Herring wrote: > Convert the adc-keys binding to DT schema format. > > Signed-off-by: Rob Herring <robh@kernel.org> > --- > .../devicetree/bindings/input/adc-keys.txt | 67 ------------ > .../devicetree/bindings/input/adc-keys.yaml | 103 ++++++++++++++++++ > 2 files changed, 103 insertions(+), 67 deletions(-) > delete mode 100644 Documentation/devicetree/bindings/input/adc-keys.txt > create mode 100644 Documentation/devicetree/bindings/input/adc-keys.yaml > > diff --git a/Documentation/devicetree/bindings/input/adc-keys.txt b/Documentation/devicetree/bindings/input/adc-keys.txt > deleted file mode 100644 > index 6c8be6a9ace2..000000000000 > --- a/Documentation/devicetree/bindings/input/adc-keys.txt > +++ /dev/null > @@ -1,67 +0,0 @@ > -ADC attached resistor ladder buttons > ------------------------------------- > - > -Required properties: > - - compatible: "adc-keys" > - - io-channels: Phandle to an ADC channel > - - io-channel-names = "buttons"; > - - keyup-threshold-microvolt: Voltage above or equal to which all the keys are > - considered up. > - > -Optional properties: > - - poll-interval: Poll interval time in milliseconds > - - autorepeat: Boolean, Enable auto repeat feature of Linux input > - subsystem. > - > -Each button (key) is represented as a sub-node of "adc-keys": > - > -Required subnode-properties: > - - label: Descriptive name of the key. > - - linux,code: Keycode to emit. > - - press-threshold-microvolt: voltage above or equal to which this key is > - considered pressed. > - > -No two values of press-threshold-microvolt may be the same. > -All values of press-threshold-microvolt must be less than > -keyup-threshold-microvolt. > - > -Example: > - > -#include <dt-bindings/input/input.h> > - > - adc-keys { > - compatible = "adc-keys"; > - io-channels = <&lradc 0>; > - io-channel-names = "buttons"; > - keyup-threshold-microvolt = <2000000>; > - > - button-up { > - label = "Volume Up"; > - linux,code = <KEY_VOLUMEUP>; > - press-threshold-microvolt = <1500000>; > - }; > - > - button-down { > - label = "Volume Down"; > - linux,code = <KEY_VOLUMEDOWN>; > - press-threshold-microvolt = <1000000>; > - }; > - > - button-enter { > - label = "Enter"; > - linux,code = <KEY_ENTER>; > - press-threshold-microvolt = <500000>; > - }; > - }; > - > -+--------------------------------+------------------------+ > -| 2.000.000 <= value | no key pressed | > -+--------------------------------+------------------------+ > -| 1.500.000 <= value < 2.000.000 | KEY_VOLUMEUP pressed | > -+--------------------------------+------------------------+ > -| 1.000.000 <= value < 1.500.000 | KEY_VOLUMEDOWN pressed | > -+--------------------------------+------------------------+ > -| 500.000 <= value < 1.000.000 | KEY_ENTER pressed | > -+--------------------------------+------------------------+ > -| value < 500.000 | no key pressed | > -+--------------------------------+------------------------+ > diff --git a/Documentation/devicetree/bindings/input/adc-keys.yaml b/Documentation/devicetree/bindings/input/adc-keys.yaml > new file mode 100644 > index 000000000000..a3a1af9550bc > --- /dev/null > +++ b/Documentation/devicetree/bindings/input/adc-keys.yaml > @@ -0,0 +1,103 @@ > +# SPDX-License-Identifier: GPL-2.0 > +%YAML 1.2 > +--- > +$id: http://devicetree.org/schemas/input/adc-keys.yaml# > +$schema: http://devicetree.org/meta-schemas/core.yaml# > + > +title: ADC attached resistor ladder buttons > + > +maintainers: > + - Heinrich Schuchardt <xypron.glpk@gmx.de> Thanks for converting to yaml. I only contributed a single patch. I am not a maintainer. Please, remove that line. scripts/get_maintainer.pl Documentation/devicetree/bindings/input/adc-keys.txt yields Dmitry Torokhov <dmitry.torokhov@gmail.com> (maintainer:INPUT (KEYBOARD, MOUSE, JOYSTICK, TOUCHSCREEN)...) It would be preferable to have a single reference point for maintainership: file /MAINTAINERS. > + - Alexandre Belloni <alexandre.belloni@bootlin.com> > + > +allOf: > + - $ref: /schemas/input/input.yaml# > + > +properties: > + compatible: > + const: adc-keys > + > + io-channels: > + maxItems: 1 Please, add a description to each property. > + > + io-channel-names: > + const: buttons > + > + keyup-threshold-microvolt: > + description: > + Voltage above or equal to which all the keys are considered up. > + > +patternProperties: > + '^button-': > + type: object > + additionalProperties: false > + description: > + Each button (key) is represented as a sub-node. > + > + properties: > + label: true Please, add a description. > + > + linux,code: > + description: Keycode to emit. > + $ref: /schemas/types.yaml#/definitions/uint32 Unfortunately usable values are only defined in include/uapi/linux/input-event-codes.h up to now. Please, consider adding that link to the description. It is unclear to me if using values above KEY_MAX (=0x2ff) could make sense of should be forbidden by this yaml file. For interoperability of device-trees with other operating systems we should have a yaml file defining an enum with used values and their meaning. > + > + press-threshold-microvolt: > + description: > + Voltage above or equal to which this key is considered pressed. No > + two values of press-threshold-microvolt may be the same. All values > + of press-threshold-microvolt must be less than > + keyup-threshold-microvolt. > + > + required: > + - label Property label is not used in our the driver code. It only exists for human readability. Why is it marked as required? Stripping the labels would reduce the DT size. Best regards Heinrich > + - linux,code > + - press-threshold-microvolt > + > +required: > + - compatible > + - io-channels > + - io-channel-names > + - keyup-threshold-microvolt > + > +unevaluatedProperties: false > + > +examples: > + - | > + #include <dt-bindings/input/input.h> > + // +--------------------------------+------------------------+ > + // | 2.000.000 <= value | no key pressed | > + // +--------------------------------+------------------------+ > + // | 1.500.000 <= value < 2.000.000 | KEY_VOLUMEUP pressed | > + // +--------------------------------+------------------------+ > + // | 1.000.000 <= value < 1.500.000 | KEY_VOLUMEDOWN pressed | > + // +--------------------------------+------------------------+ > + // | 500.000 <= value < 1.000.000 | KEY_ENTER pressed | > + // +--------------------------------+------------------------+ > + // | value < 500.000 | no key pressed | > + // +--------------------------------+------------------------+ > + > + adc-keys { > + compatible = "adc-keys"; > + io-channels = <&lradc 0>; > + io-channel-names = "buttons"; > + keyup-threshold-microvolt = <2000000>; > + > + button-up { > + label = "Volume Up"; > + linux,code = <KEY_VOLUMEUP>; > + press-threshold-microvolt = <1500000>; > + }; > + > + button-down { > + label = "Volume Down"; > + linux,code = <KEY_VOLUMEDOWN>; > + press-threshold-microvolt = <1000000>; > + }; > + > + button-enter { > + label = "Enter"; > + linux,code = <KEY_ENTER>; > + press-threshold-microvolt = <500000>; > + }; > + }; > +...
On Tue, Jun 07, 2022 at 09:48:33AM +0200, Heinrich Schuchardt wrote: > On 6/6/22 20:42, Rob Herring wrote: > > Convert the adc-keys binding to DT schema format. > > > > Signed-off-by: Rob Herring <robh@kernel.org> > > --- > > .../devicetree/bindings/input/adc-keys.txt | 67 ------------ > > .../devicetree/bindings/input/adc-keys.yaml | 103 ++++++++++++++++++ > > 2 files changed, 103 insertions(+), 67 deletions(-) > > delete mode 100644 Documentation/devicetree/bindings/input/adc-keys.txt > > create mode 100644 Documentation/devicetree/bindings/input/adc-keys.yaml > > -+--------------------------------+------------------------+ > > diff --git a/Documentation/devicetree/bindings/input/adc-keys.yaml b/Documentation/devicetree/bindings/input/adc-keys.yaml > > new file mode 100644 > > index 000000000000..a3a1af9550bc > > --- /dev/null > > +++ b/Documentation/devicetree/bindings/input/adc-keys.yaml > > @@ -0,0 +1,103 @@ > > +# SPDX-License-Identifier: GPL-2.0 > > +%YAML 1.2 > > +--- > > +$id: http://devicetree.org/schemas/input/adc-keys.yaml# > > +$schema: http://devicetree.org/meta-schemas/core.yaml# > > + > > +title: ADC attached resistor ladder buttons > > + > > +maintainers: > > + - Heinrich Schuchardt <xypron.glpk@gmx.de> > > Thanks for converting to yaml. > > I only contributed a single patch. I am not a maintainer. Please, remove > that line. Okay. > scripts/get_maintainer.pl > Documentation/devicetree/bindings/input/adc-keys.txt yields > Dmitry Torokhov <dmitry.torokhov@gmail.com> (maintainer:INPUT (KEYBOARD, > MOUSE, JOYSTICK, TOUCHSCREEN)...) The maintainer here is supposed to be someone that cares about this particular binding, not who applies patches. IOW, who would care if the binding was deleted. > It would be preferable to have a single reference point for > maintainership: file /MAINTAINERS. There's 2 main reasons why it is not. MAINTAINERS doesn't work for the DT only tree we generate[1]. Second, having an entry in MAINTAINERS for bindings is not consistent. With it in the schema, I don't have to check, the tools do it for me. > > + - Alexandre Belloni <alexandre.belloni@bootlin.com> > > + > > +allOf: > > + - $ref: /schemas/input/input.yaml# > > + > > +properties: > > + compatible: > > + const: adc-keys > > + > > + io-channels: > > + maxItems: 1 > > Please, add a description to each property. Common properties don't get descriptions unless there is something specific about this binding to say. What would that be? > > + io-channel-names: > > + const: buttons > > + > > + keyup-threshold-microvolt: > > + description: > > + Voltage above or equal to which all the keys are considered up. > > + > > +patternProperties: > > + '^button-': > > + type: object > > + additionalProperties: false > > + description: > > + Each button (key) is represented as a sub-node. > > + > > + properties: > > + label: true > > Please, add a description. No, common property. > > + linux,code: > > + description: Keycode to emit. > > + $ref: /schemas/types.yaml#/definitions/uint32 > > Unfortunately usable values are only defined in > include/uapi/linux/input-event-codes.h up to now. Please, consider > adding that link to the description. linux,code is common for lots of bindings, so really the type, description, and any common constraints need to be documented somewhere common. I'm not sure if input.yaml makes sense given that includes a bunch of other properties. > It is unclear to me if using values above KEY_MAX (=0x2ff) could make > sense of should be forbidden by this yaml file. Shrug. I have no idea. > For interoperability of device-trees with other operating systems we > should have a yaml file defining an enum with used values and their meaning. Certainly, but that's an orthogonal issue. > > + > > + press-threshold-microvolt: > > + description: > > + Voltage above or equal to which this key is considered pressed. No > > + two values of press-threshold-microvolt may be the same. All values > > + of press-threshold-microvolt must be less than > > + keyup-threshold-microvolt. > > + > > + required: > > + - label > > Property label is not used in our the driver code. It only exists for > human readability. Why is it marked as required? Stripping the labels > would reduce the DT size. From the original: > > -Required subnode-properties: > > - - label: Descriptive name of the key. But I'll drop it. Rob [1] https://git.kernel.org/pub/scm/linux/kernel/git/devicetree/devicetree-rebasing.git/tree/
On 6/7/22 17:03, Rob Herring wrote: > On Tue, Jun 07, 2022 at 09:48:33AM +0200, Heinrich Schuchardt wrote: >> On 6/6/22 20:42, Rob Herring wrote: >>> Convert the adc-keys binding to DT schema format. >>> >>> Signed-off-by: Rob Herring <robh@kernel.org> >>> --- >>> .../devicetree/bindings/input/adc-keys.txt | 67 ------------ >>> .../devicetree/bindings/input/adc-keys.yaml | 103 ++++++++++++++++++ >>> 2 files changed, 103 insertions(+), 67 deletions(-) >>> delete mode 100644 Documentation/devicetree/bindings/input/adc-keys.txt >>> create mode 100644 Documentation/devicetree/bindings/input/adc-keys.yaml > >>> -+--------------------------------+------------------------+ >>> diff --git a/Documentation/devicetree/bindings/input/adc-keys.yaml b/Documentation/devicetree/bindings/input/adc-keys.yaml >>> new file mode 100644 >>> index 000000000000..a3a1af9550bc >>> --- /dev/null >>> +++ b/Documentation/devicetree/bindings/input/adc-keys.yaml >>> @@ -0,0 +1,103 @@ >>> +# SPDX-License-Identifier: GPL-2.0 >>> +%YAML 1.2 >>> +--- >>> +$id: http://devicetree.org/schemas/input/adc-keys.yaml# >>> +$schema: http://devicetree.org/meta-schemas/core.yaml# >>> + >>> +title: ADC attached resistor ladder buttons >>> + >>> +maintainers: >>> + - Heinrich Schuchardt <xypron.glpk@gmx.de> >> >> Thanks for converting to yaml. >> >> I only contributed a single patch. I am not a maintainer. Please, remove >> that line. > > Okay. > >> scripts/get_maintainer.pl >> Documentation/devicetree/bindings/input/adc-keys.txt yields >> Dmitry Torokhov <dmitry.torokhov@gmail.com> (maintainer:INPUT (KEYBOARD, >> MOUSE, JOYSTICK, TOUCHSCREEN)...) > > The maintainer here is supposed to be someone that cares about this > particular binding, not who applies patches. IOW, who would care if the > binding was deleted. > >> It would be preferable to have a single reference point for >> maintainership: file /MAINTAINERS. > > There's 2 main reasons why it is not. MAINTAINERS doesn't work for the > DT only tree we generate[1]. Second, having an entry in MAINTAINERS > for bindings is not consistent. With it in the schema, I don't have to > check, the tools do it for me. > >>> + - Alexandre Belloni <alexandre.belloni@bootlin.com> >>> + >>> +allOf: >>> + - $ref: /schemas/input/input.yaml# >>> + >>> +properties: >>> + compatible: >>> + const: adc-keys >>> + >>> + io-channels: >>> + maxItems: 1 >> >> Please, add a description to each property. > > Common properties don't get descriptions unless there is something > specific about this binding to say. What would that be? > > >>> + io-channel-names: >>> + const: buttons >>> + >>> + keyup-threshold-microvolt: >>> + description: >>> + Voltage above or equal to which all the keys are considered up. >>> + >>> +patternProperties: >>> + '^button-': >>> + type: object >>> + additionalProperties: false >>> + description: >>> + Each button (key) is represented as a sub-node. >>> + >>> + properties: >>> + label: true >> >> Please, add a description. > > No, common property. > >>> + linux,code: >>> + description: Keycode to emit. >>> + $ref: /schemas/types.yaml#/definitions/uint32 >> >> Unfortunately usable values are only defined in >> include/uapi/linux/input-event-codes.h up to now. Please, consider >> adding that link to the description. > > linux,code is common for lots of bindings, so really the type, > description, and any common constraints need to be documented somewhere > common. I'm not sure if input.yaml makes sense given that includes a > bunch of other properties. > >> It is unclear to me if using values above KEY_MAX (=0x2ff) could make >> sense of should be forbidden by this yaml file. > > Shrug. I have no idea. > >> For interoperability of device-trees with other operating systems we >> should have a yaml file defining an enum with used values and their meaning. > > Certainly, but that's an orthogonal issue. Yes, it is orthogonal. git grep -ni keycode Documentation/devicetree/bindings/ finds 15+ documents referring to key-codes. So this will be a worthwhile undertaking. Best regards Heinrich > >>> + >>> + press-threshold-microvolt: >>> + description: >>> + Voltage above or equal to which this key is considered pressed. No >>> + two values of press-threshold-microvolt may be the same. All values >>> + of press-threshold-microvolt must be less than >>> + keyup-threshold-microvolt. >>> + >>> + required: >>> + - label >> >> Property label is not used in our the driver code. It only exists for >> human readability. Why is it marked as required? Stripping the labels >> would reduce the DT size. > >>From the original: > >>> -Required subnode-properties: >>> - - label: Descriptive name of the key. > > But I'll drop it. > > Rob > > [1] https://git.kernel.org/pub/scm/linux/kernel/git/devicetree/devicetree-rebasing.git/tree/
diff --git a/Documentation/devicetree/bindings/input/adc-keys.txt b/Documentation/devicetree/bindings/input/adc-keys.txt deleted file mode 100644 index 6c8be6a9ace2..000000000000 --- a/Documentation/devicetree/bindings/input/adc-keys.txt +++ /dev/null @@ -1,67 +0,0 @@ -ADC attached resistor ladder buttons ------------------------------------- - -Required properties: - - compatible: "adc-keys" - - io-channels: Phandle to an ADC channel - - io-channel-names = "buttons"; - - keyup-threshold-microvolt: Voltage above or equal to which all the keys are - considered up. - -Optional properties: - - poll-interval: Poll interval time in milliseconds - - autorepeat: Boolean, Enable auto repeat feature of Linux input - subsystem. - -Each button (key) is represented as a sub-node of "adc-keys": - -Required subnode-properties: - - label: Descriptive name of the key. - - linux,code: Keycode to emit. - - press-threshold-microvolt: voltage above or equal to which this key is - considered pressed. - -No two values of press-threshold-microvolt may be the same. -All values of press-threshold-microvolt must be less than -keyup-threshold-microvolt. - -Example: - -#include <dt-bindings/input/input.h> - - adc-keys { - compatible = "adc-keys"; - io-channels = <&lradc 0>; - io-channel-names = "buttons"; - keyup-threshold-microvolt = <2000000>; - - button-up { - label = "Volume Up"; - linux,code = <KEY_VOLUMEUP>; - press-threshold-microvolt = <1500000>; - }; - - button-down { - label = "Volume Down"; - linux,code = <KEY_VOLUMEDOWN>; - press-threshold-microvolt = <1000000>; - }; - - button-enter { - label = "Enter"; - linux,code = <KEY_ENTER>; - press-threshold-microvolt = <500000>; - }; - }; - -+--------------------------------+------------------------+ -| 2.000.000 <= value | no key pressed | -+--------------------------------+------------------------+ -| 1.500.000 <= value < 2.000.000 | KEY_VOLUMEUP pressed | -+--------------------------------+------------------------+ -| 1.000.000 <= value < 1.500.000 | KEY_VOLUMEDOWN pressed | -+--------------------------------+------------------------+ -| 500.000 <= value < 1.000.000 | KEY_ENTER pressed | -+--------------------------------+------------------------+ -| value < 500.000 | no key pressed | -+--------------------------------+------------------------+ diff --git a/Documentation/devicetree/bindings/input/adc-keys.yaml b/Documentation/devicetree/bindings/input/adc-keys.yaml new file mode 100644 index 000000000000..a3a1af9550bc --- /dev/null +++ b/Documentation/devicetree/bindings/input/adc-keys.yaml @@ -0,0 +1,103 @@ +# SPDX-License-Identifier: GPL-2.0 +%YAML 1.2 +--- +$id: http://devicetree.org/schemas/input/adc-keys.yaml# +$schema: http://devicetree.org/meta-schemas/core.yaml# + +title: ADC attached resistor ladder buttons + +maintainers: + - Heinrich Schuchardt <xypron.glpk@gmx.de> + - Alexandre Belloni <alexandre.belloni@bootlin.com> + +allOf: + - $ref: /schemas/input/input.yaml# + +properties: + compatible: + const: adc-keys + + io-channels: + maxItems: 1 + + io-channel-names: + const: buttons + + keyup-threshold-microvolt: + description: + Voltage above or equal to which all the keys are considered up. + +patternProperties: + '^button-': + type: object + additionalProperties: false + description: + Each button (key) is represented as a sub-node. + + properties: + label: true + + linux,code: + description: Keycode to emit. + $ref: /schemas/types.yaml#/definitions/uint32 + + press-threshold-microvolt: + description: + Voltage above or equal to which this key is considered pressed. No + two values of press-threshold-microvolt may be the same. All values + of press-threshold-microvolt must be less than + keyup-threshold-microvolt. + + required: + - label + - linux,code + - press-threshold-microvolt + +required: + - compatible + - io-channels + - io-channel-names + - keyup-threshold-microvolt + +unevaluatedProperties: false + +examples: + - | + #include <dt-bindings/input/input.h> + // +--------------------------------+------------------------+ + // | 2.000.000 <= value | no key pressed | + // +--------------------------------+------------------------+ + // | 1.500.000 <= value < 2.000.000 | KEY_VOLUMEUP pressed | + // +--------------------------------+------------------------+ + // | 1.000.000 <= value < 1.500.000 | KEY_VOLUMEDOWN pressed | + // +--------------------------------+------------------------+ + // | 500.000 <= value < 1.000.000 | KEY_ENTER pressed | + // +--------------------------------+------------------------+ + // | value < 500.000 | no key pressed | + // +--------------------------------+------------------------+ + + adc-keys { + compatible = "adc-keys"; + io-channels = <&lradc 0>; + io-channel-names = "buttons"; + keyup-threshold-microvolt = <2000000>; + + button-up { + label = "Volume Up"; + linux,code = <KEY_VOLUMEUP>; + press-threshold-microvolt = <1500000>; + }; + + button-down { + label = "Volume Down"; + linux,code = <KEY_VOLUMEDOWN>; + press-threshold-microvolt = <1000000>; + }; + + button-enter { + label = "Enter"; + linux,code = <KEY_ENTER>; + press-threshold-microvolt = <500000>; + }; + }; +...
Convert the adc-keys binding to DT schema format. Signed-off-by: Rob Herring <robh@kernel.org> --- .../devicetree/bindings/input/adc-keys.txt | 67 ------------ .../devicetree/bindings/input/adc-keys.yaml | 103 ++++++++++++++++++ 2 files changed, 103 insertions(+), 67 deletions(-) delete mode 100644 Documentation/devicetree/bindings/input/adc-keys.txt create mode 100644 Documentation/devicetree/bindings/input/adc-keys.yaml