[PATCH mvebu-dt 0/6] Turris Omnia device-tree changes

[PATCH mvebu-dt 0/6] Turris Omnia device-tree changes

[Marek Behún]

Hi Gregory,

these are some changes for Turris Omnia device tree.
This series applies onto your mvebu-dt branch.

Marek

Cc: Uwe Kleine-König <uwe@kleine-koenig.org>
Cc: Jason Cooper <jason@lakedaemon.net>
Cc: Gregory CLEMENT <gregory.clement@bootlin.com>
Cc: Andreas Färber <afaerber@suse.de>
Cc: Andrew Lunn <andrew@lunn.ch>
Cc: Rob Herring <robh+dt@kernel.org>
Cc: devicetree@vger.kernel.org

Marek Behún (6):
  ARM: dts: turris-omnia: enable HW buffer management
  ARM: dts: turris-omnia: add comphy handle to eth2
  ARM: dts: turris-omnia: describe ethernet-phy interrupt
  ARM: dts: turris-omnia: add SFP node
  ARM: dts: turris-omnia: add LED controller node
  ARM: dts: turris-omnia: update ethernet-phy node and handle name

 arch/arm/boot/dts/armada-385-turris-omnia.dts | 156 +++++++++++++++++-
 1 file changed, 151 insertions(+), 5 deletions(-)

-- 
2.26.2

[PATCH mvebu-dt 1/6] ARM: dts: turris-omnia: enable HW buffer management

[Marek Behún]

The buffer manager is available on Turris Omnia but needs to be
described in device-tree to be used.

Signed-off-by: Marek Behún <kabel@kernel.org>
Fixes: 26ca8b52d6e1 ("ARM: dts: add support for Turris Omnia")
Cc: Uwe Kleine-König <uwe@kleine-koenig.org>
Cc: Jason Cooper <jason@lakedaemon.net>
Cc: Gregory CLEMENT <gregory.clement@bootlin.com>
Cc: Andreas Färber <afaerber@suse.de>
Cc: Andrew Lunn <andrew@lunn.ch>
Cc: Rob Herring <robh+dt@kernel.org>
Cc: devicetree@vger.kernel.org
---
 arch/arm/boot/dts/armada-385-turris-omnia.dts | 17 +++++++++++++++++
 1 file changed, 17 insertions(+)

diff --git a/arch/arm/boot/dts/armada-385-turris-omnia.dts b/arch/arm/boot/dts/armada-385-turris-omnia.dts
index 768b6c5d2129..b6bd73d8f2ba 100644
--- a/arch/arm/boot/dts/armada-385-turris-omnia.dts
+++ b/arch/arm/boot/dts/armada-385-turris-omnia.dts
@@ -84,12 +84,23 @@ pcie@3,0 {
 	};
 };
 
