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.