[U-Boot] [PATCH v1 3/3] cmd: usb gadget: Add a command to bind a USB gadget driver to a UDC port

[Stephen Warren]

On 06/14/2018 09:11 AM, Simon Glass wrote:
> On 14 June 2018 at 09:02, Jean-Jacques Hiblot <jjhiblot at ti.com> wrote:
>>
>>
>>
>> On 13/06/2018 03:29, Simon Glass wrote:
>>>
>>> Hi Jean-Jacques,
>>>
>>> On 12 June 2018 at 03:31, Jean-Jacques Hiblot <jjhiblot at ti.com> wrote:
>>>>
>>>> Hi,
>>>>
>>>>
>>>> On 08/06/2018 23:59, Simon Glass wrote:
>>>>>
>>>>> Hi,
>>>>>
>>>>> On 7 June 2018 at 01:39, Lukasz Majewski <lukma at denx.de> wrote:
>>>>>>
>>>>>> Hi Jean-Jacques,
>>>>>>
>>>>>>> Most of the time the UDC is bound to a driver when a dedicated
>>>>>>> command is executed, like 'dfu'. But the ethernet gadget driver must
>>>>>>> be bound by calling usb_ether_init() in the code otherwise the USB
>>>>>>> ethernet adapter is not visible to the ethernet core.
>>>>>>>
>>>>>>> In DM context, the platform code should not be used to bind UDC to a
>>>>>>> particular driver, so adding a new command to bind a USB device port
>>>>>>> to a driver.
>>>>>>>
>>>>>>> usage example:
>>>>>>> usbdev bind 0 usb_ether
>>>>>>> usbdev unbind 0
>>>>>>
>>>>>> I would prefer a comment from Simon (so adding him to CC) - as it looks
>>>>>> to me that we shall try to use DM to avoid adding separate commands for
>>>>>> binding.
>>>>>
>>>>> We could perhaps introduce 'bind' and 'unbind' commands with similar
>>>>> arguments?
>>>>
>>>> What kind of parameters should be used to identify the device to bind ?
>>>> I see 2 possible options:
>>>> - node paths: bind /opc/omap_dwc3 at 48380000/usb at 483d0000 usb_ether
>>>> - device class + index: bind usb_dev 0 usb_ether.
>>>>
>>>> The last one looks a lot like what I proposed, only more generic, but
>>>> requires creating a table to match a string with a UCLASS id.
>>>> The first is more precise but IMO less user friendly.
>>>
>>> We can look up a uclass by name, so your second open should work OK.
>>> It matches the current U-Boot approach pretty well two since most
>>> commands work this way.
>>>
>>> However, I have sometimes thought (with driver model) of supporting
>>> the first option as a fallback.
>>>
>>> You could in fact have a function that supports both:
>>>
>>> 1. Option 1 - it does a global search for a device with that DT node
>>> 2. Option 2 - it does a uclass_get_device_by_seq() call
>>>
>>> I agree that option 2 is likely to be much preferred in normal use.
>>
>> I've been working on this and have come up with a slightly different interface:
>>
>> bind <node path> <driver name>
>> unbind <node path>
>> bind <class name> <index> <driver name>
>> unbind <class name> <index>
>>
>>
>> Interface with node path:
>> It is a symmetric interface:
>> example:
>> bind /ocp/ocp2scp at 483e8000 generic_simple_bus
>> unbind /ocp/ocp2scp at 483e8000
>>
>>
>> Interface with class/index:
>> This is by essence an asymmetric interface: the class/index pair is not the same for binding as for unbinding
>> example:
>> bind usb_dev_generic 0 usb_ether
>> unbind eth 1
>>
>> While it makes sense, it may be a bit harder to use because of the asymmetry
> 
> That seems OK to me. I added some more people for comment. Please add
> anyone else you can think of who might have ideas.

Why wouldn't the unbind arguments always match the bind arguments:

bind /ocp/ocp2scp at 483e8000 generic_simple_bus
unbind /ocp/ocp2scp at 483e8000

bind usb_dev_generic 0 usb_ether
unbind usb_dev_generic 0 usb_ether

One benefit here is that it's completely symmetric, so easier to specify 
and understand. Also, one might imagine a future where we could do:

bind usb_dev_generic 0 usb_ether
bind usb_dev_generic 0 usb_acm
# Here, can use both Ethernet and serial (console) over this port
unbind usb_dev_generic 0 usb_acm
unbind usb_dev_generic 0 usb_ether

(Although perhaps in this case, should we actually be binding not 
usb_ether and usb_acm directly, but rather binding usb_gadget, and 
somehow configuring what protocols usb_gadget exposes separately 
beforehand? For example, see how the kernel's USB gadget sysfs control 
works.)