| Message ID | 20220216000835.25400-4-quic_wcheng@quicinc.com |
|---|---|
| State | Superseded |
| Headers | show |
| Series | Fix enumeration issues during composition switching | expand |
Hi wesley, On Tue, Feb 15, 2022 at 04:08:35PM -0800, Wesley Cheng wrote: > It is recommended by the Synopsis databook to issue a DCTL.CSftReset > when reconnecting from a device-initiated disconnect routine. This > resolves issues with enumeration during fast composition switching > cases, which result in an unknown device on the host. > > Signed-off-by: Wesley Cheng <quic_wcheng@quicinc.com> > --- > drivers/usb/dwc3/core.c | 4 +--- > drivers/usb/dwc3/core.h | 2 ++ > drivers/usb/dwc3/gadget.c | 11 +++++++++++ > 3 files changed, 14 insertions(+), 3 deletions(-) > > diff --git a/drivers/usb/dwc3/core.c b/drivers/usb/dwc3/core.c > index 18adddfba3da..02d10e1cb774 100644 > --- a/drivers/usb/dwc3/core.c > +++ b/drivers/usb/dwc3/core.c > @@ -115,8 +115,6 @@ 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); > @@ -261,7 +259,7 @@ u32 dwc3_core_fifo_space(struct dwc3_ep *dep, u8 type) > * dwc3_core_soft_reset - Issues core soft reset and PHY reset > * @dwc: pointer to our context structure > */ > -static int dwc3_core_soft_reset(struct dwc3 *dwc) > +int dwc3_core_soft_reset(struct dwc3 *dwc) > { > u32 reg; > int retries = 1000; > diff --git a/drivers/usb/dwc3/core.h b/drivers/usb/dwc3/core.h > index 00348d6d479b..b27ad8dad317 100644 > --- a/drivers/usb/dwc3/core.h > +++ b/drivers/usb/dwc3/core.h > @@ -1532,6 +1532,8 @@ bool dwc3_has_imod(struct dwc3 *dwc); > int dwc3_event_buffers_setup(struct dwc3 *dwc); > void dwc3_event_buffers_cleanup(struct dwc3 *dwc); > > +int dwc3_core_soft_reset(struct dwc3 *dwc); > + > #if IS_ENABLED(CONFIG_USB_DWC3_HOST) || IS_ENABLED(CONFIG_USB_DWC3_DUAL_ROLE) > int dwc3_host_init(struct dwc3 *dwc); > void dwc3_host_exit(struct dwc3 *dwc); > diff --git a/drivers/usb/dwc3/gadget.c b/drivers/usb/dwc3/gadget.c > index 0c89baedf220..788889f924f9 100644 > --- a/drivers/usb/dwc3/gadget.c > +++ b/drivers/usb/dwc3/gadget.c > @@ -2585,6 +2585,17 @@ static int dwc3_gadget_pullup(struct usb_gadget *g, int is_on) > dwc->ev_buf->length; > } > } else { > + /* > + * In the Synopsis DesignWare Cores USB3 Databook Rev. 1.90a > + * Section 4.1.9, it specifies that for a reconnect after a > + * device-initiated disconnect requires a core soft reset > + * (DCTL.CSftRst) before enabling the run/stop bit. > + */ > + spin_unlock_irqrestore(&dwc->lock, flags); > + dwc3_core_soft_reset(dwc); > + spin_lock_irqsave(&dwc->lock, flags); > + > + dwc3_event_buffers_setup(dwc); Could you tell me why you add dwc3_event_buffer_setup? Best Regards, Jung Daehwan > __dwc3_gadget_start(dwc); > } > >
On Thu, Feb 17, 2022 at 02:52:14PM +0900, Jung Daehwan wrote: > Hi wesley, > > On Tue, Feb 15, 2022 at 04:08:35PM -0800, Wesley Cheng wrote: > > It is recommended by the Synopsis databook to issue a DCTL.CSftReset > > when reconnecting from a device-initiated disconnect routine. This > > resolves issues with enumeration during fast composition switching > > cases, which result in an unknown device on the host. > > > > Signed-off-by: Wesley Cheng <quic_wcheng@quicinc.com> > > --- > > drivers/usb/dwc3/core.c | 4 +--- > > drivers/usb/dwc3/core.h | 2 ++ > > drivers/usb/dwc3/gadget.c | 11 +++++++++++ > > 3 files changed, 14 insertions(+), 3 deletions(-) > > > > diff --git a/drivers/usb/dwc3/core.c b/drivers/usb/dwc3/core.c > > index 18adddfba3da..02d10e1cb774 100644 > > --- a/drivers/usb/dwc3/core.c > > +++ b/drivers/usb/dwc3/core.c > > @@ -115,8 +115,6 @@ 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); > > @@ -261,7 +259,7 @@ u32 dwc3_core_fifo_space(struct dwc3_ep *dep, u8 type) > > * dwc3_core_soft_reset - Issues core soft reset and PHY reset > > * @dwc: pointer to our context structure > > */ > > -static int dwc3_core_soft_reset(struct dwc3 *dwc) > > +int dwc3_core_soft_reset(struct dwc3 *dwc) > > { > > u32 reg; > > int retries = 1000; > > diff --git a/drivers/usb/dwc3/core.h b/drivers/usb/dwc3/core.h > > index 00348d6d479b..b27ad8dad317 100644 > > --- a/drivers/usb/dwc3/core.h > > +++ b/drivers/usb/dwc3/core.h > > @@ -1532,6 +1532,8 @@ bool dwc3_has_imod(struct dwc3 *dwc); > > int dwc3_event_buffers_setup(struct dwc3 *dwc); > > void dwc3_event_buffers_cleanup(struct dwc3 *dwc); > > > > +int dwc3_core_soft_reset(struct dwc3 *dwc); > > + > > #if IS_ENABLED(CONFIG_USB_DWC3_HOST) || IS_ENABLED(CONFIG_USB_DWC3_DUAL_ROLE) > > int dwc3_host_init(struct dwc3 *dwc); > > void dwc3_host_exit(struct dwc3 *dwc); > > diff --git a/drivers/usb/dwc3/gadget.c b/drivers/usb/dwc3/gadget.c > > index 0c89baedf220..788889f924f9 100644 > > --- a/drivers/usb/dwc3/gadget.c > > +++ b/drivers/usb/dwc3/gadget.c > > @@ -2585,6 +2585,17 @@ static int dwc3_gadget_pullup(struct usb_gadget *g, int is_on) > > dwc->ev_buf->length; > > } > > } else { > > + /* > > + * In the Synopsis DesignWare Cores USB3 Databook Rev. 1.90a > > + * Section 4.1.9, it specifies that for a reconnect after a > > + * device-initiated disconnect requires a core soft reset > > + * (DCTL.CSftRst) before enabling the run/stop bit. > > + */ > > + spin_unlock_irqrestore(&dwc->lock, flags); > > + dwc3_core_soft_reset(dwc); > > + spin_lock_irqsave(&dwc->lock, flags); > > + > > + dwc3_event_buffers_setup(dwc); > > Could you tell me why you add dwc3_event_buffer_setup? > > Best Regards, > Jung Daehwan > I'm sorry I misunderstood databook then. It seems to be needed. Best Regrards, Jung Daehwan > > __dwc3_gadget_start(dwc); > > } > > > >
Hi Daehwan, On 2/16/2022 9:52 PM, Jung Daehwan wrote: > Hi wesley, > > On Tue, Feb 15, 2022 at 04:08:35PM -0800, Wesley Cheng wrote: >> It is recommended by the Synopsis databook to issue a DCTL.CSftReset >> when reconnecting from a device-initiated disconnect routine. This >> resolves issues with enumeration during fast composition switching >> cases, which result in an unknown device on the host. >> >> Signed-off-by: Wesley Cheng <quic_wcheng@quicinc.com> >> --- >> drivers/usb/dwc3/core.c | 4 +--- >> drivers/usb/dwc3/core.h | 2 ++ >> drivers/usb/dwc3/gadget.c | 11 +++++++++++ >> 3 files changed, 14 insertions(+), 3 deletions(-) >> >> diff --git a/drivers/usb/dwc3/core.c b/drivers/usb/dwc3/core.c >> index 18adddfba3da..02d10e1cb774 100644 >> --- a/drivers/usb/dwc3/core.c >> +++ b/drivers/usb/dwc3/core.c >> @@ -115,8 +115,6 @@ 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); >> @@ -261,7 +259,7 @@ u32 dwc3_core_fifo_space(struct dwc3_ep *dep, u8 type) >> * dwc3_core_soft_reset - Issues core soft reset and PHY reset >> * @dwc: pointer to our context structure >> */ >> -static int dwc3_core_soft_reset(struct dwc3 *dwc) >> +int dwc3_core_soft_reset(struct dwc3 *dwc) >> { >> u32 reg; >> int retries = 1000; >> diff --git a/drivers/usb/dwc3/core.h b/drivers/usb/dwc3/core.h >> index 00348d6d479b..b27ad8dad317 100644 >> --- a/drivers/usb/dwc3/core.h >> +++ b/drivers/usb/dwc3/core.h >> @@ -1532,6 +1532,8 @@ bool dwc3_has_imod(struct dwc3 *dwc); >> int dwc3_event_buffers_setup(struct dwc3 *dwc); >> void dwc3_event_buffers_cleanup(struct dwc3 *dwc); >> >> +int dwc3_core_soft_reset(struct dwc3 *dwc); >> + >> #if IS_ENABLED(CONFIG_USB_DWC3_HOST) || IS_ENABLED(CONFIG_USB_DWC3_DUAL_ROLE) >> int dwc3_host_init(struct dwc3 *dwc); >> void dwc3_host_exit(struct dwc3 *dwc); >> diff --git a/drivers/usb/dwc3/gadget.c b/drivers/usb/dwc3/gadget.c >> index 0c89baedf220..788889f924f9 100644 >> --- a/drivers/usb/dwc3/gadget.c >> +++ b/drivers/usb/dwc3/gadget.c >> @@ -2585,6 +2585,17 @@ static int dwc3_gadget_pullup(struct usb_gadget *g, int is_on) >> dwc->ev_buf->length; >> } >> } else { >> + /* >> + * In the Synopsis DesignWare Cores USB3 Databook Rev. 1.90a >> + * Section 4.1.9, it specifies that for a reconnect after a >> + * device-initiated disconnect requires a core soft reset >> + * (DCTL.CSftRst) before enabling the run/stop bit. >> + */ >> + spin_unlock_irqrestore(&dwc->lock, flags); >> + dwc3_core_soft_reset(dwc); >> + spin_lock_irqsave(&dwc->lock, flags); >> + >> + dwc3_event_buffers_setup(dwc); > > Could you tell me why you add dwc3_event_buffer_setup? > During my testing if the event buffer re-initialization is not there, then it resulted in the controller attempting to access an invalid address space after the soft reset completed. (SMMU fault w/ FAR=0x0) Likewise, in other scenarios where core soft reset is called, there is always an event buffer re-init following it. Thanks Wesley Cheng
diff --git a/drivers/usb/dwc3/core.c b/drivers/usb/dwc3/core.c index 18adddfba3da..02d10e1cb774 100644 --- a/drivers/usb/dwc3/core.c +++ b/drivers/usb/dwc3/core.c @@ -115,8 +115,6 @@ 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); @@ -261,7 +259,7 @@ u32 dwc3_core_fifo_space(struct dwc3_ep *dep, u8 type) * dwc3_core_soft_reset - Issues core soft reset and PHY reset * @dwc: pointer to our context structure */ -static int dwc3_core_soft_reset(struct dwc3 *dwc) +int dwc3_core_soft_reset(struct dwc3 *dwc) { u32 reg; int retries = 1000; diff --git a/drivers/usb/dwc3/core.h b/drivers/usb/dwc3/core.h index 00348d6d479b..b27ad8dad317 100644 --- a/drivers/usb/dwc3/core.h +++ b/drivers/usb/dwc3/core.h @@ -1532,6 +1532,8 @@ bool dwc3_has_imod(struct dwc3 *dwc); int dwc3_event_buffers_setup(struct dwc3 *dwc); void dwc3_event_buffers_cleanup(struct dwc3 *dwc); +int dwc3_core_soft_reset(struct dwc3 *dwc); + #if IS_ENABLED(CONFIG_USB_DWC3_HOST) || IS_ENABLED(CONFIG_USB_DWC3_DUAL_ROLE) int dwc3_host_init(struct dwc3 *dwc); void dwc3_host_exit(struct dwc3 *dwc); diff --git a/drivers/usb/dwc3/gadget.c b/drivers/usb/dwc3/gadget.c index 0c89baedf220..788889f924f9 100644 --- a/drivers/usb/dwc3/gadget.c +++ b/drivers/usb/dwc3/gadget.c @@ -2585,6 +2585,17 @@ static int dwc3_gadget_pullup(struct usb_gadget *g, int is_on) dwc->ev_buf->length; } } else { + /* + * In the Synopsis DesignWare Cores USB3 Databook Rev. 1.90a + * Section 4.1.9, it specifies that for a reconnect after a + * device-initiated disconnect requires a core soft reset + * (DCTL.CSftRst) before enabling the run/stop bit. + */ + spin_unlock_irqrestore(&dwc->lock, flags); + dwc3_core_soft_reset(dwc); + spin_lock_irqsave(&dwc->lock, flags); + + dwc3_event_buffers_setup(dwc); __dwc3_gadget_start(dwc); }
It is recommended by the Synopsis databook to issue a DCTL.CSftReset when reconnecting from a device-initiated disconnect routine. This resolves issues with enumeration during fast composition switching cases, which result in an unknown device on the host. Signed-off-by: Wesley Cheng <quic_wcheng@quicinc.com> --- drivers/usb/dwc3/core.c | 4 +--- drivers/usb/dwc3/core.h | 2 ++ drivers/usb/dwc3/gadget.c | 11 +++++++++++ 3 files changed, 14 insertions(+), 3 deletions(-)