Re: [PATCH] dt-bindings: input: Convert adc-keys to DT schema

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



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@xxxxxxxxxx>
---
   .../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@xxxxxx>

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@xxxxxxxxx> (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@xxxxxxxxxxx>
+
+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/





[Index of Archives]     [Device Tree Compilter]     [Device Tree Spec]     [Linux Driver Backports]     [Video for Linux]     [Linux USB Devel]     [Linux PCI Devel]     [Linux Audio Users]     [Linux Kernel]     [Linux SCSI]     [XFree86]     [Yosemite Backpacking]


  Powered by Linux