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          | 3 +++
 1 file changed, 3 insertions(+)

diff --git a/Documentation/devicetree/bindings/input/gpio-matrix-keypad.yaml b/Documentation/devicetree/bindings/input/gpio-matrix-keypad.yaml
index 75975a1..b10da65 100644
--- a/Documentation/devicetree/bindings/input/gpio-matrix-keypad.yaml
+++ b/Documentation/devicetree/bindings/input/gpio-matrix-keypad.yaml
@@ -38,6 +38,9 @@ properties:
     type: boolean
     description: Do not enable autorepeat feature.
 
+  gpio-activelow:
+    type: boolean
+    description: The GPIOs are low active.
 
   debounce-delay-ms:
     description: Debounce interval in milliseconds.
-- 
2.39.5

On Tue, Jan 07, 2025 at 02:56:56PM +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.

Wasn't this added for ACPI or some platform-data users? It is not really
a correct reason to document something post-factum just because review
was skipped...

Best regards,
Krzysztof

On Tue, Jan 07, 2025 at 02:56:56PM +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          | 3 +++
>  1 file changed, 3 insertions(+)
> 
> diff --git a/Documentation/devicetree/bindings/input/gpio-matrix-keypad.yaml b/Documentation/devicetree/bindings/input/gpio-matrix-keypad.yaml
> index 75975a1..b10da65 100644
> --- a/Documentation/devicetree/bindings/input/gpio-matrix-keypad.yaml
> +++ b/Documentation/devicetree/bindings/input/gpio-matrix-keypad.yaml
> @@ -38,6 +38,9 @@ properties:
>      type: boolean
>      description: Do not enable autorepeat feature.
>  
> +  gpio-activelow:
> +    type: boolean
> +    description: The GPIOs are low active.

Ideally this should be defined correctly in the gpio properties. The 
problem is that does a 0 flag value mean active high or I forgot to 
define it. There's a similar issue in spi-controller.yaml. I *think* the 
problem is better here since this is an active low boolean rather than 
an active high boolean.

Of the users in the kernel tree, only 
arch/arm/boot/dts/ti/omap/am335x-guardian.dts got this right.

So I think we should mark this deprecated and put a note to use GPIO 
flags instead.

Rob

On Tue, Jan 07, 2025 at 01:27:01PM -0600, Rob Herring wrote:
> On Tue, Jan 07, 2025 at 02:56:56PM +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          | 3 +++
> >  1 file changed, 3 insertions(+)
> > 
> > diff --git a/Documentation/devicetree/bindings/input/gpio-matrix-keypad.yaml b/Documentation/devicetree/bindings/input/gpio-matrix-keypad.yaml
> > index 75975a1..b10da65 100644
> > --- a/Documentation/devicetree/bindings/input/gpio-matrix-keypad.yaml
> > +++ b/Documentation/devicetree/bindings/input/gpio-matrix-keypad.yaml
> > @@ -38,6 +38,9 @@ properties:
> >      type: boolean
> >      description: Do not enable autorepeat feature.
> >  
> > +  gpio-activelow:
> > +    type: boolean
> > +    description: The GPIOs are low active.
> 
> Ideally this should be defined correctly in the gpio properties. The 
> problem is that does a 0 flag value mean active high or I forgot to 
> define it. There's a similar issue in spi-controller.yaml. I *think* the 
> problem is better here since this is an active low boolean rather than 
> an active high boolean.
> 
> Of the users in the kernel tree, only 
> arch/arm/boot/dts/ti/omap/am335x-guardian.dts got this right.
> 
> So I think we should mark this deprecated and put a note to use GPIO 
> flags instead.

So is the proposal to force GPIO as active low if the property is
present and leave as is if it is missing? Because current driver
behavior is to force GPIOs as active high if the property is missing.

Also, what is the benefit from having property marked as deprecated vs
not documenting it in hopes that DTSes will fail validation and be
fixed?

Thanks.

-- 
Dmitry

On Wed, Jan 08, 2025 at 10:04:24PM -0800, Dmitry Torokhov wrote:
> On Tue, Jan 07, 2025 at 01:27:01PM -0600, Rob Herring wrote:
> > On Tue, Jan 07, 2025 at 02:56:56PM +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          | 3 +++
> > >  1 file changed, 3 insertions(+)
> > > 
> > > diff --git a/Documentation/devicetree/bindings/input/gpio-matrix-keypad.yaml b/Documentation/devicetree/bindings/input/gpio-matrix-keypad.yaml
> > > index 75975a1..b10da65 100644
> > > --- a/Documentation/devicetree/bindings/input/gpio-matrix-keypad.yaml
> > > +++ b/Documentation/devicetree/bindings/input/gpio-matrix-keypad.yaml
> > > @@ -38,6 +38,9 @@ properties:
> > >      type: boolean
> > >      description: Do not enable autorepeat feature.
> > >  
> > > +  gpio-activelow:
> > > +    type: boolean
> > > +    description: The GPIOs are low active.
> > 
> > Ideally this should be defined correctly in the gpio properties. The 
> > problem is that does a 0 flag value mean active high or I forgot to 
> > define it. There's a similar issue in spi-controller.yaml. I *think* the 
> > problem is better here since this is an active low boolean rather than 
> > an active high boolean.
> > 
> > Of the users in the kernel tree, only 
> > arch/arm/boot/dts/ti/omap/am335x-guardian.dts got this right.
> > 
> > So I think we should mark this deprecated and put a note to use GPIO 
> > flags instead.
> 
> So is the proposal to force GPIO as active low if the property is
> present and leave as is if it is missing? Because current driver
> behavior is to force GPIOs as active high if the property is missing.
> 

I do not touch the current implementation.
Currently if the property is set the GPIO's are toggled to active low or if
the property is missing to active high.

> Also, what is the benefit from having property marked as deprecated vs
> not documenting it in hopes that DTSes will fail validation and be
> fixed?

Good question?
The dt schema checker will complain since it is used in some dtb's
I do not like to see warnings

> 
> Thanks.
> 
> -- 
> Dmitry