[U-Boot] [PATCH] usb_ether: register usb ethernet gadget at each eth init

[Lei Wen]

Hi Vitaly,

On Tue, Nov 30, 2010 at 11:13 PM, Vitaly Kuzmichev
<vkuzmichev at mvista.com> wrote:
> Hi Lei,
>
> Lei Wen wrote:
>> Hi Vitaly,
>> [...]
>>> And additional calling of usb_gadget_unregister_driver will at most
>>> return an error.
>>>
>>
>> For current ether.c state, there is no usb_gadget_unregister_driver in
>> it. Even it has, we still need
>> usb_gadget_register_driver call in each eth_init().
>
> Yes, if we do 'unregister' we should do 'register' somewhere before. If
> we do 'register' we should do 'unregister' somewhere after.
> This is the symmetry such like in dynamic data allocation (like
> malloc/free), and we should comply it.
> The reason why there is no 'usb_gadget_unregister_driver' in the Ether
> driver yet is that there is no symmetrical function for
> 'usb_eth_initialize' because there is no way to remove a network device
> from the U-Boot environment.
>
What we need to do is not trying to remove the network device, but to register
another gadget when we want to switch from tftp to fastboot, and make UDC take
a different protocol to communicate with host.
>
>> [...]
>>>> I think the gadget driver should adopt like this to allow two gadget coexist.
>>>> And from gadget driver's review, the switch from one gadget to another
>>>> is easy, just
>>>> register it to another should be enough. Does it really need to return EBUSY?
>>>
>>> If the UDC (not a gadget) driver does not support more than 1 gadget
>>> driver in the same time it must return an error (EBUSY is fine for now)
>>> when another gadget tries to register itself.
>> Right, if at the same time certainly need to return error.
>
> So your code that expects successful register will make Ether gadget
> broken...

Don't understand here, why the Ether gadget would be broken?
>
>
>> [...]
>>> I really do not understand what a problem with using 2 gadgets in the
>>> same time without your patch if the UDC driver supports this.
>>> Especially I do not understand...
>>>
>> Sorry, maybe I make a confused saying here. I don't mean to support two gadget
>> at the "same time", but to let one gadget could work after another
>> gadget is finished.
>> Like after tftp, I could use fastboot, or after return from fastboot,
>> I could still use the tftp.
>
> ...But if you add usb_gadget_unregister_driver in eth_halt (and
> additional checking that dev->gadget is not equal to NULL) all existing
> UDC drivers (mine and Remy's at91_udc) will be working fine. And your
> fastboot will be working too.
Yep.

>
>> [...]
>>> ...how can the gadget (or UDC?) driver be confused here?
>> [...]
>>>
>> The confused thing is for the ether.c would register its gadget at the
>> usb_eth_initialize() call,
>> which is a very begining of the uboot. If we use some other gadget,
>> like fastboot, it would take
>> place of gadget registered in the gadget driver.
>
> You probably meant "in the UDC driver"?
> Note that USB gadget stack consists of 2 sorts of drivers: gadget and
> controller (UDC, USB device controller). The gadget driver provides
> hardware independent protocol for talking with host machine. The
> controller driver interacts with hardware - USB device chip. The UDC
> driver allocates 'struct gadget' (passed in 'usb_gadget_driver.bind'
> callback) for each gadget driver which tries to register itself.
>
> However, all existing UDC drivers (both in Linux and U-Boot) are not
> able to handle more than 1 gadget driver in the same time. And now I
> guess that this impossible. So they all return EBUSY if another gadget
> tries to register itself.

I agree with you, and my intent is not support two gadget at the same time,
but let two gadget would work one by one, that is after one stop its work, the
other could work fines.

>
> This means that if you want 2 gadgets you need to register each one
> right before transferring data and unregister right after the data was
> transferred. By doing gadget unregister you will free allocated
> resources (such as USB endpoints and USB requests) in UDC and Ether
> drivers properly. Otherwise you will have memory leaks.

Sure, so we comes into a conclusion that add register call in the
eth_init and unregister call
in eth_halt? In unregister call, the UDC driver should free the ep as you said.

Best regards,
Lei