[PATCH 1/3] arm64: dts: marvell: armada-37xx: mark the gpio controllers as irq controller

[PATCH 1/3] arm64: dts: marvell: armada-37xx: mark the gpio controllers as irq controller

[Uwe Kleine-König]

This allows to reference these gpio controller as interrupt parent. Also
add a comment which cpu line names are managed by the controllers
because "nb" and "sb" usually doesn't appear in schematics, but MPPX_Y
do.

Signed-off-by: Uwe Kleine-K?nig <u.kleine-koenig@pengutronix.de>
---
 arch/arm64/boot/dts/marvell/armada-37xx.dtsi | 6 ++++++
 1 file changed, 6 insertions(+)

diff --git a/arch/arm64/boot/dts/marvell/armada-37xx.dtsi b/arch/arm64/boot/dts/marvell/armada-37xx.dtsi
index 375026867342..6a96404e7c88 100644
--- a/arch/arm64/boot/dts/marvell/armada-37xx.dtsi
+++ b/arch/arm64/boot/dts/marvell/armada-37xx.dtsi
@@ -184,10 +184,13 @@
 				compatible = "marvell,armada3710-nb-pinctrl",
 					     "syscon", "simple-mfd";
 				reg = <0x13800 0x100>, <0x13C00 0x20>;
