linux-kernel.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
* [PATCHv4 0/4] Removing USB_GADGET_*SPEED macros
@ 2011-08-22 16:04 Michal Nazarewicz
  2011-08-22 16:04 ` [PATCHv4 1/4] usb: Provide usb_speed_string() function Michal Nazarewicz
                   ` (3 more replies)
  0 siblings, 4 replies; 15+ messages in thread
From: Michal Nazarewicz @ 2011-08-22 16:04 UTC (permalink / raw)
  To: Alan Stern, Felipe Balbi
  Cc: Sebastian Andrzej Siewior, Yang Rui Rui, Dave Young, linux-usb,
	linux-kernel

From: Michal Nazarewicz <mina86@mina86.com>

Michal Nazarewicz (4):
  usb: Provide usb_speed_string() function

    Just a little helper.

    Also removes some code duplication.

  usb: gadget: replace "is_dualspeed" with "max_speed"

    This replaces "is_dualspeed" with "max_speed" so that one
    can encode in the field whether given UDC is super speed
    as well.

    Without this change, we would have to add a "is_superspeed"
    field since after removal of USB_GADGET_SUPERSPEED Kconfig
    option current implementation of gadget_is_superspeed()
    won't work.

  usb: gadget: rename usb_gadget_driver::speed to max_speed

    I don't have strong feelings about this patch, but the current
    name of the usb_gadget_driver::speed field may lead to some
    confusion.

    Changing it to "max_speed" should be apparent that it describes
    the maximum speed the gadget driver supports.

    This comes in pair with usb_gadget::max_speed and
    usb_composite_driver::max_speed.

  usb: gadget: get rid of USB_GADGET_{DUAL,SUPER}SPEED

    Removes the USB_GADGET_DUALSPEED and USB_GADGET_SUPERSPEED Kconfig
    options.  In most cases, they were checked just for optimisation
    (ie. if UDC was not dual speed, gadget drivers could skip some of
    the logic) so in the end this change was just about deleting
    #ifdefs.


Making those changes has exposed problems with some UDC drivers,
namely:

$ g grep -A3 'max_speed.*[=!]=.*USB_SPEED_HIGH'
drivers/usb/gadget/amd5536udc.c:                        || driver->max_speed != USB_SPEED_HIGH)
drivers/usb/gadget/amd5536udc.c-                return -EINVAL;
drivers/usb/gadget/amd5536udc.c-        if (!dev)
drivers/usb/gadget/amd5536udc.c-                return -ENODEV;
--
drivers/usb/gadget/m66592-udc.c:                        || driver->max_speed != USB_SPEED_HIGH
drivers/usb/gadget/m66592-udc.c-                        || !bind
drivers/usb/gadget/m66592-udc.c-                        || !driver->setup)
drivers/usb/gadget/m66592-udc.c-                return -EINVAL;
--
drivers/usb/gadget/net2272.c:       driver->max_speed != USB_SPEED_HIGH)
drivers/usb/gadget/net2272.c-           return -EINVAL;
drivers/usb/gadget/net2272.c-   if (!dev)
drivers/usb/gadget/net2272.c-           return -ENODEV;
--
drivers/usb/gadget/net2280.c:                   || driver->max_speed != USB_SPEED_HIGH
drivers/usb/gadget/net2280.c-                   || !bind || !driver->setup)
drivers/usb/gadget/net2280.c-           return -EINVAL;
drivers/usb/gadget/net2280.c-   if (!dev)
--
drivers/usb/gadget/r8a66597-udc.c:                      || driver->max_speed != USB_SPEED_HIGH
drivers/usb/gadget/r8a66597-udc.c-                      || !bind
drivers/usb/gadget/r8a66597-udc.c-                      || !driver->setup)
drivers/usb/gadget/r8a66597-udc.c-              return -EINVAL;

All those drivers require usb_gadget_driver::max_speed to be set to
USB_SPEED_HIGH.  This basically means that gmidi (which sets it to
USB_SPEED_FULL), g_ether and g_zero (which set it to USB_SPEED_SUPER)
won't work with those UDCs:

$ g grep -e '\.max_speed.*=.*USB_SPEED_SUPER' -e '\.max_speed.*=.*USB_SPEED_FULL'
dummy_hcd.c:    dum->gadget.max_speed = USB_SPEED_SUPER;;
ether.c:        .max_speed      = USB_SPEED_SUPER,
gmidi.c:        .max_speed      = USB_SPEED_FULL,
goku_udc.c:     dev->gadget.max_speed = USB_SPEED_FULL;
omap_udc.c:     udc->gadget.max_speed = USB_SPEED_FULL;
zero.c: .max_speed      = USB_SPEED_SUPER,

So as far as I can tell, UDC drivers for those need to be fixed (but
that's probably scope for another patchset).  Or am I missing
something?


Like before, this has been compile-tested only.


 drivers/usb/Makefile                   |    3 ++
 drivers/usb/common.c                   |   24 ++++++++++++++
 drivers/usb/core/hub.c                 |   27 +++++----------
 drivers/usb/gadget/Kconfig             |   30 -----------------
 drivers/usb/gadget/amd5536udc.c        |   13 ++-----
 drivers/usb/gadget/atmel_usba_udc.c    |   11 +++---
 drivers/usb/gadget/ci13xxx_udc.c       |    9 +++--
 drivers/usb/gadget/composite.c         |   31 ++---------------
 drivers/usb/gadget/dbgp.c              |    2 +-
 drivers/usb/gadget/dummy_hcd.c         |   15 ++++-----
 drivers/usb/gadget/epautoconf.c        |    6 ++--
 drivers/usb/gadget/file_storage.c      |   21 +++---------
 drivers/usb/gadget/fsl_udc_core.c      |   55 +++++++++++--------------------
 drivers/usb/gadget/fusb300_udc.c       |    2 +-
 drivers/usb/gadget/gmidi.c             |   13 +------
 drivers/usb/gadget/goku_udc.c          |    3 +-
 drivers/usb/gadget/inode.c             |   17 +++-------
 drivers/usb/gadget/langwell_udc.c      |   52 ++++++++++-------------------
 drivers/usb/gadget/m66592-udc.c        |    4 +-
 drivers/usb/gadget/mv_udc_core.c       |    2 +-
 drivers/usb/gadget/net2272.c           |    8 ++--
 drivers/usb/gadget/net2280.c           |    8 ++---
 drivers/usb/gadget/omap_udc.c          |    1 +
 drivers/usb/gadget/pch_udc.c           |    4 +-
 drivers/usb/gadget/printer.c           |   55 ++++++--------------------------
 drivers/usb/gadget/r8a66597-udc.c      |    4 +-
 drivers/usb/gadget/s3c-hsotg.c         |   10 ++----
 drivers/usb/gadget/s3c-hsudc.c         |    2 +-
 drivers/usb/gadget/u_ether.c           |    7 ----
 drivers/usb/gadget/udc-core.c          |   39 +++++++++++-----------
 drivers/usb/misc/usbtest.c             |   21 +-----------
 drivers/usb/musb/musb_gadget.c         |    2 +-
 drivers/usb/renesas_usbhs/mod_gadget.c |    2 +-
 include/linux/usb/ch9.h                |   12 +++++++
 include/linux/usb/gadget.h             |   29 ++++-------------
 35 files changed, 190 insertions(+), 354 deletions(-)
 create mode 100644 drivers/usb/common.c

-- 
1.7.3.1


^ permalink raw reply	[flat|nested] 15+ messages in thread

* [PATCHv4 1/4] usb: Provide usb_speed_string() function
  2011-08-22 16:04 [PATCHv4 0/4] Removing USB_GADGET_*SPEED macros Michal Nazarewicz
@ 2011-08-22 16:04 ` Michal Nazarewicz
  2011-08-22 17:28   ` Alan Stern
  2011-08-22 16:04 ` [PATCHv4 2/4] usb: gadget: replace "is_dualspeed" with "max_speed" Michal Nazarewicz
                   ` (2 subsequent siblings)
  3 siblings, 1 reply; 15+ messages in thread
From: Michal Nazarewicz @ 2011-08-22 16:04 UTC (permalink / raw)
  To: Alan Stern, Felipe Balbi
  Cc: Sebastian Andrzej Siewior, Yang Rui Rui, Dave Young, linux-usb,
	linux-kernel

From: Michal Nazarewicz <mina86@mina86.com>

In a few places in the kernel, the code prints
a human-readable USB device speed (eg. "high speed").
This involves a switch statement sometimes wrapped
around in ({ ... }) block leading to code repetition.

To mitigate this issue, this commit introduces
usb_speed_string() function, which returns
a human-readable name of provided speed.

It also changes a few places switch was used to use
this new function.  This changes a bit the way the
speed is printed in few instances at the same time
standardising it.

Signed-off-by: Michal Nazarewicz <mina86@mina86.com>
---
 drivers/usb/Makefile                |    3 ++
 drivers/usb/common.c                |   24 ++++++++++++++++
 drivers/usb/core/hub.c              |   27 ++++++------------
 drivers/usb/gadget/amd5536udc.c     |    9 +----
 drivers/usb/gadget/atmel_usba_udc.c |    9 ++---
 drivers/usb/gadget/composite.c      |   22 ++------------
 drivers/usb/gadget/file_storage.c   |   15 ++-------
 drivers/usb/gadget/fsl_udc_core.c   |   53 ++++++++++++----------------------
 drivers/usb/gadget/gmidi.c          |   11 +------
 drivers/usb/gadget/langwell_udc.c   |   50 +++++++++++---------------------
 drivers/usb/gadget/net2272.c        |    4 +-
 drivers/usb/gadget/net2280.c        |    4 +--
 drivers/usb/gadget/printer.c        |   14 ++-------
 drivers/usb/gadget/s3c-hsotg.c      |    8 +----
 drivers/usb/gadget/udc-core.c       |   19 +-----------
 drivers/usb/misc/usbtest.c          |   21 +------------
 include/linux/usb/ch9.h             |   12 ++++++++
 17 files changed, 110 insertions(+), 195 deletions(-)
 create mode 100644 drivers/usb/common.c

diff --git a/drivers/usb/Makefile b/drivers/usb/Makefile
index 30ddf8d..a3167cc 100644
--- a/drivers/usb/Makefile
+++ b/drivers/usb/Makefile
@@ -51,3 +51,6 @@ obj-$(CONFIG_USB_MUSB_HDRC)	+= musb/
 obj-$(CONFIG_USB_RENESAS_USBHS)	+= renesas_usbhs/
 obj-$(CONFIG_USB_OTG_UTILS)	+= otg/
 obj-$(CONFIG_USB_GADGET)	+= gadget/
+
+obj-$(CONFIG_USB)		+= common.o
+obj-$(CONFIG_USB_GADGET)	+= common.o
diff --git a/drivers/usb/common.c b/drivers/usb/common.c
new file mode 100644
index 0000000..2f6627c
--- /dev/null
+++ b/drivers/usb/common.c
@@ -0,0 +1,24 @@
+/*
+ * Provides code common for host and device side USB.
+ */
+
+#include <linux/kernel.h>  /* for ARRAY_SIZE() */
+#include <linux/module.h>  /* for EXPORT_SYMBOL_GPL() */
+#include <linux/usb/ch9.h>
+
+const char *usb_speed_string(enum usb_device_speed speed)
+{
+	static const char *const names[] = {
+		[USB_SPEED_UNKNOWN] = "UNKNOWN",
+		[USB_SPEED_LOW] = "low-speed",
+		[USB_SPEED_FULL] = "full-speed",
+		[USB_SPEED_HIGH] = "high-speed",
+		[USB_SPEED_WIRELESS] = "wireless",
+		[USB_SPEED_SUPER] = "super-speed",
+	};
+
+	if (speed < 0 || speed >= ARRAY_SIZE(names))
+		speed = USB_SPEED_UNKNOWN;
+	return names[speed];
+}
+EXPORT_SYMBOL_GPL(usb_speed_string);
diff --git a/drivers/usb/core/hub.c b/drivers/usb/core/hub.c
index a428aa0..8614cbe 100644
--- a/drivers/usb/core/hub.c
+++ b/drivers/usb/core/hub.c
@@ -2798,7 +2798,7 @@ hub_port_init (struct usb_hub *hub, struct usb_device *udev, int port1,
 	int			i, j, retval;
 	unsigned		delay = HUB_SHORT_RESET_TIME;
 	enum usb_device_speed	oldspeed = udev->speed;
-	char 			*speed, *type;
+	const char		*speed;
 	int			devnum = udev->devnum;
 
 	/* root hub ports have a slightly longer reset period
@@ -2858,25 +2858,16 @@ hub_port_init (struct usb_hub *hub, struct usb_device *udev, int port1,
 	default:
 		goto fail;
 	}
- 
-	type = "";
-	switch (udev->speed) {
-	case USB_SPEED_LOW:	speed = "low";	break;
-	case USB_SPEED_FULL:	speed = "full";	break;
-	case USB_SPEED_HIGH:	speed = "high";	break;
-	case USB_SPEED_SUPER:
-				speed = "super";
-				break;
-	case USB_SPEED_WIRELESS:
-				speed = "variable";
-				type = "Wireless ";
-				break;
-	default: 		speed = "?";	break;
-	}
+
+	if (udev->speed == USB_SPEED_WIRELESS)
+		speed = "variable speed Wireless";
+	else
+		speed = usb_speed_string(udev->speed);
+
 	if (udev->speed != USB_SPEED_SUPER)
 		dev_info(&udev->dev,
-				"%s %s speed %sUSB device number %d using %s\n",
-				(udev->config) ? "reset" : "new", speed, type,
+				"%s %s USB device number %d using %s\n",
+				(udev->config) ? "reset" : "new", speed,
 				devnum, udev->bus->controller->driver->name);
 
 	/* Set up TT records, if needed  */
diff --git a/drivers/usb/gadget/amd5536udc.c b/drivers/usb/gadget/amd5536udc.c
index 70f2b37..397c9b9 100644
--- a/drivers/usb/gadget/amd5536udc.c
+++ b/drivers/usb/gadget/amd5536udc.c
@@ -3014,13 +3014,8 @@ __acquires(dev->lock)
 
 		/* link up all endpoints */
 		udc_setup_endpoints(dev);
-		if (dev->gadget.speed == USB_SPEED_HIGH) {
-			dev_info(&dev->pdev->dev, "Connect: speed = %s\n",
-				"high");
-		} else if (dev->gadget.speed == USB_SPEED_FULL) {
-			dev_info(&dev->pdev->dev, "Connect: speed = %s\n",
-				"full");
-		}
+		dev_info(&dev->pdev->dev, "Connect: %s\n",
+			 usb_speed_string(dev->gadget.speed));
 
 		/* init ep 0 */
 		activate_control_endpoints(dev);
diff --git a/drivers/usb/gadget/atmel_usba_udc.c b/drivers/usb/gadget/atmel_usba_udc.c
index 5b1665e..2507a34 100644
--- a/drivers/usb/gadget/atmel_usba_udc.c
+++ b/drivers/usb/gadget/atmel_usba_udc.c
@@ -1718,13 +1718,12 @@ static irqreturn_t usba_udc_irq(int irq, void *devid)
 			spin_lock(&udc->lock);
 		}
 
-		if (status & USBA_HIGH_SPEED) {
-			DBG(DBG_BUS, "High-speed bus reset detected\n");
+		if (status & USBA_HIGH_SPEED)
 			udc->gadget.speed = USB_SPEED_HIGH;
-		} else {
-			DBG(DBG_BUS, "Full-speed bus reset detected\n");
+		else
 			udc->gadget.speed = USB_SPEED_FULL;
-		}
+		DBG(DBG_BUS, "%s bus reset detected\n",
+		    usb_speed_string(udc->gadget.speed));
 
 		ep0 = &usba_ep[0];
 		ep0->desc = &usba_ep0_desc;
diff --git a/drivers/usb/gadget/composite.c b/drivers/usb/gadget/composite.c
index 5a3461e..0d03936 100644
--- a/drivers/usb/gadget/composite.c
+++ b/drivers/usb/gadget/composite.c
@@ -627,25 +627,9 @@ static int set_config(struct usb_composite_dev *cdev,
 		result = 0;
 	}
 
-	INFO(cdev, "%s speed config #%d: %s\n",
-		({ char *speed;
-		switch (gadget->speed) {
-		case USB_SPEED_LOW:
-			speed = "low";
-			break;
-		case USB_SPEED_FULL:
-			speed = "full";
-			break;
-		case USB_SPEED_HIGH:
-			speed = "high";
-			break;
-		case USB_SPEED_SUPER:
-			speed = "super";
-			break;
-		default:
-			speed = "?";
-			break;
-		} ; speed; }), number, c ? c->label : "unconfigured");
+	INFO(cdev, "%s config #%d: %s\n",
+	     usb_speed_string(gadget->speed),
+	     number, c ? c->label : "unconfigured");
 
 	if (!c)
 		goto done;
diff --git a/drivers/usb/gadget/file_storage.c b/drivers/usb/gadget/file_storage.c
index cf875be..fede982 100644
--- a/drivers/usb/gadget/file_storage.c
+++ b/drivers/usb/gadget/file_storage.c
@@ -2863,17 +2863,10 @@ static int do_set_config(struct fsg_dev *fsg, u8 new_config)
 		fsg->config = new_config;
 		if ((rc = do_set_interface(fsg, 0)) != 0)
 			fsg->config = 0;	// Reset on errors
-		else {
-			char *speed;
-
-			switch (fsg->gadget->speed) {
-			case USB_SPEED_LOW:	speed = "low";	break;
-			case USB_SPEED_FULL:	speed = "full";	break;
-			case USB_SPEED_HIGH:	speed = "high";	break;
-			default: 		speed = "?";	break;
-			}
-			INFO(fsg, "%s speed config #%d\n", speed, fsg->config);
-		}
+		else
+			INFO(fsg, "%s config #%d\n",
+			     usb_speed_string(fsg->gadget->speed),
+			     fsg->config);
 	}
 	return rc;
 }
