On 4/15/2021 3:20 PM, Thinh Nguyen wrote: > From: Yu Chen <chenyu56@xxxxxxxxxx> > From: John Stultz <john.stultz@xxxxxxxxxx> > > 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@xxxxxxxxxx/ > [2] https://lore.kernel.org/linux-usb/0ba7a6ba-e6a7-9cd4-0695-64fc927e01f1@xxxxxxxxx/ > > Cc: Andy Shevchenko <andy.shevchenko@xxxxxxxxx> > Cc: Ferry Toth <fntoth@xxxxxxxxx> > Cc: Wesley Cheng <wcheng@xxxxxxxxxxxxxx> > Cc: <stable@xxxxxxxxxxxxxxx> > Fixes: 41ce1456e1db ("usb: dwc3: core: make dwc3_set_mode() work properly") > Signed-off-by: Yu Chen <chenyu56@xxxxxxxxxx> > Signed-off-by: John Stultz <john.stultz@xxxxxxxxxx> > Signed-off-by: Thinh Nguyen <Thinh.Nguyen@xxxxxxxxxxxx> > --- > 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@xxxxxxxxxxxxxx> Thanks Wesley Cheng -- The Qualcomm Innovation Center, Inc. is a member of the Code Aurora Forum, a Linux Foundation Collaborative Project
