Hi Connor, > -----Original Message----- > From: Dmitry Torokhov <dmitry.torokhov@xxxxxxxxx> > Sent: Monday, July 1, 2024 6:46 PM > To: Conor Dooley <conor@xxxxxxxxxx> > Cc: Agarwal, Utsav <Utsav.Agarwal@xxxxxxxxxx>; Hennerich, Michael > <Michael.Hennerich@xxxxxxxxxx>; 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 v4 2/2] dt-bindings: input: Update dtbinding for adp5588 > > [External] > > On Mon, Jul 01, 2024 at 04:46:12PM +0100, Conor Dooley wrote: > > On Mon, Jul 01, 2024 at 04:04:51PM +0100, Utsav Agarwal via B4 Relay > wrote: > > > From: Utsav Agarwal <utsav.agarwal@xxxxxxxxxx> > > > > > > Updating dt bindings for adp5588. Following properties are now made > > > optional: > > > - interrupts > > > - keypad,num-rows > > > - keypad,num-columns > > > - linux,keymap > > > The proposed new property "gpio-only" has been added as an optional > > > property with an additional example. > > > > I can see that as it is clear in the diff, but this doesn't explain why, > > which is what you need to do in your commit message. > > I will add more description to this commit message for context. > > > > > > Signed-off-by: Utsav Agarwal <utsav.agarwal@xxxxxxxxxx> > > > --- > > > .../devicetree/bindings/input/adi,adp5588.yaml | 28 > ++++++++++++++++++---- > > > 1 file changed, 24 insertions(+), 4 deletions(-) > > > > > > diff --git a/Documentation/devicetree/bindings/input/adi,adp5588.yaml > b/Documentation/devicetree/bindings/input/adi,adp5588.yaml > > > index 26ea66834ae2..158fbf02cc16 100644 > > > --- a/Documentation/devicetree/bindings/input/adi,adp5588.yaml > > > +++ b/Documentation/devicetree/bindings/input/adi,adp5588.yaml > > > @@ -46,6 +46,11 @@ properties: > > > '#gpio-cells': > > > const: 2 > > > > > > + gpio-only: > > > + description: > > > + This property applies if keypad,num-rows, keypad,num-columns and > > > + linux,keypad are not specified. All keys will be marked as gpio. > > > > Why is a property required for this? Is the absence of the 3 keypad > > properties not sufficient to determine that you're in this mode? > > Yes, I think it should be enough. The idea behind introducing a new property was to simplify the usage in addition to making it easier to document a pure gpio mode being supported. Would it still be better to remove this? > > > > > > > > interrupt-controller: > > > description: > > > This property applies if either keypad,num-rows lower than 8 or > > > @@ -68,10 +73,6 @@ properties: > > > required: > > > - compatible > > > - reg > > > - - interrupts > > > > I don't understand why interrupts is no longer required. > > I think it should be possible to use this chip as a GPIO controller but > not an interrupt controller, in which case one does not have to wire up > the interrupt line from it. However this requires much more elaborate > binding description (i.e. no keys and no "interrupt-controller" > property). I will add a more detailed description in the binding. > > Thanks. > > -- > Dmitry Thanks, Utsav
