ALPHA250 intermittent USB failure, potential devicetree configuration issue

[chrisjbillington]

We're seeing an issue with the ALPHA250 where the USB2 port sometimes is useable and sometimes is not, probabilistically, upon power cycling. Software reboots don't appear to matter, but a hard power cycle rolls the dice again as to whether the USB port can be used. If the USB port is working after a hard power cycle then it continues to work indefinitely, and if it doesn't work after a hard power cycle, then I haven't figured out a way to get it up (via various USB controller software resets etc) other than hard power cycling until it works.

We always see the controller in dmesg:

[    0.891200] e0002000.usb supply vbus not found, using dummy regulator
[    0.937989] usb usb1: New USB device found, idVendor=1d6b, idProduct=0002
[    0.943391] usb usb1: New USB device strings: Mfr=3, Product=2, SerialNumber=1
[    0.949321] usb usb1: Product: EHCI Host Controller
[    0.952849] usb usb1: Manufacturer: Linux 4.9.0-xilinx ehci_hcd
[    0.957465] usb usb1: SerialNumber: ci_hdrc.0
[    0.972796] usbcore: registered new interface driver usbtouchscreen
[    1.096352] usbcore: registered new interface driver usbhid
[    1.102571] usbhid: USB HID core driver
[    1.116618] usbcore: registered new interface driver snd-usb-audio

But then may or may not see our connected USB devices appear in dmesg. We've seen this over a wide range of devices and multiple ALPHA250s. It may depend on electrical environment, as it seems sometimes things work for a long time, and then maybe we move the device to a different setup and it fails more often. But I can't be sure - it's pretty easy to trick yourself into thinking a random event happens more or less often when the probability hasn't changed.

I've been looking into the problem and wonder if there is an issue with the way Koheron patches the vivado-generated device tree. The devicetree patch for the ALPHA250 contains the following two USB-relevant bits:

• in pcw.dtsi:

   &usb0 {
 	dr_mode = "host";
-	phy_type = "ulpi";
+	usb-phy = <&usb_phy0>;
 	status = "okay";
 };

• in system-top.dts:

+	usb_phy0: phy1 {
+		#phy-cells = <0>;
+		compatible = "usb-nop-xceiv";
+	};

However, the patch doesn't touch zynq-7000.dtsi, which contains the following node:

usb0: usb@e0002000 {
   compatible = "xlnx,zynq-usb-2.20a", "chipidea,usb2";
   status = "disabled";
   clocks = <&clkc 28>;
   interrupt-parent = <&intc>;
   interrupts = <0 21 4>;
   reg = <0xe0002000 0x1000>;
   phy_type = "ulpi";
};

After patching and building (rendering? compiling?) the devicetree, this all results in the following devicetree node:

usb@e0002000 {
    compatible = "xlnx,zynq-usb-2.20a", "chipidea,usb2";
    status = "okay";
    clocks = <0x01 0x1c>;
    interrupt-parent = <0x04>;
    interrupts = <0x00 0x15 0x04>;
    reg = <0xe0002000 0x1000>;
    phy_type = "ulpi";
    dr_mode = "host";
    usb-phy = <0x07>;
};

i.e. it seems that Koheron's intent was to remove phy_type = "ulpi";, and yet it is still there in the final result.

Maybe it's a long shot but I'm wondering if this could be the cause of the intermittent USB failures we're seeing. I plan to patch the devicetree further to remove phy_type = "ulpi" from zynq-7000.dtsi and see if it helps (I can't try this quite now), but thought I'd post here since the modifications made to the devicetree seem deliberate enough that I hope someone who knows more than me might immediately see whether there's a problem.

Any help appreciated.

[jeanminet]

Thanks a lot Chris for your report.
This thread may be relevant:
https://adaptivesupport.amd.com/s/question/0D52E00006iHkw1SAC/zynq-usb-phy-communication-issues?language=en_US

On the ALPHA250, the USB phy reset pin is unfortunately not exposed to the FPGA.
If I remember correctly, we moved to "usb-nop-xceiv" as a solution to overcome the absence of reset pin.

It would be nice if we could reproduce your issue.
We don't really use the USB on the ALPHA250. The only thing that we do (for each board during test) is to plug a Wifi dongle before power up and check that it works afterwards (using ifconfig).

