Re: [PATCH 1/2] usb: configfs: allow UDC binding rule configured as binding to *any* UDC

From: Krzysztof Opasiak
Date: Thu May 05 2016 - 03:31:43 EST


On 05/05/2016 07:46 AM, Du, Changbin wrote:
> Hi,
>>> On most platforms, there is only one device controller available.
>>> In this case, we desn't care the UDC's name. So let's ignore the
>>> name by setting 'UDC' to 'any'.
>> Hmm libubsgx allows to do this for a very long time. You simply pass
>> NULL instead of pointer to usbg_udc.
>> It is also possible to do this from command line, just simply:
>> $ echo `ls -1 /sys/class/udc | head -n 1` > UDC
>> So if we can easily do this from user space what's the benefit of adding
>> this special "any" keyword to kernel?
> Well, it is just for *easy to use*. Looking up /sys/class/udc mostly
> can be skipped. The UDC core support this convenience behavior,
> so why don't we export it with a little change?

Well, I'm not sure if any configfs interface has been proposed as easy
to use from cmd line. I think they all has been proposed as *usable*
from cmd line but not necessarily *easy to use*.

That's why most of configfs clients has some support in userspace. For
example for target there is a taget-cli and for usb gadget we have

So the functionality which you proposed here is not only already
implemented in libusbgx but also can be easily achieved from cmd line
like I showed above.

In addition this patch will break existing userspace tools (at least
libusbgx for sure) as it assumes that:

cat UDC

should return an empty string or an valid UDC name which can be found
inside /sys/class/udc.

After this patch the kernel can return some kind of magic string "any"
which obviously will cannot be found in udc dir.

>>> And also we can change UDC name
>>> at any time if it is not binded (no need set to "" first).
>> Not sure if:
>> $ echo "" > UDC
>> is really a problem. Personally I'm quite used to situation in which I
>> have to turn the light off before turning it on once again;)
> That is not a problem. But just avoid pseudo 'busy'. If gadget is not
> bind, it is free to reconfigure it. So seem no need block re-configuration.

What do you mean pseudo 'busy'? If we do:

echo <udc-name> > UDC

then gadget should be really bound to some udc and potentially really busy.

> In a word, this patch is just an improvement, not to fix any issues or
> add new function.

So it doesn't add any new functionality and breaks existing user space

Krzysztof Opasiak
Samsung R&D Institute Poland
Samsung Electronics