1. Patchew
  2. linux
  3. [PATCH v5 0/7] Input: matrix-keypad: Various performance improvements
  4. View patch

[PATCH v5 4/7] dt-bindings: input: matrix_keypad - add missing property

Markus Burri posted 7 patches 2 months ago
There is a newer version of this series
[PATCH v5 4/7] dt-bindings: input: matrix_keypad - add missing property
Posted by Markus Burri 2 months ago 
The property is implemented in the driver but not described in dt-bindings.
Add missing property 'gpio-activelow' to DT schema.

Signed-off-by: Markus Burri <markus.burri@mt.com>

---
 .../devicetree/bindings/input/gpio-matrix-keypad.yaml       | 6 ++++++
 1 file changed, 6 insertions(+)

diff --git a/Documentation/devicetree/bindings/input/gpio-matrix-keypad.yaml b/Documentation/devicetree/bindings/input/gpio-matrix-keypad.yaml
index 0f348b9..9c91224 100644
--- a/Documentation/devicetree/bindings/input/gpio-matrix-keypad.yaml
+++ b/Documentation/devicetree/bindings/input/gpio-matrix-keypad.yaml
@@ -40,6 +40,12 @@ properties:
     type: boolean
     description: Do not enable autorepeat feature.
 
+  gpio-activelow:
+    type: boolean
+    deprecated: true
+    description:
+      The GPIOs are low active (deprecated).
+      Use the GPIO-flags in gpio controller instead of this property.
 
   debounce-delay-ms:
     description: Debounce interval in milliseconds.
-- 
2.39.5
Re: [PATCH v5 4/7] dt-bindings: input: matrix_keypad - add missing property
Posted by Manuel Traut 3 weeks, 1 day ago 
On Fri, Jan 10, 2025 at 06:49:03AM +0100, Markus Burri wrote:
> The property is implemented in the driver but not described in dt-bindings.
> Add missing property 'gpio-activelow' to DT schema.
> 
> Signed-off-by: Markus Burri <markus.burri@mt.com>

Reviewed-by: Manuel Traut <manuel.traut@mt.com>

> ---
>  .../devicetree/bindings/input/gpio-matrix-keypad.yaml       | 6 ++++++
>  1 file changed, 6 insertions(+)
> 
> diff --git a/Documentation/devicetree/bindings/input/gpio-matrix-keypad.yaml b/Documentation/devicetree/bindings/input/gpio-matrix-keypad.yaml
> index 0f348b9..9c91224 100644
> --- a/Documentation/devicetree/bindings/input/gpio-matrix-keypad.yaml
> +++ b/Documentation/devicetree/bindings/input/gpio-matrix-keypad.yaml
> @@ -40,6 +40,12 @@ properties:
>      type: boolean
>      description: Do not enable autorepeat feature.
>  
> +  gpio-activelow:
> +    type: boolean
> +    deprecated: true
> +    description:
> +      The GPIOs are low active (deprecated).
> +      Use the GPIO-flags in gpio controller instead of this property.
>  
>    debounce-delay-ms:
>      description: Debounce interval in milliseconds.
> -- 
> 2.39.5
>
Re: [PATCH v5 4/7] dt-bindings: input: matrix_keypad - add missing property
Posted by Dmitry Torokhov 2 weeks, 2 days ago 
On Wed, Feb 19, 2025 at 05:42:00PM +0100, Manuel Traut wrote:
> On Fri, Jan 10, 2025 at 06:49:03AM +0100, Markus Burri wrote:
> > The property is implemented in the driver but not described in dt-bindings.
> > Add missing property 'gpio-activelow' to DT schema.
> > 
> > Signed-off-by: Markus Burri <markus.burri@mt.com>
> 
> Reviewed-by: Manuel Traut <manuel.traut@mt.com>
> 
> > ---
> >  .../devicetree/bindings/input/gpio-matrix-keypad.yaml       | 6 ++++++
> >  1 file changed, 6 insertions(+)
> > 
> > diff --git a/Documentation/devicetree/bindings/input/gpio-matrix-keypad.yaml b/Documentation/devicetree/bindings/input/gpio-matrix-keypad.yaml
> > index 0f348b9..9c91224 100644
> > --- a/Documentation/devicetree/bindings/input/gpio-matrix-keypad.yaml
> > +++ b/Documentation/devicetree/bindings/input/gpio-matrix-keypad.yaml
> > @@ -40,6 +40,12 @@ properties:
> >      type: boolean
> >      description: Do not enable autorepeat feature.
> >  
> > +  gpio-activelow:
> > +    type: boolean
> > +    deprecated: true
> > +    description:
> > +      The GPIOs are low active (deprecated).
> > +      Use the GPIO-flags in gpio controller instead of this property.

