[PATCH v2 1/2] Revert "ARM: dts: armada-38x: Fix compatible string for gpios"

[PATCH v2 1/2] Revert "ARM: dts: armada-38x: Fix compatible string for gpios"

[Klaus Kudielka]

This reverts commit c4de4667f15d04ef5920bacf41e514ec7d1ef03d, which causes
a regression on Turris Omnia (Armada 385): GPIO interrupts cease to work,
ending up in the DSA switch being non-functional.

The blamed commit is incorrect in the first place:
If compatible = "marvell,armadaxp-gpio", the second (address, size) pair
of the reg property must to point to the per-CPU interrupt registers
<0x18800 0x30> / <0x18840 0x30>, and not to the blink enable registers
<0x181c0 0x08> / <0x181c8 0x08>.

But even fixing that leaves the GPIO interrupts broken on the Omnia.

Furthermore:
Commit 5f79c651e81e explains very well, why the gpio-mvebu driver does not
work reliably with per-CPU interrupts.
Commit 988c8c0cd04d deprecates compatible = marvell,armadaxp-gpio for this
reason.

Fixes: c4de4667f15d ("ARM: dts: armada-38x: Fix compatible string for gpios")
Reported-by: Klaus Kudielka <klaus.kudielka@gmail.com>
Link: https://lore.kernel.org/r/f24474e70c1a4e9692bd596ef6d97ceda9511245.camel@gmail.com/

Signed-off-by: Klaus Kudielka <klaus.kudielka@gmail.com>
---
v1->v2: Be more explicit about the regression, and about the reg property.

 arch/arm/boot/dts/armada-38x.dtsi | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/arch/arm/boot/dts/armada-38x.dtsi b/arch/arm/boot/dts/armada-38x.dtsi