+				/* MPP1[19:0] */
 				gpionb: gpio {
 					#gpio-cells = <2>;
 					gpio-ranges = <&pinctrl_nb 0 0 36>;
 					gpio-controller;
+					interrupt-controller;
+					#interrupt-cells = <2>;
 					interrupts =
 					<GIC_SPI 51 IRQ_TYPE_LEVEL_HIGH>,
 					<GIC_SPI 52 IRQ_TYPE_LEVEL_HIGH>,
@@ -245,10 +248,13 @@
 				compatible = "marvell,armada3710-sb-pinctrl",
 					     "syscon", "simple-mfd";
 				reg = <0x18800 0x100>, <0x18C00 0x20>;
+				/* MPP2[23:0] */
 				gpiosb: gpio {
 					#gpio-cells = <2>;
 					gpio-ranges = <&pinctrl_sb 0 0 30>;
 					gpio-controller;
+					interrupt-controller;
+					#interrupt-cells = <2>;
 					interrupts =
 					<GIC_SPI 160 IRQ_TYPE_LEVEL_HIGH>,
 					<GIC_SPI 159 IRQ_TYPE_LEVEL_HIGH>,
-- 
2.16.2

[PATCH 2/3] arm64: dts: armada-3720-espressobin: make use of switch irq line

[Uwe Kleine-König]

The switch's INTn line is connected to the CPU's MPP2_23 pad. Wire this
up in the device tree. Note however that up to now the
marvell,armada3710-sb-pinctrl driver doesn't support level irqs, so the
switch driver doesn't make use of this feature.

Signed-off-by: Uwe Kleine-K?nig <u.kleine-koenig@pengutronix.de>
---
 arch/arm64/boot/dts/marvell/armada-3720-espressobin.dts | 13 +++++++++++++
 1 file changed, 13 insertions(+)

diff --git a/arch/arm64/boot/dts/marvell/armada-3720-espressobin.dts b/arch/arm64/boot/dts/marvell/armada-3720-espressobin.dts
index 6d070b267abe..a88f053d2f28 100644
--- a/arch/arm64/boot/dts/marvell/armada-3720-espressobin.dts
+++ b/arch/arm64/boot/dts/marvell/armada-3720-espressobin.dts
@@ -133,6 +133,11 @@
 		#size-cells = <0>;
 		reg = <1>;
 
+		interrupt-parent = <&gpiosb>;
+		interrupts = <23 IRQ_TYPE_LEVEL_LOW>;
+		interrupt-controller;
+		#interrupt-cells = <2>;
+
 		dsa,member = <0 0>;
 
 		ports {
@@ -171,12 +176,20 @@
 
 			switch0phy0: switch0phy0 at 11 {
 				reg = <0x11>;
+				interrupt-parent = <&switch0>;
+				interrupts = <1 IRQ_TYPE_LEVEL_HIGH>;
 			};
+
 			switch0phy1: switch0phy1 at 12 {
 				reg = <0x12>;
+				interrupt-parent = <&switch0>;
+				interrupts = <2 IRQ_TYPE_LEVEL_HIGH>;
 			};
+
 			switch0phy2: switch0phy2 at 13 {
 				reg = <0x13>;
+				interrupt-parent = <&switch0>;
+				interrupts = <3 IRQ_TYPE_LEVEL_HIGH>;
 			};
 		};
 	};
-- 
2.16.2

[PATCH 2/3] arm64: dts: armada-3720-espressobin: make use of switch irq line

[Andrew Lunn]

On Wed, Mar 21, 2018 at 11:50:04AM +0100, Uwe Kleine-K?nig wrote:
> The switch's INTn line is connected to the CPU's MPP2_23 pad. Wire this
> up in the device tree. Note however that up to now the
> marvell,armada3710-sb-pinctrl driver doesn't support level irqs, so the
> switch driver doesn't make use of this feature.

Hi Uwe

So that actually happens here, if the interrupt cannot be used? I
assume the mv88e6xxx driver does not fail the probe because the IRQ
cannot be configured? What does happen?

Thanks
	Andrew

[PATCH 2/3] arm64: dts: armada-3720-espressobin: make use of switch irq line

[Uwe Kleine-König]

Hello Andrew,

On Wed, Mar 21, 2018 at 02:15:36PM +0100, Andrew Lunn wrote:
> On Wed, Mar 21, 2018 at 11:50:04AM +0100, Uwe Kleine-K?nig wrote:
> > The switch's INTn line is connected to the CPU's MPP2_23 pad. Wire this
> > up in the device tree. Note however that up to now the
> > marvell,armada3710-sb-pinctrl driver doesn't support level irqs, so the
> > switch driver doesn't make use of this feature.
> 
> So that actually happens here, if the interrupt cannot be used? I
> assume the mv88e6xxx driver does not fail the probe because the IRQ
> cannot be configured? What does happen?

Grml, I just tried with all patches applied and it results in:

[    4.777524] mv88e6085 d0032004.mdio-mii:01: switch 0x3410 detected: Marvell 88E6341, revision 0
[    4.794723] genirq: Setting trigger mode 8 for irq 41 failed (armada_37xx_irq_set_type+0x0/0x160)
[    4.804470] mv88e6085: probe of d0032004.mdio-mii:01 failed with error -22

. It (kind of) worked before your patch

	net: dsa: mv88e6xxx: Use the DT IRQ trigger mode

. So I guess this has to change to

+               interrupt-parent = <&gpiosb>;
+		/* actually the irq is active low, but gpiosb doesn't support that and falling edge seems to work just fine */
+               interrupts = <23 IRQ_TYPE_EDGE_FALLING>;

Best regards
Uwe

-- 
Pengutronix e.K.                           | Uwe Kleine-K?nig            |
Industrial Linux Solutions                 | http://www.pengutronix.de/  |

[PATCH 2/3] arm64: dts: armada-3720-espressobin: make use of switch irq line

[Andrew Lunn]

> . So I guess this has to change to
> 
> +               interrupt-parent = <&gpiosb>;
> +		/* actually the irq is active low, but gpiosb doesn't support that and falling edge seems to work just fine */
> +               interrupts = <23 IRQ_TYPE_EDGE_FALLING>;

Or wait until you have simulated level interrupts?

At the moment, without this patch, you should have driver polled
interrupts. That already gives you a good speedup over phylib polling
the PHYs.

    Andrew

[PATCH 3/3] arm64: dts: armada-3720-espressobin: wire up LED2

[Uwe Kleine-König]

The led is connected to MPP1_2.

Signed-off-by: Uwe Kleine-K?nig <u.kleine-koenig@pengutronix.de>
---
Note the LED doesn't work for me on two different boards. But the
voltage levels on one side of the LED changes from 1.76V to 0V when
enabling the LED. So there is probably something fishy, don't know if it
is hardware or software related.

Best regards
Uwe
---
 arch/arm64/boot/dts/marvell/armada-3720-espressobin.dts | 9 +++++++++
 1 file changed, 9 insertions(+)

diff --git a/arch/arm64/boot/dts/marvell/armada-3720-espressobin.dts b/arch/arm64/boot/dts/marvell/armada-3720-espressobin.dts
index a88f053d2f28..0802d595e0a2 100644
--- a/arch/arm64/boot/dts/marvell/armada-3720-espressobin.dts
+++ b/arch/arm64/boot/dts/marvell/armada-3720-espressobin.dts
@@ -77,6 +77,15 @@
 			  3300000 0x0>;
 		enable-active-high;
 	};
