On 06/08/2024 10:48, Utsav Agarwal via B4 Relay wrote: > From: Utsav Agarwal <utsav.agarwal@xxxxxxxxxx> > A nit, subject: drop second/last, redundant "dtbinding". The "dt-bindings" prefix is already stating that these are bindings. See also: https://elixir.bootlin.com/linux/v6.7-rc8/source/Documentation/devicetree/bindings/submitting-patches.rst#L18 Subject: everything is an update. Be descriptive and specific. > Updating dt bindings for adp5588. Since the device can now function in a > purely gpio mode, the following keypad specific properties are now made Hardware changed? How? > optional: > - interrupts > - keypad,num-rows > - keypad,num-columns > - linux,keymap > > However the above properties are required to be specified when > configuring the device as a keypad, dependencies have been added > such that specifying either one would require the remaining as well. > > Note that interrupts are optional, but required when the device has > either been configured in keypad mode or as an interrupt controller. > > Signed-off-by: Utsav Agarwal <utsav.agarwal@xxxxxxxxxx> > --- > .../devicetree/bindings/input/adi,adp5588.yaml | 51 +++++++++++++++++++--- > 1 file changed, 45 insertions(+), 6 deletions(-) > > diff --git a/Documentation/devicetree/bindings/input/adi,adp5588.yaml b/Documentation/devicetree/bindings/input/adi,adp5588.yaml > index 26ea66834ae2..827d72ece54b 100644 > --- a/Documentation/devicetree/bindings/input/adi,adp5588.yaml > +++ b/Documentation/devicetree/bindings/input/adi,adp5588.yaml > @@ -49,7 +49,12 @@ properties: > interrupt-controller: > description: > This property applies if either keypad,num-rows lower than 8 or > - keypad,num-columns lower than 10. > + keypad,num-columns lower than 10. This property is optional if > + keypad,num-rows or keypad,num-columns are not specified since the > + device then acts as gpio only, during which interrupts may or may > + not be utilized. If specified however, interrupts must be also be > + provided as all interrupt communication is h Don't repeat constraints in free form text. andled via a single > + interrupt line. > > '#interrupt-cells': > const: 2 > @@ -65,13 +70,30 @@ properties: > minItems: 1 > maxItems: 2 > > + > +dependencies: > + keypad,num-rows: > + - linux,keymap > + - keypad,num-columns > + keypad,num-columns: > + - linux,keymap > + - keypad,num-rows > + linux,keymap: > + - keypad,num-rows > + - keypad,num-columns > + interrupt-controller: > + - interrupts How is this related to this patchset? Why? I don't understand what are you trying to achieve here. Hardware changed? Are you fixing something? How many issues are you fixing in one (!!!) commit? > + > +if: > + required: > + - linux,keymap > +then: > + required: > + - interrupts > + > required: > - compatible > - reg > - - interrupts > - - keypad,num-rows > - - keypad,num-columns > - - linux,keymap > > unevaluatedProperties: false > > @@ -108,4 +130,21 @@ examples: > >; > }; > }; > -... > + > + - | > + #include <dt-bindings/interrupt-controller/irq.h> > + #include <dt-bindings/input/input.h> > + #include <dt-bindings/gpio/gpio.h> Where do you use these headers? > + i2c { > + #address-cells = <1>; > + #size-cells = <0>; > + gpio@34 { > + compatible = "adi,adp5588"; > + reg = <0x34>; > + > + #gpio-cells = <2>; > + gpio-controller; > + }; > + }; > + > +. Why this change? Best regards, Krzysztof
