Turris Omnia device-tree changes

Hi Gregory,

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

Marek

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

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

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

index 768b6c5d2129..b6bd73d8f2ba 100644
@@ -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

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.

---
 arch/arm/boot/dts/armada-385-turris-omnia.dts | 1 +
 1 file changed, 1 insertion(+)

index b6bd73d8f2ba..9de26c78ec4c 100644
@@ -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

s/ans/and

Andrew

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.

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

index 9de26c78ec4c..599eecf1a02c 100644
@@ -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

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

Andrew

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.

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

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

Yes.

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

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)

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

Marek

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.

---
 arch/arm/boot/dts/armada-385-turris-omnia.dts | 19 ++++++++++++++++++-
 1 file changed, 18 insertions(+), 1 deletion(-)

index 599eecf1a02c..67048a88e327 100644
@@ -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

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

Andrew

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.

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

index 67048a88e327..771df9bbe271 100644
@@ -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

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.

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

index 771df9bbe271..99fe46911440 100644
@@ -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

Andrew