Re: [PATCH 1/2] usb: gadget: udc-core: independent registration of gadgets and gadget drivers

[Ruslan Bilovol <ruslan.bilovol@gmail.com>]

Hi Krzysztof,

On Tue, Feb 10, 2015 at 10:47 AM, Krzysztof Opasiak
<k.opasiak@samsung.com> wrote:
>
>
>> -----Original Message-----
>> From: Ruslan Bilovol [mailto:ruslan.bilovol@gmail.com]
>> Sent: Tuesday, February 10, 2015 12:46 AM
>> To: Alan Stern
>> Cc: Krzysztof Opasiak; Peter Chen; linux-usb@vger.kernel.org;
>> linux-kernel@vger.kernel.org; Balbi, Felipe;
>> gregkh@linuxfoundation.org; Andrzej Pietrasiewicz
>> Subject: Re: [PATCH 1/2] usb: gadget: udc-core: independent
>> registration of gadgets and gadget drivers
>>
>> Hi guys,
>>
>> On Mon, Feb 9, 2015 at 10:00 PM, Alan Stern
>> <stern@rowland.harvard.edu> wrote:
>> > On Mon, 9 Feb 2015, Krzysztof Opasiak wrote:
>> >
>> >> > Why bother matching by name?  Why not simply take the first
>> >> > available
>> >> > UDC?
>> >>
>> >> Because you may have more than one udc. This would allow to pick
>> one by
>> >> name just like using configfs interface.
>> >
>> > Clearly it would be more flexible to allow the user to do the
>> matching,
>> > the way configfs does (sysfs too).
>> >
>> >> > > Main feature of my path is not only deferred binding of
>> gadget
>> >> > driver,
>> >> > > but also possibility to register/unregister udc at any time.
>> >> > > This is useful for user who can load, for example, udc
>> module
>> >> > > if needed and unload it safely, not touching gadget driver.
>> >> >
>> >> > We can already do that with the existing code.  There's no
>> need for
>> >> > a
>> >> > patch.
>> >> >
>> >> > Also, it's not clear that the existing gadget drivers will
>> work
>> >> > properly if they are unbound from one UDC and then bound again
>> to
>> >> > another one.  They were not written with that sort of thing in
>> >> > mind.
>> >> >
>> >>
>> >> What you have described is one of basics configfs features.
>> >> You should be able to bind and unbind your gadget whenever you
>> want
>> >> and it should work properly after doing:
>> >>
>> >> ## create gadget
>> >> $ echo "udc.0" > UDC
>> >> $ echo "" > UDC
>> >> $ echo "udc.1" > UDC
>> >>
>> >> Function shouldn't care which udc it has been bound previously.
>> >> Only current one is important and on each unbind each function
>> >> should cleanup its state and prepare to be bound to another udc.
>> >> Configfs interface doesn't prohibit this and I haven't seen any
>> >> info about such restriction.
>>
>> Thank you Krzysztof for this explanation that makes things more
>> clear
>> for reviewers.
>>
>> >
>> > It's not prohibited, but it also was never required.  Therefore
>> it may
>> > not be implemented in all gadget drivers.
>> >
>> >>  If some function is not working in
>> >> such situation there is a bug in that function and it should be
>> fixed.
>> >
>> > That's fine.  I wasn't pointing out a fundamental limitation,
>> just a
>> > fact that will have to be taken into account.
>>
>> I also don't see any restrictions to bind/unbind gadget driver few
>> times
>> and in case of such bug we just can fix it. I didn't see any issue
>> with
>> gadget drivers that I used for verification, however, to be honest,
>> I didn't
>> test it with all possible ones.
>> Anyway, it should work in legacy way (one gadget driver is bound to
>> one uds)
>> so current behavior at least is not broken.
>>
>> >
>> > Anyway, instead of going through all this, why not do what I
>> suggested
>> > earlier (see http://marc.info/?l=linux-usb&m=139888691230119&w=2)
>> and
>> > create a "gadget" bus type?  That would give userspace explicit
>> control
>> > over which gadget driver was bound to which UDC.
>>
>> Exactly, my patch transforms udc-core to something like tiny bus
>> with very basic
>> functionality. But in mentioned mailthread I see good ideas that is
>> possible to
>> implement with small overhead.
>>
>> >
>> > Or maybe that's not a very good fit with the existing code, since
>> most
>> > gadget drivers assume they will be bound to only one UDC at a
>> time.  So
>> > instead, why not create a sysfs interface that allows userspace
>> to
>> > control which gadget drivers are bound to which UDCs?
>> >
>> > As I recall, the original problem people were complaining about
>> was
>> > deferred probing.  They didn't need fancy matching; all they
>> wanted was
>> > the ability to bind a gadget driver to a UDC some time after the
>> gadget
>> > driver was loaded and initialized.  Something simple like Robert
>> > Baldyga's patch is enough to do that.
>>
>> This simplicity is also a limitation. As I mentioned before,
>> sometimes it is
>> needed to be able to bind/unbind gadget driver multiple times to
>> different UDCs.
>> A real case I faced recently is SoC with HighSpeed-only and
>> SuperSpeed-only
>> UDCs. SuperSpeed-only UDC can't work on USB 2.0 speeds and vice
>> versa.
>> The system has USB3.0 USB connector with soldered HS lines from
>> mentioned HS-only and SS lines from SS-only UDCs. Each UDC has it's
>> own
>> device driver, so if we want to use both of them with one gadget
>> driver, we
>> must be able to bind/unbind it multiple times to different UDCs.
>> Another one usecase is users who can unload udc drivers without
>> carrying
>> about gadget drivers.
>> Third usecase is, for example, developers who can switch between
>> dummy_hcd
>> and real UDC hardware without unloading gadget driver.
>>
>
> Just a stupid question. Why don't you use configfs composite gadget
> instead of legacy gadgets?
>
> In my opinion all things which you have described are working out-of-box
> when you use configfs interface. It's mostly ready so you may create
> equivalent of most legacy gadgets (apart from printer and tcm) and
> just bind from one udc to another whenever you want.

It's because legacy gadgets are easy to use and usually don't need any
userspace-side configuration. Are there any plans to remove legacy
drivers in the future?

Best regards,
Ruslan

>
> Cheers,
>
> --
> Krzysztof Opasiak
> Samsung R&D Institute Poland
> Samsung Electronics
> k.opasiak@samsung.com
>
>
>
>



-- 
Best regards,
Ruslan Bilvol