Serge Semin writes: > Our Baikal-T1 SoC is equipped with DWC USB3 IP core as a USB2.0 bus > controller. In general the DWC USB3 driver is working well for it except > the ULPI-bus part. We've found out that the DWC USB3 ULPI-bus driver detected > PHY with VID:PID tuple as 0x0000:0x0000, which of course wasn't true since > it was supposed to be 0x0424:0x0006. After a short digging inside the > ulpi.c code and studying the DWC USB3 documentation, it has been > discovered that the ULPI bus IO ops didn't work quite correct. The > busy-loop had stopped waiting before the actual operation was finished. We > found out that the problem was caused by several bugs hidden in the DWC > USB3 ULPI-bus IO implementation. > > First of all in accordance with the DWC USB3 databook [1] the ULPI IO > busy-loop is supposed to use the GUSB2PHYACCn.VStsDone flag as an > indication of the PHY vendor control access completion. Instead it polled > the GUSB2PHYACCn.VStsBsy flag, which as we discovered can be cleared a > bit before the VStsDone flag. > > Secondly having the simple counter-based loop in the modern kernel is > really a weak design of the busy-looping pattern especially seeing the > ULPI operations delay can be easily estimated [2], since the bus clock is > fixed to 60MHz. > > Finally the root cause of the denoted in the prologue problem was due to > the Suspend PHY DWC USB3 feature perception. The commit e0082698b689 > ("usb: dwc3: ulpi: conditionally resume ULPI PHY") introduced the Suspend > USB2.0 HS/FS/LS PHY regression as the Low-power consumption mode would be > disable after a first attempt to read/write from the ULPI PHY control > registers, and still didn't fix the problem it was originally intended for > since the very first attempt of the ULPI PHY control registers IO would > need much more time than the busy-loop provided. So instead of disabling > the Suspend USB2.0 HS/FS/LS PHY feature we suggest to just extend the > busy-loop delay in case if the GUSB2PHYCFGn.SusPHY flag set to 1. By doing > so we'll eliminate the regression and the fix the false busy-loop timeout > problem. > > [1] Synopsys DesignWare Cores SuperSpeed USB 3.0 xHCI Host Controller > Databook, 2.70a, December 2013, p.388 > > [1] UTMI+ Low Pin Interface (ULPI) Specification, Revision 1.1, > October 20, 2004, pp. 30 - 36. > > Fixes: e0082698b689 ("usb: dwc3: ulpi: conditionally resume ULPI PHY") > Fixes: 88bc9d194ff6 ("usb: dwc3: add ULPI interface support") > Signed-off-by: Serge Semin > Cc: Alexey Malahov > Cc: Pavel Parkhomenko > Cc: linux-usb@vger.kernel.org > Cc: linux-kernel@vger.kernel.org these should go in the relevant commits instead. > Serge Semin (3): > usb: dwc3: ulpi: Use VStsDone to detect PHY regs access completion > usb: dwc3: ulpi: Replace CPU-based busyloop with Protocol-based one > usb: dwc3: ulpi: Fix USB2.0 HS/FS/LS PHY suspend regression make sure fixes don't depend on other rework otherwise they can't be taken during the -rc cycle. -- balbi