RE: [PATCH v14 7/9] usb:gadget: Add SuperSpeed support to the Gadget Framework

Tanya Brokhman
2011-06-03T18:00:39Z

Hi Felipe,

> On Tue, May 31, 2011 at 04:18:27PM +0300, Tatyana Brokhman wrote:
> > @@ -377,9 +425,10 @@ static int config_desc(struct usb_composite_dev
> *cdev, unsigned w_value)
> > u8 type = w_value >> 8;
> > enum usb_device_speed speed = USB_SPEED_UNKNOWN;
> >
> > - if (gadget_is_dualspeed(gadget)) {
> > - int hs = 0;
> > -
> > + if (gadget->speed == USB_SPEED_SUPER)
>
> if (gadget_is_superspeed(gadget)) ??
I thought so too at first but then I decided that checking gadget->speed is
better here because it's possible that gadget_is_superspeed() will return
true but the actual speed will be for example HS. In that case we want to
enter the else of this if (else if (gadget_is_dualspeed()) {... })

> > @@ -413,16 +469,22 @@ static int count_configs(struct
> usb_composite_dev *cdev, unsigned type)
> > struct usb_configuration *c;
> > unsigned count = 0;
> > int hs = 0;
> > + int ss = 0;
> >
> > if (gadget_is_dualspeed(gadget)) {
> > if (gadget->speed == USB_SPEED_HIGH)
> > hs = 1;
> > + if (gadget->speed == USB_SPEED_SUPER)
> > + ss = 1;
>
> is this really correct under gadget_is_dualspeed() ?? puzzled.
I think so. The assumption is that it's not possible that gadget speed will
be super if the gadget isn't dualspeed. When the COMFIG_GADGET_SUPERSPEED
flag was changeable to the user, it was dependant on the gadget being dual
speed. Now this flag should be turned on by the UDCs that support SS, but
the assumption that they are also dual speed remains.
BTW, are you ok with the handling of driver->speed in usb_composite_probe()?
You didn't reply to the solution me and Alan came up with...

Thanks,
Tanya Brokhman
---
Sent by an consultant of the Qualcomm Innovation Center, Inc.