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] [day] [month] [year] [list]
Message-ID: 
 <SJ0PR03MB6343BC07CD7C73DB64097C8A9BDE2@SJ0PR03MB6343.namprd03.prod.outlook.com>
Date: Thu, 4 Jul 2024 08:47:16 +0000
From: "Agarwal, Utsav" <Utsav.Agarwal@...log.com>
To: Nuno Sá <noname.nuno@...il.com>,
        Conor Dooley
	<conor@...nel.org>
CC: "Hennerich, Michael" <Michael.Hennerich@...log.com>,
        Dmitry Torokhov
	<dmitry.torokhov@...il.com>,
        Rob Herring <robh@...nel.org>,
        Krzysztof
 Kozlowski <krzk+dt@...nel.org>,
        Conor Dooley <conor+dt@...nel.org>, "Sa,
 Nuno" <Nuno.Sa@...log.com>,
        "linux-input@...r.kernel.org"
	<linux-input@...r.kernel.org>,
        "devicetree@...r.kernel.org"
	<devicetree@...r.kernel.org>,
        "linux-kernel@...r.kernel.org"
	<linux-kernel@...r.kernel.org>,
        "Artamonovs, Arturs"
	<Arturs.Artamonovs@...log.com>,
        "Bimpikas, Vasileios"
	<Vasileios.Bimpikas@...log.com>,
        "Gaskell, Oliver"
	<Oliver.Gaskell@...log.com>
Subject: RE: [PATCH v5 3/3] dt-bindings: input: Update dtbinding for adp5588

Hi Conor,

> -----Original Message-----
> From: Nuno Sá <noname.nuno@...il.com>
> Sent: Thursday, July 4, 2024 8:19 AM
> To: Conor Dooley <conor@...nel.org>; Agarwal, Utsav
> <Utsav.Agarwal@...log.com>
> Cc: Hennerich, Michael <Michael.Hennerich@...log.com>; Dmitry Torokhov
> <dmitry.torokhov@...il.com>; Rob Herring <robh@...nel.org>; Krzysztof
> Kozlowski <krzk+dt@...nel.org>; Conor Dooley <conor+dt@...nel.org>; Sa,
> Nuno <Nuno.Sa@...log.com>; linux-input@...r.kernel.org;
> devicetree@...r.kernel.org; linux-kernel@...r.kernel.org; Artamonovs,
> Arturs <Arturs.Artamonovs@...log.com>; Bimpikas, Vasileios
> <Vasileios.Bimpikas@...log.com>; Gaskell, Oliver
> <Oliver.Gaskell@...log.com>
> Subject: Re: [PATCH v5 3/3] dt-bindings: input: Update dtbinding for adp5588
> 
> [External]
> 
> On Wed, 2024-07-03 at 16:57 +0100, Conor Dooley wrote:
> > 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@...nel.org>
> > > > Sent: Wednesday, July 3, 2024 4:20 PM
> > > > To: Agarwal, Utsav <Utsav.Agarwal@...log.com>
> > > > Cc: Hennerich, Michael <Michael.Hennerich@...log.com>; Dmitry
> Torokhov
> > > > <dmitry.torokhov@...il.com>; Rob Herring <robh@...nel.org>;
> Krzysztof
> > > > Kozlowski <krzk+dt@...nel.org>; Conor Dooley
> <conor+dt@...nel.org>; Sa,
> > > > Nuno <Nuno.Sa@...log.com>; linux-input@...r.kernel.org;
> > > > devicetree@...r.kernel.org; linux-kernel@...r.kernel.org;
> Artamonovs,
> > > > Arturs <Arturs.Artamonovs@...log.com>; Bimpikas, Vasileios
> > > > <Vasileios.Bimpikas@...log.com>; Gaskell, Oliver
> > > > <Oliver.Gaskell@...log.com>
> > > > 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@...log.com>
> > > > >
> > > > > 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@...log.com>
> > > > > ---
> > > > >  .../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://urldefense.com/v3/__https://lore.kernel.org/all/d4661ddc1d253678f
> d62be4c7e19eb0cff4174f6.camel@...il.com/__;!!A3Ni8CS0y2Y!8MV0PPuz1
> XKzaNus-NtAS3oXtrcgaUWHi4sf7zsPFS5zH_NUaNz-qBqUXPBTbTE09wRTVa-
> zYLaSdTte8ymnPyRB$
> >
> > 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.
> >
> 
> I should have read the patch before replying in v4. Yes, I agree with Conor
> that
> what we need is to make the interrupt __required__ when using the keypad.
> In the
> case we have gpios, it's optional but it does not mean we should remove it.
> One
> usecase would be to use gpios still as inputs through gpio-keys.
> 
> - Nuno Sá

I see the issue, thank you for highlighting the same. I will make the changes now.

Thanks,
Utsav

Powered by blists - more mailing lists

Powered by Openwall GNU/*/Linux Powered by OpenVZ