From: Yu Chen chenyu56@huawei.com From: John Stultz john.stultz@linaro.org
According to the programming guide, to switch mode for DRD controller, the driver needs to do the following.
To switch from device to host: 1. Reset controller with GCTL.CoreSoftReset 2. Set GCTL.PrtCapDir(host mode) 3. Reset the host with USBCMD.HCRESET 4. Then follow up with the initializing host registers sequence
To switch from host to device: 1. Reset controller with GCTL.CoreSoftReset 2. Set GCTL.PrtCapDir(device mode) 3. Reset the device with DCTL.CSftRst 4. Then follow up with the initializing registers sequence
Currently we're missing step 1) to do GCTL.CoreSoftReset and step 3) of switching from host to device. John Stult reported a lockup issue seen with HiKey960 platform without these steps[1]. Similar issue is observed with Ferry's testing platform[2].
So, apply the required steps along with some fixes to Yu Chen's and John Stultz's version. The main fixes to their versions are the missing wait for clocks synchronization before clearing GCTL.CoreSoftReset and only apply DCTL.CSftRst when switching from host to device.
[1] https://lore.kernel.org/linux-usb/20210108015115.27920-1-john.stultz@linaro.... [2] https://lore.kernel.org/linux-usb/0ba7a6ba-e6a7-9cd4-0695-64fc927e01f1@gmail...
Cc: Andy Shevchenko andy.shevchenko@gmail.com Cc: Ferry Toth fntoth@gmail.com Cc: Wesley Cheng wcheng@codeaurora.org Cc: stable@vger.kernel.org Fixes: 41ce1456e1db ("usb: dwc3: core: make dwc3_set_mode() work properly") Signed-off-by: Yu Chen chenyu56@huawei.com Signed-off-by: John Stultz john.stultz@linaro.org Signed-off-by: Thinh Nguyen Thinh.Nguyen@synopsys.com --- Note: Only some basic mode switching tests were done using our HAPS platform. It'd be great if we can have some "Tested-by" with some real hardwares. Thanks.
drivers/usb/dwc3/core.c | 32 ++++++++++++++++++++++++++++++++ 1 file changed, 32 insertions(+)
diff --git a/drivers/usb/dwc3/core.c b/drivers/usb/dwc3/core.c index 5c25e6a72dbd..4ac2895331b7 100644 --- a/drivers/usb/dwc3/core.c +++ b/drivers/usb/dwc3/core.c @@ -14,6 +14,7 @@ #include <linux/kernel.h> #include <linux/slab.h> #include <linux/spinlock.h> +#include <linux/mutex.h> #include <linux/platform_device.h> #include <linux/pm_runtime.h> #include <linux/interrupt.h> @@ -40,6 +41,8 @@
#define DWC3_DEFAULT_AUTOSUSPEND_DELAY 5000 /* ms */
+static DEFINE_MUTEX(mode_switch_lock); + /** * dwc3_get_dr_mode - Validates and sets dr_mode * @dwc: pointer to our context structure @@ -114,13 +117,20 @@ void dwc3_set_prtcap(struct dwc3 *dwc, u32 mode) dwc->current_dr_role = mode; }
+static int dwc3_core_soft_reset(struct dwc3 *dwc); + static void __dwc3_set_mode(struct work_struct *work) { struct dwc3 *dwc = work_to_dwc(work); unsigned long flags; + unsigned int hw_mode; int ret; u32 reg;
+ mutex_lock(&mode_switch_lock); + + hw_mode = DWC3_GHWPARAMS0_MODE(dwc->hwparams.hwparams0); + pm_runtime_get_sync(dwc->dev);
if (dwc->current_dr_role == DWC3_GCTL_PRTCAP_OTG) @@ -154,6 +164,24 @@ static void __dwc3_set_mode(struct work_struct *work) break; }
+ if (hw_mode == DWC3_GHWPARAMS0_MODE_DRD) { + reg = dwc3_readl(dwc->regs, DWC3_GCTL); + reg |= DWC3_GCTL_CORESOFTRESET; + dwc3_writel(dwc->regs, DWC3_GCTL, reg); + + /* + * Wait for internal clocks to synchronized. DWC_usb31 and + * DWC_usb32 may need at least 50ms (less for DWC_usb3). To + * keep it consistent across different IPs, let's wait up to + * 100ms before clearing GCTL.CORESOFTRESET. + */ + msleep(100); + + reg = dwc3_readl(dwc->regs, DWC3_GCTL); + reg &= ~DWC3_GCTL_CORESOFTRESET; + dwc3_writel(dwc->regs, DWC3_GCTL, reg); + } + spin_lock_irqsave(&dwc->lock, flags);
dwc3_set_prtcap(dwc, dwc->desired_dr_role); @@ -178,6 +206,9 @@ static void __dwc3_set_mode(struct work_struct *work) } break; case DWC3_GCTL_PRTCAP_DEVICE: + if (hw_mode == DWC3_GHWPARAMS0_MODE_DRD) + dwc3_core_soft_reset(dwc); + dwc3_event_buffers_setup(dwc);
if (dwc->usb2_phy) @@ -200,6 +231,7 @@ static void __dwc3_set_mode(struct work_struct *work) out: pm_runtime_mark_last_busy(dwc->dev); pm_runtime_put_autosuspend(dwc->dev); + mutex_unlock(&mode_switch_lock); }
void dwc3_set_mode(struct dwc3 *dwc, u32 mode)
base-commit: 4b853c236c7b5161a2e444bd8b3c76fe5aa5ddcb
Hi,
Thinh Nguyen Thinh.Nguyen@synopsys.com writes:
From: Yu Chen chenyu56@huawei.com From: John Stultz john.stultz@linaro.org
According to the programming guide, to switch mode for DRD controller, the driver needs to do the following.
To switch from device to host:
- Reset controller with GCTL.CoreSoftReset
- Set GCTL.PrtCapDir(host mode)
- Reset the host with USBCMD.HCRESET
- Then follow up with the initializing host registers sequence
To switch from host to device:
- Reset controller with GCTL.CoreSoftReset
- Set GCTL.PrtCapDir(device mode)
- Reset the device with DCTL.CSftRst
- Then follow up with the initializing registers sequence
Currently we're missing step 1) to do GCTL.CoreSoftReset and step 3) of
we're not really missing, it was a deliberate choice :-) The only reason why we need the soft reset is because host and gadget registers map to the same physical space within dwc3 core. If we cache and restore the affected registers, we're good ;-)
IMHO, that's a better compromise than doing a full soft reset.
@@ -40,6 +41,8 @@ #define DWC3_DEFAULT_AUTOSUSPEND_DELAY 5000 /* ms */ +static DEFINE_MUTEX(mode_switch_lock);
there are several platforms which more than one DWC3 instance. Sure this won't break on such systems?
Felipe Balbi wrote:
Hi,
Thinh Nguyen Thinh.Nguyen@synopsys.com writes:
From: Yu Chen chenyu56@huawei.com From: John Stultz john.stultz@linaro.org
According to the programming guide, to switch mode for DRD controller, the driver needs to do the following.
To switch from device to host:
- Reset controller with GCTL.CoreSoftReset
- Set GCTL.PrtCapDir(host mode)
- Reset the host with USBCMD.HCRESET
- Then follow up with the initializing host registers sequence
To switch from host to device:
- Reset controller with GCTL.CoreSoftReset
- Set GCTL.PrtCapDir(device mode)
- Reset the device with DCTL.CSftRst
- Then follow up with the initializing registers sequence
Currently we're missing step 1) to do GCTL.CoreSoftReset and step 3) of
we're not really missing, it was a deliberate choice :-) The only reason why we need the soft reset is because host and gadget registers map to the same physical space within dwc3 core. If we cache and restore the affected registers, we're good ;-)
It's part of the programming model. I've already discussed with internal RTL designers. This is needed, and I've provided the discussion we had prior also. We have several different devices in the wild that need this. What is the concern?
IMHO, that's a better compromise than doing a full soft reset.
@@ -40,6 +41,8 @@ #define DWC3_DEFAULT_AUTOSUSPEND_DELAY 5000 /* ms */ +static DEFINE_MUTEX(mode_switch_lock);
there are several platforms which more than one DWC3 instance. Sure this won't break on such systems?
How? Am I missing something? Please let me know so I can make the change.
Thanks, Thinh
On Thu, Apr 15, 2021 at 07:10:34AM +0000, Thinh Nguyen wrote:
Felipe Balbi wrote:
Hi,
Thinh Nguyen Thinh.Nguyen@synopsys.com writes:
From: Yu Chen chenyu56@huawei.com From: John Stultz john.stultz@linaro.org
According to the programming guide, to switch mode for DRD controller, the driver needs to do the following.
To switch from device to host:
- Reset controller with GCTL.CoreSoftReset
- Set GCTL.PrtCapDir(host mode)
- Reset the host with USBCMD.HCRESET
- Then follow up with the initializing host registers sequence
To switch from host to device:
- Reset controller with GCTL.CoreSoftReset
- Set GCTL.PrtCapDir(device mode)
- Reset the device with DCTL.CSftRst
- Then follow up with the initializing registers sequence
Currently we're missing step 1) to do GCTL.CoreSoftReset and step 3) of
we're not really missing, it was a deliberate choice :-) The only reason why we need the soft reset is because host and gadget registers map to the same physical space within dwc3 core. If we cache and restore the affected registers, we're good ;-)
It's part of the programming model. I've already discussed with internal RTL designers. This is needed, and I've provided the discussion we had prior also. We have several different devices in the wild that need this. What is the concern?
IMHO, that's a better compromise than doing a full soft reset.
@@ -40,6 +41,8 @@ #define DWC3_DEFAULT_AUTOSUSPEND_DELAY 5000 /* ms */ +static DEFINE_MUTEX(mode_switch_lock);
there are several platforms which more than one DWC3 instance. Sure this won't break on such systems?
How? Am I missing something? Please let me know so I can make the change.
All data needs to be per-device, not "global for the codebase" like the way you declared this lock.
thanks,
greg k-h
Greg Kroah-Hartman wrote:
On Thu, Apr 15, 2021 at 07:10:34AM +0000, Thinh Nguyen wrote:
Felipe Balbi wrote:
Hi,
Thinh Nguyen Thinh.Nguyen@synopsys.com writes:
From: Yu Chen chenyu56@huawei.com From: John Stultz john.stultz@linaro.org
According to the programming guide, to switch mode for DRD controller, the driver needs to do the following.
To switch from device to host:
- Reset controller with GCTL.CoreSoftReset
- Set GCTL.PrtCapDir(host mode)
- Reset the host with USBCMD.HCRESET
- Then follow up with the initializing host registers sequence
To switch from host to device:
- Reset controller with GCTL.CoreSoftReset
- Set GCTL.PrtCapDir(device mode)
- Reset the device with DCTL.CSftRst
- Then follow up with the initializing registers sequence
Currently we're missing step 1) to do GCTL.CoreSoftReset and step 3) of
we're not really missing, it was a deliberate choice :-) The only reason why we need the soft reset is because host and gadget registers map to the same physical space within dwc3 core. If we cache and restore the affected registers, we're good ;-)
It's part of the programming model. I've already discussed with internal RTL designers. This is needed, and I've provided the discussion we had prior also. We have several different devices in the wild that need this. What is the concern?
IMHO, that's a better compromise than doing a full soft reset.
@@ -40,6 +41,8 @@ #define DWC3_DEFAULT_AUTOSUSPEND_DELAY 5000 /* ms */ +static DEFINE_MUTEX(mode_switch_lock);
there are several platforms which more than one DWC3 instance. Sure this won't break on such systems?
How? Am I missing something? Please let me know so I can make the change.
All data needs to be per-device, not "global for the codebase" like the way you declared this lock.
Sure. I can make the change. Thanks for the review.
BR, Thinh
Hi,
Thinh Nguyen Thinh.Nguyen@synopsys.com writes:
Thinh Nguyen Thinh.Nguyen@synopsys.com writes:
From: Yu Chen chenyu56@huawei.com From: John Stultz john.stultz@linaro.org
According to the programming guide, to switch mode for DRD controller, the driver needs to do the following.
To switch from device to host:
- Reset controller with GCTL.CoreSoftReset
- Set GCTL.PrtCapDir(host mode)
- Reset the host with USBCMD.HCRESET
- Then follow up with the initializing host registers sequence
To switch from host to device:
- Reset controller with GCTL.CoreSoftReset
- Set GCTL.PrtCapDir(device mode)
- Reset the device with DCTL.CSftRst
- Then follow up with the initializing registers sequence
Currently we're missing step 1) to do GCTL.CoreSoftReset and step 3) of
we're not really missing, it was a deliberate choice :-) The only reason why we need the soft reset is because host and gadget registers map to the same physical space within dwc3 core. If we cache and restore the affected registers, we're good ;-)
It's part of the programming model. I've already discussed with internal RTL designers. This is needed, and I've provided the discussion we had prior also. We have several different devices in the wild that need this. What is the concern?
Timing :-) If anyone wants to support OTG spec, it'll be super hard to guarantee the timing mandated by the spec if we have to go through full reset.
IMHO, that's a better compromise than doing a full soft reset.
@@ -40,6 +41,8 @@ #define DWC3_DEFAULT_AUTOSUSPEND_DELAY 5000 /* ms */ +static DEFINE_MUTEX(mode_switch_lock);
there are several platforms which more than one DWC3 instance. Sure this won't break on such systems?
How? Am I missing something? Please let me know so I can make the change.
Again timing :-)
Instance 0 swaps role and instance 1 swaps right after. Instance 1 will be waiting for the mutex held by instance 0.
Felipe Balbi wrote:
Hi,
Thinh Nguyen Thinh.Nguyen@synopsys.com writes:
Thinh Nguyen Thinh.Nguyen@synopsys.com writes:
From: Yu Chen chenyu56@huawei.com From: John Stultz john.stultz@linaro.org
According to the programming guide, to switch mode for DRD controller, the driver needs to do the following.
To switch from device to host:
- Reset controller with GCTL.CoreSoftReset
- Set GCTL.PrtCapDir(host mode)
- Reset the host with USBCMD.HCRESET
- Then follow up with the initializing host registers sequence
To switch from host to device:
- Reset controller with GCTL.CoreSoftReset
- Set GCTL.PrtCapDir(device mode)
- Reset the device with DCTL.CSftRst
- Then follow up with the initializing registers sequence
Currently we're missing step 1) to do GCTL.CoreSoftReset and step 3) of
we're not really missing, it was a deliberate choice :-) The only reason why we need the soft reset is because host and gadget registers map to the same physical space within dwc3 core. If we cache and restore the affected registers, we're good ;-)
It's part of the programming model. I've already discussed with internal RTL designers. This is needed, and I've provided the discussion we had prior also. We have several different devices in the wild that need this. What is the concern?
Timing :-) If anyone wants to support OTG spec, it'll be super hard to guarantee the timing mandated by the spec if we have to go through full reset.
This is for DRD only. It should not impact the old OTG flow. We already have the check in place.
IMHO, that's a better compromise than doing a full soft reset.
@@ -40,6 +41,8 @@ #define DWC3_DEFAULT_AUTOSUSPEND_DELAY 5000 /* ms */ +static DEFINE_MUTEX(mode_switch_lock);
there are several platforms which more than one DWC3 instance. Sure this won't break on such systems?
How? Am I missing something? Please let me know so I can make the change.
Again timing :-)
Instance 0 swaps role and instance 1 swaps right after. Instance 1 will be waiting for the mutex held by instance 0.
It should not break DRD devices, and unless we have 10+ instances swapping at once, it shouldn't be a noticeable impact. Regardless, I understand the concern and I'll make a change as mentioned in my reply to Greg.
Thanks, Thinh
Hi Felipe,
Thinh Nguyen wrote:
Felipe Balbi wrote:
Hi,
Thinh Nguyen Thinh.Nguyen@synopsys.com writes:
Thinh Nguyen Thinh.Nguyen@synopsys.com writes:
From: Yu Chen chenyu56@huawei.com From: John Stultz john.stultz@linaro.org
According to the programming guide, to switch mode for DRD controller, the driver needs to do the following.
To switch from device to host:
- Reset controller with GCTL.CoreSoftReset
- Set GCTL.PrtCapDir(host mode)
- Reset the host with USBCMD.HCRESET
- Then follow up with the initializing host registers sequence
To switch from host to device:
- Reset controller with GCTL.CoreSoftReset
- Set GCTL.PrtCapDir(device mode)
- Reset the device with DCTL.CSftRst
- Then follow up with the initializing registers sequence
Currently we're missing step 1) to do GCTL.CoreSoftReset and step 3) of
we're not really missing, it was a deliberate choice :-) The only reason why we need the soft reset is because host and gadget registers map to the same physical space within dwc3 core. If we cache and restore the affected registers, we're good ;-)
It's part of the programming model. I've already discussed with internal RTL designers. This is needed, and I've provided the discussion we had prior also. We have several different devices in the wild that need this. What is the concern?
Timing :-) If anyone wants to support OTG spec, it'll be super hard to guarantee the timing mandated by the spec if we have to go through full reset.
This is for DRD only. It should not impact the old OTG flow. We already have the check in place.
I thought I checked for OTG, but I missed it. I think v3 should cover it.
Thanks, Thinh
From: Yu Chen chenyu56@huawei.com From: John Stultz john.stultz@linaro.org
According to the programming guide, to switch mode for DRD controller, the driver needs to do the following.
To switch from device to host: 1. Reset controller with GCTL.CoreSoftReset 2. Set GCTL.PrtCapDir(host mode) 3. Reset the host with USBCMD.HCRESET 4. Then follow up with the initializing host registers sequence
To switch from host to device: 1. Reset controller with GCTL.CoreSoftReset 2. Set GCTL.PrtCapDir(device mode) 3. Reset the device with DCTL.CSftRst 4. Then follow up with the initializing registers sequence
Currently we're missing step 1) to do GCTL.CoreSoftReset and step 3) of switching from host to device. John Stult reported a lockup issue seen with HiKey960 platform without these steps[1]. Similar issue is observed with Ferry's testing platform[2].
So, apply the required steps along with some fixes to Yu Chen's and John Stultz's version. The main fixes to their versions are the missing wait for clocks synchronization before clearing GCTL.CoreSoftReset and only apply DCTL.CSftRst when switching from host to device.
[1] https://lore.kernel.org/linux-usb/20210108015115.27920-1-john.stultz@linaro.... [2] https://lore.kernel.org/linux-usb/0ba7a6ba-e6a7-9cd4-0695-64fc927e01f1@gmail...
Cc: Andy Shevchenko andy.shevchenko@gmail.com Cc: Ferry Toth fntoth@gmail.com Cc: Wesley Cheng wcheng@codeaurora.org Cc: stable@vger.kernel.org Fixes: 41ce1456e1db ("usb: dwc3: core: make dwc3_set_mode() work properly") Signed-off-by: Yu Chen chenyu56@huawei.com Signed-off-by: John Stultz john.stultz@linaro.org Signed-off-by: Thinh Nguyen Thinh.Nguyen@synopsys.com --- Changes in v2: - Initialize mutex per device and not as global mutex. - Add additional checks for DRD only mode
drivers/usb/dwc3/core.c | 34 ++++++++++++++++++++++++++++++++++ drivers/usb/dwc3/core.h | 5 +++++ 2 files changed, 39 insertions(+)
diff --git a/drivers/usb/dwc3/core.c b/drivers/usb/dwc3/core.c index 5c25e6a72dbd..8eb6242e6bce 100644 --- a/drivers/usb/dwc3/core.c +++ b/drivers/usb/dwc3/core.c @@ -114,13 +114,24 @@ void dwc3_set_prtcap(struct dwc3 *dwc, u32 mode) dwc->current_dr_role = mode; }
+static int dwc3_core_soft_reset(struct dwc3 *dwc); + static void __dwc3_set_mode(struct work_struct *work) { struct dwc3 *dwc = work_to_dwc(work); unsigned long flags; + unsigned int hw_mode; + bool otg_enabled = false; int ret; u32 reg;
+ mutex_lock(&dwc->mutex); + + hw_mode = DWC3_GHWPARAMS0_MODE(dwc->hwparams.hwparams0); + if (DWC3_VER_IS_PRIOR(DWC3, 330A) && + (dwc->hwparams.hwparams6 & DWC3_GHWPARAMS6_SRPSUPPORT)) + otg_enabled = true; + pm_runtime_get_sync(dwc->dev);
if (dwc->current_dr_role == DWC3_GCTL_PRTCAP_OTG) @@ -154,6 +165,24 @@ static void __dwc3_set_mode(struct work_struct *work) break; }
+ if (hw_mode == DWC3_GHWPARAMS0_MODE_DRD && !otg_enabled) { + reg = dwc3_readl(dwc->regs, DWC3_GCTL); + reg |= DWC3_GCTL_CORESOFTRESET; + dwc3_writel(dwc->regs, DWC3_GCTL, reg); + + /* + * Wait for internal clocks to synchronized. DWC_usb31 and + * DWC_usb32 may need at least 50ms (less for DWC_usb3). To + * keep it consistent across different IPs, let's wait up to + * 100ms before clearing GCTL.CORESOFTRESET. + */ + msleep(100); + + reg = dwc3_readl(dwc->regs, DWC3_GCTL); + reg &= ~DWC3_GCTL_CORESOFTRESET; + dwc3_writel(dwc->regs, DWC3_GCTL, reg); + } + spin_lock_irqsave(&dwc->lock, flags);
dwc3_set_prtcap(dwc, dwc->desired_dr_role); @@ -178,6 +207,9 @@ static void __dwc3_set_mode(struct work_struct *work) } break; case DWC3_GCTL_PRTCAP_DEVICE: + if (hw_mode == DWC3_GHWPARAMS0_MODE_DRD && !otg_enabled) + dwc3_core_soft_reset(dwc); + dwc3_event_buffers_setup(dwc);
if (dwc->usb2_phy) @@ -200,6 +232,7 @@ static void __dwc3_set_mode(struct work_struct *work) out: pm_runtime_mark_last_busy(dwc->dev); pm_runtime_put_autosuspend(dwc->dev); + mutex_unlock(&dwc->mutex); }
void dwc3_set_mode(struct dwc3 *dwc, u32 mode) @@ -1553,6 +1586,7 @@ static int dwc3_probe(struct platform_device *pdev) dwc3_cache_hwparams(dwc);
spin_lock_init(&dwc->lock); + mutex_init(&dwc->mutex);
pm_runtime_set_active(dev); pm_runtime_use_autosuspend(dev); diff --git a/drivers/usb/dwc3/core.h b/drivers/usb/dwc3/core.h index 695ff2d791e4..7e3afa5378e8 100644 --- a/drivers/usb/dwc3/core.h +++ b/drivers/usb/dwc3/core.h @@ -13,6 +13,7 @@
#include <linux/device.h> #include <linux/spinlock.h> +#include <linux/mutex.h> #include <linux/ioport.h> #include <linux/list.h> #include <linux/bitops.h> @@ -947,6 +948,7 @@ struct dwc3_scratchpad_array { * @scratch_addr: dma address of scratchbuf * @ep0_in_setup: one control transfer is completed and enter setup phase * @lock: for synchronizing + * @mutex: for mode switching * @dev: pointer to our struct device * @sysdev: pointer to the DMA-capable device * @xhci: pointer to our xHCI child @@ -1088,6 +1090,9 @@ struct dwc3 { /* device lock */ spinlock_t lock;
+ /* mode switching lock */ + struct mutex mutex; + struct device *dev; struct device *sysdev;
base-commit: 4b853c236c7b5161a2e444bd8b3c76fe5aa5ddcb
On Thu, Apr 15, 2021 at 9:29 AM Thinh Nguyen Thinh.Nguyen@synopsys.com wrote:
From: Yu Chen chenyu56@huawei.com From: John Stultz john.stultz@linaro.org
According to the programming guide, to switch mode for DRD controller, the driver needs to do the following.
To switch from device to host:
- Reset controller with GCTL.CoreSoftReset
- Set GCTL.PrtCapDir(host mode)
- Reset the host with USBCMD.HCRESET
- Then follow up with the initializing host registers sequence
To switch from host to device:
- Reset controller with GCTL.CoreSoftReset
- Set GCTL.PrtCapDir(device mode)
- Reset the device with DCTL.CSftRst
- Then follow up with the initializing registers sequence
Currently we're missing step 1) to do GCTL.CoreSoftReset and step 3) of switching from host to device. John Stult reported a lockup issue seen with HiKey960 platform without these steps[1]. Similar issue is observed with Ferry's testing platform[2].
So, apply the required steps along with some fixes to Yu Chen's and John Stultz's version. The main fixes to their versions are the missing wait for clocks synchronization before clearing GCTL.CoreSoftReset and only apply DCTL.CSftRst when switching from host to device.
[1] https://lore.kernel.org/linux-usb/20210108015115.27920-1-john.stultz@linaro.... [2] https://lore.kernel.org/linux-usb/0ba7a6ba-e6a7-9cd4-0695-64fc927e01f1@gmail...
Cc: Andy Shevchenko andy.shevchenko@gmail.com Cc: Ferry Toth fntoth@gmail.com Cc: Wesley Cheng wcheng@codeaurora.org Cc: stable@vger.kernel.org Fixes: 41ce1456e1db ("usb: dwc3: core: make dwc3_set_mode() work properly") Signed-off-by: Yu Chen chenyu56@huawei.com Signed-off-by: John Stultz john.stultz@linaro.org Signed-off-by: Thinh Nguyen Thinh.Nguyen@synopsys.com
Changes in v2:
- Initialize mutex per device and not as global mutex.
- Add additional checks for DRD only mode
Hey Thinh!
Thanks so much for your persisting effort on this issue! Its something I'd love to see finally resolved!
static void __dwc3_set_mode(struct work_struct *work) { struct dwc3 *dwc = work_to_dwc(work); unsigned long flags;
unsigned int hw_mode;bool otg_enabled = false; int ret; u32 reg;mutex_lock(&dwc->mutex);hw_mode = DWC3_GHWPARAMS0_MODE(dwc->hwparams.hwparams0);if (DWC3_VER_IS_PRIOR(DWC3, 330A) &&(dwc->hwparams.hwparams6 & DWC3_GHWPARAMS6_SRPSUPPORT))otg_enabled = true;
Unfortunately on HiKey960, this check ends up being true, and that basically disables the needed (on HiKey960 at least) soft reset logic below, so we still end up hitting the issue.
The revision/hwparams6 values on the board are: revision: 0x5533300a hwparams6: 0xfeaec20
Just to make sure, I did test disabling the check here, and it does seem to avoid the !COREIDLE stuck problem seen frequently on the board.
thanks -john
John Stultz wrote:
On Thu, Apr 15, 2021 at 9:29 AM Thinh Nguyen Thinh.Nguyen@synopsys.com wrote:
From: Yu Chen chenyu56@huawei.com From: John Stultz john.stultz@linaro.org
According to the programming guide, to switch mode for DRD controller, the driver needs to do the following.
To switch from device to host:
- Reset controller with GCTL.CoreSoftReset
- Set GCTL.PrtCapDir(host mode)
- Reset the host with USBCMD.HCRESET
- Then follow up with the initializing host registers sequence
To switch from host to device:
- Reset controller with GCTL.CoreSoftReset
- Set GCTL.PrtCapDir(device mode)
- Reset the device with DCTL.CSftRst
- Then follow up with the initializing registers sequence
Currently we're missing step 1) to do GCTL.CoreSoftReset and step 3) of switching from host to device. John Stult reported a lockup issue seen with HiKey960 platform without these steps[1]. Similar issue is observed with Ferry's testing platform[2].
So, apply the required steps along with some fixes to Yu Chen's and John Stultz's version. The main fixes to their versions are the missing wait for clocks synchronization before clearing GCTL.CoreSoftReset and only apply DCTL.CSftRst when switching from host to device.
[1] https://urldefense.com/v3/__https://lore.kernel.org/linux-usb/20210108015115... [2] https://urldefense.com/v3/__https://lore.kernel.org/linux-usb/0ba7a6ba-e6a7-...
Cc: Andy Shevchenko andy.shevchenko@gmail.com Cc: Ferry Toth fntoth@gmail.com Cc: Wesley Cheng wcheng@codeaurora.org Cc: stable@vger.kernel.org Fixes: 41ce1456e1db ("usb: dwc3: core: make dwc3_set_mode() work properly") Signed-off-by: Yu Chen chenyu56@huawei.com Signed-off-by: John Stultz john.stultz@linaro.org Signed-off-by: Thinh Nguyen Thinh.Nguyen@synopsys.com
Changes in v2:
- Initialize mutex per device and not as global mutex.
- Add additional checks for DRD only mode
Hey Thinh!
Thanks so much for your persisting effort on this issue! Its something I'd love to see finally resolved!
static void __dwc3_set_mode(struct work_struct *work) { struct dwc3 *dwc = work_to_dwc(work); unsigned long flags;
unsigned int hw_mode;bool otg_enabled = false; int ret; u32 reg;mutex_lock(&dwc->mutex);hw_mode = DWC3_GHWPARAMS0_MODE(dwc->hwparams.hwparams0);if (DWC3_VER_IS_PRIOR(DWC3, 330A) &&(dwc->hwparams.hwparams6 & DWC3_GHWPARAMS6_SRPSUPPORT))otg_enabled = true;Unfortunately on HiKey960, this check ends up being true, and that basically disables the needed (on HiKey960 at least) soft reset logic below, so we still end up hitting the issue.
The revision/hwparams6 values on the board are: revision: 0x5533300a hwparams6: 0xfeaec20
Just to make sure, I did test disabling the check here, and it does seem to avoid the !COREIDLE stuck problem seen frequently on the board.
Hi John,
That extra check for OTG support is unnecessary (and I believe is incorrect), but I wanted to completely alleviate Felipe's concern. With static host-only/device-only DRD mode, we don't care about OTG since PrtCapDir is not set to OTG.
Thanks for the test. I'll make the fix and discuss further with Felipe if necessary.
Thanks, Thinh
From: Yu Chen chenyu56@huawei.com From: John Stultz john.stultz@linaro.org
According to the programming guide, to switch mode for DRD controller, the driver needs to do the following.
To switch from device to host: 1. Reset controller with GCTL.CoreSoftReset 2. Set GCTL.PrtCapDir(host mode) 3. Reset the host with USBCMD.HCRESET 4. Then follow up with the initializing host registers sequence
To switch from host to device: 1. Reset controller with GCTL.CoreSoftReset 2. Set GCTL.PrtCapDir(device mode) 3. Reset the device with DCTL.CSftRst 4. Then follow up with the initializing registers sequence
Currently we're missing step 1) to do GCTL.CoreSoftReset and step 3) of switching from host to device. John Stult reported a lockup issue seen with HiKey960 platform without these steps[1]. Similar issue is observed with Ferry's testing platform[2].
So, apply the required steps along with some fixes to Yu Chen's and John Stultz's version. The main fixes to their versions are the missing wait for clocks synchronization before clearing GCTL.CoreSoftReset and only apply DCTL.CSftRst when switching from host to device.
[1] https://lore.kernel.org/linux-usb/20210108015115.27920-1-john.stultz@linaro.... [2] https://lore.kernel.org/linux-usb/0ba7a6ba-e6a7-9cd4-0695-64fc927e01f1@gmail...
Cc: Andy Shevchenko andy.shevchenko@gmail.com Cc: Ferry Toth fntoth@gmail.com Cc: Wesley Cheng wcheng@codeaurora.org Cc: stable@vger.kernel.org Fixes: 41ce1456e1db ("usb: dwc3: core: make dwc3_set_mode() work properly") Signed-off-by: Yu Chen chenyu56@huawei.com Signed-off-by: John Stultz john.stultz@linaro.org Signed-off-by: Thinh Nguyen Thinh.Nguyen@synopsys.com --- Changes in v3: - Check if the desired mode is OTG, then keep the old flow - Remove condition for OTG support only since the device can still be configured DRD host/device mode only - Remove redundant hw_mode check since __dwc3_set_mode() only applies when hw_mode is DRD Changes in v2: - Initialize mutex per device and not as global mutex. - Add additional checks for DRD only mode
drivers/usb/dwc3/core.c | 27 +++++++++++++++++++++++++++ drivers/usb/dwc3/core.h | 5 +++++ 2 files changed, 32 insertions(+)
diff --git a/drivers/usb/dwc3/core.c b/drivers/usb/dwc3/core.c index 5c25e6a72dbd..2f118ad43571 100644 --- a/drivers/usb/dwc3/core.c +++ b/drivers/usb/dwc3/core.c @@ -114,6 +114,8 @@ void dwc3_set_prtcap(struct dwc3 *dwc, u32 mode) dwc->current_dr_role = mode; }
+static int dwc3_core_soft_reset(struct dwc3 *dwc); + static void __dwc3_set_mode(struct work_struct *work) { struct dwc3 *dwc = work_to_dwc(work); @@ -121,6 +123,8 @@ static void __dwc3_set_mode(struct work_struct *work) int ret; u32 reg;
+ mutex_lock(&dwc->mutex); + pm_runtime_get_sync(dwc->dev);
if (dwc->current_dr_role == DWC3_GCTL_PRTCAP_OTG) @@ -154,6 +158,25 @@ static void __dwc3_set_mode(struct work_struct *work) break; }
+ /* For DRD host or device mode only */ + if (dwc->desired_dr_role != DWC3_GCTL_PRTCAP_OTG) { + reg = dwc3_readl(dwc->regs, DWC3_GCTL); + reg |= DWC3_GCTL_CORESOFTRESET; + dwc3_writel(dwc->regs, DWC3_GCTL, reg); + + /* + * Wait for internal clocks to synchronized. DWC_usb31 and + * DWC_usb32 may need at least 50ms (less for DWC_usb3). To + * keep it consistent across different IPs, let's wait up to + * 100ms before clearing GCTL.CORESOFTRESET. + */ + msleep(100); + + reg = dwc3_readl(dwc->regs, DWC3_GCTL); + reg &= ~DWC3_GCTL_CORESOFTRESET; + dwc3_writel(dwc->regs, DWC3_GCTL, reg); + } + spin_lock_irqsave(&dwc->lock, flags);
dwc3_set_prtcap(dwc, dwc->desired_dr_role); @@ -178,6 +201,8 @@ static void __dwc3_set_mode(struct work_struct *work) } break; case DWC3_GCTL_PRTCAP_DEVICE: + dwc3_core_soft_reset(dwc); + dwc3_event_buffers_setup(dwc);
if (dwc->usb2_phy) @@ -200,6 +225,7 @@ static void __dwc3_set_mode(struct work_struct *work) out: pm_runtime_mark_last_busy(dwc->dev); pm_runtime_put_autosuspend(dwc->dev); + mutex_unlock(&dwc->mutex); }
void dwc3_set_mode(struct dwc3 *dwc, u32 mode) @@ -1553,6 +1579,7 @@ static int dwc3_probe(struct platform_device *pdev) dwc3_cache_hwparams(dwc);
spin_lock_init(&dwc->lock); + mutex_init(&dwc->mutex);
pm_runtime_set_active(dev); pm_runtime_use_autosuspend(dev); diff --git a/drivers/usb/dwc3/core.h b/drivers/usb/dwc3/core.h index 695ff2d791e4..7e3afa5378e8 100644 --- a/drivers/usb/dwc3/core.h +++ b/drivers/usb/dwc3/core.h @@ -13,6 +13,7 @@
#include <linux/device.h> #include <linux/spinlock.h> +#include <linux/mutex.h> #include <linux/ioport.h> #include <linux/list.h> #include <linux/bitops.h> @@ -947,6 +948,7 @@ struct dwc3_scratchpad_array { * @scratch_addr: dma address of scratchbuf * @ep0_in_setup: one control transfer is completed and enter setup phase * @lock: for synchronizing + * @mutex: for mode switching * @dev: pointer to our struct device * @sysdev: pointer to the DMA-capable device * @xhci: pointer to our xHCI child @@ -1088,6 +1090,9 @@ struct dwc3 { /* device lock */ spinlock_t lock;
+ /* mode switching lock */ + struct mutex mutex; + struct device *dev; struct device *sysdev;
base-commit: 4b853c236c7b5161a2e444bd8b3c76fe5aa5ddcb
Thinh Nguyen wrote:
From: Yu Chen chenyu56@huawei.com From: John Stultz john.stultz@linaro.org
According to the programming guide, to switch mode for DRD controller, the driver needs to do the following.
To switch from device to host:
- Reset controller with GCTL.CoreSoftReset
- Set GCTL.PrtCapDir(host mode)
- Reset the host with USBCMD.HCRESET
- Then follow up with the initializing host registers sequence
To switch from host to device:
- Reset controller with GCTL.CoreSoftReset
- Set GCTL.PrtCapDir(device mode)
- Reset the device with DCTL.CSftRst
- Then follow up with the initializing registers sequence
Currently we're missing step 1) to do GCTL.CoreSoftReset and step 3) of switching from host to device. John Stult reported a lockup issue seen with HiKey960 platform without these steps[1]. Similar issue is observed with Ferry's testing platform[2].
So, apply the required steps along with some fixes to Yu Chen's and John Stultz's version. The main fixes to their versions are the missing wait for clocks synchronization before clearing GCTL.CoreSoftReset and only apply DCTL.CSftRst when switching from host to device.
[1] https://urldefense.com/v3/__https://lore.kernel.org/linux-usb/20210108015115... [2] https://urldefense.com/v3/__https://lore.kernel.org/linux-usb/0ba7a6ba-e6a7-...
Cc: Andy Shevchenko andy.shevchenko@gmail.com Cc: Ferry Toth fntoth@gmail.com Cc: Wesley Cheng wcheng@codeaurora.org Cc: stable@vger.kernel.org Fixes: 41ce1456e1db ("usb: dwc3: core: make dwc3_set_mode() work properly") Signed-off-by: Yu Chen chenyu56@huawei.com Signed-off-by: John Stultz john.stultz@linaro.org Signed-off-by: Thinh Nguyen Thinh.Nguyen@synopsys.com
Changes in v3:
- Check if the desired mode is OTG, then keep the old flow
- Remove condition for OTG support only since the device can still be configured DRD host/device mode only
- Remove redundant hw_mode check since __dwc3_set_mode() only applies when hw_mode is DRD
Changes in v2:
- Initialize mutex per device and not as global mutex.
- Add additional checks for DRD only mode
drivers/usb/dwc3/core.c | 27 +++++++++++++++++++++++++++ drivers/usb/dwc3/core.h | 5 +++++ 2 files changed, 32 insertions(+)
Hi John,
If possible, can you run a test with this version on your platform?
Thanks, Thinh
Hi
Op 16-04-2021 om 00:23 schreef Thinh Nguyen:
Thinh Nguyen wrote:
From: Yu Chen chenyu56@huawei.com From: John Stultz john.stultz@linaro.org
According to the programming guide, to switch mode for DRD controller, the driver needs to do the following.
To switch from device to host:
- Reset controller with GCTL.CoreSoftReset
- Set GCTL.PrtCapDir(host mode)
- Reset the host with USBCMD.HCRESET
- Then follow up with the initializing host registers sequence
To switch from host to device:
- Reset controller with GCTL.CoreSoftReset
- Set GCTL.PrtCapDir(device mode)
- Reset the device with DCTL.CSftRst
- Then follow up with the initializing registers sequence
Currently we're missing step 1) to do GCTL.CoreSoftReset and step 3) of switching from host to device. John Stult reported a lockup issue seen with HiKey960 platform without these steps[1]. Similar issue is observed with Ferry's testing platform[2].
So, apply the required steps along with some fixes to Yu Chen's and John Stultz's version. The main fixes to their versions are the missing wait for clocks synchronization before clearing GCTL.CoreSoftReset and only apply DCTL.CSftRst when switching from host to device.
[1] https://urldefense.com/v3/__https://lore.kernel.org/linux-usb/20210108015115... [2] https://urldefense.com/v3/__https://lore.kernel.org/linux-usb/0ba7a6ba-e6a7-...
Cc: Andy Shevchenko andy.shevchenko@gmail.com Cc: Ferry Toth fntoth@gmail.com Cc: Wesley Cheng wcheng@codeaurora.org Cc: stable@vger.kernel.org Fixes: 41ce1456e1db ("usb: dwc3: core: make dwc3_set_mode() work properly") Signed-off-by: Yu Chen chenyu56@huawei.com Signed-off-by: John Stultz john.stultz@linaro.org Signed-off-by: Thinh Nguyen Thinh.Nguyen@synopsys.com
Changes in v3:
- Check if the desired mode is OTG, then keep the old flow
- Remove condition for OTG support only since the device can still be configured DRD host/device mode only
- Remove redundant hw_mode check since __dwc3_set_mode() only applies when hw_mode is DRD
Changes in v2:
Initialize mutex per device and not as global mutex.
Add additional checks for DRD only mode
drivers/usb/dwc3/core.c | 27 +++++++++++++++++++++++++++ drivers/usb/dwc3/core.h | 5 +++++ 2 files changed, 32 insertions(+)
Hi John,
If possible, can you run a test with this version on your platform?
Thanks, Thinh
I tested this on edison-arduino with this patch on top of usb-next (5.12-rc7 + "increase BESL baseline to 6" to prevent throttling").
On this platform there is a physical switch to switch roles. With this patch I find:
- switch to host mode always works fine
- switch to gadget mode I need to flip the switch 3x (gadget-host-gadget).
An error message appears on the gadget side "dwc3 dwc3.0.auto: timed out waiting for SETUP phase" appears, but then the device connects to my PC, no throttling.
- alternatively I can switch to gadget 1x and then unplug/replug the cable.
No error message and connects fine.
- if I flip the switch only once, on the PC side I get:
kernel: usb 1-5: new high-speed USB device number 18 using xhci_hcd kernel: usb 1-5: New USB device found, idVendor=1d6b, idProduct=0104, bcdDevice= 1.00 kernel: usb 1-5: New USB device strings: Mfr=1, Product=2, SerialNumber=3 kernel: usb 1-5: Product: USBArmory Gadget kernel: usb 1-5: Manufacturer: USBArmory kernel: usb 1-5: SerialNumber: 0123456789abcdef kernel: usb 1-5: can't set config #1, error -110
Then if I wait long enough on the gadget side I get:
root@yuna:~# ifconfig
usb0: flags=-28605<UP,BROADCAST,RUNNING,MULTICAST,DYNAMIC> mtu 1500 inet 169.254.119.239 netmask 255.255.0.0 broadcast 169.254.255.255 inet6 fe80::a8bb:ccff:fedd:eef1 prefixlen 64 scopeid 0x20<link> ether aa:bb:cc:dd:ee:f1 txqueuelen 1000 (Ethernet) RX packets 490424 bytes 735146578 (701.0 MiB) RX errors 0 dropped 191 overruns 0 frame 0 TX packets 35279 bytes 2532746 (2.4 MiB) TX errors 0 dropped 0 overruns 0 carrier 0 collisions 0
(correct would be: inet 10.42.0.221 netmask 255.255.255.0 broadcast 10.42.0.255)
So much improved now, but it seems I am still missing something on plug.
Ferry Toth wrote:
Hi
Op 16-04-2021 om 00:23 schreef Thinh Nguyen:
Thinh Nguyen wrote:
From: Yu Chen chenyu56@huawei.com From: John Stultz john.stultz@linaro.org
According to the programming guide, to switch mode for DRD controller, the driver needs to do the following.
To switch from device to host:
- Reset controller with GCTL.CoreSoftReset
- Set GCTL.PrtCapDir(host mode)
- Reset the host with USBCMD.HCRESET
- Then follow up with the initializing host registers sequence
To switch from host to device:
- Reset controller with GCTL.CoreSoftReset
- Set GCTL.PrtCapDir(device mode)
- Reset the device with DCTL.CSftRst
- Then follow up with the initializing registers sequence
Currently we're missing step 1) to do GCTL.CoreSoftReset and step 3) of switching from host to device. John Stult reported a lockup issue seen with HiKey960 platform without these steps[1]. Similar issue is observed with Ferry's testing platform[2].
So, apply the required steps along with some fixes to Yu Chen's and John Stultz's version. The main fixes to their versions are the missing wait for clocks synchronization before clearing GCTL.CoreSoftReset and only apply DCTL.CSftRst when switching from host to device.
[1] https://urldefense.com/v3/__https://lore.kernel.org/linux-usb/20210108015115...
[2] https://urldefense.com/v3/__https://lore.kernel.org/linux-usb/0ba7a6ba-e6a7-...
Cc: Andy Shevchenko andy.shevchenko@gmail.com Cc: Ferry Toth fntoth@gmail.com Cc: Wesley Cheng wcheng@codeaurora.org Cc: stable@vger.kernel.org Fixes: 41ce1456e1db ("usb: dwc3: core: make dwc3_set_mode() work properly") Signed-off-by: Yu Chen chenyu56@huawei.com Signed-off-by: John Stultz john.stultz@linaro.org Signed-off-by: Thinh Nguyen Thinh.Nguyen@synopsys.com
Changes in v3:
- Check if the desired mode is OTG, then keep the old flow
- Remove condition for OTG support only since the device can still be
configured DRD host/device mode only
- Remove redundant hw_mode check since __dwc3_set_mode() only applies
when hw_mode is DRD Changes in v2:
- Initialize mutex per device and not as global mutex.
- Add additional checks for DRD only mode
drivers/usb/dwc3/core.c | 27 +++++++++++++++++++++++++++ drivers/usb/dwc3/core.h | 5 +++++ 2 files changed, 32 insertions(+)
Hi John,
If possible, can you run a test with this version on your platform?
Thanks, Thinh
I tested this on edison-arduino with this patch on top of usb-next (5.12-rc7 + "increase BESL baseline to 6" to prevent throttling").
On this platform there is a physical switch to switch roles. With this patch I find:
switch to host mode always works fine
switch to gadget mode I need to flip the switch 3x (gadget-host-gadget).
An error message appears on the gadget side "dwc3 dwc3.0.auto: timed out waiting for SETUP phase" appears, but then the device connects to my PC, no throttling.
- alternatively I can switch to gadget 1x and then unplug/replug the cable.
No error message and connects fine.
- if I flip the switch only once, on the PC side I get:
kernel: usb 1-5: new high-speed USB device number 18 using xhci_hcd kernel: usb 1-5: New USB device found, idVendor=1d6b, idProduct=0104, bcdDevice= 1.00 kernel: usb 1-5: New USB device strings: Mfr=1, Product=2, SerialNumber=3 kernel: usb 1-5: Product: USBArmory Gadget kernel: usb 1-5: Manufacturer: USBArmory kernel: usb 1-5: SerialNumber: 0123456789abcdef kernel: usb 1-5: can't set config #1, error -110
The device failed at set_configuration() request and timed out. It probably timed out from the status stage looking at the device err print.
Then if I wait long enough on the gadget side I get:
root@yuna:~# ifconfig
usb0: flags=-28605<UP,BROADCAST,RUNNING,MULTICAST,DYNAMIC> mtu 1500 inet 169.254.119.239 netmask 255.255.0.0 broadcast 169.254.255.255 inet6 fe80::a8bb:ccff:fedd:eef1 prefixlen 64 scopeid 0x20<link> ether aa:bb:cc:dd:ee:f1 txqueuelen 1000 (Ethernet) RX packets 490424 bytes 735146578 (701.0 MiB) RX errors 0 dropped 191 overruns 0 frame 0 TX packets 35279 bytes 2532746 (2.4 MiB) TX errors 0 dropped 0 overruns 0 carrier 0 collisions 0
(correct would be: inet 10.42.0.221 netmask 255.255.255.0 broadcast 10.42.0.255)
So much improved now, but it seems I am still missing something on plug.
That's great! We can look at it further. Can you capture the tracepoints of the issue. Also, can you try with mass_storage gadget to see if the result is the same?
Thanks, Thinh
Hi
Op 17-04-2021 om 04:27 schreef Thinh Nguyen:
Ferry Toth wrote:
Hi
Op 16-04-2021 om 00:23 schreef Thinh Nguyen:
Thinh Nguyen wrote:
From: Yu Chen chenyu56@huawei.com From: John Stultz john.stultz@linaro.org
According to the programming guide, to switch mode for DRD controller, the driver needs to do the following.
To switch from device to host:
- Reset controller with GCTL.CoreSoftReset
- Set GCTL.PrtCapDir(host mode)
- Reset the host with USBCMD.HCRESET
- Then follow up with the initializing host registers sequence
To switch from host to device:
- Reset controller with GCTL.CoreSoftReset
- Set GCTL.PrtCapDir(device mode)
- Reset the device with DCTL.CSftRst
- Then follow up with the initializing registers sequence
Currently we're missing step 1) to do GCTL.CoreSoftReset and step 3) of switching from host to device. John Stult reported a lockup issue seen with HiKey960 platform without these steps[1]. Similar issue is observed with Ferry's testing platform[2].
So, apply the required steps along with some fixes to Yu Chen's and John Stultz's version. The main fixes to their versions are the missing wait for clocks synchronization before clearing GCTL.CoreSoftReset and only apply DCTL.CSftRst when switching from host to device.
[1] https://urldefense.com/v3/__https://lore.kernel.org/linux-usb/20210108015115...
[2] https://urldefense.com/v3/__https://lore.kernel.org/linux-usb/0ba7a6ba-e6a7-...
Cc: Andy Shevchenko andy.shevchenko@gmail.com Cc: Ferry Toth fntoth@gmail.com Cc: Wesley Cheng wcheng@codeaurora.org Cc: stable@vger.kernel.org Fixes: 41ce1456e1db ("usb: dwc3: core: make dwc3_set_mode() work properly") Signed-off-by: Yu Chen chenyu56@huawei.com Signed-off-by: John Stultz john.stultz@linaro.org Signed-off-by: Thinh Nguyen Thinh.Nguyen@synopsys.com
Changes in v3:
- Check if the desired mode is OTG, then keep the old flow
- Remove condition for OTG support only since the device can still be
configured DRD host/device mode only
- Remove redundant hw_mode check since __dwc3_set_mode() only applies
when hw_mode is DRD Changes in v2:
- Initialize mutex per device and not as global mutex.
- Add additional checks for DRD only mode
drivers/usb/dwc3/core.c | 27 +++++++++++++++++++++++++++ drivers/usb/dwc3/core.h | 5 +++++ 2 files changed, 32 insertions(+)
Hi John,
If possible, can you run a test with this version on your platform?
Thanks, Thinh
I tested this on edison-arduino with this patch on top of usb-next (5.12-rc7 + "increase BESL baseline to 6" to prevent throttling").
On this platform there is a physical switch to switch roles. With this patch I find:
switch to host mode always works fine
switch to gadget mode I need to flip the switch 3x (gadget-host-gadget).
An error message appears on the gadget side "dwc3 dwc3.0.auto: timed out waiting for SETUP phase" appears, but then the device connects to my PC, no throttling.
- alternatively I can switch to gadget 1x and then unplug/replug the cable.
No error message and connects fine.
- if I flip the switch only once, on the PC side I get:
kernel: usb 1-5: new high-speed USB device number 18 using xhci_hcd kernel: usb 1-5: New USB device found, idVendor=1d6b, idProduct=0104, bcdDevice= 1.00 kernel: usb 1-5: New USB device strings: Mfr=1, Product=2, SerialNumber=3 kernel: usb 1-5: Product: USBArmory Gadget kernel: usb 1-5: Manufacturer: USBArmory kernel: usb 1-5: SerialNumber: 0123456789abcdef kernel: usb 1-5: can't set config #1, error -110
The device failed at set_configuration() request and timed out. It probably timed out from the status stage looking at the device err print.
Then if I wait long enough on the gadget side I get:
root@yuna:~# ifconfig
usb0: flags=-28605<UP,BROADCAST,RUNNING,MULTICAST,DYNAMIC> mtu 1500 inet 169.254.119.239 netmask 255.255.0.0 broadcast 169.254.255.255 inet6 fe80::a8bb:ccff:fedd:eef1 prefixlen 64 scopeid 0x20<link> ether aa:bb:cc:dd:ee:f1 txqueuelen 1000 (Ethernet) RX packets 490424 bytes 735146578 (701.0 MiB) RX errors 0 dropped 191 overruns 0 frame 0 TX packets 35279 bytes 2532746 (2.4 MiB) TX errors 0 dropped 0 overruns 0 carrier 0 collisions 0
(correct would be: inet 10.42.0.221 netmask 255.255.255.0 broadcast 10.42.0.255)
So much improved now, but it seems I am still missing something on plug.
That's great! We can look at it further. Can you capture the tracepoints of the issue. Also, can you try with mass_storage gadget to see if the result is the same?
I have already gser, eem, mass_storage and uac2 combo. When eem fails, the mass_storage and uac2 don't appear (on KDE you get all kind of popups when they appear).
So either all works, or all fails.
I'll trace this later today.
Thanks, Thinh
Hi
Op 17-04-2021 om 16:22 schreef Ferry Toth:
Hi
Op 17-04-2021 om 04:27 schreef Thinh Nguyen:
Ferry Toth wrote:
Hi
Op 16-04-2021 om 00:23 schreef Thinh Nguyen:
Thinh Nguyen wrote:
From: Yu Chen chenyu56@huawei.com From: John Stultz john.stultz@linaro.org
According to the programming guide, to switch mode for DRD controller, the driver needs to do the following.
To switch from device to host:
- Reset controller with GCTL.CoreSoftReset
- Set GCTL.PrtCapDir(host mode)
- Reset the host with USBCMD.HCRESET
- Then follow up with the initializing host registers sequence
To switch from host to device:
- Reset controller with GCTL.CoreSoftReset
- Set GCTL.PrtCapDir(device mode)
- Reset the device with DCTL.CSftRst
- Then follow up with the initializing registers sequence
Currently we're missing step 1) to do GCTL.CoreSoftReset and step 3) of switching from host to device. John Stult reported a lockup issue seen with HiKey960 platform without these steps[1]. Similar issue is observed with Ferry's testing platform[2].
So, apply the required steps along with some fixes to Yu Chen's and John Stultz's version. The main fixes to their versions are the missing wait for clocks synchronization before clearing GCTL.CoreSoftReset and only apply DCTL.CSftRst when switching from host to device.
[1] https://urldefense.com/v3/__https://lore.kernel.org/linux-usb/20210108015115...
[2] https://urldefense.com/v3/__https://lore.kernel.org/linux-usb/0ba7a6ba-e6a7-...
Cc: Andy Shevchenko andy.shevchenko@gmail.com Cc: Ferry Toth fntoth@gmail.com Cc: Wesley Cheng wcheng@codeaurora.org Cc: stable@vger.kernel.org Fixes: 41ce1456e1db ("usb: dwc3: core: make dwc3_set_mode() work properly") Signed-off-by: Yu Chen chenyu56@huawei.com Signed-off-by: John Stultz john.stultz@linaro.org Signed-off-by: Thinh Nguyen Thinh.Nguyen@synopsys.com
Changes in v3:
- Check if the desired mode is OTG, then keep the old flow
- Remove condition for OTG support only since the device can still be
configured DRD host/device mode only
- Remove redundant hw_mode check since __dwc3_set_mode() only applies
when hw_mode is DRD Changes in v2:
- Initialize mutex per device and not as global mutex.
- Add additional checks for DRD only mode
drivers/usb/dwc3/core.c | 27 +++++++++++++++++++++++++++ drivers/usb/dwc3/core.h | 5 +++++ 2 files changed, 32 insertions(+)
Hi John,
If possible, can you run a test with this version on your platform?
Thanks, Thinh
I tested this on edison-arduino with this patch on top of usb-next (5.12-rc7 + "increase BESL baseline to 6" to prevent throttling").
On this platform there is a physical switch to switch roles. With this patch I find:
switch to host mode always works fine
switch to gadget mode I need to flip the switch 3x
(gadget-host-gadget).
An error message appears on the gadget side "dwc3 dwc3.0.auto: timed out waiting for SETUP phase" appears, but then the device connects to my PC, no throttling.
- alternatively I can switch to gadget 1x and then unplug/replug the
cable.
No error message and connects fine.
- if I flip the switch only once, on the PC side I get:
kernel: usb 1-5: new high-speed USB device number 18 using xhci_hcd kernel: usb 1-5: New USB device found, idVendor=1d6b, idProduct=0104, bcdDevice= 1.00 kernel: usb 1-5: New USB device strings: Mfr=1, Product=2, SerialNumber=3 kernel: usb 1-5: Product: USBArmory Gadget kernel: usb 1-5: Manufacturer: USBArmory kernel: usb 1-5: SerialNumber: 0123456789abcdef kernel: usb 1-5: can't set config #1, error -110
The device failed at set_configuration() request and timed out. It probably timed out from the status stage looking at the device err print.
Then if I wait long enough on the gadget side I get:
root@yuna:~# ifconfig
usb0: flags=-28605<UP,BROADCAST,RUNNING,MULTICAST,DYNAMIC> mtu 1500 inet 169.254.119.239 netmask 255.255.0.0 broadcast 169.254.255.255 inet6 fe80::a8bb:ccff:fedd:eef1 prefixlen 64 scopeid 0x20<link> ether aa:bb:cc:dd:ee:f1 txqueuelen 1000 (Ethernet) RX packets 490424 bytes 735146578 (701.0 MiB) RX errors 0 dropped 191 overruns 0 frame 0 TX packets 35279 bytes 2532746 (2.4 MiB) TX errors 0 dropped 0 overruns 0 carrier 0 collisions 0
(correct would be: inet 10.42.0.221 netmask 255.255.255.0 broadcast 10.42.0.255)
So much improved now, but it seems I am still missing something on plug.
That's great! We can look at it further. Can you capture the tracepoints of the issue. Also, can you try with mass_storage gadget to see if the result is the same?
I have already gser, eem, mass_storage and uac2 combo. When eem fails, the mass_storage and uac2 don't appear (on KDE you get all kind of popups when they appear).
So either all works, or all fails.
I'll trace this later today.
Trace capturing switch from host-> gadget here https://github.com/andy-shev/linux/files/6329600/5.12-rc7%2Busb-next.zip
(Issue history: https://github.com/andy-shev/linux/issues/31)
On the PC side this resulted to:
apr 17 18:17:44 delfion kernel: usb 1-5: new high-speed USB device number 12 using xhci_hcd apr 17 18:17:44 delfion kernel: usb 1-5: New USB device found, idVendor=1d6b, idProduct=0104, bcdDevice= 1.00 apr 17 18:17:44 delfion kernel: usb 1-5: New USB device strings: Mfr=1, Product=2, SerialNumber=3 apr 17 18:17:44 delfion kernel: usb 1-5: Product: USBArmory Gadget apr 17 18:17:44 delfion kernel: usb 1-5: Manufacturer: USBArmory apr 17 18:17:44 delfion kernel: usb 1-5: SerialNumber: 0123456789abcdef apr 17 18:17:49 delfion kernel: usb 1-5: can't set config #1, error -110
Thanks for all your help!
Thanks, Thinh
Ferry Toth wrote:
Hi
Op 17-04-2021 om 16:22 schreef Ferry Toth:
Hi
Op 17-04-2021 om 04:27 schreef Thinh Nguyen:
Ferry Toth wrote:
Hi
Op 16-04-2021 om 00:23 schreef Thinh Nguyen:
Thinh Nguyen wrote:
From: Yu Chen chenyu56@huawei.com From: John Stultz john.stultz@linaro.org
According to the programming guide, to switch mode for DRD controller, the driver needs to do the following.
To switch from device to host:
- Reset controller with GCTL.CoreSoftReset
- Set GCTL.PrtCapDir(host mode)
- Reset the host with USBCMD.HCRESET
- Then follow up with the initializing host registers sequence
To switch from host to device:
- Reset controller with GCTL.CoreSoftReset
- Set GCTL.PrtCapDir(device mode)
- Reset the device with DCTL.CSftRst
- Then follow up with the initializing registers sequence
Currently we're missing step 1) to do GCTL.CoreSoftReset and step 3) of switching from host to device. John Stult reported a lockup issue seen with HiKey960 platform without these steps[1]. Similar issue is observed with Ferry's testing platform[2].
So, apply the required steps along with some fixes to Yu Chen's and John Stultz's version. The main fixes to their versions are the missing wait for clocks synchronization before clearing GCTL.CoreSoftReset and only apply DCTL.CSftRst when switching from host to device.
[1] https://urldefense.com/v3/__https://lore.kernel.org/linux-usb/20210108015115...
[2] https://urldefense.com/v3/__https://lore.kernel.org/linux-usb/0ba7a6ba-e6a7-...
Cc: Andy Shevchenko andy.shevchenko@gmail.com Cc: Ferry Toth fntoth@gmail.com Cc: Wesley Cheng wcheng@codeaurora.org Cc: stable@vger.kernel.org Fixes: 41ce1456e1db ("usb: dwc3: core: make dwc3_set_mode() work properly") Signed-off-by: Yu Chen chenyu56@huawei.com Signed-off-by: John Stultz john.stultz@linaro.org Signed-off-by: Thinh Nguyen Thinh.Nguyen@synopsys.com
Changes in v3:
- Check if the desired mode is OTG, then keep the old flow
- Remove condition for OTG support only since the device can still be
configured DRD host/device mode only
- Remove redundant hw_mode check since __dwc3_set_mode() only applies
when hw_mode is DRD Changes in v2:
- Initialize mutex per device and not as global mutex.
- Add additional checks for DRD only mode
drivers/usb/dwc3/core.c | 27 +++++++++++++++++++++++++++ drivers/usb/dwc3/core.h | 5 +++++ 2 files changed, 32 insertions(+)
Hi John,
If possible, can you run a test with this version on your platform?
Thanks, Thinh
I tested this on edison-arduino with this patch on top of usb-next (5.12-rc7 + "increase BESL baseline to 6" to prevent throttling").
On this platform there is a physical switch to switch roles. With this patch I find:
switch to host mode always works fine
switch to gadget mode I need to flip the switch 3x
(gadget-host-gadget).
An error message appears on the gadget side "dwc3 dwc3.0.auto: timed out waiting for SETUP phase" appears, but then the device connects to my PC, no throttling.
- alternatively I can switch to gadget 1x and then unplug/replug the
cable.
No error message and connects fine.
- if I flip the switch only once, on the PC side I get:
kernel: usb 1-5: new high-speed USB device number 18 usingxhci_hcd kernel: usb 1-5: New USB device found, idVendor=1d6b, idProduct=0104, bcdDevice= 1.00 kernel: usb 1-5: New USB device strings: Mfr=1, Product=2, SerialNumber=3 kernel: usb 1-5: Product: USBArmory Gadget kernel: usb 1-5: Manufacturer: USBArmory kernel: usb 1-5: SerialNumber: 0123456789abcdef kernel: usb 1-5: can't set config #1, error -110
The device failed at set_configuration() request and timed out. It probably timed out from the status stage looking at the device err print.
Then if I wait long enough on the gadget side I get:
root@yuna:~# ifconfig
usb0: flags=-28605<UP,BROADCAST,RUNNING,MULTICAST,DYNAMIC> mtu 1500 inet 169.254.119.239 netmask 255.255.0.0 broadcast 169.254.255.255 inet6 fe80::a8bb:ccff:fedd:eef1 prefixlen 64 scopeid 0x20<link> ether aa:bb:cc:dd:ee:f1 txqueuelen 1000 (Ethernet) RX packets 490424 bytes 735146578 (701.0 MiB) RX errors 0 dropped 191 overruns 0 frame 0 TX packets 35279 bytes 2532746 (2.4 MiB) TX errors 0 dropped 0 overruns 0 carrier 0 collisions 0
(correct would be: inet 10.42.0.221 netmask 255.255.255.0 broadcast 10.42.0.255)
So much improved now, but it seems I am still missing something on plug.
That's great! We can look at it further. Can you capture the tracepoints of the issue. Also, can you try with mass_storage gadget to see if the result is the same?
I have already gser, eem, mass_storage and uac2 combo. When eem fails, the mass_storage and uac2 don't appear (on KDE you get all kind of popups when they appear).
So either all works, or all fails.
I'll trace this later today.
Trace capturing switch from host-> gadget here https://urldefense.com/v3/__https://github.com/andy-shev/linux/files/6329600...
(Issue history: https://urldefense.com/v3/__https://github.com/andy-shev/linux/issues/31__%3... )
On the PC side this resulted to:
apr 17 18:17:44 delfion kernel: usb 1-5: new high-speed USB device number 12 using xhci_hcd apr 17 18:17:44 delfion kernel: usb 1-5: New USB device found, idVendor=1d6b, idProduct=0104, bcdDevice= 1.00 apr 17 18:17:44 delfion kernel: usb 1-5: New USB device strings: Mfr=1, Product=2, SerialNumber=3 apr 17 18:17:44 delfion kernel: usb 1-5: Product: USBArmory Gadget apr 17 18:17:44 delfion kernel: usb 1-5: Manufacturer: USBArmory apr 17 18:17:44 delfion kernel: usb 1-5: SerialNumber: 0123456789abcdef apr 17 18:17:49 delfion kernel: usb 1-5: can't set config #1, error -110
Thanks for all your help!
Looks like it's LPM related again. To confirm, try this: Disable LPM with this property "snps,usb2-gadget-lpm-disable" (Note that it's not the same as "snps,dis_enblslpm_quirk")
Make sure that your testing kernel has this patch [1] 475e8be53d04 ("usb: dwc3: gadget: Check for disabled LPM quirk")
[1] https://git.kernel.org/pub/scm/linux/kernel/git/gregkh/usb.git/commit/?h=usb...
The failure you saw was probably due the gadget function attempting to start a delayed status stage of the SET_CONFIGURATION request. By this time, the host already put the device in low power.
The START_TRANSFER command needs to be executed while the device is on "ON" state (or U0 if eSS). We shouldn't use dwc->link_state to check for link state because we only enable link state change interrupt for some controller versions.
Once you confirms disabling LPM works, try this fix:
diff --git a/drivers/usb/dwc3/gadget.c b/drivers/usb/dwc3/gadget.c index 6227641f2d31..06cdec79244e 100644 --- a/drivers/usb/dwc3/gadget.c +++ b/drivers/usb/dwc3/gadget.c @@ -309,10 +309,14 @@ int dwc3_send_gadget_ep_cmd(struct dwc3_ep *dep, unsigned int cmd,
if (DWC3_DEPCMD_CMD(cmd) == DWC3_DEPCMD_STARTTRANSFER) { int needs_wakeup; + u8 link_state;
- needs_wakeup = (dwc->link_state == DWC3_LINK_STATE_U1 || - dwc->link_state == DWC3_LINK_STATE_U2 || - dwc->link_state == DWC3_LINK_STATE_U3); + reg = dwc3_readl(dwc->regs, DWC3_DSTS); + link_state = DWC3_DSTS_USBLNKST(reg); + + needs_wakeup = (link_state == DWC3_LINK_STATE_U1 || + link_state == DWC3_LINK_STATE_U2 || + link_state == DWC3_LINK_STATE_U3);
if (unlikely(needs_wakeup)) { ret = __dwc3_gadget_wakeup(dwc); @@ -1989,6 +1993,8 @@ static int __dwc3_gadget_wakeup(struct dwc3 *dwc) case DWC3_LINK_STATE_RESET: case DWC3_LINK_STATE_RX_DET: /* in HS, means Early Suspend */ case DWC3_LINK_STATE_U3: /* in HS, means SUSPEND */ + case DWC3_LINK_STATE_U2: /* in HS, means Sleep (L1) */ + case DWC3_LINK_STATE_U1: case DWC3_LINK_STATE_RESUME: break; default:
BR, Thinh
On Mon, Apr 19, 2021 at 2:03 AM Thinh Nguyen Thinh.Nguyen@synopsys.com wrote:
Ferry Toth wrote:
Op 17-04-2021 om 16:22 schreef Ferry Toth:
Op 17-04-2021 om 04:27 schreef Thinh Nguyen:
Ferry Toth wrote:
Op 16-04-2021 om 00:23 schreef Thinh Nguyen:
Thinh Nguyen wrote:
On the PC side this resulted to:
apr 17 18:17:44 delfion kernel: usb 1-5: new high-speed USB device number 12 using xhci_hcd apr 17 18:17:44 delfion kernel: usb 1-5: New USB device found, idVendor=1d6b, idProduct=0104, bcdDevice= 1.00 apr 17 18:17:44 delfion kernel: usb 1-5: New USB device strings: Mfr=1, Product=2, SerialNumber=3 apr 17 18:17:44 delfion kernel: usb 1-5: Product: USBArmory Gadget apr 17 18:17:44 delfion kernel: usb 1-5: Manufacturer: USBArmory apr 17 18:17:44 delfion kernel: usb 1-5: SerialNumber: 0123456789abcdef apr 17 18:17:49 delfion kernel: usb 1-5: can't set config #1, error -110
Thanks for all your help!
Looks like it's LPM related again. To confirm, try this: Disable LPM with this property "snps,usb2-gadget-lpm-disable" (Note that it's not the same as "snps,dis_enblslpm_quirk")
Make sure that your testing kernel has this patch [1] 475e8be53d04 ("usb: dwc3: gadget: Check for disabled LPM quirk")
Thinh, Ferry, I'm a bit lost in this thread. Can you summarize what patches I have to apply on top of v5.12-rc8 to mitigate issues, mentioned in this thread?
(Sounds to me there are like ~5 patches floating around)
I'll try to find time to test on my side.
[1] https://git.kernel.org/pub/scm/linux/kernel/git/gregkh/usb.git/commit/?h=usb...
Op 19-04-2021 om 10:43 schreef Andy Shevchenko:
On Mon, Apr 19, 2021 at 2:03 AM Thinh Nguyen Thinh.Nguyen@synopsys.com wrote:
Ferry Toth wrote:
Op 17-04-2021 om 16:22 schreef Ferry Toth:
Op 17-04-2021 om 04:27 schreef Thinh Nguyen:
Ferry Toth wrote:
Op 16-04-2021 om 00:23 schreef Thinh Nguyen: > Thinh Nguyen wrote:
On the PC side this resulted to:
apr 17 18:17:44 delfion kernel: usb 1-5: new high-speed USB device number 12 using xhci_hcd apr 17 18:17:44 delfion kernel: usb 1-5: New USB device found, idVendor=1d6b, idProduct=0104, bcdDevice= 1.00 apr 17 18:17:44 delfion kernel: usb 1-5: New USB device strings: Mfr=1, Product=2, SerialNumber=3 apr 17 18:17:44 delfion kernel: usb 1-5: Product: USBArmory Gadget apr 17 18:17:44 delfion kernel: usb 1-5: Manufacturer: USBArmory apr 17 18:17:44 delfion kernel: usb 1-5: SerialNumber: 0123456789abcdef apr 17 18:17:49 delfion kernel: usb 1-5: can't set config #1, error -110
Thanks for all your help!
Looks like it's LPM related again. To confirm, try this: Disable LPM with this property "snps,usb2-gadget-lpm-disable" (Note that it's not the same as "snps,dis_enblslpm_quirk") Make sure that your testing kernel has this patch [1] 475e8be53d04 ("usb: dwc3: gadget: Check for disabled LPM quirk")
Thinh, Ferry, I'm a bit lost in this thread. Can you summarize what patches I have to apply on top of v5.12-rc8 to mitigate issues, mentioned in this thread?
(Sounds to me there are like ~5 patches floating around)
I have 3 on top usb-next (-rc7)
usb: dwc3: core: Do core softreset when switch mode
usb: dwc3: gadget: START_TRANSFER command needs to be executed while the device is on "ON" state
usb: gadget: increase BESL baseline to 6
See here: https://github.com/htot/linux/commits/eds-acpi-5.12-rc7
I'll try to find time to test on my side.
[1] https://git.kernel.org/pub/scm/linux/kernel/git/gregkh/usb.git/commit/?h=usb...
Hi,
Thinh Nguyen Thinh.Nguyen@synopsys.com writes:
(Issue history: https://urldefense.com/v3/__https://github.com/andy-shev/linux/issues/31__%3... )
On the PC side this resulted to:
apr 17 18:17:44 delfion kernel: usb 1-5: new high-speed USB device number 12 using xhci_hcd apr 17 18:17:44 delfion kernel: usb 1-5: New USB device found, idVendor=1d6b, idProduct=0104, bcdDevice= 1.00 apr 17 18:17:44 delfion kernel: usb 1-5: New USB device strings: Mfr=1, Product=2, SerialNumber=3 apr 17 18:17:44 delfion kernel: usb 1-5: Product: USBArmory Gadget apr 17 18:17:44 delfion kernel: usb 1-5: Manufacturer: USBArmory apr 17 18:17:44 delfion kernel: usb 1-5: SerialNumber: 0123456789abcdef apr 17 18:17:49 delfion kernel: usb 1-5: can't set config #1, error -110
Thanks for all your help!
Looks like it's LPM related again. To confirm, try this: Disable LPM with this property "snps,usb2-gadget-lpm-disable" (Note that it's not the same as "snps,dis_enblslpm_quirk")
Make sure that your testing kernel has this patch [1] 475e8be53d04 ("usb: dwc3: gadget: Check for disabled LPM quirk")
[1] https://git.kernel.org/pub/scm/linux/kernel/git/gregkh/usb.git/commit/?h=usb...
The failure you saw was probably due the gadget function attempting to start a delayed status stage of the SET_CONFIGURATION request. By this time, the host already put the device in low power.
The START_TRANSFER command needs to be executed while the device is on "ON" state (or U0 if eSS). We shouldn't use dwc->link_state to check for link state because we only enable link state change interrupt for some controller versions.
Once you confirms disabling LPM works, try this fix:
diff --git a/drivers/usb/dwc3/gadget.c b/drivers/usb/dwc3/gadget.c index 6227641f2d31..06cdec79244e 100644 --- a/drivers/usb/dwc3/gadget.c +++ b/drivers/usb/dwc3/gadget.c @@ -309,10 +309,14 @@ int dwc3_send_gadget_ep_cmd(struct dwc3_ep *dep, unsigned int cmd, if (DWC3_DEPCMD_CMD(cmd) == DWC3_DEPCMD_STARTTRANSFER) { int needs_wakeup;
u8 link_state;
needs_wakeup = (dwc->link_state == DWC3_LINK_STATE_U1 ||dwc->link_state == DWC3_LINK_STATE_U2 ||dwc->link_state == DWC3_LINK_STATE_U3);
reg = dwc3_readl(dwc->regs, DWC3_DSTS);link_state = DWC3_DSTS_USBLNKST(reg);needs_wakeup = (link_state == DWC3_LINK_STATE_U1 ||link_state == DWC3_LINK_STATE_U2 ||link_state == DWC3_LINK_STATE_U3);
this makes sense. We used to track state using the state change interrupts, but that's long since being disabled. I think, either way, we need this fix.
Hi
Op 19-04-2021 om 01:03 schreef Thinh Nguyen:
Ferry Toth wrote:
Hi
Op 17-04-2021 om 16:22 schreef Ferry Toth:
Hi
Op 17-04-2021 om 04:27 schreef Thinh Nguyen:
Ferry Toth wrote:
Hi
Op 16-04-2021 om 00:23 schreef Thinh Nguyen:
Thinh Nguyen wrote: > From: Yu Chen chenyu56@huawei.com > From: John Stultz john.stultz@linaro.org > > According to the programming guide, to switch mode for DRD > controller, > the driver needs to do the following. > > To switch from device to host: > 1. Reset controller with GCTL.CoreSoftReset > 2. Set GCTL.PrtCapDir(host mode) > 3. Reset the host with USBCMD.HCRESET > 4. Then follow up with the initializing host registers sequence > > To switch from host to device: > 1. Reset controller with GCTL.CoreSoftReset > 2. Set GCTL.PrtCapDir(device mode) > 3. Reset the device with DCTL.CSftRst > 4. Then follow up with the initializing registers sequence > > Currently we're missing step 1) to do GCTL.CoreSoftReset and step > 3) of > switching from host to device. John Stult reported a lockup issue > seen > with HiKey960 platform without these steps[1]. Similar issue is > observed > with Ferry's testing platform[2]. > > So, apply the required steps along with some fixes to Yu Chen's > and John > Stultz's version. The main fixes to their versions are the missing > wait > for clocks synchronization before clearing GCTL.CoreSoftReset and > only > apply DCTL.CSftRst when switching from host to device. > > [1] > https://urldefense.com/v3/__https://lore.kernel.org/linux-usb/20210108015115... > > > [2] > https://urldefense.com/v3/__https://lore.kernel.org/linux-usb/0ba7a6ba-e6a7-... > > > > Cc: Andy Shevchenko andy.shevchenko@gmail.com > Cc: Ferry Toth fntoth@gmail.com > Cc: Wesley Cheng wcheng@codeaurora.org > Cc: stable@vger.kernel.org > Fixes: 41ce1456e1db ("usb: dwc3: core: make dwc3_set_mode() work > properly") > Signed-off-by: Yu Chen chenyu56@huawei.com > Signed-off-by: John Stultz john.stultz@linaro.org > Signed-off-by: Thinh Nguyen Thinh.Nguyen@synopsys.com > --- > Changes in v3: > - Check if the desired mode is OTG, then keep the old flow > - Remove condition for OTG support only since the device can still be > configured DRD host/device mode only > - Remove redundant hw_mode check since __dwc3_set_mode() only applies > when > hw_mode is DRD > Changes in v2: > - Initialize mutex per device and not as global mutex. > - Add additional checks for DRD only mode > > drivers/usb/dwc3/core.c | 27 +++++++++++++++++++++++++++ > drivers/usb/dwc3/core.h | 5 +++++ > 2 files changed, 32 insertions(+) > Hi John,
If possible, can you run a test with this version on your platform?
Thanks, Thinh
I tested this on edison-arduino with this patch on top of usb-next (5.12-rc7 + "increase BESL baseline to 6" to prevent throttling").
On this platform there is a physical switch to switch roles. With this patch I find:
switch to host mode always works fine
switch to gadget mode I need to flip the switch 3x
(gadget-host-gadget).
An error message appears on the gadget side "dwc3 dwc3.0.auto: timed out waiting for SETUP phase" appears, but then the device connects to my PC, no throttling.
- alternatively I can switch to gadget 1x and then unplug/replug the
cable.
No error message and connects fine.
- if I flip the switch only once, on the PC side I get:
kernel: usb 1-5: new high-speed USB device number 18 usingxhci_hcd kernel: usb 1-5: New USB device found, idVendor=1d6b, idProduct=0104, bcdDevice= 1.00 kernel: usb 1-5: New USB device strings: Mfr=1, Product=2, SerialNumber=3 kernel: usb 1-5: Product: USBArmory Gadget kernel: usb 1-5: Manufacturer: USBArmory kernel: usb 1-5: SerialNumber: 0123456789abcdef kernel: usb 1-5: can't set config #1, error -110
The device failed at set_configuration() request and timed out. It probably timed out from the status stage looking at the device err print.
Then if I wait long enough on the gadget side I get:
root@yuna:~# ifconfig
usb0: flags=-28605<UP,BROADCAST,RUNNING,MULTICAST,DYNAMIC> mtu 1500 inet 169.254.119.239 netmask 255.255.0.0 broadcast 169.254.255.255 inet6 fe80::a8bb:ccff:fedd:eef1 prefixlen 64 scopeid 0x20<link> ether aa:bb:cc:dd:ee:f1 txqueuelen 1000 (Ethernet) RX packets 490424 bytes 735146578 (701.0 MiB) RX errors 0 dropped 191 overruns 0 frame 0 TX packets 35279 bytes 2532746 (2.4 MiB) TX errors 0 dropped 0 overruns 0 carrier 0 collisions 0
(correct would be: inet 10.42.0.221 netmask 255.255.255.0 broadcast 10.42.0.255)
So much improved now, but it seems I am still missing something on plug.
That's great! We can look at it further. Can you capture the tracepoints of the issue. Also, can you try with mass_storage gadget to see if the result is the same?
I have already gser, eem, mass_storage and uac2 combo. When eem fails, the mass_storage and uac2 don't appear (on KDE you get all kind of popups when they appear).
So either all works, or all fails.
I'll trace this later today.
Trace capturing switch from host-> gadget here https://urldefense.com/v3/__https://github.com/andy-shev/linux/files/6329600...
(Issue history: https://urldefense.com/v3/__https://github.com/andy-shev/linux/issues/31__%3... )
On the PC side this resulted to:
apr 17 18:17:44 delfion kernel: usb 1-5: new high-speed USB device number 12 using xhci_hcd apr 17 18:17:44 delfion kernel: usb 1-5: New USB device found, idVendor=1d6b, idProduct=0104, bcdDevice= 1.00 apr 17 18:17:44 delfion kernel: usb 1-5: New USB device strings: Mfr=1, Product=2, SerialNumber=3 apr 17 18:17:44 delfion kernel: usb 1-5: Product: USBArmory Gadget apr 17 18:17:44 delfion kernel: usb 1-5: Manufacturer: USBArmory apr 17 18:17:44 delfion kernel: usb 1-5: SerialNumber: 0123456789abcdef apr 17 18:17:49 delfion kernel: usb 1-5: can't set config #1, error -110
Thanks for all your help!
Looks like it's LPM related again. To confirm, try this: Disable LPM with this property "snps,usb2-gadget-lpm-disable" (Note that it's not the same as "snps,dis_enblslpm_quirk")
Yes, I confirm this helps.
Note: on startup I was in host mode, with gadget cable plugged. The first switch to gadget didn't work, all subsequent switches did work, as well as unplug/plug the cable.
Make sure that your testing kernel has this patch [1] 475e8be53d04 ("usb: dwc3: gadget: Check for disabled LPM quirk")
[1] https://git.kernel.org/pub/scm/linux/kernel/git/gregkh/usb.git/commit/?h=usb...
The failure you saw was probably due the gadget function attempting to start a delayed status stage of the SET_CONFIGURATION request. By this time, the host already put the device in low power.
The START_TRANSFER command needs to be executed while the device is on "ON" state (or U0 if eSS). We shouldn't use dwc->link_state to check for link state because we only enable link state change interrupt for some controller versions.
Once you confirms disabling LPM works, try this fix:
diff --git a/drivers/usb/dwc3/gadget.c b/drivers/usb/dwc3/gadget.c index 6227641f2d31..06cdec79244e 100644 --- a/drivers/usb/dwc3/gadget.c +++ b/drivers/usb/dwc3/gadget.c @@ -309,10 +309,14 @@ int dwc3_send_gadget_ep_cmd(struct dwc3_ep *dep, unsigned int cmd, if (DWC3_DEPCMD_CMD(cmd) == DWC3_DEPCMD_STARTTRANSFER) { int needs_wakeup;
u8 link_state;
needs_wakeup = (dwc->link_state == DWC3_LINK_STATE_U1 ||dwc->link_state == DWC3_LINK_STATE_U2 ||dwc->link_state == DWC3_LINK_STATE_U3);
reg = dwc3_readl(dwc->regs, DWC3_DSTS);link_state = DWC3_DSTS_USBLNKST(reg);needs_wakeup = (link_state == DWC3_LINK_STATE_U1 ||link_state == DWC3_LINK_STATE_U2 ||link_state == DWC3_LINK_STATE_U3);if (unlikely(needs_wakeup)) { ret = __dwc3_gadget_wakeup(dwc); @@ -1989,6 +1993,8 @@ static int __dwc3_gadget_wakeup(struct dwc3 *dwc) case DWC3_LINK_STATE_RESET: case DWC3_LINK_STATE_RX_DET: /* in HS, means Early Suspend */ case DWC3_LINK_STATE_U3: /* in HS, means SUSPEND */
case DWC3_LINK_STATE_U2: /* in HS, means Sleep (L1) */case DWC3_LINK_STATE_U1: case DWC3_LINK_STATE_RESUME: break; default:
Same (good) result as with "snps,usb2-gadget-lpm-disable". Including first switch from host->gadget not working.
After a 2 - 4 minutes the connection is dropped and reconnected.
On the gadget end journal shows:
Apr 19 22:08:42 yuna systemd-networkd[507]: usb0: Lost carrier Apr 19 22:08:42 yuna systemd-journald[417]: Forwarding to syslog missed 1 messages. Apr 19 22:08:42 yuna systemd-timesyncd[469]: No network connectivity, watching for changes. Apr 19 22:08:42 yuna kernel: IPv6: ADDRCONF(NETDEV_CHANGE): usb0: link becomes ready Apr 19 22:08:42 yuna kernel[480]: [ 624.382929] IPv6: ADDRCONF(NETDEV_CHANGE): usb0: link becomes ready Apr 19 22:08:42 yuna systemd-networkd[507]: usb0: Gained carrier Apr 19 22:08:44 yuna systemd-networkd[507]: usb0: Gained IPv6LL Apr 19 22:08:44 yuna systemd-timesyncd[469]: Network configuration changed, trying to establish connection. Apr 19 22:08:57 yuna systemd-timesyncd[469]: Initial synchronization to time server 216.239.35.8:123 (time3.google.com).
So, drops and immediately reconnects.
BR, Thinh
Ferry Toth wrote:
Hi
Op 19-04-2021 om 01:03 schreef Thinh Nguyen:
Ferry Toth wrote:
Hi
Op 17-04-2021 om 16:22 schreef Ferry Toth:
Hi
Op 17-04-2021 om 04:27 schreef Thinh Nguyen:
Ferry Toth wrote:
Hi
Op 16-04-2021 om 00:23 schreef Thinh Nguyen: > Thinh Nguyen wrote: >> From: Yu Chen chenyu56@huawei.com >> From: John Stultz john.stultz@linaro.org >> >> According to the programming guide, to switch mode for DRD >> controller, >> the driver needs to do the following. >> >> To switch from device to host: >> 1. Reset controller with GCTL.CoreSoftReset >> 2. Set GCTL.PrtCapDir(host mode) >> 3. Reset the host with USBCMD.HCRESET >> 4. Then follow up with the initializing host registers sequence >> >> To switch from host to device: >> 1. Reset controller with GCTL.CoreSoftReset >> 2. Set GCTL.PrtCapDir(device mode) >> 3. Reset the device with DCTL.CSftRst >> 4. Then follow up with the initializing registers sequence >> >> Currently we're missing step 1) to do GCTL.CoreSoftReset and step >> 3) of >> switching from host to device. John Stult reported a lockup issue >> seen >> with HiKey960 platform without these steps[1]. Similar issue is >> observed >> with Ferry's testing platform[2]. >> >> So, apply the required steps along with some fixes to Yu Chen's >> and John >> Stultz's version. The main fixes to their versions are the missing >> wait >> for clocks synchronization before clearing GCTL.CoreSoftReset and >> only >> apply DCTL.CSftRst when switching from host to device. >> >> [1] >> https://urldefense.com/v3/__https://lore.kernel.org/linux-usb/20210108015115... >> >> >> >> [2] >> https://urldefense.com/v3/__https://lore.kernel.org/linux-usb/0ba7a6ba-e6a7-... >> >> >> >> >> Cc: Andy Shevchenko andy.shevchenko@gmail.com >> Cc: Ferry Toth fntoth@gmail.com >> Cc: Wesley Cheng wcheng@codeaurora.org >> Cc: stable@vger.kernel.org >> Fixes: 41ce1456e1db ("usb: dwc3: core: make dwc3_set_mode() work >> properly") >> Signed-off-by: Yu Chen chenyu56@huawei.com >> Signed-off-by: John Stultz john.stultz@linaro.org >> Signed-off-by: Thinh Nguyen Thinh.Nguyen@synopsys.com >> --- >> Changes in v3: >> - Check if the desired mode is OTG, then keep the old flow >> - Remove condition for OTG support only since the device can >> still be >> configured DRD host/device mode only >> - Remove redundant hw_mode check since __dwc3_set_mode() only >> applies >> when >> hw_mode is DRD >> Changes in v2: >> - Initialize mutex per device and not as global mutex. >> - Add additional checks for DRD only mode >> >> drivers/usb/dwc3/core.c | 27 +++++++++++++++++++++++++++ >> drivers/usb/dwc3/core.h | 5 +++++ >> 2 files changed, 32 insertions(+) >> > Hi John, > > If possible, can you run a test with this version on your platform? > > Thanks, > Thinh > I tested this on edison-arduino with this patch on top of usb-next (5.12-rc7 + "increase BESL baseline to 6" to prevent throttling").
On this platform there is a physical switch to switch roles. With this patch I find:
switch to host mode always works fine
switch to gadget mode I need to flip the switch 3x
(gadget-host-gadget).
An error message appears on the gadget side "dwc3 dwc3.0.auto: timed out waiting for SETUP phase" appears, but then the device connects to my PC, no throttling.
- alternatively I can switch to gadget 1x and then unplug/replug the
cable.
No error message and connects fine.
- if I flip the switch only once, on the PC side I get:
kernel: usb 1-5: new high-speed USB device number 18 usingxhci_hcd kernel: usb 1-5: New USB device found, idVendor=1d6b, idProduct=0104, bcdDevice= 1.00 kernel: usb 1-5: New USB device strings: Mfr=1, Product=2, SerialNumber=3 kernel:usb 1-5: Product: USBArmory Gadget kernel: usb 1-5: Manufacturer: USBArmory kernel: usb 1-5: SerialNumber: 0123456789abcdef kernel: usb 1-5: can't set config #1, error -110
The device failed at set_configuration() request and timed out. It probably timed out from the status stage looking at the device err print.
Then if I wait long enough on the gadget side I get:
root@yuna:~# ifconfig
usb0: flags=-28605<UP,BROADCAST,RUNNING,MULTICAST,DYNAMIC> mtu 1500 inet 169.254.119.239 netmask 255.255.0.0 broadcast 169.254.255.255 inet6 fe80::a8bb:ccff:fedd:eef1 prefixlen 64 scopeid 0x20<link> ether aa:bb:cc:dd:ee:f1 txqueuelen 1000 (Ethernet) RX packets 490424 bytes 735146578 (701.0 MiB) RX errors 0 dropped 191 overruns 0 frame 0 TX packets 35279 bytes 2532746 (2.4 MiB) TX errors 0 dropped 0 overruns 0 carrier 0 collisions 0
(correct would be: inet 10.42.0.221 netmask 255.255.255.0 broadcast 10.42.0.255)
So much improved now, but it seems I am still missing something on plug.
That's great! We can look at it further. Can you capture the tracepoints of the issue. Also, can you try with mass_storage gadget to see if the result is the same?
I have already gser, eem, mass_storage and uac2 combo. When eem fails, the mass_storage and uac2 don't appear (on KDE you get all kind of popups when they appear).
So either all works, or all fails.
I'll trace this later today.
Trace capturing switch from host-> gadget here https://urldefense.com/v3/__https://github.com/andy-shev/linux/files/6329600...
(Issue history: https://urldefense.com/v3/__https://github.com/andy-shev/linux/issues/31__%3...
)
On the PC side this resulted to:
apr 17 18:17:44 delfion kernel: usb 1-5: new high-speed USB device number 12 using xhci_hcd apr 17 18:17:44 delfion kernel: usb 1-5: New USB device found, idVendor=1d6b, idProduct=0104, bcdDevice= 1.00 apr 17 18:17:44 delfion kernel: usb 1-5: New USB device strings: Mfr=1, Product=2, SerialNumber=3 apr 17 18:17:44 delfion kernel: usb 1-5: Product: USBArmory Gadget apr 17 18:17:44 delfion kernel: usb 1-5: Manufacturer: USBArmory apr 17 18:17:44 delfion kernel: usb 1-5: SerialNumber: 0123456789abcdef apr 17 18:17:49 delfion kernel: usb 1-5: can't set config #1, error -110
Thanks for all your help!
Looks like it's LPM related again. To confirm, try this: Disable LPM with this property "snps,usb2-gadget-lpm-disable" (Note that it's not the same as "snps,dis_enblslpm_quirk")
Yes, I confirm this helps.
Note: on startup I was in host mode, with gadget cable plugged. The first switch to gadget didn't work, all subsequent switches did work, as well as unplug/plug the cable.
Make sure that your testing kernel has this patch [1] 475e8be53d04 ("usb: dwc3: gadget: Check for disabled LPM quirk")
[1] https://urldefense.com/v3/__https://git.kernel.org/pub/scm/linux/kernel/git/...
The failure you saw was probably due the gadget function attempting to start a delayed status stage of the SET_CONFIGURATION request. By this time, the host already put the device in low power.
The START_TRANSFER command needs to be executed while the device is on "ON" state (or U0 if eSS). We shouldn't use dwc->link_state to check for link state because we only enable link state change interrupt for some controller versions.
Once you confirms disabling LPM works, try this fix:
diff --git a/drivers/usb/dwc3/gadget.c b/drivers/usb/dwc3/gadget.c index 6227641f2d31..06cdec79244e 100644 --- a/drivers/usb/dwc3/gadget.c +++ b/drivers/usb/dwc3/gadget.c @@ -309,10 +309,14 @@ int dwc3_send_gadget_ep_cmd(struct dwc3_ep *dep, unsigned int cmd, if (DWC3_DEPCMD_CMD(cmd) == DWC3_DEPCMD_STARTTRANSFER) { int needs_wakeup; + u8 link_state; - needs_wakeup = (dwc->link_state == DWC3_LINK_STATE_U1 || - dwc->link_state == DWC3_LINK_STATE_U2|| - dwc->link_state == DWC3_LINK_STATE_U3); + reg = dwc3_readl(dwc->regs, DWC3_DSTS); + link_state = DWC3_DSTS_USBLNKST(reg);
+ needs_wakeup = (link_state == DWC3_LINK_STATE_U1 || + link_state == DWC3_LINK_STATE_U2 || + link_state == DWC3_LINK_STATE_U3); if (unlikely(needs_wakeup)) { ret = __dwc3_gadget_wakeup(dwc); @@ -1989,6 +1993,8 @@ static int __dwc3_gadget_wakeup(struct dwc3 *dwc) case DWC3_LINK_STATE_RESET: case DWC3_LINK_STATE_RX_DET: /* in HS, means Early Suspend */ case DWC3_LINK_STATE_U3: /* in HS, means SUSPEND */ + case DWC3_LINK_STATE_U2: /* in HS, means Sleep (L1) */ + case DWC3_LINK_STATE_U1: case DWC3_LINK_STATE_RESUME: break; default:
Same (good) result as with "snps,usb2-gadget-lpm-disable". Including first switch from host->gadget not working.
Great! Not sure why the first switch is not working, but it seems like we were able to eliminate quite a few issues. If you have more dwc3 tracepoints, we can take a look further.
After a 2 - 4 minutes the connection is dropped and reconnected.
Does this occur with LPM disabled also? We can review this issue further with more dwc3 tracepoints.
On the gadget end journal shows:
Apr 19 22:08:42 yuna systemd-networkd[507]: usb0: Lost carrier Apr 19 22:08:42 yuna systemd-journald[417]: Forwarding to syslog missed 1 messages. Apr 19 22:08:42 yuna systemd-timesyncd[469]: No network connectivity, watching for changes. Apr 19 22:08:42 yuna kernel: IPv6: ADDRCONF(NETDEV_CHANGE): usb0: link becomes ready Apr 19 22:08:42 yuna kernel[480]: [ 624.382929] IPv6: ADDRCONF(NETDEV_CHANGE): usb0: link becomes ready Apr 19 22:08:42 yuna systemd-networkd[507]: usb0: Gained carrier Apr 19 22:08:44 yuna systemd-networkd[507]: usb0: Gained IPv6LL Apr 19 22:08:44 yuna systemd-timesyncd[469]: Network configuration changed, trying to establish connection. Apr 19 22:08:57 yuna systemd-timesyncd[469]: Initial synchronization to time server 216.239.35.8:123 (time3.google.com).
So, drops and immediately reconnects.
From the look at the log here, it seems to be a reset from host (and an issue at the protocol level) unrelated to dwc3 driver or the controller. Hopefully and maybe we can get more clues from dwc3 tracepoints.
Thanks, Thinh
Hi
Op 19-04-2021 om 23:23 schreef Thinh Nguyen:
Ferry Toth wrote:
Hi
Op 19-04-2021 om 01:03 schreef Thinh Nguyen:
Ferry Toth wrote:
Hi
Op 17-04-2021 om 16:22 schreef Ferry Toth:
Hi
Op 17-04-2021 om 04:27 schreef Thinh Nguyen:
Ferry Toth wrote: > Hi > > Op 16-04-2021 om 00:23 schreef Thinh Nguyen: >> Thinh Nguyen wrote: >>> From: Yu Chen chenyu56@huawei.com >>> From: John Stultz john.stultz@linaro.org >>> >>> According to the programming guide, to switch mode for DRD >>> controller, >>> the driver needs to do the following. >>> >>> To switch from device to host: >>> 1. Reset controller with GCTL.CoreSoftReset >>> 2. Set GCTL.PrtCapDir(host mode) >>> 3. Reset the host with USBCMD.HCRESET >>> 4. Then follow up with the initializing host registers sequence >>> >>> To switch from host to device: >>> 1. Reset controller with GCTL.CoreSoftReset >>> 2. Set GCTL.PrtCapDir(device mode) >>> 3. Reset the device with DCTL.CSftRst >>> 4. Then follow up with the initializing registers sequence >>> >>> Currently we're missing step 1) to do GCTL.CoreSoftReset and step >>> 3) of >>> switching from host to device. John Stult reported a lockup issue >>> seen >>> with HiKey960 platform without these steps[1]. Similar issue is >>> observed >>> with Ferry's testing platform[2]. >>> >>> So, apply the required steps along with some fixes to Yu Chen's >>> and John >>> Stultz's version. The main fixes to their versions are the missing >>> wait >>> for clocks synchronization before clearing GCTL.CoreSoftReset and >>> only >>> apply DCTL.CSftRst when switching from host to device. >>> >>> [1] >>> https://urldefense.com/v3/__https://lore.kernel.org/linux-usb/20210108015115... >>> >>> >>> >>> [2] >>> https://urldefense.com/v3/__https://lore.kernel.org/linux-usb/0ba7a6ba-e6a7-... >>> >>> >>> >>> >>> Cc: Andy Shevchenko andy.shevchenko@gmail.com >>> Cc: Ferry Toth fntoth@gmail.com >>> Cc: Wesley Cheng wcheng@codeaurora.org >>> Cc: stable@vger.kernel.org >>> Fixes: 41ce1456e1db ("usb: dwc3: core: make dwc3_set_mode() work >>> properly") >>> Signed-off-by: Yu Chen chenyu56@huawei.com >>> Signed-off-by: John Stultz john.stultz@linaro.org >>> Signed-off-by: Thinh Nguyen Thinh.Nguyen@synopsys.com >>> --- >>> Changes in v3: >>> - Check if the desired mode is OTG, then keep the old flow >>> - Remove condition for OTG support only since the device can >>> still be >>> configured DRD host/device mode only >>> - Remove redundant hw_mode check since __dwc3_set_mode() only >>> applies >>> when >>> hw_mode is DRD >>> Changes in v2: >>> - Initialize mutex per device and not as global mutex. >>> - Add additional checks for DRD only mode >>> >>> drivers/usb/dwc3/core.c | 27 +++++++++++++++++++++++++++ >>> drivers/usb/dwc3/core.h | 5 +++++ >>> 2 files changed, 32 insertions(+) >>> >> Hi John, >> >> If possible, can you run a test with this version on your platform? >> >> Thanks, >> Thinh >> > I tested this on edison-arduino with this patch on top of usb-next > (5.12-rc7 + "increase BESL baseline to 6" to prevent throttling"). > > On this platform there is a physical switch to switch roles. With > this > patch I find: > > - switch to host mode always works fine > > - switch to gadget mode I need to flip the switch 3x > (gadget-host-gadget). > > An error message appears on the gadget side "dwc3 dwc3.0.auto: timed > out > waiting for SETUP phase" appears, but then the device connects to my > PC, > no throttling. > > - alternatively I can switch to gadget 1x and then unplug/replug the > cable. > > No error message and connects fine. > > - if I flip the switch only once, on the PC side I get: > > kernel: usb 1-5: new high-speed USB device number 18 > usingxhci_hcd > kernel: usb 1-5: New USB device found, idVendor=1d6b, > idProduct=0104, bcdDevice= 1.00 kernel: usb 1-5: New USB device > strings: Mfr=1, Product=2, SerialNumber=3 kernel:usb 1-5: > Product: > USBArmory Gadget kernel: usb 1-5: Manufacturer: USBArmory kernel: > usb 1-5: SerialNumber: 0123456789abcdef kernel: usb 1-5: can't > set > config #1, error -110 The device failed at set_configuration() request and timed out. It probably timed out from the status stage looking at the device err print.
> Then if I wait long enough on the gadget side I get: > > root@yuna:~# ifconfig > > usb0: flags=-28605<UP,BROADCAST,RUNNING,MULTICAST,DYNAMIC> mtu > 1500 > inet 169.254.119.239 netmask 255.255.0.0 broadcast > 169.254.255.255 > inet6 fe80::a8bb:ccff:fedd:eef1 prefixlen 64 scopeid 0x20<link> > ether aa:bb:cc:dd:ee:f1 txqueuelen 1000 (Ethernet) RX packets > 490424 > bytes 735146578 (701.0 MiB) RX errors 0 dropped 191 overruns 0 > frame > 0 TX packets 35279 bytes 2532746 (2.4 MiB) TX errors 0 dropped 0 > overruns 0 carrier 0 collisions 0 > > (correct would be: inet 10.42.0.221 netmask 255.255.255.0 broadcast > 10.42.0.255) > > So much improved now, but it seems I am still missing something on > plug. > That's great! We can look at it further. Can you capture the tracepoints of the issue. Also, can you try with mass_storage gadget to see if the result is the same?
I have already gser, eem, mass_storage and uac2 combo. When eem fails, the mass_storage and uac2 don't appear (on KDE you get all kind of popups when they appear).
So either all works, or all fails.
I'll trace this later today.
Trace capturing switch from host-> gadget here https://urldefense.com/v3/__https://github.com/andy-shev/linux/files/6329600...
(Issue history: https://urldefense.com/v3/__https://github.com/andy-shev/linux/issues/31__%3...
)
On the PC side this resulted to:
apr 17 18:17:44 delfion kernel: usb 1-5: new high-speed USB device number 12 using xhci_hcd apr 17 18:17:44 delfion kernel: usb 1-5: New USB device found, idVendor=1d6b, idProduct=0104, bcdDevice= 1.00 apr 17 18:17:44 delfion kernel: usb 1-5: New USB device strings: Mfr=1, Product=2, SerialNumber=3 apr 17 18:17:44 delfion kernel: usb 1-5: Product: USBArmory Gadget apr 17 18:17:44 delfion kernel: usb 1-5: Manufacturer: USBArmory apr 17 18:17:44 delfion kernel: usb 1-5: SerialNumber: 0123456789abcdef apr 17 18:17:49 delfion kernel: usb 1-5: can't set config #1, error -110
Thanks for all your help!
Looks like it's LPM related again. To confirm, try this: Disable LPM with this property "snps,usb2-gadget-lpm-disable" (Note that it's not the same as "snps,dis_enblslpm_quirk")
Yes, I confirm this helps.
Note: on startup I was in host mode, with gadget cable plugged. The first switch to gadget didn't work, all subsequent switches did work, as well as unplug/plug the cable.
Make sure that your testing kernel has this patch [1] 475e8be53d04 ("usb: dwc3: gadget: Check for disabled LPM quirk")
[1] https://urldefense.com/v3/__https://git.kernel.org/pub/scm/linux/kernel/git/...
The failure you saw was probably due the gadget function attempting to start a delayed status stage of the SET_CONFIGURATION request. By this time, the host already put the device in low power.
The START_TRANSFER command needs to be executed while the device is on "ON" state (or U0 if eSS). We shouldn't use dwc->link_state to check for link state because we only enable link state change interrupt for some controller versions.
Once you confirms disabling LPM works, try this fix:
diff --git a/drivers/usb/dwc3/gadget.c b/drivers/usb/dwc3/gadget.c index 6227641f2d31..06cdec79244e 100644 --- a/drivers/usb/dwc3/gadget.c +++ b/drivers/usb/dwc3/gadget.c @@ -309,10 +309,14 @@ int dwc3_send_gadget_ep_cmd(struct dwc3_ep *dep, unsigned int cmd, if (DWC3_DEPCMD_CMD(cmd) == DWC3_DEPCMD_STARTTRANSFER) { int needs_wakeup; + u8 link_state; - needs_wakeup = (dwc->link_state == DWC3_LINK_STATE_U1 || - dwc->link_state == DWC3_LINK_STATE_U2|| - dwc->link_state == DWC3_LINK_STATE_U3); + reg = dwc3_readl(dwc->regs, DWC3_DSTS); + link_state = DWC3_DSTS_USBLNKST(reg);
+ needs_wakeup = (link_state == DWC3_LINK_STATE_U1 || + link_state == DWC3_LINK_STATE_U2 || + link_state == DWC3_LINK_STATE_U3); if (unlikely(needs_wakeup)) { ret = __dwc3_gadget_wakeup(dwc); @@ -1989,6 +1993,8 @@ static int __dwc3_gadget_wakeup(struct dwc3 *dwc) case DWC3_LINK_STATE_RESET: case DWC3_LINK_STATE_RX_DET: /* in HS, means Early Suspend */ case DWC3_LINK_STATE_U3: /* in HS, means SUSPEND */ + case DWC3_LINK_STATE_U2: /* in HS, means Sleep (L1) */ + case DWC3_LINK_STATE_U1: case DWC3_LINK_STATE_RESUME: break; default:
Same (good) result as with "snps,usb2-gadget-lpm-disable". Including first switch from host->gadget not working.
Great! Not sure why the first switch is not working, but it seems like we were able to eliminate quite a few issues. If you have more dwc3 tracepoints, we can take a look further.
I traced but the file is empty. I captured the registers as well. The zip file is here:
https://github.com/andy-shev/linux/files/6346271/first-switch.zip
I found the gadget configuration script was not called, which normally gets called due to a udev rule:
ACTION=="add", KERNEL=="dwc3.0.auto", SUBSYSTEMS=="udc", ATTRS{state}=="not attached", RUN+="/usr/bin/conf-gadget.sh"
So I retried and see with ~# udevadm monitor:
# flipping the switch from host->gadget
KERNEL[51.824914] remove /devices/pci0000:00/0000:00:11.0/dwc3.0.auto/xhci-hcd.2.auto/usb1/1-1/1-1.1/1-1.1:1.0/net/enp0s17u1u1/queues/rx-0 (queues) KERNEL[51.825682] remove /devices/pci0000:00/0000:00:11.0/dwc3.0.auto/xhci-hcd.2.auto/usb1/1-1/1-1.1/1-1.1:1.0/net/enp0s17u1u1/queues/tx-0 (queues) KERNEL[51.826226] remove /devices/pci0000:00/0000:00:11.0/dwc3.0.auto/xhci-hcd.2.auto/usb1/1-1/1-1.1/1-1.1:1.0/net/enp0s17u1u1 (net) KERNEL[51.836041] unbind /devices/pci0000:00/0000:00:11.0/dwc3.0.auto/xhci-hcd.2.auto/usb1/1-1/1-1.1/mdio_bus/usb-001:003/usb-001:003:01 (mdio_bus) KERNEL[51.836709] remove /devices/pci0000:00/0000:00:11.0/dwc3.0.auto/xhci-hcd.2.auto/usb1/1-1/1-1.1/mdio_bus/usb-001:003/usb-001:003:01 (mdio_bus) KERNEL[51.837342] remove /devices/pci0000:00/0000:00:11.0/dwc3.0.auto/xhci-hcd.2.auto/usb1/1-1/1-1.1/mdio_bus/usb-001:003 (mdio_bus) KERNEL[51.837763] unbind /devices/pci0000:00/0000:00:11.0/dwc3.0.auto/xhci-hcd.2.auto/usb1/1-1/1-1.1/1-1.1:1.0 (usb) KERNEL[51.838116] remove /devices/pci0000:00/0000:00:11.0/dwc3.0.auto/xhci-hcd.2.auto/usb1/1-1/1-1.1/1-1.1:1.0 (usb) KERNEL[51.873712] unbind /devices/pci0000:00/0000:00:11.0/dwc3.0.auto/xhci-hcd.2.auto/usb1/1-1/1-1.1 (usb) KERNEL[51.874000] remove /devices/pci0000:00/0000:00:11.0/dwc3.0.auto/xhci-hcd.2.auto/usb1/1-1/1-1.1 (usb) KERNEL[51.874207] unbind /devices/pci0000:00/0000:00:11.0/dwc3.0.auto/xhci-hcd.2.auto/usb1/1-1/1-1:1.0 (usb) KERNEL[51.874431] remove /devices/pci0000:00/0000:00:11.0/dwc3.0.auto/xhci-hcd.2.auto/usb1/1-1/1-1:1.0 (usb) KERNEL[51.897175] unbind /devices/pci0000:00/0000:00:11.0/dwc3.0.auto/xhci-hcd.2.auto/usb1/1-1 (usb) KERNEL[51.897486] remove /devices/pci0000:00/0000:00:11.0/dwc3.0.auto/xhci-hcd.2.auto/usb1/1-1 (usb)
# stopped capture tracepoints here, then switch back to host
KERNEL[253.214406] add /devices/pci0000:00/0000:00:11.0/dwc3.0.auto/xhci-hcd.2.auto/usb1/1-1 (usb) KERNEL[253.263305] change /devices/pci0000:00/0000:00:11.0/dwc3.0.auto/xhci-hcd.2.auto/usb1/1-1 (usb) KERNEL[253.263687] add /devices/pci0000:00/0000:00:11.0/dwc3.0.auto/xhci-hcd.2.auto/usb1/1-1/1-1:1.0 (usb) KERNEL[253.328354] bind /devices/pci0000:00/0000:00:11.0/dwc3.0.auto/xhci-hcd.2.auto/usb1/1-1/1-1:1.0 (usb) KERNEL[253.328734] bind /devices/pci0000:00/0000:00:11.0/dwc3.0.auto/xhci-hcd.2.auto/usb1/1-1 (usb) KERNEL[253.699341] add /devices/pci0000:00/0000:00:11.0/dwc3.0.auto/xhci-hcd.2.auto/usb1/1-1/1-1.1 (usb) KERNEL[253.744911] change /devices/pci0000:00/0000:00:11.0/dwc3.0.auto/xhci-hcd.2.auto/usb1/1-1/1-1.1 (usb) KERNEL[253.745804] add /devices/pci0000:00/0000:00:11.0/dwc3.0.auto/xhci-hcd.2.auto/usb1/1-1/1-1.1/1-1.1:1.0 (usb) KERNEL[253.805307] add /devices/pci0000:00/0000:00:11.0/dwc3.0.auto/xhci-hcd.2.auto/usb1/1-1/1-1.1/mdio_bus/usb-001:005 (mdio_bus) KERNEL[253.812978] add /devices/pci0000:00/0000:00:11.0/dwc3.0.auto/xhci-hcd.2.auto/usb1/1-1/1-1.1/mdio_bus/usb-001:005/usb-001:005:01 (mdio_bus) KERNEL[253.814318] bind /devices/pci0000:00/0000:00:11.0/dwc3.0.auto/xhci-hcd.2.auto/usb1/1-1/1-1.1/mdio_bus/usb-001:005/usb-001:005:01 (mdio_bus) KERNEL[253.815386] add /devices/pci0000:00/0000:00:11.0/dwc3.0.auto/xhci-hcd.2.auto/usb1/1-1/1-1.1/1-1.1:1.0/net/eth0 (net) KERNEL[253.815552] add /devices/pci0000:00/0000:00:11.0/dwc3.0.auto/xhci-hcd.2.auto/usb1/1-1/1-1.1/1-1.1:1.0/net/eth0/queues/rx-0 (queues) KERNEL[253.815778] add /devices/pci0000:00/0000:00:11.0/dwc3.0.auto/xhci-hcd.2.auto/usb1/1-1/1-1.1/1-1.1:1.0/net/eth0/queues/tx-0 (queues) KERNEL[253.825279] bind /devices/pci0000:00/0000:00:11.0/dwc3.0.auto/xhci-hcd.2.auto/usb1/1-1/1-1.1/1-1.1:1.0 (usb) KERNEL[253.825667] bind /devices/pci0000:00/0000:00:11.0/dwc3.0.auto/xhci-hcd.2.auto/usb1/1-1/1-1.1 (usb)
# switch to gadget again
KERNEL[314.212144] remove /devices/pci0000:00/0000:00:11.0/dwc3.0.auto/xhci-hcd.2.auto/usb1/1-1/1-1.1/1-1.1:1.0/net/eth0/queues/rx-0 (queues) KERNEL[314.212473] remove /devices/pci0000:00/0000:00:11.0/dwc3.0.auto/xhci-hcd.2.auto/usb1/1-1/1-1.1/1-1.1:1.0/net/eth0/queues/tx-0 (queues) KERNEL[314.214691] remove /devices/pci0000:00/0000:00:11.0/dwc3.0.auto/xhci-hcd.2.auto/usb1/1-1/1-1.1/1-1.1:1.0/net/eth0 (net)
# extcon event didn't show the first time
KERNEL[314.238385] change /devices/pci0000:00/0000:00:13.0/INTC100E:00/mrfld_bcove_pwrsrc/extcon/extcon0 (extcon) KERNEL[314.238677] unbind /devices/pci0000:00/0000:00:11.0/dwc3.0.auto/xhci-hcd.2.auto/usb1/1-1/1-1.1/mdio_bus/usb-001:005/usb-001:005:01 (mdio_bus) KERNEL[314.238863] remove /devices/pci0000:00/0000:00:11.0/dwc3.0.auto/xhci-hcd.2.auto/usb1/1-1/1-1.1/mdio_bus/usb-001:005/usb-001:005:01 (mdio_bus) KERNEL[314.239015] remove /devices/pci0000:00/0000:00:11.0/dwc3.0.auto/xhci-hcd.2.auto/usb1/1-1/1-1.1/mdio_bus/usb-001:005 (mdio_bus) KERNEL[314.239205] unbind /devices/pci0000:00/0000:00:11.0/dwc3.0.auto/xhci-hcd.2.auto/usb1/1-1/1-1.1/1-1.1:1.0 (usb) KERNEL[314.239429] remove /devices/pci0000:00/0000:00:11.0/dwc3.0.auto/xhci-hcd.2.auto/usb1/1-1/1-1.1/1-1.1:1.0 (usb) KERNEL[314.239666] unbind /devices/pci0000:00/0000:00:11.0/dwc3.0.auto/xhci-hcd.2.auto/usb2/2-0:1.0 (usb) KERNEL[314.239933] remove /devices/pci0000:00/0000:00:11.0/dwc3.0.auto/xhci-hcd.2.auto/usb2/2-0:1.0 (usb) KERNEL[314.262713] unbind /devices/pci0000:00/0000:00:11.0/dwc3.0.auto/xhci-hcd.2.auto/usb2 (usb) KERNEL[314.263030] unbind /devices/pci0000:00/0000:00:11.0/dwc3.0.auto/xhci-hcd.2.auto/usb1/1-1/1-1.1 (usb) KERNEL[314.263298] remove /devices/pci0000:00/0000:00:11.0/dwc3.0.auto/xhci-hcd.2.auto/usb2 (usb) KERNEL[314.263569] remove /devices/pci0000:00/0000:00:11.0/dwc3.0.auto/xhci-hcd.2.auto/usb1/1-1/1-1.1 (usb) KERNEL[314.263815] unbind /devices/pci0000:00/0000:00:11.0/dwc3.0.auto/xhci-hcd.2.auto/usb1/1-1/1-1:1.0 (usb) KERNEL[314.264042] remove /devices/pci0000:00/0000:00:11.0/dwc3.0.auto/xhci-hcd.2.auto/usb1/1-1/1-1:1.0 (usb) KERNEL[314.264753] remove /devices/pci0000:00/0000:00:11.0/dwc3.0.auto/xhci-hcd.2.auto/usbmon/usbmon2 (usbmon) KERNEL[314.265019] unbind /devices/pci0000:00/0000:00:11.0/dwc3.0.auto/xhci-hcd.2.auto/usb1/1-1 (usb) KERNEL[314.265289] remove /devices/pci0000:00/0000:00:11.0/dwc3.0.auto/xhci-hcd.2.auto/usb1/1-1 (usb) KERNEL[314.288792] unbind /devices/pci0000:00/0000:00:11.0/dwc3.0.auto/xhci-hcd.2.auto/usb1/1-0:1.0 (usb) KERNEL[314.289057] remove /devices/pci0000:00/0000:00:11.0/dwc3.0.auto/xhci-hcd.2.auto/usb1/1-0:1.0 (usb) KERNEL[314.289327] unbind /devices/pci0000:00/0000:00:11.0/dwc3.0.auto/xhci-hcd.2.auto/usb1 (usb) KERNEL[314.289661] remove /devices/pci0000:00/0000:00:11.0/dwc3.0.auto/xhci-hcd.2.auto/usb1 (usb) KERNEL[314.647375] remove /devices/pci0000:00/0000:00:11.0/dwc3.0.auto/xhci-hcd.2.auto/usbmon/usbmon1 (usbmon) KERNEL[314.647816] unbind /devices/pci0000:00/0000:00:11.0/dwc3.0.auto/xhci-hcd.2.auto (platform) KERNEL[314.648143] remove /kernel/software_nodes/node1 (software_nodes) KERNEL[314.648672] remove /devices/pci0000:00/0000:00:11.0/dwc3.0.auto/xhci-hcd.2.auto (platform)
# here is the event we were waiting for
KERNEL[314.649158] add /devices/pci0000:00/0000:00:11.0/dwc3.0.auto/udc/dwc3.0.auto (udc)
# after this gadget devices appear normally
Maybe this issue is due to extcon missing the event?
After a 2 - 4 minutes the connection is dropped and reconnected.
Does this occur with LPM disabled also? We can review this issue further with more dwc3 tracepoints.
I captured connection dropping and reconnecting in this fairly long trace near the end of the file:
https://github.com/andy-shev/linux/files/6346323/lost-connection.zip
On the gadget end journal shows:
Apr 19 22:08:42 yuna systemd-networkd[507]: usb0: Lost carrier Apr 19 22:08:42 yuna systemd-journald[417]: Forwarding to syslog missed 1 messages. Apr 19 22:08:42 yuna systemd-timesyncd[469]: No network connectivity, watching for changes. Apr 19 22:08:42 yuna kernel: IPv6: ADDRCONF(NETDEV_CHANGE): usb0: link becomes ready Apr 19 22:08:42 yuna kernel[480]: [ 624.382929] IPv6: ADDRCONF(NETDEV_CHANGE): usb0: link becomes ready Apr 19 22:08:42 yuna systemd-networkd[507]: usb0: Gained carrier Apr 19 22:08:44 yuna systemd-networkd[507]: usb0: Gained IPv6LL Apr 19 22:08:44 yuna systemd-timesyncd[469]: Network configuration changed, trying to establish connection. Apr 19 22:08:57 yuna systemd-timesyncd[469]: Initial synchronization to time server 216.239.35.8:123 (time3.google.com).
So, drops and immediately reconnects.
From the look at the log here, it seems to be a reset from host (and an issue at the protocol level) unrelated to dwc3 driver or the controller. Hopefully and maybe we can get more clues from dwc3 tracepoints.
Thanks, Thinh
Ferry Toth wrote:
Hi
Op 19-04-2021 om 23:23 schreef Thinh Nguyen:
Ferry Toth wrote:
Hi
Op 19-04-2021 om 01:03 schreef Thinh Nguyen:
Ferry Toth wrote:
Hi
Op 17-04-2021 om 16:22 schreef Ferry Toth:
Hi
Op 17-04-2021 om 04:27 schreef Thinh Nguyen: > Ferry Toth wrote: >> Hi >> >> Op 16-04-2021 om 00:23 schreef Thinh Nguyen: >>> Thinh Nguyen wrote: >>>> From: Yu Chen chenyu56@huawei.com >>>> From: John Stultz john.stultz@linaro.org >>>> >>>> According to the programming guide, to switch mode for DRD >>>> controller, >>>> the driver needs to do the following. >>>> >>>> To switch from device to host: >>>> 1. Reset controller with GCTL.CoreSoftReset >>>> 2. Set GCTL.PrtCapDir(host mode) >>>> 3. Reset the host with USBCMD.HCRESET >>>> 4. Then follow up with the initializing host registers sequence >>>> >>>> To switch from host to device: >>>> 1. Reset controller with GCTL.CoreSoftReset >>>> 2. Set GCTL.PrtCapDir(device mode) >>>> 3. Reset the device with DCTL.CSftRst >>>> 4. Then follow up with the initializing registers sequence >>>> >>>> Currently we're missing step 1) to do GCTL.CoreSoftReset and step >>>> 3) of >>>> switching from host to device. John Stult reported a lockup issue >>>> seen >>>> with HiKey960 platform without these steps[1]. Similar issue is >>>> observed >>>> with Ferry's testing platform[2]. >>>> >>>> So, apply the required steps along with some fixes to Yu Chen's >>>> and John >>>> Stultz's version. The main fixes to their versions are the >>>> missing >>>> wait >>>> for clocks synchronization before clearing GCTL.CoreSoftReset and >>>> only >>>> apply DCTL.CSftRst when switching from host to device. >>>> >>>> [1] >>>> https://urldefense.com/v3/__https://lore.kernel.org/linux-usb/20210108015115... >>>> >>>> >>>> >>>> >>>> [2] >>>> https://urldefense.com/v3/__https://lore.kernel.org/linux-usb/0ba7a6ba-e6a7-... >>>> >>>> >>>> >>>> >>>> >>>> Cc: Andy Shevchenko andy.shevchenko@gmail.com >>>> Cc: Ferry Toth fntoth@gmail.com >>>> Cc: Wesley Cheng wcheng@codeaurora.org >>>> Cc: stable@vger.kernel.org >>>> Fixes: 41ce1456e1db ("usb: dwc3: core: make dwc3_set_mode() work >>>> properly") >>>> Signed-off-by: Yu Chen chenyu56@huawei.com >>>> Signed-off-by: John Stultz john.stultz@linaro.org >>>> Signed-off-by: Thinh Nguyen Thinh.Nguyen@synopsys.com >>>> --- >>>> Changes in v3: >>>> - Check if the desired mode is OTG, then keep the old flow >>>> - Remove condition for OTG support only since the device can >>>> still be >>>> configured DRD host/device mode only >>>> - Remove redundant hw_mode check since __dwc3_set_mode() only >>>> applies >>>> when >>>> hw_mode is DRD >>>> Changes in v2: >>>> - Initialize mutex per device and not as global mutex. >>>> - Add additional checks for DRD only mode >>>> >>>> drivers/usb/dwc3/core.c | 27 +++++++++++++++++++++++++++ >>>> drivers/usb/dwc3/core.h | 5 +++++ >>>> 2 files changed, 32 insertions(+) >>>> >>> Hi John, >>> >>> If possible, can you run a test with this version on your >>> platform? >>> >>> Thanks, >>> Thinh >>> >> I tested this on edison-arduino with this patch on top of usb-next >> (5.12-rc7 + "increase BESL baseline to 6" to prevent throttling"). >> >> On this platform there is a physical switch to switch roles. With >> this >> patch I find: >> >> - switch to host mode always works fine >> >> - switch to gadget mode I need to flip the switch 3x >> (gadget-host-gadget). >> >> An error message appears on the gadget side "dwc3 dwc3.0.auto: >> timed >> out >> waiting for SETUP phase" appears, but then the device connects >> to my >> PC, >> no throttling. >> >> - alternatively I can switch to gadget 1x and then unplug/replug >> the >> cable. >> >> No error message and connects fine. >> >> - if I flip the switch only once, on the PC side I get: >> >> kernel: usb 1-5: new high-speed USB device number 18 >> usingxhci_hcd >> kernel: usb 1-5: New USB device found, idVendor=1d6b, >> idProduct=0104, bcdDevice= 1.00 kernel: usb1-5: New USB device >> strings: Mfr=1, Product=2, SerialNumber=3kernel:usb 1-5: >> Product: >> USBArmory Gadget kernel: usb 1-5: Manufacturer:USBArmory >> kernel: >> usb 1-5: SerialNumber: 0123456789abcdef kernel:usb 1-5: can't >> set >> config #1, error -110 > The device failed at set_configuration() request and timed out. It > probably timed out from the status stage looking at the device err > print. > >> Then if I wait long enough on the gadget side I get: >> >> root@yuna:~# ifconfig >> >> usb0: flags=-28605<UP,BROADCAST,RUNNING,MULTICAST,DYNAMIC> mtu >> 1500 >> inet 169.254.119.239 netmask 255.255.0.0 broadcast >> 169.254.255.255 >> inet6 fe80::a8bb:ccff:fedd:eef1 prefixlen 64 scopeid >> 0x20<link> >> ether aa:bb:cc:dd:ee:f1 txqueuelen 1000 (Ethernet) RX packets >> 490424 >> bytes 735146578 (701.0 MiB) RX errors 0 dropped191 overruns 0 >> frame >> 0 TX packets 35279 bytes 2532746 (2.4 MiB) TX errors 0 >> dropped 0 >> overruns 0 carrier 0 collisions 0 >> >> (correct would be: inet 10.42.0.221 netmask 255.255.255.0 broadcast >> 10.42.0.255) >> >> So much improved now, but it seems I am still missing something on >> plug. >> > That's great! We can look at it further. Can you capture the > tracepoints > of the issue. Also, can you try with mass_storage gadget to see > if the > result is the same? I have already gser, eem, mass_storage and uac2 combo. When eem fails, the mass_storage and uac2 don't appear (on KDE you get all kind of popups when they appear).
So either all works, or all fails.
I'll trace this later today.
Trace capturing switch from host-> gadget here https://urldefense.com/v3/__https://github.com/andy-shev/linux/files/6329600...
(Issue history: https://urldefense.com/v3/__https://github.com/andy-shev/linux/issues/31__%3...
)
On the PC side this resulted to:
apr 17 18:17:44 delfion kernel: usb 1-5: new high-speed USB device number 12 using xhci_hcd apr 17 18:17:44 delfion kernel: usb 1-5: New USB device found, idVendor=1d6b, idProduct=0104, bcdDevice= 1.00 apr 17 18:17:44 delfion kernel: usb 1-5: New USB device strings: Mfr=1, Product=2, SerialNumber=3 apr 17 18:17:44 delfion kernel: usb 1-5: Product: USBArmory Gadget apr 17 18:17:44 delfion kernel: usb 1-5: Manufacturer: USBArmory apr 17 18:17:44 delfion kernel: usb 1-5: SerialNumber: 0123456789abcdef apr 17 18:17:49 delfion kernel: usb 1-5: can't set config #1, error -110
Thanks for all your help!
Looks like it's LPM related again. To confirm, try this: Disable LPM with this property "snps,usb2-gadget-lpm-disable" (Note that it's not the same as "snps,dis_enblslpm_quirk")
Yes, I confirm this helps.
Note: on startup I was in host mode, with gadget cable plugged. The first switch to gadget didn't work, all subsequent switches did work, as well as unplug/plug the cable.
Make sure that your testing kernel has this patch [1] 475e8be53d04 ("usb: dwc3: gadget: Check for disabled LPM quirk")
[1] https://urldefense.com/v3/__https://git.kernel.org/pub/scm/linux/kernel/git/...
The failure you saw was probably due the gadget function attempting to start a delayed status stage of the SET_CONFIGURATION request. By this time, the host already put the device in low power.
The START_TRANSFER command needs to be executed while the device is on "ON" state (or U0 if eSS). We shouldn't use dwc->link_state to check for link state because we only enable link state change interrupt for some controller versions.
Once you confirms disabling LPM works, try this fix:
diff --git a/drivers/usb/dwc3/gadget.c b/drivers/usb/dwc3/gadget.c index 6227641f2d31..06cdec79244e 100644 --- a/drivers/usb/dwc3/gadget.c +++ b/drivers/usb/dwc3/gadget.c @@ -309,10 +309,14 @@ int dwc3_send_gadget_ep_cmd(struct dwc3_ep *dep, unsigned int cmd, if (DWC3_DEPCMD_CMD(cmd) == DWC3_DEPCMD_STARTTRANSFER) { int needs_wakeup; + u8 link_state; - needs_wakeup = (dwc->link_state == DWC3_LINK_STATE_U1 || - dwc->link_state == DWC3_LINK_STATE_U2|| - dwc->link_state == DWC3_LINK_STATE_U3); + reg = dwc3_readl(dwc->regs, DWC3_DSTS); + link_state = DWC3_DSTS_USBLNKST(reg);
+ needs_wakeup = (link_state == DWC3_LINK_STATE_U1 || + link_state == DWC3_LINK_STATE_U2 || + link_state == DWC3_LINK_STATE_U3); if (unlikely(needs_wakeup)) { ret = __dwc3_gadget_wakeup(dwc); @@ -1989,6 +1993,8 @@ static int __dwc3_gadget_wakeup(struct dwc3 *dwc) case DWC3_LINK_STATE_RESET: case DWC3_LINK_STATE_RX_DET: /* in HS, means Early Suspend */ case DWC3_LINK_STATE_U3: /* in HS, means SUSPEND */ + case DWC3_LINK_STATE_U2: /* in HS, means Sleep (L1) */ + case DWC3_LINK_STATE_U1: case DWC3_LINK_STATE_RESUME: break; default:
Same (good) result as with "snps,usb2-gadget-lpm-disable". Including first switch from host->gadget not working.
Great! Not sure why the first switch is not working, but it seems like we were able to eliminate quite a few issues. If you have more dwc3 tracepoints, we can take a look further.
I traced but the file is empty. I captured the registers as well. The zip file is here:
https://urldefense.com/v3/__https://github.com/andy-shev/linux/files/6346271...
I found the gadget configuration script was not called, which normally gets called due to a udev rule:
ACTION=="add", KERNEL=="dwc3.0.auto", SUBSYSTEMS=="udc", ATTRS{state}=="not attached", RUN+="/usr/bin/conf-gadget.sh"
So I retried and see with ~# udevadm monitor:
# flipping the switch from host->gadget
KERNEL[51.824914] remove /devices/pci0000:00/0000:00:11.0/dwc3.0.auto/xhci-hcd.2.auto/usb1/1-1/1-1.1/1-1.1:1.0/net/enp0s17u1u1/queues/rx-0 (queues) KERNEL[51.825682] remove /devices/pci0000:00/0000:00:11.0/dwc3.0.auto/xhci-hcd.2.auto/usb1/1-1/1-1.1/1-1.1:1.0/net/enp0s17u1u1/queues/tx-0 (queues) KERNEL[51.826226] remove /devices/pci0000:00/0000:00:11.0/dwc3.0.auto/xhci-hcd.2.auto/usb1/1-1/1-1.1/1-1.1:1.0/net/enp0s17u1u1 (net) KERNEL[51.836041] unbind /devices/pci0000:00/0000:00:11.0/dwc3.0.auto/xhci-hcd.2.auto/usb1/1-1/1-1.1/mdio_bus/usb-001:003/usb-001:003:01 (mdio_bus) KERNEL[51.836709] remove /devices/pci0000:00/0000:00:11.0/dwc3.0.auto/xhci-hcd.2.auto/usb1/1-1/1-1.1/mdio_bus/usb-001:003/usb-001:003:01 (mdio_bus) KERNEL[51.837342] remove /devices/pci0000:00/0000:00:11.0/dwc3.0.auto/xhci-hcd.2.auto/usb1/1-1/1-1.1/mdio_bus/usb-001:003 (mdio_bus) KERNEL[51.837763] unbind /devices/pci0000:00/0000:00:11.0/dwc3.0.auto/xhci-hcd.2.auto/usb1/1-1/1-1.1/1-1.1:1.0 (usb) KERNEL[51.838116] remove /devices/pci0000:00/0000:00:11.0/dwc3.0.auto/xhci-hcd.2.auto/usb1/1-1/1-1.1/1-1.1:1.0 (usb) KERNEL[51.873712] unbind /devices/pci0000:00/0000:00:11.0/dwc3.0.auto/xhci-hcd.2.auto/usb1/1-1/1-1.1 (usb) KERNEL[51.874000] remove /devices/pci0000:00/0000:00:11.0/dwc3.0.auto/xhci-hcd.2.auto/usb1/1-1/1-1.1 (usb) KERNEL[51.874207] unbind /devices/pci0000:00/0000:00:11.0/dwc3.0.auto/xhci-hcd.2.auto/usb1/1-1/1-1:1.0 (usb) KERNEL[51.874431] remove /devices/pci0000:00/0000:00:11.0/dwc3.0.auto/xhci-hcd.2.auto/usb1/1-1/1-1:1.0 (usb) KERNEL[51.897175] unbind /devices/pci0000:00/0000:00:11.0/dwc3.0.auto/xhci-hcd.2.auto/usb1/1-1 (usb) KERNEL[51.897486] remove /devices/pci0000:00/0000:00:11.0/dwc3.0.auto/xhci-hcd.2.auto/usb1/1-1 (usb)
# stopped capture tracepoints here, then switch back to host
KERNEL[253.214406] add /devices/pci0000:00/0000:00:11.0/dwc3.0.auto/xhci-hcd.2.auto/usb1/1-1 (usb) KERNEL[253.263305] change /devices/pci0000:00/0000:00:11.0/dwc3.0.auto/xhci-hcd.2.auto/usb1/1-1 (usb) KERNEL[253.263687] add /devices/pci0000:00/0000:00:11.0/dwc3.0.auto/xhci-hcd.2.auto/usb1/1-1/1-1:1.0 (usb) KERNEL[253.328354] bind /devices/pci0000:00/0000:00:11.0/dwc3.0.auto/xhci-hcd.2.auto/usb1/1-1/1-1:1.0 (usb) KERNEL[253.328734] bind /devices/pci0000:00/0000:00:11.0/dwc3.0.auto/xhci-hcd.2.auto/usb1/1-1 (usb) KERNEL[253.699341] add /devices/pci0000:00/0000:00:11.0/dwc3.0.auto/xhci-hcd.2.auto/usb1/1-1/1-1.1 (usb) KERNEL[253.744911] change /devices/pci0000:00/0000:00:11.0/dwc3.0.auto/xhci-hcd.2.auto/usb1/1-1/1-1.1 (usb) KERNEL[253.745804] add /devices/pci0000:00/0000:00:11.0/dwc3.0.auto/xhci-hcd.2.auto/usb1/1-1/1-1.1/1-1.1:1.0 (usb) KERNEL[253.805307] add /devices/pci0000:00/0000:00:11.0/dwc3.0.auto/xhci-hcd.2.auto/usb1/1-1/1-1.1/mdio_bus/usb-001:005 (mdio_bus) KERNEL[253.812978] add /devices/pci0000:00/0000:00:11.0/dwc3.0.auto/xhci-hcd.2.auto/usb1/1-1/1-1.1/mdio_bus/usb-001:005/usb-001:005:01 (mdio_bus) KERNEL[253.814318] bind /devices/pci0000:00/0000:00:11.0/dwc3.0.auto/xhci-hcd.2.auto/usb1/1-1/1-1.1/mdio_bus/usb-001:005/usb-001:005:01 (mdio_bus) KERNEL[253.815386] add /devices/pci0000:00/0000:00:11.0/dwc3.0.auto/xhci-hcd.2.auto/usb1/1-1/1-1.1/1-1.1:1.0/net/eth0 (net) KERNEL[253.815552] add /devices/pci0000:00/0000:00:11.0/dwc3.0.auto/xhci-hcd.2.auto/usb1/1-1/1-1.1/1-1.1:1.0/net/eth0/queues/rx-0 (queues) KERNEL[253.815778] add /devices/pci0000:00/0000:00:11.0/dwc3.0.auto/xhci-hcd.2.auto/usb1/1-1/1-1.1/1-1.1:1.0/net/eth0/queues/tx-0 (queues) KERNEL[253.825279] bind /devices/pci0000:00/0000:00:11.0/dwc3.0.auto/xhci-hcd.2.auto/usb1/1-1/1-1.1/1-1.1:1.0 (usb) KERNEL[253.825667] bind /devices/pci0000:00/0000:00:11.0/dwc3.0.auto/xhci-hcd.2.auto/usb1/1-1/1-1.1 (usb)
# switch to gadget again
KERNEL[314.212144] remove /devices/pci0000:00/0000:00:11.0/dwc3.0.auto/xhci-hcd.2.auto/usb1/1-1/1-1.1/1-1.1:1.0/net/eth0/queues/rx-0 (queues) KERNEL[314.212473] remove /devices/pci0000:00/0000:00:11.0/dwc3.0.auto/xhci-hcd.2.auto/usb1/1-1/1-1.1/1-1.1:1.0/net/eth0/queues/tx-0 (queues) KERNEL[314.214691] remove /devices/pci0000:00/0000:00:11.0/dwc3.0.auto/xhci-hcd.2.auto/usb1/1-1/1-1.1/1-1.1:1.0/net/eth0 (net)
# extcon event didn't show the first time
KERNEL[314.238385] change /devices/pci0000:00/0000:00:13.0/INTC100E:00/mrfld_bcove_pwrsrc/extcon/extcon0 (extcon) KERNEL[314.238677] unbind /devices/pci0000:00/0000:00:11.0/dwc3.0.auto/xhci-hcd.2.auto/usb1/1-1/1-1.1/mdio_bus/usb-001:005/usb-001:005:01 (mdio_bus) KERNEL[314.238863] remove /devices/pci0000:00/0000:00:11.0/dwc3.0.auto/xhci-hcd.2.auto/usb1/1-1/1-1.1/mdio_bus/usb-001:005/usb-001:005:01 (mdio_bus) KERNEL[314.239015] remove /devices/pci0000:00/0000:00:11.0/dwc3.0.auto/xhci-hcd.2.auto/usb1/1-1/1-1.1/mdio_bus/usb-001:005 (mdio_bus) KERNEL[314.239205] unbind /devices/pci0000:00/0000:00:11.0/dwc3.0.auto/xhci-hcd.2.auto/usb1/1-1/1-1.1/1-1.1:1.0 (usb) KERNEL[314.239429] remove /devices/pci0000:00/0000:00:11.0/dwc3.0.auto/xhci-hcd.2.auto/usb1/1-1/1-1.1/1-1.1:1.0 (usb) KERNEL[314.239666] unbind /devices/pci0000:00/0000:00:11.0/dwc3.0.auto/xhci-hcd.2.auto/usb2/2-0:1.0 (usb)
KERNEL[314.239933] remove /devices/pci0000:00/0000:00:11.0/dwc3.0.auto/xhci-hcd.2.auto/usb2/2-0:1.0 (usb)
KERNEL[314.262713] unbind /devices/pci0000:00/0000:00:11.0/dwc3.0.auto/xhci-hcd.2.auto/usb2 (usb) KERNEL[314.263030] unbind /devices/pci0000:00/0000:00:11.0/dwc3.0.auto/xhci-hcd.2.auto/usb1/1-1/1-1.1 (usb) KERNEL[314.263298] remove /devices/pci0000:00/0000:00:11.0/dwc3.0.auto/xhci-hcd.2.auto/usb2 (usb) KERNEL[314.263569] remove /devices/pci0000:00/0000:00:11.0/dwc3.0.auto/xhci-hcd.2.auto/usb1/1-1/1-1.1 (usb) KERNEL[314.263815] unbind /devices/pci0000:00/0000:00:11.0/dwc3.0.auto/xhci-hcd.2.auto/usb1/1-1/1-1:1.0 (usb) KERNEL[314.264042] remove /devices/pci0000:00/0000:00:11.0/dwc3.0.auto/xhci-hcd.2.auto/usb1/1-1/1-1:1.0 (usb) KERNEL[314.264753] remove /devices/pci0000:00/0000:00:11.0/dwc3.0.auto/xhci-hcd.2.auto/usbmon/usbmon2 (usbmon) KERNEL[314.265019] unbind /devices/pci0000:00/0000:00:11.0/dwc3.0.auto/xhci-hcd.2.auto/usb1/1-1 (usb) KERNEL[314.265289] remove /devices/pci0000:00/0000:00:11.0/dwc3.0.auto/xhci-hcd.2.auto/usb1/1-1 (usb) KERNEL[314.288792] unbind /devices/pci0000:00/0000:00:11.0/dwc3.0.auto/xhci-hcd.2.auto/usb1/1-0:1.0 (usb)
KERNEL[314.289057] remove /devices/pci0000:00/0000:00:11.0/dwc3.0.auto/xhci-hcd.2.auto/usb1/1-0:1.0 (usb)
KERNEL[314.289327] unbind /devices/pci0000:00/0000:00:11.0/dwc3.0.auto/xhci-hcd.2.auto/usb1 (usb) KERNEL[314.289661] remove /devices/pci0000:00/0000:00:11.0/dwc3.0.auto/xhci-hcd.2.auto/usb1 (usb) KERNEL[314.647375] remove /devices/pci0000:00/0000:00:11.0/dwc3.0.auto/xhci-hcd.2.auto/usbmon/usbmon1 (usbmon) KERNEL[314.647816] unbind /devices/pci0000:00/0000:00:11.0/dwc3.0.auto/xhci-hcd.2.auto (platform) KERNEL[314.648143] remove /kernel/software_nodes/node1 (software_nodes) KERNEL[314.648672] remove /devices/pci0000:00/0000:00:11.0/dwc3.0.auto/xhci-hcd.2.auto (platform)
# here is the event we were waiting for
KERNEL[314.649158] add /devices/pci0000:00/0000:00:11.0/dwc3.0.auto/udc/dwc3.0.auto (udc)
# after this gadget devices appear normally
Maybe this issue is due to extcon missing the event?
From the info here, it doesn't look like the host platform device was removed on the first switch. Also, as you pointed it out, the extcon event was not shown on the first switch either. Without a notification to switch mode, the dwc3 driver won't do anything. You need to check why that's the case as I can't help much here.
After a 2 - 4 minutes the connection is dropped and reconnected.
Does this occur with LPM disabled also? We can review this issue further with more dwc3 tracepoints.
I captured connection dropping and reconnecting in this fairly long trace near the end of the file:
https://urldefense.com/v3/__https://github.com/andy-shev/linux/files/6346323...
Nothing obvious stands out as a problem from the dwc3 driver or the controller. I see a (port) reset after 30 seconds of inactivity, which is a typical timeout and recovery mechanism in the upperlayer from host.
* Is this a new issue or was it always there? * Does turning off LPM help? * Are the other gadget functions still work during the 30 seconds inactivity?
On the gadget end journal shows:
Apr 19 22:08:42 yuna systemd-networkd[507]: usb0: Lost carrier Apr 19 22:08:42 yuna systemd-journald[417]: Forwarding to syslog missed 1 messages. Apr 19 22:08:42 yuna systemd-timesyncd[469]: No network connectivity, watching for changes. Apr 19 22:08:42 yuna kernel: IPv6: ADDRCONF(NETDEV_CHANGE): usb0: link becomes ready Apr 19 22:08:42 yuna kernel[480]: [ 624.382929] IPv6: ADDRCONF(NETDEV_CHANGE): usb0: link becomes ready Apr 19 22:08:42 yuna systemd-networkd[507]: usb0: Gained carrier Apr 19 22:08:44 yuna systemd-networkd[507]: usb0: Gained IPv6LL Apr 19 22:08:44 yuna systemd-timesyncd[469]: Network configuration changed, trying to establish connection. Apr 19 22:08:57 yuna systemd-timesyncd[469]: Initial synchronization to time server 216.239.35.8:123 (time3.google.com).
So, drops and immediately reconnects.
From the look at the log here, it seems to be a reset from host (and an issue at the protocol level) unrelated to dwc3 driver or the controller. Hopefully and maybe we can get more clues from dwc3 tracepoints.
BR, Thinh
Hi
Op 21-04-2021 om 21:01 schreef Thinh Nguyen:
Ferry Toth wrote:
Hi
Op 19-04-2021 om 23:23 schreef Thinh Nguyen:
Ferry Toth wrote:
Hi
Op 19-04-2021 om 01:03 schreef Thinh Nguyen:
Ferry Toth wrote:
Hi
Op 17-04-2021 om 16:22 schreef Ferry Toth: > Hi > > Op 17-04-2021 om 04:27 schreef Thinh Nguyen: >> Ferry Toth wrote: >>> Hi >>> >>> Op 16-04-2021 om 00:23 schreef Thinh Nguyen: >>>> Thinh Nguyen wrote: >>>>> From: Yu Chen chenyu56@huawei.com >>>>> From: John Stultz john.stultz@linaro.org >>>>> >>>>> According to the programming guide, to switch mode for DRD >>>>> controller, >>>>> the driver needs to do the following. >>>>> >>>>> To switch from device to host: >>>>> 1. Reset controller with GCTL.CoreSoftReset >>>>> 2. Set GCTL.PrtCapDir(host mode) >>>>> 3. Reset the host with USBCMD.HCRESET >>>>> 4. Then follow up with the initializing host registers sequence >>>>> >>>>> To switch from host to device: >>>>> 1. Reset controller with GCTL.CoreSoftReset >>>>> 2. Set GCTL.PrtCapDir(device mode) >>>>> 3. Reset the device with DCTL.CSftRst >>>>> 4. Then follow up with the initializing registers sequence >>>>> >>>>> Currently we're missing step 1) to do GCTL.CoreSoftReset and step >>>>> 3) of >>>>> switching from host to device. John Stult reported a lockup issue >>>>> seen >>>>> with HiKey960 platform without these steps[1]. Similar issue is >>>>> observed >>>>> with Ferry's testing platform[2]. >>>>> >>>>> So, apply the required steps along with some fixes to Yu Chen's >>>>> and John >>>>> Stultz's version. The main fixes to their versions are the >>>>> missing >>>>> wait >>>>> for clocks synchronization before clearing GCTL.CoreSoftReset and >>>>> only >>>>> apply DCTL.CSftRst when switching from host to device. >>>>> >>>>> [1] >>>>> https://urldefense.com/v3/__https://lore.kernel.org/linux-usb/20210108015115... >>>>> >>>>> >>>>> >>>>> >>>>> [2] >>>>> https://urldefense.com/v3/__https://lore.kernel.org/linux-usb/0ba7a6ba-e6a7-... >>>>> >>>>> >>>>> >>>>> >>>>> >>>>> Cc: Andy Shevchenko andy.shevchenko@gmail.com >>>>> Cc: Ferry Toth fntoth@gmail.com >>>>> Cc: Wesley Cheng wcheng@codeaurora.org >>>>> Cc: stable@vger.kernel.org >>>>> Fixes: 41ce1456e1db ("usb: dwc3: core: make dwc3_set_mode() work >>>>> properly") >>>>> Signed-off-by: Yu Chen chenyu56@huawei.com >>>>> Signed-off-by: John Stultz john.stultz@linaro.org >>>>> Signed-off-by: Thinh Nguyen Thinh.Nguyen@synopsys.com >>>>> --- >>>>> Changes in v3: >>>>> - Check if the desired mode is OTG, then keep the old flow >>>>> - Remove condition for OTG support only since the device can >>>>> still be >>>>> configured DRD host/device mode only >>>>> - Remove redundant hw_mode check since __dwc3_set_mode() only >>>>> applies >>>>> when >>>>> hw_mode is DRD >>>>> Changes in v2: >>>>> - Initialize mutex per device and not as global mutex. >>>>> - Add additional checks for DRD only mode >>>>> >>>>> drivers/usb/dwc3/core.c | 27 +++++++++++++++++++++++++++ >>>>> drivers/usb/dwc3/core.h | 5 +++++ >>>>> 2 files changed, 32 insertions(+) >>>>> >>>> Hi John, >>>> >>>> If possible, can you run a test with this version on your >>>> platform? >>>> >>>> Thanks, >>>> Thinh >>>> >>> I tested this on edison-arduino with this patch on top of usb-next >>> (5.12-rc7 + "increase BESL baseline to 6" to prevent throttling"). >>> >>> On this platform there is a physical switch to switch roles. With >>> this >>> patch I find: >>> >>> - switch to host mode always works fine >>> >>> - switch to gadget mode I need to flip the switch 3x >>> (gadget-host-gadget). >>> >>> An error message appears on the gadget side "dwc3 dwc3.0.auto: >>> timed >>> out >>> waiting for SETUP phase" appears, but then the device connects >>> to my >>> PC, >>> no throttling. >>> >>> - alternatively I can switch to gadget 1x and then unplug/replug >>> the >>> cable. >>> >>> No error message and connects fine. >>> >>> - if I flip the switch only once, on the PC side I get: >>> >>> kernel: usb 1-5: new high-speed USB device number 18 >>> usingxhci_hcd >>> kernel: usb 1-5: New USB device found, idVendor=1d6b, >>> idProduct=0104, bcdDevice= 1.00 kernel: usb1-5: New USB device >>> strings: Mfr=1, Product=2, SerialNumber=3kernel:usb 1-5: >>> Product: >>> USBArmory Gadget kernel: usb 1-5: Manufacturer:USBArmory >>> kernel: >>> usb 1-5: SerialNumber: 0123456789abcdef kernel:usb 1-5: can't >>> set >>> config #1, error -110 >> The device failed at set_configuration() request and timed out. It >> probably timed out from the status stage looking at the device err >> print. >> >>> Then if I wait long enough on the gadget side I get: >>> >>> root@yuna:~# ifconfig >>> >>> usb0: flags=-28605<UP,BROADCAST,RUNNING,MULTICAST,DYNAMIC> mtu >>> 1500 >>> inet 169.254.119.239 netmask 255.255.0.0 broadcast >>> 169.254.255.255 >>> inet6 fe80::a8bb:ccff:fedd:eef1 prefixlen 64 scopeid >>> 0x20<link> >>> ether aa:bb:cc:dd:ee:f1 txqueuelen 1000 (Ethernet) RX packets >>> 490424 >>> bytes 735146578 (701.0 MiB) RX errors 0 dropped191 overruns 0 >>> frame >>> 0 TX packets 35279 bytes 2532746 (2.4 MiB) TX errors 0 >>> dropped 0 >>> overruns 0 carrier 0 collisions 0 >>> >>> (correct would be: inet 10.42.0.221 netmask 255.255.255.0 broadcast >>> 10.42.0.255) >>> >>> So much improved now, but it seems I am still missing something on >>> plug. >>> >> That's great! We can look at it further. Can you capture the >> tracepoints >> of the issue. Also, can you try with mass_storage gadget to see >> if the >> result is the same? > I have already gser, eem, mass_storage and uac2 combo. When eem > fails, > the mass_storage and uac2 don't appear (on KDE you get all kind of > popups when they appear). > > So either all works, or all fails. > > I'll trace this later today. Trace capturing switch from host-> gadget here https://urldefense.com/v3/__https://github.com/andy-shev/linux/files/6329600...
(Issue history: https://urldefense.com/v3/__https://github.com/andy-shev/linux/issues/31__%3...
)
On the PC side this resulted to:
apr 17 18:17:44 delfion kernel: usb 1-5: new high-speed USB device number 12 using xhci_hcd apr 17 18:17:44 delfion kernel: usb 1-5: New USB device found, idVendor=1d6b, idProduct=0104, bcdDevice= 1.00 apr 17 18:17:44 delfion kernel: usb 1-5: New USB device strings: Mfr=1, Product=2, SerialNumber=3 apr 17 18:17:44 delfion kernel: usb 1-5: Product: USBArmory Gadget apr 17 18:17:44 delfion kernel: usb 1-5: Manufacturer: USBArmory apr 17 18:17:44 delfion kernel: usb 1-5: SerialNumber: 0123456789abcdef apr 17 18:17:49 delfion kernel: usb 1-5: can't set config #1, error -110
Thanks for all your help!
Looks like it's LPM related again. To confirm, try this: Disable LPM with this property "snps,usb2-gadget-lpm-disable" (Note that it's not the same as "snps,dis_enblslpm_quirk")
Yes, I confirm this helps.
Note: on startup I was in host mode, with gadget cable plugged. The first switch to gadget didn't work, all subsequent switches did work, as well as unplug/plug the cable.
Make sure that your testing kernel has this patch [1] 475e8be53d04 ("usb: dwc3: gadget: Check for disabled LPM quirk")
[1] https://urldefense.com/v3/__https://git.kernel.org/pub/scm/linux/kernel/git/...
The failure you saw was probably due the gadget function attempting to start a delayed status stage of the SET_CONFIGURATION request. By this time, the host already put the device in low power.
The START_TRANSFER command needs to be executed while the device is on "ON" state (or U0 if eSS). We shouldn't use dwc->link_state to check for link state because we only enable link state change interrupt for some controller versions.
Once you confirms disabling LPM works, try this fix:
diff --git a/drivers/usb/dwc3/gadget.c b/drivers/usb/dwc3/gadget.c index 6227641f2d31..06cdec79244e 100644 --- a/drivers/usb/dwc3/gadget.c +++ b/drivers/usb/dwc3/gadget.c @@ -309,10 +309,14 @@ int dwc3_send_gadget_ep_cmd(struct dwc3_ep *dep, unsigned int cmd, if (DWC3_DEPCMD_CMD(cmd) == DWC3_DEPCMD_STARTTRANSFER) { int needs_wakeup; + u8 link_state; - needs_wakeup = (dwc->link_state == DWC3_LINK_STATE_U1 || - dwc->link_state == DWC3_LINK_STATE_U2|| - dwc->link_state == DWC3_LINK_STATE_U3); + reg = dwc3_readl(dwc->regs, DWC3_DSTS); + link_state = DWC3_DSTS_USBLNKST(reg);
+ needs_wakeup = (link_state == DWC3_LINK_STATE_U1 || + link_state == DWC3_LINK_STATE_U2 || + link_state == DWC3_LINK_STATE_U3); if (unlikely(needs_wakeup)) { ret = __dwc3_gadget_wakeup(dwc); @@ -1989,6 +1993,8 @@ static int __dwc3_gadget_wakeup(struct dwc3 *dwc) case DWC3_LINK_STATE_RESET: case DWC3_LINK_STATE_RX_DET: /* in HS, means Early Suspend */ case DWC3_LINK_STATE_U3: /* in HS, means SUSPEND */ + case DWC3_LINK_STATE_U2: /* in HS, means Sleep (L1) */ + case DWC3_LINK_STATE_U1: case DWC3_LINK_STATE_RESUME: break; default:
Same (good) result as with "snps,usb2-gadget-lpm-disable". Including first switch from host->gadget not working.
Great! Not sure why the first switch is not working, but it seems like we were able to eliminate quite a few issues. If you have more dwc3 tracepoints, we can take a look further.
I traced but the file is empty. I captured the registers as well. The zip file is here:
https://urldefense.com/v3/__https://github.com/andy-shev/linux/files/6346271...
I found the gadget configuration script was not called, which normally gets called due to a udev rule:
ACTION=="add", KERNEL=="dwc3.0.auto", SUBSYSTEMS=="udc", ATTRS{state}=="not attached", RUN+="/usr/bin/conf-gadget.sh"
So I retried and see with ~# udevadm monitor:
# flipping the switch from host->gadget
KERNEL[51.824914] remove /devices/pci0000:00/0000:00:11.0/dwc3.0.auto/xhci-hcd.2.auto/usb1/1-1/1-1.1/1-1.1:1.0/net/enp0s17u1u1/queues/rx-0 (queues) KERNEL[51.825682] remove /devices/pci0000:00/0000:00:11.0/dwc3.0.auto/xhci-hcd.2.auto/usb1/1-1/1-1.1/1-1.1:1.0/net/enp0s17u1u1/queues/tx-0 (queues) KERNEL[51.826226] remove /devices/pci0000:00/0000:00:11.0/dwc3.0.auto/xhci-hcd.2.auto/usb1/1-1/1-1.1/1-1.1:1.0/net/enp0s17u1u1 (net) KERNEL[51.836041] unbind /devices/pci0000:00/0000:00:11.0/dwc3.0.auto/xhci-hcd.2.auto/usb1/1-1/1-1.1/mdio_bus/usb-001:003/usb-001:003:01 (mdio_bus) KERNEL[51.836709] remove /devices/pci0000:00/0000:00:11.0/dwc3.0.auto/xhci-hcd.2.auto/usb1/1-1/1-1.1/mdio_bus/usb-001:003/usb-001:003:01 (mdio_bus) KERNEL[51.837342] remove /devices/pci0000:00/0000:00:11.0/dwc3.0.auto/xhci-hcd.2.auto/usb1/1-1/1-1.1/mdio_bus/usb-001:003 (mdio_bus) KERNEL[51.837763] unbind /devices/pci0000:00/0000:00:11.0/dwc3.0.auto/xhci-hcd.2.auto/usb1/1-1/1-1.1/1-1.1:1.0 (usb) KERNEL[51.838116] remove /devices/pci0000:00/0000:00:11.0/dwc3.0.auto/xhci-hcd.2.auto/usb1/1-1/1-1.1/1-1.1:1.0 (usb) KERNEL[51.873712] unbind /devices/pci0000:00/0000:00:11.0/dwc3.0.auto/xhci-hcd.2.auto/usb1/1-1/1-1.1 (usb) KERNEL[51.874000] remove /devices/pci0000:00/0000:00:11.0/dwc3.0.auto/xhci-hcd.2.auto/usb1/1-1/1-1.1 (usb) KERNEL[51.874207] unbind /devices/pci0000:00/0000:00:11.0/dwc3.0.auto/xhci-hcd.2.auto/usb1/1-1/1-1:1.0 (usb) KERNEL[51.874431] remove /devices/pci0000:00/0000:00:11.0/dwc3.0.auto/xhci-hcd.2.auto/usb1/1-1/1-1:1.0 (usb) KERNEL[51.897175] unbind /devices/pci0000:00/0000:00:11.0/dwc3.0.auto/xhci-hcd.2.auto/usb1/1-1 (usb) KERNEL[51.897486] remove /devices/pci0000:00/0000:00:11.0/dwc3.0.auto/xhci-hcd.2.auto/usb1/1-1 (usb)
# stopped capture tracepoints here, then switch back to host
KERNEL[253.214406] add /devices/pci0000:00/0000:00:11.0/dwc3.0.auto/xhci-hcd.2.auto/usb1/1-1 (usb) KERNEL[253.263305] change /devices/pci0000:00/0000:00:11.0/dwc3.0.auto/xhci-hcd.2.auto/usb1/1-1 (usb) KERNEL[253.263687] add /devices/pci0000:00/0000:00:11.0/dwc3.0.auto/xhci-hcd.2.auto/usb1/1-1/1-1:1.0 (usb) KERNEL[253.328354] bind /devices/pci0000:00/0000:00:11.0/dwc3.0.auto/xhci-hcd.2.auto/usb1/1-1/1-1:1.0 (usb) KERNEL[253.328734] bind /devices/pci0000:00/0000:00:11.0/dwc3.0.auto/xhci-hcd.2.auto/usb1/1-1 (usb) KERNEL[253.699341] add /devices/pci0000:00/0000:00:11.0/dwc3.0.auto/xhci-hcd.2.auto/usb1/1-1/1-1.1 (usb) KERNEL[253.744911] change /devices/pci0000:00/0000:00:11.0/dwc3.0.auto/xhci-hcd.2.auto/usb1/1-1/1-1.1 (usb) KERNEL[253.745804] add /devices/pci0000:00/0000:00:11.0/dwc3.0.auto/xhci-hcd.2.auto/usb1/1-1/1-1.1/1-1.1:1.0 (usb) KERNEL[253.805307] add /devices/pci0000:00/0000:00:11.0/dwc3.0.auto/xhci-hcd.2.auto/usb1/1-1/1-1.1/mdio_bus/usb-001:005 (mdio_bus) KERNEL[253.812978] add /devices/pci0000:00/0000:00:11.0/dwc3.0.auto/xhci-hcd.2.auto/usb1/1-1/1-1.1/mdio_bus/usb-001:005/usb-001:005:01 (mdio_bus) KERNEL[253.814318] bind /devices/pci0000:00/0000:00:11.0/dwc3.0.auto/xhci-hcd.2.auto/usb1/1-1/1-1.1/mdio_bus/usb-001:005/usb-001:005:01 (mdio_bus) KERNEL[253.815386] add /devices/pci0000:00/0000:00:11.0/dwc3.0.auto/xhci-hcd.2.auto/usb1/1-1/1-1.1/1-1.1:1.0/net/eth0 (net) KERNEL[253.815552] add /devices/pci0000:00/0000:00:11.0/dwc3.0.auto/xhci-hcd.2.auto/usb1/1-1/1-1.1/1-1.1:1.0/net/eth0/queues/rx-0 (queues) KERNEL[253.815778] add /devices/pci0000:00/0000:00:11.0/dwc3.0.auto/xhci-hcd.2.auto/usb1/1-1/1-1.1/1-1.1:1.0/net/eth0/queues/tx-0 (queues) KERNEL[253.825279] bind /devices/pci0000:00/0000:00:11.0/dwc3.0.auto/xhci-hcd.2.auto/usb1/1-1/1-1.1/1-1.1:1.0 (usb) KERNEL[253.825667] bind /devices/pci0000:00/0000:00:11.0/dwc3.0.auto/xhci-hcd.2.auto/usb1/1-1/1-1.1 (usb)
# switch to gadget again
KERNEL[314.212144] remove /devices/pci0000:00/0000:00:11.0/dwc3.0.auto/xhci-hcd.2.auto/usb1/1-1/1-1.1/1-1.1:1.0/net/eth0/queues/rx-0 (queues) KERNEL[314.212473] remove /devices/pci0000:00/0000:00:11.0/dwc3.0.auto/xhci-hcd.2.auto/usb1/1-1/1-1.1/1-1.1:1.0/net/eth0/queues/tx-0 (queues) KERNEL[314.214691] remove /devices/pci0000:00/0000:00:11.0/dwc3.0.auto/xhci-hcd.2.auto/usb1/1-1/1-1.1/1-1.1:1.0/net/eth0 (net)
# extcon event didn't show the first time
KERNEL[314.238385] change /devices/pci0000:00/0000:00:13.0/INTC100E:00/mrfld_bcove_pwrsrc/extcon/extcon0 (extcon) KERNEL[314.238677] unbind /devices/pci0000:00/0000:00:11.0/dwc3.0.auto/xhci-hcd.2.auto/usb1/1-1/1-1.1/mdio_bus/usb-001:005/usb-001:005:01 (mdio_bus) KERNEL[314.238863] remove /devices/pci0000:00/0000:00:11.0/dwc3.0.auto/xhci-hcd.2.auto/usb1/1-1/1-1.1/mdio_bus/usb-001:005/usb-001:005:01 (mdio_bus) KERNEL[314.239015] remove /devices/pci0000:00/0000:00:11.0/dwc3.0.auto/xhci-hcd.2.auto/usb1/1-1/1-1.1/mdio_bus/usb-001:005 (mdio_bus) KERNEL[314.239205] unbind /devices/pci0000:00/0000:00:11.0/dwc3.0.auto/xhci-hcd.2.auto/usb1/1-1/1-1.1/1-1.1:1.0 (usb) KERNEL[314.239429] remove /devices/pci0000:00/0000:00:11.0/dwc3.0.auto/xhci-hcd.2.auto/usb1/1-1/1-1.1/1-1.1:1.0 (usb) KERNEL[314.239666] unbind /devices/pci0000:00/0000:00:11.0/dwc3.0.auto/xhci-hcd.2.auto/usb2/2-0:1.0 (usb)
KERNEL[314.239933] remove /devices/pci0000:00/0000:00:11.0/dwc3.0.auto/xhci-hcd.2.auto/usb2/2-0:1.0 (usb)
KERNEL[314.262713] unbind /devices/pci0000:00/0000:00:11.0/dwc3.0.auto/xhci-hcd.2.auto/usb2 (usb) KERNEL[314.263030] unbind /devices/pci0000:00/0000:00:11.0/dwc3.0.auto/xhci-hcd.2.auto/usb1/1-1/1-1.1 (usb) KERNEL[314.263298] remove /devices/pci0000:00/0000:00:11.0/dwc3.0.auto/xhci-hcd.2.auto/usb2 (usb) KERNEL[314.263569] remove /devices/pci0000:00/0000:00:11.0/dwc3.0.auto/xhci-hcd.2.auto/usb1/1-1/1-1.1 (usb) KERNEL[314.263815] unbind /devices/pci0000:00/0000:00:11.0/dwc3.0.auto/xhci-hcd.2.auto/usb1/1-1/1-1:1.0 (usb) KERNEL[314.264042] remove /devices/pci0000:00/0000:00:11.0/dwc3.0.auto/xhci-hcd.2.auto/usb1/1-1/1-1:1.0 (usb) KERNEL[314.264753] remove /devices/pci0000:00/0000:00:11.0/dwc3.0.auto/xhci-hcd.2.auto/usbmon/usbmon2 (usbmon) KERNEL[314.265019] unbind /devices/pci0000:00/0000:00:11.0/dwc3.0.auto/xhci-hcd.2.auto/usb1/1-1 (usb) KERNEL[314.265289] remove /devices/pci0000:00/0000:00:11.0/dwc3.0.auto/xhci-hcd.2.auto/usb1/1-1 (usb) KERNEL[314.288792] unbind /devices/pci0000:00/0000:00:11.0/dwc3.0.auto/xhci-hcd.2.auto/usb1/1-0:1.0 (usb)
KERNEL[314.289057] remove /devices/pci0000:00/0000:00:11.0/dwc3.0.auto/xhci-hcd.2.auto/usb1/1-0:1.0 (usb)
KERNEL[314.289327] unbind /devices/pci0000:00/0000:00:11.0/dwc3.0.auto/xhci-hcd.2.auto/usb1 (usb) KERNEL[314.289661] remove /devices/pci0000:00/0000:00:11.0/dwc3.0.auto/xhci-hcd.2.auto/usb1 (usb) KERNEL[314.647375] remove /devices/pci0000:00/0000:00:11.0/dwc3.0.auto/xhci-hcd.2.auto/usbmon/usbmon1 (usbmon) KERNEL[314.647816] unbind /devices/pci0000:00/0000:00:11.0/dwc3.0.auto/xhci-hcd.2.auto (platform) KERNEL[314.648143] remove /kernel/software_nodes/node1 (software_nodes) KERNEL[314.648672] remove /devices/pci0000:00/0000:00:11.0/dwc3.0.auto/xhci-hcd.2.auto (platform)
# here is the event we were waiting for
KERNEL[314.649158] add /devices/pci0000:00/0000:00:11.0/dwc3.0.auto/udc/dwc3.0.auto (udc)
# after this gadget devices appear normally
Maybe this issue is due to extcon missing the event?
From the info here, it doesn't look like the host platform device was removed on the first switch. Also, as you pointed it out, the extcon event was not shown on the first switch either. Without a notification to switch mode, the dwc3 driver won't do anything. You need to check why that's the case as I can't help much here.
Thanks, I am looking into our extcon driver now for this.
After a 2 - 4 minutes the connection is dropped and reconnected.
Does this occur with LPM disabled also? We can review this issue further with more dwc3 tracepoints.
I captured connection dropping and reconnecting in this fairly long trace near the end of the file:
https://urldefense.com/v3/__https://github.com/andy-shev/linux/files/6346323...
Nothing obvious stands out as a problem from the dwc3 driver or the controller. I see a (port) reset after 30 seconds of inactivity, which is a typical timeout and recovery mechanism in the upperlayer from host.
- Is this a new issue or was it always there?
I've seen it before, but because of all the other issues it wasn't obvious.
- Does turning off LPM help?
I think so, but maybe didn't wait long enough. This I will retest tomorrow evening.
- Are the other gadget functions still work during the 30 seconds
inactivity?
No. The behavior seems same as unplug/plug the cable.
On the gadget end journal shows:
Apr 19 22:08:42 yuna systemd-networkd[507]: usb0: Lost carrier Apr 19 22:08:42 yuna systemd-journald[417]: Forwarding to syslog missed 1 messages. Apr 19 22:08:42 yuna systemd-timesyncd[469]: No network connectivity, watching for changes. Apr 19 22:08:42 yuna kernel: IPv6: ADDRCONF(NETDEV_CHANGE): usb0: link becomes ready Apr 19 22:08:42 yuna kernel[480]: [ 624.382929] IPv6: ADDRCONF(NETDEV_CHANGE): usb0: link becomes ready Apr 19 22:08:42 yuna systemd-networkd[507]: usb0: Gained carrier Apr 19 22:08:44 yuna systemd-networkd[507]: usb0: Gained IPv6LL Apr 19 22:08:44 yuna systemd-timesyncd[469]: Network configuration changed, trying to establish connection. Apr 19 22:08:57 yuna systemd-timesyncd[469]: Initial synchronization to time server 216.239.35.8:123 (time3.google.com).
So, drops and immediately reconnects.
From the look at the log here, it seems to be a reset from host (and an issue at the protocol level) unrelated to dwc3 driver or the controller. Hopefully and maybe we can get more clues from dwc3 tracepoints.
BR, Thinh
Hi
Op 21-04-2021 om 21:01 schreef Thinh Nguyen:
Ferry Toth wrote:
Hi
Op 19-04-2021 om 23:23 schreef Thinh Nguyen:
Ferry Toth wrote:
Hi
Op 19-04-2021 om 01:03 schreef Thinh Nguyen:
Ferry Toth wrote:
Hi
Op 17-04-2021 om 16:22 schreef Ferry Toth: > Hi > > Op 17-04-2021 om 04:27 schreef Thinh Nguyen: >> Ferry Toth wrote: >>> Hi >>> >>> Op 16-04-2021 om 00:23 schreef Thinh Nguyen: >>>> Thinh Nguyen wrote: >>>>> From: Yu Chen chenyu56@huawei.com >>>>> From: John Stultz john.stultz@linaro.org >>>>> >>>>> According to the programming guide, to switch mode for DRD >>>>> controller, >>>>> the driver needs to do the following. >>>>> >>>>> To switch from device to host: >>>>> 1. Reset controller with GCTL.CoreSoftReset >>>>> 2. Set GCTL.PrtCapDir(host mode) >>>>> 3. Reset the host with USBCMD.HCRESET >>>>> 4. Then follow up with the initializing host registers sequence >>>>> >>>>> To switch from host to device: >>>>> 1. Reset controller with GCTL.CoreSoftReset >>>>> 2. Set GCTL.PrtCapDir(device mode) >>>>> 3. Reset the device with DCTL.CSftRst >>>>> 4. Then follow up with the initializing registers sequence >>>>> >>>>> Currently we're missing step 1) to do GCTL.CoreSoftReset and step >>>>> 3) of >>>>> switching from host to device. John Stult reported a lockup issue >>>>> seen >>>>> with HiKey960 platform without these steps[1]. Similar issue is >>>>> observed >>>>> with Ferry's testing platform[2]. >>>>> >>>>> So, apply the required steps along with some fixes to Yu Chen's >>>>> and John >>>>> Stultz's version. The main fixes to their versions are the >>>>> missing >>>>> wait >>>>> for clocks synchronization before clearing GCTL.CoreSoftReset and >>>>> only >>>>> apply DCTL.CSftRst when switching from host to device. >>>>> >>>>> [1] >>>>> https://urldefense.com/v3/__https://lore.kernel.org/linux-usb/20210108015115... >>>>> >>>>> >>>>> >>>>> >>>>> [2] >>>>> https://urldefense.com/v3/__https://lore.kernel.org/linux-usb/0ba7a6ba-e6a7-... >>>>> >>>>> >>>>> >>>>> >>>>> >>>>> Cc: Andy Shevchenko andy.shevchenko@gmail.com >>>>> Cc: Ferry Toth fntoth@gmail.com >>>>> Cc: Wesley Cheng wcheng@codeaurora.org >>>>> Cc: stable@vger.kernel.org >>>>> Fixes: 41ce1456e1db ("usb: dwc3: core: make dwc3_set_mode() work >>>>> properly") >>>>> Signed-off-by: Yu Chen chenyu56@huawei.com >>>>> Signed-off-by: John Stultz john.stultz@linaro.org >>>>> Signed-off-by: Thinh Nguyen Thinh.Nguyen@synopsys.com >>>>> --- >>>>> Changes in v3: >>>>> - Check if the desired mode is OTG, then keep the old flow >>>>> - Remove condition for OTG support only since the device can >>>>> still be >>>>> configured DRD host/device mode only >>>>> - Remove redundant hw_mode check since __dwc3_set_mode() only >>>>> applies >>>>> when >>>>> hw_mode is DRD >>>>> Changes in v2: >>>>> - Initialize mutex per device and not as global mutex. >>>>> - Add additional checks for DRD only mode >>>>> >>>>> drivers/usb/dwc3/core.c | 27 +++++++++++++++++++++++++++ >>>>> drivers/usb/dwc3/core.h | 5 +++++ >>>>> 2 files changed, 32 insertions(+) >>>>> >>>> Hi John, >>>> >>>> If possible, can you run a test with this version on your >>>> platform? >>>> >>>> Thanks, >>>> Thinh >>>> >>> I tested this on edison-arduino with this patch on top of usb-next >>> (5.12-rc7 + "increase BESL baseline to 6" to prevent throttling"). >>> >>> On this platform there is a physical switch to switch roles. With >>> this >>> patch I find: >>> >>> - switch to host mode always works fine >>> >>> - switch to gadget mode I need to flip the switch 3x >>> (gadget-host-gadget). >>> >>> An error message appears on the gadget side "dwc3 dwc3.0.auto: >>> timed >>> out >>> waiting for SETUP phase" appears, but then the device connects >>> to my >>> PC, >>> no throttling. >>> >>> - alternatively I can switch to gadget 1x and then unplug/replug >>> the >>> cable. >>> >>> No error message and connects fine. >>> >>> - if I flip the switch only once, on the PC side I get: >>> >>> kernel: usb 1-5: new high-speed USB device number 18 >>> usingxhci_hcd >>> kernel: usb 1-5: New USB device found, idVendor=1d6b, >>> idProduct=0104, bcdDevice= 1.00 kernel: usb1-5: New USB device >>> strings: Mfr=1, Product=2, SerialNumber=3kernel:usb 1-5: >>> Product: >>> USBArmory Gadget kernel: usb 1-5: Manufacturer:USBArmory >>> kernel: >>> usb 1-5: SerialNumber: 0123456789abcdef kernel:usb 1-5: can't >>> set >>> config #1, error -110 >> The device failed at set_configuration() request and timed out. It >> probably timed out from the status stage looking at the device err >> print. >> >>> Then if I wait long enough on the gadget side I get: >>> >>> root@yuna:~# ifconfig >>> >>> usb0: flags=-28605<UP,BROADCAST,RUNNING,MULTICAST,DYNAMIC> mtu >>> 1500 >>> inet 169.254.119.239 netmask 255.255.0.0 broadcast >>> 169.254.255.255 >>> inet6 fe80::a8bb:ccff:fedd:eef1 prefixlen 64 scopeid >>> 0x20<link> >>> ether aa:bb:cc:dd:ee:f1 txqueuelen 1000 (Ethernet) RX packets >>> 490424 >>> bytes 735146578 (701.0 MiB) RX errors 0 dropped191 overruns 0 >>> frame >>> 0 TX packets 35279 bytes 2532746 (2.4 MiB) TX errors 0 >>> dropped 0 >>> overruns 0 carrier 0 collisions 0 >>> >>> (correct would be: inet 10.42.0.221 netmask 255.255.255.0 broadcast >>> 10.42.0.255) >>> >>> So much improved now, but it seems I am still missing something on >>> plug. >>> >> That's great! We can look at it further. Can you capture the >> tracepoints >> of the issue. Also, can you try with mass_storage gadget to see >> if the >> result is the same? > I have already gser, eem, mass_storage and uac2 combo. When eem > fails, > the mass_storage and uac2 don't appear (on KDE you get all kind of > popups when they appear). > > So either all works, or all fails. > > I'll trace this later today. Trace capturing switch from host-> gadget here https://urldefense.com/v3/__https://github.com/andy-shev/linux/files/6329600...
(Issue history: https://urldefense.com/v3/__https://github.com/andy-shev/linux/issues/31__%3...
)
On the PC side this resulted to:
apr 17 18:17:44 delfion kernel: usb 1-5: new high-speed USB device number 12 using xhci_hcd apr 17 18:17:44 delfion kernel: usb 1-5: New USB device found, idVendor=1d6b, idProduct=0104, bcdDevice= 1.00 apr 17 18:17:44 delfion kernel: usb 1-5: New USB device strings: Mfr=1, Product=2, SerialNumber=3 apr 17 18:17:44 delfion kernel: usb 1-5: Product: USBArmory Gadget apr 17 18:17:44 delfion kernel: usb 1-5: Manufacturer: USBArmory apr 17 18:17:44 delfion kernel: usb 1-5: SerialNumber: 0123456789abcdef apr 17 18:17:49 delfion kernel: usb 1-5: can't set config #1, error -110
Thanks for all your help!
Looks like it's LPM related again. To confirm, try this: Disable LPM with this property "snps,usb2-gadget-lpm-disable" (Note that it's not the same as "snps,dis_enblslpm_quirk")
Yes, I confirm this helps.
Note: on startup I was in host mode, with gadget cable plugged. The first switch to gadget didn't work, all subsequent switches did work, as well as unplug/plug the cable.
Make sure that your testing kernel has this patch [1] 475e8be53d04 ("usb: dwc3: gadget: Check for disabled LPM quirk")
[1] https://urldefense.com/v3/__https://git.kernel.org/pub/scm/linux/kernel/git/...
The failure you saw was probably due the gadget function attempting to start a delayed status stage of the SET_CONFIGURATION request. By this time, the host already put the device in low power.
The START_TRANSFER command needs to be executed while the device is on "ON" state (or U0 if eSS). We shouldn't use dwc->link_state to check for link state because we only enable link state change interrupt for some controller versions.
Once you confirms disabling LPM works, try this fix:
diff --git a/drivers/usb/dwc3/gadget.c b/drivers/usb/dwc3/gadget.c index 6227641f2d31..06cdec79244e 100644 --- a/drivers/usb/dwc3/gadget.c +++ b/drivers/usb/dwc3/gadget.c @@ -309,10 +309,14 @@ int dwc3_send_gadget_ep_cmd(struct dwc3_ep *dep, unsigned int cmd, if (DWC3_DEPCMD_CMD(cmd) == DWC3_DEPCMD_STARTTRANSFER) { int needs_wakeup; + u8 link_state; - needs_wakeup = (dwc->link_state == DWC3_LINK_STATE_U1 || - dwc->link_state == DWC3_LINK_STATE_U2|| - dwc->link_state == DWC3_LINK_STATE_U3); + reg = dwc3_readl(dwc->regs, DWC3_DSTS); + link_state = DWC3_DSTS_USBLNKST(reg);
+ needs_wakeup = (link_state == DWC3_LINK_STATE_U1 || + link_state == DWC3_LINK_STATE_U2 || + link_state == DWC3_LINK_STATE_U3); if (unlikely(needs_wakeup)) { ret = __dwc3_gadget_wakeup(dwc); @@ -1989,6 +1993,8 @@ static int __dwc3_gadget_wakeup(struct dwc3 *dwc) case DWC3_LINK_STATE_RESET: case DWC3_LINK_STATE_RX_DET: /* in HS, means Early Suspend */ case DWC3_LINK_STATE_U3: /* in HS, means SUSPEND */ + case DWC3_LINK_STATE_U2: /* in HS, means Sleep (L1) */ + case DWC3_LINK_STATE_U1: case DWC3_LINK_STATE_RESUME: break; default:
Same (good) result as with "snps,usb2-gadget-lpm-disable". Including first switch from host->gadget not working.
Great! Not sure why the first switch is not working, but it seems like we were able to eliminate quite a few issues. If you have more dwc3 tracepoints, we can take a look further.
I traced but the file is empty. I captured the registers as well. The zip file is here:
https://urldefense.com/v3/__https://github.com/andy-shev/linux/files/6346271...
I found the gadget configuration script was not called, which normally gets called due to a udev rule:
ACTION=="add", KERNEL=="dwc3.0.auto", SUBSYSTEMS=="udc", ATTRS{state}=="not attached", RUN+="/usr/bin/conf-gadget.sh"
So I retried and see with ~# udevadm monitor:
# flipping the switch from host->gadget
KERNEL[51.824914] remove /devices/pci0000:00/0000:00:11.0/dwc3.0.auto/xhci-hcd.2.auto/usb1/1-1/1-1.1/1-1.1:1.0/net/enp0s17u1u1/queues/rx-0 (queues) KERNEL[51.825682] remove /devices/pci0000:00/0000:00:11.0/dwc3.0.auto/xhci-hcd.2.auto/usb1/1-1/1-1.1/1-1.1:1.0/net/enp0s17u1u1/queues/tx-0 (queues) KERNEL[51.826226] remove /devices/pci0000:00/0000:00:11.0/dwc3.0.auto/xhci-hcd.2.auto/usb1/1-1/1-1.1/1-1.1:1.0/net/enp0s17u1u1 (net) KERNEL[51.836041] unbind /devices/pci0000:00/0000:00:11.0/dwc3.0.auto/xhci-hcd.2.auto/usb1/1-1/1-1.1/mdio_bus/usb-001:003/usb-001:003:01 (mdio_bus) KERNEL[51.836709] remove /devices/pci0000:00/0000:00:11.0/dwc3.0.auto/xhci-hcd.2.auto/usb1/1-1/1-1.1/mdio_bus/usb-001:003/usb-001:003:01 (mdio_bus) KERNEL[51.837342] remove /devices/pci0000:00/0000:00:11.0/dwc3.0.auto/xhci-hcd.2.auto/usb1/1-1/1-1.1/mdio_bus/usb-001:003 (mdio_bus) KERNEL[51.837763] unbind /devices/pci0000:00/0000:00:11.0/dwc3.0.auto/xhci-hcd.2.auto/usb1/1-1/1-1.1/1-1.1:1.0 (usb) KERNEL[51.838116] remove /devices/pci0000:00/0000:00:11.0/dwc3.0.auto/xhci-hcd.2.auto/usb1/1-1/1-1.1/1-1.1:1.0 (usb) KERNEL[51.873712] unbind /devices/pci0000:00/0000:00:11.0/dwc3.0.auto/xhci-hcd.2.auto/usb1/1-1/1-1.1 (usb) KERNEL[51.874000] remove /devices/pci0000:00/0000:00:11.0/dwc3.0.auto/xhci-hcd.2.auto/usb1/1-1/1-1.1 (usb) KERNEL[51.874207] unbind /devices/pci0000:00/0000:00:11.0/dwc3.0.auto/xhci-hcd.2.auto/usb1/1-1/1-1:1.0 (usb) KERNEL[51.874431] remove /devices/pci0000:00/0000:00:11.0/dwc3.0.auto/xhci-hcd.2.auto/usb1/1-1/1-1:1.0 (usb) KERNEL[51.897175] unbind /devices/pci0000:00/0000:00:11.0/dwc3.0.auto/xhci-hcd.2.auto/usb1/1-1 (usb) KERNEL[51.897486] remove /devices/pci0000:00/0000:00:11.0/dwc3.0.auto/xhci-hcd.2.auto/usb1/1-1 (usb)
# stopped capture tracepoints here, then switch back to host
KERNEL[253.214406] add /devices/pci0000:00/0000:00:11.0/dwc3.0.auto/xhci-hcd.2.auto/usb1/1-1 (usb) KERNEL[253.263305] change /devices/pci0000:00/0000:00:11.0/dwc3.0.auto/xhci-hcd.2.auto/usb1/1-1 (usb) KERNEL[253.263687] add /devices/pci0000:00/0000:00:11.0/dwc3.0.auto/xhci-hcd.2.auto/usb1/1-1/1-1:1.0 (usb) KERNEL[253.328354] bind /devices/pci0000:00/0000:00:11.0/dwc3.0.auto/xhci-hcd.2.auto/usb1/1-1/1-1:1.0 (usb) KERNEL[253.328734] bind /devices/pci0000:00/0000:00:11.0/dwc3.0.auto/xhci-hcd.2.auto/usb1/1-1 (usb) KERNEL[253.699341] add /devices/pci0000:00/0000:00:11.0/dwc3.0.auto/xhci-hcd.2.auto/usb1/1-1/1-1.1 (usb) KERNEL[253.744911] change /devices/pci0000:00/0000:00:11.0/dwc3.0.auto/xhci-hcd.2.auto/usb1/1-1/1-1.1 (usb) KERNEL[253.745804] add /devices/pci0000:00/0000:00:11.0/dwc3.0.auto/xhci-hcd.2.auto/usb1/1-1/1-1.1/1-1.1:1.0 (usb) KERNEL[253.805307] add /devices/pci0000:00/0000:00:11.0/dwc3.0.auto/xhci-hcd.2.auto/usb1/1-1/1-1.1/mdio_bus/usb-001:005 (mdio_bus) KERNEL[253.812978] add /devices/pci0000:00/0000:00:11.0/dwc3.0.auto/xhci-hcd.2.auto/usb1/1-1/1-1.1/mdio_bus/usb-001:005/usb-001:005:01 (mdio_bus) KERNEL[253.814318] bind /devices/pci0000:00/0000:00:11.0/dwc3.0.auto/xhci-hcd.2.auto/usb1/1-1/1-1.1/mdio_bus/usb-001:005/usb-001:005:01 (mdio_bus) KERNEL[253.815386] add /devices/pci0000:00/0000:00:11.0/dwc3.0.auto/xhci-hcd.2.auto/usb1/1-1/1-1.1/1-1.1:1.0/net/eth0 (net) KERNEL[253.815552] add /devices/pci0000:00/0000:00:11.0/dwc3.0.auto/xhci-hcd.2.auto/usb1/1-1/1-1.1/1-1.1:1.0/net/eth0/queues/rx-0 (queues) KERNEL[253.815778] add /devices/pci0000:00/0000:00:11.0/dwc3.0.auto/xhci-hcd.2.auto/usb1/1-1/1-1.1/1-1.1:1.0/net/eth0/queues/tx-0 (queues) KERNEL[253.825279] bind /devices/pci0000:00/0000:00:11.0/dwc3.0.auto/xhci-hcd.2.auto/usb1/1-1/1-1.1/1-1.1:1.0 (usb) KERNEL[253.825667] bind /devices/pci0000:00/0000:00:11.0/dwc3.0.auto/xhci-hcd.2.auto/usb1/1-1/1-1.1 (usb)
# switch to gadget again
KERNEL[314.212144] remove /devices/pci0000:00/0000:00:11.0/dwc3.0.auto/xhci-hcd.2.auto/usb1/1-1/1-1.1/1-1.1:1.0/net/eth0/queues/rx-0 (queues) KERNEL[314.212473] remove /devices/pci0000:00/0000:00:11.0/dwc3.0.auto/xhci-hcd.2.auto/usb1/1-1/1-1.1/1-1.1:1.0/net/eth0/queues/tx-0 (queues) KERNEL[314.214691] remove /devices/pci0000:00/0000:00:11.0/dwc3.0.auto/xhci-hcd.2.auto/usb1/1-1/1-1.1/1-1.1:1.0/net/eth0 (net)
# extcon event didn't show the first time
KERNEL[314.238385] change /devices/pci0000:00/0000:00:13.0/INTC100E:00/mrfld_bcove_pwrsrc/extcon/extcon0 (extcon) KERNEL[314.238677] unbind /devices/pci0000:00/0000:00:11.0/dwc3.0.auto/xhci-hcd.2.auto/usb1/1-1/1-1.1/mdio_bus/usb-001:005/usb-001:005:01 (mdio_bus) KERNEL[314.238863] remove /devices/pci0000:00/0000:00:11.0/dwc3.0.auto/xhci-hcd.2.auto/usb1/1-1/1-1.1/mdio_bus/usb-001:005/usb-001:005:01 (mdio_bus) KERNEL[314.239015] remove /devices/pci0000:00/0000:00:11.0/dwc3.0.auto/xhci-hcd.2.auto/usb1/1-1/1-1.1/mdio_bus/usb-001:005 (mdio_bus) KERNEL[314.239205] unbind /devices/pci0000:00/0000:00:11.0/dwc3.0.auto/xhci-hcd.2.auto/usb1/1-1/1-1.1/1-1.1:1.0 (usb) KERNEL[314.239429] remove /devices/pci0000:00/0000:00:11.0/dwc3.0.auto/xhci-hcd.2.auto/usb1/1-1/1-1.1/1-1.1:1.0 (usb) KERNEL[314.239666] unbind /devices/pci0000:00/0000:00:11.0/dwc3.0.auto/xhci-hcd.2.auto/usb2/2-0:1.0 (usb)
KERNEL[314.239933] remove /devices/pci0000:00/0000:00:11.0/dwc3.0.auto/xhci-hcd.2.auto/usb2/2-0:1.0 (usb)
KERNEL[314.262713] unbind /devices/pci0000:00/0000:00:11.0/dwc3.0.auto/xhci-hcd.2.auto/usb2 (usb) KERNEL[314.263030] unbind /devices/pci0000:00/0000:00:11.0/dwc3.0.auto/xhci-hcd.2.auto/usb1/1-1/1-1.1 (usb) KERNEL[314.263298] remove /devices/pci0000:00/0000:00:11.0/dwc3.0.auto/xhci-hcd.2.auto/usb2 (usb) KERNEL[314.263569] remove /devices/pci0000:00/0000:00:11.0/dwc3.0.auto/xhci-hcd.2.auto/usb1/1-1/1-1.1 (usb) KERNEL[314.263815] unbind /devices/pci0000:00/0000:00:11.0/dwc3.0.auto/xhci-hcd.2.auto/usb1/1-1/1-1:1.0 (usb) KERNEL[314.264042] remove /devices/pci0000:00/0000:00:11.0/dwc3.0.auto/xhci-hcd.2.auto/usb1/1-1/1-1:1.0 (usb) KERNEL[314.264753] remove /devices/pci0000:00/0000:00:11.0/dwc3.0.auto/xhci-hcd.2.auto/usbmon/usbmon2 (usbmon) KERNEL[314.265019] unbind /devices/pci0000:00/0000:00:11.0/dwc3.0.auto/xhci-hcd.2.auto/usb1/1-1 (usb) KERNEL[314.265289] remove /devices/pci0000:00/0000:00:11.0/dwc3.0.auto/xhci-hcd.2.auto/usb1/1-1 (usb) KERNEL[314.288792] unbind /devices/pci0000:00/0000:00:11.0/dwc3.0.auto/xhci-hcd.2.auto/usb1/1-0:1.0 (usb)
KERNEL[314.289057] remove /devices/pci0000:00/0000:00:11.0/dwc3.0.auto/xhci-hcd.2.auto/usb1/1-0:1.0 (usb)
KERNEL[314.289327] unbind /devices/pci0000:00/0000:00:11.0/dwc3.0.auto/xhci-hcd.2.auto/usb1 (usb) KERNEL[314.289661] remove /devices/pci0000:00/0000:00:11.0/dwc3.0.auto/xhci-hcd.2.auto/usb1 (usb) KERNEL[314.647375] remove /devices/pci0000:00/0000:00:11.0/dwc3.0.auto/xhci-hcd.2.auto/usbmon/usbmon1 (usbmon) KERNEL[314.647816] unbind /devices/pci0000:00/0000:00:11.0/dwc3.0.auto/xhci-hcd.2.auto (platform) KERNEL[314.648143] remove /kernel/software_nodes/node1 (software_nodes) KERNEL[314.648672] remove /devices/pci0000:00/0000:00:11.0/dwc3.0.auto/xhci-hcd.2.auto (platform)
# here is the event we were waiting for
KERNEL[314.649158] add /devices/pci0000:00/0000:00:11.0/dwc3.0.auto/udc/dwc3.0.auto (udc)
# after this gadget devices appear normally
Maybe this issue is due to extcon missing the event?
From the info here, it doesn't look like the host platform device was removed on the first switch. Also, as you pointed it out, the extcon event was not shown on the first switch either. Without a notification to switch mode, the dwc3 driver won't do anything. You need to check why that's the case as I can't help much here.
After a 2 - 4 minutes the connection is dropped and reconnected.
Does this occur with LPM disabled also? We can review this issue further with more dwc3 tracepoints.
I captured connection dropping and reconnecting in this fairly long trace near the end of the file:
https://urldefense.com/v3/__https://github.com/andy-shev/linux/files/6346323...
Nothing obvious stands out as a problem from the dwc3 driver or the controller. I see a (port) reset after 30 seconds of inactivity, which is a typical timeout and recovery mechanism in the upperlayer from host.
- Is this a new issue or was it always there?
- Does turning off LPM help?
I reverted my "usb: gadget: increase BESL baseline to 6" and picked "usb: dwc3: pci: Enable dis_enblslpm_quirk for Intel Merrifield".
So this is again the big hammer you suggested earlier to turn off LPM.
After 15 min (at least 4x longer then normal to get a port reset) I have still not seen a port reset.
So for now I conclude, yes turning off LPM helps.
- Are the other gadget functions still work during the 30 seconds
inactivity?
On the gadget end journal shows:
Apr 19 22:08:42 yuna systemd-networkd[507]: usb0: Lost carrier Apr 19 22:08:42 yuna systemd-journald[417]: Forwarding to syslog missed 1 messages. Apr 19 22:08:42 yuna systemd-timesyncd[469]: No network connectivity, watching for changes. Apr 19 22:08:42 yuna kernel: IPv6: ADDRCONF(NETDEV_CHANGE): usb0: link becomes ready Apr 19 22:08:42 yuna kernel[480]: [ 624.382929] IPv6: ADDRCONF(NETDEV_CHANGE): usb0: link becomes ready Apr 19 22:08:42 yuna systemd-networkd[507]: usb0: Gained carrier Apr 19 22:08:44 yuna systemd-networkd[507]: usb0: Gained IPv6LL Apr 19 22:08:44 yuna systemd-timesyncd[469]: Network configuration changed, trying to establish connection. Apr 19 22:08:57 yuna systemd-timesyncd[469]: Initial synchronization to time server 216.239.35.8:123 (time3.google.com).
So, drops and immediately reconnects.
From the look at the log here, it seems to be a reset from host (and an issue at the protocol level) unrelated to dwc3 driver or the controller. Hopefully and maybe we can get more clues from dwc3 tracepoints.
BR, Thinh
Ferry Toth wrote:
Hi
Op 21-04-2021 om 21:01 schreef Thinh Nguyen:
Ferry Toth wrote:
Hi
Op 19-04-2021 om 23:23 schreef Thinh Nguyen:
Ferry Toth wrote:
Hi
Op 19-04-2021 om 01:03 schreef Thinh Nguyen:
Ferry Toth wrote: > Hi > > Op 17-04-2021 om 16:22 schreef Ferry Toth: >> Hi >> >> Op 17-04-2021 om 04:27 schreef Thinh Nguyen: >>> Ferry Toth wrote: >>>> Hi >>>> >>>> Op 16-04-2021 om 00:23 schreef Thinh Nguyen: >>>>> Thinh Nguyen wrote: >>>>>> From: Yu Chen chenyu56@huawei.com >>>>>> From: John Stultz john.stultz@linaro.org >>>>>> >>>>>> According to the programming guide, to switch mode for DRD >>>>>> controller, >>>>>> the driver needs to do the following. >>>>>> >>>>>> To switch from device to host: >>>>>> 1. Reset controller with GCTL.CoreSoftReset >>>>>> 2. Set GCTL.PrtCapDir(host mode) >>>>>> 3. Reset the host with USBCMD.HCRESET >>>>>> 4. Then follow up with the initializing host registers sequence >>>>>> >>>>>> To switch from host to device: >>>>>> 1. Reset controller with GCTL.CoreSoftReset >>>>>> 2. Set GCTL.PrtCapDir(device mode) >>>>>> 3. Reset the device with DCTL.CSftRst >>>>>> 4. Then follow up with the initializing registers sequence >>>>>> >>>>>> Currently we're missing step 1) to do GCTL.CoreSoftReset and >>>>>> step >>>>>> 3) of >>>>>> switching from host to device. John Stult reported a lockup >>>>>> issue >>>>>> seen >>>>>> with HiKey960 platform without these steps[1]. Similar issue is >>>>>> observed >>>>>> with Ferry's testing platform[2]. >>>>>> >>>>>> So, apply the required steps along with some fixes to Yu Chen's >>>>>> and John >>>>>> Stultz's version. The main fixes to their versions are the >>>>>> missing >>>>>> wait >>>>>> for clocks synchronization before clearing >>>>>> GCTL.CoreSoftReset and >>>>>> only >>>>>> apply DCTL.CSftRst when switching from host to device. >>>>>> >>>>>> [1] >>>>>> https://urldefense.com/v3/__https://lore.kernel.org/linux-usb/20210108015115... >>>>>> >>>>>> >>>>>> >>>>>> >>>>>> >>>>>> [2] >>>>>> https://urldefense.com/v3/__https://lore.kernel.org/linux-usb/0ba7a6ba-e6a7-... >>>>>> >>>>>> >>>>>> >>>>>> >>>>>> >>>>>> >>>>>> Cc: Andy Shevchenko andy.shevchenko@gmail.com >>>>>> Cc: Ferry Toth fntoth@gmail.com >>>>>> Cc: Wesley Cheng wcheng@codeaurora.org >>>>>> Cc: stable@vger.kernel.org >>>>>> Fixes: 41ce1456e1db ("usb: dwc3: core: make dwc3_set_mode() >>>>>> work >>>>>> properly") >>>>>> Signed-off-by: Yu Chen chenyu56@huawei.com >>>>>> Signed-off-by: John Stultz john.stultz@linaro.org >>>>>> Signed-off-by: Thinh Nguyen Thinh.Nguyen@synopsys.com >>>>>> --- >>>>>> Changes in v3: >>>>>> - Check if the desired mode is OTG, then keep the old flow >>>>>> - Remove condition for OTG support only since the device can >>>>>> still be >>>>>> configured DRD host/device mode only >>>>>> - Remove redundant hw_mode check since __dwc3_set_mode() only >>>>>> applies >>>>>> when >>>>>> hw_mode is DRD >>>>>> Changes in v2: >>>>>> - Initialize mutex per device and not as global mutex. >>>>>> - Add additional checks for DRD only mode >>>>>> >>>>>> drivers/usb/dwc3/core.c | 27 +++++++++++++++++++++++++++ >>>>>> drivers/usb/dwc3/core.h | 5 +++++ >>>>>> 2 files changed, 32 insertions(+) >>>>>> >>>>> Hi John, >>>>> >>>>> If possible, can you run a test with this version on your >>>>> platform? >>>>> >>>>> Thanks, >>>>> Thinh >>>>> >>>> I tested this on edison-arduino with this patch on top of >>>> usb-next >>>> (5.12-rc7 + "increase BESL baseline to 6" to prevent >>>> throttling"). >>>> >>>> On this platform there is a physical switch to switch roles. With >>>> this >>>> patch I find: >>>> >>>> - switch to host mode always works fine >>>> >>>> - switch to gadget mode I need to flip the switch 3x >>>> (gadget-host-gadget). >>>> >>>> An error message appears on the gadget side "dwc3 dwc3.0.auto: >>>> timed >>>> out >>>> waiting for SETUP phase" appears, but then the device connects >>>> to my >>>> PC, >>>> no throttling. >>>> >>>> - alternatively I can switch to gadget 1x and then unplug/replug >>>> the >>>> cable. >>>> >>>> No error message and connects fine. >>>> >>>> - if I flip the switch only once, on the PC side I get: >>>> >>>> kernel: usb 1-5: new high-speed USB device number 18 >>>> usingxhci_hcd >>>> kernel: usb 1-5: New USB device found, idVendor=1d6b, >>>> idProduct=0104, bcdDevice= 1.00 kernel: usb1-5: New USB >>>> device >>>> strings: Mfr=1, Product=2, SerialNumber=3kernel:usb 1-5: >>>> Product: >>>> USBArmory Gadget kernel: usb 1-5: Manufacturer:USBArmory >>>> kernel: >>>> usb 1-5: SerialNumber: 0123456789abcdef kernel:usb 1-5: >>>> can't >>>> set >>>> config #1, error -110 >>> The device failed at set_configuration() request and timed out. It >>> probably timed out from the status stage looking at the device err >>> print. >>> >>>> Then if I wait long enough on the gadget side I get: >>>> >>>> root@yuna:~# ifconfig >>>> >>>> usb0: >>>> flags=-28605<UP,BROADCAST,RUNNING,MULTICAST,DYNAMIC> mtu >>>> 1500 >>>> inet 169.254.119.239 netmask 255.255.0.0 broadcast >>>> 169.254.255.255 >>>> inet6 fe80::a8bb:ccff:fedd:eef1 prefixlen 64 scopeid >>>> 0x20<link> >>>> ether aa:bb:cc:dd:ee:f1 txqueuelen 1000 (Ethernet) RX >>>> packets >>>> 490424 >>>> bytes 735146578 (701.0 MiB) RX errors 0 dropped191 >>>> overruns 0 >>>> frame >>>> 0 TX packets 35279 bytes 2532746 (2.4 MiB) TX errors 0 >>>> dropped 0 >>>> overruns 0 carrier 0 collisions 0 >>>> >>>> (correct would be: inet 10.42.0.221 netmask 255.255.255.0 >>>> broadcast >>>> 10.42.0.255) >>>> >>>> So much improved now, but it seems I am still missing >>>> something on >>>> plug. >>>> >>> That's great! We can look at it further. Can you capture the >>> tracepoints >>> of the issue. Also, can you try with mass_storage gadget to see >>> if the >>> result is the same? >> I have already gser, eem, mass_storage and uac2 combo. When eem >> fails, >> the mass_storage and uac2 don't appear (on KDE you get all kind of >> popups when they appear). >> >> So either all works, or all fails. >> >> I'll trace this later today. > Trace capturing switch from host-> gadget here > https://urldefense.com/v3/__https://github.com/andy-shev/linux/files/6329600... > > > > > (Issue history: > https://urldefense.com/v3/__https://github.com/andy-shev/linux/issues/31__%3... > > > > ) > > On the PC side this resulted to: > > apr 17 18:17:44 delfion kernel: usb 1-5: new high-speed USB device > number 12 using xhci_hcd > apr 17 18:17:44 delfion kernel: usb 1-5: New USB device found, > idVendor=1d6b, idProduct=0104, bcdDevice= 1.00 > apr 17 18:17:44 delfion kernel: usb 1-5: New USB device strings: > Mfr=1, > Product=2, SerialNumber=3 > apr 17 18:17:44 delfion kernel: usb 1-5: Product: USBArmory Gadget > apr 17 18:17:44 delfion kernel: usb 1-5: Manufacturer: USBArmory > apr 17 18:17:44 delfion kernel: usb 1-5: SerialNumber: > 0123456789abcdef > apr 17 18:17:49 delfion kernel: usb 1-5: can't set config #1, error > -110 > > > Thanks for all your help! > Looks like it's LPM related again. To confirm, try this: Disable LPM with this property "snps,usb2-gadget-lpm-disable" (Note that it's not the same as "snps,dis_enblslpm_quirk")
Yes, I confirm this helps.
Note: on startup I was in host mode, with gadget cable plugged. The first switch to gadget didn't work, all subsequent switches did work, as well as unplug/plug the cable.
Make sure that your testing kernel has this patch [1] 475e8be53d04 ("usb: dwc3: gadget: Check for disabled LPM quirk")
[1] https://urldefense.com/v3/__https://git.kernel.org/pub/scm/linux/kernel/git/...
The failure you saw was probably due the gadget function attempting to start a delayed status stage of the SET_CONFIGURATION request. By this time, the host already put the device in low power.
The START_TRANSFER command needs to be executed while the device is on "ON" state (or U0 if eSS). We shouldn't use dwc->link_state to check for link state because we only enable link state change interrupt for some controller versions.
Once you confirms disabling LPM works, try this fix:
diff --git a/drivers/usb/dwc3/gadget.c b/drivers/usb/dwc3/gadget.c index 6227641f2d31..06cdec79244e 100644 --- a/drivers/usb/dwc3/gadget.c +++ b/drivers/usb/dwc3/gadget.c @@ -309,10 +309,14 @@ int dwc3_send_gadget_ep_cmd(struct dwc3_ep *dep, unsigned int cmd, if (DWC3_DEPCMD_CMD(cmd) == DWC3_DEPCMD_STARTTRANSFER) { int needs_wakeup; + u8 link_state; - needs_wakeup = (dwc->link_state == DWC3_LINK_STATE_U1 || - dwc->link_state == DWC3_LINK_STATE_U2|| - dwc->link_state == DWC3_LINK_STATE_U3); + reg = dwc3_readl(dwc->regs, DWC3_DSTS); + link_state = DWC3_DSTS_USBLNKST(reg);
+ needs_wakeup = (link_state == DWC3_LINK_STATE_U1 || + link_state == DWC3_LINK_STATE_U2 || + link_state == DWC3_LINK_STATE_U3); if (unlikely(needs_wakeup)) { ret = __dwc3_gadget_wakeup(dwc); @@ -1989,6 +1993,8 @@ static int __dwc3_gadget_wakeup(struct dwc3 *dwc) case DWC3_LINK_STATE_RESET: case DWC3_LINK_STATE_RX_DET: /* in HS, means Early Suspend */ case DWC3_LINK_STATE_U3: /* in HS, means SUSPEND */ + case DWC3_LINK_STATE_U2: /* in HS, means Sleep (L1) */ + case DWC3_LINK_STATE_U1: case DWC3_LINK_STATE_RESUME: break; default:
Same (good) result as with "snps,usb2-gadget-lpm-disable". Including first switch from host->gadget not working.
Great! Not sure why the first switch is not working, but it seems like we were able to eliminate quite a few issues. If you have more dwc3 tracepoints, we can take a look further.
I traced but the file is empty. I captured the registers as well. The zip file is here:
https://urldefense.com/v3/__https://github.com/andy-shev/linux/files/6346271...
<snip>
Maybe this issue is due to extcon missing the event?
From the info here, it doesn't look like the host platform device was removed on the first switch. Also, as you pointed it out, the extcon event was not shown on the first switch either. Without a notification to switch mode, the dwc3 driver won't do anything. You need to check why that's the case as I can't help much here.
After a 2 - 4 minutes the connection is dropped and reconnected.
Does this occur with LPM disabled also? We can review this issue further with more dwc3 tracepoints.
I captured connection dropping and reconnecting in this fairly long trace near the end of the file:
https://urldefense.com/v3/__https://github.com/andy-shev/linux/files/6346323...
Nothing obvious stands out as a problem from the dwc3 driver or the controller. I see a (port) reset after 30 seconds of inactivity, which is a typical timeout and recovery mechanism in the upperlayer from host.
- Is this a new issue or was it always there?
- Does turning off LPM help?
I reverted my "usb: gadget: increase BESL baseline to 6" and picked "usb: dwc3: pci: Enable dis_enblslpm_quirk for Intel Merrifield".
So this is again the big hammer you suggested earlier to turn off LPM.
After 15 min (at least 4x longer then normal to get a port reset) I have still not seen a port reset.
So for now I conclude, yes turning off LPM helps.
Ok. Thanks for the updates. Looks like we may have to use the hammer afterall.
Btw, earlier I mistakenly say "dis_enblslpm_quirk" will disable LPM completely, it only disables the device going into "sleep" state. If you want to completely disable LPM, use "usb2-gadget-lpm-disable"
Thanks, Thinh
Hi,
Op 22-04-2021 om 23:58 schreef Thinh Nguyen:
Ferry Toth wrote:
Hi
Op 21-04-2021 om 21:01 schreef Thinh Nguyen:
Ferry Toth wrote:
Hi
Op 19-04-2021 om 23:23 schreef Thinh Nguyen:
Ferry Toth wrote:
Hi
Op 19-04-2021 om 01:03 schreef Thinh Nguyen: > Ferry Toth wrote: >> Hi >> >> Op 17-04-2021 om 16:22 schreef Ferry Toth: >>> Hi >>> >>> Op 17-04-2021 om 04:27 schreef Thinh Nguyen: >>>> Ferry Toth wrote: >>>>> Hi >>>>> >>>>> Op 16-04-2021 om 00:23 schreef Thinh Nguyen: >>>>>> Thinh Nguyen wrote: >>>>>>> From: Yu Chen chenyu56@huawei.com >>>>>>> From: John Stultz john.stultz@linaro.org >>>>>>> >>>>>>> According to the programming guide, to switch mode for DRD >>>>>>> controller, >>>>>>> the driver needs to do the following. >>>>>>> >>>>>>> To switch from device to host: >>>>>>> 1. Reset controller with GCTL.CoreSoftReset >>>>>>> 2. Set GCTL.PrtCapDir(host mode) >>>>>>> 3. Reset the host with USBCMD.HCRESET >>>>>>> 4. Then follow up with the initializing host registers sequence >>>>>>> >>>>>>> To switch from host to device: >>>>>>> 1. Reset controller with GCTL.CoreSoftReset >>>>>>> 2. Set GCTL.PrtCapDir(device mode) >>>>>>> 3. Reset the device with DCTL.CSftRst >>>>>>> 4. Then follow up with the initializing registers sequence >>>>>>> >>>>>>> Currently we're missing step 1) to do GCTL.CoreSoftReset and >>>>>>> step >>>>>>> 3) of >>>>>>> switching from host to device. John Stult reported a lockup >>>>>>> issue >>>>>>> seen >>>>>>> with HiKey960 platform without these steps[1]. Similar issue is >>>>>>> observed >>>>>>> with Ferry's testing platform[2]. >>>>>>> >>>>>>> So, apply the required steps along with some fixes to Yu Chen's >>>>>>> and John >>>>>>> Stultz's version. The main fixes to their versions are the >>>>>>> missing >>>>>>> wait >>>>>>> for clocks synchronization before clearing >>>>>>> GCTL.CoreSoftReset and >>>>>>> only >>>>>>> apply DCTL.CSftRst when switching from host to device. >>>>>>> >>>>>>> [1] >>>>>>> https://urldefense.com/v3/__https://lore.kernel.org/linux-usb/20210108015115... >>>>>>> >>>>>>> >>>>>>> >>>>>>> >>>>>>> >>>>>>> [2] >>>>>>> https://urldefense.com/v3/__https://lore.kernel.org/linux-usb/0ba7a6ba-e6a7-... >>>>>>> >>>>>>> >>>>>>> >>>>>>> >>>>>>> >>>>>>> >>>>>>> Cc: Andy Shevchenko andy.shevchenko@gmail.com >>>>>>> Cc: Ferry Toth fntoth@gmail.com >>>>>>> Cc: Wesley Cheng wcheng@codeaurora.org >>>>>>> Cc: stable@vger.kernel.org >>>>>>> Fixes: 41ce1456e1db ("usb: dwc3: core: make dwc3_set_mode() >>>>>>> work >>>>>>> properly") >>>>>>> Signed-off-by: Yu Chen chenyu56@huawei.com >>>>>>> Signed-off-by: John Stultz john.stultz@linaro.org >>>>>>> Signed-off-by: Thinh Nguyen Thinh.Nguyen@synopsys.com >>>>>>> --- >>>>>>> Changes in v3: >>>>>>> - Check if the desired mode is OTG, then keep the old flow >>>>>>> - Remove condition for OTG support only since the device can >>>>>>> still be >>>>>>> configured DRD host/device mode only >>>>>>> - Remove redundant hw_mode check since __dwc3_set_mode() only >>>>>>> applies >>>>>>> when >>>>>>> hw_mode is DRD >>>>>>> Changes in v2: >>>>>>> - Initialize mutex per device and not as global mutex. >>>>>>> - Add additional checks for DRD only mode >>>>>>> >>>>>>> drivers/usb/dwc3/core.c | 27 +++++++++++++++++++++++++++ >>>>>>> drivers/usb/dwc3/core.h | 5 +++++ >>>>>>> 2 files changed, 32 insertions(+) >>>>>>> >>>>>> Hi John, >>>>>> >>>>>> If possible, can you run a test with this version on your >>>>>> platform? >>>>>> >>>>>> Thanks, >>>>>> Thinh >>>>>> >>>>> I tested this on edison-arduino with this patch on top of >>>>> usb-next >>>>> (5.12-rc7 + "increase BESL baseline to 6" to prevent >>>>> throttling"). >>>>> >>>>> On this platform there is a physical switch to switch roles. With >>>>> this >>>>> patch I find: >>>>> >>>>> - switch to host mode always works fine >>>>> >>>>> - switch to gadget mode I need to flip the switch 3x >>>>> (gadget-host-gadget). >>>>> >>>>> An error message appears on the gadget side "dwc3 dwc3.0.auto: >>>>> timed >>>>> out >>>>> waiting for SETUP phase" appears, but then the device connects >>>>> to my >>>>> PC, >>>>> no throttling. >>>>> >>>>> - alternatively I can switch to gadget 1x and then unplug/replug >>>>> the >>>>> cable. >>>>> >>>>> No error message and connects fine. >>>>> >>>>> - if I flip the switch only once, on the PC side I get: >>>>> >>>>> kernel: usb 1-5: new high-speed USB device number 18 >>>>> usingxhci_hcd >>>>> kernel: usb 1-5: New USB device found, idVendor=1d6b, >>>>> idProduct=0104, bcdDevice= 1.00 kernel: usb1-5: New USB >>>>> device >>>>> strings: Mfr=1, Product=2, SerialNumber=3kernel:usb 1-5: >>>>> Product: >>>>> USBArmory Gadget kernel: usb 1-5: Manufacturer:USBArmory >>>>> kernel: >>>>> usb 1-5: SerialNumber: 0123456789abcdef kernel:usb 1-5: >>>>> can't >>>>> set >>>>> config #1, error -110 >>>> The device failed at set_configuration() request and timed out. It >>>> probably timed out from the status stage looking at the device err >>>> print. >>>> >>>>> Then if I wait long enough on the gadget side I get: >>>>> >>>>> root@yuna:~# ifconfig >>>>> >>>>> usb0: >>>>> flags=-28605<UP,BROADCAST,RUNNING,MULTICAST,DYNAMIC> mtu >>>>> 1500 >>>>> inet 169.254.119.239 netmask 255.255.0.0 broadcast >>>>> 169.254.255.255 >>>>> inet6 fe80::a8bb:ccff:fedd:eef1 prefixlen 64 scopeid >>>>> 0x20<link> >>>>> ether aa:bb:cc:dd:ee:f1 txqueuelen 1000 (Ethernet) RX >>>>> packets >>>>> 490424 >>>>> bytes 735146578 (701.0 MiB) RX errors 0 dropped191 >>>>> overruns 0 >>>>> frame >>>>> 0 TX packets 35279 bytes 2532746 (2.4 MiB) TX errors 0 >>>>> dropped 0 >>>>> overruns 0 carrier 0 collisions 0 >>>>> >>>>> (correct would be: inet 10.42.0.221 netmask 255.255.255.0 >>>>> broadcast >>>>> 10.42.0.255) >>>>> >>>>> So much improved now, but it seems I am still missing >>>>> something on >>>>> plug. >>>>> >>>> That's great! We can look at it further. Can you capture the >>>> tracepoints >>>> of the issue. Also, can you try with mass_storage gadget to see >>>> if the >>>> result is the same? >>> I have already gser, eem, mass_storage and uac2 combo. When eem >>> fails, >>> the mass_storage and uac2 don't appear (on KDE you get all kind of >>> popups when they appear). >>> >>> So either all works, or all fails. >>> >>> I'll trace this later today. >> Trace capturing switch from host-> gadget here >> https://urldefense.com/v3/__https://github.com/andy-shev/linux/files/6329600... >> >> >> >> >> (Issue history: >> https://urldefense.com/v3/__https://github.com/andy-shev/linux/issues/31__%3... >> >> >> >> ) >> >> On the PC side this resulted to: >> >> apr 17 18:17:44 delfion kernel: usb 1-5: new high-speed USB device >> number 12 using xhci_hcd >> apr 17 18:17:44 delfion kernel: usb 1-5: New USB device found, >> idVendor=1d6b, idProduct=0104, bcdDevice= 1.00 >> apr 17 18:17:44 delfion kernel: usb 1-5: New USB device strings: >> Mfr=1, >> Product=2, SerialNumber=3 >> apr 17 18:17:44 delfion kernel: usb 1-5: Product: USBArmory Gadget >> apr 17 18:17:44 delfion kernel: usb 1-5: Manufacturer: USBArmory >> apr 17 18:17:44 delfion kernel: usb 1-5: SerialNumber: >> 0123456789abcdef >> apr 17 18:17:49 delfion kernel: usb 1-5: can't set config #1, error >> -110 >> >> >> Thanks for all your help! >> > Looks like it's LPM related again. To confirm, try this: > Disable LPM with this property "snps,usb2-gadget-lpm-disable" > (Note that it's not the same as "snps,dis_enblslpm_quirk") Yes, I confirm this helps.
Note: on startup I was in host mode, with gadget cable plugged. The first switch to gadget didn't work, all subsequent switches did work, as well as unplug/plug the cable.
> Make sure that your testing kernel has this patch [1] > 475e8be53d04 ("usb: dwc3: gadget: Check for disabled LPM quirk") > > [1] > https://urldefense.com/v3/__https://git.kernel.org/pub/scm/linux/kernel/git/... > > > > > The failure you saw was probably due the gadget function attempting > to start a delayed status stage of the SET_CONFIGURATION request. > By this time, the host already put the device in low power. > > The START_TRANSFER command needs to be executed while the device > is on "ON" state (or U0 if eSS). We shouldn't use dwc->link_state > to check for link state because we only enable link state change > interrupt for some controller versions. > > Once you confirms disabling LPM works, try this fix: > > diff --git a/drivers/usb/dwc3/gadget.c b/drivers/usb/dwc3/gadget.c > index 6227641f2d31..06cdec79244e 100644 > --- a/drivers/usb/dwc3/gadget.c > +++ b/drivers/usb/dwc3/gadget.c > @@ -309,10 +309,14 @@ int dwc3_send_gadget_ep_cmd(struct dwc3_ep > *dep, > unsigned int cmd, > if (DWC3_DEPCMD_CMD(cmd) == DWC3_DEPCMD_STARTTRANSFER) { > int needs_wakeup; > + u8 link_state; > - needs_wakeup = (dwc->link_state == > DWC3_LINK_STATE_U1 || > - dwc->link_state == > DWC3_LINK_STATE_U2|| > - dwc->link_state == > DWC3_LINK_STATE_U3); > + reg = dwc3_readl(dwc->regs, DWC3_DSTS); > + link_state = DWC3_DSTS_USBLNKST(reg); > + > + needs_wakeup = (link_state == DWC3_LINK_STATE_U1 || > + link_state == DWC3_LINK_STATE_U2 || > + link_state == DWC3_LINK_STATE_U3); > if (unlikely(needs_wakeup)) { > ret = __dwc3_gadget_wakeup(dwc); > @@ -1989,6 +1993,8 @@ static int __dwc3_gadget_wakeup(struct dwc3 > *dwc) > case DWC3_LINK_STATE_RESET: > case DWC3_LINK_STATE_RX_DET: /* in HS, means Early > Suspend */ > case DWC3_LINK_STATE_U3: /* in HS, means SUSPEND */ > + case DWC3_LINK_STATE_U2: /* in HS, means Sleep (L1) */ > + case DWC3_LINK_STATE_U1: > case DWC3_LINK_STATE_RESUME: > break; > default: > Same (good) result as with "snps,usb2-gadget-lpm-disable". Including first switch from host->gadget not working.
Great! Not sure why the first switch is not working, but it seems like we were able to eliminate quite a few issues. If you have more dwc3 tracepoints, we can take a look further.
I traced but the file is empty. I captured the registers as well. The zip file is here:
https://urldefense.com/v3/__https://github.com/andy-shev/linux/files/6346271...
<snip>
Maybe this issue is due to extcon missing the event?
From the info here, it doesn't look like the host platform device was removed on the first switch. Also, as you pointed it out, the extcon event was not shown on the first switch either. Without a notification to switch mode, the dwc3 driver won't do anything. You need to check why that's the case as I can't help much here.
I was able to resolve this in extcon.
After a 2 - 4 minutes the connection is dropped and reconnected.
Does this occur with LPM disabled also? We can review this issue further with more dwc3 tracepoints.
I captured connection dropping and reconnecting in this fairly long trace near the end of the file:
https://urldefense.com/v3/__https://github.com/andy-shev/linux/files/6346323...
Nothing obvious stands out as a problem from the dwc3 driver or the controller. I see a (port) reset after 30 seconds of inactivity, which is a typical timeout and recovery mechanism in the upperlayer from host.
- Is this a new issue or was it always there?
- Does turning off LPM help?
I reverted my "usb: gadget: increase BESL baseline to 6" and picked "usb: dwc3: pci: Enable dis_enblslpm_quirk for Intel Merrifield".
So this is again the big hammer you suggested earlier to turn off LPM.
For future reference: the hammer is not dis_enblslpm_quirk but usb2-gadget-lpm-disable. Sorry for the mixup.
After 15 min (at least 4x longer then normal to get a port reset) I have still not seen a port reset.
So for now I conclude, yes turning off LPM helps.
Ok. Thanks for the updates. Looks like we may have to use the hammer afterall.
Ok, so we leave it at this then. With your fixes all issues seem to be resolved. Thanks so much for all your help!
Btw, earlier I mistakenly say "dis_enblslpm_quirk" will disable LPM completely, it only disables the device going into "sleep" state. If you want to completely disable LPM, use "usb2-gadget-lpm-disable"
Thanks, Thinh
On Thu, Apr 15, 2021 at 3:20 PM Thinh Nguyen Thinh.Nguyen@synopsys.com wrote:
From: Yu Chen chenyu56@huawei.com From: John Stultz john.stultz@linaro.org
According to the programming guide, to switch mode for DRD controller, the driver needs to do the following.
To switch from device to host:
- Reset controller with GCTL.CoreSoftReset
- Set GCTL.PrtCapDir(host mode)
- Reset the host with USBCMD.HCRESET
- Then follow up with the initializing host registers sequence
To switch from host to device:
- Reset controller with GCTL.CoreSoftReset
- Set GCTL.PrtCapDir(device mode)
- Reset the device with DCTL.CSftRst
- Then follow up with the initializing registers sequence
Currently we're missing step 1) to do GCTL.CoreSoftReset and step 3) of switching from host to device. John Stult reported a lockup issue seen with HiKey960 platform without these steps[1]. Similar issue is observed with Ferry's testing platform[2].
So, apply the required steps along with some fixes to Yu Chen's and John Stultz's version. The main fixes to their versions are the missing wait for clocks synchronization before clearing GCTL.CoreSoftReset and only apply DCTL.CSftRst when switching from host to device.
[1] https://lore.kernel.org/linux-usb/20210108015115.27920-1-john.stultz@linaro.... [2] https://lore.kernel.org/linux-usb/0ba7a6ba-e6a7-9cd4-0695-64fc927e01f1@gmail...
Cc: Andy Shevchenko andy.shevchenko@gmail.com Cc: Ferry Toth fntoth@gmail.com Cc: Wesley Cheng wcheng@codeaurora.org Cc: stable@vger.kernel.org Fixes: 41ce1456e1db ("usb: dwc3: core: make dwc3_set_mode() work properly") Signed-off-by: Yu Chen chenyu56@huawei.com Signed-off-by: John Stultz john.stultz@linaro.org Signed-off-by: Thinh Nguyen Thinh.Nguyen@synopsys.com
Changes in v3:
- Check if the desired mode is OTG, then keep the old flow
- Remove condition for OTG support only since the device can still be configured DRD host/device mode only
- Remove redundant hw_mode check since __dwc3_set_mode() only applies when hw_mode is DRD
Changes in v2:
- Initialize mutex per device and not as global mutex.
- Add additional checks for DRD only mode
I've not been able to test all the different modes on HiKey960 yet, but with this patch we avoid the !COREIDLE hangs that we see frequently on bootup, so it looks pretty good to me. I'll get back to you tonight when I can put hands on the board to test the gadget to host switching to make sure all is well (I really don't expect any issues, but just want to be sure).
thanks -john
On Thu, Apr 15, 2021 at 5:12 PM John Stultz john.stultz@linaro.org wrote:
On Thu, Apr 15, 2021 at 3:20 PM Thinh Nguyen Thinh.Nguyen@synopsys.com wrote:
From: Yu Chen chenyu56@huawei.com From: John Stultz john.stultz@linaro.org
According to the programming guide, to switch mode for DRD controller, the driver needs to do the following.
To switch from device to host:
- Reset controller with GCTL.CoreSoftReset
- Set GCTL.PrtCapDir(host mode)
- Reset the host with USBCMD.HCRESET
- Then follow up with the initializing host registers sequence
To switch from host to device:
- Reset controller with GCTL.CoreSoftReset
- Set GCTL.PrtCapDir(device mode)
- Reset the device with DCTL.CSftRst
- Then follow up with the initializing registers sequence
Currently we're missing step 1) to do GCTL.CoreSoftReset and step 3) of switching from host to device. John Stult reported a lockup issue seen with HiKey960 platform without these steps[1]. Similar issue is observed with Ferry's testing platform[2].
So, apply the required steps along with some fixes to Yu Chen's and John Stultz's version. The main fixes to their versions are the missing wait for clocks synchronization before clearing GCTL.CoreSoftReset and only apply DCTL.CSftRst when switching from host to device.
[1] https://lore.kernel.org/linux-usb/20210108015115.27920-1-john.stultz@linaro.... [2] https://lore.kernel.org/linux-usb/0ba7a6ba-e6a7-9cd4-0695-64fc927e01f1@gmail...
Cc: Andy Shevchenko andy.shevchenko@gmail.com Cc: Ferry Toth fntoth@gmail.com Cc: Wesley Cheng wcheng@codeaurora.org Cc: stable@vger.kernel.org Fixes: 41ce1456e1db ("usb: dwc3: core: make dwc3_set_mode() work properly") Signed-off-by: Yu Chen chenyu56@huawei.com Signed-off-by: John Stultz john.stultz@linaro.org Signed-off-by: Thinh Nguyen Thinh.Nguyen@synopsys.com
Changes in v3:
- Check if the desired mode is OTG, then keep the old flow
- Remove condition for OTG support only since the device can still be configured DRD host/device mode only
- Remove redundant hw_mode check since __dwc3_set_mode() only applies when hw_mode is DRD
Changes in v2:
- Initialize mutex per device and not as global mutex.
- Add additional checks for DRD only mode
I've not been able to test all the different modes on HiKey960 yet, but with this patch we avoid the !COREIDLE hangs that we see frequently on bootup, so it looks pretty good to me. I'll get back to you tonight when I can put hands on the board to test the gadget to host switching to make sure all is well (I really don't expect any issues, but just want to be sure).
Ok, got a chance to test the mode switching and everything is looking good.
Tested-by: John Stultz john.stultz@linaro.org
Thanks again for continuing to push this! -john
Hi
Op 16-04-2021 om 05:28 schreef John Stultz:
On Thu, Apr 15, 2021 at 5:12 PM John Stultzjohn.stultz@linaro.org wrote:
On Thu, Apr 15, 2021 at 3:20 PM Thinh NguyenThinh.Nguyen@synopsys.com wrote:
From: Yu Chenchenyu56@huawei.com From: John Stultzjohn.stultz@linaro.org
According to the programming guide, to switch mode for DRD controller, the driver needs to do the following.
To switch from device to host:
- Reset controller with GCTL.CoreSoftReset
- Set GCTL.PrtCapDir(host mode)
- Reset the host with USBCMD.HCRESET
- Then follow up with the initializing host registers sequence
To switch from host to device:
- Reset controller with GCTL.CoreSoftReset
- Set GCTL.PrtCapDir(device mode)
- Reset the device with DCTL.CSftRst
- Then follow up with the initializing registers sequence
Currently we're missing step 1) to do GCTL.CoreSoftReset and step 3) of switching from host to device. John Stult reported a lockup issue seen with HiKey960 platform without these steps[1]. Similar issue is observed with Ferry's testing platform[2].
So, apply the required steps along with some fixes to Yu Chen's and John Stultz's version. The main fixes to their versions are the missing wait for clocks synchronization before clearing GCTL.CoreSoftReset and only apply DCTL.CSftRst when switching from host to device.
[1]https://lore.kernel.org/linux-usb/20210108015115.27920-1-john.stultz@linaro.... [2]https://lore.kernel.org/linux-usb/0ba7a6ba-e6a7-9cd4-0695-64fc927e01f1@gmail...
Cc: Andy Shevchenkoandy.shevchenko@gmail.com Cc: Ferry Tothfntoth@gmail.com Cc: Wesley Chengwcheng@codeaurora.org Cc:stable@vger.kernel.org Fixes: 41ce1456e1db ("usb: dwc3: core: make dwc3_set_mode() work properly") Signed-off-by: Yu Chenchenyu56@huawei.com Signed-off-by: John Stultzjohn.stultz@linaro.org Signed-off-by: Thinh NguyenThinh.Nguyen@synopsys.com
Changes in v3:
- Check if the desired mode is OTG, then keep the old flow
- Remove condition for OTG support only since the device can still be configured DRD host/device mode only
- Remove redundant hw_mode check since __dwc3_set_mode() only applies when hw_mode is DRD
Changes in v2:
- Initialize mutex per device and not as global mutex.
- Add additional checks for DRD only mode
I've not been able to test all the different modes on HiKey960 yet, but with this patch we avoid the !COREIDLE hangs that we see frequently on bootup, so it looks pretty good to me. I'll get back to you tonight when I can put hands on the board to test the gadget to host switching to make sure all is well (I really don't expect any issues, but just want to be sure).
Ok, got a chance to test the mode switching and everything is looking good.
I expect to be able to test this weekend on my platform.
Tested-by: John Stultzjohn.stultz@linaro.org
Thanks again for continuing to push this! -john
Hi,
Thinh Nguyen Thinh.Nguyen@synopsys.com writes:
From: Yu Chen chenyu56@huawei.com From: John Stultz john.stultz@linaro.org
According to the programming guide, to switch mode for DRD controller, the driver needs to do the following.
To switch from device to host:
- Reset controller with GCTL.CoreSoftReset
- Set GCTL.PrtCapDir(host mode)
- Reset the host with USBCMD.HCRESET
- Then follow up with the initializing host registers sequence
To switch from host to device:
- Reset controller with GCTL.CoreSoftReset
- Set GCTL.PrtCapDir(device mode)
- Reset the device with DCTL.CSftRst
- Then follow up with the initializing registers sequence
Currently we're missing step 1) to do GCTL.CoreSoftReset and step 3) of switching from host to device. John Stult reported a lockup issue seen with HiKey960 platform without these steps[1]. Similar issue is observed with Ferry's testing platform[2].
So, apply the required steps along with some fixes to Yu Chen's and John Stultz's version. The main fixes to their versions are the missing wait for clocks synchronization before clearing GCTL.CoreSoftReset and only apply DCTL.CSftRst when switching from host to device.
[1] https://lore.kernel.org/linux-usb/20210108015115.27920-1-john.stultz@linaro.... [2] https://lore.kernel.org/linux-usb/0ba7a6ba-e6a7-9cd4-0695-64fc927e01f1@gmail...
Cc: Andy Shevchenko andy.shevchenko@gmail.com Cc: Ferry Toth fntoth@gmail.com Cc: Wesley Cheng wcheng@codeaurora.org Cc: stable@vger.kernel.org Fixes: 41ce1456e1db ("usb: dwc3: core: make dwc3_set_mode() work properly") Signed-off-by: Yu Chen chenyu56@huawei.com Signed-off-by: John Stultz john.stultz@linaro.org Signed-off-by: Thinh Nguyen Thinh.Nguyen@synopsys.com
I still have concerns about the soft reset, but I won't block you guys from fixing Hikey's problem :-)
The only thing I would like to confirm is that this has been verified with hundreds of swaps happening as quickly as possible. DWC3 should still be functional after several hundred swaps.
Can someone confirm this is the case? (I'm assuming this can be scripted)
On 4/16/2021 3:47 AM, Felipe Balbi wrote:
Hi,
Thinh Nguyen Thinh.Nguyen@synopsys.com writes:
From: Yu Chen chenyu56@huawei.com From: John Stultz john.stultz@linaro.org
According to the programming guide, to switch mode for DRD controller, the driver needs to do the following.
To switch from device to host:
- Reset controller with GCTL.CoreSoftReset
- Set GCTL.PrtCapDir(host mode)
- Reset the host with USBCMD.HCRESET
- Then follow up with the initializing host registers sequence
To switch from host to device:
- Reset controller with GCTL.CoreSoftReset
- Set GCTL.PrtCapDir(device mode)
- Reset the device with DCTL.CSftRst
- Then follow up with the initializing registers sequence
Currently we're missing step 1) to do GCTL.CoreSoftReset and step 3) of switching from host to device. John Stult reported a lockup issue seen with HiKey960 platform without these steps[1]. Similar issue is observed with Ferry's testing platform[2].
So, apply the required steps along with some fixes to Yu Chen's and John Stultz's version. The main fixes to their versions are the missing wait for clocks synchronization before clearing GCTL.CoreSoftReset and only apply DCTL.CSftRst when switching from host to device.
[1] https://lore.kernel.org/linux-usb/20210108015115.27920-1-john.stultz@linaro.... [2] https://lore.kernel.org/linux-usb/0ba7a6ba-e6a7-9cd4-0695-64fc927e01f1@gmail...
Cc: Andy Shevchenko andy.shevchenko@gmail.com Cc: Ferry Toth fntoth@gmail.com Cc: Wesley Cheng wcheng@codeaurora.org Cc: stable@vger.kernel.org Fixes: 41ce1456e1db ("usb: dwc3: core: make dwc3_set_mode() work properly") Signed-off-by: Yu Chen chenyu56@huawei.com Signed-off-by: John Stultz john.stultz@linaro.org Signed-off-by: Thinh Nguyen Thinh.Nguyen@synopsys.com
I still have concerns about the soft reset, but I won't block you guys from fixing Hikey's problem :-)
The only thing I would like to confirm is that this has been verified with hundreds of swaps happening as quickly as possible. DWC3 should still be functional after several hundred swaps.
Can someone confirm this is the case? (I'm assuming this can be scripted)
Hi Thinh/Felipe,
Thanks Thinh for this change. Will verify this on our platform as well with a mode switch loop over the weekend.
Thanks Wesley Cheng
On Fri, Apr 16, 2021 at 3:47 AM Felipe Balbi balbi@kernel.org wrote:
Hi,
Thinh Nguyen Thinh.Nguyen@synopsys.com writes:
From: Yu Chen chenyu56@huawei.com From: John Stultz john.stultz@linaro.org
According to the programming guide, to switch mode for DRD controller, the driver needs to do the following.
To switch from device to host:
- Reset controller with GCTL.CoreSoftReset
- Set GCTL.PrtCapDir(host mode)
- Reset the host with USBCMD.HCRESET
- Then follow up with the initializing host registers sequence
To switch from host to device:
- Reset controller with GCTL.CoreSoftReset
- Set GCTL.PrtCapDir(device mode)
- Reset the device with DCTL.CSftRst
- Then follow up with the initializing registers sequence
Currently we're missing step 1) to do GCTL.CoreSoftReset and step 3) of switching from host to device. John Stult reported a lockup issue seen with HiKey960 platform without these steps[1]. Similar issue is observed with Ferry's testing platform[2].
So, apply the required steps along with some fixes to Yu Chen's and John Stultz's version. The main fixes to their versions are the missing wait for clocks synchronization before clearing GCTL.CoreSoftReset and only apply DCTL.CSftRst when switching from host to device.
[1] https://lore.kernel.org/linux-usb/20210108015115.27920-1-john.stultz@linaro.... [2] https://lore.kernel.org/linux-usb/0ba7a6ba-e6a7-9cd4-0695-64fc927e01f1@gmail...
Cc: Andy Shevchenko andy.shevchenko@gmail.com Cc: Ferry Toth fntoth@gmail.com Cc: Wesley Cheng wcheng@codeaurora.org Cc: stable@vger.kernel.org Fixes: 41ce1456e1db ("usb: dwc3: core: make dwc3_set_mode() work properly") Signed-off-by: Yu Chen chenyu56@huawei.com Signed-off-by: John Stultz john.stultz@linaro.org Signed-off-by: Thinh Nguyen Thinh.Nguyen@synopsys.com
I still have concerns about the soft reset, but I won't block you guys from fixing Hikey's problem :-)
The only thing I would like to confirm is that this has been verified with hundreds of swaps happening as quickly as possible. DWC3 should still be functional after several hundred swaps.
Can someone confirm this is the case? (I'm assuming this can be scripted)
I unfortunately don't have an easy way to automate the switching right off. But I'll try to hack up the mux switch driver to provide an interface we can script against.
thanks -john
John Stultz wrote:
On Fri, Apr 16, 2021 at 3:47 AM Felipe Balbi balbi@kernel.org wrote:
Hi,
Thinh Nguyen Thinh.Nguyen@synopsys.com writes:
From: Yu Chen chenyu56@huawei.com From: John Stultz john.stultz@linaro.org
According to the programming guide, to switch mode for DRD controller, the driver needs to do the following.
To switch from device to host:
- Reset controller with GCTL.CoreSoftReset
- Set GCTL.PrtCapDir(host mode)
- Reset the host with USBCMD.HCRESET
- Then follow up with the initializing host registers sequence
To switch from host to device:
- Reset controller with GCTL.CoreSoftReset
- Set GCTL.PrtCapDir(device mode)
- Reset the device with DCTL.CSftRst
- Then follow up with the initializing registers sequence
Currently we're missing step 1) to do GCTL.CoreSoftReset and step 3) of switching from host to device. John Stult reported a lockup issue seen with HiKey960 platform without these steps[1]. Similar issue is observed with Ferry's testing platform[2].
So, apply the required steps along with some fixes to Yu Chen's and John Stultz's version. The main fixes to their versions are the missing wait for clocks synchronization before clearing GCTL.CoreSoftReset and only apply DCTL.CSftRst when switching from host to device.
[1] https://urldefense.com/v3/__https://lore.kernel.org/linux-usb/20210108015115... [2] https://urldefense.com/v3/__https://lore.kernel.org/linux-usb/0ba7a6ba-e6a7-...
Cc: Andy Shevchenko andy.shevchenko@gmail.com Cc: Ferry Toth fntoth@gmail.com Cc: Wesley Cheng wcheng@codeaurora.org Cc: stable@vger.kernel.org Fixes: 41ce1456e1db ("usb: dwc3: core: make dwc3_set_mode() work properly") Signed-off-by: Yu Chen chenyu56@huawei.com Signed-off-by: John Stultz john.stultz@linaro.org Signed-off-by: Thinh Nguyen Thinh.Nguyen@synopsys.com
I still have concerns about the soft reset, but I won't block you guys from fixing Hikey's problem :-)
The only thing I would like to confirm is that this has been verified with hundreds of swaps happening as quickly as possible. DWC3 should still be functional after several hundred swaps.
Can someone confirm this is the case? (I'm assuming this can be scripted)
I unfortunately don't have an easy way to automate the switching right off. But I'll try to hack up the mux switch driver to provide an interface we can script against.
FYI, you can do the following:
1) Enable "usb-role-switch" DT property if not already done so 2) Add userspace control:
diff --git a/drivers/usb/dwc3/drd.c b/drivers/usb/dwc3/drd.c index e2b68bb770d1..b203e3d87291 100644 --- a/drivers/usb/dwc3/drd.c +++ b/drivers/usb/dwc3/drd.c @@ -555,6 +555,7 @@ static int dwc3_setup_role_switch(struct dwc3 *dwc) mode = DWC3_GCTL_PRTCAP_DEVICE; }
+ dwc3_role_switch.allow_userspace_control = true; dwc3_role_switch.fwnode = dev_fwnode(dwc->dev); dwc3_role_switch.set = dwc3_usb_role_switch_set; dwc3_role_switch.get = dwc3_usb_role_switch_get;
3) Write a script to do the following:
# echo host > /sys/class/usb_role/<UDC>/role
and
# echo device > /sys/class/usb_role/<UDC>/role
BR, Thinh
On Fri, Apr 16, 2021 at 12:49 PM Thinh Nguyen Thinh.Nguyen@synopsys.com wrote:
John Stultz wrote:
On Fri, Apr 16, 2021 at 3:47 AM Felipe Balbi balbi@kernel.org wrote:
Hi,
Thinh Nguyen Thinh.Nguyen@synopsys.com writes:
From: Yu Chen chenyu56@huawei.com From: John Stultz john.stultz@linaro.org
According to the programming guide, to switch mode for DRD controller, the driver needs to do the following.
To switch from device to host:
- Reset controller with GCTL.CoreSoftReset
- Set GCTL.PrtCapDir(host mode)
- Reset the host with USBCMD.HCRESET
- Then follow up with the initializing host registers sequence
To switch from host to device:
- Reset controller with GCTL.CoreSoftReset
- Set GCTL.PrtCapDir(device mode)
- Reset the device with DCTL.CSftRst
- Then follow up with the initializing registers sequence
Currently we're missing step 1) to do GCTL.CoreSoftReset and step 3) of switching from host to device. John Stult reported a lockup issue seen with HiKey960 platform without these steps[1]. Similar issue is observed with Ferry's testing platform[2].
So, apply the required steps along with some fixes to Yu Chen's and John Stultz's version. The main fixes to their versions are the missing wait for clocks synchronization before clearing GCTL.CoreSoftReset and only apply DCTL.CSftRst when switching from host to device.
[1] https://urldefense.com/v3/__https://lore.kernel.org/linux-usb/20210108015115... [2] https://urldefense.com/v3/__https://lore.kernel.org/linux-usb/0ba7a6ba-e6a7-...
Cc: Andy Shevchenko andy.shevchenko@gmail.com Cc: Ferry Toth fntoth@gmail.com Cc: Wesley Cheng wcheng@codeaurora.org Cc: stable@vger.kernel.org Fixes: 41ce1456e1db ("usb: dwc3: core: make dwc3_set_mode() work properly") Signed-off-by: Yu Chen chenyu56@huawei.com Signed-off-by: John Stultz john.stultz@linaro.org Signed-off-by: Thinh Nguyen Thinh.Nguyen@synopsys.com
I still have concerns about the soft reset, but I won't block you guys from fixing Hikey's problem :-)
The only thing I would like to confirm is that this has been verified with hundreds of swaps happening as quickly as possible. DWC3 should still be functional after several hundred swaps.
Can someone confirm this is the case? (I'm assuming this can be scripted)
I unfortunately don't have an easy way to automate the switching right off. But I'll try to hack up the mux switch driver to provide an interface we can script against.
FYI, you can do the following:
- Enable "usb-role-switch" DT property if not already done so
- Add userspace control:
diff --git a/drivers/usb/dwc3/drd.c b/drivers/usb/dwc3/drd.c index e2b68bb770d1..b203e3d87291 100644 --- a/drivers/usb/dwc3/drd.c +++ b/drivers/usb/dwc3/drd.c @@ -555,6 +555,7 @@ static int dwc3_setup_role_switch(struct dwc3 *dwc) mode = DWC3_GCTL_PRTCAP_DEVICE; }
dwc3_role_switch.allow_userspace_control = true; dwc3_role_switch.fwnode = dev_fwnode(dwc->dev); dwc3_role_switch.set = dwc3_usb_role_switch_set; dwc3_role_switch.get = dwc3_usb_role_switch_get;
- Write a script to do the following:
# echo host > /sys/class/usb_role/<UDC>/role
and
# echo device > /sys/class/usb_role/<UDC>/role
Thanks so much for this. So I ran both of those commands in a while loop for awhile and didn't see any trouble.
HiKey960 is interesting as well because we have a mux switch, which is sort of an intermediary roll switcher (it gets the role switch signal from the tcpci_rt1711h, tweaks some gpios and then signals the dwc3). So I also did the above tweaks to the mux-switch and had it switching between device/none and validated the onboard hub came up and down along with the dwc3 core.
Everything still looks good here.
thanks -john
thanks -john
Hi,
John Stultz john.stultz@linaro.org writes:
On Fri, Apr 16, 2021 at 12:49 PM Thinh Nguyen Thinh.Nguyen@synopsys.com wrote:
John Stultz wrote:
On Fri, Apr 16, 2021 at 3:47 AM Felipe Balbi balbi@kernel.org wrote:
Hi,
Thinh Nguyen Thinh.Nguyen@synopsys.com writes:
From: Yu Chen chenyu56@huawei.com From: John Stultz john.stultz@linaro.org
According to the programming guide, to switch mode for DRD controller, the driver needs to do the following.
To switch from device to host:
- Reset controller with GCTL.CoreSoftReset
- Set GCTL.PrtCapDir(host mode)
- Reset the host with USBCMD.HCRESET
- Then follow up with the initializing host registers sequence
To switch from host to device:
- Reset controller with GCTL.CoreSoftReset
- Set GCTL.PrtCapDir(device mode)
- Reset the device with DCTL.CSftRst
- Then follow up with the initializing registers sequence
Currently we're missing step 1) to do GCTL.CoreSoftReset and step 3) of switching from host to device. John Stult reported a lockup issue seen with HiKey960 platform without these steps[1]. Similar issue is observed with Ferry's testing platform[2].
So, apply the required steps along with some fixes to Yu Chen's and John Stultz's version. The main fixes to their versions are the missing wait for clocks synchronization before clearing GCTL.CoreSoftReset and only apply DCTL.CSftRst when switching from host to device.
[1] https://urldefense.com/v3/__https://lore.kernel.org/linux-usb/20210108015115... [2] https://urldefense.com/v3/__https://lore.kernel.org/linux-usb/0ba7a6ba-e6a7-...
Cc: Andy Shevchenko andy.shevchenko@gmail.com Cc: Ferry Toth fntoth@gmail.com Cc: Wesley Cheng wcheng@codeaurora.org Cc: stable@vger.kernel.org Fixes: 41ce1456e1db ("usb: dwc3: core: make dwc3_set_mode() work properly") Signed-off-by: Yu Chen chenyu56@huawei.com Signed-off-by: John Stultz john.stultz@linaro.org Signed-off-by: Thinh Nguyen Thinh.Nguyen@synopsys.com
I still have concerns about the soft reset, but I won't block you guys from fixing Hikey's problem :-)
The only thing I would like to confirm is that this has been verified with hundreds of swaps happening as quickly as possible. DWC3 should still be functional after several hundred swaps.
Can someone confirm this is the case? (I'm assuming this can be scripted)
I unfortunately don't have an easy way to automate the switching right off. But I'll try to hack up the mux switch driver to provide an interface we can script against.
FYI, you can do the following:
- Enable "usb-role-switch" DT property if not already done so
- Add userspace control:
diff --git a/drivers/usb/dwc3/drd.c b/drivers/usb/dwc3/drd.c index e2b68bb770d1..b203e3d87291 100644 --- a/drivers/usb/dwc3/drd.c +++ b/drivers/usb/dwc3/drd.c @@ -555,6 +555,7 @@ static int dwc3_setup_role_switch(struct dwc3 *dwc) mode = DWC3_GCTL_PRTCAP_DEVICE; }
dwc3_role_switch.allow_userspace_control = true; dwc3_role_switch.fwnode = dev_fwnode(dwc->dev); dwc3_role_switch.set = dwc3_usb_role_switch_set; dwc3_role_switch.get = dwc3_usb_role_switch_get;
- Write a script to do the following:
# echo host > /sys/class/usb_role/<UDC>/role
and
# echo device > /sys/class/usb_role/<UDC>/role
Thanks so much for this. So I ran both of those commands in a while loop for awhile and didn't see any trouble.
HiKey960 is interesting as well because we have a mux switch, which is sort of an intermediary roll switcher (it gets the role switch signal from the tcpci_rt1711h, tweaks some gpios and then signals the dwc3). So I also did the above tweaks to the mux-switch and had it switching between device/none and validated the onboard hub came up and down along with the dwc3 core.
Everything still looks good here.
Sounds good, happy to see so many platforms supported by Thinh's change. Thanks for doing this work, Thinh :-)
Felipe Balbi wrote:
Hi,
John Stultz john.stultz@linaro.org writes:
On Fri, Apr 16, 2021 at 12:49 PM Thinh Nguyen Thinh.Nguyen@synopsys.com wrote:
John Stultz wrote:
On Fri, Apr 16, 2021 at 3:47 AM Felipe Balbi balbi@kernel.org wrote:
Hi,
Thinh Nguyen Thinh.Nguyen@synopsys.com writes:
From: Yu Chen chenyu56@huawei.com From: John Stultz john.stultz@linaro.org
According to the programming guide, to switch mode for DRD controller, the driver needs to do the following.
To switch from device to host:
- Reset controller with GCTL.CoreSoftReset
- Set GCTL.PrtCapDir(host mode)
- Reset the host with USBCMD.HCRESET
- Then follow up with the initializing host registers sequence
To switch from host to device:
- Reset controller with GCTL.CoreSoftReset
- Set GCTL.PrtCapDir(device mode)
- Reset the device with DCTL.CSftRst
- Then follow up with the initializing registers sequence
Currently we're missing step 1) to do GCTL.CoreSoftReset and step 3) of switching from host to device. John Stult reported a lockup issue seen with HiKey960 platform without these steps[1]. Similar issue is observed with Ferry's testing platform[2].
So, apply the required steps along with some fixes to Yu Chen's and John Stultz's version. The main fixes to their versions are the missing wait for clocks synchronization before clearing GCTL.CoreSoftReset and only apply DCTL.CSftRst when switching from host to device.
[1] https://urldefense.com/v3/__https://lore.kernel.org/linux-usb/20210108015115... [2] https://urldefense.com/v3/__https://lore.kernel.org/linux-usb/0ba7a6ba-e6a7-...
Cc: Andy Shevchenko andy.shevchenko@gmail.com Cc: Ferry Toth fntoth@gmail.com Cc: Wesley Cheng wcheng@codeaurora.org Cc: stable@vger.kernel.org Fixes: 41ce1456e1db ("usb: dwc3: core: make dwc3_set_mode() work properly") Signed-off-by: Yu Chen chenyu56@huawei.com Signed-off-by: John Stultz john.stultz@linaro.org Signed-off-by: Thinh Nguyen Thinh.Nguyen@synopsys.com
I still have concerns about the soft reset, but I won't block you guys from fixing Hikey's problem :-)
The only thing I would like to confirm is that this has been verified with hundreds of swaps happening as quickly as possible. DWC3 should still be functional after several hundred swaps.
Can someone confirm this is the case? (I'm assuming this can be scripted)
I unfortunately don't have an easy way to automate the switching right off. But I'll try to hack up the mux switch driver to provide an interface we can script against.
FYI, you can do the following:
- Enable "usb-role-switch" DT property if not already done so
- Add userspace control:
diff --git a/drivers/usb/dwc3/drd.c b/drivers/usb/dwc3/drd.c index e2b68bb770d1..b203e3d87291 100644 --- a/drivers/usb/dwc3/drd.c +++ b/drivers/usb/dwc3/drd.c @@ -555,6 +555,7 @@ static int dwc3_setup_role_switch(struct dwc3 *dwc) mode = DWC3_GCTL_PRTCAP_DEVICE; }
dwc3_role_switch.allow_userspace_control = true; dwc3_role_switch.fwnode = dev_fwnode(dwc->dev); dwc3_role_switch.set = dwc3_usb_role_switch_set; dwc3_role_switch.get = dwc3_usb_role_switch_get;
- Write a script to do the following:
# echo host > /sys/class/usb_role/<UDC>/role
and
# echo device > /sys/class/usb_role/<UDC>/role
Thanks so much for this. So I ran both of those commands in a while loop for awhile and didn't see any trouble.
HiKey960 is interesting as well because we have a mux switch, which is sort of an intermediary roll switcher (it gets the role switch signal from the tcpci_rt1711h, tweaks some gpios and then signals the dwc3). So I also did the above tweaks to the mux-switch and had it switching between device/none and validated the onboard hub came up and down along with the dwc3 core.
Everything still looks good here.
Sounds good, happy to see so many platforms supported by Thinh's change. Thanks for doing this work, Thinh :-)
Thanks for the review Felipe :)
Thinh
On 4/15/2021 3:20 PM, Thinh Nguyen wrote:
From: Yu Chen chenyu56@huawei.com From: John Stultz john.stultz@linaro.org
According to the programming guide, to switch mode for DRD controller, the driver needs to do the following.
To switch from device to host:
- Reset controller with GCTL.CoreSoftReset
- Set GCTL.PrtCapDir(host mode)
- Reset the host with USBCMD.HCRESET
- Then follow up with the initializing host registers sequence
To switch from host to device:
- Reset controller with GCTL.CoreSoftReset
- Set GCTL.PrtCapDir(device mode)
- Reset the device with DCTL.CSftRst
- Then follow up with the initializing registers sequence
Currently we're missing step 1) to do GCTL.CoreSoftReset and step 3) of switching from host to device. John Stult reported a lockup issue seen with HiKey960 platform without these steps[1]. Similar issue is observed with Ferry's testing platform[2].
So, apply the required steps along with some fixes to Yu Chen's and John Stultz's version. The main fixes to their versions are the missing wait for clocks synchronization before clearing GCTL.CoreSoftReset and only apply DCTL.CSftRst when switching from host to device.
[1] https://lore.kernel.org/linux-usb/20210108015115.27920-1-john.stultz@linaro.... [2] https://lore.kernel.org/linux-usb/0ba7a6ba-e6a7-9cd4-0695-64fc927e01f1@gmail...
Cc: Andy Shevchenko andy.shevchenko@gmail.com Cc: Ferry Toth fntoth@gmail.com Cc: Wesley Cheng wcheng@codeaurora.org Cc: stable@vger.kernel.org Fixes: 41ce1456e1db ("usb: dwc3: core: make dwc3_set_mode() work properly") Signed-off-by: Yu Chen chenyu56@huawei.com Signed-off-by: John Stultz john.stultz@linaro.org Signed-off-by: Thinh Nguyen Thinh.Nguyen@synopsys.com
Changes in v3:
- Check if the desired mode is OTG, then keep the old flow
- Remove condition for OTG support only since the device can still be configured DRD host/device mode only
- Remove redundant hw_mode check since __dwc3_set_mode() only applies when hw_mode is DRD
Changes in v2:
- Initialize mutex per device and not as global mutex.
- Add additional checks for DRD only mode
drivers/usb/dwc3/core.c | 27 +++++++++++++++++++++++++++ drivers/usb/dwc3/core.h | 5 +++++ 2 files changed, 32 insertions(+)
diff --git a/drivers/usb/dwc3/core.c b/drivers/usb/dwc3/core.c index 5c25e6a72dbd..2f118ad43571 100644 --- a/drivers/usb/dwc3/core.c +++ b/drivers/usb/dwc3/core.c @@ -114,6 +114,8 @@ void dwc3_set_prtcap(struct dwc3 *dwc, u32 mode) dwc->current_dr_role = mode; } +static int dwc3_core_soft_reset(struct dwc3 *dwc);
static void __dwc3_set_mode(struct work_struct *work) { struct dwc3 *dwc = work_to_dwc(work); @@ -121,6 +123,8 @@ static void __dwc3_set_mode(struct work_struct *work) int ret; u32 reg;
- mutex_lock(&dwc->mutex);
- pm_runtime_get_sync(dwc->dev);
if (dwc->current_dr_role == DWC3_GCTL_PRTCAP_OTG) @@ -154,6 +158,25 @@ static void __dwc3_set_mode(struct work_struct *work) break; }
- /* For DRD host or device mode only */
- if (dwc->desired_dr_role != DWC3_GCTL_PRTCAP_OTG) {
reg = dwc3_readl(dwc->regs, DWC3_GCTL);reg |= DWC3_GCTL_CORESOFTRESET;dwc3_writel(dwc->regs, DWC3_GCTL, reg);/** Wait for internal clocks to synchronized. DWC_usb31 and* DWC_usb32 may need at least 50ms (less for DWC_usb3). To* keep it consistent across different IPs, let's wait up to* 100ms before clearing GCTL.CORESOFTRESET.*/msleep(100);reg = dwc3_readl(dwc->regs, DWC3_GCTL);reg &= ~DWC3_GCTL_CORESOFTRESET;dwc3_writel(dwc->regs, DWC3_GCTL, reg);- }
- spin_lock_irqsave(&dwc->lock, flags);
dwc3_set_prtcap(dwc, dwc->desired_dr_role); @@ -178,6 +201,8 @@ static void __dwc3_set_mode(struct work_struct *work) } break; case DWC3_GCTL_PRTCAP_DEVICE:
dwc3_core_soft_reset(dwc);- dwc3_event_buffers_setup(dwc);
if (dwc->usb2_phy) @@ -200,6 +225,7 @@ static void __dwc3_set_mode(struct work_struct *work) out: pm_runtime_mark_last_busy(dwc->dev); pm_runtime_put_autosuspend(dwc->dev);
- mutex_unlock(&dwc->mutex);
} void dwc3_set_mode(struct dwc3 *dwc, u32 mode) @@ -1553,6 +1579,7 @@ static int dwc3_probe(struct platform_device *pdev) dwc3_cache_hwparams(dwc); spin_lock_init(&dwc->lock);
- mutex_init(&dwc->mutex);
pm_runtime_set_active(dev); pm_runtime_use_autosuspend(dev); diff --git a/drivers/usb/dwc3/core.h b/drivers/usb/dwc3/core.h index 695ff2d791e4..7e3afa5378e8 100644 --- a/drivers/usb/dwc3/core.h +++ b/drivers/usb/dwc3/core.h @@ -13,6 +13,7 @@ #include <linux/device.h> #include <linux/spinlock.h> +#include <linux/mutex.h> #include <linux/ioport.h> #include <linux/list.h> #include <linux/bitops.h> @@ -947,6 +948,7 @@ struct dwc3_scratchpad_array {
- @scratch_addr: dma address of scratchbuf
- @ep0_in_setup: one control transfer is completed and enter setup phase
- @lock: for synchronizing
- @mutex: for mode switching
- @dev: pointer to our struct device
- @sysdev: pointer to the DMA-capable device
- @xhci: pointer to our xHCI child
@@ -1088,6 +1090,9 @@ struct dwc3 { /* device lock */ spinlock_t lock;
- /* mode switching lock */
- struct mutex mutex;
- struct device *dev; struct device *sysdev;
base-commit: 4b853c236c7b5161a2e444bd8b3c76fe5aa5ddcb
Hi Thinh,
Thanks for this change! Tested this on our platform w/ a DRD mode switch loop and it looks good.
Tested-by: Wesley Cheng wcheng@codeaurora.org
Thanks Wesley Cheng
linux-stable-mirror@lists.linaro.org
-
Andy Shevchenko -
Felipe Balbi -
Ferry Toth -
Greg Kroah-Hartman -
John Stultz -
Thinh Nguyen -
Wesley Cheng