Hi, Serge Semin <Sergey.Semin@xxxxxxxxxxxxxxxxxxxx> writes: > Originally the procedure of the ULPI transaction finish detection has been > developed as a simple busy-loop with just decrementing counter and no > delays. It's wrong since on different systems the loop will take a > different time to complete. So if the system bus and CPU are fast enough > to overtake the ULPI bus and the companion PHY reaction, then we'll get to > take a false timeout error. Fix this by converting the busy-loop procedure > to take the standard bus speed, address value and the registers access > mode into account for the busy-loop delay calculation. > > Here is the way the fix works. It's known that the ULPI bus is clocked > with 60MHz signal. In accordance with [1] the ULPI bus protocol is created > so to spend 5 and 6 clock periods for immediate register write and read > operations respectively, and 6 and 7 clock periods - for the extended > register writes and reads. Based on that we can easily pre-calculate the > time which will be needed for the controller to perform a requested IO > operation. Note we'll still preserve the attempts counter in case if the > DWC USB3 controller has got some internals delays. > > [1] UTMI+ Low Pin Interface (ULPI) Specification, Revision 1.1, > October 20, 2004, pp. 30 - 36. > > Fixes: 88bc9d194ff6 ("usb: dwc3: add ULPI interface support") > Signed-off-by: Serge Semin <Sergey.Semin@xxxxxxxxxxxxxxxxxxxx> > --- > drivers/usb/dwc3/ulpi.c | 18 +++++++++++++++--- > 1 file changed, 15 insertions(+), 3 deletions(-) > > diff --git a/drivers/usb/dwc3/ulpi.c b/drivers/usb/dwc3/ulpi.c > index 20f5d9aba317..0dbc826355a5 100644 > --- a/drivers/usb/dwc3/ulpi.c > +++ b/drivers/usb/dwc3/ulpi.c > @@ -7,6 +7,8 @@ > * Author: Heikki Krogerus <heikki.krogerus@xxxxxxxxxxxxxxx> > */ > > +#include <linux/delay.h> > +#include <linux/time64.h> > #include <linux/ulpi/regs.h> > > #include "core.h" > @@ -17,12 +19,22 @@ > DWC3_GUSB2PHYACC_ADDR(ULPI_ACCESS_EXTENDED) | \ > DWC3_GUSB2PHYACC_EXTEND_ADDR(a) : DWC3_GUSB2PHYACC_ADDR(a)) > > -static int dwc3_ulpi_busyloop(struct dwc3 *dwc) > +#define DWC3_ULPI_BASE_DELAY DIV_ROUND_UP(NSEC_PER_SEC, 60000000L) > + > +static int dwc3_ulpi_busyloop(struct dwc3 *dwc, u8 addr, bool read) > { > + unsigned long ns = 5L * DWC3_ULPI_BASE_DELAY; > unsigned count = 1000; > u32 reg; > > + if (addr >= ULPI_EXT_VENDOR_SPECIFIC) > + ns += DWC3_ULPI_BASE_DELAY; > + > + if (read) > + ns += DWC3_ULPI_BASE_DELAY; > + > while (count--) { > + ndelay(ns); could we allow for a sleep here instead of a delay? Also, I wonder if you need to make this so complex or should we just take the larger access time of 7 clock cycles. -- balbi
Attachment:
signature.asc
Description: PGP signature