On Tue Jan 14, 2025 at 9:11 AM CET, Krzysztof Kozlowski wrote:

On Mon, Jan 13, 2025 at 01:42:25PM +0100, Mathieu Dubois-Briand wrote:

quoted

+
+  rotary-debounce-delay-ms:
+    description: Rotary encoder debounce delay in ms
+    minimum: 0
+    maximum: 15
+    default: 0
+
+  linux,axis:
+    description: The input subsystem axis to map to this rotary encoder.

Missing type. I guess you wanted to reference rotary encoder schema,
next to input and matrix-keymap?

I'm not sure I fully understood your suggestion. Do you mean adding a
reference to rotary-encoder.yaml, at the root of the document? Like:

allOf:
  - $ref: /schemas/input/matrix-keymap.yaml#
  - $ref: /schemas/input/input.yaml#
  - $ref: /schemas/input/rotary-encoder.yaml#

I did base the schema of the rotary encoder part on rotary-encoder.yaml,
but I believe we cannot reference it directly: it adds some properties
that do not make sense here (gpios, rotary-encoder,steps...) and also
some of them are mandatory.

Yet I see that I'm not referring to any type here. Also I did not
specify the default value. Would the following be OK?

  linux,axis:
    description: The input subsystem axis to map to the rotary encoder.
    $ref: /schemas/types.yaml#/definitions/uint32
    default: 0

quoted

+      COL2 to COL7 general purpose output pins configuration.
+      Allows to use unused keypad columns as outputs.
+      The MAX7360 has 8 column lines and 6 of them can be used as GPOs. Value
+      of ngpios must be coherent with the value of keypad,num-columns, as their
+      sum must not exceed the number of physical lines.
+
+required:
+  - compatible
+  - reg
+  - interrupts
+  - interrupt-names
+  - linux,keymap
+  - linux,axis
+  - "#pwm-cells"

gpio and gpo nodes are optional? How would the driver behave? I assume
you need to define the partition between GPIOs, especially that 'ngpios'
are a required property in their schema.

No, you are right. In my mind it was optional, but current driver
implementation will complain if the gpo node is missing. I could make it
optional in the code, but it's probably better to make it required in
the device tree, so the hardware is correctly described.

quoted

-- 
2.39.5

I have fixed the other points listed in your mail. Thanks for your review.


-- 
Mathieu Dubois-Briand, Bootlin
Embedded Linux and Kernel engineering
https://bootlin.com