No, we unfortunately can not rely on GPIO-flags. This is not how driver
works: current driver behavior is to force GPIOs as active high if the
property is missing and ignore polarity specified in GPIO property.

Thanks.

-- 
Dmitry
Re: [PATCH v5 4/7] dt-bindings: input: matrix_keypad - add missing property
Posted by Markus Burri 2 weeks, 2 days ago 
On Mon, Feb 24, 2025 at 10:46:09PM -0800, Dmitry Torokhov wrote:
> On Wed, Feb 19, 2025 at 05:42:00PM +0100, Manuel Traut wrote:
> > On Fri, Jan 10, 2025 at 06:49:03AM +0100, Markus Burri wrote:
> > > The property is implemented in the driver but not described in dt-bindings.
> > > Add missing property 'gpio-activelow' to DT schema.
> > > 
> > > Signed-off-by: Markus Burri <markus.burri@mt.com>
> > 
> > Reviewed-by: Manuel Traut <manuel.traut@mt.com>
> > 
> > > ---
> > >  .../devicetree/bindings/input/gpio-matrix-keypad.yaml       | 6 ++++++
> > >  1 file changed, 6 insertions(+)
> > > 
> > > diff --git a/Documentation/devicetree/bindings/input/gpio-matrix-keypad.yaml b/Documentation/devicetree/bindings/input/gpio-matrix-keypad.yaml
> > > index 0f348b9..9c91224 100644
> > > --- a/Documentation/devicetree/bindings/input/gpio-matrix-keypad.yaml
> > > +++ b/Documentation/devicetree/bindings/input/gpio-matrix-keypad.yaml
> > > @@ -40,6 +40,12 @@ properties:
> > >      type: boolean
> > >      description: Do not enable autorepeat feature.
> > >  
> > > +  gpio-activelow:
> > > +    type: boolean
> > > +    deprecated: true
> > > +    description:
> > > +      The GPIOs are low active (deprecated).
> > > +      Use the GPIO-flags in gpio controller instead of this property.
> 
> No, we unfortunately can not rely on GPIO-flags. This is not how driver
> works: current driver behavior is to force GPIOs as active high if the
> property is missing and ignore polarity specified in GPIO property.

So you mean this property is still valid?
Current implementation toggle the GPIO flags to low active if the property and the GPIO flags are different.
So in case of missing property (false) the settings from GPIO flags will be used.
In case of gpio-activelow (true) GPIO flags are set to low active.

So I will remove the deprecated flag and change the description to:
"Force GPIO flags to active low. Use GPIO flags if property is missing."
OK?

