lkml.org 
[lkml]   [2011]   [Aug]   [23]   [last100]   RSS Feed
Views: [wrap][no wrap]   [headers]  [forward] 
 
Messages in this thread
/
SubjectRe: [PATCHv3 2/4] usb: gadget: replace "is_dualspeed" with "max_speed"
From
Date
On Tue, 23 Aug 2011 15:58:17 +0200, Felipe Balbi <balbi@ti.com> wrote:

>> On Sat, 20 Aug 2011 01:28:06 +0200, Felipe Balbi <balbi@ti.com> wrote:
>>> IMHO the logic is inverted here. It should start from the function
>>> drivers. They should say which USB speeds they support, that would go
>>> up to composite layer and composite would call e.g.
>>> usb_gadget_set_speed(gadget, maximum_speed);
>>
>> This is actually not how composite works at the moment. Currently,
>
> my suggestion was exactly to change that :-) Speed is something
> functions support. composite.c shouldn't dictate which speed functions
> should use, rather composite.c should use the maximum speed which all
> functions support.

Strictly speaking, composite.c does not dictate anything. It just copies
what the composite gadget driver declared as maximum speed.

My understanding was that one could consciously create a composite gadget
such that not all of the functions support all of the speeds.

I would suggest leaving everything as is, expect if usb_composite_driver's
max_speed is set to USB_SPEED_UNKNOWN in which case composite would iterate
over all the functions and figure out the maximum speed that all of the
functions support.

>> a composite gadget can declare a maximum speed of say “high” even if
>> all the functions do not support that speed. Of course when host asks
>> about descriptors for given speed, only functions that support that
>> speed will be returned (and hence only configurations that have at
>> least one function supporting that speed).
>>
>> Whether the behaviour should be changed is, in my opinion, issue
>> separate from the patchset that I'm sending.
>
> I wouldn't say that, actually. Just replacing is_dualspeed with
> max_speed isn't changing much and if you want to make that part of the
> code better, why not doing The Right Thing(TM) ?

I'm just saying that the main reason for this patchset is to get rid of
the USB_GADGET_DUALSPEED and USB_GADGET_SUPERSPEED Kconfig options. So
the purpose of changing is_dualspeed with max_speed is to be able to
check if gadget is super speed at run-time so that gadget_is_superspeed()
can be implemented.

So I would like to just get this done and then figure out what to do with
composite.c. How does that sound?

> All of the speed negotiation between composite.c and f_*.c should happen
> before even connecting to host

Yep, obviously. The usb_gadget_probe_driver() is called at the very and
once all the functions and everything is added so composite.c can do all
the analysis it wants and figure out the maximum speed.

> (before attaching data pullups, enabling IRQs, etc), that's exactly why
> me and Sebastian have decided (at that time off list) to add
> udc_start()/udc_stop() methods.

I don't really follow why those would be needed...

> One of the reason was that it would be a quite intrusive change to
> all UDC drivers, second we wanted to give maintainers/authors of
> those UDC drivers some grace period for the change, third when
> all UDCs are converted, it allow us to do the speed negotiation
> before connecting to host.

Again, I don't follow. We can figure out the max_speed before calling
usb_gadget_probe_driver() just fine. We don't even have to have UDC
to figure that out (ie. gadget driver's max_speed does not change
depending on UDC, right?).

>>> how about "current_speed" ?
>>
>> Is there a big advantage? That would change external interface and I
>> don't see reason to do so. Of course, udc class is quite recent so if
>> you feel we can ignore this issue I can go forward with that change.

> you already maximum_speed (below) and speed alone looses some extra
> hint of what kind of information will be there. I think it's better to
> change this to current_speed and make a symbolic link called 'speed'
> which we can keep for the next 5 years and remove it in e.g. Linux v5.0

OK, I'll do that (as soon as I figure out/recall how to make symlinks that
is ;) ).

--
Best regards, _ _
.o. | Liege of Serenely Enlightened Majesty of o' \,=./ `o
..o | Computer Science, Michal "mina86" Nazarewicz (o o)
ooo +-----<email/xmpp: mnazarewicz@google.com>-----ooO--(_)--Ooo--
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/
\
 
 \ /
  Last update: 2011-08-23 16:17    [from the cache]
©2003-2011 Jasper Spaans