diff --git a/drivers/usb/gadget/fsl_udc_core.c b/drivers/usb/gadget/fsl_udc_core.c
index de24a42..a3ce075 100644
--- a/drivers/usb/gadget/fsl_udc_core.c
+++ b/drivers/usb/gadget/fsl_udc_core.c
@@ -1715,34 +1715,31 @@ static void dtd_complete_irq(struct fsl_udc *udc)
 	}
 }
 
+static inline enum usb_device_speed portscx_device_speed(u32 reg)
+{
+	switch (speed & PORTSCX_PORT_SPEED_MASK) {
+	case PORTSCX_PORT_SPEED_HIGH:
+		return USB_SPEED_HIGH;
+	case PORTSCX_PORT_SPEED_FULL:
+		return USB_SPEED_FULL;
+	case PORTSCX_PORT_SPEED_LOW:
+		return USB_SPEED_LOW;
+	default:
+		return USB_SPEED_UNKNOWN;
+	}
+}
+
 /* Process a port change interrupt */
 static void port_change_irq(struct fsl_udc *udc)
 {
-	u32 speed;
-
 	if (udc->bus_reset)
 		udc->bus_reset = 0;
 
 	/* Bus resetting is finished */
-	if (!(fsl_readl(&dr_regs->portsc1) & PORTSCX_PORT_RESET)) {
+	if (!(fsl_readl(&dr_regs->portsc1) & PORTSCX_PORT_RESET))
 		/* Get the speed */
-		speed = (fsl_readl(&dr_regs->portsc1)
-				& PORTSCX_PORT_SPEED_MASK);
-		switch (speed) {
-		case PORTSCX_PORT_SPEED_HIGH:
-			udc->gadget.speed = USB_SPEED_HIGH;
-			break;
-		case PORTSCX_PORT_SPEED_FULL:
-			udc->gadget.speed = USB_SPEED_FULL;
-			break;
-		case PORTSCX_PORT_SPEED_LOW:
-			udc->gadget.speed = USB_SPEED_LOW;
-			break;
-		default:
-			udc->gadget.speed = USB_SPEED_UNKNOWN;
-			break;
-		}
-	}
+		udc->gadget.speed =
+			portscx_device_speed(fsl_readl(&dr_regs->portsc1));
 
 	/* Update USB state */
 	if (!udc->resume_state)
@@ -2167,20 +2164,8 @@ static int fsl_proc_read(char *page, char **start, off_t off, int count,
 			default:
 				s = "None"; break;
 			}
-			s;} ), ( {
-			char *s;
-			switch (tmp_reg & PORTSCX_PORT_SPEED_UNDEF) {
-			case PORTSCX_PORT_SPEED_FULL:
-				s = "Full Speed"; break;
-			case PORTSCX_PORT_SPEED_LOW:
-				s = "Low Speed"; break;
-			case PORTSCX_PORT_SPEED_HIGH:
-				s = "High Speed"; break;
-			default:
-				s = "Undefined"; break;
-			}
-			s;
-		} ),
+			s;} ),
+		usb_speed_string(portscx_device_speed(tmp_reg)),
 		(tmp_reg & PORTSCX_PHY_LOW_POWER_SPD) ?
 		"Normal PHY mode" : "Low power mode",
 		(tmp_reg & PORTSCX_PORT_RESET) ? "In Reset" :
diff --git a/drivers/usb/gadget/gmidi.c b/drivers/usb/gadget/gmidi.c
index 305a8a8..6c3d213 100644
--- a/drivers/usb/gadget/gmidi.c
+++ b/drivers/usb/gadget/gmidi.c
@@ -641,17 +641,8 @@ gmidi_set_config(struct gmidi_device *dev, unsigned number, gfp_t gfp_flags)
 	if (result) {
 		gmidi_reset_config(dev);
 	} else {
-		char *speed;
-
-		switch (gadget->speed) {
-		case USB_SPEED_LOW:	speed = "low"; break;
-		case USB_SPEED_FULL:	speed = "full"; break;
-		case USB_SPEED_HIGH:	speed = "high"; break;
-		default:		speed = "?"; break;
-		}
-
 		dev->config = number;
-		INFO(dev, "%s speed\n", speed);
+		INFO(dev, "%s speed\n", usb_speed_string(gadget->speed));
 	}
 	return result;
 }
diff --git a/drivers/usb/gadget/langwell_udc.c b/drivers/usb/gadget/langwell_udc.c
index a06e2c2..806a5df 100644
--- a/drivers/usb/gadget/langwell_udc.c
+++ b/drivers/usb/gadget/langwell_udc.c
@@ -1700,20 +1700,7 @@ static ssize_t show_langwell_udc(struct device *_dev,
 		"BmAttributes: %d\n\n",
 		LPM_PTS(tmp_reg),
 		(tmp_reg & LPM_STS) ? 1 : 0,
-		({
-			char	*s;
-			switch (LPM_PSPD(tmp_reg)) {
-			case LPM_SPEED_FULL:
-				s = "Full Speed"; break;
-			case LPM_SPEED_LOW:
-				s = "Low Speed"; break;
-			case LPM_SPEED_HIGH:
-				s = "High Speed"; break;
-			default:
-				s = "Unknown Speed"; break;
-			}
-			s;
-		}),
+		usb_speed_string(lpm_device_speed(tmp_reg)),
 		(tmp_reg & LPM_PFSC) ? "Force Full Speed" : "Not Force",
 		(tmp_reg & LPM_PHCD) ? "Disabled" : "Enabled",
 		LPM_BA(tmp_reg));
@@ -2657,12 +2644,24 @@ done:
 	dev_vdbg(&dev->pdev->dev, "<--- %s()\n", __func__);
 }
 
+static inline enum usb_device_speed lpm_device_speed(u32 reg)
+{
+	switch (LPM_PSPD(reg)) {
+	case LPM_SPEED_HIGH:
+		return USB_SPEED_HIGH;
+	case LPM_SPEED_FULL:
+		return USB_SPEED_FULL;
+	case LPM_SPEED_LOW:
+		return USB_SPEED_LOW;
+	default:
+		return USB_SPEED_UNKNOWN;
+	}
+}
 
 /* port change detect interrupt handler */
 static void handle_port_change(struct langwell_udc *dev)
 {
 	u32	portsc1, devlc;
-	u32	speed;
 
 	dev_vdbg(&dev->pdev->dev, "---> %s()\n", __func__);
 
@@ -2677,24 +2676,9 @@ static void handle_port_change(struct langwell_udc *dev)
 	/* bus reset is finished */
 	if (!(portsc1 & PORTS_PR)) {
 		/* get the speed */
-		speed = LPM_PSPD(devlc);
-		switch (speed) {
-		case LPM_SPEED_HIGH:
-			dev->gadget.speed = USB_SPEED_HIGH;
-			break;
-		case LPM_SPEED_FULL:
-			dev->gadget.speed = USB_SPEED_FULL;
-			break;
-		case LPM_SPEED_LOW:
-			dev->gadget.speed = USB_SPEED_LOW;
-			break;
-		default:
-			dev->gadget.speed = USB_SPEED_UNKNOWN;
-			break;
-		}
-		dev_vdbg(&dev->pdev->dev,
-				"speed = %d, dev->gadget.speed = %d\n",
-				speed, dev->gadget.speed);
+		dev->gadget.speed = lpm_device_speed(devlc);
+		dev_vdbg(&dev->pdev->dev, "dev->gadget.speed = %d\n",
+			dev->gadget.speed);
 	}
 
 	/* LPM L0 to L1 */
diff --git a/drivers/usb/gadget/net2272.c b/drivers/usb/gadget/net2272.c
index ab98ea9..81d3dfa 100644
--- a/drivers/usb/gadget/net2272.c
+++ b/drivers/usb/gadget/net2272.c
@@ -1764,8 +1764,8 @@ net2272_handle_stat0_irqs(struct net2272 *dev, u8 stat)
 				dev->gadget.speed = USB_SPEED_HIGH;
 			else
 				dev->gadget.speed = USB_SPEED_FULL;
-			dev_dbg(dev->dev, "%s speed\n",
-				(dev->gadget.speed == USB_SPEED_HIGH) ? "high" : "full");
+			dev_dbg(dev->dev, "%s\n",
+				usb_speed_string(dev->gadget.speed));
 		}
 
 		ep = &dev->ep[0];