What do you connect to the USB port?
Do you have something connected to the 5V on the extension connector?

[chrisjbillington]

Thanks for the reply @jeanminet.

For now I've removed the phy_type = "ulpi"; from the device tree, and although USB wasn't working upon the subsequent power cycle, it's worked for the 3 power cycles since. Since the repackaging of this system that seems to have triggered the issue, that's a 1 / 4 success rate before the device tree change, and a 3 / 4 success rate afterwards. Not statistically significant, but we'll see. It's a "production" system so I can't power cycle it just to test, unfortunately.

We have an LCD screen attached to the USB port. On another system that occasionally experiences the same issue we have an FTDI USB-to-RS485 adaptor. Earlier this year when I first encountered the issue I tested with a USB-to-ethernet adaptor and a USB thumb drive. All these devices do work at least sometimes, validating that that it's not just e.g. missing drivers (we've modified the kernel build config to include the relevant drivers).

We do have an expansion board on the expansion connector that is powered by the 5V, but have seen the issue on systems without anything connected to the expansion connector as well. But it obviously could be shifting the probabilities.

When there's next some downtime for the system that demonstrates the issue most often, I plan to look at U-boot's output over the serial monitor at startup, and to try the approach here of modifying the FSBL to change the order PHY pins are initialised:

https://adaptivesupport.amd.com/s/question/0D52E00007G0SUzSAN/ulpi-initialization-timeout-on-zynq-based-board?language=en_US

I actually don't know what USB PHY the ALPHA250 is using. It is also a USB3320?

[chrisjbillington]

Just an update on this, the system that had a high failure rate (>50%) now has the USB controller working after every power cycle (probably 15 or so of them) since I made the below devicetree change, except for the very first power cycle after the change:

diff -rupN devicetree.orig/zynq-7000.dtsi devicetree/zynq-7000.dtsi
--- devicetree.orig/zynq-7000.dtsi
+++ devicetree/zynq-7000.dtsi
@@ -405,7 +405,6 @@
 			interrupt-parent = <&intc>;
 			interrupts = <0 21 4>;
 			reg = <0xe0002000 0x1000>;
-			phy_type = "ulpi";
 		};
 
 		usb1: usb@e0003000 {

USB not working still upon that first power cycle is disconcerting, but if we never see the problem again I'll chalk it up to maybe being too rapid a power cycle for capacitors in our power supply to discharge (the power cycle was done via a switch for power to the whole system, not the switch on the ALPH250).

I didn't really expect the devicetree change to resolve the problem, so I'll keep an eye out for it still. It's always possible that there was some minor unrelated change to the electrical environment this ALPHA250 is seeing that is just hiding the problem for now.

[chrisjbillington]

Another update: the issue has occurred again, so it looks like the devicetree change wasn't a fix after all.

[chrisjbillington]

Tentatively, I think the FSBL change actually fixes the problem!

The suggestion there is to modify the order in which two pins connected to the USB PHY are initialised by the FSBL.

I've built a FSBL and boot.bin with the below patch to ps7_init.c that reverses the order those pins are initialised, and then repeatedly power cycled a system that today was experiencing the USB failure problem on about 50% of power cycles. I swapped boot.bin back and forth between the patched and unpatched ones every 5 power cycles or so, and saw (wasn't counting, just an estimate) maybe 7/15 failures with the unpatched version and 0 out of maybe 15 with the patched version.

--- fsbl.orig/ps7_init.c	2024-12-11 20:06:44.943600527 +1100
+++ fsbl/ps7_init.c	2024-12-11 20:07:17.018891393 +1100
@@ -2893,35 +2893,6 @@
     // ..     ==> MASK : 0x00002000U    VAL : 0x00000000U
     // .. 
     EMIT_MASKWRITE(0XF8000774, 0x00003FFFU ,0x00001205U),
