Re: [PATCH v2] usb: dwc3: core: fix a issue about clear connect state
On Mon, Oct 19, 2020 at 10:04:41PM +, Thinh Nguyen wrote: > Dejin Zheng wrote: Hi Thinh: > > According to Synopsys Programming Guide chapter 2.2 Register Resets, > > it cannot reset the DCTL register by setting DCTL.CSFTRST for core soft > > reset, if DWC3 controller as a slave device and stay connected with a usb > > host, then, while rebooting linux, it will fail to reinitialize dwc3 as a > > slave device when the DWC3 controller did not power off. > > If you reboot the OS, wouldn't it go through the driver tear-down > sequence and clear the run_stop bit anyway? Yes, you are right, this is a point worth checking. and I think it might still be necessary to reset it. > However, I can see how this can be an issue. > > > because the > > connection status is incorrect, so we also need to clear DCTL.RUN_STOP > > bit for disabling connect when doing core soft reset. > > > > Fixes: f59dcab176293b6 ("usb: dwc3: core: improve reset sequence") > > Signed-off-by: Dejin Zheng > > --- > > v1 -> v2: > > * modify some commit messages by Sergei's suggest, Thanks > > very much for Sergei's help! > > > > drivers/usb/dwc3/core.c | 1 + > > 1 file changed, 1 insertion(+) > > > > diff --git a/drivers/usb/dwc3/core.c b/drivers/usb/dwc3/core.c > > index 2eb34c8b4065..239636c454c2 100644 > > --- a/drivers/usb/dwc3/core.c > > +++ b/drivers/usb/dwc3/core.c > > @@ -256,6 +256,7 @@ static int dwc3_core_soft_reset(struct dwc3 *dwc) > > > > reg = dwc3_readl(dwc->regs, DWC3_DCTL); > > reg |= DWC3_DCTL_CSFTRST; > > + reg &= ~DWC3_DCTL_RUN_STOP; > > dwc3_writel(dwc->regs, DWC3_DCTL, reg); > > > > /* > > There will still be other stale configuration in DCTL if you do this. I > think it's better to reset the other fields of DCTL to the default > (should be all 0s) instead of doing register read-modify-write as what > we're doing here. If not, at least we should use > dwc3_gadget_dctl_write_safe(). > Thinh, thanks very much for your suggestion, I think it might be better to reset all areas of DCTL register. I tested it on my SOC platform and it worked. BR, Dejin > Thanks, > Thinh >
Re: [PATCH v2] usb: dwc3: core: fix a issue about clear connect state
Dejin Zheng wrote: > According to Synopsys Programming Guide chapter 2.2 Register Resets, > it cannot reset the DCTL register by setting DCTL.CSFTRST for core soft > reset, if DWC3 controller as a slave device and stay connected with a usb > host, then, while rebooting linux, it will fail to reinitialize dwc3 as a > slave device when the DWC3 controller did not power off. If you reboot the OS, wouldn't it go through the driver tear-down sequence and clear the run_stop bit anyway? However, I can see how this can be an issue. > because the > connection status is incorrect, so we also need to clear DCTL.RUN_STOP > bit for disabling connect when doing core soft reset. > > Fixes: f59dcab176293b6 ("usb: dwc3: core: improve reset sequence") > Signed-off-by: Dejin Zheng > --- > v1 -> v2: > * modify some commit messages by Sergei's suggest, Thanks > very much for Sergei's help! > > drivers/usb/dwc3/core.c | 1 + > 1 file changed, 1 insertion(+) > > diff --git a/drivers/usb/dwc3/core.c b/drivers/usb/dwc3/core.c > index 2eb34c8b4065..239636c454c2 100644 > --- a/drivers/usb/dwc3/core.c > +++ b/drivers/usb/dwc3/core.c > @@ -256,6 +256,7 @@ static int dwc3_core_soft_reset(struct dwc3 *dwc) > > reg = dwc3_readl(dwc->regs, DWC3_DCTL); > reg |= DWC3_DCTL_CSFTRST; > + reg &= ~DWC3_DCTL_RUN_STOP; > dwc3_writel(dwc->regs, DWC3_DCTL, reg); > > /* There will still be other stale configuration in DCTL if you do this. I think it's better to reset the other fields of DCTL to the default (should be all 0s) instead of doing register read-modify-write as what we're doing here. If not, at least we should use dwc3_gadget_dctl_write_safe(). Thanks, Thinh
[PATCH v2] usb: dwc3: core: fix a issue about clear connect state
According to Synopsys Programming Guide chapter 2.2 Register Resets, it cannot reset the DCTL register by setting DCTL.CSFTRST for core soft reset, if DWC3 controller as a slave device and stay connected with a usb host, then, while rebooting linux, it will fail to reinitialize dwc3 as a slave device when the DWC3 controller did not power off. because the connection status is incorrect, so we also need to clear DCTL.RUN_STOP bit for disabling connect when doing core soft reset. Fixes: f59dcab176293b6 ("usb: dwc3: core: improve reset sequence") Signed-off-by: Dejin Zheng --- v1 -> v2: * modify some commit messages by Sergei's suggest, Thanks very much for Sergei's help! drivers/usb/dwc3/core.c | 1 + 1 file changed, 1 insertion(+) diff --git a/drivers/usb/dwc3/core.c b/drivers/usb/dwc3/core.c index 2eb34c8b4065..239636c454c2 100644 --- a/drivers/usb/dwc3/core.c +++ b/drivers/usb/dwc3/core.c @@ -256,6 +256,7 @@ static int dwc3_core_soft_reset(struct dwc3 *dwc) reg = dwc3_readl(dwc->regs, DWC3_DCTL); reg |= DWC3_DCTL_CSFTRST; + reg &= ~DWC3_DCTL_RUN_STOP; dwc3_writel(dwc->regs, DWC3_DCTL, reg); /* -- 2.25.0