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

[Vitaly Kuzmichev]

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.


> [...]
>>> 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...


> [...]
>> 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.


> [...]
>> ...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.

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.

____
Best regards,
Vitaly.