2020-09-08 20:13:15

by Krzysztof Kozlowski

[permalink] [raw]
Subject: [PATCH] dt-bindings: mfd: Correct interrupt flags in examples

GPIO_ACTIVE_x flags are not correct in the context of interrupt flags.
These are simple defines so they could be used in DTS but they will not
have the same meaning:
1. GPIO_ACTIVE_HIGH = 0 = IRQ_TYPE_NONE
2. GPIO_ACTIVE_LOW = 1 = IRQ_TYPE_EDGE_RISING

Correct the interrupt flags, assuming the author of the code wanted some
logical behavior behind the name "ACTIVE_xxx", this is:
ACTIVE_LOW => IRQ_TYPE_LEVEL_LOW

Signed-off-by: Krzysztof Kozlowski <[email protected]>
---
Documentation/devicetree/bindings/mfd/act8945a.txt | 2 +-
Documentation/devicetree/bindings/mfd/gateworks-gsc.yaml | 3 ++-
Documentation/devicetree/bindings/mfd/rohm,bd70528-pmic.txt | 2 +-
3 files changed, 4 insertions(+), 3 deletions(-)

diff --git a/Documentation/devicetree/bindings/mfd/act8945a.txt b/Documentation/devicetree/bindings/mfd/act8945a.txt
index e6f168db6c72..5ca75d888b4a 100644
--- a/Documentation/devicetree/bindings/mfd/act8945a.txt
+++ b/Documentation/devicetree/bindings/mfd/act8945a.txt
@@ -71,7 +71,7 @@ Example:
pinctrl-names = "default";
pinctrl-0 = <&pinctrl_charger_chglev &pinctrl_charger_lbo &pinctrl_charger_irq>;
interrupt-parent = <&pioA>;
- interrupts = <45 GPIO_ACTIVE_LOW>;
+ interrupts = <45 IRQ_TYPE_LEVEL_LOW>;

