Messages in this thread | | | Date | Tue, 7 Jun 2022 22:45:58 +0200 | Subject | Re: [PATCH] dt-bindings: input: Convert adc-keys to DT schema | From | Heinrich Schuchardt <> |
| |
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/
| |