Skip to contentSkip to conversation

vf610-zii-dev updates

PATCH New Russell King - ARM Linux 7 participants 19 messages Dec 20, 2017 – Dec 26, 2017 Device tree bindings

lore
Share & tools
lore
#0 lore raw

cover vf610-zii-dev updates

Hi,

These patches update the DT for the ZII VF610 boards.

The first patch fixes complaints at boot about missing DMAs on rev C boards, particularly for the SPI interface. This is because edma1 is not enabled. This seems to be a regression from the 4.10 era.

The second patch fixes an interrupt storm during boot on rev B boards, which causes boot to take 80+ seconds - this seems to be a long standing issue since the DT description was first added. The PTB28 pin is definitely GPIO 98, and GPIO 98 is definitely part of the gpio3 block, not the gpio2 block. Since GPIO 66 (which is the corresponding GPIO in gpio2) is low, and the IRQ trigger is level-low, this causes an interrupt storm.

The last two patches add an explicit description of the PHYs that are actually connected to the switch - the 88e1545 is a quad PHY, and without describing the MDIO bus, DSA assumes that any PHYs it can discover are present for the switch. As only the first three PHYs are connected, this leads the 4th port to believe it is connected to the 4th PHY when the fixed-link definition is (eventually) removed.

Head this off by providing the proper descriptions, and as we have them, also describe the interrupts for these PHYs.

Note, however, that the interrupt description is not quite correct - the 88e1545 PHYs all share one interrupt line, and there is a register in the PHY which can be used to demux the interrupt to the specific PHY. However, in this description, we ignore the demux register, and just share the interrupt between the PHYs. That much is fine, but the pinmuxing becomes problematical - if we describe the same pinmux settings for each PHY for the interrupt line, the 2nd/3rd PHYs fail. This has no known solution. Suggestions welcome.

 arch/arm/boot/dts/vf610-zii-dev-rev-b.dts | 34 ++++++++++++++++++++++++++++++-
 arch/arm/boot/dts/vf610-zii-dev.dtsi      |  4 ++++
 2 files changed, 37 insertions(+), 1 deletion(-)
Signature · 3 lines
RMK's Patch system: http://www.armlinux.org.uk/developer/patches/
FTTC broadband for 0.8mile line in suburbia: sync at 8.8Mbps down 630kbps up
According to speedtest.net: 8.21Mbps down 510kbps up
#1 lore raw

PATCH 1/4 ARM: dts: vf610-zii-dev: enable edma1

EDMA1 is required for the SPI controller used on the ZII boards to be functional. Enable EDMA1.

Fixes 14c416336820 ("ARM: dts: vfxxx: Enable DMA for DSPI on Vybrid")
---
 arch/arm/boot/dts/vf610-zii-dev.dtsi | 4 ++++
 1 file changed, 4 insertions(+)
arch/arm/boot/dts/vf610-zii-dev.dtsi +4 -0
index 6b58d3a97992..aadd36db0092 100644
@@ -96,6 +96,10 @@
 	status = "okay";
 };
 
+&edma1 {
+	status = "okay";
+};
+
 &esdhc1 {
 	pinctrl-names = "default";
 	pinctrl-0 = <&pinctrl_esdhc1>;
Signature · 1 line
2.7.4
#2 lore raw

PATCH 2/4 ARM: dts: vf610-zii-dev-rev-b: fix interrupt for GPIO expander

In reply to Russell King - ARM Linux

The interrupt specification for the GPIO expander is wrong - the expander is wired to PTB28, which is GPIO98. GPIO98 is on gpio chip 3, not 2.

In addition, the device is missing a required property. Interrupt controllers must have the "interrupt-controller" property specified. Add this.

---
 arch/arm/boot/dts/vf610-zii-dev-rev-b.dts | 3 ++-
 1 file changed, 2 insertions(+), 1 deletion(-)
arch/arm/boot/dts/vf610-zii-dev-rev-b.dts +2 -1
index acdf12ad0622..ede8649ba515 100644
@@ -371,7 +371,8 @@
 		reg = <0x22>;
 		gpio-controller;
 		#gpio-cells = <2>;
-		interrupt-parent = <&gpio2>;
+		interrupt-controller;
+		interrupt-parent = <&gpio3>;
 		interrupts = <2 IRQ_TYPE_LEVEL_LOW>;
 	};
 };
Signature · 1 line
2.7.4
#3 lore raw

