Re: [PATCH] usb: dwc3: gadget: giveback request if start transfer fail

From: Felipe Balbi <balbi@ti.com>
To: Zhuang Jin Can <jin.can.zhuang@intel.com>
Cc: Felipe Balbi <balbi@ti.com>, <linux-usb@vger.kernel.org>,
	<linux-omap@vger.kernel.org>, <linux-kernel@vger.kernel.org>,
	David Cohen <david.a.cohen@linux.intel.com>
Subject: Re: [PATCH] usb: dwc3: gadget: giveback request if start transfer fail
Date: Thu, 1 May 2014 10:13:28 -0500	[thread overview]
Message-ID: <20140501151328.GB6355@saruman.home> (raw)
In-Reply-To: <20140501204452.GD30575@intel.com>

[-- Attachment #1: Type: text/plain, Size: 2125 bytes --]

Hi,

On Thu, May 01, 2014 at 04:44:52PM -0400, Zhuang Jin Can wrote:
> On Wed, Apr 30, 2014 at 02:58:29PM -0500, Felipe Balbi wrote:
> > On Thu, May 01, 2014 at 02:36:08AM -0400, Zhuang Jin Can wrote:
> > > At least we should giveback the current request to the
> > > gadget. Otherwise, the gadget will be stuck without knowing
> > > anything.
> > > 
> > > It was oberved that the failure can happen if the request is
> > > queued when the run/stop bit of controller is not set.
> > 
> > why is your gadget queueing any requests before calling ->udc_start() ?
> > 
> > A better question, what modification have you done to udc-core.c which
> > broke this ? udc-core *always* calls ->udc_start() by the time you load
> > a gadget driver so this case will *never* happen. Whatever modification
> > you did, broke this assumption and I will *not* accept this patch
> > because the bug is elsewhere and *not* in mainline kernel.
> > 
> It's found in Android using kernel 3.10.20. Android has its own
> usb_composite_driver usb/gadget/android.c (not in mainline), and it

so you found something on an old kernel using an out-of-tree gadget
driver.

> allows userspace to disconnect the pullup (i.e clear run/stop bit in dwc3)
> and remove the gadget functions like adb, mtp and then add new functions
> like rndis, acm. The problem is when you disconnect the pullup, a gadget
> maybe in the middle of queuing a request, and result in the "start
> transfer cmd failure". I think this is also a common issue for other

Android gadget needs to learn how to cope with that.

> usb_composite_drivers too. Normally, if one of the gadget deactivate its
> own function, the pullup will be disconnected, other gadgets won't get
> notified until their requests are failed. So it makes dwc3 more robust
> to deal with these situations.

Right, but Android gadget can run on top of several other UDCs and you
want to have a single one of them cope with android's bug ?

You'd be better off getting google to accept a bugfix to the android
gadget, since that's where the problem lies.

-- 
balbi

[-- Attachment #2: Digital signature --]
[-- Type: application/pgp-signature, Size: 819 bytes --]