-    // .. TRI_ENABLE = 0
-    // .. ==> 0XF8000778[0:0] = 0x00000000U
-    // ..     ==> MASK : 0x00000001U    VAL : 0x00000000U
-    // .. L0_SEL = 0
-    // .. ==> 0XF8000778[1:1] = 0x00000000U
-    // ..     ==> MASK : 0x00000002U    VAL : 0x00000000U
-    // .. L1_SEL = 1
-    // .. ==> 0XF8000778[2:2] = 0x00000001U
-    // ..     ==> MASK : 0x00000004U    VAL : 0x00000004U
-    // .. L2_SEL = 0
-    // .. ==> 0XF8000778[4:3] = 0x00000000U
-    // ..     ==> MASK : 0x00000018U    VAL : 0x00000000U
-    // .. L3_SEL = 0
-    // .. ==> 0XF8000778[7:5] = 0x00000000U
-    // ..     ==> MASK : 0x000000E0U    VAL : 0x00000000U
-    // .. Speed = 0
-    // .. ==> 0XF8000778[8:8] = 0x00000000U
-    // ..     ==> MASK : 0x00000100U    VAL : 0x00000000U
-    // .. IO_Type = 1
-    // .. ==> 0XF8000778[11:9] = 0x00000001U
-    // ..     ==> MASK : 0x00000E00U    VAL : 0x00000200U
-    // .. PULLUP = 1
-    // .. ==> 0XF8000778[12:12] = 0x00000001U
-    // ..     ==> MASK : 0x00001000U    VAL : 0x00001000U
-    // .. DisableRcvr = 0
-    // .. ==> 0XF8000778[13:13] = 0x00000000U
-    // ..     ==> MASK : 0x00002000U    VAL : 0x00000000U
-    // .. 
-    EMIT_MASKWRITE(0XF8000778, 0x00003FFFU ,0x00001204U),
     // .. TRI_ENABLE = 1
     // .. ==> 0XF800077C[0:0] = 0x00000001U
     // ..     ==> MASK : 0x00000001U    VAL : 0x00000001U
@@ -3184,6 +3155,35 @@
     // .. 
     EMIT_MASKWRITE(0XF800079C, 0x00003FFFU ,0x00001204U),
     // .. TRI_ENABLE = 0
+    // .. ==> 0XF8000778[0:0] = 0x00000000U
+    // ..     ==> MASK : 0x00000001U    VAL : 0x00000000U
+    // .. L0_SEL = 0
+    // .. ==> 0XF8000778[1:1] = 0x00000000U
+    // ..     ==> MASK : 0x00000002U    VAL : 0x00000000U
+    // .. L1_SEL = 1
+    // .. ==> 0XF8000778[2:2] = 0x00000001U
+    // ..     ==> MASK : 0x00000004U    VAL : 0x00000004U
+    // .. L2_SEL = 0
+    // .. ==> 0XF8000778[4:3] = 0x00000000U
+    // ..     ==> MASK : 0x00000018U    VAL : 0x00000000U
+    // .. L3_SEL = 0
+    // .. ==> 0XF8000778[7:5] = 0x00000000U
+    // ..     ==> MASK : 0x000000E0U    VAL : 0x00000000U
+    // .. Speed = 0
+    // .. ==> 0XF8000778[8:8] = 0x00000000U
+    // ..     ==> MASK : 0x00000100U    VAL : 0x00000000U
+    // .. IO_Type = 1
+    // .. ==> 0XF8000778[11:9] = 0x00000001U
+    // ..     ==> MASK : 0x00000E00U    VAL : 0x00000200U
+    // .. PULLUP = 1
+    // .. ==> 0XF8000778[12:12] = 0x00000001U
+    // ..     ==> MASK : 0x00001000U    VAL : 0x00001000U
+    // .. DisableRcvr = 0
+    // .. ==> 0XF8000778[13:13] = 0x00000000U
+    // ..     ==> MASK : 0x00002000U    VAL : 0x00000000U
+    // .. 
+    EMIT_MASKWRITE(0XF8000778, 0x00003FFFU ,0x00001204U),
+    // .. TRI_ENABLE = 0
     // .. ==> 0XF80007A0[0:0] = 0x00000000U
     // ..     ==> MASK : 0x00000001U    VAL : 0x00000000U
     // .. L0_SEL = 0
@@ -7223,35 +7223,6 @@
     // ..     ==> MASK : 0x00002000U    VAL : 0x00000000U
     // .. 
     EMIT_MASKWRITE(0XF8000774, 0x00003FFFU ,0x00001205U),