diff --git a/drivers/usb/gadget/net2280.c b/drivers/usb/gadget/net2280.c
index 3dd40b4..149ade6 100644
--- a/drivers/usb/gadget/net2280.c
+++ b/drivers/usb/gadget/net2280.c
@@ -2266,9 +2266,7 @@ static void handle_stat0_irqs (struct net2280 *dev, u32 stat)
 			else
 				dev->gadget.speed = USB_SPEED_FULL;
 			net2280_led_speed (dev, dev->gadget.speed);
-			DEBUG (dev, "%s speed\n",
-				(dev->gadget.speed == USB_SPEED_HIGH)
-					? "high" : "full");
+			DEBUG(dev, "%s\n", usb_speed_string(dev->gadget.speed));
 		}
 
 		ep = &dev->ep [0];
diff --git a/drivers/usb/gadget/printer.c b/drivers/usb/gadget/printer.c
index a341dde..78fc14b 100644
--- a/drivers/usb/gadget/printer.c
+++ b/drivers/usb/gadget/printer.c
@@ -971,23 +971,15 @@ printer_set_config(struct printer_dev *dev, unsigned number)
 		usb_gadget_vbus_draw(dev->gadget,
 				dev->gadget->is_otg ? 8 : 100);
 	} else {
-		char *speed;
 		unsigned power;
 
 		power = 2 * config_desc.bMaxPower;
 		usb_gadget_vbus_draw(dev->gadget, power);
 
-		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;
-		}
-
 		dev->config = number;
-		INFO(dev, "%s speed config #%d: %d mA, %s\n",
-				speed, number, power, driver_desc);
+		INFO(dev, "%s config #%d: %d mA, %s\n",
+		     usb_speed_string(gadget->speed),
+		     number, power, driver_desc);
 	}
 	return result;
 }
diff --git a/drivers/usb/gadget/s3c-hsotg.c b/drivers/usb/gadget/s3c-hsotg.c
index 8bdee67..48b5c71 100644
--- a/drivers/usb/gadget/s3c-hsotg.c
+++ b/drivers/usb/gadget/s3c-hsotg.c
@@ -1951,30 +1951,26 @@ static void s3c_hsotg_irq_enumdone(struct s3c_hsotg *hsotg)
 	case S3C_DSTS_EnumSpd_FS:
 	case S3C_DSTS_EnumSpd_FS48:
 		hsotg->gadget.speed = USB_SPEED_FULL;
-		dev_info(hsotg->dev, "new device is full-speed\n");
-
 		ep0_mps = EP0_MPS_LIMIT;
 		ep_mps = 64;
 		break;
 
 	case S3C_DSTS_EnumSpd_HS:
-		dev_info(hsotg->dev, "new device is high-speed\n");
 		hsotg->gadget.speed = USB_SPEED_HIGH;
-
 		ep0_mps = EP0_MPS_LIMIT;
 		ep_mps = 512;
 		break;
 
 	case S3C_DSTS_EnumSpd_LS:
 		hsotg->gadget.speed = USB_SPEED_LOW;
-		dev_info(hsotg->dev, "new device is low-speed\n");
-
 		/* note, we don't actually support LS in this driver at the
 		 * moment, and the documentation seems to imply that it isn't
 		 * supported by the PHYs on some of the devices.
 		 */
 		break;
 	}
+	dev_info(hsotg->dev, "new device is %s\n",
+		 usb_speed_string(hsotg->gadget.speed));
 
 	/* we should now know the maximum packet size for an
 	 * endpoint, so set the endpoints to a default value. */
diff --git a/drivers/usb/gadget/udc-core.c b/drivers/usb/gadget/udc-core.c
index 05ba472..5e77a46 100644
--- a/drivers/usb/gadget/udc-core.c
+++ b/drivers/usb/gadget/udc-core.c
@@ -375,23 +375,8 @@ static ssize_t usb_udc_speed_show(struct device *dev,
 		struct device_attribute *attr, char *buf)
 {
 	struct usb_udc		*udc = container_of(dev, struct usb_udc, dev);
-	struct usb_gadget	*gadget = udc->gadget;
-
-	switch (gadget->speed) {
-	case USB_SPEED_LOW:
-		return snprintf(buf, PAGE_SIZE, "low-speed\n");
-	case USB_SPEED_FULL:
-		return snprintf(buf, PAGE_SIZE, "full-speed\n");
-	case USB_SPEED_HIGH:
-		return snprintf(buf, PAGE_SIZE, "high-speed\n");
-	case USB_SPEED_WIRELESS:
-		return snprintf(buf, PAGE_SIZE, "wireless\n");
-	case USB_SPEED_SUPER:
-		return snprintf(buf, PAGE_SIZE, "super-speed\n");
-	case USB_SPEED_UNKNOWN:	/* FALLTHROUGH */
-	default:
-		return snprintf(buf, PAGE_SIZE, "UNKNOWN\n");
-	}
+	return snprintf(buf, PAGE_SIZE, "%s\n",
+			usb_speed_string(udc->gadget->speed));
 }
 static DEVICE_ATTR(speed, S_IRUSR, usb_udc_speed_show, NULL);
 
diff --git a/drivers/usb/misc/usbtest.c b/drivers/usb/misc/usbtest.c
index bb10846..48b31e6 100644
--- a/drivers/usb/misc/usbtest.c
+++ b/drivers/usb/misc/usbtest.c
@@ -2298,25 +2298,8 @@ usbtest_probe(struct usb_interface *intf, const struct usb_device_id *id)
 
 	usb_set_intfdata(intf, dev);
 	dev_info(&intf->dev, "%s\n", info->name);
