Re: CSW endpoint status returned STALL after BOT MSC Reset

[Pratyush Anand]

On Thu, Oct 31, 2013 at 12:32:29AM +0800, Alan Stern wrote:
> On Wed, 30 Oct 2013, Alan Stern wrote:
> 
> > I think you have found a bug in the dwc3 driver.
> 
> > At this point, because the IGNORE_BULK_OUT bit is set, g_mass_storage
> > issues a usb_ep_clear_halt() call for the bulk-in (CSW) endpoint.  
> > This tells the dwc3 driver to change the endpoint's status back to 0:
> > 
> >             if (test_and_clear_bit(IGNORE_BULK_OUT,
> >                                    &common->fsg->atomic_bitflags))
> >                     usb_ep_clear_halt(common->fsg->bulk_in);
> > 
> > > INFO      Retrieving status on CBW endpoint
> > > INFO      CBW endpoint status = 0x0
> > > INFO      Retrieving status on CSW endpoint
> > > INFO      CSW endpoint status = 0x1
> > 
> > But the status is still set to 1.  Clearly this is a bug in dwc3.
> 
> And indeed it is.  The dwc3 driver does not implement the wedge method 
> correctly.  This patch should fix it.  Let me know how it works.
> 
> Alan Stern
> 
> 
> 
> Index: usb-3.12/drivers/usb/dwc3/ep0.c
> ===================================================================
> --- usb-3.12.orig/drivers/usb/dwc3/ep0.c
> +++ usb-3.12/drivers/usb/dwc3/ep0.c
> @@ -459,6 +459,8 @@ static int dwc3_ep0_handle_feature(struc
>                       dep = dwc3_wIndex_to_dep(dwc, wIndex);
>                       if (!dep)
>                               return -EINVAL;
> +                     if (set == 0 && (dep->flags & DWC3_EP_WEDGE))
> +                             break;
>                       ret = __dwc3_gadget_ep_set_halt(dep, set);
>                       if (ret)
>                               return -EINVAL;
> Index: usb-3.12/drivers/usb/dwc3/gadget.c
> ===================================================================
> --- usb-3.12.orig/drivers/usb/dwc3/gadget.c
> +++ usb-3.12/drivers/usb/dwc3/gadget.c
> @@ -1200,9 +1200,6 @@ int __dwc3_gadget_ep_set_halt(struct dwc
>               else
>                       dep->flags |= DWC3_EP_STALL;
>       } else {
> -             if (dep->flags & DWC3_EP_WEDGE)
> -                     return 0;
> -
>               ret = dwc3_send_gadget_ep_cmd(dwc, dep->number,
>                       DWC3_DEPCMD_CLEARSTALL, &params);
>               if (ret)
> @@ -1210,7 +1207,7 @@ int __dwc3_gadget_ep_set_halt(struct dwc
>                                       value ? "set" : "clear",
>                                       dep->name);
>               else
> -                     dep->flags &= ~DWC3_EP_STALL;
> +                     dep->flags &= ~(DWC3_EP_STALL | DWC3_EP_WEDGE);
>       }
>  
>       return ret;

Tested-by: Pratyush Anand <pratyush.an...@st.com> 

However, I noticed that the same error recovery test fails even after
above patch if dwc3 dbg/vdbg messages are enabled. However, reason is
not in dwc3 driver, rather in mass storage driver.

Any specific reason for returning DELAYED_STATUS and not 
USB_GADGET_DELAYED_STATUS
while handling bulk reset request.? It seems a bug to me in mass
storage driver.


Test passes with dwc3 debug messages enabled if following patch is
used.

diff --git a/drivers/usb/gadget/f_mass_storage.c 
b/drivers/usb/gadget/f_mass_storage.c
index 1b443e3..915024b 100644
--- a/drivers/usb/gadget/f_mass_storage.c
+++ b/drivers/usb/gadget/f_mass_storage.c
@@ -634,7 +634,7 @@ static int fsg_setup(struct usb_function *f,
                 */
                DBG(fsg, "bulk reset request\n");
                raise_exception(fsg->common, FSG_STATE_RESET);
-               return DELAYED_STATUS;
+               return USB_GADGET_DELAYED_STATUS;
 
        case USB_BULK_GET_MAX_LUN_REQUEST:
                if (ctrl->bRequestType !=

Regards
Pratyush
--
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