Re: CSW endpoint status returned STALL after BOT MSC Reset

[Pratyush Anand <pratyush.anand@xxxxxx>]

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.anand@xxxxxx> 

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 majordomo@xxxxxxxxxxxxxxx
More majordomo info at  http://vger.kernel.org/majordomo-info.html