Hi Everyone, On Wed, Apr 24, 2019 at 02:33:48PM -0400, Alan Stern wrote: > On Wed, 24 Apr 2019, Claus H. Stovgaard wrote: > > > Hi Balbi and other USB developers. > > > > I am developing camera devices based on Xilinx ZynqMP, using the > > gadget framework and the build-in dwc3 core of the ZynqMP as USB3 > > controller and the build-in Cirrus SERDES as phy. > > Testing with a number of hosts and Windows 7, has shown sporadic > > reconnects when leaving U2/U1, caused by failing link training, where > > the host resets the bus. Sometime it also means it reconnect via > > USB2. > > > > So to overcome this, I will like to have the option for disabling > > U1/U2 on the core when working with those hosts. > > > > Currently I have made a hack in ep0.c where I return EINVAL in > > dwc3_ep0_handle_u1 and dwc3_ep0_handle_u2 together with not setting > > DWC3_DCTL_ACCEPTU1ENA and DWC3_DCTL_ACCEPTU2ENA in > > dwc3_ep0_set_config > > Will though prefer a solution possible to upstream, so was thinking > > about adding two devicetree bindings. > > > > * snps,u1_disable_as_gadget: When set the core will not enable U1 if > > requested from host, nor initiate U1. > > * snps,u2_disable_as_gadget: When set the core will not enable U2 if > > requested from host, nor initiate U2. > > > > If you think this might be something which can be upstreamed I will > > prepare the code and send a patch for discussion. > > On the other hand, if you think that disabling U1/U2 via device tree > > as suggested should not be a feature no need for me to try making it > > a feature. > > Speaking as an interested bystander, I would first wonder why your > hardware fails during link training. If it is properly designed, that > should not happen. The fact that it does happen suggests your devices > might also experience problems during normal data transport. I was recently debugging a similar problem where my device would improperly transition from U2 to U0. This caused the connection to reset and the device to be re-enumerated as a USB2 device. Our design uses an Intel CherryTrail z8550 SoC that has an internal dwc3 USB device controller. I worked with Felipe a couple of weeks ago [1] to come up with the same workaround you mentioned above. I disable device-initiated U1/U2 in ep0_set_config and also return -EINVAL when handling SetFeature requests from the USB host. I've attached my patch for kernel 4.9.115 below for reference. Although we have applied this workaround, we still have not identified a root cause of the issue. Intel has no known errata related to the symptoms we are experiencing. We've done quite a bit of analysis on our design and are pretty confident we've followed all design guidelines for USB. Cheers, Rob Weber [1] https://marc.info/?t=155349935600001&r=1&w=2 --- drivers/usb/dwc3/ep0.c | 26 ++++++++++++++++---------- 1 file changed, 16 insertions(+), 10 deletions(-) diff --git a/drivers/usb/dwc3/ep0.c b/drivers/usb/dwc3/ep0.c index 2331469f943d..78b75d65b435 100644 --- a/drivers/usb/dwc3/ep0.c +++ b/drivers/usb/dwc3/ep0.c @@ -433,10 +433,13 @@ static int dwc3_ep0_handle_feature(struct dwc3 *dwc, return -EINVAL; reg = dwc3_readl(dwc->regs, DWC3_DCTL); - if (set) - reg |= DWC3_DCTL_INITU1ENA; - else + if (set) { + /* reg |= DWC3_DCTL_INITU1ENA; */ + dwc3_trace(trace_dwc3_ep0, "GBX: Skipping U1 Enable"); + return -EINVAL; + } else { reg &= ~DWC3_DCTL_INITU1ENA; + } dwc3_writel(dwc->regs, DWC3_DCTL, reg); break; @@ -448,10 +451,13 @@ static int dwc3_ep0_handle_feature(struct dwc3 *dwc, return -EINVAL; reg = dwc3_readl(dwc->regs, DWC3_DCTL); - if (set) - reg |= DWC3_DCTL_INITU2ENA; - else + if (set) { + /* reg |= DWC3_DCTL_INITU2ENA; */ + dwc3_trace(trace_dwc3_ep0, "GBX: Skipping U2 Enable"); + return -EINVAL; + } else { reg &= ~DWC3_DCTL_INITU2ENA; + } dwc3_writel(dwc->regs, DWC3_DCTL, reg); break; @@ -567,7 +573,7 @@ static int dwc3_ep0_set_config(struct dwc3 *dwc, struct usb_ctrlrequest *ctrl) enum usb_device_state state = dwc->gadget.state; u32 cfg; int ret; - u32 reg; + /* u32 reg; */ cfg = le16_to_cpu(ctrl->wValue); @@ -594,9 +600,9 @@ static int dwc3_ep0_set_config(struct dwc3 *dwc, struct usb_ctrlrequest *ctrl) * Enable transition to U1/U2 state when * nothing is pending from application. */ - reg = dwc3_readl(dwc->regs, DWC3_DCTL); - reg |= (DWC3_DCTL_ACCEPTU1ENA | DWC3_DCTL_ACCEPTU2ENA); - dwc3_writel(dwc->regs, DWC3_DCTL, reg); + /* reg = dwc3_readl(dwc->regs, DWC3_DCTL); */ + /* reg |= (DWC3_DCTL_ACCEPTU1ENA | DWC3_DCTL_ACCEPTU2ENA); */ + /* dwc3_writel(dwc->regs, DWC3_DCTL, reg); */ } break; --
