linux-kernel - Re: [PATCH v3 1/7] dt-bindings: mfd: gpio: Add MAX7360

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Message-Id: <D71TE4TEJKDF.1QRNYB2TKOMYJ@bootlin.com>
Date: Tue, 14 Jan 2025 14:02:18 +0100
From: "Mathieu Dubois-Briand" <mathieu.dubois-briand@...tlin.com>
To: "Krzysztof Kozlowski" <krzk@...nel.org>
Cc: "Lee Jones" <lee@...nel.org>, "Rob Herring" <robh@...nel.org>,
 "Krzysztof Kozlowski" <krzk+dt@...nel.org>, "Conor Dooley"
 <conor+dt@...nel.org>, "Kamel Bouhara" <kamel.bouhara@...tlin.com>, "Linus
 Walleij" <linus.walleij@...aro.org>, "Bartosz Golaszewski" <brgl@...ev.pl>,
 "Dmitry Torokhov" <dmitry.torokhov@...il.com>,
 Uwe Kleine-König <ukleinek@...nel.org>,
 <devicetree@...r.kernel.org>, <linux-kernel@...r.kernel.org>,
 <linux-gpio@...r.kernel.org>, <linux-input@...r.kernel.org>,
 <linux-pwm@...r.kernel.org>, Grégory Clement
 <gregory.clement@...tlin.com>, "Thomas Petazzoni"
 <thomas.petazzoni@...tlin.com>
Subject: Re: [PATCH v3 1/7] dt-bindings: mfd: gpio: Add MAX7360

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