+
+	leds {
+		compatible = "gpio-leds";
+
+		led2 {
+			gpios = <&gpionb 2 GPIO_ACTIVE_LOW>;
+			linux,default-trigger = "heartbeat";
+		};
+	};
 };
 
 /* J9 */
-- 
2.16.2

[PATCH 3/3] arm64: dts: armada-3720-espressobin: wire up LED2

[Andrew Lunn]

On Wed, Mar 21, 2018 at 11:50:05AM +0100, Uwe Kleine-K?nig wrote:
> The led is connected to MPP1_2.
> 
> Signed-off-by: Uwe Kleine-K?nig <u.kleine-koenig@pengutronix.de>
> ---
> Note the LED doesn't work for me on two different boards.

Do you have a board it does work with?

> But the voltage levels on one side of the LED changes from 1.76V to
> 0V when enabling the LED. So there is probably something fishy,
> don't know if it is hardware or software related.

I think we should not apply this patch until we get a better idea why
it does not work. Is 1.76V not enough? Is the LED mounted the wrong
way around? Not enough current?

    Andrew

[PATCH 3/3] arm64: dts: armada-3720-espressobin: wire up LED2

[Uwe Kleine-König]

On Wed, Mar 21, 2018 at 02:19:30PM +0100, Andrew Lunn wrote:
> On Wed, Mar 21, 2018 at 11:50:05AM +0100, Uwe Kleine-K?nig wrote:
> > The led is connected to MPP1_2.
> > 
> > Signed-off-by: Uwe Kleine-K?nig <u.kleine-koenig@pengutronix.de>
> > ---
> > Note the LED doesn't work for me on two different boards.
> 
> Do you have a board it does work with?

No, it doesn't work anywhere.
> 
> > But the voltage levels on one side of the LED changes from 1.76V to
> > 0V when enabling the LED. So there is probably something fishy,
> > don't know if it is hardware or software related.
> 
> I think we should not apply this patch until we get a better idea why
> it does not work.

Ack. I hoped to get some feedback about this issue.

> Is 1.76V not enough? Is the LED mounted the wrong way around? Not
> enough current?

There is another LED on the board called LED3 that is connected between
12V and ground with a R1k that is very bright. It looks identical to the
non-working LED2 which is connected between 1.8V and MPP1_2 with R100.
But there is no BOM available that would tell if the led parts are
different.

With no PSU connected I can make LED1 and LED3 light up with my
multimeter, but not LED2.

I'll try to contact globalscale and ask there.

Best regards
Uwe

-- 
Pengutronix e.K.                           | Uwe Kleine-K?nig            |
Industrial Linux Solutions                 | http://www.pengutronix.de/  |

Re: [PATCH 3/3] arm64: dts: armada-3720-espressobin: wire up LED2

[Pali Rohár]

Hello! It is old thread, but I have final results about this issue.

