| Message ID | 20250107135659.185293-5-markus.burri@mt.com (mailing list archive) |
|---|---|
| State | Superseded |
| Headers | show |
| Series | Input: matrix-keypad: Various performance improvements | expand |
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 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 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.
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
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.
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(+)