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:
> > +
> > + 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
> > + 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.
> >
> > --
> > 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