active-semi,chglev-gpios = <&pioA 12 GPIO_ACTIVE_HIGH>;
active-semi,lbo-gpios = <&pioA 72 GPIO_ACTIVE_LOW>;
diff --git a/Documentation/devicetree/bindings/mfd/gateworks-gsc.yaml b/Documentation/devicetree/bindings/mfd/gateworks-gsc.yaml
index 9b6eb50606e8..e541eb0d02ed 100644
--- a/Documentation/devicetree/bindings/mfd/gateworks-gsc.yaml
+++ b/Documentation/devicetree/bindings/mfd/gateworks-gsc.yaml
@@ -147,6 +147,7 @@ required:
examples:
- |
#include <dt-bindings/gpio/gpio.h>
+ #include <dt-bindings/interrupt-controller/irq.h>
i2c {
#address-cells = <1>;
#size-cells = <0>;
@@ -155,7 +156,7 @@ examples:
compatible = "gw,gsc";
reg = <0x20>;
interrupt-parent = <&gpio1>;
- interrupts = <4 GPIO_ACTIVE_LOW>;
+ interrupts = <4 IRQ_TYPE_LEVEL_LOW>;
interrupt-controller;
#interrupt-cells = <1>;
#address-cells = <1>;
diff --git a/Documentation/devicetree/bindings/mfd/rohm,bd70528-pmic.txt b/Documentation/devicetree/bindings/mfd/rohm,bd70528-pmic.txt
index c3c02ce73cde..386eec06cf08 100644
--- a/Documentation/devicetree/bindings/mfd/rohm,bd70528-pmic.txt
+++ b/Documentation/devicetree/bindings/mfd/rohm,bd70528-pmic.txt
@@ -39,7 +39,7 @@ pmic: pmic@4b {
compatible = "rohm,bd70528";
reg = <0x4b>;
interrupt-parent = <&gpio1>;
- interrupts = <29 GPIO_ACTIVE_LOW>;
+ interrupts = <29 IRQ_TYPE_LEVEL_LOW>;
clocks = <&osc 0>;
#clock-cells = <0>;
clock-output-names = "bd70528-32k-out";
--
2.17.1


2020-09-09 06:31:11

by Lee Jones

[permalink] [raw]
Subject: Re: [PATCH] dt-bindings: mfd: Correct interrupt flags in examples

On Tue, 08 Sep 2020, Krzysztof Kozlowski wrote:

> GPIO_ACTIVE_x flags are not correct in the context of interrupt flags.
> These are simple defines so they could be used in DTS but they will not
> have the same meaning:
> 1. GPIO_ACTIVE_HIGH = 0 = IRQ_TYPE_NONE
> 2. GPIO_ACTIVE_LOW = 1 = IRQ_TYPE_EDGE_RISING
>
> Correct the interrupt flags, assuming the author of the code wanted some
> logical behavior behind the name "ACTIVE_xxx", this is:
> ACTIVE_LOW => IRQ_TYPE_LEVEL_LOW
>
> Signed-off-by: Krzysztof Kozlowski <[email protected]>
> ---
> Documentation/devicetree/bindings/mfd/act8945a.txt | 2 +-
> Documentation/devicetree/bindings/mfd/gateworks-gsc.yaml | 3 ++-
> Documentation/devicetree/bindings/mfd/rohm,bd70528-pmic.txt | 2 +-
> 3 files changed, 4 insertions(+), 3 deletions(-)

Looks good, but I'd like the author to review.

--
Lee Jones [李琼斯]
Senior Technical Lead - Developer Services
Linaro.org │ Open source software for Arm SoCs
Follow Linaro: Facebook | Twitter | Blog

2020-09-09 06:32:35

by Matti Vaittinen

[permalink] [raw]
Subject: Re: [PATCH] dt-bindings: mfd: Correct interrupt flags in examples


On Tue, 2020-09-08 at 16:59 +0200, Krzysztof Kozlowski wrote:
> GPIO_ACTIVE_x flags are not correct in the context of interrupt
> flags.
> These are simple defines so they could be used in DTS but they will
> not
> have the same meaning:
> 1. GPIO_ACTIVE_HIGH = 0 = IRQ_TYPE_NONE
> 2. GPIO_ACTIVE_LOW = 1 = IRQ_TYPE_EDGE_RISING
>
> Correct the interrupt flags, assuming the author of the code wanted
> some
> logical behavior behind the name "ACTIVE_xxx", this is:
> ACTIVE_LOW => IRQ_TYPE_LEVEL_LOW
>
> Signed-off-by: Krzysztof Kozlowski <[email protected]>

For BD70528:
Acked-By: Matti Vaittinen <[email protected]>

> ---
> Documentation/devicetree/bindings/mfd/act8945a.txt | 2 +-
> Documentation/devicetree/bindings/mfd/gateworks-gsc.yaml | 3 ++-
> Documentation/devicetree/bindings/mfd/rohm,bd70528-pmic.txt | 2 +-
> 3 files changed, 4 insertions(+), 3 deletions(-)
>
>
> diff --git a/Documentation/devicetree/bindings/mfd/rohm,bd70528-
> pmic.txt b/Documentation/devicetree/bindings/mfd/rohm,bd70528-
> pmic.txt
> index c3c02ce73cde..386eec06cf08 100644
> --- a/Documentation/devicetree/bindings/mfd/rohm,bd70528-pmic.txt
> +++ b/Documentation/devicetree/bindings/mfd/rohm,bd70528-pmic.txt
> @@ -39,7 +39,7 @@ pmic: pmic@4b {
> compatible = "rohm,bd70528";
> reg = <0x4b>;
> interrupt-parent = <&gpio1>;
> - interrupts = <29 GPIO_ACTIVE_LOW>;
> + interrupts = <29 IRQ_TYPE_LEVEL_LOW>;

This is how it should have been from the beginning :) Thanks!

Br,
Matti

--
Matti Vaittinen, Linux device drivers
ROHM Semiconductors, Finland
SWDC
Kiviharjunlenkki 1E
90220 OULU
FINLAND

~~~ "I don't think so," said Rene Descartes. Just then he vanished ~~~

Simon says - in Latin please.
"non cogito me" dixit Rene Descarte, deinde evanescavit

(Thanks for the translation Simon)

2020-09-09 08:20:20

by Krzysztof Kozlowski

[permalink] [raw]
Subject: Re: [PATCH] dt-bindings: mfd: Correct interrupt flags in examples

On Wed, Sep 09, 2020 at 06:30:44AM +0000, Vaittinen, Matti wrote:
>
> On Tue, 2020-09-08 at 16:59 +0200, Krzysztof Kozlowski wrote:
> > GPIO_ACTIVE_x flags are not correct in the context of interrupt
> > flags.
> > These are simple defines so they could be used in DTS but they will
> > not
> > have the same meaning:
> > 1. GPIO_ACTIVE_HIGH = 0 = IRQ_TYPE_NONE
> > 2. GPIO_ACTIVE_LOW = 1 = IRQ_TYPE_EDGE_RISING
> >
> > Correct the interrupt flags, assuming the author of the code wanted
> > some
> > logical behavior behind the name "ACTIVE_xxx", this is:
> > ACTIVE_LOW => IRQ_TYPE_LEVEL_LOW
> >
> > Signed-off-by: Krzysztof Kozlowski <[email protected]>
>
> For BD70528:
> Acked-By: Matti Vaittinen <[email protected]>
>
> > ---
> > Documentation/devicetree/bindings/mfd/act8945a.txt | 2 +-
> > Documentation/devicetree/bindings/mfd/gateworks-gsc.yaml | 3 ++-
> > Documentation/devicetree/bindings/mfd/rohm,bd70528-pmic.txt | 2 +-
> > 3 files changed, 4 insertions(+), 3 deletions(-)
> >
> >
> > diff --git a/Documentation/devicetree/bindings/mfd/rohm,bd70528-
> > pmic.txt b/Documentation/devicetree/bindings/mfd/rohm,bd70528-
> > pmic.txt
> > index c3c02ce73cde..386eec06cf08 100644
> > --- a/Documentation/devicetree/bindings/mfd/rohm,bd70528-pmic.txt
> > +++ b/Documentation/devicetree/bindings/mfd/rohm,bd70528-pmic.txt
> > @@ -39,7 +39,7 @@ pmic: pmic@4b {
> > compatible = "rohm,bd70528";
> > reg = <0x4b>;
> > interrupt-parent = <&gpio1>;
> > - interrupts = <29 GPIO_ACTIVE_LOW>;
> > + interrupts = <29 IRQ_TYPE_LEVEL_LOW>;
>
> This is how it should have been from the beginning :) Thanks!

I start to wonder now. It seems some boards do not configure a pull up
there, so IRQ_TYPE_LEVEL_LOW is wrong - causes the line to stay in low
state. But actually this maybe is a problem of missing pull up, not the
IRQ flag?

Best regards,
Krzysztof

2020-09-09 08:59:00

by Matti Vaittinen

[permalink] [raw]
Subject: Re: [PATCH] dt-bindings: mfd: Correct interrupt flags in examples

Hello Krzysztof,

On Wed, 2020-09-09 at 10:17 +0200, [email protected] wrote:
> On Wed, Sep 09, 2020 at 06:30:44AM +0000, Vaittinen, Matti wrote:
> > On Tue, 2020-09-08 at 16:59 +0200, Krzysztof Kozlowski wrote:
> > > GPIO_ACTIVE_x flags are not correct in the context of interrupt
> > > flags.
> > > These are simple defines so they could be used in DTS but they
> > > will
> > > not
> > > have the same meaning:
> > > 1. GPIO_ACTIVE_HIGH = 0 = IRQ_TYPE_NONE
> > > 2. GPIO_ACTIVE_LOW = 1 = IRQ_TYPE_EDGE_RISING
> > >
> > > Correct the interrupt flags, assuming the author of the code
> > > wanted
> > > some
> > > logical behavior behind the name "ACTIVE_xxx", this is:
> > > ACTIVE_LOW => IRQ_TYPE_LEVEL_LOW
> > >
> > > Signed-off-by: Krzysztof Kozlowski <[email protected]>
> >
> > For BD70528:
> > Acked-By: Matti Vaittinen <[email protected]>
> >
> > > ---
> > > Documentation/devicetree/bindings/mfd/act8945a.txt | 2
> > > +-
> > > Documentation/devicetree/bindings/mfd/gateworks-gsc.yaml | 3
> > > ++-
> > > Documentation/devicetree/bindings/mfd/rohm,bd70528-pmic.txt | 2
> > > +-
> > > 3 files changed, 4 insertions(+), 3 deletions(-)
> > >
> > >
> > > diff --git a/Documentation/devicetree/bindings/mfd/rohm,bd70528-
> > > pmic.txt b/Documentation/devicetree/bindings/mfd/rohm,bd70528-
> > > pmic.txt
> > > index c3c02ce73cde..386eec06cf08 100644
> > > --- a/Documentation/devicetree/bindings/mfd/rohm,bd70528-pmic.txt
> > > +++ b/Documentation/devicetree/bindings/mfd/rohm,bd70528-pmic.txt
> > > @@ -39,7 +39,7 @@ pmic: pmic@4b {
> > > compatible = "rohm,bd70528";
> > > reg = <0x4b>;
> > > interrupt-parent = <&gpio1>;
> > > - interrupts = <29 GPIO_ACTIVE_LOW>;
> > > + interrupts = <29 IRQ_TYPE_LEVEL_LOW>;
> >
> > This is how it should have been from the beginning :) Thanks!
>
> I start to wonder now. It seems some boards do not configure a pull
> up
> there, so IRQ_TYPE_LEVEL_LOW is wrong - causes the line to stay in
> low
> state. But actually this maybe is a problem of missing pull up, not
> the
> IRQ flag?

The BD70528 is designed so that it will use level active interrupts -
and line is pulled down when IRQ is active. Thus the example should
have IRQ_TYPE_LEVEL_LOW - and your fix is correct.

After that being said - I can't comment on actual board using BD70528
(or other ROHM ICs) - even less I can comment boards using other ICs.

After that being said - it's not a rare mistake to configure level
active IRQs to be triggered at edge - it actually works most of the
time - untill they deadlock at the race of generating new IRQ between
reading the status and acking the line... I've debugged way too many
such cases...

Anyways, for BD70528 DTS example your fix looks correct. Thanks.

>
> Best regards,
> Krzysztof
>

2020-09-09 09:21:56

by Krzysztof Kozlowski

[permalink] [raw]
Subject: Re: [PATCH] dt-bindings: mfd: Correct interrupt flags in examples

On Wed, Sep 09, 2020 at 08:57:36AM +0000, Vaittinen, Matti wrote:
> Hello Krzysztof,
>
> On Wed, 2020-09-09 at 10:17 +0200, [email protected] wrote:
> > On Wed, Sep 09, 2020 at 06:30:44AM +0000, Vaittinen, Matti wrote:
> > > On Tue, 2020-09-08 at 16:59 +0200, Krzysztof Kozlowski wrote:
> > > > GPIO_ACTIVE_x flags are not correct in the context of interrupt
> > > > flags.
> > > > These are simple defines so they could be used in DTS but they
> > > > will
> > > > not
> > > > have the same meaning:
> > > > 1. GPIO_ACTIVE_HIGH = 0 = IRQ_TYPE_NONE
> > > > 2. GPIO_ACTIVE_LOW = 1 = IRQ_TYPE_EDGE_RISING
> > > >
> > > > Correct the interrupt flags, assuming the author of the code
> > > > wanted
> > > > some
> > > > logical behavior behind the name "ACTIVE_xxx", this is:
> > > > ACTIVE_LOW => IRQ_TYPE_LEVEL_LOW
> > > >
> > > > Signed-off-by: Krzysztof Kozlowski <[email protected]>
> > >
> > > For BD70528:
> > > Acked-By: Matti Vaittinen <[email protected]>
> > >
> > > > ---
> > > > Documentation/devicetree/bindings/mfd/act8945a.txt | 2
> > > > +-
> > > > Documentation/devicetree/bindings/mfd/gateworks-gsc.yaml | 3
> > > > ++-
> > > > Documentation/devicetree/bindings/mfd/rohm,bd70528-pmic.txt | 2
> > > > +-
> > > > 3 files changed, 4 insertions(+), 3 deletions(-)
> > > >
> > > >
> > > > diff --git a/Documentation/devicetree/bindings/mfd/rohm,bd70528-
> > > > pmic.txt b/Documentation/devicetree/bindings/mfd/rohm,bd70528-
> > > > pmic.txt
> > > > index c3c02ce73cde..386eec06cf08 100644
> > > > --- a/Documentation/devicetree/bindings/mfd/rohm,bd70528-pmic.txt
> > > > +++ b/Documentation/devicetree/bindings/mfd/rohm,bd70528-pmic.txt
> > > > @@ -39,7 +39,7 @@ pmic: pmic@4b {
> > > > compatible = "rohm,bd70528";
> > > > reg = <0x4b>;
> > > > interrupt-parent = <&gpio1>;
> > > > - interrupts = <29 GPIO_ACTIVE_LOW>;
> > > > + interrupts = <29 IRQ_TYPE_LEVEL_LOW>;
> > >
> > > This is how it should have been from the beginning :) Thanks!
> >
> > I start to wonder now. It seems some boards do not configure a pull
> > up
> > there, so IRQ_TYPE_LEVEL_LOW is wrong - causes the line to stay in
> > low
> > state. But actually this maybe is a problem of missing pull up, not
> > the
> > IRQ flag?
>
> The BD70528 is designed so that it will use level active interrupts -
> and line is pulled down when IRQ is active. Thus the example should
> have IRQ_TYPE_LEVEL_LOW - and your fix is correct.
>
> After that being said - I can't comment on actual board using BD70528
> (or other ROHM ICs) - even less I can comment boards using other ICs.
>
> After that being said - it's not a rare mistake to configure level
> active IRQs to be triggered at edge - it actually works most of the
> time - untill they deadlock at the race of generating new IRQ between
> reading the status and acking the line... I've debugged way too many
> such cases...
>
> Anyways, for BD70528 DTS example your fix looks correct. Thanks.

Thanks. I found this error in multiple DTS files - most probably a copy
paste from example or from evalkit (e.g. imx8mm-evk.dts). The trouble is
that I don't have the schematics for them and at least in one hardware
(Variscite VAR-SOM-MX8M which I am using) it looks like logic got
reversed...

Best regards,
Krzysztof

2020-09-09 10:20:40

by Matti Vaittinen

[permalink] [raw]
Subject: Re: [PATCH] dt-bindings: mfd: Correct interrupt flags in examples

On Wed, 2020-09-09 at 11:20 +0200, [email protected] wrote:
> On Wed, Sep 09, 2020 at 08:57:36AM +0000, Vaittinen, Matti wrote:
> > Hello Krzysztof,
> >
> > On Wed, 2020-09-09 at 10:17 +0200, [email protected] wrote:
> > > On Wed, Sep 09, 2020 at 06:30:44AM +0000, Vaittinen, Matti wrote:
> > > > On Tue, 2020-09-08 at 16:59 +0200, Krzysztof Kozlowski wrote:
> > > > > GPIO_ACTIVE_x flags are not correct in the context of
> > > > > interrupt
> > > > > flags.
> > > > > These are simple defines so they could be used in DTS but
> > > > > they
> > > > > will
> > > > > not
> > > > > have the same meaning:
> > > > > 1. GPIO_ACTIVE_HIGH = 0 = IRQ_TYPE_NONE
> > > > > 2. GPIO_ACTIVE_LOW = 1 = IRQ_TYPE_EDGE_RISING
> > > > >
> > > > > Correct the interrupt flags, assuming the author of the code
> > > > > wanted
> > > > > some
> > > > > logical behavior behind the name "ACTIVE_xxx", this is:
> > > > > ACTIVE_LOW => IRQ_TYPE_LEVEL_LOW
> > > > >
> > > > > Signed-off-by: Krzysztof Kozlowski <[email protected]>
> > > >
> > > > For BD70528:
> > > > Acked-By: Matti Vaittinen <[email protected]>
> > > >
> > > > > ---
> > > > > Documentation/devicetree/bindings/mfd/act8945a.txt
> > > > > | 2
> > > > > +-
> > > > > Documentation/devicetree/bindings/mfd/gateworks-
> > > > > gsc.yaml | 3
> > > > > ++-
> > > > > Documentation/devicetree/bindings/mfd/rohm,bd70528-pmic.txt
> > > > > | 2
> > > > > +-
> > > > > 3 files changed, 4 insertions(+), 3 deletions(-)
> > > > >
> > > > >
> > > > > diff --git
> > > > > a/Documentation/devicetree/bindings/mfd/rohm,bd70528-
> > > > > pmic.txt
> > > > > b/Documentation/devicetree/bindings/mfd/rohm,bd70528-
> > > > > pmic.txt
> > > > > index c3c02ce73cde..386eec06cf08 100644
> > > > > --- a/Documentation/devicetree/bindings/mfd/rohm,bd70528-
> > > > > pmic.txt
> > > > > +++ b/Documentation/devicetree/bindings/mfd/rohm,bd70528-
> > > > > pmic.txt
> > > > > @@ -39,7 +39,7 @@ pmic: pmic@4b {
> > > > > compatible = "rohm,bd70528";
> > > > > reg = <0x4b>;
> > > > > interrupt-parent = <&gpio1>;
> > > > > - interrupts = <29 GPIO_ACTIVE_LOW>;
> > > > > + interrupts = <29 IRQ_TYPE_LEVEL_LOW>;
> > > >
> > > > This is how it should have been from the beginning :) Thanks!
> > >
> > > I start to wonder now. It seems some boards do not configure a
> > > pull
> > > up
> > > there, so IRQ_TYPE_LEVEL_LOW is wrong - causes the line to stay
> > > in
> > > low
> > > state. But actually this maybe is a problem of missing pull up,
> > > not
> > > the
> > > IRQ flag?
> >
> > The BD70528 is designed so that it will use level active interrupts
> > -
> > and line is pulled down when IRQ is active. Thus the example should
> > have IRQ_TYPE_LEVEL_LOW - and your fix is correct.
> >
> > After that being said - I can't comment on actual board using
> > BD70528
> > (or other ROHM ICs) - even less I can comment boards using other
> > ICs.
> >
> > After that being said - it's not a rare mistake to configure level
> > active IRQs to be triggered at edge - it actually works most of the
> > time - untill they deadlock at the race of generating new IRQ
> > between
> > reading the status and acking the line... I've debugged way too
> > many
> > such cases...
> >
> > Anyways, for BD70528 DTS example your fix looks correct. Thanks.
>
> Thanks. I found this error in multiple DTS files - most probably a
> copy
> paste from example or from evalkit (e.g. imx8mm-evk.dts). The trouble
> is
> that I don't have the schematics for them and at least in one
> hardware
> (Variscite VAR-SOM-MX8M which I am using) it looks like logic got
> reversed...

Hmm. According to the Variscite materials they use the BD71847AMWV -
not the BD70528. BD71847 does also have level active IORQs (active low)
- but misconfiguration may go unnoticed as (AFAIR) the BD71847 IRQs do
mostly inform conditions leading to reset by HW - the power button
short push being an exception. Thus configuring the IRQ to falling edge
is likely to work without deadlocking due to the race I mentioned.
(BD70528 would use IRQs for RTC so it would possibly be more
errorprone). Anyways the board dtses go beyond my area - but the
example fix for BD70528 definitely looks good :) Thanks again.



--
Matti Vaittinen, Linux device drivers
ROHM Semiconductors, Finland
SWDC
Kiviharjunlenkki 1E
90220 OULU
FINLAND

~~~ "I don't think so," said Rene Descartes. Just then he vanished ~~~

Simon says - in Latin please.
"non cogito me" dixit Rene Descarte, deinde evanescavit

(Thanks for the translation Simon)



2020-09-09 15:31:50

by Tim Harvey

[permalink] [raw]
Subject: Re: [PATCH] dt-bindings: mfd: Correct interrupt flags in examples

On Tue, Sep 8, 2020 at 7:59 AM Krzysztof Kozlowski <[email protected]> wrote:
>
> GPIO_ACTIVE_x flags are not correct in the context of interrupt flags.
> These are simple defines so they could be used in DTS but they will not
> have the same meaning:
> 1. GPIO_ACTIVE_HIGH = 0 = IRQ_TYPE_NONE
> 2. GPIO_ACTIVE_LOW = 1 = IRQ_TYPE_EDGE_RISING
>
> Correct the interrupt flags, assuming the author of the code wanted some
> logical behavior behind the name "ACTIVE_xxx", this is:
> ACTIVE_LOW => IRQ_TYPE_LEVEL_LOW
>
> Signed-off-by: Krzysztof Kozlowski <[email protected]>
> ---
> Documentation/devicetree/bindings/mfd/act8945a.txt | 2 +-
> Documentation/devicetree/bindings/mfd/gateworks-gsc.yaml | 3 ++-
> Documentation/devicetree/bindings/mfd/rohm,bd70528-pmic.txt | 2 +-
> 3 files changed, 4 insertions(+), 3 deletions(-)
>
> diff --git a/Documentation/devicetree/bindings/mfd/act8945a.txt b/Documentation/devicetree/bindings/mfd/act8945a.txt
> index e6f168db6c72..5ca75d888b4a 100644
> --- a/Documentation/devicetree/bindings/mfd/act8945a.txt
> +++ b/Documentation/devicetree/bindings/mfd/act8945a.txt
> @@ -71,7 +71,7 @@ Example:
> pinctrl-names = "default";
> pinctrl-0 = <&pinctrl_charger_chglev &pinctrl_charger_lbo &pinctrl_charger_irq>;
> interrupt-parent = <&pioA>;
> - interrupts = <45 GPIO_ACTIVE_LOW>;
> + interrupts = <45 IRQ_TYPE_LEVEL_LOW>;
>
> active-semi,chglev-gpios = <&pioA 12 GPIO_ACTIVE_HIGH>;
> active-semi,lbo-gpios = <&pioA 72 GPIO_ACTIVE_LOW>;
> diff --git a/Documentation/devicetree/bindings/mfd/gateworks-gsc.yaml b/Documentation/devicetree/bindings/mfd/gateworks-gsc.yaml
> index 9b6eb50606e8..e541eb0d02ed 100644
> --- a/Documentation/devicetree/bindings/mfd/gateworks-gsc.yaml
> +++ b/Documentation/devicetree/bindings/mfd/gateworks-gsc.yaml
> @@ -147,6 +147,7 @@ required:
> examples:
> - |
> #include <dt-bindings/gpio/gpio.h>
> + #include <dt-bindings/interrupt-controller/irq.h>
> i2c {
> #address-cells = <1>;
> #size-cells = <0>;
> @@ -155,7 +156,7 @@ examples:
> compatible = "gw,gsc";
> reg = <0x20>;
> interrupt-parent = <&gpio1>;
> - interrupts = <4 GPIO_ACTIVE_LOW>;
> + interrupts = <4 IRQ_TYPE_LEVEL_LOW>;
> interrupt-controller;
> #interrupt-cells = <1>;
> #address-cells = <1>;

For gateworks-gsc.yaml:
Acked-By: Tim Harvey <[email protected]>

Thanks for finding this!

Tim

2020-09-15 18:53:56

by Rob Herring (Arm)

[permalink] [raw]
Subject: Re: [PATCH] dt-bindings: mfd: Correct interrupt flags in examples

On Tue, 08 Sep 2020 16:59:00 +0200, Krzysztof Kozlowski wrote:
> GPIO_ACTIVE_x flags are not correct in the context of interrupt flags.
> These are simple defines so they could be used in DTS but they will not
> have the same meaning:
> 1. GPIO_ACTIVE_HIGH = 0 = IRQ_TYPE_NONE
> 2. GPIO_ACTIVE_LOW = 1 = IRQ_TYPE_EDGE_RISING
>
> Correct the interrupt flags, assuming the author of the code wanted some
> logical behavior behind the name "ACTIVE_xxx", this is:
> ACTIVE_LOW => IRQ_TYPE_LEVEL_LOW
>
> Signed-off-by: Krzysztof Kozlowski <[email protected]>
> ---
> Documentation/devicetree/bindings/mfd/act8945a.txt | 2 +-
> Documentation/devicetree/bindings/mfd/gateworks-gsc.yaml | 3 ++-
> Documentation/devicetree/bindings/mfd/rohm,bd70528-pmic.txt | 2 +-
> 3 files changed, 4 insertions(+), 3 deletions(-)
>

Acked-by: Rob Herring <[email protected]>