PATCH 2/4 ARM: dts: vf610-zii-dev-rev-b: fix interrupt for GPIO expander

Quoted 3 lines · Russell King

The interrupt specification for the GPIO expander is wrong - the expander is wired to PTB28, which is GPIO98. GPIO98 is on gpio chip 3, not 2.

Hi Russell

I'd also seen this interrupt storm. The whole interrupt architecture for this expander does not look so good, so i just assumed it was a design issue. Instead, it was me who probably made a typ0 :-(

Andrew

#4 lore raw

PATCH 3/4 ARM: dts: vf610-zii-dev-rev-b: add PHYs for switch2

In reply to Russell King - ARM Linux

Switch 2 has an 88e1545 PHY behind it, which is a quad PHY. Only the first three PHYs are used, the remaining PHY is unused. When we wire up the SFF sockets in a later commit, the omission of this causes the fourth PHY to be used for port 3. Specifying the PHYs in DT avoids the auto-probing of the bus, and discovery of this PHY.

---
 arch/arm/boot/dts/vf610-zii-dev-rev-b.dts | 17 +++++++++++++++++
 1 file changed, 17 insertions(+)
arch/arm/boot/dts/vf610-zii-dev-rev-b.dts +17 -0
index ede8649ba515..782b69a3acdf 100644
@@ -255,16 +255,19 @@
 					port at 0 {
 						reg = <0>;
 						label = "lan6";
+						phy-handle = <&switch2phy0>;
 					};
 
 					port at 1 {
 						reg = <1>;
 						label = "lan7";
+						phy-handle = <&switch2phy1>;
 					};
 
 					port at 2 {
 						reg = <2>;
 						label = "lan8";
+						phy-handle = <&switch2phy2>;
 					};
 
 					port at 3 {
@@ -304,6 +307,20 @@
 						};
 					};
 				};
+				mdio {
+					#address-cells = <1>;
+					#size-cells = <0>;
+
+					switch2phy0: phy at 0 {
+						reg = <0>;
+					};
+					switch2phy1: phy at 1 {
+						reg = <1>;
+					};
+					switch2phy2: phy at 2 {
+						reg = <2>;
+					};
+				};
 			};
 		};
 
Signature · 1 line
2.7.4
#5 lore raw

PATCH 3/4 ARM: dts: vf610-zii-dev-rev-b: add PHYs for switch2

Quoted 7 lines · Russell King

Switch 2 has an 88e1545 PHY behind it, which is a quad PHY. Only the first three PHYs are used, the remaining PHY is unused. When we wire up the SFF sockets in a later commit, the omission of this causes the fourth PHY to be used for port 3. Specifying the PHYs in DT avoids the auto-probing of the bus, and discovery of this PHY.

Andrew

#6 lore raw

PATCH 3/4 ARM: dts: vf610-zii-dev-rev-b: add PHYs for switch2

In reply to Russell King
Quoted 7 lines · Russell King

Switch 2 has an 88e1545 PHY behind it, which is a quad PHY. Only the first three PHYs are used, the remaining PHY is unused. When we wire up the SFF sockets in a later commit, the omission of this causes the fourth PHY to be used for port 3. Specifying the PHYs in DT avoids the auto-probing of the bus, and discovery of this PHY.

Ah this is elegant and what Andrew requested me to do for another switch as well. Makes perfect sense. FWIW:

Yours,
Linus Walleij

#7 lore raw

PATCH 4/4 ARM: dts: vf610-zii-dev-rev-b: add interrupts for 88e1545 PHY

In reply to Russell King - ARM Linux

The 88e1545 PHY has its interrupts wired to the VF610, so we might as well use them.

---
This is certainly not correct, as all PHYs on this device share the same interrupt line, but we can't specify the pinmux settings individually on each PHY. How should this be handled?

---
 arch/arm/boot/dts/vf610-zii-dev-rev-b.dts | 14 ++++++++++++++
 1 file changed, 14 insertions(+)
