[U-Boot] [PATCH 1/3] usb: ci_udc: Fix set address to work with older controllers

[Stephen Warren]

On 02/24/2015 10:41 AM, Alban Bedel wrote:
> On Tue, 24 Feb 2015 10:00:43 -0700
> Stephen Warren <swarren at wwwdotorg.org> wrote:
>
>> On 02/24/2015 09:44 AM, Alban Bedel wrote:
>>> Older controllers don't implement "Device Address Advance" which allow
>>> to pass the device address to the controller when it is received.
>>> To support such controller we need to store the requested address and
>>> only apply it after the next IN transfer completed on EP0.
>>
>>> diff --git a/drivers/usb/gadget/ci_udc.c b/drivers/usb/gadget/ci_udc.c
>>
>>>    	case SETUP(USB_RECIP_DEVICE, USB_REQ_SET_ADDRESS):
>>> -		/*
>>> -		 * write address delayed (will take effect
>>> -		 * after the next IN txn)
>>> -		 */
>>> -		writel((r.wValue << 25) | (1 << 24), &udc->devaddr);
>>> +		/* The device address must be updated after the next IN
>>> +		 * request completed */
>>> +		controller.set_address = r.wValue;
>>
>> Presumably, bit 24 is the "device address advance" feature?
>
> Yes, bit 24 is the "device address advance" feature
>
>> I'd prefer it if new controllers used the existing code, but we deferred
>> the write only for older controllers that don't support "device address
>> advance". That reduces the possibility of regressions on controller HW
>> that's already working. Presumably, there is some advantage using the
>> new feature, rather than deferring the address change manually, e.g. it
>> solves some race condition?
>
> I'm no USB expert, but AFAIU it is only a convenience to make the
> driver code simpler. I though that having less code path and ifdef
> would make the whole thing easier to maintain. However if that is
> preferred I can implement it as you suggested.

Is there not a race condition?

1) USB device controller completes the set address's IN transaction 
(which I assume is the status stage of a control transaction)

2) USB device re-programs address register according to the address that 
was set

3) USB host controller sends a USB transaction to the new address.

(1) must always happen first, but are (2) and (3) always guaranteed to 
happen in the desired order? I would have assumed the "auto advance" 
feature was so that the HW could atomically switch to responding to the 
new address while it completes the set address transaction, to avoid any 
window where it doesn't respond to the new address.

Of course, this is just pure conjecture.