Messages in this thread Patch in this message | | | Date | Thu, 18 Aug 2011 11:01:11 +0800 | From | Yang Rui Rui <> | Subject | Re: [PATCHv2] usb: gadget: get rid of USB_GADGET_DUALSPEED and USB_GADGET_SUPERSPEED |
| |
On 08/17/2011 11:33 PM, Michal Nazarewicz wrote: > From: Michal Nazarewicz<mina86@mina86.com> > > This commit removes the use of USB_GADGET_DUALSPEED and > USB_GADGET_SUPERSPEED Kconfig options. Those were selected > by UDC drivers which supported respective speeds. > > However, since kernel now allows multiple UDC drivers to be > compiled, the options in question may no longer reflect the > state of all gadgets. > > For instance, if one driver that supports dual speed is selected > and another that does not, the USB_GADGE_DUALSPEED will be set > "for both". > > This commit replaces all the #ifdefs by a run-time checks made > by calling gadget_is_dualspeed().
I did not see the include/linux/usb/gadget.h changes, did you have another patch? There's CONFIG_USB_GADGET_DUALSPEED ifdefs in gadget_is_dualspeed function of the gadget.h If so, I will get oops as before.
> --- > drivers/usb/gadget/Kconfig | 30 ------------------------- > drivers/usb/gadget/composite.c | 9 +------- > drivers/usb/gadget/file_storage.c | 4 --- > drivers/usb/gadget/inode.c | 10 -------- > drivers/usb/gadget/printer.c | 43 ++++++------------------------------- > drivers/usb/gadget/u_ether.c | 7 ------ > 6 files changed, 8 insertions(+), 95 deletions(-) > > Like before, this has been only compile-tested (make allmodconfig > bzImage modules on x86_64). > > Compared to v1: > * Removed unrelated whitespace changes > * Removed the use of min() in usb_composite_probe() > > diff --git a/drivers/usb/gadget/Kconfig b/drivers/usb/gadget/Kconfig > index 5a084b9..c60c167 100644 > --- a/drivers/usb/gadget/Kconfig > +++ b/drivers/usb/gadget/Kconfig > @@ -133,7 +133,6 @@ config USB_AT91 > > config USB_ATMEL_USBA > tristate "Atmel USBA" > - select USB_GADGET_DUALSPEED > depends on AVR32 || ARCH_AT91CAP9 || ARCH_AT91SAM9RL || ARCH_AT91SAM9G45 > help > USBA is the integrated high-speed USB Device controller on > @@ -142,7 +141,6 @@ config USB_ATMEL_USBA > config USB_FSL_USB2 > tristate "Freescale Highspeed USB DR Peripheral Controller" > depends on FSL_SOC || ARCH_MXC > - select USB_GADGET_DUALSPEED > select USB_FSL_MPH_DR_OF if OF > help > Some of Freescale PowerPC processors have a High Speed > @@ -158,7 +156,6 @@ config USB_FSL_USB2 > config USB_FUSB300 > tristate "Faraday FUSB300 USB Peripheral Controller" > depends on !PHYS_ADDR_T_64BIT > - select USB_GADGET_DUALSPEED > help > Faraday usb device controller FUSB300 driver > > @@ -206,7 +203,6 @@ config USB_PXA25X_SMALL > > config USB_R8A66597 > tristate "Renesas R8A66597 USB Peripheral Controller" > - select USB_GADGET_DUALSPEED > help > R8A66597 is a discrete USB host and peripheral controller chip that > supports both full and high speed USB 2.0 data transfers. > @@ -220,7 +216,6 @@ config USB_RENESAS_USBHS_UDC > tristate 'Renesas USBHS controller' > depends on SUPERH || ARCH_SHMOBILE > depends on USB_RENESAS_USBHS > - select USB_GADGET_DUALSPEED > help > Renesas USBHS is a discrete USB host and peripheral controller chip > that supports both full and high speed USB 2.0 data transfers. > @@ -249,7 +244,6 @@ config USB_S3C_HSOTG > tristate "S3C HS/OtG USB Device controller" > depends on S3C_DEV_USB_HSOTG > select USB_GADGET_S3C_HSOTG_PIO > - select USB_GADGET_DUALSPEED > help > The Samsung S3C64XX USB2.0 high-speed gadget controller > integrated into the S3C64XX series SoC. > @@ -287,7 +281,6 @@ config USB_S3C2410_DEBUG > config USB_S3C_HSUDC > tristate "S3C2416, S3C2443 and S3C2450 USB Device Controller" > depends on ARCH_S3C2410 > - select USB_GADGET_DUALSPEED > help > Samsung's S3C2416, S3C2443 and S3C2450 is an ARM9 based SoC > integrated with dual speed USB 2.0 device controller. It has > @@ -298,7 +291,6 @@ config USB_S3C_HSUDC > config USB_PXA_U2O > tristate "PXA9xx Processor USB2.0 controller" > depends on ARCH_MMP > - select USB_GADGET_DUALSPEED > help > PXA9xx Processor series include a high speed USB2.0 device > controller, which support high speed and full speed USB peripheral. > @@ -311,14 +303,12 @@ config USB_PXA_U2O > config USB_GADGET_MUSB_HDRC > tristate "Inventra HDRC USB Peripheral (TI, ADI, ...)" > depends on USB_MUSB_HDRC > - select USB_GADGET_DUALSPEED > help > This OTG-capable silicon IP is used in dual designs including > the TI DaVinci, OMAP 243x, OMAP 343x, TUSB 6010, and ADI Blackfin > > config USB_M66592 > tristate "Renesas M66592 USB Peripheral Controller" > - select USB_GADGET_DUALSPEED > help > M66592 is a discrete USB peripheral controller chip that > supports both full and high speed USB 2.0 data transfers. > @@ -335,7 +325,6 @@ config USB_M66592 > config USB_AMD5536UDC > tristate "AMD5536 UDC" > depends on PCI > - select USB_GADGET_DUALSPEED > help > The AMD5536 UDC is part of the AMD Geode CS5536, an x86 southbridge. > It is a USB Highspeed DMA capable USB device controller. Beside ep0 > @@ -363,7 +352,6 @@ config USB_FSL_QE > config USB_CI13XXX_PCI > tristate "MIPS USB CI13xxx PCI UDC" > depends on PCI > - select USB_GADGET_DUALSPEED > help > MIPS USB IP core family device controller > Currently it only supports IP part number CI13412 > @@ -374,7 +362,6 @@ config USB_CI13XXX_PCI > > config USB_NET2272 > tristate "PLX NET2272" > - select USB_GADGET_DUALSPEED > help > PLX NET2272 is a USB peripheral controller which supports > both full and high speed USB 2.0 data transfers. > @@ -398,7 +385,6 @@ config USB_NET2272_DMA > config USB_NET2280 > tristate "NetChip 228x" > depends on PCI > - select USB_GADGET_DUALSPEED > help > NetChip 2280 / 2282 is a PCI based USB peripheral controller which > supports both full and high speed USB 2.0 data transfers. > @@ -429,7 +415,6 @@ config USB_LANGWELL > tristate "Intel Langwell USB Device Controller" > depends on PCI > depends on !PHYS_ADDR_T_64BIT > - select USB_GADGET_DUALSPEED > help > Intel Langwell USB Device Controller is a High-Speed USB > On-The-Go device controller. > @@ -444,7 +429,6 @@ config USB_LANGWELL > config USB_EG20T > tristate "Intel EG20T PCH/OKI SEMICONDUCTOR ML7213 IOH UDC" > depends on PCI > - select USB_GADGET_DUALSPEED > help > This is a USB device driver for EG20T PCH. > EG20T PCH is the platform controller hub that is used in Intel's > @@ -466,7 +450,6 @@ config USB_EG20T > config USB_CI13XXX_MSM > tristate "MIPS USB CI13xxx for MSM" > depends on ARCH_MSM > - select USB_GADGET_DUALSPEED > select USB_MSM_OTG > help > MSM SoC has chipidea USB controller. This driver uses > @@ -487,8 +470,6 @@ config USB_CI13XXX_MSM > config USB_DUMMY_HCD > tristate "Dummy HCD (DEVELOPMENT)" > depends on USB=y || (USB=m&& USB_GADGET=m) > - select USB_GADGET_DUALSPEED > - select USB_GADGET_SUPERSPEED > help > This host controller driver emulates USB, looping all data transfer > requests back to a USB "gadget driver" in the same host. The host > @@ -513,17 +494,6 @@ config USB_DUMMY_HCD > > endchoice > > -# Selected by UDC drivers that support high-speed operation. > -config USB_GADGET_DUALSPEED > - bool > - depends on USB_GADGET > - > -# Selected by UDC drivers that support super-speed opperation > -config USB_GADGET_SUPERSPEED > - bool > - depends on USB_GADGET > - depends on USB_GADGET_DUALSPEED > - > # > # USB Gadget Drivers > # > diff --git a/drivers/usb/gadget/composite.c b/drivers/usb/gadget/composite.c > index 5a3461e..c5249b3 100644 > --- a/drivers/usb/gadget/composite.c > +++ b/drivers/usb/gadget/composite.c > @@ -1559,12 +1559,6 @@ composite_resume(struct usb_gadget *gadget) > /*-------------------------------------------------------------------------*/ > > static struct usb_gadget_driver composite_driver = { > -#ifdef CONFIG_USB_GADGET_SUPERSPEED > - .speed = USB_SPEED_SUPER, > -#else > - .speed = USB_SPEED_HIGH, > -#endif > - > .unbind = composite_unbind, > > .setup = composite_setup, > @@ -1609,8 +1603,7 @@ int usb_composite_probe(struct usb_composite_driver *driver, > driver->iProduct = driver->name; > composite_driver.function = (char *) driver->name; > composite_driver.driver.name = driver->name; > - composite_driver.speed = min((u8)composite_driver.speed, > - (u8)driver->max_speed); > + composite_driver.speed = driver->max_speed;
This does not works, many drivers will check the driver->speed, ie in musg_gadget.c
static int musb_gadget_start(struct usb_gadget *g, struct usb_gadget_driver *driver) { struct musb *musb = gadget_to_musb(g); unsigned long flags; int retval = -EINVAL;
if (driver->speed != USB_SPEED_HIGH) goto err0; [snip]
> composite = driver; > composite_gadget_bind = bind; > > diff --git a/drivers/usb/gadget/file_storage.c b/drivers/usb/gadget/file_storage.c > index cf875be..61ed4ec 100644 > --- a/drivers/usb/gadget/file_storage.c > +++ b/drivers/usb/gadget/file_storage.c > @@ -3562,11 +3562,7 @@ static void fsg_resume(struct usb_gadget *gadget) > /*-------------------------------------------------------------------------*/ > > static struct usb_gadget_driver fsg_driver = { > -#ifdef CONFIG_USB_GADGET_DUALSPEED > .speed = USB_SPEED_HIGH, > -#else > - .speed = USB_SPEED_FULL, > -#endif > .function = (char *) fsg_string_product, > .unbind = fsg_unbind, > .disconnect = fsg_disconnect, > diff --git a/drivers/usb/gadget/inode.c b/drivers/usb/gadget/inode.c > index 1b24099..dddb3e4 100644 > --- a/drivers/usb/gadget/inode.c > +++ b/drivers/usb/gadget/inode.c > @@ -837,7 +837,6 @@ ep_config (struct file *fd, const char __user *buf, size_t len, loff_t *ptr) > if (value == 0) > data->state = STATE_EP_ENABLED; > break; > -#ifdef CONFIG_USB_GADGET_DUALSPEED > case USB_SPEED_HIGH: > /* fails if caller didn't provide that descriptor... */ > ep->desc =&data->hs_desc; > @@ -845,7 +844,6 @@ ep_config (struct file *fd, const char __user *buf, size_t len, loff_t *ptr) > if (value == 0) > data->state = STATE_EP_ENABLED; > break; > -#endif > default: > DBG(data->dev, "unconnected, %s init abandoned\n", > data->name); > @@ -1331,7 +1329,6 @@ static const struct file_operations ep0_io_operations = { > * Unrecognized ep0 requests may be handled in user space. > */ > > -#ifdef CONFIG_USB_GADGET_DUALSPEED > static void make_qualifier (struct dev_data *dev) > { > struct usb_qualifier_descriptor qual; > @@ -1354,7 +1351,6 @@ static void make_qualifier (struct dev_data *dev) > > memcpy (dev->rbuf,&qual, sizeof qual); > } > -#endif > > static int > config_buf (struct dev_data *dev, u8 type, unsigned index) > @@ -1434,7 +1430,6 @@ gadgetfs_setup (struct usb_gadget *gadget, const struct usb_ctrlrequest *ctrl) > dev->dev->bMaxPacketSize0 = dev->gadget->ep0->maxpacket; > req->buf = dev->dev; > break; > -#ifdef CONFIG_USB_GADGET_DUALSPEED > case USB_DT_DEVICE_QUALIFIER: > if (!dev->hs_config) > break; > @@ -1444,7 +1439,6 @@ gadgetfs_setup (struct usb_gadget *gadget, const struct usb_ctrlrequest *ctrl) > break; > case USB_DT_OTHER_SPEED_CONFIG: > // FALLTHROUGH > -#endif > case USB_DT_CONFIG: > value = config_buf (dev, > w_value>> 8, > @@ -1773,11 +1767,7 @@ gadgetfs_suspend (struct usb_gadget *gadget) > } > > static struct usb_gadget_driver gadgetfs_driver = { > -#ifdef CONFIG_USB_GADGET_DUALSPEED > .speed = USB_SPEED_HIGH, > -#else > - .speed = USB_SPEED_FULL, > -#endif > .function = (char *) driver_desc, > .unbind = gadgetfs_unbind, > .setup = gadgetfs_setup, > diff --git a/drivers/usb/gadget/printer.c b/drivers/usb/gadget/printer.c > index a341dde..901b7ba 100644 > --- a/drivers/usb/gadget/printer.c > +++ b/drivers/usb/gadget/printer.c > @@ -164,12 +164,6 @@ module_param(qlen, uint, S_IRUGO|S_IWUSR); > > #define QLEN qlen > > -#ifdef CONFIG_USB_GADGET_DUALSPEED > -#define DEVSPEED USB_SPEED_HIGH > -#else /* full speed (low speed doesn't do bulk) */ > -#define DEVSPEED USB_SPEED_FULL > -#endif > - > /*-------------------------------------------------------------------------*/ > > #define xprintk(d, level, fmt, args...) \ > @@ -288,8 +282,6 @@ static const struct usb_descriptor_header *fs_printer_function [11] = { > NULL > }; > > -#ifdef CONFIG_USB_GADGET_DUALSPEED > - > /* > * usb 2.0 devices need to expose both high speed and full speed > * descriptors, unless they only run at full speed. > @@ -328,13 +320,6 @@ static const struct usb_descriptor_header *hs_printer_function [11] = { > /* maxpacket and other transfer characteristics vary by speed. */ > #define ep_desc(g, hs, fs) (((g)->speed == USB_SPEED_HIGH)?(hs):(fs)) > > -#else > - > -/* if there's no high speed support, maxpacket doesn't change. */ > -#define ep_desc(g, hs, fs) (((void)(g)), (fs)) > - > -#endif /* !CONFIG_USB_GADGET_DUALSPEED */ > - > /*-------------------------------------------------------------------------*/ > > /* descriptors that are built on-demand */ > @@ -979,9 +964,7 @@ printer_set_config(struct printer_dev *dev, unsigned number) > > switch (gadget->speed) { > case USB_SPEED_FULL: speed = "full"; break; > -#ifdef CONFIG_USB_GADGET_DUALSPEED > case USB_SPEED_HIGH: speed = "high"; break; > -#endif > default: speed = "?"; break; > } > > @@ -998,23 +981,15 @@ config_buf(enum usb_device_speed speed, u8 *buf, u8 type, unsigned index, > { > int len; > const struct usb_descriptor_header **function; > -#ifdef CONFIG_USB_GADGET_DUALSPEED > int hs = (speed == USB_SPEED_HIGH); > > + if (index>= device_desc.bNumConfigurations) > + return -EINVAL; > + > if (type == USB_DT_OTHER_SPEED_CONFIG) > hs = !hs; > > - if (hs) { > - function = hs_printer_function; > - } else { > - function = fs_printer_function; > - } > -#else > - function = fs_printer_function; > -#endif > - > - if (index>= device_desc.bNumConfigurations) > - return -EINVAL; > + function = hs ? hs_printer_function : fs_printer_function; > > /* for now, don't advertise srp-only devices */ > if (!is_otg) > @@ -1156,9 +1131,8 @@ printer_setup(struct usb_gadget *gadget, const struct usb_ctrlrequest *ctrl) > value = min(wLength, (u16) sizeof device_desc); > memcpy(req->buf,&device_desc, value); > break; > -#ifdef CONFIG_USB_GADGET_DUALSPEED > case USB_DT_DEVICE_QUALIFIER: > - if (!gadget->is_dualspeed) > + if (!gadget_is_dualspeed(gadget)) > break; > /* > * assumes ep0 uses the same value for both > @@ -1172,10 +1146,9 @@ printer_setup(struct usb_gadget *gadget, const struct usb_ctrlrequest *ctrl) > break; > > case USB_DT_OTHER_SPEED_CONFIG: > - if (!gadget->is_dualspeed) > + if (!gadget_is_dualspeed(gadget)) > break; > /* FALLTHROUGH */ > -#endif /* CONFIG_USB_GADGET_DUALSPEED */ > case USB_DT_CONFIG: > value = config_buf(gadget->speed, req->buf, > wValue>> 8, > @@ -1460,11 +1433,9 @@ autoconf_fail: > goto autoconf_fail; > out_ep->driver_data = out_ep; /* claim */ > > -#ifdef CONFIG_USB_GADGET_DUALSPEED > /* assumes that all endpoints are dual-speed */ > hs_ep_in_desc.bEndpointAddress = fs_ep_in_desc.bEndpointAddress; > hs_ep_out_desc.bEndpointAddress = fs_ep_out_desc.bEndpointAddress; > -#endif /* DUALSPEED */ > > usb_gadget_set_selfpowered(gadget); > > @@ -1552,7 +1523,7 @@ fail: > /*-------------------------------------------------------------------------*/ > > static struct usb_gadget_driver printer_driver = { > - .speed = DEVSPEED, > + .speed = USB_SPEED_HIGH, > > .function = (char *) driver_desc, > .unbind = printer_unbind, > diff --git a/drivers/usb/gadget/u_ether.c b/drivers/usb/gadget/u_ether.c > index dfed4c1..8642abb 100644 > --- a/drivers/usb/gadget/u_ether.c > +++ b/drivers/usb/gadget/u_ether.c > @@ -92,17 +92,10 @@ struct eth_dev { > > #define DEFAULT_QLEN 2 /* double buffering by default */ > > - > -#ifdef CONFIG_USB_GADGET_DUALSPEED > - > static unsigned qmult = 5; > module_param(qmult, uint, S_IRUGO|S_IWUSR); > MODULE_PARM_DESC(qmult, "queue length multiplier at high/super speed"); > > -#else /* full speed (low speed doesn't do bulk) */ > -#define qmult 1 > -#endif > - > /* for dual-speed hardware, use deeper queues at high/super speed */ > static inline int qlen(struct usb_gadget *gadget) > { > -- > 1.7.3.1 >
-- Thanks Yang Ruirui
| |