usb: dwc2: Reset device address on EnumDone

[Felipe Balbi <balbi@kernel.org>]

Hi,

Minas Harutyunyan <minas.harutyunyan@synopsys.com> writes:
> Hi Felipe,
>
> On 1/21/2019 11:13 AM, Minas Harutyunyan wrote:
>> Hi Felipe,
>> 
>> On 12/12/2018 3:43 PM, Minas Harutyunyan wrote:
>>> Initially resetting device address was done in USB RESET interrupt
>>> handler. In case, when power saving mode enabled (hibernation) USB
>>> RESET interrupt handled in dwc2_handle_gpwrdn_intr() and then it
>>> not seen in dwc2_hsotg_irq() handler. This is why reset device
>>> address to zero moved from USB RESET handler to EnumDone handler.
>>>
>>> Signed-off-by: Minas Harutyunyan <hminas@synopsys.com>
>>> ---
>>>    drivers/usb/dwc2/gadget.c | 6 +++---
>>>    1 file changed, 3 insertions(+), 3 deletions(-)
>>>
>>> diff --git a/drivers/usb/dwc2/gadget.c b/drivers/usb/dwc2/gadget.c
>>> index 68ad75a7460d..7f922f19f8e1 100644
>>> --- a/drivers/usb/dwc2/gadget.c
>>> +++ b/drivers/usb/dwc2/gadget.c
>>> @@ -3072,6 +3072,9 @@ static void dwc2_hsotg_irq_enumdone(struct dwc2_hsotg *hsotg)
>>>    
>>>    	dev_dbg(hsotg->dev, "EnumDone (DSTS=0x%08x)\n", dsts);
>>>    
>>> +	/* Reset device address to zero */
>>> +	dwc2_clear_bit(hsotg, DCFG, DCFG_DEVADDR_MASK);
>>> +
>>>    	/*
>>>    	 * note, since we're limited by the size of transfer on EP0, and
>>>    	 * it seems IN transfers must be a even number of packets we do
>>> @@ -3614,9 +3617,6 @@ static irqreturn_t dwc2_hsotg_irq(int irq, void *pw)
>>>    		/* Report disconnection if it is not already done. */
>>>    		dwc2_hsotg_disconnect(hsotg);
>>>    
>>> -		/* Reset device address to zero */
>>> -		dwc2_clear_bit(hsotg, DCFG, DCFG_DEVADDR_MASK);
>>> -
>>>    		if (usb_status & GOTGCTL_BSESVLD && connected)
>>>    			dwc2_hsotg_core_init_disconnected(hsotg, true);
>>>    	}
>>>
>> 
>> This patch not seen yet in your testing/fixes or next. Any reason for
>> delay or you missed it?
>> 
>> Thanks,
>> Minas
>> 
>> 
> Not seen yet. Ping again.

I don't see any indication that this is a bug fix that needs to go
during -rc cycle, so I was gonna queue it for next merge window. 

Frankly, moving address reset to enumdone sounds like a bad idea. It
looks to me that you're moving the problem from one place to another
because of hibernation.

I would, rather, suggest that you review your interrupt handler and make
sure it's compliant with your documentation. Why is RESET handled by
HIBERNATION handler, for example?

Anyway, I'm gonna wait for your reply before doing anything with this
patch.