Subject: Re: [PATCH] usb: dwc3: usb/dwc3: fake dissconnect event when turn off
 pullup
To: Felipe Balbi <balbi@kernel.org>, "Du, Changbin" <changbin.du@intel.com>
References: <1461745745-3954-1-git-send-email-changbin.du@intel.com>
 <87wpnjmm7u.fsf@intel.com>
 <0C18FE92A7765D4EB9EE5D38D86A563A05D1E1A7@SHSMSX103.ccr.corp.intel.com>
 <87vb32kz7s.fsf@intel.com>
From: John Youn <John.Youn@synopsys.com>
CC: "gregkh@linuxfoundation.org" <gregkh@linuxfoundation.org>,
        "linux-usb@vger.kernel.org" <linux-usb@vger.kernel.org>,
        "linux-kernel@vger.kernel.org" <linux-kernel@vger.kernel.org>
Message-ID: <572266F9.7000204@synopsys.com>
Date: Thu, 28 Apr 2016 12:39:37 -0700
User-Agent: Mozilla/5.0 (Windows NT 6.1; WOW64; rv:38.0) Gecko/20100101
 Thunderbird/38.7.2
MIME-Version: 1.0
In-Reply-To: <87vb32kz7s.fsf@intel.com>
Content-Type: text/plain; charset="utf-8"
Content-Transfer-Encoding: 7bit
Sender: linux-kernel-owner@vger.kernel.org
Content-Length: 2690
Lines: 65

On 4/27/2016 11:48 PM, Felipe Balbi wrote:
> 
> Hi,
> 
> (we don't top-post on this forum ;-)
> 
> "Du, Changbin" <changbin.du@intel.com> writes:
>> Hi, Balbi,
>>
>> The step to reproduce this issue is:
>> 1) connect device to a host and wait its enumeration.
>> 2) trigger software disconnect by calling function
>>     usb_gadget_disconnect(), which finally call
>>    dwc3_gadget_pullup(false). Do not reconnect device
>>   (I mean no enumeration go on, keep bit Run/Stop 0.).
>>
>> At here, gadget driver's disconnect callback should be
>> Called, right? We has been disconnected. But no, as
>> You said " not generating disconnect IRQ after you
>> drop Run/Stop is expected".
>>
>> And I am testing on an Android device, Android only
>> use dwc3_gadget_pullup(false) to issue a soft disconnection.
>> This confused user that the UI still show usb as connected
>> State, caused by missing a disconnect event.
> 
> okay, so I know what this is. This is caused by Android gadget itself
> not notifying the gadget that a disconnect has happened. Just look at
> udc-core's soft_connect implementation for the sysfs interface, and
> you'll see what I mean.
> 
> This should be fixed at Android gadget itself. The only thing we could
> do is introduce a new usb_gadget_soft_connect()/disconnect() to wrap the
> logic so it's easier for Android gadget to use; but even that I'm a
> little bit reluctant to do because Android should be using our
> soft_connect interface instead of reimplementing it (wrongly) by its
> own.
> 

We've run in to the same issue with our usb_gadget_driver.

If the usb_gadget_disconnect() API function, which seems like it is
intended to be called by the gadget_driver, does cause the gadget to
disconnect, it seems reasonable to expect the gadget or the UDC core
to notify the gadget_driver via the callback.

As you mentioned this is handled in the soft_disconnect sysfs. Why
shouldn't usb_gadget_disconnect() do the same thing, if not the gadget
itself? Exposing the sysfs as an API function would work too. Though
both functions are "soft" disconnects and both are called
"disconnect".

In our gadget_driver we do the workaround where we notify ourself that
we called the usb_gadget_disconnect() and that we should now be
disconnected. It just seems more correct that we shouldn't have to
handle that.

By the way, I'm not completely sure of the correct terminology, but
I'm referring to the struct usb_gadget (dwc3, dwc2, etc) as the
"gadget" and the struct usb_gadget_driver as the "gadget_driver"
(normally this would be the composite gadget framework, but we are
using our own driver in this case). Is there a less confusing way to
refer to these :)

John