Hi, On Sat, Aug 20, 2011 at 12:33:00AM +0200, Michal Nazarewicz wrote: > From: Michal Nazarewicz <mina86@xxxxxxxxxx> > > This commit replaces usb_gadget's is_dualspeed field with > a max_speed field. > > This change is made so that one will be able to check at > run-time if given gadget supports super speed. 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); that function will set gadget->maximum_speed and call into the UDC driver which would write maximum_speed into its internal registers, thus preventing the core from working at speeds unsupported by the gadget driver. All of this should happen before we even connect to the host, so first all UDCs should have their conversion to udc-core finished and start using udc_start()/udc_stop(). The driver I just sent [1] illustrates what udc_start()/udc_stop() should do. [1] http://marc.info/?l=linux-usb&m=131376669022473&w=2 <snip> > diff --git a/drivers/usb/gadget/udc-core.c b/drivers/usb/gadget/udc-core.c > index e1ecdbc..25058b4 100644 > --- a/drivers/usb/gadget/udc-core.c > +++ b/drivers/usb/gadget/udc-core.c > @@ -371,14 +371,28 @@ static ssize_t usb_udc_softconn_store(struct device *dev, > } > static DEVICE_ATTR(soft_connect, S_IWUSR, NULL, usb_udc_softconn_store); > > -static ssize_t usb_udc_speed_show(struct device *dev, > +#define USB_UDC_SPEED_ATTR(name) \ > +ssize_t usb_udc_##name##_show(struct device *dev, \ > + struct device_attribute *attr, char *buf) \ > +{ \ > + struct usb_udc *udc = container_of(dev, struct usb_udc, dev); \ > + return snprintf(buf, PAGE_SIZE, "%s\n", \ > + usb_device_speed_name(udc->gadget->name)); \ > +} \ > +static DEVICE_ATTR(name, S_IRUSR, usb_udc_##name##_show, NULL) > + > +static USB_UDC_SPEED_ATTR(speed); how about "current_speed" ? > +static USB_UDC_SPEED_ATTR(max_speed); in this case, humans will be reading sysfs, so maximum_speed will look nicer, IMHO. > +/* Provide "is_dualspeed" for backward compatibility. */ > +static ssize_t usb_udc_is_dualspeed_show(struct device *dev, > struct device_attribute *attr, char *buf) > { > - struct usb_udc *udc = container_of(dev, struct usb_udc, dev); > - return snprintf(buf, PAGE_SIZE, "%s\n", > - usb_device_speed_name(udc->gadget->speed)); > + struct usb_udc *udc = container_of(dev, struct usb_udc, dev); > + return snprintf(buf, PAGE_SIZE, "%d\n", > + gadget_is_dualspeed(udc->gadget)); > } > -static DEVICE_ATTR(speed, S_IRUSR, usb_udc_speed_show, NULL); > +static DEVICE_ATTR(is_dualspeed, S_IRUSR, usb_udc_is_dualspeed_show, NULL); maybe deprecate this one on feature-removal-schedule ?? -- balbi
Attachment:
signature.asc
Description: Digital signature
