> -----Original Message----- > From: Krzysztof Kozlowski <krzk@xxxxxxxxxx> > Sent: Tuesday, August 6, 2024 10:04 AM > To: Agarwal, Utsav <Utsav.Agarwal@xxxxxxxxxx>; Hennerich, Michael > <Michael.Hennerich@xxxxxxxxxx>; Dmitry Torokhov > <dmitry.torokhov@xxxxxxxxx>; Rob Herring <robh@xxxxxxxxxx>; Krzysztof > Kozlowski <krzk+dt@xxxxxxxxxx>; Conor Dooley <conor+dt@xxxxxxxxxx>; Sa, > Nuno <Nuno.Sa@xxxxxxxxxx> > Cc: linux-input@xxxxxxxxxxxxxxx; devicetree@xxxxxxxxxxxxxxx; linux- > kernel@xxxxxxxxxxxxxxx; Artamonovs, Arturs > <Arturs.Artamonovs@xxxxxxxxxx>; Bimpikas, Vasileios > <Vasileios.Bimpikas@xxxxxxxxxx>; Gaskell, Oliver > <Oliver.Gaskell@xxxxxxxxxx> > Subject: Re: [PATCH v9 3/3] dt-bindings: input: Update dtbinding for adp5588 > > [External] > > 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://urldefense.com/v3/__https://elixir.bootlin.com/linux/v6.7- > rc8/source/Documentation/devicetree/bindings/submitting- > patches.rst*L18__;Iw!!A3Ni8CS0y2Y!_px- > SkCaAOezCa_s0eiP5BTliLmvyA110d4MXahf8mHkAXr0vJtpM0EXu7EsoG2jWg > 5qW9FgaYf2gCZf$ > > 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? The hardware was not changed, rather support was added for an already present functionality in regard to a new use case where the chip was being used purely for gpio. I will update the commit description to be more elaborate. > > 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. > Thank you for pointing the same out, I will remove it. > > 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? Apologies for the confusion, but the issue I'm wanting to address is the lack of support for a pure gpio mode. Since in this case, interrupts are not used, they are made optional, but in doing so, I also need to be careful of its requirements as a keypad, which it was originally structured as. As a result, there is an interdependency that was established between the keypad properties (prior to this as you can see, they were all required). This was followed by a requirement to make the interrupts required if the device was intended to be an interrupt controller in the last review (https://lore.kernel.org/all/Zq17uABHdNENnwVq@xxxxxxxxxx/) > > > + > > +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? I will remove the extra headers > Utsav