arch/arm/boot/dts/vf610-zii-dev-rev-b.dts +14 -0
index 782b69a3acdf..d6786c5d0109 100644
@@ -312,12 +312,20 @@
 					#size-cells = <0>;
 
 					switch2phy0: phy at 0 {
+						interrupt-parent = <&gpio0>;
+						interrupts = <22 IRQ_TYPE_LEVEL_LOW>;
 						reg = <0>;
+						pinctrl-names = "default";
+						pinctrl-0 = <&pinctrl_mv88e1545>;
 					};
 					switch2phy1: phy at 1 {
+						interrupt-parent = <&gpio0>;
+						interrupts = <22 IRQ_TYPE_LEVEL_LOW>;
 						reg = <1>;
 					};
 					switch2phy2: phy at 2 {
+						interrupt-parent = <&gpio0>;
+						interrupts = <22 IRQ_TYPE_LEVEL_LOW>;
 						reg = <2>;
 					};
 				};
@@ -488,6 +496,12 @@
 		>;
 	};
 
+	pinctrl_mv88e1545: pinctrl-mv88e1545 {
+		fsl,pins = <
+			VF610_PAD_PTB0__GPIO_22		0x219d
+		>;
+	};
+
 	pinctrl_pca9554_22: pinctrl-pca95540-22 {
 		fsl,pins = <
 			VF610_PAD_PTB28__GPIO_98	0x219d
Signature · 1 line
2.7.4
#8 lore raw

PATCH 4/4 ARM: dts: vf610-zii-dev-rev-b: add interrupts for 88e1545 PHY

Quoted 8 lines · Russell King

The 88e1545 PHY has its interrupts wired to the VF610, so we might as well use them.

---
This is certainly not correct, as all PHYs on this device share the same interrupt line, but we can't specify the pinmux settings individually on each PHY. How should this be handled?

Hi Russell

You could put it as a hog on the gpio controller node. However, i don't think that is much better.

Andrew

#9 lore raw

PATCH 4/4 ARM: dts: vf610-zii-dev-rev-b: add interrupts for 88e1545 PHY

In reply to Russell King
Quoted 8 lines · Russell King

The 88e1545 PHY has its interrupts wired to the VF610, so we might as well use them.

---
This is certainly not correct, as all PHYs on this device share the same interrupt line, but we can't specify the pinmux settings individually on each PHY. How should this be handled?

I do not know the details of the Marvell switch.

Sorry for any possible misunderstandings below.

What I did with the Realtek switch I was playing around with was to create a separate irqchip, also in the device tree, embedded inside the DSA switch, then referenced the IRQs from that chip as 0, 1 .. n.

The patches are here:
DTS:
https://marc.info/?l=linux-netdev&m=150992420713391&w=2
Driver:
https://marc.info/?l=linux-netdev&m=150992421113393&w=2

Note that this RFC is wrong: it assigns the IRQs to ports instead of PHYs, but the idea with an IRQchip inside the DSA is pretty solid IMO. (I will rewrite it using your method of a separate mdio bus node and phy-handle references.)

Anyway I was inspired to this model from certain PCI bridges that contain an IRQ demuxer and thus instantiate an irqchip for this, that is then part of the bridge itself.

Then for the pin control, I guess the irqchip inside the bridge should be the entity taking the IRQ from the GPIO-backed irq controller and also the pin control handle. As pin control handles are tied to Linux devices, that requires it to be a device proper though. I don't know if it's possible to properly spawn a device for this irqchip from the switch, but I guess it is what I would try.

I hope this helps.

Yours,
Linus Walleij

#10 lore raw

PATCH 4/4 ARM: dts: vf610-zii-dev-rev-b: add interrupts for 88e1545 PHY

Quoted 6 lines · Linus Walleij
Quoted 3 lines · Russell King

The 88e1545 PHY has its interrupts wired to the VF610, so we might as well use them.

Quoted 7 lines
Quoted 5 lines

---
This is certainly not correct, as all PHYs on this device share the same interrupt line, but we can't specify the pinmux settings individually on each PHY. How should this be handled?

I do not know the details of the Marvell switch.

Hi Linus

The 88e1545 is a discreet quad PHY. It is connected to the switch, but not integrated into the switch. All its interrupt handling is done with a GPIO onto the Freescale processor, via a GPIO. There is nothing DSA related here at all with respect to the interrupt. It is just a normal GPIO interrupt. What is a bit odd is that it one shared interrupt for all four PHYs.

What you described with an irqchip inside the switch is what we actually do for the internal PHYs on Marvell devices. And it is what i recommend for all DSA drivers. Expose standard IRQs, and let phylib use them in its normal way.

Andrew

#11 lore raw

PATCH 4/4 ARM: dts: vf610-zii-dev-rev-b: add interrupts for 88e1545 PHY

Quoted 24 lines · Andrew Lunn
Quoted 6 lines · Linus Walleij
Quoted 3 lines · Russell King

The 88e1545 PHY has its interrupts wired to the VF610, so we might as well use them.

Quoted 7 lines
Quoted 5 lines

---
This is certainly not correct, as all PHYs on this device share the same interrupt line, but we can't specify the pinmux settings individually on each PHY. How should this be handled?

I do not know the details of the Marvell switch.

Hi Linus

The 88e1545 is a discreet quad PHY. It is connected to the switch, but not integrated into the switch. All its interrupt handling is done with a GPIO onto the Freescale processor, via a GPIO. There is nothing DSA related here at all with respect to the interrupt. It is just a normal GPIO interrupt. What is a bit odd is that it one shared interrupt for all four PHYs.

Quoted 4 lines

What you described with an irqchip inside the switch is what we actually do for the internal PHYs on Marvell devices. And it is what i recommend for all DSA drivers. Expose standard IRQs, and let phylib use them in its normal way.

... and it has to be said that model doesn't work in this case, because, although there is the possibility to demux the interrupt any of the PHYs, you already need to be driving one of the PHYs.

It's not an interrupt controller itself (there's no possibility to enable/disable individual interrupts from a PHY) so it doesn't make sense.