On Wednesday 21 March 2018 15:06:16 Uwe Kleine-König wrote:
> On Wed, Mar 21, 2018 at 02:19:30PM +0100, Andrew Lunn wrote:
> > On Wed, Mar 21, 2018 at 11:50:05AM +0100, Uwe Kleine-K?nig wrote:
> > > The led is connected to MPP1_2.
> > > 
> > > Signed-off-by: Uwe Kleine-K?nig <u.kleine-koenig@pengutronix.de>
> > > ---
> > > Note the LED doesn't work for me on two different boards.
> > 
> > Do you have a board it does work with?
> 
> No, it doesn't work anywhere.
> > 
> > > But the voltage levels on one side of the LED changes from 1.76V to
> > > 0V when enabling the LED. So there is probably something fishy,
> > > don't know if it is hardware or software related.
> > 
> > I think we should not apply this patch until we get a better idea why
> > it does not work.
> 
> Ack. I hoped to get some feedback about this issue.
> 
> > Is 1.76V not enough? Is the LED mounted the wrong way around? Not
> > enough current?
> 
> There is another LED on the board called LED3 that is connected between
> 12V and ground with a R1k that is very bright. It looks identical to the
> non-working LED2 which is connected between 1.8V and MPP1_2 with R100.
> But there is no BOM available that would tell if the led parts are
> different.
> 
> With no PSU connected I can make LED1 and LED3 light up with my
> multimeter, but not LED2.
> 
> I'll try to contact globalscale and ask there.

I contacted globalscale about this issue and I got answer that the led2
on EspressoBin v5 can't be turn on, because of the LED2 vcc use 1.8v,
the voltage is too low to drive the led2.

So it basically answers why that led was not working on any tested
EspressoBin (V5) board.

Also I was told that this hw bug was fixed in EpressoBin v7 revision and
MPP1_2 can be used to control LED2.

So Uwe's patch should be useful for EspressoBin v7 DTS file. On V5 is
LED2 nonworking.


Just to note that LED1 is working fine on EspressoBin V5, it is
controlled by mPCIe card via wlan led pin. So e.g. wifi card which has
supported wlan led driver (e.g. ath9k_pci) can power LED1 on/off.

_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

[PATCH 3/3] arm64: dts: armada-3720-espressobin: wire up LED2

[Gregory CLEMENT]

Hi Uwe,
 
 On mer., mars 21 2018, Uwe Kleine-K?nig <u.kleine-koenig@pengutronix.de> wrote:

> The led is connected to MPP1_2.
>
> Signed-off-by: Uwe Kleine-K?nig <u.kleine-koenig@pengutronix.de>
> ---
> Note the LED doesn't work for me on two different boards. But the
> voltage levels on one side of the LED changes from 1.76V to 0V when
> enabling the LED. So there is probably something fishy, don't know if it
> is hardware or software related.

Initially I thought that maybe you should use the "led" function for
it. But this function is only available for pin 11 to 14 not for pin 2.

Another tricky thing is that the pin controller for Armada 37xx really
work on groups: you can't only change the function of a single pin. But
the pin 2 is linked to pin 3 and they have as alternate function "i2c"
which is not used for the espressobin.

Moreover you seemed to managed to controller the pin.

So I have no more ideas for this issue.

At least according to the datasheet, there is no way to control the
behavior of the pin (pull-up, pull-don, open-drain, ...). It is only the
pin 11 to 14 which can be put in Open-Drain if they use the function
"led".

Gregory



>
> Best regards
> Uwe
> ---
>  arch/arm64/boot/dts/marvell/armada-3720-espressobin.dts | 9 +++++++++
>  1 file changed, 9 insertions(+)
>
> diff --git a/arch/arm64/boot/dts/marvell/armada-3720-espressobin.dts b/arch/arm64/boot/dts/marvell/armada-3720-espressobin.dts
> index a88f053d2f28..0802d595e0a2 100644
> --- a/arch/arm64/boot/dts/marvell/armada-3720-espressobin.dts
> +++ b/arch/arm64/boot/dts/marvell/armada-3720-espressobin.dts
> @@ -77,6 +77,15 @@
>  			  3300000 0x0>;
>  		enable-active-high;
>  	};
> +
> +	leds {
> +		compatible = "gpio-leds";
> +
> +		led2 {
> +			gpios = <&gpionb 2 GPIO_ACTIVE_LOW>;
> +			linux,default-trigger = "heartbeat";
> +		};
> +	};
>  };
>  
>  /* J9 */
> -- 
> 2.16.2
>