> 
> Thanks.
> 
> -- 
> Dmitry
Re: [PATCH v5 4/7] dt-bindings: input: matrix_keypad - add missing property
Posted by Dmitry Torokhov 2 weeks, 2 days ago 
On Tue, Feb 25, 2025 at 11:16:07AM +0100, Markus Burri wrote:
> On Mon, Feb 24, 2025 at 10:46:09PM -0800, Dmitry Torokhov wrote:
> > On Wed, Feb 19, 2025 at 05:42:00PM +0100, Manuel Traut wrote:
> > > On Fri, Jan 10, 2025 at 06:49:03AM +0100, Markus Burri wrote:
> > > > The property is implemented in the driver but not described in dt-bindings.
> > > > Add missing property 'gpio-activelow' to DT schema.
> > > > 
> > > > Signed-off-by: Markus Burri <markus.burri@mt.com>
> > > 
> > > Reviewed-by: Manuel Traut <manuel.traut@mt.com>
> > > 
> > > > ---
> > > >  .../devicetree/bindings/input/gpio-matrix-keypad.yaml       | 6 ++++++
> > > >  1 file changed, 6 insertions(+)
> > > > 
> > > > diff --git a/Documentation/devicetree/bindings/input/gpio-matrix-keypad.yaml b/Documentation/devicetree/bindings/input/gpio-matrix-keypad.yaml
> > > > index 0f348b9..9c91224 100644
> > > > --- a/Documentation/devicetree/bindings/input/gpio-matrix-keypad.yaml
> > > > +++ b/Documentation/devicetree/bindings/input/gpio-matrix-keypad.yaml
> > > > @@ -40,6 +40,12 @@ properties:
> > > >      type: boolean
> > > >      description: Do not enable autorepeat feature.
> > > >  
> > > > +  gpio-activelow:
> > > > +    type: boolean
> > > > +    deprecated: true
> > > > +    description:
> > > > +      The GPIOs are low active (deprecated).
> > > > +      Use the GPIO-flags in gpio controller instead of this property.
> > 
> > No, we unfortunately can not rely on GPIO-flags. This is not how driver
> > works: current driver behavior is to force GPIOs as active high if the
> > property is missing and ignore polarity specified in GPIO property.
> 
> So you mean this property is still valid?
> Current implementation toggle the GPIO flags to low active if the property and the GPIO flags are different.
> So in case of missing property (false) the settings from GPIO flags will be used.
> In case of gpio-activelow (true) GPIO flags are set to low active.

The driver does:

	active_low = device_property_read_bool(&pdev->dev, "gpio-activelow");

	...

		if (active_low ^ gpiod_is_active_low(keypad->col_gpios[i]))
			gpiod_toggle_active_low(keypad->col_gpios[i]);

f the property is present active_low is true. If it is absent active_low
is false. And the code below, if GPIO polarity setting from DT disagree
with active_low value it will flip GPIO polarity.

You probably think that gpiod_toggle_active_low() sets line as active
low, when in fact it flips to opposite setting:

void gpiod_toggle_active_low(struct gpio_desc *desc)
{
	VALIDATE_DESC_VOID(desc);
	change_bit(FLAG_ACTIVE_LOW, &desc->flags);

	^^^ flip, not set or clear.

	gpiod_line_state_notify(desc, GPIO_V2_LINE_CHANGED_CONFIG);
}

> 
> So I will remove the deprecated flag and change the description to:
> "Force GPIO flags to active low. Use GPIO flags if property is missing."
> OK?

No. It should say:

"Force GPIO polarity to active low. In the absence of this property
GPIOs are treated as active high."

Thanks.