What we have here is _really_ a shared interrupt between four separate devices, and we need a way to sanely describe resources shared between several device instances to pinmux. Unfortunately, it seems pinmux is designed around one device having exclusive use of a resource, which makes it hard to describe shared interrupts in DT.

Given that DT should be a description of the hardware, and should be independent of the OS implementation, I'd say this is a pinmux bug, because pinmux gets in the way of describing the hardware correctly. ;)

Signature · 3 lines
RMK's Patch system: http://www.armlinux.org.uk/developer/patches/
FTTC broadband for 0.8mile line in suburbia: sync at 8.8Mbps down 630kbps up
According to speedtest.net: 8.21Mbps down 510kbps up
#12 lore raw

PATCH 4/4 ARM: dts: vf610-zii-dev-rev-b: add interrupts for 88e1545 PHY

Quoted 11 lines · Russell King - ARM Linux

What we have here is _really_ a shared interrupt between four separate devices, and we need a way to sanely describe resources shared between several device instances to pinmux. Unfortunately, it seems pinmux is designed around one device having exclusive use of a resource, which makes it hard to describe shared interrupts in DT.

Given that DT should be a description of the hardware, and should be independent of the OS implementation, I'd say this is a pinmux bug, because pinmux gets in the way of describing the hardware correctly. ;)

Hm that would be annoying. But when I look at it I think it would actually work. Did you try just assigning the same pin control state to all the PHY's and see what happens?

Just set
pinctrl-names = "default";
pinctrl-0 = <&pinctrl_mv88e1545>;

on all of them?

I don't think DTC would complain at least.

When I look at the driver subsystem, I don't see anything really stopping you from doing that and even have three devices selecting the same "default" pin control state. It seems it will just wind up three times in pinctrl_select_state() and the first time it calls the pin control driver to actually set it and the three other times it finds the state is already correct and returns success.

So the way I read it actually several devices can reference the same pin control state.

That is, unless there is something I missed. Which I often do...

If it happens to work we should probably put a blurb in the DT binding that this is expected behaviour though.

Yours,
Linus Walleij

#13 lore raw

PATCH 4/4 ARM: dts: vf610-zii-dev-rev-b: add interrupts for 88e1545 PHY

Quoted 19 lines · Linus Walleij
Quoted 11 lines · Russell King - ARM Linux

What we have here is _really_ a shared interrupt between four separate devices, and we need a way to sanely describe resources shared between several device instances to pinmux. Unfortunately, it seems pinmux is designed around one device having exclusive use of a resource, which makes it hard to describe shared interrupts in DT.

Given that DT should be a description of the hardware, and should be independent of the OS implementation, I'd say this is a pinmux bug, because pinmux gets in the way of describing the hardware correctly. ;)

Hm that would be annoying. But when I look at it I think it would actually work. Did you try just assigning the same pin control state to all the PHY's and see what happens?

Quoted 5 lines

Just set
pinctrl-names = "default";
pinctrl-0 = <&pinctrl_mv88e1545>;

on all of them?

It was tried, DT was happy, but the kernel on boot complained because pinctrl objected, which caused the drivers to fail to bind:

libphy: mdio: probed
vf610-pinctrl 40048000.iomuxc: pin VF610_PAD_PTB0 already requested by !mdio-mux!mdio at 4!switch at 0!mdio:00; cannot claim for !mdio-mux!mdio at 4!switch at 0!mdio:01 vf610-pinctrl 40048000.iomuxc: pin-22 (!mdio-mux!mdio at 4!switch at 0!mdio:01) status -22
vf610-pinctrl 40048000.iomuxc: could not request pin 22 (VF610_PAD_PTB0) from group pinctrl-mv88e1545 on device 40048000.iomuxc
Marvell 88E1545 !mdio-mux!mdio at 4!switch at 0!mdio:01: Error applying setting, reverse things back
Marvell 88E1545: probe of !mdio-mux!mdio at 4!switch at 0!mdio:01 failed with error -22
vf610-pinctrl 40048000.iomuxc: pin VF610_PAD_PTB0 already requested by !mdio-mux!mdio at 4!switch at 0!mdio:00; cannot claim for !mdio-mux!mdio at 4!switch at 0!mdio:02 vf610-pinctrl 40048000.iomuxc: pin-22 (!mdio-mux!mdio at 4!switch at 0!mdio:02) status -22
vf610-pinctrl 40048000.iomuxc: could not request pin 22 (VF610_PAD_PTB0) from group pinctrl-mv88e1545 on device 40048000.iomuxc
Marvell 88E1545 !mdio-mux!mdio at 4!switch at 0!mdio:02: Error applying setting, reverse things back
Marvell 88E1545: probe of !mdio-mux!mdio at 4!switch at 0!mdio:02 failed with error -22

Signature · 3 lines
RMK's Patch system: http://www.armlinux.org.uk/developer/patches/
FTTC broadband for 0.8mile line in suburbia: sync at 8.8Mbps down 630kbps up
According to speedtest.net: 8.21Mbps down 510kbps up
#14 lore raw

PATCH 4/4 ARM: dts: vf610-zii-dev-rev-b: add interrupts for 88e1545 PHY

Quoted 29 lines · Russell King - ARM Linux
Quoted 19 lines · Linus Walleij
Quoted 11 lines · Russell King - ARM Linux

What we have here is _really_ a shared interrupt between four separate devices, and we need a way to sanely describe resources shared between several device instances to pinmux. Unfortunately, it seems pinmux is designed around one device having exclusive use of a resource, which makes it hard to describe shared interrupts in DT.

Given that DT should be a description of the hardware, and should be independent of the OS implementation, I'd say this is a pinmux bug, because pinmux gets in the way of describing the hardware correctly. ;)

Hm that would be annoying. But when I look at it I think it would actually work. Did you try just assigning the same pin control state to all the PHY's and see what happens?

Quoted 5 lines

Just set
pinctrl-names = "default";
pinctrl-0 = <&pinctrl_mv88e1545>;

on all of them?

It was tried, DT was happy, but the kernel on boot complained because pinctrl objected, which caused the drivers to fail to bind:

Quoted 12 lines

libphy: mdio: probed
vf610-pinctrl 40048000.iomuxc: pin VF610_PAD_PTB0 already requested by !mdio-mux!mdio at 4!switch at 0!mdio:00; cannot claim for !mdio-mux!mdio at 4!switch at 0!mdio:01 vf610-pinctrl 40048000.iomuxc: pin-22 (!mdio-mux!mdio at 4!switch at 0!mdio:01) status -22
vf610-pinctrl 40048000.iomuxc: could not request pin 22 (VF610_PAD_PTB0) from group pinctrl-mv88e1545 on device 40048000.iomuxc
Marvell 88E1545 !mdio-mux!mdio at 4!switch at 0!mdio:01: Error applying setting, reverse things back
Marvell 88E1545: probe of !mdio-mux!mdio at 4!switch at 0!mdio:01 failed with error -22
vf610-pinctrl 40048000.iomuxc: pin VF610_PAD_PTB0 already requested by !mdio-mux!mdio at 4!switch at 0!mdio:00; cannot claim for !mdio-mux!mdio at 4!switch at 0!mdio:02 vf610-pinctrl 40048000.iomuxc: pin-22 (!mdio-mux!mdio at 4!switch at 0!mdio:02) status -22
vf610-pinctrl 40048000.iomuxc: could not request pin 22 (VF610_PAD_PTB0) from group pinctrl-mv88e1545 on device 40048000.iomuxc
Marvell 88E1545 !mdio-mux!mdio at 4!switch at 0!mdio:02: Error applying setting, reverse things back
Marvell 88E1545: probe of !mdio-mux!mdio at 4!switch at 0!mdio:02 failed with error -22