index 12933eff41..446861b6b1 100644
--- a/arch/arm/boot/dts/armada-38x.dtsi
+++ b/arch/arm/boot/dts/armada-38x.dtsi
@@ -304,7 +304,7 @@ spdif_pins: spdif-pins {
 			};
 
 			gpio0: gpio@18100 {
-				compatible = "marvell,armadaxp-gpio",
+				compatible = "marvell,armada-370-gpio",
 					     "marvell,orion-gpio";
 				reg = <0x18100 0x40>, <0x181c0 0x08>;
 				reg-names = "gpio", "pwm";
@@ -323,7 +323,7 @@ gpio0: gpio@18100 {
 			};
 
 			gpio1: gpio@18140 {
-				compatible = "marvell,armadaxp-gpio",
+				compatible = "marvell,armada-370-gpio",
 					     "marvell,orion-gpio";
 				reg = <0x18140 0x40>, <0x181c8 0x08>;
 				reg-names = "gpio", "pwm";
-- 
2.39.0

[PATCH v2 2/2] Revert "ARM: dts: armada-39x: Fix compatible string for gpios"

[Klaus Kudielka]

This reverts commit d10886a4e6f85ee18d47a1066a52168461370ded.

If compatible = "marvell,armadaxp-gpio", the reg property requires a
second (address, size) pair, which points to the per-CPU interrupt
registers <0x18800 0x30> / <0x18840 0x30>.

Furthermore:
Commit 5f79c651e81e explains very well, why the gpio-mvebu driver does not
work reliably with per-CPU interrupts.
Commit 988c8c0cd04d deprecates compatible = marvell,armadaxp-gpio for this
reason.

Signed-off-by: Klaus Kudielka <klaus.kudielka@gmail.com>
---
v1->v2: Be more explicit about the reg property.

 arch/arm/boot/dts/armada-39x.dtsi | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/arch/arm/boot/dts/armada-39x.dtsi b/arch/arm/boot/dts/armada-39x.dtsi
index 1e05208d9f..9d1cac49c0 100644
--- a/arch/arm/boot/dts/armada-39x.dtsi
+++ b/arch/arm/boot/dts/armada-39x.dtsi
@@ -213,7 +213,7 @@ nand_pins: nand-pins {
 			};
 
 			gpio0: gpio@18100 {
-				compatible = "marvell,armadaxp-gpio", "marvell,orion-gpio";
+				compatible = "marvell,orion-gpio";
 				reg = <0x18100 0x40>;
 				ngpios = <32>;
 				gpio-controller;
@@ -227,7 +227,7 @@ gpio0: gpio@18100 {
 			};
 
 			gpio1: gpio@18140 {
-				compatible = "marvell,armadaxp-gpio", "marvell,orion-gpio";
+				compatible = "marvell,orion-gpio";
 				reg = <0x18140 0x40>;
 				ngpios = <28>;
 				gpio-controller;
-- 
2.39.0

Re: [PATCH v2 2/2] Revert "ARM: dts: armada-39x: Fix compatible string for gpios"

[Andrew Lunn]

On Sat, Jan 07, 2023 at 03:41:49PM +0100, Klaus Kudielka wrote:
> This reverts commit d10886a4e6f85ee18d47a1066a52168461370ded.
> 
> If compatible = "marvell,armadaxp-gpio", the reg property requires a
> second (address, size) pair, which points to the per-CPU interrupt
> registers <0x18800 0x30> / <0x18840 0x30>.
> 
> Furthermore:
> Commit 5f79c651e81e explains very well, why the gpio-mvebu driver does not
> work reliably with per-CPU interrupts.
> Commit 988c8c0cd04d deprecates compatible = marvell,armadaxp-gpio for this
> reason.
> 
> Signed-off-by: Klaus Kudielka <klaus.kudielka@gmail.com>

Reviewed-by: Andrew Lunn <andrew@lunn.ch>

    Andrew

Re: [PATCH v2 2/2] Revert "ARM: dts: armada-39x: Fix compatible string for gpios"

[Gregory CLEMENT]

Andrew Lunn <andrew@lunn.ch> writes:

> On Sat, Jan 07, 2023 at 03:41:49PM +0100, Klaus Kudielka wrote:
>> This reverts commit d10886a4e6f85ee18d47a1066a52168461370ded.
>> 
>> If compatible = "marvell,armadaxp-gpio", the reg property requires a
>> second (address, size) pair, which points to the per-CPU interrupt
>> registers <0x18800 0x30> / <0x18840 0x30>.
>> 
>> Furthermore:
>> Commit 5f79c651e81e explains very well, why the gpio-mvebu driver does not
>> work reliably with per-CPU interrupts.
>> Commit 988c8c0cd04d deprecates compatible = marvell,armadaxp-gpio for this
>> reason.
>> 
>> Signed-off-by: Klaus Kudielka <klaus.kudielka@gmail.com>
>
> Reviewed-by: Andrew Lunn <andrew@lunn.ch>

Applied on mvebu/fixes

Thanks,

Gregory
>
>     Andrew

-- 
Gregory Clement, Bootlin
Embedded Linux and Kernel engineering
http://bootlin.com

Re: [PATCH v2 1/2] Revert "ARM: dts: armada-38x: Fix compatible string for gpios"

[Andrew Lunn]

On Sat, Jan 07, 2023 at 03:41:48PM +0100, Klaus Kudielka wrote:
> This reverts commit c4de4667f15d04ef5920bacf41e514ec7d1ef03d, which causes
> a regression on Turris Omnia (Armada 385): GPIO interrupts cease to work,
> ending up in the DSA switch being non-functional.
> 
> The blamed commit is incorrect in the first place:
> If compatible = "marvell,armadaxp-gpio", the second (address, size) pair
> of the reg property must to point to the per-CPU interrupt registers
> <0x18800 0x30> / <0x18840 0x30>, and not to the blink enable registers
> <0x181c0 0x08> / <0x181c8 0x08>.
> 
> But even fixing that leaves the GPIO interrupts broken on the Omnia.
> 
> Furthermore:
> Commit 5f79c651e81e explains very well, why the gpio-mvebu driver does not
> work reliably with per-CPU interrupts.
> Commit 988c8c0cd04d deprecates compatible = marvell,armadaxp-gpio for this
> reason.
> 
> Fixes: c4de4667f15d ("ARM: dts: armada-38x: Fix compatible string for gpios")
> Reported-by: Klaus Kudielka <klaus.kudielka@gmail.com>
> Link: https://lore.kernel.org/r/f24474e70c1a4e9692bd596ef6d97ceda9511245.camel@gmail.com/
> 
> Signed-off-by: Klaus Kudielka <klaus.kudielka@gmail.com>

Hi Klaus

I was hoping Pali would comment, maybe fix it. But i think enough time
has passed, we should accept your reverts.

Reviewed-by: Andrew Lunn <andrew@lunn.ch>

    Andrew

Re: [PATCH v2 1/2] Revert "ARM: dts: armada-38x: Fix compatible string for gpios"

[Pali Rohár]

On Saturday 07 January 2023 18:18:24 Andrew Lunn wrote:
> On Sat, Jan 07, 2023 at 03:41:48PM +0100, Klaus Kudielka wrote:
> > This reverts commit c4de4667f15d04ef5920bacf41e514ec7d1ef03d, which causes
> > a regression on Turris Omnia (Armada 385): GPIO interrupts cease to work,
> > ending up in the DSA switch being non-functional.
> > 
> > The blamed commit is incorrect in the first place:
> > If compatible = "marvell,armadaxp-gpio", the second (address, size) pair
> > of the reg property must to point to the per-CPU interrupt registers
> > <0x18800 0x30> / <0x18840 0x30>, and not to the blink enable registers
> > <0x181c0 0x08> / <0x181c8 0x08>.
> > 
> > But even fixing that leaves the GPIO interrupts broken on the Omnia.
> > 
> > Furthermore:
> > Commit 5f79c651e81e explains very well, why the gpio-mvebu driver does not
> > work reliably with per-CPU interrupts.
> > Commit 988c8c0cd04d deprecates compatible = marvell,armadaxp-gpio for this
> > reason.
> > 
> > Fixes: c4de4667f15d ("ARM: dts: armada-38x: Fix compatible string for gpios")
> > Reported-by: Klaus Kudielka <klaus.kudielka@gmail.com>
> > Link: https://lore.kernel.org/r/f24474e70c1a4e9692bd596ef6d97ceda9511245.camel@gmail.com/
> > 
> > Signed-off-by: Klaus Kudielka <klaus.kudielka@gmail.com>
> 
> Hi Klaus
> 
> I was hoping Pali would comment, maybe fix it. But i think enough time
> has passed, we should accept your reverts.

Well, enough time for sure has not passed as I'm waiting for other
reviews and comments for more than half of year, reminding them, and
there is no input. So why should I react quickly in few days?

And why should fix it? Just prepare another patch and wait another year
until somebody notice it and merge it? I really do not have motivation
here. I have already spent too much time on this stuff, sent lot of
changes for which nobody cares.

> Reviewed-by: Andrew Lunn <andrew@lunn.ch>
> 
>     Andrew