Re: [PATCH v3] usb: dwc2/gadget: report disconnect event from 'end session' irq

[Marek Szyprowski <m.szyprowski@samsung.com>]

Hello,

On 2014-11-13 21:50, Paul Zimmerman wrote:
>> From: Marek Szyprowski [mailto:m.szyprowski@samsung.com]
>> Sent: Thursday, November 13, 2014 5:40 AM
>>
>> On 2014-10-31 19:15, Paul Zimmerman wrote:
>>>> From: Marek Szyprowski [mailto:m.szyprowski@samsung.com]
>>>> Sent: Friday, October 31, 2014 1:04 AM
>>>> To: linux-usb@vger.kernel.org; linux-samsung-soc@vger.kernel.org
>>>> Cc: Marek Szyprowski; Kyungmin Park; Robert Baldyga; Paul Zimmerman; Krzysztof Kozlowski; Felipe
>> Balbi
>>>> Subject: [PATCH v3] usb: dwc2/gadget: report disconnect event from 'end session' irq
>>>>
>>>> This patch adds a call to s3c_hsotg_disconnect() from 'end session'
>>>> interrupt (GOTGINT_SES_END_DET) to correctly notify gadget subsystem
>>>> about unplugged usb cable. 'disconnected' interrupt (DISCONNINT) might
>>>> look a bit more suitable for this event, but it is asserted only in
>>>> host mode, so in device mode we need to use something else.
>>>>
>>>> Additional check has been added in s3c_hsotg_disconnect() function
>>>> to ensure that the event is reported only once after successful device
>>>> enumeration.
>>>>
>>>> Signed-off-by: Marek Szyprowski <m.szyprowski@samsung.com>
>>>> ---
>>>>    drivers/usb/dwc2/core.h   |  1 +
>>>>    drivers/usb/dwc2/gadget.c | 10 ++++++++++
>>>>    2 files changed, 11 insertions(+)
>>>>
>>>> diff --git a/drivers/usb/dwc2/core.h b/drivers/usb/dwc2/core.h
>>>> index 55c90c53f2d6..b42df32e7737 100644
>>>> --- a/drivers/usb/dwc2/core.h
>>>> +++ b/drivers/usb/dwc2/core.h
>>>> @@ -212,6 +212,7 @@ struct s3c_hsotg {
>>>>    	struct usb_gadget       gadget;
>>>>    	unsigned int            setup;
>>>>    	unsigned long           last_rst;
>>>> +	unsigned int		address;
>>>>    	struct s3c_hsotg_ep     *eps;
>>>>    };
>>>>
>>>> diff --git a/drivers/usb/dwc2/gadget.c b/drivers/usb/dwc2/gadget.c
>>>> index fcd2bb55ccca..6304efba11aa 100644
>>>> --- a/drivers/usb/dwc2/gadget.c
>>>> +++ b/drivers/usb/dwc2/gadget.c
>>>> @@ -1114,6 +1114,7 @@ static void s3c_hsotg_process_control(struct s3c_hsotg *hsotg,
>>>>    				 DCFG_DEVADDR_SHIFT) & DCFG_DEVADDR_MASK;
>>>>    			writel(dcfg, hsotg->regs + DCFG);
>>>>
>>>> +			hsotg->address = ctrl->wValue;
>>>>    			dev_info(hsotg->dev, "new address %d\n", ctrl->wValue);
>>>>
>>>>    			ret = s3c_hsotg_send_reply(hsotg, ep0, NULL, 0);
>>>> @@ -2031,6 +2032,10 @@ static void s3c_hsotg_disconnect(struct s3c_hsotg *hsotg)
>>>>    {
>>>>    	unsigned ep;
>>>>
>>>> +	if (!hsotg->address)
>>>> +		return;
>>>> +
>>>> +	hsotg->address = 0;
>>>>    	for (ep = 0; ep < hsotg->num_of_eps; ep++)
>>>>    		kill_all_requests(hsotg, &hsotg->eps[ep], -ESHUTDOWN, true);
>>>>
>>>> @@ -2290,6 +2295,11 @@ irq_retry:
>>>>    		dev_info(hsotg->dev, "OTGInt: %08x\n", otgint);
>>>>
>>>>    		writel(otgint, hsotg->regs + GOTGINT);
>>>> +
>>>> +		if (otgint & GOTGINT_SES_END_DET) {
>>>> +			s3c_hsotg_disconnect(hsotg);
>>>> +			hsotg->gadget.speed = USB_SPEED_UNKNOWN;
>>>> +		}
>>>>    	}
>>>>
>>>>    	if (gintsts & GINTSTS_SESSREQINT) {
>>> I don't think this is right. The host can send control requests to
>>> the device before it sends a SetAddress to change from the default
>>> address of 0. So if a GOTGINT_SES_END_DET happens before the
>>> SetAddress, it will be ignored.
>>>
>>> Or am I missing something?
>> Well, right. However before finishing enumeration (setting the address)
>> host usually
>> only retrieves some usb descriptors what doesn't change the state of the
>> gadget.
>> Right now we always reported 'disconnected' event before setting the new
>> address,
>> what is a bit overkill (in some cases gadget driver got this even more
>> than once).
>> The above code handles all cases correctly and reports disconnect event
>> only once.
> Well, if the disconnect happens before the SetAddress, the disconnect
> won't be reported at all. Unless I'm reading the code wrong.

Ok, I found other way to ensure that disconnect event is reported only 
once.
I will post a patch in a few minutes.


Best regards
-- 
Marek Szyprowski, PhD
Samsung R&D Institute Poland