You could also see it another way, because this is a quad PHY in a single package, you could theoretically have a representation that exposes a node container for the 4 PHYs, and that container node requests the pinmux/pinctrl. Of course, this would not work with the MDIO code which would not go one level down, and would expect the PHYs to be at the same level as the container node...

Oh well.

Signature · 1 line
Florian
#15 lore raw

PATCH 4/4 ARM: dts: vf610-zii-dev-rev-b: add interrupts for 88e1545 PHY

Quoted 43 lines · Florian Fainelli
Quoted 29 lines · Russell King - ARM Linux
Quoted 19 lines · Linus Walleij
Quoted 11 lines · Russell King - ARM Linux

What we have here is _really_ a shared interrupt between four separate devices, and we need a way to sanely describe resources shared between several device instances to pinmux. Unfortunately, it seems pinmux is designed around one device having exclusive use of a resource, which makes it hard to describe shared interrupts in DT.

Given that DT should be a description of the hardware, and should be independent of the OS implementation, I'd say this is a pinmux bug, because pinmux gets in the way of describing the hardware correctly. ;)

Hm that would be annoying. But when I look at it I think it would actually work. Did you try just assigning the same pin control state to all the PHY's and see what happens?

Quoted 5 lines

Just set
pinctrl-names = "default";
pinctrl-0 = <&pinctrl_mv88e1545>;

on all of them?

It was tried, DT was happy, but the kernel on boot complained because pinctrl objected, which caused the drivers to fail to bind:

Quoted 12 lines

libphy: mdio: probed
vf610-pinctrl 40048000.iomuxc: pin VF610_PAD_PTB0 already requested by !mdio-mux!mdio at 4!switch at 0!mdio:00; cannot claim for !mdio-mux!mdio at 4!switch at 0!mdio:01 vf610-pinctrl 40048000.iomuxc: pin-22 (!mdio-mux!mdio at 4!switch at 0!mdio:01) status -22
vf610-pinctrl 40048000.iomuxc: could not request pin 22 (VF610_PAD_PTB0) from group pinctrl-mv88e1545 on device 40048000.iomuxc
Marvell 88E1545 !mdio-mux!mdio at 4!switch at 0!mdio:01: Error applying setting, reverse things back
Marvell 88E1545: probe of !mdio-mux!mdio at 4!switch at 0!mdio:01 failed with error -22
vf610-pinctrl 40048000.iomuxc: pin VF610_PAD_PTB0 already requested by !mdio-mux!mdio at 4!switch at 0!mdio:00; cannot claim for !mdio-mux!mdio at 4!switch at 0!mdio:02 vf610-pinctrl 40048000.iomuxc: pin-22 (!mdio-mux!mdio at 4!switch at 0!mdio:02) status -22
vf610-pinctrl 40048000.iomuxc: could not request pin 22 (VF610_PAD_PTB0) from group pinctrl-mv88e1545 on device 40048000.iomuxc
Marvell 88E1545 !mdio-mux!mdio at 4!switch at 0!mdio:02: Error applying setting, reverse things back
Marvell 88E1545: probe of !mdio-mux!mdio at 4!switch at 0!mdio:02 failed with error -22

Quoted 6 lines

You could also see it another way, because this is a quad PHY in a single package, you could theoretically have a representation that exposes a node container for the 4 PHYs, and that container node requests the pinmux/pinctrl. Of course, this would not work with the MDIO code which would not go one level down, and would expect the PHYs to be at the same level as the container node...

It would actually - we have other devices that sit on buses that take several addresses, and we describe the "first" main device or use MFD for it.

For example, in the case of the TDA998x HDMI encoder, these are two devices merged into one package - the HDMI encoder at one address, and a TDA9950 at another address. Both addresses are related, so if you tie the address configuration pins, the offset is added to both base addresses. We represent the TDA998x in DT, and have the TDA998x driver create a separate device itself for the TDA9950.

What we could do for any multi-package PHY is describe the first PHY as a multi-package PHY in DT, extend the phy binding to include a PHY package index, and have the PHY driver create the MDIO devices for the other PHYs. Eg,