-- 
Gregory Clement, Bootlin (formerly Free Electrons)
Embedded Linux and Kernel engineering
http://bootlin.com

[PATCH 1/3] arm64: dts: marvell: armada-37xx: mark the gpio controllers as irq controller

[Andrew Lunn]

On Wed, Mar 21, 2018 at 11:50:03AM +0100, Uwe Kleine-K?nig wrote:
> This allows to reference these gpio controller as interrupt parent. Also
> add a comment which cpu line names are managed by the controllers
> because "nb" and "sb" usually doesn't appear in schematics, but MPPX_Y
> do.
> 
> Signed-off-by: Uwe Kleine-K?nig <u.kleine-koenig@pengutronix.de>

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

    Andrew

[PATCH 1/3] arm64: dts: marvell: armada-37xx: mark the gpio controllers as irq controller

[Gregory CLEMENT]

Hi Uwe,
 
 On mer., mars 21 2018, Uwe Kleine-K?nig <u.kleine-koenig@pengutronix.de> wrote:

> This allows to reference these gpio controller as interrupt parent. Also
> add a comment which cpu line names are managed by the controllers
> because "nb" and "sb" usually doesn't appear in schematics, but MPPX_Y
> do.
>
> Signed-off-by: Uwe Kleine-K?nig <u.kleine-koenig@pengutronix.de>


Applied on mvebu/dt-for-4.18 (I wonders why I didn't updated the dtsi
earlier...)

Thanks,

Gregory

> ---
>  arch/arm64/boot/dts/marvell/armada-37xx.dtsi | 6 ++++++
>  1 file changed, 6 insertions(+)
>
> diff --git a/arch/arm64/boot/dts/marvell/armada-37xx.dtsi b/arch/arm64/boot/dts/marvell/armada-37xx.dtsi
> index 375026867342..6a96404e7c88 100644
> --- a/arch/arm64/boot/dts/marvell/armada-37xx.dtsi
> +++ b/arch/arm64/boot/dts/marvell/armada-37xx.dtsi
> @@ -184,10 +184,13 @@
>  				compatible = "marvell,armada3710-nb-pinctrl",
>  					     "syscon", "simple-mfd";
>  				reg = <0x13800 0x100>, <0x13C00 0x20>;
> +				/* MPP1[19:0] */
>  				gpionb: gpio {
>  					#gpio-cells = <2>;
>  					gpio-ranges = <&pinctrl_nb 0 0 36>;
>  					gpio-controller;
> +					interrupt-controller;
> +					#interrupt-cells = <2>;
>  					interrupts =
>  					<GIC_SPI 51 IRQ_TYPE_LEVEL_HIGH>,
>  					<GIC_SPI 52 IRQ_TYPE_LEVEL_HIGH>,
> @@ -245,10 +248,13 @@
>  				compatible = "marvell,armada3710-sb-pinctrl",
>  					     "syscon", "simple-mfd";
>  				reg = <0x18800 0x100>, <0x18C00 0x20>;
> +				/* MPP2[23:0] */
>  				gpiosb: gpio {
>  					#gpio-cells = <2>;
>  					gpio-ranges = <&pinctrl_sb 0 0 30>;
>  					gpio-controller;
> +					interrupt-controller;
> +					#interrupt-cells = <2>;
>  					interrupts =
>  					<GIC_SPI 160 IRQ_TYPE_LEVEL_HIGH>,
>  					<GIC_SPI 159 IRQ_TYPE_LEVEL_HIGH>,
> -- 
> 2.16.2
>

-- 
Gregory Clement, Bootlin (formerly Free Electrons)
Embedded Linux and Kernel engineering
http://bootlin.com