-    // .. TRI_ENABLE = 0
-    // .. ==> 0XF8000778[0:0] = 0x00000000U
-    // ..     ==> MASK : 0x00000001U    VAL : 0x00000000U
-    // .. L0_SEL = 0
-    // .. ==> 0XF8000778[1:1] = 0x00000000U
-    // ..     ==> MASK : 0x00000002U    VAL : 0x00000000U
-    // .. L1_SEL = 1
-    // .. ==> 0XF8000778[2:2] = 0x00000001U
-    // ..     ==> MASK : 0x00000004U    VAL : 0x00000004U
-    // .. L2_SEL = 0
-    // .. ==> 0XF8000778[4:3] = 0x00000000U
-    // ..     ==> MASK : 0x00000018U    VAL : 0x00000000U
-    // .. L3_SEL = 0
-    // .. ==> 0XF8000778[7:5] = 0x00000000U
-    // ..     ==> MASK : 0x000000E0U    VAL : 0x00000000U
-    // .. Speed = 0
-    // .. ==> 0XF8000778[8:8] = 0x00000000U
-    // ..     ==> MASK : 0x00000100U    VAL : 0x00000000U
-    // .. IO_Type = 1
-    // .. ==> 0XF8000778[11:9] = 0x00000001U
-    // ..     ==> MASK : 0x00000E00U    VAL : 0x00000200U
-    // .. PULLUP = 1
-    // .. ==> 0XF8000778[12:12] = 0x00000001U
-    // ..     ==> MASK : 0x00001000U    VAL : 0x00001000U
-    // .. DisableRcvr = 0
-    // .. ==> 0XF8000778[13:13] = 0x00000000U
-    // ..     ==> MASK : 0x00002000U    VAL : 0x00000000U
-    // .. 
-    EMIT_MASKWRITE(0XF8000778, 0x00003FFFU ,0x00001204U),
     // .. TRI_ENABLE = 1
     // .. ==> 0XF800077C[0:0] = 0x00000001U
     // ..     ==> MASK : 0x00000001U    VAL : 0x00000001U
@@ -7514,6 +7485,35 @@
     // .. 
     EMIT_MASKWRITE(0XF800079C, 0x00003FFFU ,0x00001204U),
     // .. TRI_ENABLE = 0
+    // .. ==> 0XF8000778[0:0] = 0x00000000U
+    // ..     ==> MASK : 0x00000001U    VAL : 0x00000000U
+    // .. L0_SEL = 0
+    // .. ==> 0XF8000778[1:1] = 0x00000000U
+    // ..     ==> MASK : 0x00000002U    VAL : 0x00000000U
+    // .. L1_SEL = 1
+    // .. ==> 0XF8000778[2:2] = 0x00000001U
+    // ..     ==> MASK : 0x00000004U    VAL : 0x00000004U
+    // .. L2_SEL = 0
+    // .. ==> 0XF8000778[4:3] = 0x00000000U
+    // ..     ==> MASK : 0x00000018U    VAL : 0x00000000U
+    // .. L3_SEL = 0
+    // .. ==> 0XF8000778[7:5] = 0x00000000U
+    // ..     ==> MASK : 0x000000E0U    VAL : 0x00000000U
+    // .. Speed = 0
+    // .. ==> 0XF8000778[8:8] = 0x00000000U
+    // ..     ==> MASK : 0x00000100U    VAL : 0x00000000U
+    // .. IO_Type = 1
+    // .. ==> 0XF8000778[11:9] = 0x00000001U
+    // ..     ==> MASK : 0x00000E00U    VAL : 0x00000200U
+    // .. PULLUP = 1
+    // .. ==> 0XF8000778[12:12] = 0x00000001U
+    // ..     ==> MASK : 0x00001000U    VAL : 0x00001000U
+    // .. DisableRcvr = 0
+    // .. ==> 0XF8000778[13:13] = 0x00000000U
+    // ..     ==> MASK : 0x00002000U    VAL : 0x00000000U
+    // .. 
+    EMIT_MASKWRITE(0XF8000778, 0x00003FFFU ,0x00001204U),
+    // .. TRI_ENABLE = 0
     // .. ==> 0XF80007A0[0:0] = 0x00000000U
     // ..     ==> MASK : 0x00000001U    VAL : 0x00000000U
     // .. L0_SEL = 0
@@ -11490,35 +11490,6 @@
     // ..     ==> MASK : 0x00002000U    VAL : 0x00000000U
     // .. 
     EMIT_MASKWRITE(0XF8000774, 0x00003FFFU ,0x00001205U),
