RE: [PATCH v1 27/32] usb: dwc2: gadget: kill ep0 requests before reinitializing core

[Kaukab, Yousaf]

> -Original Message-
> From: John Youn [mailto:john.y...@synopsys.com]
> Sent: Friday, September 18, 2015 5:03 AM
> To: Kaukab, Yousaf; linux-usb@vger.kernel.org; ba...@ti.com;
> john.y...@synopsys.com
> Cc: Herrero, Gregory; he...@sntech.de; diand...@chromium.org;
> r.bald...@samsung.com; dingu...@opensource.altera.com;
> zhangfei@linaro.org; sergei.shtyl...@cogentembedded.com;
> david.a.co...@linux.intel.com
> Subject: Re: [PATCH v1 27/32] usb: dwc2: gadget: kill ep0 requests before
> reinitializing core
> 
> On 9/9/2015 3:21 AM, Mian Yousaf Kaukab wrote:
> > Make sure there are no requests pending on ep0 before reinitializing
> > core. Otherwise, dwc2_hsotg_enqueue_setup will fail afterwards.
> >
> > Signed-off-by: Mian Yousaf Kaukab <yousaf.kau...@intel.com>
> > Tested-by: Robert Baldyga <r.bald...@samsung.com>
> > ---
> >  drivers/usb/dwc2/gadget.c | 8 +++-
> >  1 file changed, 3 insertions(+), 5 deletions(-)
> >
> > diff --git a/drivers/usb/dwc2/gadget.c b/drivers/usb/dwc2/gadget.c
> > index c7da6b7..a6a1a6a 100644
> > --- a/drivers/usb/dwc2/gadget.c
> > +++ b/drivers/usb/dwc2/gadget.c
> > @@ -2287,6 +2287,9 @@ void dwc2_hsotg_core_init_disconnected(struct
> > dwc2_hsotg *hsotg,  {
> > u32 val;
> >
> > +   /* Kill any ep0 requests as controller will be reinitialized */
> > +   kill_all_requests(hsotg, hsotg->eps_out[0], -ECONNRESET);
> > +
> 
> I am seeing this hang when I start off as a B-device, then plug in an A-cable
> with no device attached on the other end. Then unplug it.
> 
> I haven't verified but it is probably due to the
> dwc2_hsotg_complete_request() (called from kill_all_requests) expecting lock
> to be held. But this is not locked when called from
> dwc2_conn_id_status_change().

You are spot on. dwc2_conn_id_status_change() is the only place where 
dwc2_hsotg_core_init_disconnected() is called without locks. I will add 
following
change to " usb: dwc2: gadget: kill ep0 requests before reinitializing core" 
patch.

@@ -1386,7 +1387,9 @@ static void dwc2_conn_id_status_change(struct work_struct 
*work)
hsotg->op_state = OTG_STATE_B_PERIPHERAL;
dwc2_core_init(hsotg, false, -1);
dwc2_enable_global_interrupts(hsotg);
+   spin_lock(>lock);
dwc2_hsotg_core_init_disconnected(hsotg, false);
+   spin_unlock(>lock);
dwc2_hsotg_core_connect(hsotg);

> 
> John

BR,
Yousaf
--
To unsubscribe from this list: send the line "unsubscribe linux-usb" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH v1 27/32] usb: dwc2: gadget: kill ep0 requests before reinitializing core

[John Youn]

On 9/9/2015 3:21 AM, Mian Yousaf Kaukab wrote:
> Make sure there are no requests pending on ep0 before reinitializing
> core. Otherwise, dwc2_hsotg_enqueue_setup will fail afterwards.
> 
> Signed-off-by: Mian Yousaf Kaukab 
> Tested-by: Robert Baldyga 
> ---
>  drivers/usb/dwc2/gadget.c | 8 +++-
>  1 file changed, 3 insertions(+), 5 deletions(-)
> 
> diff --git a/drivers/usb/dwc2/gadget.c b/drivers/usb/dwc2/gadget.c
> index c7da6b7..a6a1a6a 100644
> --- a/drivers/usb/dwc2/gadget.c
> +++ b/drivers/usb/dwc2/gadget.c
> @@ -2287,6 +2287,9 @@ void dwc2_hsotg_core_init_disconnected(struct 
> dwc2_hsotg *hsotg,
>  {
>   u32 val;
>  
> + /* Kill any ep0 requests as controller will be reinitialized */
> + kill_all_requests(hsotg, hsotg->eps_out[0], -ECONNRESET);
> +

I am seeing this hang when I start off as a B-device, then plug
in an A-cable with no device attached on the other end. Then
unplug it.

I haven't verified but it is probably due to the
dwc2_hsotg_complete_request() (called from kill_all_requests)
expecting lock to be held. But this is not locked when called
from dwc2_conn_id_status_change().

John

--
To unsubscribe from this list: send the line "unsubscribe linux-usb" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html