+&bm {
+	status = "okay";
+};
+
+&bm_bppi {
+	status = "okay";
+};
+
 /* Connected to 88E6176 switch, port 6 */
 &eth0 {
 	pinctrl-names = "default";
 	pinctrl-0 = <&ge0_rgmii_pins>;
 	status = "okay";
 	phy-mode = "rgmii";
+	buffer-manager = <&bm>;
+	bm,pool-long = <0>;
+	bm,pool-short = <3>;
 
 	fixed-link {
 		speed = <1000>;
@@ -103,6 +114,9 @@ &eth1 {
 	pinctrl-0 = <&ge1_rgmii_pins>;
 	status = "okay";
 	phy-mode = "rgmii";
+	buffer-manager = <&bm>;
+	bm,pool-long = <1>;
+	bm,pool-short = <3>;
 
 	fixed-link {
 		speed = <1000>;
@@ -115,6 +129,9 @@ &eth2 {
 	status = "okay";
 	phy-mode = "sgmii";
 	phy = <&phy1>;
+	buffer-manager = <&bm>;
+	bm,pool-long = <2>;
+	bm,pool-short = <3>;
 };
 
 &i2c0 {
-- 
2.26.2

[PATCH mvebu-dt 2/6] ARM: dts: turris-omnia: add comphy handle to eth2

[Marek Behún]

The eth2 controller on Turris Omnia is connected to SerDes. For SFP to
be able to switch between 1G ans 2.5G modes the comphy link has to be
defined.

Signed-off-by: Marek Behún <kabel@kernel.org>
Fixes: f3a6a9f3704a ("ARM: dts: add description for Armada 38x ...")
Cc: Uwe Kleine-König <uwe@kleine-koenig.org>
Cc: Jason Cooper <jason@lakedaemon.net>
Cc: Gregory CLEMENT <gregory.clement@bootlin.com>
Cc: Andreas Färber <afaerber@suse.de>
Cc: Andrew Lunn <andrew@lunn.ch>
Cc: Rob Herring <robh+dt@kernel.org>
Cc: devicetree@vger.kernel.org
---
 arch/arm/boot/dts/armada-385-turris-omnia.dts | 1 +
 1 file changed, 1 insertion(+)

diff --git a/arch/arm/boot/dts/armada-385-turris-omnia.dts b/arch/arm/boot/dts/armada-385-turris-omnia.dts
index b6bd73d8f2ba..9de26c78ec4c 100644
--- a/arch/arm/boot/dts/armada-385-turris-omnia.dts
+++ b/arch/arm/boot/dts/armada-385-turris-omnia.dts
@@ -129,6 +129,7 @@ &eth2 {
 	status = "okay";
 	phy-mode = "sgmii";
 	phy = <&phy1>;
+	phys = <&comphy5 2>;
 	buffer-manager = <&bm>;
 	bm,pool-long = <2>;
 	bm,pool-short = <3>;
-- 
2.26.2

[PATCH mvebu-dt 3/6] ARM: dts: turris-omnia: describe ethernet-phy interrupt

[Marek Behún]

Describe ethernet-phy interrupt for Turris Omnia so that the PHY does
not need to be polled. We also need to describe the PCA9538 as an
interrupt controller.

Signed-off-by: Marek Behún <kabel@kernel.org>
Fixes: 26ca8b52d6e1 ("ARM: dts: add support for Turris Omnia")
Cc: Uwe Kleine-König <uwe@kleine-koenig.org>
Cc: Jason Cooper <jason@lakedaemon.net>
Cc: Gregory CLEMENT <gregory.clement@bootlin.com>
Cc: Andreas Färber <afaerber@suse.de>
Cc: Andrew Lunn <andrew@lunn.ch>
Cc: Rob Herring <robh+dt@kernel.org>
Cc: devicetree@vger.kernel.org
---
 arch/arm/boot/dts/armada-385-turris-omnia.dts | 6 +++++-
 1 file changed, 5 insertions(+), 1 deletion(-)

diff --git a/arch/arm/boot/dts/armada-385-turris-omnia.dts b/arch/arm/boot/dts/armada-385-turris-omnia.dts
index 9de26c78ec4c..599eecf1a02c 100644
--- a/arch/arm/boot/dts/armada-385-turris-omnia.dts
+++ b/arch/arm/boot/dts/armada-385-turris-omnia.dts
@@ -238,6 +238,9 @@ pcawan: gpio@71 {
 				interrupt-parent = <&gpio1>;
 				interrupts = <14 IRQ_TYPE_LEVEL_LOW>;
 
+				interrupt-controller;
+				#interrupt-cells = <2>;
+
 				gpio-controller;
 				#gpio-cells = <2>;
 			};
@@ -255,7 +258,8 @@ phy1: phy@1 {
 		compatible = "ethernet-phy-id0141.0DD1", "ethernet-phy-ieee802.3-c22";
 		reg = <1>;
 
-		/* irq is connected to &pcawan pin 7 */
+		interrupt-parent = <&pcawan>;
+		interrupt = <7 IRQ_TYPE_LEVEL_LOW>;
 	};
 
 	/* Switch MV88E6176 at address 0x10 */
-- 
2.26.2

[PATCH mvebu-dt 4/6] ARM: dts: turris-omnia: add SFP node

[Marek Behún]

Turris Omnia has a SFP cage that, together with WAN PHY is connected to
eth2 SerDes via a SerDes multiplexor. Describe the SFP cage, but leave
it disabled. Until phylink has support for such multiplexor we will
leave it to U-Boot to enable SFP and disable WAN PHY at boot time
depending on whether a SFP module is present.

Signed-off-by: Marek Behún <kabel@kernel.org>
Fixes: 26ca8b52d6e1 ("ARM: dts: add support for Turris Omnia")
Cc: Uwe Kleine-König <uwe@kleine-koenig.org>
Cc: Jason Cooper <jason@lakedaemon.net>
Cc: Gregory CLEMENT <gregory.clement@bootlin.com>
Cc: Andreas Färber <afaerber@suse.de>
Cc: Andrew Lunn <andrew@lunn.ch>
Cc: Rob Herring <robh+dt@kernel.org>
Cc: devicetree@vger.kernel.org
---
 arch/arm/boot/dts/armada-385-turris-omnia.dts | 19 ++++++++++++++++++-
 1 file changed, 18 insertions(+), 1 deletion(-)

diff --git a/arch/arm/boot/dts/armada-385-turris-omnia.dts b/arch/arm/boot/dts/armada-385-turris-omnia.dts
index 599eecf1a02c..67048a88e327 100644
--- a/arch/arm/boot/dts/armada-385-turris-omnia.dts
+++ b/arch/arm/boot/dts/armada-385-turris-omnia.dts
@@ -82,6 +82,22 @@ pcie@3,0 {
 			};
 		};
 	};
+
+	sfp: sfp {
+		compatible = "sff,sfp";
+		i2c-bus = <&sfp_i2c>;
+		tx-fault-gpios = <&pcawan 0 GPIO_ACTIVE_HIGH>;
+		tx-disable-gpios = <&pcawan 1 GPIO_ACTIVE_HIGH>;
+		rate-select0-gpios = <&pcawan 2 GPIO_ACTIVE_HIGH>;
+		los-gpios = <&pcawan 3 GPIO_ACTIVE_HIGH>;
+		mod-def0-gpios = <&pcawan 4 GPIO_ACTIVE_LOW>;
+
+		/*
+		 * For now this has to be enabled at boot time by U-Boot when
+		 * a SFP module is present.
+		 */
+		status = "disabled";
+	};
 };
 
 &bm {
@@ -130,6 +146,7 @@ &eth2 {
 	phy-mode = "sgmii";
 	phy = <&phy1>;
 	phys = <&comphy5 2>;
+	sfp = <&sfp>;
 	buffer-manager = <&bm>;
 	bm,pool-long = <2>;
 	bm,pool-short = <3>;
@@ -195,7 +212,7 @@ i2c@3 {
 			/* routed to PCIe2 connector (CN62A) */
 		};
 
-		i2c@4 {
+		sfp_i2c: i2c@4 {
 			#address-cells = <1>;
 			#size-cells = <0>;
 			reg = <4>;
-- 
2.26.2

[PATCH mvebu-dt 5/6] ARM: dts: turris-omnia: add LED controller node

[Marek Behún]

Linux now has incomplete support for the LED controller on Turris Omnia:
it can set brightness and colors for each LED.

The controller can also put these LEDs into HW controlled mode, in which
the LEDs are controlled by HW: for example the WAN LED is connected via
MCU to the WAN PHY LED pin.

The driver does not support these HW controlled modes yet, and on probe
puts the LEDs into SW controlled mode.

Add node describing the LED controller, but disable it for now.

Signed-off-by: Marek Behún <kabel@kernel.org>
Cc: Uwe Kleine-König <uwe@kleine-koenig.org>
Cc: Jason Cooper <jason@lakedaemon.net>
Cc: Gregory CLEMENT <gregory.clement@bootlin.com>
Cc: Andreas Färber <afaerber@suse.de>
Cc: Rob Herring <robh+dt@kernel.org>
Cc: devicetree@vger.kernel.org
---
 arch/arm/boot/dts/armada-385-turris-omnia.dts | 107 ++++++++++++++++++
 1 file changed, 107 insertions(+)

diff --git a/arch/arm/boot/dts/armada-385-turris-omnia.dts b/arch/arm/boot/dts/armada-385-turris-omnia.dts
index 67048a88e327..771df9bbe271 100644
--- a/arch/arm/boot/dts/armada-385-turris-omnia.dts
+++ b/arch/arm/boot/dts/armada-385-turris-omnia.dts
@@ -12,6 +12,7 @@
 
 #include <dt-bindings/gpio/gpio.h>
 #include <dt-bindings/input/input.h>
+#include <dt-bindings/leds/common.h>
 #include "armada-385.dtsi"
 
 / {
@@ -172,6 +173,112 @@ i2c@0 {
 			/* STM32F0 command interface at address 0x2a */
 			/* leds device (in STM32F0) at address 0x2b */
 
+			led-controller@2b {
+				compatible = "cznic,turris-omnia-leds";
+				reg = <0x2b>;
+				#address-cells = <1>;
+				#size-cells = <0>;
+
+				/*
+				 * The driver does not support HW control mode
+				 * for the LEDs yet. Disable the LEDs for now.
+				 *
+				 * Also LED functions are not stable yet:
+				 * - there are 3 LEDs connected via MCU to PCIe
+				 *   ports. One of these ports supports mSATA.
+				 *   There is no mSATA nor PCIe function.
+				 *   For now we use LED_FUNCITION_WLAN, since
+				 *   in most cases users have wifi cards in
+				 *   these slots
+				 * - there are 2 LEDs dedicated for user: A and
+				 *   B. Again there is no such function defined.
+				 *   For now we use LED_FUNCTION_DEBUG
+				 */
+				status = "disabled";
+
+				multi-led@0 {
+					reg = <0x0>;
+					color = <LED_COLOR_ID_RGB>;
+					function = LED_FUNCTION_DEBUG;
+					function-enumerator = <2>;
+				};
+
+				multi-led@1 {
+					reg = <0x1>;
+					color = <LED_COLOR_ID_RGB>;
+					function = LED_FUNCTION_DEBUG;
+					function-enumerator = <1>;
+				};
+
+				multi-led@2 {
+					reg = <0x2>;
+					color = <LED_COLOR_ID_RGB>;
+					function = LED_FUNCTION_WLAN;
+					function-enumerator = <3>;
+				};
+
+				multi-led@3 {
+					reg = <0x3>;
+					color = <LED_COLOR_ID_RGB>;
+					function = LED_FUNCTION_WLAN;
+					function-enumerator = <2>;
+				};
+
+				multi-led@4 {
+					reg = <0x4>;
+					color = <LED_COLOR_ID_RGB>;
+					function = LED_FUNCTION_WLAN;
+					function-enumerator = <1>;
+				};
+
+				multi-led@5 {
+					reg = <0x5>;
+					color = <LED_COLOR_ID_RGB>;
+					function = LED_FUNCTION_WAN;
+				};
+
+				multi-led@6 {
+					reg = <0x6>;
+					color = <LED_COLOR_ID_RGB>;
+					function = LED_FUNCTION_LAN;
+					function-enumerator = <4>;
+				};
+
+				multi-led@7 {
+					reg = <0x7>;
+					color = <LED_COLOR_ID_RGB>;
+					function = LED_FUNCTION_LAN;
+					function-enumerator = <3>;
+				};
+
+				multi-led@8 {
+					reg = <0x8>;
+					color = <LED_COLOR_ID_RGB>;
+					function = LED_FUNCTION_LAN;
+					function-enumerator = <2>;
+				};
+
+				multi-led@9 {
+					reg = <0x9>;
+					color = <LED_COLOR_ID_RGB>;
+					function = LED_FUNCTION_LAN;
+					function-enumerator = <1>;
+				};
+
+				multi-led@a {
+					reg = <0xa>;
+					color = <LED_COLOR_ID_RGB>;
+					function = LED_FUNCTION_LAN;
+					function-enumerator = <0>;
+				};
+
+				multi-led@b {
+					reg = <0xb>;
+					color = <LED_COLOR_ID_RGB>;
+					function = LED_FUNCTION_POWER;
+				};
+			};
+
 			eeprom@54 {
 				compatible = "atmel,24c64";
 				reg = <0x54>;
-- 
2.26.2

[PATCH mvebu-dt 6/6] ARM: dts: turris-omnia: update ethernet-phy node and handle name

[Marek Behún]

Use property name `phy-handle` instead of the deprecated `phy` to
connect eth2 to the PHY.
Rename the node from "phy@1" to "ethernet-phy@1", since "phy@1" is
incorrect according to device-tree bindings documentation.
Also remove the "ethernet-phy-id0141.0DD1" compatible string, it is not
needed. Kernel can read the PHY identifier itself.

Signed-off-by: Marek Behún <kabel@kernel.org>
Cc: Uwe Kleine-König <uwe@kleine-koenig.org>
Cc: Jason Cooper <jason@lakedaemon.net>
Cc: Gregory CLEMENT <gregory.clement@bootlin.com>
Cc: Andreas Färber <afaerber@suse.de>
Cc: Andrew Lunn <andrew@lunn.ch>
Cc: Rob Herring <robh+dt@kernel.org>
Cc: devicetree@vger.kernel.org
---
 arch/arm/boot/dts/armada-385-turris-omnia.dts | 6 +++---
 1 file changed, 3 insertions(+), 3 deletions(-)

diff --git a/arch/arm/boot/dts/armada-385-turris-omnia.dts b/arch/arm/boot/dts/armada-385-turris-omnia.dts
index 771df9bbe271..99fe46911440 100644
--- a/arch/arm/boot/dts/armada-385-turris-omnia.dts
+++ b/arch/arm/boot/dts/armada-385-turris-omnia.dts
@@ -145,7 +145,7 @@ fixed-link {
 &eth2 {
 	status = "okay";
 	phy-mode = "sgmii";
-	phy = <&phy1>;
+	phy-handle = <&phy1>;
 	phys = <&comphy5 2>;
 	sfp = <&sfp>;
 	buffer-manager = <&bm>;
@@ -377,9 +377,9 @@ &mdio {
 	pinctrl-0 = <&mdio_pins>;
 	status = "okay";
 
-	phy1: phy@1 {
+	phy1: ethernet-phy@1 {
 		status = "okay";
-		compatible = "ethernet-phy-id0141.0DD1", "ethernet-phy-ieee802.3-c22";
+		compatible = "ethernet-phy-ieee802.3-c22";
 		reg = <1>;
 
 		interrupt-parent = <&pcawan>;
-- 
2.26.2

Re: [PATCH mvebu-dt 2/6] ARM: dts: turris-omnia: add comphy handle to eth2

[Andrew Lunn]

On Sat, Nov 14, 2020 at 05:04:05PM +0100, Marek Behún wrote:
> The eth2 controller on Turris Omnia is connected to SerDes. For SFP to
> be able to switch between 1G ans 2.5G modes the comphy link has to be

s/ans/and

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

    Andrew

Re: [PATCH mvebu-dt 3/6] ARM: dts: turris-omnia: describe ethernet-phy interrupt

[Andrew Lunn]

On Sat, Nov 14, 2020 at 05:04:06PM +0100, Marek Behún wrote:
> Describe ethernet-phy interrupt for Turris Omnia so that the PHY does
> not need to be polled. We also need to describe the PCA9538 as an
> interrupt controller.
> 
> Signed-off-by: Marek Behún <kabel@kernel.org>
> Fixes: 26ca8b52d6e1 ("ARM: dts: add support for Turris Omnia")

I'm not sure the fixes is justified. This is an optimisation, not a
fix.

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

    Andrew

Re: [PATCH mvebu-dt 4/6] ARM: dts: turris-omnia: add SFP node

[Andrew Lunn]

On Sat, Nov 14, 2020 at 05:04:07PM +0100, Marek Behún wrote:
> Turris Omnia has a SFP cage that, together with WAN PHY is connected to
> eth2 SerDes via a SerDes multiplexor. Describe the SFP cage, but leave
> it disabled. Until phylink has support for such multiplexor we will
> leave it to U-Boot to enable SFP and disable WAN PHY at boot time
> depending on whether a SFP module is present.
> 
> Signed-off-by: Marek Behún <kabel@kernel.org>
> Fixes: 26ca8b52d6e1 ("ARM: dts: add support for Turris Omnia")

Same comment for the fixes, especially since this is disabled by
default.

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

    Andrew

Re: [PATCH mvebu-dt 6/6] ARM: dts: turris-omnia: update ethernet-phy node and handle name

[Andrew Lunn]

On Sat, Nov 14, 2020 at 05:04:09PM +0100, Marek Behún wrote:
> Use property name `phy-handle` instead of the deprecated `phy` to
> connect eth2 to the PHY.
> Rename the node from "phy@1" to "ethernet-phy@1", since "phy@1" is
> incorrect according to device-tree bindings documentation.
> Also remove the "ethernet-phy-id0141.0DD1" compatible string, it is not
> needed. Kernel can read the PHY identifier itself.
> 
> Signed-off-by: Marek Behún <kabel@kernel.org>

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

    Andrew

Re: [PATCH mvebu-dt 3/6] ARM: dts: turris-omnia: describe ethernet-phy interrupt

[Marek Behún]

On Sat, 14 Nov 2020 17:04:06 +0100
Marek Behún <kabel@kernel.org> wrote:

> +		interrupt-parent = <&pcawan>;
> +		interrupt = <7 IRQ_TYPE_LEVEL_LOW>;

This has to be interrupts, or I can use interrupts-extended. Sorry,
will send a fix.

Re: [PATCH mvebu-dt 3/6] ARM: dts: turris-omnia: describe ethernet-phy interrupt

[Marek Behún]

On Sat, 14 Nov 2020 17:04:06 +0100
Marek Behún <kabel@kernel.org> wrote:

> +		interrupt-parent = <&pcawan>;
> +		interrupt = <7 IRQ_TYPE_LEVEL_LOW>;

Also we need to use IRQ_TYPE_EDGE_FALLING. The gpio-pca953x driver does
not support IRQ_TYPE_LEVEL_LOW...

Re: [PATCH mvebu-dt 3/6] ARM: dts: turris-omnia: describe ethernet-phy interrupt

[Marek Behún]

On Sat, 14 Nov 2020 17:34:39 +0100
Andrew Lunn <andrew@lunn.ch> wrote:

> On Sat, Nov 14, 2020 at 05:04:06PM +0100, Marek Behún wrote:
> > Describe ethernet-phy interrupt for Turris Omnia so that the PHY does
> > not need to be polled. We also need to describe the PCA9538 as an
> > interrupt controller.
> > 
> > Signed-off-by: Marek Behún <kabel@kernel.org>
> > Fixes: 26ca8b52d6e1 ("ARM: dts: add support for Turris Omnia")  
> 
> I'm not sure the fixes is justified. This is an optimisation, not a
> fix.

I considered this as a fix in the sense that it should have been there
from the beginning... Although the driver works, the devicetree does not
describe the device correctly without it.

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

Re: [PATCH mvebu-dt 3/6] ARM: dts: turris-omnia: describe ethernet-phy interrupt

[Andrew Lunn]

On Sat, Nov 14, 2020 at 05:49:28PM +0100, Marek Behún wrote:
> On Sat, 14 Nov 2020 17:04:06 +0100
> Marek Behún <kabel@kernel.org> wrote:
> 
> > +		interrupt-parent = <&pcawan>;
> > +		interrupt = <7 IRQ_TYPE_LEVEL_LOW>;
> 
> Also we need to use IRQ_TYPE_EDGE_FALLING. The gpio-pca953x driver does
> not support IRQ_TYPE_LEVEL_LOW...

Please check the datasheet for the PHY. I expect you will find it is
level triggering, not edge. So you can miss interrupts, and have the
wrong state.

I've also had bad experiences with pca953x and interrupt storms. I
hope you are using one with the extended registers including the
interrupt mask.

	  Andrew

Re: [PATCH mvebu-dt 3/6] ARM: dts: turris-omnia: describe ethernet-phy interrupt

[Marek Behún]

On Sat, 14 Nov 2020 18:16:39 +0100
Andrew Lunn <andrew@lunn.ch> wrote:

> On Sat, Nov 14, 2020 at 05:49:28PM +0100, Marek Behún wrote:
> > On Sat, 14 Nov 2020 17:04:06 +0100
> > Marek Behún <kabel@kernel.org> wrote:
> >   
> > > +		interrupt-parent = <&pcawan>;
> > > +		interrupt = <7 IRQ_TYPE_LEVEL_LOW>;  
> > 
> > Also we need to use IRQ_TYPE_EDGE_FALLING. The gpio-pca953x driver does
> > not support IRQ_TYPE_LEVEL_LOW...  
> 
> Please check the datasheet for the PHY. I expect you will find it is
> level triggering, not edge. So you can miss interrupts, and have the
> wrong state.
> 
> I've also had bad experiences with pca953x and interrupt storms. I
> hope you are using one with the extended registers including the
> interrupt mask.
> 
> 	  Andrew

Andrew, the PHY is a 88E1514, which triggers on level. We can set
either active-low or active-high, but not edge. Default is active-low.

The expander is a PCA9538 (in schematics called PCA9538PWR). The driver
does not support level interrupts:
	if (!(type & IRQ_TYPE_EDGE_BOTH)) {
		dev_err(&chip->client->dev, "irq %d: unsupported type %d\n",
			d->irq, type);
		return -EINVAL;
	}

It seems that for PCA9538 the interrupts are read from input. Some of
the chips supported by this driver can latch interrupt status into
special register, but this chip does not...

On interrupt the driver looks at the input register, compares it with
cached state and looks which bits changed. So theoretically level
support could be implemented in this driver, but currently it is not.

Do you think we should just poll for interrupts with the PHY?

Marek

Re: [PATCH mvebu-dt 3/6] ARM: dts: turris-omnia: describe ethernet-phy interrupt

[Marek Behun]

On Sat, 14 Nov 2020 18:42:21 +0100
Marek Behún <kabel@kernel.org> wrote:

> Do you think we should just poll for interrupts with the PHY?

Andrew, JFI, there are also SFP GPIOs connected to this expander. So
interrupt will be generated on changes from those gpios as well. Is an
interrupt storm seriosly probable here?

Marek

Re: [PATCH mvebu-dt 3/6] ARM: dts: turris-omnia: describe ethernet-phy interrupt

[Andrew Lunn]

On Sat, Nov 14, 2020 at 06:47:17PM +0100, Marek Behun wrote:
> On Sat, 14 Nov 2020 18:42:21 +0100
> Marek Behún <kabel@kernel.org> wrote:
> 
> > Do you think we should just poll for interrupts with the PHY?

Yes.

> 
> Andrew, JFI, there are also SFP GPIOs connected to this expander. So
> interrupt will be generated on changes from those gpios as well. Is an
> interrupt storm seriosly probable here?

As far as i remember on the device i was using, all inputs are
interrupt sources. And all pins default as inputs, so you don't
accidentally drive the output against something else. So you need to
get the device properly configured before enabling its interrupt. And
i'm not too sure how that works with the GPIO framework where you
effectively have a collection of individual GPIOs. How do you know
they have all be configured, and it is safe to enable the interrupt
output? Do you have any pins floating? Those you are going to have to
configure as output. Plus is the interrupt output from the gpio
expander level triggered? If so you need to ensure the upstream
interrupt controller can do level. And Marvell SoCs often cannot.

The device i was working on i had trouble even getting it to boot
without dying. Polling the PHY and SFP was working O.K, so i gave
up. And i suggested the next revision of the board used an sx150x
which has much better interrupt support. That has worked well against
a Vybrid.

	   Andrew

Re: [PATCH mvebu-dt 3/6] ARM: dts: turris-omnia: describe ethernet-phy interrupt

[Marek Behun]

On Sat, 14 Nov 2020 21:16:37 +0100
Andrew Lunn <andrew@lunn.ch> wrote:

> On Sat, Nov 14, 2020 at 06:47:17PM +0100, Marek Behun wrote:
> > On Sat, 14 Nov 2020 18:42:21 +0100
> > Marek Behún <kabel@kernel.org> wrote:
> >   
> > > Do you think we should just poll for interrupts with the PHY?  
> 
> Yes.
> 
> > 
> > Andrew, JFI, there are also SFP GPIOs connected to this expander. So
> > interrupt will be generated on changes from those gpios as well. Is an
> > interrupt storm seriosly probable here?  
> 
> As far as i remember on the device i was using, all inputs are
> interrupt sources. And all pins default as inputs, so you don't
> accidentally drive the output against something else. So you need to
> get the device properly configured before enabling its interrupt. And
> i'm not too sure how that works with the GPIO framework where you
> effectively have a collection of individual GPIOs. How do you know
> they have all be configured, and it is safe to enable the interrupt
> output? Do you have any pins floating? Those you are going to have to
> configure as output. Plus is the interrupt output from the gpio
> expander level triggered? If so you need to ensure the upstream
> interrupt controller can do level. And Marvell SoCs often cannot.
> 
> The device i was working on i had trouble even getting it to boot
> without dying. Polling the PHY and SFP was working O.K, so i gave
> up. And i suggested the next revision of the board used an sx150x
> which has much better interrupt support. That has worked well against
> a Vybrid.
> 
> 	   Andrew

Thanks, Andrew for this explanation.

What about when a interrupt pin from a Marvell PHY (level interrupt) is
connected directly to a gpio pin on Marvell SOC (edge only interrupt),
without any GPIO expander?
This is the case on Turris MOX, and in
drivers/pinctrl/mvebu/pinctrl-armada-37xx.c the function
armada_37xx_irq_set_type again supports only edge interrupts.
But in this case I think setting it to EDGE_FALLING should be okay,
should it not? This controller has a special register where interrupt
status is latched...

Marek

Re: [PATCH mvebu-dt 3/6] ARM: dts: turris-omnia: describe ethernet-phy interrupt

[Andreas Färber]

On 14.11.20 18:16, Andrew Lunn wrote:
> On Sat, Nov 14, 2020 at 05:49:28PM +0100, Marek Behún wrote:
>> On Sat, 14 Nov 2020 17:04:06 +0100
>> Marek Behún <kabel@kernel.org> wrote:
>>
>>> +		interrupt-parent = <&pcawan>;
>>> +		interrupt = <7 IRQ_TYPE_LEVEL_LOW>;
>>
>> Also we need to use IRQ_TYPE_EDGE_FALLING. The gpio-pca953x driver does
>> not support IRQ_TYPE_LEVEL_LOW...
> 
> Please check the datasheet for the PHY. I expect you will find it is
> level triggering, not edge. So you can miss interrupts, and have the
> wrong state.

I can confirm that neither rising nor falling had worked for me, so I
reverted such changes again. I did not look into pca953x code myself.

Regards,
Andreas

P.S. Was LAKML intentionally omitted from CC?

-- 
SUSE Software Solutions Germany GmbH
Maxfeldstr. 5, 90409 Nürnberg, Germany
GF: Felix Imendörffer
HRB 36809 (AG Nürnberg)

Re: [PATCH mvebu-dt 3/6] ARM: dts: turris-omnia: describe ethernet-phy interrupt

[Marek Behún]

On Sat, 14 Nov 2020 22:20:38 +0100
Andreas Färber <afaerber@suse.de> wrote:

> P.S. Was LAKML intentionally omitted from CC?

I do not include LAKML to Cc in most cases, just the corresponding
maintainers and mailing lists of given subsystems.

Marek