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

[Tanya Brokhman]

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.
The Qualcomm Innovation Center, Inc. is a member of the Code Aurora Forum.




--
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/