diff mbox series

[v2,1/2] ARM: dts: rk3188: correct interrupt flags

Message ID 20200917185211.5483-1-krzk@kernel.org (mailing list archive)
State New, archived
Headers show
Series [v2,1/2] ARM: dts: rk3188: correct interrupt flags | expand

Commit Message

Krzysztof Kozlowski Sept. 17, 2020, 6:52 p.m. UTC
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 without affecting the code:
  ACTIVE_HIGH => IRQ_TYPE_NONE

Signed-off-by: Krzysztof Kozlowski <krzk@kernel.org>

---

Not tested on HW.

Changes since v1:
1. Correct title
---
 arch/arm/boot/dts/rk3188-bqedison2qc.dts | 3 ++-
 1 file changed, 2 insertions(+), 1 deletion(-)

Comments

Krzysztof Kozlowski Oct. 2, 2020, 4:11 p.m. UTC | #1
On Thu, Sep 17, 2020 at 08:52:10PM +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 without affecting the code:
>   ACTIVE_HIGH => IRQ_TYPE_NONE
> 
> Signed-off-by: Krzysztof Kozlowski <krzk@kernel.org>
> 
> ---
> 
> Not tested on HW.
> 
> Changes since v1:
> 1. Correct title
> ---
>  arch/arm/boot/dts/rk3188-bqedison2qc.dts | 3 ++-
>  1 file changed, 2 insertions(+), 1 deletion(-)

Hi,

Any comments/review/testing from Heiko or other Rockchip folks? Shall I
cc here someone?

Best regards,
Krzysztof
Heiko Stuebner Oct. 17, 2020, 12:08 p.m. UTC | #2
Hi,

Am Freitag, 2. Oktober 2020, 18:11:28 CEST schrieb Krzysztof Kozlowski:
> On Thu, Sep 17, 2020 at 08:52:10PM +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 without affecting the code:
> >   ACTIVE_HIGH => IRQ_TYPE_NONE
> > 
> > Signed-off-by: Krzysztof Kozlowski <krzk@kernel.org>
> > 
> > ---
> > 
> > Not tested on HW.
> > 
> > Changes since v1:
> > 1. Correct title
> > ---
> >  arch/arm/boot/dts/rk3188-bqedison2qc.dts | 3 ++-
> >  1 file changed, 2 insertions(+), 1 deletion(-)
> 
> Hi,
> 
> Any comments/review/testing from Heiko or other Rockchip folks? Shall I
> cc here someone?

I'm actually wondering about this ... I somehow remember writing a response,
but don't see it in my history - so it might have gotten lost before I
actually sent it.

I think the biggest issue I have is that none of that is tested on any
hardware and looking at other brcm wifi drivers in the kernel, the
interrupt polarity seems to be all over the place, some set it high,
some low and I even have seen edge triggers.

As all changes are in regard to (copied) brcm wifi node, it would be
really interesting to actually know what trigger is the right one.

I've Cc'ed Jagan who I think has worked on an affected board,
maybe he can check which trigger is correct.


Heiko
Krzysztof Kozlowski April 2, 2022, 4:49 p.m. UTC | #3
On Sat, 17 Oct 2020 at 14:08, Heiko Stübner <heiko@sntech.de> wrote:
>
> Hi,
>
> Am Freitag, 2. Oktober 2020, 18:11:28 CEST schrieb Krzysztof Kozlowski:
> > On Thu, Sep 17, 2020 at 08:52:10PM +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 without affecting the code:
> > >   ACTIVE_HIGH => IRQ_TYPE_NONE
> > >
> > > Signed-off-by: Krzysztof Kozlowski <krzk@kernel.org>
> > >
> > > ---
> > >
> > > Not tested on HW.
> > >
> > > Changes since v1:
> > > 1. Correct title
> > > ---
> > >  arch/arm/boot/dts/rk3188-bqedison2qc.dts | 3 ++-
> > >  1 file changed, 2 insertions(+), 1 deletion(-)
> >
> > Hi,
> >
> > Any comments/review/testing from Heiko or other Rockchip folks? Shall I
> > cc here someone?
>
> I'm actually wondering about this ... I somehow remember writing a response,
> but don't see it in my history - so it might have gotten lost before I
> actually sent it.
>
> I think the biggest issue I have is that none of that is tested on any
> hardware and looking at other brcm wifi drivers in the kernel, the
> interrupt polarity seems to be all over the place, some set it high,
> some low and I even have seen edge triggers.
>
> As all changes are in regard to (copied) brcm wifi node, it would be
> really interesting to actually know what trigger is the right one.
>
> I've Cc'ed Jagan who I think has worked on an affected board,
> maybe he can check which trigger is correct.

Hi Heiko,

Nothing moved here since 2020 and the DTSes still have incorrect flags
for interrupts.

I can rebase and resend (although maybe it still applies cleanly), but
more important - is someone going to take this patch? or test it?

Best regards,
Krzysztof
Heiko Stuebner April 4, 2022, 8:59 a.m. UTC | #4
On Thu, 17 Sep 2020 20:52:10 +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 without affecting the code:
>   ACTIVE_HIGH => IRQ_TYPE_NONE

Applied, thanks!

[1/2] ARM: dts: rk3188: correct interrupt flags
      commit: 7e5a7e39d767b60d8631792bd3d7820a6f4a43f8
[2/2] arm64: dts: rk3399: correct interrupt flags
      commit: 57f3b0bf5c346f73a848c3d74270b21ed110e530

Best regards,
diff mbox series

Patch

diff --git a/arch/arm/boot/dts/rk3188-bqedison2qc.dts b/arch/arm/boot/dts/rk3188-bqedison2qc.dts
index 66a0ff196eb1..b191347894f6 100644
--- a/arch/arm/boot/dts/rk3188-bqedison2qc.dts
+++ b/arch/arm/boot/dts/rk3188-bqedison2qc.dts
@@ -7,6 +7,7 @@ 
 /dts-v1/;
 #include <dt-bindings/i2c/i2c.h>
 #include <dt-bindings/input/input.h>
+#include <dt-bindings/interrupt-controller/irq.h>
 #include "rk3188.dtsi"
 
 / {
@@ -479,7 +480,7 @@ 
 		reg = <1>;
 		compatible = "brcm,bcm4329-fmac";
 		interrupt-parent = <&gpio3>;
-		interrupts = <RK_PD2 GPIO_ACTIVE_HIGH>;
+		interrupts = <RK_PD2 IRQ_TYPE_NONE>;
 		interrupt-names = "host-wake";
 		brcm,drive-strength = <5>;
 		pinctrl-names = "default";