switch {
...

	ports {
		port at 0 {
			reg = <0>;
			label = "lan6";
			phy-handle = <&switch2phy 0>;
		};
		port at 1 {
			reg = <1>;
			label = "lan6";
			phy-handle = <&switch2phy 1>;
		};
		port at 2 {
			reg = <2>;
			label = "lan6";
			phy-handle = <&switch2phy 2>;
		};
	};

	mdio {
		#address-cells = <1>;
		#size-cells = <0>;

		switch2phy: phy at 0 {
			compatible = "marvell,88e1545", "ethernet-phy-ieee802.3-c22";
			interrupt-parent = <&gpio0>;
			interrupts = <22 IRQ_TYPE_LEVEL_LOW>;
			reg = <0>;
			pinctrl-names = "default";
			pinctrl-0 = <&pinctrl_mv88e1545>;
		};
	};
};

The "marvell,88e1545" driver would be responsible for creating the PHY devices for the other MDIO bus addresses (iow 1 to 3.)

This would be an accurate respresentation of the hardware in DT, probably more so than the trap we seem to have fallen into by describing the individual PHYs - which we've fallen into because that's how our current implementation requires us to describe them.

Since DT is supposed to be a hardware description, I think the question we ought to ask is: if we were starting afresh, how would we describe these packages that contain multiple PHYs?

Signature · 3 lines
RMK's Patch system: http://www.armlinux.org.uk/developer/patches/
FTTC broadband for 0.8mile line in suburbia: sync at 8.8Mbps down 630kbps up
According to speedtest.net: 8.21Mbps down 510kbps up
#16 lore raw

cover vf610-zii-dev updates

In reply to Russell King - ARM Linux
Quoted 35 lines · Russell King - ARM Linux

Hi,

These patches update the DT for the ZII VF610 boards.

The first patch fixes complaints at boot about missing DMAs on rev C boards, particularly for the SPI interface. This is because edma1 is not enabled. This seems to be a regression from the 4.10 era.

The second patch fixes an interrupt storm during boot on rev B boards, which causes boot to take 80+ seconds - this seems to be a long standing issue since the DT description was first added. The PTB28 pin is definitely GPIO 98, and GPIO 98 is definitely part of the gpio3 block, not the gpio2 block. Since GPIO 66 (which is the corresponding GPIO in gpio2) is low, and the IRQ trigger is level-low, this causes an interrupt storm.

The last two patches add an explicit description of the PHYs that are actually connected to the switch - the 88e1545 is a quad PHY, and without describing the MDIO bus, DSA assumes that any PHYs it can discover are present for the switch. As only the first three PHYs are connected, this leads the 4th port to believe it is connected to the 4th PHY when the fixed-link definition is (eventually) removed.

Head this off by providing the proper descriptions, and as we have them, also describe the interrupts for these PHYs.

Note, however, that the interrupt description is not quite correct - the 88e1545 PHYs all share one interrupt line, and there is a register in the PHY which can be used to demux the interrupt to the specific PHY. However, in this description, we ignore the demux register, and just share the interrupt between the PHYs. That much is fine, but the pinmuxing becomes problematical - if we describe the same pinmux settings for each PHY for the interrupt line, the 2nd/3rd PHYs fail. This has no known solution. Suggestions welcome.

Quoted 3 lines
 arch/arm/boot/dts/vf610-zii-dev-rev-b.dts | 34 ++++++++++++++++++++++++++++++-
 arch/arm/boot/dts/vf610-zii-dev.dtsi      |  4 ++++
 2 files changed, 37 insertions(+), 1 deletion(-)

There's more stuff that isn't correct in the Vybrid DTS files...

When the SoC was first added, the vfxxx.dtsi had:

+ adc0: adc at 4003b000 {
...
+ status = "disabled";
+ };
+ adc1: adc at 400bb000 {
...
+ status = "disabled";
+ };

This default status remains today.

IIO hwmon support was added later:

+ iio-hwmon {
+ compatible = "iio-hwmon";
+ io-channels = <&adc0 16>, <&adc1 16>;
+ };

which is all fine and dandy, but iio-hwmon fails to probe unless it can find _all_ the io-channels specified.

Given that the two ADC channels referenced by iio-hwmon default to being disabled, it makes no sense for iio-hwmon to default to being enabled.

What's more is that if, say, adc0 is enabled by a board, the iio-hwmon device still fails to be probed because it fails to get adc1.

