Re: [RFC/PATCH 2/2] usb:gadget: Add SuperSpeed support to the Gadget Framework

[David Brownell]

--- On Sun, 10/3/10, tlinder <tlinder@xxxxxxxxxxxxxx> wrote:

> In order not to force all the FDs to supply 

What do File Descriptors have to do with this?

If you don't mean FD == File Descriptor, then
please spell out what you do mean, instead of
trying to repurpose a widely used abbreviation.


SuperSpeed
> descriptors when
> operating in SuperSpeed mode the following approach was
> taken:
> If we're operating in SuperSpeed mode and the FD didn't
> supply SuperSpeed
> descriptors, the composite layer will automatically create
> SuperSpeed
> descriptors with default values.

That bothers me in two ways.  First, I want to
see a solution that maintains today's policy where
the composite framework is optional for all gadget
drivers.

Second, that kind of automagic bothers me.

What could be wrong with expecting gadget
drivers to provide all the descriptors they need,
instead of introducing automagic?


> Support for new SuperSpeed BOS descriptor was 

Wireless USB BOS descriptors exist too, yes?  Does
this approach cover them, or just SuperSpeed?  (We
may someday want to support Wireless USB on the
peripheral/gadget side too...



> +++++++++++++++++++++++++++++++++++++---
>  include/linux/usb/ch9.h

I like to see patches related to USB-IF formats
and protocols be separate from functional changes
in the USB stack or its drivers. which may rely
on those formats/protocols; less entanglement.
       
> +

> +config USB_GADGET_SUPERSPEED
> +    boolean "Gadget opperating in Super
> Speed"
> +    depends on USB_GADGET
> +    depends on USB_GADGET_DUALSPEED
> +    default n
> +    help
> +      Enabling this feature enables
> Super Speed support in the Gadget
> +      driver. It means that gadget
> drivers should include extra (SuperSpeed)
> +      descriptors.

That is:  the automagic isn't needed.  The
concepts in this patch seem to be a bit on
the self-contradictory side...

ep_comp_desc = {
> +        .bDescriptorType =
> USB_DT_SS_ENDPOINT_COMP,
> +        .bLength = 0x06,
> +        .bMaxBurst = 0,
> /*the default is we don't support bursting*/

I've not followed the SuperSpeed stuff as closely
as I might, but ... doesn't bursting require some
hardware support?  So that not all UDC + driver
stacks can support it?   (That'd be a case, if so,
for more sanity checks ... and the gadget driver to
explicitly say if it handles bursting.



--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/