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

[Lei Wen]

Hi Vitaly,

On Wed, Dec 1, 2010 at 12:41 AM, Vitaly Kuzmichev <vkuzmichev at mvista.com> wrote:
> Hi Lei,
>
> Lei Wen wrote:
>> [...]
>>>> 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 have not asked you to remove network device, I have talked about the
> reason for the following:
>> For current ether.c state, there is no usb_gadget_unregister_driver in
>> it.
>
>> [...]
>>>> 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?
>
> Because UDC driver does not allow to register more than 1 gadget driver
> in the same time. It just returns EBUSY. Even if the first gadget driver
> is idle (when Ether gadget driver is registered but tftp command is not
> issued). This is done to protect against memory leaks.
> Just checkout  cdc-at91  branch in  u-boot-usb.git  for example of UDC
> driver (drivers/usb/gadget/at91_udc.c). Or look at any UDC driver in
> linux kernel.
>

I add unregister function in eth_halt, then the udc driver could do the proper
memory free to prevent the memory leak.

>
>> [...]
>>> 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.
>
> Yes, it will free by 'eth_unbind' called from
> 'usb_gadget_unregister_driver'.
>
> Note that 'usb_gadget_register_driver' should be removed from
> 'usb_eth_initialize' and dev->gadget checking should be added in eth_halt.
>
Ack this idea. Apply to the next patch.

Best regards,
Lei