On Wed, Jul 03, 2024 at 03:55:11PM +0000, Agarwal, Utsav wrote: > Hi Conor, > > Thank you for your feedback. > > -----Original Message----- > > From: Conor Dooley <conor@xxxxxxxxxx> > > Sent: Wednesday, July 3, 2024 4:20 PM > > To: Agarwal, Utsav <Utsav.Agarwal@xxxxxxxxxx> > > Cc: 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>; 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 v5 3/3] dt-bindings: input: Update dtbinding for adp5588 > > > > On Wed, Jul 03, 2024 at 11:58:16AM +0100, Utsav Agarwal via B4 Relay > > wrote: > > > From: Utsav Agarwal <utsav.agarwal@xxxxxxxxxx> > > > > > > Updating dt bindings for adp5588. Since the device can now function in a > > > purely gpio mode, the following keypad specific properties are now made > > > optional: > > > - interrupts > > > - keypad,num-rows > > > - keypad,num-columns > > > - linux,keymap > > > > > > However since 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. > > > > > > Signed-off-by: Utsav Agarwal <utsav.agarwal@xxxxxxxxxx> > > > --- > > > .../devicetree/bindings/input/adi,adp5588.yaml | 33 > > ++++++++++++++++++---- > > > 1 file changed, 28 insertions(+), 5 deletions(-) > > > > > > diff --git a/Documentation/devicetree/bindings/input/adi,adp5588.yaml > > b/Documentation/devicetree/bindings/input/adi,adp5588.yaml > > > index 26ea66834ae2..6c06464f822b 100644 > > > --- a/Documentation/devicetree/bindings/input/adi,adp5588.yaml > > > +++ b/Documentation/devicetree/bindings/input/adi,adp5588.yaml > > > @@ -49,7 +49,10 @@ 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 does not apply if > > > + keypad,num-rows or keypad,num-columns are not specified since the > > > + device then acts as gpio only, during which interrupts are not > > > + utilized. > > > > > > '#interrupt-cells': > > > const: 2 > > > @@ -65,13 +68,15 @@ properties: > > > minItems: 1 > > > maxItems: 2 > > > > > > +dependencies: > > > + keypad,num-rows: ["keypad,num-columns"] > > > + keypad,num-cols: ["keypad,num-rows"] > > > + linux,keymap: ["keypad,num-rows"] > > > > Is what you've got here sufficient? Adding "keypad,num-rows" won't > > mandate "linux,keymap" which I think is wrong. I think all 3 entries > > here need to contain both of the other two. > > > > Ah, I can see the issue, thank you for pointing it out - I will be correcting that. > > > > + interrupts: ["linux,keymap"] > > > > I still don't understand why interrupts are only allowed when the keymap > > is present. I'd cover the interrupts with something like > > > > if: > > required: > > - linux,keymap > > then: > > required: > > - interrupts > > > > so that interrupts can be used while not in keypad mode. Unless of > > course there's something (unmentioned in this patch) that prevents that. > > In case when the device is not in keypad mode, i.e, is purely using gpio - it doesn't trigger the interrupt. > Due to this, I had restricted the same to keypad mode only(as a requirement). This was mentioned > here: > https://lore.kernel.org/all/d4661ddc1d253678fd62be4c7e19eb0cff4174f6.camel@xxxxxxxxx/ This says "not required", not "not functional". How come generating interrupts becomes impossible when not in keypad mode? That's what needs to be explained. Thanks, Conor.
Attachment:
signature.asc
Description: PGP signature
