Messages in this thread | | | Subject | Re: [PATCHv3 2/4] usb: gadget: replace "is_dualspeed" with "max_speed" | From | "Michal Nazarewicz" <> | Date | Tue, 23 Aug 2011 16:15:08 +0200 |
| |
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/
| |