Mass Storage Gadget Device Falls from SuperSpeed to High Speed
Subsystems (2)
Hi Felipe,
Quoted 58 lines · Felipe Balbi
Quoted 57 lines · Felipe Balbi
Quoted 22 lines
Quoted 5 lines
modphy is the USB PHY integrated in your SoC. There's no control for that from OS side, only BIOS unfortunately. There is, however, one thing we can try. DWC3 has several quirk flags for known quirky PHYs; perhaps CHT needs one of those. Can you try with this patch and let me know whether it helps?
Sure thing, I will try tomorrow. Could you possibly explain what a quirk is as it relates to the kernel? I see this all over the source tree but never knew how it was used. Does the dwc3 also know about "quirks" and these particular flags? or are these flags just specific to the kernel and its functionality?
Quoted 10 lines
modified
drivers/usb/dwc3/dwc3-pci.c(quoted diff) +2
-0@@ -105,6 +105,8 @@ static int dwc3_byt_enable_ulpi_refclock(struct pci_dev *pci) static const struct property_entry dwc3_pci_intel_properties[] = { PROPERTY_ENTRY_STRING("dr_mode", "peripheral"), PROPERTY_ENTRY_BOOL("linux,sysdev_is_parent"), + PROPERTY_ENTRY_BOOL("snps,dis_u3_susphy_quirk"), + PROPERTY_ENTRY_BOOL("snps,dis_u2_susphy_quirk"), {} };Quoted 1 line
Quoted 1 line
These two quirks will PHY suspend. There are other relevant quirk flags
^^^^^^^^
will DISABLE phy suspend :-)Quoted 1 line
What do you mean by PHY suspend? Will it disable U2/U3 for the dwc3? I
No, no. DWC3 can still enter U1/U2/U3, but the PHY will not enter the matching P1/P2/P3 state.
Quoted 3 lines
see it modified the
DWC3_GUSB2PHYCFG_SUSPHYbit in the configuration register, but I don't have access to the dwc3 databook to dig deeper into this.The second quirk flag (snps,dis_u2_susphy_quirk) will tell dwc3 to *NOT* request USB2 PHY to enter low power state. While the first flag (snps,dis_u3_susphy_quirk) will tell dwc3 to *NOT* request USB3 PHY to enter low power state.
In reality, they are a single block of HW but they _can_ be different and even if they are a single block, they _can_ have separate clock domains and we _may_ be able to control them separately. I haven't read documentation for modphy because the OS doesn't touch that. If necessary, I can try to find out more details about it, but I will probably take some time to find the correct documentation.
Quoted 10 lines
Quoted 7 lines
which we can try in case these two don't help. I'd like to figure out exactly which quirk flag helps (if any). After that, we would need to check if a similar problem happens on any CHT system or just your design.
If it happens on any other system, then I can make sure we add a quirk flag to all CHTs.
Sounds good!
Quoted 7 lines
Quoted 5 lines
Quoted 3 lines
Thanks for taking the time to answer my questions! It's definitely helpful for my understanding of USB. I'm learning quite a bit of new information with each email and it's pretty awesome.
No problems at all, happy to help.
Any updates here? Hopefully the quirk flags above helped.
Thanks for following up. My team and I were doing a bunch of testing today as well as this past weekend. We are still seeing mixed results with the suggestion you provided to us earlier. I unfortunately do not have any logs, traces, or analyzer captures to back this up yet, but I hope to gather more concrete data tomorrow.
There were two main issues we observed with the quirk flags above:
* Host mode no longer worked with these quirks applied. Please keep in mind that we backported part of a patchset [1] to support switching the internal SS mux from the xHCI controller to the xDCI controller. At a quick glance, do you see anything in this patchset that you think might cause a problem with the role switch driver communicating with the xHCI controller when the quirks are applied?
* We still saw the drop from SS to HS on a Windows laptop. Linux and Mac seemed to be fine, but there's definitely a chance we did not test with enough samples to confidently say this quirk is stable with those hosts.
So tomorrow I hope to gather more concrete data to back up these findings. Our USB protocol analyzer is also out-of-order at the moment, so we might not be able to get analyzer data for a day or two either.
Are there some other quirks you would like us to try out?
Over the weekend, one of our team members was hacking in the dwc3 driver to try and disable LPM at various points in the chain of events. I've attached a patch that we believe is pretty stable. We have never seen it drop from SS to HS. USB type A to type C connections from any host to our device seem pretty stable. I've attached an archive called "gnarbox-success-a-to-c.tar.xz" That contains the trace events for a successful run where we connect the device to a Windows host and it enumerates at SuperSpeed.
This patch isn't entirely stable as I haven't been able to get a reliable USB C-to-C connection on any host yet. I do not have any logs or traces for this particular case and I hope to get them to you soon.
Do you remember the "failed to enable ep0out" error I briefly mentioned earlier? I was able to capture some trace events while this occurs. This issue occaisonally occurs when modprobing g_mass_storage. The exact steps are as follows:
1. switch from host mode to device mode (echo device > /sys/class/usb_role/.../role)
2. modprobe g_mass_storage ...
3. Try and rest device mode with a host computer, but you'll observe that device mode is not working.
4. modprobe -r g_mass_storage to remove the gadget driver.
5. switch back to host mode (echo host > /sys/class/usb_role/.../role)
6. switch back to device mode
7. modprobe g_mass_storage ...
8. Observe the error
I've attached the traces and dmesg output in an archive titled "gnarbox-ep0out-failure.tar.xz". Please let me know if you need more information about this particular issue.
Lastly, I started digging through the errata available for our SoC series [2] and there are a few that stand out to me, including CHT40, CHT47, CHT48, and CHT50. CHT47 is the most concerning because I recall seeing an "Unknown" LFPS status coming from the device as it is trying to leave U2. I asked the support specialists on our IPS ticket about these errata and I'm hoping to hear back from them within the day. Do any of these errata stand out to you as possible culprits?
Thanks for taking a look at all of this again!
Cheers,
Rob Weber
[1] https://lkml.org/lkml/2018/3/20/387
[2] https://www.intel.com/content/dam/www/public/us/en/documents/specification-updates/atom-z8000-spec-update.pdf
From de8b9e63df1e24d1b9402ae13224edbdae80cd5c Mon Sep 17 00:00:00 2001
From: Mike Lemmon <[email protected]>
Date: Sun, 31 Mar 2019 13:18:12 -0700
Subject: [PATCH] dwc3: Disable U1 and U2 link states
--- drivers/usb/dwc3/core.c | 10 ++++++++++ drivers/usb/dwc3/debug.h | 28 ++++++++++++++++++++++++++ drivers/usb/dwc3/ep0.c | 10 ++++++++++ drivers/usb/dwc3/gadget.c | 51 ++++++++++++++++++++++++++++++++++++++++++++--- 4 files changed, 96 insertions(+), 3 deletions(-)
drivers/usb/dwc3/core.c +10 -0
@@ -641,6 +641,16 @@ static int dwc3_core_init(struct dwc3 *dwc) goto err0; } +printk (KERN_INFO "GBX: (dwc3) dwc->revision=%x patch=0038-disable-U1-U2\n" , dwc->revision); + +/* +// Tried overriding to old revision but it just hung on transition +// to mass-storage mode... +printk(KERN_INFO "GBX: (dwc3) Overriding dwc3 revision from %x to %x", +dwc->revision, DWC3_REVISION_173A); +dwc->revision = DWC3_REVISION_173A; +*/ +/* * Write Linux Version Code to our GUID register so it's easy to figure * out which kernel version a bug was found.
drivers/usb/dwc3/debug.h +28 -0
@@ -124,6 +124,34 @@ dwc3_gadget_link_string(enum dwc3_link_state link_state) } } +static inline const char * +dwc3_dsts_speed_string (u8 speed) +{ +static char str[256 ]; +switch (speed) { +case DWC3_DSTS_SUPERSPEED_PLUS :sprintf (str,"Super-Speed-Plus" );break ; +case DWC3_DSTS_SUPERSPEED :sprintf (str,"Super-Speed" );break ; +case DWC3_DSTS_HIGHSPEED :sprintf (str,"High-Speed" );break ; +case DWC3_DSTS_FULLSPEED :sprintf (str,"Full-Speed" );break ; +case DWC3_DSTS_LOWSPEED :sprintf (str,"Low-Speed" );break ; +default :sprintf (str,"UNKNOWN" ); + } +return str; +} + +static inline const char * +dwc3_dr_mode_string (enum usb_dr_mode mode) +{ +static char str[256 ]; +switch (mode) { +case USB_DR_MODE_PERIPHERAL :sprintf (str,"Device-Mode" );break ; +case USB_DR_MODE_HOST :sprintf (str,"Host-Mode" );break ; +case USB_DR_MODE_OTG :sprintf (str,"OTG-Mode" );break ; +default :sprintf (str,"Unsupported Mode (%d)" , mode); + } +return str; +} +/** * dwc3_gadget_event_string - returns event name * @event: the event code
drivers/usb/dwc3/ep0.c +10 -0
@@ -420,12 +420,14 @@ static int dwc3_ep0_handle_feature(struct dwc3 *dwc,switch (wValue) {case USB_DEVICE_REMOTE_WAKEUP : +dwc3_trace (trace_dwc3_gadget,"GBX: USB_RECIP_DEVICE: USB_DEVICE_REMOTE_WAKEUP" );break ;/* * 9.4.1 says only only for SS, in AddressState only for * default control pipe */ case USB_DEVICE_U1_ENABLE : +dwc3_trace (trace_dwc3_gadget,"GBX: USB_RECIP_DEVICE: USB_DEVICE_U1_ENABLE" );if (state !=USB_STATE_CONFIGURED )return -EINVAL ; if ((dwc->speed != DWC3_DSTS_SUPERSPEED) &&
@@ -441,6 +443,7 @@ static int dwc3_ep0_handle_feature(struct dwc3 *dwc,break ;case USB_DEVICE_U2_ENABLE : +dwc3_trace (trace_dwc3_gadget,"GBX: USB_RECIP_DEVICE: USB_DEVICE_U2_ENABLE" );if (state !=USB_STATE_CONFIGURED )return -EINVAL ; if ((dwc->speed != DWC3_DSTS_SUPERSPEED) &&
@@ -456,9 +459,11 @@ static int dwc3_ep0_handle_feature(struct dwc3 *dwc,break ;case USB_DEVICE_LTM_ENABLE : +dwc3_trace (trace_dwc3_gadget,"GBX: USB_RECIP_DEVICE: USB_DEVICE_LTM_ENABLE" );return -EINVAL ;case USB_DEVICE_TEST_MODE : +dwc3_trace (trace_dwc3_gadget,"GBX: USB_RECIP_DEVICE: USB_DEVICE_TEST_MODE" );if ((wIndex &0xff ) !=0 )return -EINVAL ;if (!set)
@@ -478,6 +483,7 @@ static int dwc3_ep0_handle_feature(struct dwc3 *dwc, }break ;default : +dwc3_trace (trace_dwc3_gadget,"GBX: USB_RECIP_DEVICE: UNKNOWN FEATURE" );return -EINVAL ; }break ;
@@ -485,6 +491,7 @@ static int dwc3_ep0_handle_feature(struct dwc3 *dwc,case USB_RECIP_INTERFACE :switch (wValue) {case USB_INTRF_FUNC_SUSPEND : +dwc3_trace (trace_dwc3_gadget,"GBX: USB_RECIP_INTERFACE: USB_INTRF_FUNC_SUSPEND" );if (wIndex &USB_INTRF_FUNC_SUSPEND_LP )/* XXX enable Low power suspend */ ;
@@ -493,6 +500,7 @@ static int dwc3_ep0_handle_feature(struct dwc3 *dwc, ;break ;default : +dwc3_trace (trace_dwc3_gadget,"GBX: USB_RECIP_INTERFACE: UNKNOWN" );return -EINVAL ; }break ;
@@ -500,6 +508,7 @@ static int dwc3_ep0_handle_feature(struct dwc3 *dwc,case USB_RECIP_ENDPOINT :switch (wValue) {case USB_ENDPOINT_HALT : +dwc3_trace (trace_dwc3_gadget,"GBX: USB_RECIP_ENDPOINT: USB_ENDPOINT_HALT" ); dep =dwc3_wIndex_to_dep (dwc, ctrl->wIndex);if (!dep)return -EINVAL ;
@@ -510,6 +519,7 @@ static int dwc3_ep0_handle_feature(struct dwc3 *dwc,return -EINVAL ;break ;default : +dwc3_trace (trace_dwc3_gadget,"GBX: USB_RECIP_ENDPOINT: UNKNOWN" );return -EINVAL ; }break ;
drivers/usb/dwc3/gadget.c +48 -3
127 lines not on this page · raw message
Signature · 1 line
2.14.1