-	dev_info(&intf->dev, "%s speed {control%s%s%s%s%s} tests%s\n",
-			({ char *tmp;
-			switch (udev->speed) {
-			case USB_SPEED_LOW:
-				tmp = "low";
-				break;
-			case USB_SPEED_FULL:
-				tmp = "full";
-				break;
-			case USB_SPEED_HIGH:
-				tmp = "high";
-				break;
-			case USB_SPEED_SUPER:
-				tmp = "super";
-				break;
-			default:
-				tmp = "unknown";
-				break;
-			}; tmp; }),
+	dev_info(&intf->dev, "%s {control%s%s%s%s%s} tests%s\n",
+			usb_speed_string(udev->speed),
 			info->ctrl_out ? " in/out" : "",
 			rtest, wtest,
 			irtest, iwtest,
diff --git a/include/linux/usb/ch9.h b/include/linux/usb/ch9.h
index 0fd3fbd..7d4188f 100644
--- a/include/linux/usb/ch9.h
+++ b/include/linux/usb/ch9.h
@@ -851,6 +851,18 @@ enum usb_device_speed {
 	USB_SPEED_SUPER,			/* usb 3.0 */
 };
 
+#ifdef __KERNEL__
+
+/**
+ * usb_speed_string() - Returns human readable-name of the speed.
+ * @speed: The speed to return human-readable name for.  If it's not
+ *   any of the speeds defined in usb_device_speed enum, string for
+ *   USB_SPEED_UNKNOWN will be returned.
+ */
+extern const char *usb_speed_string(enum usb_device_speed speed);
+
+#endif
+
 enum usb_device_state {
 	/* NOTATTACHED isn't in the USB spec, and this state acts
 	 * the same as ATTACHED ... but it's clearer this way.
-- 
1.7.3.1


^ permalink raw reply related	[flat|nested] 15+ messages in thread

* [PATCHv4 2/4] usb: gadget: replace "is_dualspeed" with "max_speed"
  2011-08-22 16:04 [PATCHv4 0/4] Removing USB_GADGET_*SPEED macros Michal Nazarewicz
  2011-08-22 16:04 ` [PATCHv4 1/4] usb: Provide usb_speed_string() function Michal Nazarewicz
@ 2011-08-22 16:04 ` Michal Nazarewicz
  2011-08-22 16:04 ` [PATCHv4 3/4] usb: gadget: rename usb_gadget_driver::speed to max_speed Michal Nazarewicz
  2011-08-22 16:04 ` [PATCHv4 4/4] usb: gadget: get rid of USB_GADGET_{DUAL,SUPER}SPEED Michal Nazarewicz
  3 siblings, 0 replies; 15+ messages in thread
From: Michal Nazarewicz @ 2011-08-22 16:04 UTC (permalink / raw)
  To: Alan Stern, Felipe Balbi
  Cc: Sebastian Andrzej Siewior, Yang Rui Rui, Dave Young, linux-usb,
	linux-kernel

From: Michal Nazarewicz <mina86@mina86.com>

This commit replaces usb_gadget's is_dualspeed field with
a max_speed field.

Signed-off-by: Michal Nazarewicz <mina86@mina86.com>
---
 drivers/usb/gadget/amd5536udc.c        |    2 +-
 drivers/usb/gadget/atmel_usba_udc.c    |    2 +-
 drivers/usb/gadget/ci13xxx_udc.c       |    7 +++++--
 drivers/usb/gadget/dummy_hcd.c         |    2 +-
 drivers/usb/gadget/epautoconf.c        |    6 +++---
 drivers/usb/gadget/fsl_udc_core.c      |    2 +-
 drivers/usb/gadget/fusb300_udc.c       |    2 +-
 drivers/usb/gadget/goku_udc.c          |    1 +
 drivers/usb/gadget/langwell_udc.c      |    2 +-
 drivers/usb/gadget/m66592-udc.c        |    2 +-
 drivers/usb/gadget/mv_udc_core.c       |    2 +-
 drivers/usb/gadget/net2272.c           |    2 +-
 drivers/usb/gadget/net2280.c           |    2 +-
 drivers/usb/gadget/omap_udc.c          |    1 +
 drivers/usb/gadget/pch_udc.c           |    2 +-
 drivers/usb/gadget/printer.c           |    4 ++--
 drivers/usb/gadget/r8a66597-udc.c      |    2 +-
 drivers/usb/gadget/s3c-hsotg.c         |    2 +-
 drivers/usb/gadget/s3c-hsudc.c         |    2 +-
 drivers/usb/gadget/udc-core.c          |   24 +++++++++++++++++++-----
 drivers/usb/musb/musb_gadget.c         |    2 +-
 drivers/usb/renesas_usbhs/mod_gadget.c |    2 +-
 include/linux/usb/gadget.h             |   10 +++++-----
 23 files changed, 52 insertions(+), 33 deletions(-)

diff --git a/drivers/usb/gadget/amd5536udc.c b/drivers/usb/gadget/amd5536udc.c
index 397c9b9..8b684b0 100644
--- a/drivers/usb/gadget/amd5536udc.c
+++ b/drivers/usb/gadget/amd5536udc.c
@@ -3358,7 +3358,7 @@ static int udc_probe(struct udc *dev)
 	dev_set_name(&dev->gadget.dev, "gadget");
 	dev->gadget.dev.release = gadget_release;
 	dev->gadget.name = name;
-	dev->gadget.is_dualspeed = 1;
+	dev->gadget.max_speed = USB_SPEED_HIGH;
 
 	/* init registers, interrupts, ... */
 	startup_registers(dev);
diff --git a/drivers/usb/gadget/atmel_usba_udc.c b/drivers/usb/gadget/atmel_usba_udc.c
index 2507a34..cb87090 100644
--- a/drivers/usb/gadget/atmel_usba_udc.c
+++ b/drivers/usb/gadget/atmel_usba_udc.c
@@ -1038,7 +1038,7 @@ static struct usba_udc the_udc = {
 	.gadget	= {
 		.ops		= &usba_udc_ops,
 		.ep_list	= LIST_HEAD_INIT(the_udc.gadget.ep_list),
-		.is_dualspeed	= 1,
+		.max_speed	= USB_SPEED_HIGH,
 		.name		= "atmel_usba_udc",
 		.dev	= {
 			.init_name	= "gadget",
diff --git a/drivers/usb/gadget/ci13xxx_udc.c b/drivers/usb/gadget/ci13xxx_udc.c
index 1265a85..b877464 100644
--- a/drivers/usb/gadget/ci13xxx_udc.c
+++ b/drivers/usb/gadget/ci13xxx_udc.c
@@ -751,8 +751,11 @@ static ssize_t show_device(struct device *dev, struct device_attribute *attr,
 
 	n += scnprintf(buf + n, PAGE_SIZE - n, "speed             = %d\n",
 		       gadget->speed);
+	n += scnprintf(buf + n, PAGE_SIZE - n, "max_speed         = %d\n",
+		       gadget->max_speed);
+	/* Provide "is_dualspeed" for backward compatibility. */
 	n += scnprintf(buf + n, PAGE_SIZE - n, "is_dualspeed      = %d\n",
-		       gadget->is_dualspeed);
+		       gadget_is_dualspeed(gadget));
 	n += scnprintf(buf + n, PAGE_SIZE - n, "is_otg            = %d\n",
 		       gadget->is_otg);
 	n += scnprintf(buf + n, PAGE_SIZE - n, "is_a_peripheral   = %d\n",
@@ -2866,7 +2869,7 @@ static int udc_probe(struct ci13xxx_udc_driver *driver, struct device *dev,
 
 	udc->gadget.ops          = &usb_gadget_ops;
 	udc->gadget.speed        = USB_SPEED_UNKNOWN;
-	udc->gadget.is_dualspeed = 1;
+	udc->gadget.max_speed    = USB_SPEED_HIGH;
 	udc->gadget.is_otg       = 0;
 	udc->gadget.name         = driver->name;
 
diff --git a/drivers/usb/gadget/dummy_hcd.c b/drivers/usb/gadget/dummy_hcd.c
index e755a9d..03f074f 100644
--- a/drivers/usb/gadget/dummy_hcd.c
+++ b/drivers/usb/gadget/dummy_hcd.c
@@ -986,7 +986,7 @@ static int dummy_udc_probe (struct platform_device *pdev)
 
 	dum->gadget.name = gadget_name;
 	dum->gadget.ops = &dummy_ops;
-	dum->gadget.is_dualspeed = 1;
+	dum->gadget.max_speed = USB_SPEED_SUPER;
 
 	dev_set_name(&dum->gadget.dev, "gadget");
 	dum->gadget.dev.parent = &pdev->dev;
diff --git a/drivers/usb/gadget/epautoconf.c b/drivers/usb/gadget/epautoconf.c
index 7a7e6b7..5b40f4b 100644
--- a/drivers/usb/gadget/epautoconf.c
+++ b/drivers/usb/gadget/epautoconf.c
@@ -162,7 +162,7 @@ ep_matches (
 	switch (type) {
 	case USB_ENDPOINT_XFER_INT:
 		/* INT:  limit 64 bytes full speed, 1024 high/super speed */
-		if (!gadget->is_dualspeed && max > 64)
+		if (!gadget_is_dualspeed(gadget) && max > 64)
 			return 0;
 		/* FALLTHROUGH */
 
@@ -170,12 +170,12 @@ ep_matches (
 		/* ISO:  limit 1023 bytes full speed, 1024 high/super speed */
 		if (ep->maxpacket < max)
 			return 0;
-		if (!gadget->is_dualspeed && max > 1023)
+		if (!gadget_is_dualspeed(gadget) && max > 1023)
 			return 0;
 
 		/* BOTH:  "high bandwidth" works only at high speed */
 		if ((desc->wMaxPacketSize & cpu_to_le16(3<<11))) {
-			if (!gadget->is_dualspeed)
+			if (!gadget_is_dualspeed(gadget))
 				return 0;
 			/* configure your hardware with enough buffering!! */
 		}
diff --git a/drivers/usb/gadget/fsl_udc_core.c b/drivers/usb/gadget/fsl_udc_core.c
index a3ce075..f1fac0d 100644
--- a/drivers/usb/gadget/fsl_udc_core.c
+++ b/drivers/usb/gadget/fsl_udc_core.c
@@ -2531,7 +2531,7 @@ static int __init fsl_udc_probe(struct platform_device *pdev)
 
 	/* Setup gadget structure */
 	udc_controller->gadget.ops = &fsl_gadget_ops;
-	udc_controller->gadget.is_dualspeed = 1;
+	udc_controller->gadget.max_speed = USB_SPEED_HIGH;
 	udc_controller->gadget.ep0 = &udc_controller->eps[0].ep;
 	INIT_LIST_HEAD(&udc_controller->gadget.ep_list);
 	udc_controller->gadget.speed = USB_SPEED_UNKNOWN;
diff --git a/drivers/usb/gadget/fusb300_udc.c b/drivers/usb/gadget/fusb300_udc.c
index 0e4489f..731675f 100644
--- a/drivers/usb/gadget/fusb300_udc.c
+++ b/drivers/usb/gadget/fusb300_udc.c
@@ -1473,7 +1473,7 @@ static int __init fusb300_probe(struct platform_device *pdev)
 
 	dev_set_name(&fusb300->gadget.dev, "gadget");
 
-	fusb300->gadget.is_dualspeed = 1;
+	fusb300->gadget.max_speed = USB_SPEED_HIGH;
 	fusb300->gadget.dev.parent = &pdev->dev;
 	fusb300->gadget.dev.dma_mask = pdev->dev.dma_mask;
 	fusb300->gadget.dev.release = pdev->dev.release;
diff --git a/drivers/usb/gadget/goku_udc.c b/drivers/usb/gadget/goku_udc.c
index 7f87805..ab9c924 100644
--- a/drivers/usb/gadget/goku_udc.c
+++ b/drivers/usb/gadget/goku_udc.c
@@ -1796,6 +1796,7 @@ static int goku_probe(struct pci_dev *pdev, const struct pci_device_id *id)
 	spin_lock_init(&dev->lock);
 	dev->pdev = pdev;
 	dev->gadget.ops = &goku_ops;
+	dev->gadget.max_speed = USB_SPEED_FULL;
 
 	/* the "gadget" abstracts/virtualizes the controller */
 	dev_set_name(&dev->gadget.dev, "gadget");
diff --git a/drivers/usb/gadget/langwell_udc.c b/drivers/usb/gadget/langwell_udc.c
index 806a5df..22f4a4d 100644
--- a/drivers/usb/gadget/langwell_udc.c
+++ b/drivers/usb/gadget/langwell_udc.c
@@ -3304,7 +3304,7 @@ static int langwell_udc_probe(struct pci_dev *pdev,
 	dev->gadget.ep0 = &dev->ep[0].ep;	/* gadget ep0 */
 	INIT_LIST_HEAD(&dev->gadget.ep_list);	/* ep_list */
 	dev->gadget.speed = USB_SPEED_UNKNOWN;	/* speed */
-	dev->gadget.is_dualspeed = 1;		/* support dual speed */
+	dev->gadget.max_speed = USB_SPEED_HIGH;		/* support dual speed */
 #ifdef	OTG_TRANSCEIVER
 	dev->gadget.is_otg = 1;			/* support otg mode */
 #endif
diff --git a/drivers/usb/gadget/m66592-udc.c b/drivers/usb/gadget/m66592-udc.c
index 491f825..fbdfca8 100644
--- a/drivers/usb/gadget/m66592-udc.c
+++ b/drivers/usb/gadget/m66592-udc.c
@@ -1663,7 +1663,7 @@ static int __init m66592_probe(struct platform_device *pdev)
 	m66592->gadget.ops = &m66592_gadget_ops;
 	device_initialize(&m66592->gadget.dev);
 	dev_set_name(&m66592->gadget.dev, "gadget");
-	m66592->gadget.is_dualspeed = 1;
+	m66592->gadget.max_speed = USB_SPEED_HIGH;
 	m66592->gadget.dev.parent = &pdev->dev;
 	m66592->gadget.dev.dma_mask = pdev->dev.dma_mask;
 	m66592->gadget.dev.release = pdev->dev.release;
diff --git a/drivers/usb/gadget/mv_udc_core.c b/drivers/usb/gadget/mv_udc_core.c
index ce1ac2b..17e9c5d 100644
--- a/drivers/usb/gadget/mv_udc_core.c
+++ b/drivers/usb/gadget/mv_udc_core.c
@@ -2061,7 +2061,7 @@ int mv_udc_probe(struct platform_device *dev)
 	udc->gadget.ep0 = &udc->eps[0].ep;	/* gadget ep0 */
 	INIT_LIST_HEAD(&udc->gadget.ep_list);	/* ep_list */
 	udc->gadget.speed = USB_SPEED_UNKNOWN;	/* speed */
-	udc->gadget.is_dualspeed = 1;		/* support dual speed */
+	udc->gadget.max_speed = USB_SPEED_HIGH;		/* support dual speed */
 
 	/* the "gadget" abstracts/virtualizes the controller */
 	dev_set_name(&udc->gadget.dev, "gadget");
diff --git a/drivers/usb/gadget/net2272.c b/drivers/usb/gadget/net2272.c
index 81d3dfa..8747432 100644
--- a/drivers/usb/gadget/net2272.c
+++ b/drivers/usb/gadget/net2272.c
@@ -2265,7 +2265,7 @@ net2272_probe_init(struct device *dev, unsigned int irq)
 	ret->irq = irq;
 	ret->dev = dev;
 	ret->gadget.ops = &net2272_ops;
-	ret->gadget.is_dualspeed = 1;
+	ret->gadget.max_speed = USB_SPEED_HIGH;
 
 	/* the "gadget" abstracts/virtualizes the controller */
 	dev_set_name(&ret->gadget.dev, "gadget");
diff --git a/drivers/usb/gadget/net2280.c b/drivers/usb/gadget/net2280.c
index 149ade6..40b86a7 100644
--- a/drivers/usb/gadget/net2280.c
+++ b/drivers/usb/gadget/net2280.c
@@ -2741,7 +2741,7 @@ static int net2280_probe (struct pci_dev *pdev, const struct pci_device_id *id)
 	spin_lock_init (&dev->lock);
 	dev->pdev = pdev;
 	dev->gadget.ops = &net2280_ops;
-	dev->gadget.is_dualspeed = 1;
+	dev->gadget.max_speed = USB_SPEED_HIGH;
 
 	/* the "gadget" abstracts/virtualizes the controller */
 	dev_set_name(&dev->gadget.dev, "gadget");
diff --git a/drivers/usb/gadget/omap_udc.c b/drivers/usb/gadget/omap_udc.c
index 740c7da..fff7e1c 100644
--- a/drivers/usb/gadget/omap_udc.c
+++ b/drivers/usb/gadget/omap_udc.c
@@ -2686,6 +2686,7 @@ omap_udc_setup(struct platform_device *odev, struct otg_transceiver *xceiv)
 	INIT_LIST_HEAD(&udc->gadget.ep_list);
 	INIT_LIST_HEAD(&udc->iso);
 	udc->gadget.speed = USB_SPEED_UNKNOWN;
+	udc->gadget.max_speed = USB_SPEED_FULL;
 	udc->gadget.name = driver_name;
 
 	device_initialize(&udc->gadget.dev);
diff --git a/drivers/usb/gadget/pch_udc.c b/drivers/usb/gadget/pch_udc.c
index f96615a..d781540 100644
--- a/drivers/usb/gadget/pch_udc.c
+++ b/drivers/usb/gadget/pch_udc.c
@@ -2949,7 +2949,7 @@ static int pch_udc_probe(struct pci_dev *pdev,
 	dev->gadget.dev.dma_mask = pdev->dev.dma_mask;
 	dev->gadget.dev.release = gadget_release;
 	dev->gadget.name = KBUILD_MODNAME;
-	dev->gadget.is_dualspeed = 1;
+	dev->gadget.max_speed = USB_SPEED_HIGH;
 
 	retval = device_register(&dev->gadget.dev);
 	if (retval)
diff --git a/drivers/usb/gadget/printer.c b/drivers/usb/gadget/printer.c
index 78fc14b..c39dd8c 100644
--- a/drivers/usb/gadget/printer.c
+++ b/drivers/usb/gadget/printer.c
@@ -1150,7 +1150,7 @@ printer_setup(struct usb_gadget *gadget, const struct usb_ctrlrequest *ctrl)
 				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
@@ -1164,7 +1164,7 @@ 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 */
diff --git a/drivers/usb/gadget/r8a66597-udc.c b/drivers/usb/gadget/r8a66597-udc.c
index 50991e5..a8f7a05 100644
--- a/drivers/usb/gadget/r8a66597-udc.c
+++ b/drivers/usb/gadget/r8a66597-udc.c
@@ -1616,7 +1616,7 @@ static int __init r8a66597_probe(struct platform_device *pdev)
 	r8a66597->gadget.ops = &r8a66597_gadget_ops;
 	device_initialize(&r8a66597->gadget.dev);
 	dev_set_name(&r8a66597->gadget.dev, "gadget");
-	r8a66597->gadget.is_dualspeed = 1;
+	r8a66597->gadget.max_speed = USB_SPEED_HIGH;
 	r8a66597->gadget.dev.parent = &pdev->dev;
 	r8a66597->gadget.dev.dma_mask = pdev->dev.dma_mask;
 	r8a66597->gadget.dev.release = pdev->dev.release;
diff --git a/drivers/usb/gadget/s3c-hsotg.c b/drivers/usb/gadget/s3c-hsotg.c
index 48b5c71..9311a20 100644
--- a/drivers/usb/gadget/s3c-hsotg.c
+++ b/drivers/usb/gadget/s3c-hsotg.c
@@ -3364,7 +3364,7 @@ static int __devinit s3c_hsotg_probe(struct platform_device *pdev)
 
 	dev_set_name(&hsotg->gadget.dev, "gadget");
 
-	hsotg->gadget.is_dualspeed = 1;
+	hsotg->gadget.max_speed = USB_SPEED_HIGH;
 	hsotg->gadget.ops = &s3c_hsotg_gadget_ops;
 	hsotg->gadget.name = dev_name(dev);
 
diff --git a/drivers/usb/gadget/s3c-hsudc.c b/drivers/usb/gadget/s3c-hsudc.c
index 3fa717c..3ac4241 100644
--- a/drivers/usb/gadget/s3c-hsudc.c
+++ b/drivers/usb/gadget/s3c-hsudc.c
@@ -1287,7 +1287,7 @@ static int s3c_hsudc_probe(struct platform_device *pdev)
 	device_initialize(&hsudc->gadget.dev);
 	dev_set_name(&hsudc->gadget.dev, "gadget");
 
-	hsudc->gadget.is_dualspeed = 1;
+	hsudc->gadget.max_speed = USB_SPEED_HIGH;
 	hsudc->gadget.ops = &s3c_hsudc_gadget_ops;
 	hsudc->gadget.name = dev_name(dev);
 	hsudc->gadget.dev.parent = dev;
diff --git a/drivers/usb/gadget/udc-core.c b/drivers/usb/gadget/udc-core.c
index 5e77a46..e775f3a 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_speed_string(udc->gadget->name));		\
+}									\
+static DEVICE_ATTR(name, S_IRUSR, usb_udc_##name##_show, NULL)
+
+static USB_UDC_SPEED_ATTR(speed);
+static USB_UDC_SPEED_ATTR(max_speed);
+
+/* 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_speed_string(udc->gadget->speed));
+	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);
 
 #define USB_UDC_ATTR(name)					\
 ssize_t usb_udc_##name##_show(struct device *dev,		\
@@ -391,7 +405,6 @@ ssize_t usb_udc_##name##_show(struct device *dev,		\
 }								\
 static DEVICE_ATTR(name, S_IRUSR, usb_udc_##name##_show, NULL)
 
-static USB_UDC_ATTR(is_dualspeed);
 static USB_UDC_ATTR(is_otg);
 static USB_UDC_ATTR(is_a_peripheral);
 static USB_UDC_ATTR(b_hnp_enable);
@@ -402,6 +415,7 @@ static struct attribute *usb_udc_attrs[] = {
 	&dev_attr_srp.attr,
 	&dev_attr_soft_connect.attr,
 	&dev_attr_speed.attr,
+	&dev_attr_max_speed.attr,
 
 	&dev_attr_is_dualspeed.attr,
 	&dev_attr_is_otg.attr,
diff --git a/drivers/usb/musb/musb_gadget.c b/drivers/usb/musb/musb_gadget.c
index 8c41a2e..0b3fb20 100644
--- a/drivers/usb/musb/musb_gadget.c
+++ b/drivers/usb/musb/musb_gadget.c
@@ -1827,7 +1827,7 @@ int __init musb_gadget_setup(struct musb *musb)
 	 */
 
 	musb->g.ops = &musb_gadget_operations;
-	musb->g.is_dualspeed = 1;
+	musb->g.max_speed = USB_SPEED_HIGH;
 	musb->g.speed = USB_SPEED_UNKNOWN;
 
 	/* this "gadget" abstracts/virtualizes the controller */
diff --git a/drivers/usb/renesas_usbhs/mod_gadget.c b/drivers/usb/renesas_usbhs/mod_gadget.c
index cb2d451..a8ae54f 100644
--- a/drivers/usb/renesas_usbhs/mod_gadget.c
+++ b/drivers/usb/renesas_usbhs/mod_gadget.c
@@ -917,7 +917,7 @@ int __devinit usbhs_mod_gadget_probe(struct usbhs_priv *priv)
 	gpriv->gadget.dev.parent	= dev;
 	gpriv->gadget.name		= "renesas_usbhs_udc";
 	gpriv->gadget.ops		= &usbhsg_gadget_ops;
-	gpriv->gadget.is_dualspeed	= 1;
+	gpriv->gadget.max_speed		= USB_SPEED_HIGH;
 
 	INIT_LIST_HEAD(&gpriv->gadget.ep_list);
 
diff --git a/include/linux/usb/gadget.h b/include/linux/usb/gadget.h
index 087f4b9..7eb2ec3 100644
--- a/include/linux/usb/gadget.h
+++ b/include/linux/usb/gadget.h
@@ -477,8 +477,8 @@ struct usb_gadget_ops {
  *	driver setup() requests
  * @ep_list: List of other endpoints supported by the device.
  * @speed: Speed of current connection to USB host.
- * @is_dualspeed: True if the controller supports both high and full speed
- *	operation.  If it does, the gadget driver must also support both.
+ * @max_speed: Maximal speed the UDC can handle.  UDC must support this
+ *      and all slower speeds.
  * @is_otg: True if the USB device port uses a Mini-AB jack, so that the
  *	gadget driver must provide a USB OTG descriptor.
  * @is_a_peripheral: False unless is_otg, the "A" end of a USB cable
@@ -518,7 +518,7 @@ struct usb_gadget {
 	struct usb_ep			*ep0;
 	struct list_head		ep_list;	/* of usb_ep */
 	enum usb_device_speed		speed;
-	unsigned			is_dualspeed:1;
+	enum usb_device_speed		max_speed;
 	unsigned			is_otg:1;
 	unsigned			is_a_peripheral:1;
 	unsigned			b_hnp_enable:1;
@@ -549,7 +549,7 @@ static inline struct usb_gadget *dev_to_usb_gadget(struct device *dev)
 static inline int gadget_is_dualspeed(struct usb_gadget *g)
 {
 #ifdef CONFIG_USB_GADGET_DUALSPEED
-	/* runtime test would check "g->is_dualspeed" ... that might be
+	/* runtime test would check "g->max_speed" ... that might be
 	 * useful to work around hardware bugs, but is mostly pointless
 	 */
 	return 1;
@@ -567,7 +567,7 @@ static inline int gadget_is_superspeed(struct usb_gadget *g)
 {
 #ifdef CONFIG_USB_GADGET_SUPERSPEED
 	/*
-	 * runtime test would check "g->is_superspeed" ... that might be
+	 * runtime test would check "g->max_speed" ... that might be
 	 * useful to work around hardware bugs, but is mostly pointless
 	 */
 	return 1;
-- 
1.7.3.1


^ permalink raw reply related	[flat|nested] 15+ messages in thread

* [PATCHv4 3/4] usb: gadget: rename usb_gadget_driver::speed to max_speed
  2011-08-22 16:04 [PATCHv4 0/4] Removing USB_GADGET_*SPEED macros Michal Nazarewicz
  2011-08-22 16:04 ` [PATCHv4 1/4] usb: Provide usb_speed_string() function Michal Nazarewicz
  2011-08-22 16:04 ` [PATCHv4 2/4] usb: gadget: replace "is_dualspeed" with "max_speed" Michal Nazarewicz
@ 2011-08-22 16:04 ` Michal Nazarewicz
  2011-08-24  2:26   ` Dave Young
  2011-08-22 16:04 ` [PATCHv4 4/4] usb: gadget: get rid of USB_GADGET_{DUAL,SUPER}SPEED Michal Nazarewicz
  3 siblings, 1 reply; 15+ messages in thread
From: Michal Nazarewicz @ 2011-08-22 16:04 UTC (permalink / raw)
  To: Alan Stern, Felipe Balbi
  Cc: Sebastian Andrzej Siewior, Yang Rui Rui, Dave Young, linux-usb,
	linux-kernel

From: Michal Nazarewicz <mina86@mina86.com>

This commit renames the “speed” field of the usb_gadget_driver
structure to “max_speed”.  This is so that to make it more
apparent that the field represents the maximum speed gadget
driver can support.

This also make the field look more like fields with the same
name in usb_gadget and usb_composite_driver structures.  All
of those represent the *maximal* speed given entity supports.

After this commit, there are the following fields in various
structures:
* usb_gadget::speed - the current connection speed,
* usb_gadget::max_speed - maximal speed UDC supports,
* usb_gadget_driver::max_speed - maximal speed gadget driver
  supports, and
* usb_composite_driver::max_speed - maximal speed composite
  gadget supports.

Signed-off-by: Michal Nazarewicz <mina86@mina86.com>
---
 drivers/usb/gadget/amd5536udc.c   |    2 +-
 drivers/usb/gadget/ci13xxx_udc.c  |    2 +-
 drivers/usb/gadget/composite.c    |    8 ++++----
 drivers/usb/gadget/dbgp.c         |    2 +-
 drivers/usb/gadget/dummy_hcd.c    |   13 ++++++-------
 drivers/usb/gadget/file_storage.c |    4 ++--
 drivers/usb/gadget/gmidi.c        |    2 +-
 drivers/usb/gadget/goku_udc.c     |    2 +-
 drivers/usb/gadget/inode.c        |    6 +++---
 drivers/usb/gadget/m66592-udc.c   |    2 +-
 drivers/usb/gadget/net2272.c      |    2 +-
 drivers/usb/gadget/net2280.c      |    2 +-
 drivers/usb/gadget/pch_udc.c      |    2 +-
 drivers/usb/gadget/printer.c      |    2 +-
 drivers/usb/gadget/r8a66597-udc.c |    2 +-
 include/linux/usb/gadget.h        |    4 ++--
 16 files changed, 28 insertions(+), 29 deletions(-)

diff --git a/drivers/usb/gadget/amd5536udc.c b/drivers/usb/gadget/amd5536udc.c
index 8b684b0..cd520f9 100644
--- a/drivers/usb/gadget/amd5536udc.c
+++ b/drivers/usb/gadget/amd5536udc.c
@@ -1968,7 +1968,7 @@ static int amd5536_start(struct usb_gadget_driver *driver,
 	u32 tmp;
 
 	if (!driver || !bind || !driver->setup
-			|| driver->speed != USB_SPEED_HIGH)
+			|| driver->max_speed != USB_SPEED_HIGH)
 		return -EINVAL;
 	if (!dev)
 		return -ENODEV;
diff --git a/drivers/usb/gadget/ci13xxx_udc.c b/drivers/usb/gadget/ci13xxx_udc.c
index b877464..6c13174 100644
--- a/drivers/usb/gadget/ci13xxx_udc.c
+++ b/drivers/usb/gadget/ci13xxx_udc.c
@@ -798,7 +798,7 @@ static ssize_t show_driver(struct device *dev, struct device_attribute *attr,
 	n += scnprintf(buf + n, PAGE_SIZE - n, "function  = %s\n",
 		       (driver->function ? driver->function : ""));
 	n += scnprintf(buf + n, PAGE_SIZE - n, "max speed = %d\n",
-		       driver->speed);
+		       driver->max_speed);
 
 	return n;
 }
diff --git a/drivers/usb/gadget/composite.c b/drivers/usb/gadget/composite.c
index 0d03936..36f326a 100644
--- a/drivers/usb/gadget/composite.c
+++ b/drivers/usb/gadget/composite.c
@@ -1544,9 +1544,9 @@ composite_resume(struct usb_gadget *gadget)
 
 static struct usb_gadget_driver composite_driver = {
 #ifdef CONFIG_USB_GADGET_SUPERSPEED
-	.speed		= USB_SPEED_SUPER,
+	.max_speed	= USB_SPEED_SUPER,
 #else
-	.speed		= USB_SPEED_HIGH,
+	.max_speed	= USB_SPEED_HIGH,
 #endif
 
 	.unbind		= composite_unbind,
@@ -1593,8 +1593,8 @@ 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.max_speed = min((u8)composite_driver.max_speed,
+					 (u8)driver->max_speed);
 	composite = driver;
 	composite_gadget_bind = bind;
 
diff --git a/drivers/usb/gadget/dbgp.c b/drivers/usb/gadget/dbgp.c
index e8737b5..4e6f8a2 100644
--- a/drivers/usb/gadget/dbgp.c
+++ b/drivers/usb/gadget/dbgp.c
@@ -405,7 +405,7 @@ fail:
 
 static struct usb_gadget_driver dbgp_driver = {
 	.function = "dbgp",
-	.speed = USB_SPEED_HIGH,
+	.max_speed = USB_SPEED_HIGH,
 	.unbind = dbgp_unbind,
 	.setup = dbgp_setup,
 	.disconnect = dbgp_disconnect,
diff --git a/drivers/usb/gadget/dummy_hcd.c b/drivers/usb/gadget/dummy_hcd.c
index 03f074f..c0fefc5 100644
--- a/drivers/usb/gadget/dummy_hcd.c
+++ b/drivers/usb/gadget/dummy_hcd.c
@@ -832,19 +832,18 @@ static int dummy_pullup (struct usb_gadget *_gadget, int value)
 
 	if (value && dum->driver) {
 		if (mod_data.is_super_speed)
-			dum->gadget.speed = dum->driver->speed;
+			dum->gadget.speed = dum->driver->max_speed;
 		else if (mod_data.is_high_speed)
 			dum->gadget.speed = min_t(u8, USB_SPEED_HIGH,
-					dum->driver->speed);
+					dum->driver->max_speed);
 		else
 			dum->gadget.speed = USB_SPEED_FULL;
 		dummy_udc_udpate_ep0(dum);
 
-		if (dum->gadget.speed < dum->driver->speed)
+		if (dum->gadget.speed < dum->driver->max_speed)
 			dev_dbg(udc_dev(dum), "This device can perform faster"
-					" if you connect it to a %s port...\n",
-					(dum->driver->speed == USB_SPEED_SUPER ?
-					 "SuperSpeed" : "HighSpeed"));
+				" if you connect it to a %s port...\n",
+				usb_speed_string(dum->driver->max_speed));
 	}
 	dum_hcd = gadget_to_dummy_hcd(_gadget);
 
@@ -907,7 +906,7 @@ static int dummy_udc_start(struct usb_gadget *g,
 	struct dummy_hcd	*dum_hcd = gadget_to_dummy_hcd(g);
 	struct dummy		*dum = dum_hcd->dum;
 
-	if (driver->speed == USB_SPEED_UNKNOWN)
+	if (driver->max_speed == USB_SPEED_UNKNOWN)
 		return -EINVAL;
 
 	/*
diff --git a/drivers/usb/gadget/file_storage.c b/drivers/usb/gadget/file_storage.c
index fede982..7526d4a 100644
--- a/drivers/usb/gadget/file_storage.c
+++ b/drivers/usb/gadget/file_storage.c
@@ -3556,9 +3556,9 @@ static void fsg_resume(struct usb_gadget *gadget)
 
 static struct usb_gadget_driver		fsg_driver = {
 #ifdef CONFIG_USB_GADGET_DUALSPEED
-	.speed		= USB_SPEED_HIGH,
+	.max_speed	= USB_SPEED_HIGH,
 #else
-	.speed		= USB_SPEED_FULL,
+	.max_speed	= USB_SPEED_FULL,
 #endif
 	.function	= (char *) fsg_string_product,
 	.unbind		= fsg_unbind,
diff --git a/drivers/usb/gadget/gmidi.c b/drivers/usb/gadget/gmidi.c
index 6c3d213..0ea862c 100644
--- a/drivers/usb/gadget/gmidi.c
+++ b/drivers/usb/gadget/gmidi.c
@@ -1283,7 +1283,7 @@ static void gmidi_resume(struct usb_gadget *gadget)
 
 
 static struct usb_gadget_driver gmidi_driver = {
-	.speed		= USB_SPEED_FULL,
+	.max_speed	= USB_SPEED_FULL,
 	.function	= (char *)longname,
 	.unbind		= gmidi_unbind,
 
diff --git a/drivers/usb/gadget/goku_udc.c b/drivers/usb/gadget/goku_udc.c
index ab9c924..5af70fcc 100644
--- a/drivers/usb/gadget/goku_udc.c
+++ b/drivers/usb/gadget/goku_udc.c
@@ -1357,7 +1357,7 @@ static int goku_start(struct usb_gadget_driver *driver,
 	int			retval;
 
 	if (!driver
-			|| driver->speed < USB_SPEED_FULL
+			|| driver->max_speed < USB_SPEED_FULL
 			|| !bind
 			|| !driver->disconnect
 			|| !driver->setup)
diff --git a/drivers/usb/gadget/inode.c b/drivers/usb/gadget/inode.c
index 1b24099..4bd48ea 100644
--- a/drivers/usb/gadget/inode.c
+++ b/drivers/usb/gadget/inode.c
@@ -1774,9 +1774,9 @@ gadgetfs_suspend (struct usb_gadget *gadget)
 
 static struct usb_gadget_driver gadgetfs_driver = {
 #ifdef	CONFIG_USB_GADGET_DUALSPEED
-	.speed		= USB_SPEED_HIGH,
+	.max_speed	= USB_SPEED_HIGH,
 #else
-	.speed		= USB_SPEED_FULL,
+	.max_speed	= USB_SPEED_FULL,
 #endif
 	.function	= (char *) driver_desc,
 	.unbind		= gadgetfs_unbind,
@@ -1800,7 +1800,7 @@ static int gadgetfs_probe (struct usb_gadget *gadget)
 }
 
 static struct usb_gadget_driver probe_driver = {
-	.speed		= USB_SPEED_HIGH,
+	.max_speed	= USB_SPEED_HIGH,
 	.unbind		= gadgetfs_nop,
 	.setup		= (void *)gadgetfs_nop,
 	.disconnect	= gadgetfs_nop,
diff --git a/drivers/usb/gadget/m66592-udc.c b/drivers/usb/gadget/m66592-udc.c
index fbdfca8..72dc3f1 100644
--- a/drivers/usb/gadget/m66592-udc.c
+++ b/drivers/usb/gadget/m66592-udc.c
@@ -1482,7 +1482,7 @@ static int m66592_start(struct usb_gadget_driver *driver,
 	int retval;
 
 	if (!driver
-			|| driver->speed != USB_SPEED_HIGH
+			|| driver->max_speed != USB_SPEED_HIGH
 			|| !bind
 			|| !driver->setup)
 		return -EINVAL;
diff --git a/drivers/usb/gadget/net2272.c b/drivers/usb/gadget/net2272.c
index 8747432..1767230 100644
--- a/drivers/usb/gadget/net2272.c
+++ b/drivers/usb/gadget/net2272.c
@@ -1461,7 +1461,7 @@ static int net2272_start(struct usb_gadget_driver *driver,
 	unsigned i;
 
 	if (!driver || !bind || !driver->unbind || !driver->setup ||
-	    driver->speed != USB_SPEED_HIGH)
+	    driver->max_speed != USB_SPEED_HIGH)
 		return -EINVAL;
 	if (!dev)
 		return -ENODEV;
diff --git a/drivers/usb/gadget/net2280.c b/drivers/usb/gadget/net2280.c
index 40b86a7..11b9959 100644
--- a/drivers/usb/gadget/net2280.c
+++ b/drivers/usb/gadget/net2280.c
@@ -1892,7 +1892,7 @@ static int net2280_start(struct usb_gadget_driver *driver,
 	 * "must not be used in normal operation"
 	 */
 	if (!driver
-			|| driver->speed != USB_SPEED_HIGH
+			|| driver->max_speed != USB_SPEED_HIGH
 			|| !bind || !driver->setup)
 		return -EINVAL;
 	if (!dev)
diff --git a/drivers/usb/gadget/pch_udc.c b/drivers/usb/gadget/pch_udc.c
index d781540..1db6d0e 100644
--- a/drivers/usb/gadget/pch_udc.c
+++ b/drivers/usb/gadget/pch_udc.c
@@ -2701,7 +2701,7 @@ static int pch_udc_start(struct usb_gadget_driver *driver,
 	struct pch_udc_dev	*dev = pch_udc;
 	int			retval;
 
-	if (!driver || (driver->speed == USB_SPEED_UNKNOWN) || !bind ||
+	if (!driver || (driver->max_speed == USB_SPEED_UNKNOWN) || !bind ||
 	    !driver->setup || !driver->unbind || !driver->disconnect) {
 		dev_err(&dev->pdev->dev,
 			"%s: invalid driver parameter\n", __func__);
diff --git a/drivers/usb/gadget/printer.c b/drivers/usb/gadget/printer.c
index c39dd8c..2b9965b 100644
--- a/drivers/usb/gadget/printer.c
+++ b/drivers/usb/gadget/printer.c
@@ -1544,7 +1544,7 @@ fail:
 /*-------------------------------------------------------------------------*/
 
 static struct usb_gadget_driver printer_driver = {
-	.speed		= DEVSPEED,
+	.max_speed	= DEVSPEED,
 
 	.function	= (char *) driver_desc,
 	.unbind		= printer_unbind,
diff --git a/drivers/usb/gadget/r8a66597-udc.c b/drivers/usb/gadget/r8a66597-udc.c
index a8f7a05..df99690 100644
--- a/drivers/usb/gadget/r8a66597-udc.c
+++ b/drivers/usb/gadget/r8a66597-udc.c
@@ -1442,7 +1442,7 @@ static int r8a66597_start(struct usb_gadget_driver *driver,
 	int retval;
 
 	if (!driver
-			|| driver->speed != USB_SPEED_HIGH
+			|| driver->max_speed != USB_SPEED_HIGH
 			|| !bind
 			|| !driver->setup)
 		return -EINVAL;
diff --git a/include/linux/usb/gadget.h b/include/linux/usb/gadget.h
index 7eb2ec3..c8ea048 100644
--- a/include/linux/usb/gadget.h
+++ b/include/linux/usb/gadget.h
@@ -760,7 +760,7 @@ static inline int usb_gadget_disconnect(struct usb_gadget *gadget)
 /**
  * struct usb_gadget_driver - driver for usb 'slave' devices
  * @function: String describing the gadget's function
- * @speed: Highest speed the driver handles.
+ * @max_speed: Highest speed the driver handles.
  * @setup: Invoked for ep0 control requests that aren't handled by
  *	the hardware level driver. Most calls must be handled by
  *	the gadget driver, including descriptor and configuration
@@ -824,7 +824,7 @@ static inline int usb_gadget_disconnect(struct usb_gadget *gadget)
  */
 struct usb_gadget_driver {
 	char			*function;
-	enum usb_device_speed	speed;
+	enum usb_device_speed	max_speed;
 	void			(*unbind)(struct usb_gadget *);
 	int			(*setup)(struct usb_gadget *,
 					const struct usb_ctrlrequest *);
-- 
1.7.3.1


^ permalink raw reply related	[flat|nested] 15+ messages in thread

* [PATCHv4 4/4] usb: gadget: get rid of USB_GADGET_{DUAL,SUPER}SPEED
  2011-08-22 16:04 [PATCHv4 0/4] Removing USB_GADGET_*SPEED macros Michal Nazarewicz
                   ` (2 preceding siblings ...)
  2011-08-22 16:04 ` [PATCHv4 3/4] usb: gadget: rename usb_gadget_driver::speed to max_speed Michal Nazarewicz
@ 2011-08-22 16:04 ` Michal Nazarewicz
  3 siblings, 0 replies; 15+ messages in thread
From: Michal Nazarewicz @ 2011-08-22 16:04 UTC (permalink / raw)
  To: Alan Stern, Felipe Balbi
  Cc: Sebastian Andrzej Siewior, Yang Rui Rui, Dave Young, linux-usb,
	linux-kernel

From: Michal Nazarewicz <mina86@mina86.com>

This commit removes USB_GADGET_DUALSPEED and USB_GADGET_SUPERSPEED
Kconfig options.  Since now kernel allows many UDC drivers to be
compiled, those options may turn to no longer be valid.  For
instance, if someone decides to build UDC that supports super
speed and UDC that supports high speed only, the latter will be
"assumed" to support super speed since USB_GADGET_SUPERSPEED will
be selected by the former.

The test of whether CONFIG_USB_GADGET_*SPEED was defined was just
an optimisation which removed otherwise dead code (ie. if UDC is
not dual speed, there is no need to handle cases that can happen
if speed is high).  This commit removes those checks.

Signed-off-by: Michal Nazarewicz <mina86@mina86.com>
---
 drivers/usb/gadget/Kconfig        |   30 ------------------------------
 drivers/usb/gadget/composite.c    |    9 +--------
 drivers/usb/gadget/file_storage.c |    4 ----
 drivers/usb/gadget/inode.c        |   15 ++++-----------
 drivers/usb/gadget/printer.c      |   37 +++++--------------------------------
 drivers/usb/gadget/u_ether.c      |    7 -------
 include/linux/usb/gadget.h        |   19 ++-----------------
 7 files changed, 12 insertions(+), 109 deletions(-)

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 36f326a..70b015d 100644
--- a/drivers/usb/gadget/composite.c
+++ b/drivers/usb/gadget/composite.c
@@ -1543,12 +1543,6 @@ composite_resume(struct usb_gadget *gadget)
 /*-------------------------------------------------------------------------*/
 
 static struct usb_gadget_driver composite_driver = {
-#ifdef CONFIG_USB_GADGET_SUPERSPEED
-	.max_speed	= USB_SPEED_SUPER,
-#else
-	.max_speed	= USB_SPEED_HIGH,
-#endif
-
 	.unbind		= composite_unbind,
 
 	.setup		= composite_setup,
@@ -1593,8 +1587,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.max_speed = min((u8)composite_driver.max_speed,
-					 (u8)driver->max_speed);
+	composite_driver.max_speed = driver->max_speed;
 	composite = driver;
 	composite_gadget_bind = bind;
 
diff --git a/drivers/usb/gadget/file_storage.c b/drivers/usb/gadget/file_storage.c
index 7526d4a..9be7581 100644
--- a/drivers/usb/gadget/file_storage.c
+++ b/drivers/usb/gadget/file_storage.c
@@ -3555,11 +3555,7 @@ static void fsg_resume(struct usb_gadget *gadget)
 /*-------------------------------------------------------------------------*/
 
 static struct usb_gadget_driver		fsg_driver = {
-#ifdef CONFIG_USB_GADGET_DUALSPEED
 	.max_speed	= USB_SPEED_HIGH,
-#else
-	.max_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 4bd48ea..fee9ef3 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,6 @@ gadgetfs_suspend (struct usb_gadget *gadget)
 }
 
 static struct usb_gadget_driver gadgetfs_driver = {
-#ifdef	CONFIG_USB_GADGET_DUALSPEED
-	.max_speed	= USB_SPEED_HIGH,
-#else
-	.max_speed	= USB_SPEED_FULL,
-#endif
 	.function	= (char *) driver_desc,
 	.unbind		= gadgetfs_unbind,
 	.setup		= gadgetfs_setup,
@@ -1910,6 +1899,10 @@ dev_config (struct file *fd, const char __user *buf, size_t len, loff_t *ptr)
 
 	/* triggers gadgetfs_bind(); then we can enumerate. */
 	spin_unlock_irq (&dev->lock);
+	if (dev->hs_config)
+		gadgetfs_driver.max_speed = USB_SPEED_HIGH;
+	else
+		gadgetfs_driver.max_speed = USB_SPEED_FULL;
 	value = usb_gadget_probe_driver(&gadgetfs_driver, gadgetfs_bind);
 	if (value != 0) {
 		kfree (dev->buf);
diff --git a/drivers/usb/gadget/printer.c b/drivers/usb/gadget/printer.c
index 2b9965b..0595c75 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 */
@@ -990,23 +975,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)
@@ -1148,7 +1125,6 @@ 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(gadget))
 					break;
@@ -1167,7 +1143,6 @@ printer_setup(struct usb_gadget *gadget, const struct usb_ctrlrequest *ctrl)
 				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,
@@ -1452,11 +1427,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);
 
@@ -1544,7 +1517,7 @@ fail:
 /*-------------------------------------------------------------------------*/
 
 static struct usb_gadget_driver printer_driver = {
-	.max_speed	= DEVSPEED,
+	.max_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)
 {
diff --git a/include/linux/usb/gadget.h b/include/linux/usb/gadget.h
index c8ea048..22404cf 100644
--- a/include/linux/usb/gadget.h
+++ b/include/linux/usb/gadget.h
@@ -548,14 +548,7 @@ static inline struct usb_gadget *dev_to_usb_gadget(struct device *dev)
  */
 static inline int gadget_is_dualspeed(struct usb_gadget *g)
 {
-#ifdef CONFIG_USB_GADGET_DUALSPEED
-	/* runtime test would check "g->max_speed" ... that might be
-	 * useful to work around hardware bugs, but is mostly pointless
-	 */
-	return 1;
-#else
-	return 0;
-#endif
+	return g->max_speed >= USB_SPEED_HIGH;
 }
 
 /**
@@ -565,15 +558,7 @@ static inline int gadget_is_dualspeed(struct usb_gadget *g)
  */
 static inline int gadget_is_superspeed(struct usb_gadget *g)
 {
-#ifdef CONFIG_USB_GADGET_SUPERSPEED
-	/*
-	 * runtime test would check "g->max_speed" ... that might be
-	 * useful to work around hardware bugs, but is mostly pointless
-	 */
-	return 1;
-#else
-	return 0;
-#endif
+	return g->max_speed >= USB_SPEED_SUPER;
 }
 
 /**
-- 
1.7.3.1


^ permalink raw reply related	[flat|nested] 15+ messages in thread

* Re: [PATCHv4 1/4] usb: Provide usb_speed_string() function
  2011-08-22 16:04 ` [PATCHv4 1/4] usb: Provide usb_speed_string() function Michal Nazarewicz
@ 2011-08-22 17:28   ` Alan Stern
  2011-08-22 19:42     ` Michal Nazarewicz
  2011-08-24  2:16     ` Dave Young
  0 siblings, 2 replies; 15+ messages in thread
From: Alan Stern @ 2011-08-22 17:28 UTC (permalink / raw)
  To: Michal Nazarewicz
  Cc: Felipe Balbi, Sebastian Andrzej Siewior, Yang Rui Rui,
	Dave Young, linux-usb, linux-kernel

On Mon, 22 Aug 2011, Michal Nazarewicz wrote:

> From: Michal Nazarewicz <mina86@mina86.com>
> 
> In a few places in the kernel, the code prints
> a human-readable USB device speed (eg. "high speed").
> This involves a switch statement sometimes wrapped
> around in ({ ... }) block leading to code repetition.
> 
> To mitigate this issue, this commit introduces
> usb_speed_string() function, which returns
> a human-readable name of provided speed.
> 
> It also changes a few places switch was used to use
> this new function.  This changes a bit the way the
> speed is printed in few instances at the same time
> standardising it.

> --- a/drivers/usb/Makefile
> +++ b/drivers/usb/Makefile
> @@ -51,3 +51,6 @@ obj-$(CONFIG_USB_MUSB_HDRC)	+= musb/
>  obj-$(CONFIG_USB_RENESAS_USBHS)	+= renesas_usbhs/
>  obj-$(CONFIG_USB_OTG_UTILS)	+= otg/
>  obj-$(CONFIG_USB_GADGET)	+= gadget/
> +
> +obj-$(CONFIG_USB)		+= common.o
> +obj-$(CONFIG_USB_GADGET)	+= common.o

Will this cause problems?  For example, what if CONFIG_USB is Y and 
CONFIG_USB_GADGET is M?

Also, it seems a little silly to have an entire kernel module devoted 
to a single, little function.  This is a symptom of the difficulty of 
sharing code between the host-side and gadget-side USB stacks.

Alan Stern


^ permalink raw reply	[flat|nested] 15+ messages in thread

* Re: [PATCHv4 1/4] usb: Provide usb_speed_string() function
  2011-08-22 17:28   ` Alan Stern
@ 2011-08-22 19:42     ` Michal Nazarewicz
  2011-08-22 21:06       ` Alan Stern
  2011-08-24  2:16     ` Dave Young
  1 sibling, 1 reply; 15+ messages in thread
From: Michal Nazarewicz @ 2011-08-22 19:42 UTC (permalink / raw)
  To: Alan Stern
  Cc: Felipe Balbi, Sebastian Andrzej Siewior, Yang Rui Rui,
	Dave Young, linux-usb, linux-kernel

On Mon, 22 Aug 2011 19:28:46 +0200, Alan Stern <stern@rowland.harvard.edu>  
wrote:

> On Mon, 22 Aug 2011, Michal Nazarewicz wrote:
>
>> From: Michal Nazarewicz <mina86@mina86.com>
>>
>> In a few places in the kernel, the code prints
>> a human-readable USB device speed (eg. "high speed").
>> This involves a switch statement sometimes wrapped
>> around in ({ ... }) block leading to code repetition.
>>
>> To mitigate this issue, this commit introduces
>> usb_speed_string() function, which returns
>> a human-readable name of provided speed.
>>
>> It also changes a few places switch was used to use
>> this new function.  This changes a bit the way the
>> speed is printed in few instances at the same time
>> standardising it.
>
>> --- a/drivers/usb/Makefile
>> +++ b/drivers/usb/Makefile
>> @@ -51,3 +51,6 @@ obj-$(CONFIG_USB_MUSB_HDRC)	+= musb/
>>  obj-$(CONFIG_USB_RENESAS_USBHS)	+= renesas_usbhs/
>>  obj-$(CONFIG_USB_OTG_UTILS)	+= otg/
>>  obj-$(CONFIG_USB_GADGET)	+= gadget/
>> +
>> +obj-$(CONFIG_USB)		+= common.o
>> +obj-$(CONFIG_USB_GADGET)	+= common.o
>
> Will this cause problems?  For example, what if CONFIG_USB is Y and
> CONFIG_USB_GADGET is M?

Hmm, I haven't thought about that.

> Also, it seems a little silly to have an entire kernel module devoted
> to a single, little function.

Agreed.  And again, I haven't thought about compiling as a module which
would result in a major overkill.

> This is a symptom of the difficulty of
> sharing code between the host-side and gadget-side USB stacks.

Agreed.

I'm open to any suggestions on how this could be solved.

Sticking the function to the header file as static inline seems
like bit... ”bloaty”.

If this is not easily solvable, I'll probably just drop this patch
for now.

-- 
Best regards,                                         _     _
.o. | Liege of Serenely Enlightened Majesty of      o' \,=./ `o
..o | Computer Science,  Michal "mina86" Nazarewicz    (o o)
ooo +-----<email/xmpp: mnazarewicz@google.com>-----ooO--(_)--Ooo--

^ permalink raw reply	[flat|nested] 15+ messages in thread

* Re: [PATCHv4 1/4] usb: Provide usb_speed_string() function
  2011-08-22 19:42     ` Michal Nazarewicz
@ 2011-08-22 21:06       ` Alan Stern
  2011-08-22 21:16         ` Michal Nazarewicz
  0 siblings, 1 reply; 15+ messages in thread
From: Alan Stern @ 2011-08-22 21:06 UTC (permalink / raw)
  To: Michal Nazarewicz
  Cc: Felipe Balbi, Sebastian Andrzej Siewior, Yang Rui Rui,
	Dave Young, linux-usb, linux-kernel

On Mon, 22 Aug 2011, Michal Nazarewicz wrote:

> >> --- a/drivers/usb/Makefile
> >> +++ b/drivers/usb/Makefile
> >> @@ -51,3 +51,6 @@ obj-$(CONFIG_USB_MUSB_HDRC)	+= musb/
> >>  obj-$(CONFIG_USB_RENESAS_USBHS)	+= renesas_usbhs/
> >>  obj-$(CONFIG_USB_OTG_UTILS)	+= otg/
> >>  obj-$(CONFIG_USB_GADGET)	+= gadget/
> >> +
> >> +obj-$(CONFIG_USB)		+= common.o
> >> +obj-$(CONFIG_USB_GADGET)	+= common.o
> >
> > Will this cause problems?  For example, what if CONFIG_USB is Y and
> > CONFIG_USB_GADGET is M?
> 
> Hmm, I haven't thought about that.
> 
> > Also, it seems a little silly to have an entire kernel module devoted
> > to a single, little function.
> 
> Agreed.  And again, I haven't thought about compiling as a module which
> would result in a major overkill.
> 
> > This is a symptom of the difficulty of
> > sharing code between the host-side and gadget-side USB stacks.
> 
> Agreed.
> 
> I'm open to any suggestions on how this could be solved.
> 
> Sticking the function to the header file as static inline seems
> like bit... ”bloaty”.
> 
> If this is not easily solvable, I'll probably just drop this patch
> for now.

You could do:

	obj-$(CONFIG_USB_SUPPORT)  += common.o

which would build that subroutine into the main kernel whenever either 
side of the USB stack is enabled.

Alan Stern


^ permalink raw reply	[flat|nested] 15+ messages in thread

* Re: [PATCHv4 1/4] usb: Provide usb_speed_string() function
  2011-08-22 21:06       ` Alan Stern
@ 2011-08-22 21:16         ` Michal Nazarewicz
  2011-08-24  8:57           ` Felipe Balbi
  0 siblings, 1 reply; 15+ messages in thread
From: Michal Nazarewicz @ 2011-08-22 21:16 UTC (permalink / raw)
  To: Alan Stern
  Cc: Felipe Balbi, Sebastian Andrzej Siewior, Yang Rui Rui,
	Dave Young, linux-usb, linux-kernel

>>>> +obj-$(CONFIG_USB)		+= common.o
>>>> +obj-$(CONFIG_USB_GADGET)	+= common.o

>>> Will this cause problems?  For example, what if CONFIG_USB is Y and
>>> CONFIG_USB_GADGET is M?

>>> Also, it seems a little silly to have an entire kernel module devoted
>>> to a single, little function.

On Mon, 22 Aug 2011 23:06:43 +0200, Alan Stern <stern@rowland.harvard.edu>  
wrote:
> You could do:
>
> 	obj-$(CONFIG_USB_SUPPORT)  += common.o
>
> which would build that subroutine into the main kernel whenever either
> side of the USB stack is enabled.

OK.  I'll wait for any more possible comments and post changed version
by the end of the week.

-- 
Best regards,                                         _     _
.o. | Liege of Serenely Enlightened Majesty of      o' \,=./ `o
..o | Computer Science,  Michal "mina86" Nazarewicz    (o o)
ooo +-----<email/xmpp: mnazarewicz@google.com>-----ooO--(_)--Ooo--

^ permalink raw reply	[flat|nested] 15+ messages in thread

* Re: [PATCHv4 1/4] usb: Provide usb_speed_string() function
  2011-08-22 17:28   ` Alan Stern
  2011-08-22 19:42     ` Michal Nazarewicz
@ 2011-08-24  2:16     ` Dave Young
  1 sibling, 0 replies; 15+ messages in thread
From: Dave Young @ 2011-08-24  2:16 UTC (permalink / raw)
  To: Alan Stern
  Cc: Michal Nazarewicz, Felipe Balbi, Sebastian Andrzej Siewior,
	Yang Rui Rui, linux-usb, linux-kernel

On Tue, Aug 23, 2011 at 1:28 AM, Alan Stern <stern@rowland.harvard.edu> wrote:
> On Mon, 22 Aug 2011, Michal Nazarewicz wrote:
>
>> From: Michal Nazarewicz <mina86@mina86.com>
>>
>> In a few places in the kernel, the code prints
>> a human-readable USB device speed (eg. "high speed").
>> This involves a switch statement sometimes wrapped
>> around in ({ ... }) block leading to code repetition.
>>
>> To mitigate this issue, this commit introduces
>> usb_speed_string() function, which returns
>> a human-readable name of provided speed.
>>
>> It also changes a few places switch was used to use
>> this new function.  This changes a bit the way the
>> speed is printed in few instances at the same time
>> standardising it.
>
>> --- a/drivers/usb/Makefile
>> +++ b/drivers/usb/Makefile
>> @@ -51,3 +51,6 @@ obj-$(CONFIG_USB_MUSB_HDRC) += musb/
>>  obj-$(CONFIG_USB_RENESAS_USBHS)      += renesas_usbhs/
>>  obj-$(CONFIG_USB_OTG_UTILS)  += otg/
>>  obj-$(CONFIG_USB_GADGET)     += gadget/
>> +
>> +obj-$(CONFIG_USB)            += common.o
>> +obj-$(CONFIG_USB_GADGET)     += common.o
>
> Will this cause problems?  For example, what if CONFIG_USB is Y and
> CONFIG_USB_GADGET is M?
>
> Also, it seems a little silly to have an entire kernel module devoted
> to a single, little function.  This is a symptom of the difficulty of
> sharing code between the host-side and gadget-side USB stacks.
>

How about put the code as inline function in ch9.h?

> Alan Stern
>
>



-- 
Regards
Yang RuiRui

^ permalink raw reply	[flat|nested] 15+ messages in thread

* Re: [PATCHv4 3/4] usb: gadget: rename usb_gadget_driver::speed to max_speed
  2011-08-22 16:04 ` [PATCHv4 3/4] usb: gadget: rename usb_gadget_driver::speed to max_speed Michal Nazarewicz
@ 2011-08-24  2:26   ` Dave Young
  2011-08-24  8:58     ` Felipe Balbi
  0 siblings, 1 reply; 15+ messages in thread
From: Dave Young @ 2011-08-24  2:26 UTC (permalink / raw)
  To: Michal Nazarewicz
  Cc: Alan Stern, Felipe Balbi, Sebastian Andrzej Siewior,
	Yang Rui Rui, linux-usb, linux-kernel

On Tue, Aug 23, 2011 at 12:04 AM, Michal Nazarewicz
<mnazarewicz@google.com> wrote:
> From: Michal Nazarewicz <mina86@mina86.com>
>
> This commit renames the “speed” field of the usb_gadget_driver
> structure to “max_speed”.  This is so that to make it more
> apparent that the field represents the maximum speed gadget
> driver can support.
>
> This also make the field look more like fields with the same
> name in usb_gadget and usb_composite_driver structures.  All
> of those represent the *maximal* speed given entity supports.
>
> After this commit, there are the following fields in various
> structures:
> * usb_gadget::speed - the current connection speed,
> * usb_gadget::max_speed - maximal speed UDC supports,
> * usb_gadget_driver::max_speed - maximal speed gadget driver
>  supports, and
> * usb_composite_driver::max_speed - maximal speed composite
>  gadget supports.
>
> Signed-off-by: Michal Nazarewicz <mina86@mina86.com>
> ---
>  drivers/usb/gadget/amd5536udc.c   |    2 +-
>  drivers/usb/gadget/ci13xxx_udc.c  |    2 +-
>  drivers/usb/gadget/composite.c    |    8 ++++----
>  drivers/usb/gadget/dbgp.c         |    2 +-
>  drivers/usb/gadget/dummy_hcd.c    |   13 ++++++-------
>  drivers/usb/gadget/file_storage.c |    4 ++--
>  drivers/usb/gadget/gmidi.c        |    2 +-
>  drivers/usb/gadget/goku_udc.c     |    2 +-
>  drivers/usb/gadget/inode.c        |    6 +++---
>  drivers/usb/gadget/m66592-udc.c   |    2 +-
>  drivers/usb/gadget/net2272.c      |    2 +-
>  drivers/usb/gadget/net2280.c      |    2 +-
>  drivers/usb/gadget/pch_udc.c      |    2 +-
>  drivers/usb/gadget/printer.c      |    2 +-
>  drivers/usb/gadget/r8a66597-udc.c |    2 +-
>  include/linux/usb/gadget.h        |    4 ++--
>  16 files changed, 28 insertions(+), 29 deletions(-)
>

Still missing drivers/usb/musb/musb_gadget for the speed check


-- 
Regards
Yang RuiRui

^ permalink raw reply	[flat|nested] 15+ messages in thread

* Re: [PATCHv4 1/4] usb: Provide usb_speed_string() function
  2011-08-22 21:16         ` Michal Nazarewicz
@ 2011-08-24  8:57           ` Felipe Balbi
  2011-08-24 12:59             ` Michal Nazarewicz
  0 siblings, 1 reply; 15+ messages in thread
From: Felipe Balbi @ 2011-08-24  8:57 UTC (permalink / raw)
  To: Michal Nazarewicz
  Cc: Alan Stern, Felipe Balbi, Sebastian Andrzej Siewior,
	Yang Rui Rui, Dave Young, linux-usb, linux-kernel

[-- Attachment #1: Type: text/plain, Size: 870 bytes --]

Hi,

On Mon, Aug 22, 2011 at 11:16:07PM +0200, Michal Nazarewicz wrote:
> >>>>+obj-$(CONFIG_USB)		+= common.o
> >>>>+obj-$(CONFIG_USB_GADGET)	+= common.o
> 
> >>>Will this cause problems?  For example, what if CONFIG_USB is Y and
> >>>CONFIG_USB_GADGET is M?
> 
> >>>Also, it seems a little silly to have an entire kernel module devoted
> >>>to a single, little function.
> 
> On Mon, 22 Aug 2011 23:06:43 +0200, Alan Stern
> <stern@rowland.harvard.edu> wrote:
> >You could do:
> >
> >	obj-$(CONFIG_USB_SUPPORT)  += common.o
> >
> >which would build that subroutine into the main kernel whenever either
> >side of the USB stack is enabled.
> 
> OK.  I'll wait for any more possible comments and post changed version
> by the end of the week.

if it's only for one single function, wouldn't a static inline in a
common header do ?

-- 
balbi

[-- Attachment #2: Digital signature --]
[-- Type: application/pgp-signature, Size: 490 bytes --]

^ permalink raw reply	[flat|nested] 15+ messages in thread

* Re: [PATCHv4 3/4] usb: gadget: rename usb_gadget_driver::speed to max_speed
  2011-08-24  2:26   ` Dave Young
@ 2011-08-24  8:58     ` Felipe Balbi
  2011-08-24 13:26       ` Michal Nazarewicz
  0 siblings, 1 reply; 15+ messages in thread
From: Felipe Balbi @ 2011-08-24  8:58 UTC (permalink / raw)
  To: Dave Young
  Cc: Michal Nazarewicz, Alan Stern, Felipe Balbi,
	Sebastian Andrzej Siewior, Yang Rui Rui, linux-usb, linux-kernel

[-- Attachment #1: Type: text/plain, Size: 2205 bytes --]

Hi,

On Wed, Aug 24, 2011 at 10:26:53AM +0800, Dave Young wrote:
> On Tue, Aug 23, 2011 at 12:04 AM, Michal Nazarewicz
> <mnazarewicz@google.com> wrote:
> > From: Michal Nazarewicz <mina86@mina86.com>
> >
> > This commit renames the “speed” field of the usb_gadget_driver
> > structure to “max_speed”.  This is so that to make it more
> > apparent that the field represents the maximum speed gadget
> > driver can support.
> >
> > This also make the field look more like fields with the same
> > name in usb_gadget and usb_composite_driver structures.  All
> > of those represent the *maximal* speed given entity supports.
> >
> > After this commit, there are the following fields in various
> > structures:
> > * usb_gadget::speed - the current connection speed,
> > * usb_gadget::max_speed - maximal speed UDC supports,
> > * usb_gadget_driver::max_speed - maximal speed gadget driver
> >  supports, and
> > * usb_composite_driver::max_speed - maximal speed composite
> >  gadget supports.
> >
> > Signed-off-by: Michal Nazarewicz <mina86@mina86.com>
> > ---
> >  drivers/usb/gadget/amd5536udc.c   |    2 +-
> >  drivers/usb/gadget/ci13xxx_udc.c  |    2 +-
> >  drivers/usb/gadget/composite.c    |    8 ++++----
> >  drivers/usb/gadget/dbgp.c         |    2 +-
> >  drivers/usb/gadget/dummy_hcd.c    |   13 ++++++-------
> >  drivers/usb/gadget/file_storage.c |    4 ++--
> >  drivers/usb/gadget/gmidi.c        |    2 +-
> >  drivers/usb/gadget/goku_udc.c     |    2 +-
> >  drivers/usb/gadget/inode.c        |    6 +++---
> >  drivers/usb/gadget/m66592-udc.c   |    2 +-
> >  drivers/usb/gadget/net2272.c      |    2 +-
> >  drivers/usb/gadget/net2280.c      |    2 +-
> >  drivers/usb/gadget/pch_udc.c      |    2 +-
> >  drivers/usb/gadget/printer.c      |    2 +-
> >  drivers/usb/gadget/r8a66597-udc.c |    2 +-
> >  include/linux/usb/gadget.h        |    4 ++--
> >  16 files changed, 28 insertions(+), 29 deletions(-)
> >
> 
> Still missing drivers/usb/musb/musb_gadget for the speed check

and the new DWC3 on Greg's usb-next branch.

-- 
balbi

[-- Attachment #2: Digital signature --]
[-- Type: application/pgp-signature, Size: 490 bytes --]

^ permalink raw reply	[flat|nested] 15+ messages in thread

* Re: [PATCHv4 1/4] usb: Provide usb_speed_string() function
  2011-08-24  8:57           ` Felipe Balbi
@ 2011-08-24 12:59             ` Michal Nazarewicz
  0 siblings, 0 replies; 15+ messages in thread
From: Michal Nazarewicz @ 2011-08-24 12:59 UTC (permalink / raw)
  To: Felipe Balbi
  Cc: Alan Stern, Sebastian Andrzej Siewior, Yang Rui Rui, Dave Young,
	linux-usb, linux-kernel

On Wed, 24 Aug 2011 10:57:39 +0200, Felipe Balbi <balbi@ti.com> wrote:

> Hi,
>
> On Mon, Aug 22, 2011 at 11:16:07PM +0200, Michal Nazarewicz wrote:
>> >>>>+obj-$(CONFIG_USB)		+= common.o
>> >>>>+obj-$(CONFIG_USB_GADGET)	+= common.o
>>
>> >>>Will this cause problems?  For example, what if CONFIG_USB is Y and
>> >>>CONFIG_USB_GADGET is M?
>>
>> >>>Also, it seems a little silly to have an entire kernel module devoted
>> >>>to a single, little function.
>>
>> On Mon, 22 Aug 2011 23:06:43 +0200, Alan Stern
>> <stern@rowland.harvard.edu> wrote:
>> >You could do:
>> >
>> >	obj-$(CONFIG_USB_SUPPORT)  += common.o
>> >
>> >which would build that subroutine into the main kernel whenever either
>> >side of the USB stack is enabled.
>>
>> OK.  I'll wait for any more possible comments and post changed version
>> by the end of the week.
>
> if it's only for one single function, wouldn't a static inline in a
> common header do ?

That would defeat the purpose a little.  The code would not get duplicated
in many source files but the actual compiled code would end up in several
places.

-- 
Best regards,                                         _     _
.o. | Liege of Serenely Enlightened Majesty of      o' \,=./ `o
..o | Computer Science,  Michal "mina86" Nazarewicz    (o o)
ooo +-----<email/xmpp: mnazarewicz@google.com>-----ooO--(_)--Ooo--

^ permalink raw reply	[flat|nested] 15+ messages in thread

* Re: [PATCHv4 3/4] usb: gadget: rename usb_gadget_driver::speed to max_speed
  2011-08-24  8:58     ` Felipe Balbi
@ 2011-08-24 13:26       ` Michal Nazarewicz
  0 siblings, 0 replies; 15+ messages in thread
From: Michal Nazarewicz @ 2011-08-24 13:26 UTC (permalink / raw)
  To: Dave Young, Felipe Balbi
  Cc: Alan Stern, Sebastian Andrzej Siewior, Yang Rui Rui, linux-usb,
	linux-kernel

> On Wed, Aug 24, 2011 at 10:26:53AM +0800, Dave Young wrote:
>> Still missing drivers/usb/musb/musb_gadget for the speed check

Fixed.

On Wed, 24 Aug 2011 10:58:29 +0200, Felipe Balbi <balbi@ti.com> wrote:
> and the new DWC3 on Greg's usb-next branch.

Fixed.

-- 
Best regards,                                         _     _
.o. | Liege of Serenely Enlightened Majesty of      o' \,=./ `o
..o | Computer Science,  Michal "mina86" Nazarewicz    (o o)
ooo +-----<email/xmpp: mnazarewicz@google.com>-----ooO--(_)--Ooo--

^ permalink raw reply	[flat|nested] 15+ messages in thread

end of thread, other threads:[~2011-08-24 13:27 UTC | newest]

Thread overview: 15+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2011-08-22 16:04 [PATCHv4 0/4] Removing USB_GADGET_*SPEED macros Michal Nazarewicz
2011-08-22 16:04 ` [PATCHv4 1/4] usb: Provide usb_speed_string() function Michal Nazarewicz
2011-08-22 17:28   ` Alan Stern
2011-08-22 19:42     ` Michal Nazarewicz
2011-08-22 21:06       ` Alan Stern
2011-08-22 21:16         ` Michal Nazarewicz
2011-08-24  8:57           ` Felipe Balbi
2011-08-24 12:59             ` Michal Nazarewicz
2011-08-24  2:16     ` Dave Young
2011-08-22 16:04 ` [PATCHv4 2/4] usb: gadget: replace "is_dualspeed" with "max_speed" Michal Nazarewicz
2011-08-22 16:04 ` [PATCHv4 3/4] usb: gadget: rename usb_gadget_driver::speed to max_speed Michal Nazarewicz
2011-08-24  2:26   ` Dave Young
2011-08-24  8:58     ` Felipe Balbi
2011-08-24 13:26       ` Michal Nazarewicz
2011-08-22 16:04 ` [PATCHv4 4/4] usb: gadget: get rid of USB_GADGET_{DUAL,SUPER}SPEED Michal Nazarewicz

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).