-- 
Dmitry
Re: [PATCH v5 4/7] dt-bindings: input: matrix_keypad - add missing property
Posted by Markus Burri 2 weeks, 1 day ago 
On Tue, Feb 25, 2025 at 09:43:21AM -0800, Dmitry Torokhov wrote:
> On Tue, Feb 25, 2025 at 11:16:07AM +0100, Markus Burri wrote:
> > On Mon, Feb 24, 2025 at 10:46:09PM -0800, Dmitry Torokhov wrote:
> > > On Wed, Feb 19, 2025 at 05:42:00PM +0100, Manuel Traut wrote:
> > > > On Fri, Jan 10, 2025 at 06:49:03AM +0100, Markus Burri wrote:
> > > > > The property is implemented in the driver but not described in dt-bindings.
> > > > > Add missing property 'gpio-activelow' to DT schema.
> > > > > 
> > > > > Signed-off-by: Markus Burri <markus.burri@mt.com>
> > > > 
> > > > Reviewed-by: Manuel Traut <manuel.traut@mt.com>
> > > > 
> > > > > ---
> > > > >  .../devicetree/bindings/input/gpio-matrix-keypad.yaml       | 6 ++++++
> > > > >  1 file changed, 6 insertions(+)
> > > > > 
> > > > > diff --git a/Documentation/devicetree/bindings/input/gpio-matrix-keypad.yaml b/Documentation/devicetree/bindings/input/gpio-matrix-keypad.yaml
> > > > > index 0f348b9..9c91224 100644
> > > > > --- a/Documentation/devicetree/bindings/input/gpio-matrix-keypad.yaml
> > > > > +++ b/Documentation/devicetree/bindings/input/gpio-matrix-keypad.yaml
> > > > > @@ -40,6 +40,12 @@ properties:
> > > > >      type: boolean
> > > > >      description: Do not enable autorepeat feature.
> > > > >  
> > > > > +  gpio-activelow:
> > > > > +    type: boolean
> > > > > +    deprecated: true
> > > > > +    description:
> > > > > +      The GPIOs are low active (deprecated).
> > > > > +      Use the GPIO-flags in gpio controller instead of this property.
> > > 
> > > No, we unfortunately can not rely on GPIO-flags. This is not how driver
> > > works: current driver behavior is to force GPIOs as active high if the
> > > property is missing and ignore polarity specified in GPIO property.
> > 
> > So you mean this property is still valid?
> > Current implementation toggle the GPIO flags to low active if the property and the GPIO flags are different.
> > So in case of missing property (false) the settings from GPIO flags will be used.
> > In case of gpio-activelow (true) GPIO flags are set to low active.
> 
> The driver does:
> 
> 	active_low = device_property_read_bool(&pdev->dev, "gpio-activelow");
> 
> 	...
> 
> 		if (active_low ^ gpiod_is_active_low(keypad->col_gpios[i]))
> 			gpiod_toggle_active_low(keypad->col_gpios[i]);
> 
> f the property is present active_low is true. If it is absent active_low
> is false. And the code below, if GPIO polarity setting from DT disagree
> with active_low value it will flip GPIO polarity.
> 
> You probably think that gpiod_toggle_active_low() sets line as active
> low, when in fact it flips to opposite setting:

Yes exactly my fault. Thanks for the hint!
> 
> void gpiod_toggle_active_low(struct gpio_desc *desc)
> {
> 	VALIDATE_DESC_VOID(desc);
> 	change_bit(FLAG_ACTIVE_LOW, &desc->flags);
> 
> 	^^^ flip, not set or clear.
> 
> 	gpiod_line_state_notify(desc, GPIO_V2_LINE_CHANGED_CONFIG);
> }
> 
> > 
> > So I will remove the deprecated flag and change the description to:
> > "Force GPIO flags to active low. Use GPIO flags if property is missing."
> > OK?
> 
> No. It should say:
> 
> "Force GPIO polarity to active low. In the absence of this property
> GPIOs are treated as active high."
I will send a new version of the patch

> 
> Thanks.
> 
> -- 
> Dmitry
Re: [PATCH v5 4/7] dt-bindings: input: matrix_keypad - add missing property
Posted by Rob Herring (Arm) 1 month, 4 weeks ago 
On Fri, 10 Jan 2025 06:49:03 +0100, Markus Burri wrote:
> The property is implemented in the driver but not described in dt-bindings.
> Add missing property 'gpio-activelow' to DT schema.
> 
> Signed-off-by: Markus Burri <markus.burri@mt.com>
> 
> ---
>  .../devicetree/bindings/input/gpio-matrix-keypad.yaml       | 6 ++++++
>  1 file changed, 6 insertions(+)
> 

Reviewed-by: Rob Herring (Arm) <robh@kernel.org>

Patchew 2.1-devel-b67e4c2

© 2016 - 2025 Red Hat, Inc.