-    // .. TRI_ENABLE = 0
-    // .. ==> 0XF8000778[0:0] = 0x00000000U
-    // ..     ==> MASK : 0x00000001U    VAL : 0x00000000U
-    // .. L0_SEL = 0
-    // .. ==> 0XF8000778[1:1] = 0x00000000U
-    // ..     ==> MASK : 0x00000002U    VAL : 0x00000000U
-    // .. L1_SEL = 1
-    // .. ==> 0XF8000778[2:2] = 0x00000001U
-    // ..     ==> MASK : 0x00000004U    VAL : 0x00000004U
-    // .. L2_SEL = 0
-    // .. ==> 0XF8000778[4:3] = 0x00000000U
-    // ..     ==> MASK : 0x00000018U    VAL : 0x00000000U
-    // .. L3_SEL = 0
-    // .. ==> 0XF8000778[7:5] = 0x00000000U
-    // ..     ==> MASK : 0x000000E0U    VAL : 0x00000000U
-    // .. Speed = 0
-    // .. ==> 0XF8000778[8:8] = 0x00000000U
-    // ..     ==> MASK : 0x00000100U    VAL : 0x00000000U
-    // .. IO_Type = 1
-    // .. ==> 0XF8000778[11:9] = 0x00000001U
-    // ..     ==> MASK : 0x00000E00U    VAL : 0x00000200U
-    // .. PULLUP = 1
-    // .. ==> 0XF8000778[12:12] = 0x00000001U
-    // ..     ==> MASK : 0x00001000U    VAL : 0x00001000U
-    // .. DisableRcvr = 0
-    // .. ==> 0XF8000778[13:13] = 0x00000000U
-    // ..     ==> MASK : 0x00002000U    VAL : 0x00000000U
-    // .. 
-    EMIT_MASKWRITE(0XF8000778, 0x00003FFFU ,0x00001204U),
     // .. TRI_ENABLE = 1
     // .. ==> 0XF800077C[0:0] = 0x00000001U
     // ..     ==> MASK : 0x00000001U    VAL : 0x00000001U
@@ -11781,6 +11752,35 @@
     // .. 
     EMIT_MASKWRITE(0XF800079C, 0x00003FFFU ,0x00001204U),
     // .. TRI_ENABLE = 0
+    // .. ==> 0XF8000778[0:0] = 0x00000000U
+    // ..     ==> MASK : 0x00000001U    VAL : 0x00000000U
+    // .. L0_SEL = 0
+    // .. ==> 0XF8000778[1:1] = 0x00000000U
+    // ..     ==> MASK : 0x00000002U    VAL : 0x00000000U
+    // .. L1_SEL = 1
+    // .. ==> 0XF8000778[2:2] = 0x00000001U
+    // ..     ==> MASK : 0x00000004U    VAL : 0x00000004U
+    // .. L2_SEL = 0
+    // .. ==> 0XF8000778[4:3] = 0x00000000U
+    // ..     ==> MASK : 0x00000018U    VAL : 0x00000000U
+    // .. L3_SEL = 0
+    // .. ==> 0XF8000778[7:5] = 0x00000000U
+    // ..     ==> MASK : 0x000000E0U    VAL : 0x00000000U
+    // .. Speed = 0
+    // .. ==> 0XF8000778[8:8] = 0x00000000U
+    // ..     ==> MASK : 0x00000100U    VAL : 0x00000000U
+    // .. IO_Type = 1
+    // .. ==> 0XF8000778[11:9] = 0x00000001U
+    // ..     ==> MASK : 0x00000E00U    VAL : 0x00000200U
+    // .. PULLUP = 1
+    // .. ==> 0XF8000778[12:12] = 0x00000001U
+    // ..     ==> MASK : 0x00001000U    VAL : 0x00001000U
+    // .. DisableRcvr = 0
+    // .. ==> 0XF8000778[13:13] = 0x00000000U
+    // ..     ==> MASK : 0x00002000U    VAL : 0x00000000U
+    // .. 
+    EMIT_MASKWRITE(0XF8000778, 0x00003FFFU ,0x00001204U),
+    // .. TRI_ENABLE = 0
     // .. ==> 0XF80007A0[0:0] = 0x00000000U
     // ..     ==> MASK : 0x00000001U    VAL : 0x00000000U
     // .. L0_SEL = 0