As I see it, there's two possible solutions to this:
1. remove the default disabled status of the ADCs so both are always available, or
2. default iio-hwmon to disabled, and provide a label for this device so that a correct io-channels specification can be suppled for the board in question, and for iio-hwmon to be enabled where appropriate.

Signature · 3 lines
RMK's Patch system: http://www.armlinux.org.uk/developer/patches/
FTTC broadband for 0.8mile line in suburbia: sync at 8.8Mbps down 630kbps up
According to speedtest.net: 8.21Mbps down 510kbps up
#17 lore raw

cover vf610-zii-dev updates

Quoted 71 lines · Russell King - ARM Linux
Quoted 35 lines · Russell King - ARM Linux

Hi,

These patches update the DT for the ZII VF610 boards.

The first patch fixes complaints at boot about missing DMAs on rev C boards, particularly for the SPI interface. This is because edma1 is not enabled. This seems to be a regression from the 4.10 era.

The second patch fixes an interrupt storm during boot on rev B boards, which causes boot to take 80+ seconds - this seems to be a long standing issue since the DT description was first added. The PTB28 pin is definitely GPIO 98, and GPIO 98 is definitely part of the gpio3 block, not the gpio2 block. Since GPIO 66 (which is the corresponding GPIO in gpio2) is low, and the IRQ trigger is level-low, this causes an interrupt storm.

The last two patches add an explicit description of the PHYs that are actually connected to the switch - the 88e1545 is a quad PHY, and without describing the MDIO bus, DSA assumes that any PHYs it can discover are present for the switch. As only the first three PHYs are connected, this leads the 4th port to believe it is connected to the 4th PHY when the fixed-link definition is (eventually) removed.

Head this off by providing the proper descriptions, and as we have them, also describe the interrupts for these PHYs.

Note, however, that the interrupt description is not quite correct - the 88e1545 PHYs all share one interrupt line, and there is a register in the PHY which can be used to demux the interrupt to the specific PHY. However, in this description, we ignore the demux register, and just share the interrupt between the PHYs. That much is fine, but the pinmuxing becomes problematical - if we describe the same pinmux settings for each PHY for the interrupt line, the 2nd/3rd PHYs fail. This has no known solution. Suggestions welcome.

Quoted 3 lines
 arch/arm/boot/dts/vf610-zii-dev-rev-b.dts | 34 ++++++++++++++++++++++++++++++-
 arch/arm/boot/dts/vf610-zii-dev.dtsi      |  4 ++++
 2 files changed, 37 insertions(+), 1 deletion(-)

There's more stuff that isn't correct in the Vybrid DTS files...

When the SoC was first added, the vfxxx.dtsi had:

(quoted diff) +6 -0
+                       adc0: adc at 4003b000 {
...
+                               status = "disabled";
+                       };
+                       adc1: adc at 400bb000 {
...
+                               status = "disabled";
+                       };

This default status remains today.

IIO hwmon support was added later:

(quoted diff) +4 -0
+               iio-hwmon {
+                       compatible = "iio-hwmon";
+                       io-channels = <&adc0 16>, <&adc1 16>;
+               };

which is all fine and dandy, but iio-hwmon fails to probe unless it can find _all_ the io-channels specified.

Given that the two ADC channels referenced by iio-hwmon default to being disabled, it makes no sense for iio-hwmon to default to being enabled.

What's more is that if, say, adc0 is enabled by a board, the iio-hwmon device still fails to be probed because it fails to get adc1.

Quoted 6 lines

As I see it, there's two possible solutions to this:
1. remove the default disabled status of the ADCs so both are always available, or
2. default iio-hwmon to disabled, and provide a label for this device so that a correct io-channels specification can be suppled for the board in question, and for iio-hwmon to be enabled where appropriate.

I prefer solution 2. Some Vybrid SoC include a Cortex-M4 core which might access the ADCs too. In that case, the Linux device tree should have them disabled.

--
Stefan

#18 lore raw

cover vf610-zii-dev updates

In reply to Russell King - ARM Linux
Quoted 3 lines · Russell King - ARM Linux
 arch/arm/boot/dts/vf610-zii-dev-rev-b.dts | 34 ++++++++++++++++++++++++++++++-
 arch/arm/boot/dts/vf610-zii-dev.dtsi      |  4 ++++
 2 files changed, 37 insertions(+), 1 deletion(-)

Applied 1 - 3, thanks.