[PATCHv6 0/7] Several USB fixes

[PATCHv6 0/7] Several USB fixes

[Michal Nazarewicz]

Hello again,

Michal Nazarewicz (7):

  USB: gadget: storage_common: fixed warning building mass storage
    function

    This fixes a warning in the storage_common.c file.

  USB: gadget: g_multi: fixed vendor and product ID
  USB: gadget: g_ffs: fixed vendor and product ID

    Those fix the product and vendor IDs of the g_multi and g_ffs
    gadgets.

Greg, could you pull the above three patches regardless of the status
of next four.  The first one was already sent and no one seemed to
have any troubles with it.  The second and the third are trivial
fixes.

  USB: gadget: composite: Better string override handling

    This fixes the string override in the composite framework.

David, could you look at it again and point out where exactly does it
use an unmanaged serial number, please?  I still don't see it...

  USB: gadget: mass_storage: moved strings handling code to composite
  USB: gadget: functionfs: code cleanup
  USB: gadget: g_multi: moved strings handling code to composite

    Those change g_mass_storage, g_ffs and g_multi to use new
    functionality of the composite layer.  In previous patchset I sent
    it as a single patch, splatted it as per David's request.

 drivers/usb/gadget/composite.c      |   95 +++++++++++++++++++++++------------
 drivers/usb/gadget/g_ffs.c          |   85 ++++++++++---------------------
 drivers/usb/gadget/mass_storage.c   |   72 ++------------------------
 drivers/usb/gadget/multi.c          |   23 ++-------
 drivers/usb/gadget/storage_common.c |   10 ++--
 include/linux/usb/composite.h       |   13 +++++
 6 files changed, 120 insertions(+), 178 deletions(-)

[PATCHv6 1/7] USB: gadget: storage_common: fixed warning building mass storage function

[Michal Nazarewicz]

The "nofua" attribute is used in the File-Storage Gadget but the
functions handling it are defined in storage_common.c (which may be
not a bad thing if anyone will want to port tho "nofua" attribute to
mass storage function) which causes the following warnings:

drivers/usb/gadget/storage_common.c:718: \
  warning: ‘fsg_show_nofua’ defined but not used
drivers/usb/gadget/storage_common.c:782: \
  warning: ‘fsg_store_nofua’ defined but not used

Adding __maybe_unused fixes this issue.

Signed-off-by: Michal Nazarewicz <m.nazarewicz@samsung.com>
Cc: Andy Shevchenko <andy.shevchenko@gmail.com>
---
 drivers/usb/gadget/storage_common.c |   10 +++++-----
 1 files changed, 5 insertions(+), 5 deletions(-)

diff --git a/drivers/usb/gadget/storage_common.c b/drivers/usb/gadget/storage_common.c
index 484acfb..3379cc3 100644
--- a/drivers/usb/gadget/storage_common.c
+++ b/drivers/usb/gadget/storage_common.c
@@ -715,8 +715,8 @@ static ssize_t fsg_show_ro(struct device *dev, struct device_attribute *attr,
 				  : curlun->initially_ro);
 }
 
-static ssize_t fsg_show_nofua(struct device *dev, struct device_attribute *attr,
-			      char *buf)
+static ssize_t __maybe_unused
+fsg_show_nofua(struct device *dev, struct device_attribute *attr, char *buf)
 {
 	struct fsg_lun	*curlun = fsg_lun_from_dev(dev);
 
@@ -779,9 +779,9 @@ static ssize_t fsg_store_ro(struct device *dev, struct device_attribute *attr,
 	return rc;
 }
 
-static ssize_t fsg_store_nofua(struct device *dev,
-			       struct device_attribute *attr,
-			       const char *buf, size_t count)
+static ssize_t __maybe_unused
+fsg_store_nofua(struct device *dev, struct device_attribute *attr,
+		const char *buf, size_t count)
 {
 	struct fsg_lun	*curlun = fsg_lun_from_dev(dev);
 	unsigned long	nofua;
-- 
1.7.1

[PATCHv6 2/7] USB: gadget: g_multi: fixed vendor and product ID

[Michal Nazarewicz]

This patch fixes the vendor and product ID the gadget uses
by replacing the temporary IDs that were used during
development (which should never get into mainline) with
proper IDs.

Signed-off-by: Michal Nazarewicz <m.nazarewicz@samsung.com>
Signed-off-by: Kyungmin Park <kyungmin.park@samsung.com>
---
 drivers/usb/gadget/multi.c |    4 ++--
 1 files changed, 2 insertions(+), 2 deletions(-)

diff --git a/drivers/usb/gadget/multi.c b/drivers/usb/gadget/multi.c
index 795d762..36d67a3 100644
--- a/drivers/usb/gadget/multi.c
+++ b/drivers/usb/gadget/multi.c
@@ -74,8 +74,8 @@ MODULE_LICENSE("GPL");
 
 /***************************** Device Descriptor ****************************/
 
-#define MULTI_VENDOR_NUM	0x0525	/* XXX NetChip */
-#define MULTI_PRODUCT_NUM	0xa4ab	/* XXX */
+#define MULTI_VENDOR_NUM	0x1d6b	/* Linux Foundation */
+#define MULTI_PRODUCT_NUM	0x0104	/* Multifunction Composite Gadget */
 
 
 enum {
-- 
1.7.1

[PATCHv6 3/7] USB: gadget: g_ffs: fixed vendor and product ID

[Michal Nazarewicz]

This patch fixes the vendor and product ID the gadget uses
by replacing the temporary IDs that were used during
development (which should never get into mainline) with
proper IDs.

Signed-off-by: Michal Nazarewicz <m.nazarewicz@samsung.com>
Signed-off-by: Kyungmin Park <kyungmin.park@samsung.com>
---
 drivers/usb/gadget/g_ffs.c |    4 ++--
 1 files changed, 2 insertions(+), 2 deletions(-)

diff --git a/drivers/usb/gadget/g_ffs.c b/drivers/usb/gadget/g_ffs.c
index a9474f8..3c2f0a4 100644
--- a/drivers/usb/gadget/g_ffs.c
+++ b/drivers/usb/gadget/g_ffs.c
@@ -53,8 +53,8 @@ MODULE_AUTHOR("Michal Nazarewicz");
 MODULE_LICENSE("GPL");
 
 
-static unsigned short gfs_vendor_id    = 0x0525;	/* XXX NetChip */
-static unsigned short gfs_product_id   = 0xa4ac;	/* XXX */
+static unsigned short gfs_vendor_id    = 0x1d6b;	/* Linux Foundation */
+static unsigned short gfs_product_id   = 0x0105;	/* FunctionFS Gadget */
 
 static struct usb_device_descriptor gfs_dev_desc = {
 	.bLength		= sizeof gfs_dev_desc,
-- 
1.7.1

[PATCHv6 4/7] USB: gadget: composite: Better string override handling

[Michal Nazarewicz]

The iManufatcurer, iProduct and iSerialNumber composite module
parameters were only used when the gadget driver registers
strings for manufacturer, product and serial number.  If the
gadget never bothered to set corresponding fields in USB device
descriptors those module parameters are ignored.

This commit makes the parameters work even if the strings ID
have not been assigned.  It also changes the way IDs are
overridden -- what IDs are overridden is now saved in
usb_composite_dev structure -- which makes it unnecessary to
modify the string tables the way previous code did.

The commit also adds a iProduct and iManufatcurer fields to the
usb_composite_device structure.  If they are set, appropriate
strings are reserved and added to device descriptor.  This makes
it unnecessary for gadget drivers to maintain code for setting
those.  If iProduct is not set it defaults to
usb_composite_device::name; if iManufatcurer is not set
a default "<system> <release> with <gadget-name>" is used.

The last thing is that if needs_serial field of
usb_composite_device is set and user failed to provided
iSerialNumber parameter a warning is issued.

Signed-off-by: Michal Nazarewicz <m.nazarewicz@samsung.com>
Signed-off-by: Kyungmin Park <kyungmin.park@samsung.com>
---
 drivers/usb/gadget/composite.c |   95 ++++++++++++++++++++++++++-------------
 include/linux/usb/composite.h  |   13 +++++
 2 files changed, 76 insertions(+), 32 deletions(-)

diff --git a/drivers/usb/gadget/composite.c b/drivers/usb/gadget/composite.c
index e483f80..38f7982 100644
--- a/drivers/usb/gadget/composite.c
+++ b/drivers/usb/gadget/composite.c
@@ -69,6 +69,8 @@ static char *iSerialNumber;
 module_param(iSerialNumber, charp, 0);
 MODULE_PARM_DESC(iSerialNumber, "SerialNumber string");
 
+static char composite_manufacturer[50];
+
 /*-------------------------------------------------------------------------*/
 
 /**
@@ -599,6 +601,7 @@ static int get_string(struct usb_composite_dev *cdev,
 	struct usb_configuration	*c;
 	struct usb_function		*f;
 	int				len;
+	const char			*str;
 
 	/* Yes, not only is USB's I18N support probably more than most
 	 * folk will ever care about ... also, it's all supported here.
@@ -638,9 +641,29 @@ static int get_string(struct usb_composite_dev *cdev,
 		return s->bLength;
 	}
 
-	/* Otherwise, look up and return a specified string.  String IDs
-	 * are device-scoped, so we look up each string table we're told
-	 * about.  These lookups are infrequent; simpler-is-better here.
+	/* Otherwise, look up and return a specified string.  First
+	 * check if the string has not been overridden.
+	 */
+	if (cdev->manufacturer_override == id)
+		str = iManufacturer ?: composite->iManufacturer ?:
+			composite_manufacturer;
+	else if (cdev->product_override == id)
+		str = iProduct ?: composite->iProduct;
+	else if (cdev->serial_override == id)
+		str = iSerialNumber;
+	else
+		str = NULL;
+	if (str) {
+		struct usb_gadget_strings strings = {
+			.language = language,
+			.strings  = &(struct usb_string) { 0xff, str }
+		};
+		return usb_gadget_get_string(&strings, 0xff, buf);
+	}
+
+	/* String IDs are device-scoped, so we look up each string
+	 * table we're told about.  These lookups are infrequent;
+	 * simpler-is-better here.
 	 */
 	if (composite->strings) {
 		len = lookup_string(composite->strings, buf, language, id);
@@ -1025,26 +1048,17 @@ composite_unbind(struct usb_gadget *gadget)
 	composite = NULL;
 }
 
-static void
-string_override_one(struct usb_gadget_strings *tab, u8 id, const char *s)
+static u8 override_id(struct usb_composite_dev *cdev, u8 *desc)
 {
-	struct usb_string		*str = tab->strings;
-
-	for (str = tab->strings; str->s; str++) {
-		if (str->id == id) {
-			str->s = s;
-			return;
-		}
+	if (!*desc) {
+		int ret = usb_string_id(cdev);
+		if (unlikely(ret < 0))
+			WARNING(cdev, "failed to override string ID\n");
+		else
+			*desc = ret;
 	}
-}
 
-static void
-string_override(struct usb_gadget_strings **tab, u8 id, const char *s)
-{
-	while (*tab) {
-		string_override_one(*tab, id, s);
-		tab++;
-	}
+	return *desc;
 }
 
 static int composite_bind(struct usb_gadget *gadget)
@@ -1101,19 +1115,34 @@ static int composite_bind(struct usb_gadget *gadget)
 	cdev->desc = *composite->dev;
 	cdev->desc.bMaxPacketSize0 = gadget->ep0->maxpacket;
 
-	/* strings can't be assigned before bind() allocates the
-	 * releavnt identifiers
-	 */
-	if (cdev->desc.iManufacturer && iManufacturer)
-		string_override(composite->strings,
-			cdev->desc.iManufacturer, iManufacturer);
-	if (cdev->desc.iProduct && iProduct)
-		string_override(composite->strings,
-			cdev->desc.iProduct, iProduct);
-	if (cdev->desc.iSerialNumber && iSerialNumber)
-		string_override(composite->strings,
-			cdev->desc.iSerialNumber, iSerialNumber);
+	/* stirng overrides */
+	if (iManufacturer || !cdev->desc.iManufacturer) {
+		if (!iManufacturer && !composite->iManufacturer &&
+		    !*composite_manufacturer)
+			snprintf(composite_manufacturer,
+				 sizeof composite_manufacturer,
+				 "%s %s with %s",
+				 init_utsname()->sysname,
+				 init_utsname()->release,
+				 gadget->name);
+
+		cdev->manufacturer_override =
+			override_id(cdev, &cdev->desc.iManufacturer);
+	}
+
+	if (iProduct || (!cdev->desc.iProduct && composite->iProduct))
+		cdev->product_override =
+			override_id(cdev, &cdev->desc.iProduct);
+
+	if (iSerialNumber)
+		cdev->serial_override =
+			override_id(cdev, &cdev->desc.iSerialNumber);
+
+	/* has userspace failed to provide a serial number? */
+	if (composite->needs_serial && !cdev->desc.iSerialNumber)
+		WARNING(cdev, "userspace failed to provide iSerialNumber\n");
 
+	/* finish up */
 	status = device_create_file(&gadget->dev, &dev_attr_suspended);
 	if (status)
 		goto fail;
@@ -1211,6 +1240,8 @@ int usb_composite_register(struct usb_composite_driver *driver)
 	if (!driver || !driver->dev || !driver->bind || composite)
 		return -EINVAL;
 
+	if (!driver->iProduct)
+		driver->iProduct = driver->name;
 	if (!driver->name)
 		driver->name = "composite";
 	composite_driver.function =  (char *) driver->name;
diff --git a/include/linux/usb/composite.h b/include/linux/usb/composite.h
index 890bc14..d590904 100644
--- a/include/linux/usb/composite.h
+++ b/include/linux/usb/composite.h
@@ -237,10 +237,17 @@ int usb_add_config(struct usb_composite_dev *,
 /**
  * struct usb_composite_driver - groups configurations into a gadget
  * @name: For diagnostics, identifies the driver.
+ * @iProduct: Used as iProduct override if @dev->iProduct is not set.
+ *	If NULL value of @name is taken.
+ * @iManufacturer: Used as iManufacturer override if @dev->iManufacturer is
+ *	not set. If NULL a default "<system> <release> with <udc>" value
+ *	will be used.
  * @dev: Template descriptor for the device, including default device
  *	identifiers.
  * @strings: tables of strings, keyed by identifiers assigned during bind()
  *	and language IDs provided in control requests
+ * @needs_serial: set to 1 if the gadget needs userspace to provide
+ * 	a serial number.  If one is not provided, warning will be printed.
  * @bind: (REQUIRED) Used to allocate resources that are shared across the
  *	whole device, such as string IDs, and add its configurations using
  *	@usb_add_config().  This may fail by returning a negative errno
@@ -265,8 +272,11 @@ int usb_add_config(struct usb_composite_dev *,
  */
 struct usb_composite_driver {
 	const char				*name;
+	const char				*iProduct;
+	const char				*iManufacturer;
 	const struct usb_device_descriptor	*dev;
 	struct usb_gadget_strings		**strings;
+	unsigned		needs_serial:1;
 
 	/* REVISIT:  bind() functions can be marked __init, which
 	 * makes trouble for section mismatch analysis.  See if
@@ -333,6 +343,9 @@ struct usb_composite_dev {
 	struct list_head		configs;
 	struct usb_composite_driver	*driver;
 	u8				next_string_id;
+	u8				manufacturer_override;
+	u8				product_override;
+	u8				serial_override;
 
 	/* the gadget driver won't enable the data pullup
 	 * while the deactivation count is nonzero.
-- 
1.7.1

[PATCHv6 5/7] USB: gadget: mass_storage: moved strings handling code to composite

[Michal Nazarewicz]

This patch removes string registration from the Mass Storage
Gadget.  With recent changes to the composite framework, all
that we need is handled by the composite layer.  This means
composite registers a string ID for manufacturer and product.

This also adds the "needs_serial" so that composite layer will
issue a warning if user space fails to provide the iSerialNumber
module parameter.

Signed-off-by: Michal Nazarewicz <m.nazarewicz@samsung.com>
Signed-off-by: Kyungmin Park <kyungmin.park@samsung.com>
---
 drivers/usb/gadget/mass_storage.c |   72 +++---------------------------------
 1 files changed, 6 insertions(+), 66 deletions(-)

diff --git a/drivers/usb/gadget/mass_storage.c b/drivers/usb/gadget/mass_storage.c
index 585f255..493d153 100644
--- a/drivers/usb/gadget/mass_storage.c
+++ b/drivers/usb/gadget/mass_storage.c
@@ -75,10 +75,6 @@ static struct usb_device_descriptor msg_device_desc = {
 	/* Vendor and product id can be overridden by module parameters.  */
 	.idVendor =		cpu_to_le16(FSG_VENDOR_ID),
 	.idProduct =		cpu_to_le16(FSG_PRODUCT_ID),
-	/* .bcdDevice = f(hardware) */
-	/* .iManufacturer = DYNAMIC */
-	/* .iProduct = DYNAMIC */
-	/* NO SERIAL NUMBER */
 	.bNumConfigurations =	1,
 };
 
@@ -86,7 +82,8 @@ static struct usb_otg_descriptor otg_descriptor = {
 	.bLength =		sizeof otg_descriptor,
 	.bDescriptorType =	USB_DT_OTG,
 
-	/* REVISIT SRP-only hardware is possible, although
+	/*
+	 * REVISIT SRP-only hardware is possible, although
 	 * it would not be called "OTG" ...
 	 */
 	.bmAttributes =		USB_OTG_SRP | USB_OTG_HNP,
@@ -98,33 +95,6 @@ static const struct usb_descriptor_header *otg_desc[] = {
 };
 
 
-/* string IDs are assigned dynamically */
-
-#define STRING_MANUFACTURER_IDX		0
-#define STRING_PRODUCT_IDX		1
-#define STRING_CONFIGURATION_IDX	2
-
-static char manufacturer[50];
-
-static struct usb_string strings_dev[] = {
-	[STRING_MANUFACTURER_IDX].s = manufacturer,
-	[STRING_PRODUCT_IDX].s = DRIVER_DESC,
-	[STRING_CONFIGURATION_IDX].s = "Self Powered",
-	{  } /* end of list */
-};
-
-static struct usb_gadget_strings stringtab_dev = {
-	.language	= 0x0409,	/* en-us */
-	.strings	= strings_dev,
-};
-
-static struct usb_gadget_strings *dev_strings[] = {
-	&stringtab_dev,
-	NULL,
-};
-
-
-
 /****************************** Configurations ******************************/
 
 static struct fsg_module_parameters mod_data = {
@@ -173,52 +143,22 @@ static struct usb_configuration msg_config_driver = {
 	.label			= "Linux File-Backed Storage",
 	.bind			= msg_do_config,
 	.bConfigurationValue	= 1,
-	/* .iConfiguration = DYNAMIC */
 	.bmAttributes		= USB_CONFIG_ATT_SELFPOWER,
 };
 
 
-
 /****************************** Gadget Bind ******************************/
 
-
 static int __ref msg_bind(struct usb_composite_dev *cdev)
 {
-	struct usb_gadget *gadget = cdev->gadget;
 	int status;
 
-	/* Allocate string descriptor numbers ... note that string
-	 * contents can be overridden by the composite_dev glue.
-	 */
-
-	/* device descriptor strings: manufacturer, product */
-	snprintf(manufacturer, sizeof manufacturer, "%s %s with %s",
-	         init_utsname()->sysname, init_utsname()->release,
-	         gadget->name);
-	status = usb_string_id(cdev);
-	if (status < 0)
-		return status;
-	strings_dev[STRING_MANUFACTURER_IDX].id = status;
-	msg_device_desc.iManufacturer = status;
-
-	status = usb_string_id(cdev);
-	if (status < 0)
-		return status;
-	strings_dev[STRING_PRODUCT_IDX].id = status;
-	msg_device_desc.iProduct = status;
-
-	status = usb_string_id(cdev);
-	if (status < 0)
-		return status;
-	strings_dev[STRING_CONFIGURATION_IDX].id = status;
-	msg_config_driver.iConfiguration = status;
-
-	/* register our second configuration */
 	status = usb_add_config(cdev, &msg_config_driver);
 	if (status < 0)
 		return status;
 
-	dev_info(&gadget->dev, DRIVER_DESC ", version: " DRIVER_VERSION "\n");
+	dev_info(&cdev->gadget->dev,
+		 DRIVER_DESC ", version: " DRIVER_VERSION "\n");
 	set_bit(0, &msg_registered);
 	return 0;
 }
@@ -226,12 +166,12 @@ static int __ref msg_bind(struct usb_composite_dev *cdev)
 
 /****************************** Some noise ******************************/
 
-
 static struct usb_composite_driver msg_driver = {
 	.name		= "g_mass_storage",
 	.dev		= &msg_device_desc,
-	.strings	= dev_strings,
 	.bind		= msg_bind,
+	.iProduct	= DRIVER_DESC,
+	.needs_serial	= 1,
 };
 
 MODULE_DESCRIPTION(DRIVER_DESC);
-- 
1.7.1

[PATCHv6 6/7] USB: gadget: functionfs: code cleanup

[Michal Nazarewicz]

This patch removes some of the string registration from the
FunctionFS Gadget as composite layer can handle the
iManufacturer and iProduct for us.

It also removes some of the module parameters which were
redundant as well as changes the name of others to better much
the module parameter of the composite layer.

Other then that, it also fixes formatting of multiline comments
to match the coding style.

Signed-off-by: Michal Nazarewicz <m.nazarewicz@samsung.com>
Signed-off-by: Kyungmin Park <kyungmin.park@samsung.com>
---
 drivers/usb/gadget/g_ffs.c |   85 ++++++++++++++-----------------------------
 1 files changed, 28 insertions(+), 57 deletions(-)

diff --git a/drivers/usb/gadget/g_ffs.c b/drivers/usb/gadget/g_ffs.c
index 3c2f0a4..7533328 100644
--- a/drivers/usb/gadget/g_ffs.c
+++ b/drivers/usb/gadget/g_ffs.c
@@ -52,9 +52,8 @@ MODULE_DESCRIPTION(DRIVER_DESC);
 MODULE_AUTHOR("Michal Nazarewicz");
 MODULE_LICENSE("GPL");
 
-
-static unsigned short gfs_vendor_id    = 0x1d6b;	/* Linux Foundation */
-static unsigned short gfs_product_id   = 0x0105;	/* FunctionFS Gadget */
+#define GFS_VENDOR_ID	0x1d6b	/* Linux Foundation */
+#define GFS_PRODUCT_ID	0x0105	/* FunctionFS Gadget */
 
 static struct usb_device_descriptor gfs_dev_desc = {
 	.bLength		= sizeof gfs_dev_desc,
@@ -63,29 +62,16 @@ static struct usb_device_descriptor gfs_dev_desc = {
 	.bcdUSB			= cpu_to_le16(0x0200),
 	.bDeviceClass		= USB_CLASS_PER_INTERFACE,
 
-	/* Vendor and product id can be overridden by module parameters.  */
-	/* .idVendor		= cpu_to_le16(gfs_vendor_id), */
-	/* .idProduct		= cpu_to_le16(gfs_product_id), */
-	/* .bcdDevice		= f(hardware) */
-	/* .iManufacturer	= DYNAMIC */
-	/* .iProduct		= DYNAMIC */
-	/* NO SERIAL NUMBER */
-	.bNumConfigurations	= 1,
+	.idVendor		= cpu_to_le16(GFS_VENDOR_ID),
+	.idProduct		= cpu_to_le16(GFS_PRODUCT_ID),
 };
 
-#define GFS_MODULE_PARAM_DESC(name, field) \
-	MODULE_PARM_DESC(name, "Value of the " #field " field of the device descriptor sent to the host.  Takes effect only prior to the user-space driver registering to the FunctionFS.")
-
-module_param_named(usb_class,    gfs_dev_desc.bDeviceClass,    byte,   0644);
-GFS_MODULE_PARAM_DESC(usb_class, bDeviceClass);
-module_param_named(usb_subclass, gfs_dev_desc.bDeviceSubClass, byte,   0644);
-GFS_MODULE_PARAM_DESC(usb_subclass, bDeviceSubClass);
-module_param_named(usb_protocol, gfs_dev_desc.bDeviceProtocol, byte,   0644);
-GFS_MODULE_PARAM_DESC(usb_protocol, bDeviceProtocol);
-module_param_named(usb_vendor,   gfs_vendor_id,                ushort, 0644);
-GFS_MODULE_PARAM_DESC(usb_vendor, idVendor);
-module_param_named(usb_product,  gfs_product_id,               ushort, 0644);
-GFS_MODULE_PARAM_DESC(usb_product, idProduct);
+module_param_named(bDeviceClass,    gfs_dev_desc.bDeviceClass,    byte,   0644);
+MODULE_PARAM_DESC(bDeviceClass, "USB Device class");
+module_param_named(bDeviceSubClass, gfs_dev_desc.bDeviceSubClass, byte,   0644);
+MODULE_PARAM_DESC(bDeviceSubClass, "USB Device subclass");
+module_param_named(bDeviceProtocol, gfs_dev_desc.bDeviceProtocol, byte,   0644);
+MODULE_PARAM_DESC(bDeviceProtocol, "USB Device protocol");
 
 
 
@@ -95,8 +81,10 @@ static const struct usb_descriptor_header *gfs_otg_desc[] = {
 		.bLength		= sizeof(struct usb_otg_descriptor),
 		.bDescriptorType	= USB_DT_OTG,
 
-		/* REVISIT SRP-only hardware is possible, although
-		 * it would not be called "OTG" ... */
+		/*
+		 * REVISIT SRP-only hardware is possible, although
+		 * it would not be called "OTG" ...
+		 */
 		.bmAttributes		= USB_OTG_SRP | USB_OTG_HNP,
 	},
 
@@ -105,19 +93,7 @@ static const struct usb_descriptor_header *gfs_otg_desc[] = {
 
 /* string IDs are assigned dynamically */
 
-enum {
-	GFS_STRING_MANUFACTURER_IDX,
-	GFS_STRING_PRODUCT_IDX,
-	GFS_STRING_FIRST_CONFIG_IDX,
-};
-
-static       char gfs_manufacturer[50];
-static const char gfs_driver_desc[] = DRIVER_DESC;
-static const char gfs_short_name[]  = DRIVER_NAME;
-
 static struct usb_string gfs_strings[] = {
-	[GFS_STRING_MANUFACTURER_IDX].s = gfs_manufacturer,
-	[GFS_STRING_PRODUCT_IDX].s = gfs_driver_desc,
 #ifdef CONFIG_USB_FUNCTIONFS_RNDIS
 	{ .s = "FunctionFS + RNDIS" },
 #endif
@@ -168,11 +144,12 @@ static int gfs_unbind(struct usb_composite_dev *cdev);
 static int gfs_do_config(struct usb_configuration *c);
 
 static struct usb_composite_driver gfs_driver = {
-	.name		= gfs_short_name,
+	.name		= DRIVER_NAME,
 	.dev		= &gfs_dev_desc,
 	.strings	= gfs_dev_strings,
 	.bind		= gfs_bind,
 	.unbind		= gfs_unbind,
+	.iProduct	= DRIVER_DESC,
 };
 
 
@@ -245,20 +222,10 @@ static int gfs_bind(struct usb_composite_dev *cdev)
 	if (unlikely(ret < 0))
 		goto error_quick;
 
-	gfs_dev_desc.idVendor  = cpu_to_le16(gfs_vendor_id);
-	gfs_dev_desc.idProduct = cpu_to_le16(gfs_product_id);
-
-	snprintf(gfs_manufacturer, sizeof gfs_manufacturer, "%s %s with %s",
-		 init_utsname()->sysname, init_utsname()->release,
-		 cdev->gadget->name);
-
 	ret = usb_string_ids_tab(cdev, gfs_strings);
 	if (unlikely(ret < 0))
 		goto error;
 
-	gfs_dev_desc.iManufacturer = gfs_strings[GFS_STRING_MANUFACTURER_IDX].id;
-	gfs_dev_desc.iProduct      = gfs_strings[GFS_STRING_PRODUCT_IDX].id;
-
 	ret = functionfs_bind(gfs_ffs_data, cdev);
 	if (unlikely(ret < 0))
 		goto error;
@@ -266,9 +233,8 @@ static int gfs_bind(struct usb_composite_dev *cdev)
 	for (i = 0; i < ARRAY_SIZE(gfs_configurations); ++i) {
 		struct gfs_configuration *c = gfs_configurations + i;
 
-		ret = GFS_STRING_FIRST_CONFIG_IDX + i;
-		c->c.label			= gfs_strings[ret].s;
-		c->c.iConfiguration		= gfs_strings[ret].id;
+		c->c.label			= gfs_strings[i].s;
+		c->c.iConfiguration		= gfs_strings[i].id;
 		c->c.bind			= gfs_do_config;
 		c->c.bConfigurationValue	= 1 + i;
 		c->c.bmAttributes		= USB_CONFIG_ATT_SELFPOWER;
@@ -293,13 +259,14 @@ static int gfs_unbind(struct usb_composite_dev *cdev)
 {
 	ENTER();
 
-	/* We may have been called in an error recovery frem
+	/*
+	 * We may have been called in an error recovery from
 	 * composite_bind() after gfs_unbind() failure so we need to
 	 * check if gfs_ffs_data is not NULL since gfs_bind() handles
 	 * all error recovery itself.  I'd rather we werent called
 	 * from composite on orror recovery, but what you're gonna
-	 * do...? */
-
+	 * do...?
+	 */
 	if (gfs_ffs_data) {
 		gether_cleanup();
 		functionfs_unbind(gfs_ffs_data);
@@ -334,14 +301,16 @@ static int gfs_do_config(struct usb_configuration *c)
 	if (unlikely(ret < 0))
 		return ret;
 
-	/* After previous do_configs there may be some invalid
+	/*
+	 * After previous do_configs there may be some invalid
 	 * pointers in c->interface array.  This happens every time
 	 * a user space function with fewer interfaces than a user
 	 * space function that was run before the new one is run.  The
 	 * compasit's set_config() assumes that if there is no more
 	 * then MAX_CONFIG_INTERFACES interfaces in a configuration
 	 * then there is a NULL pointer after the last interface in
-	 * c->interface array.  We need to make sure this is true. */
+	 * c->interface array.  We need to make sure this is true.
+	 */
 	if (c->next_interface_id < ARRAY_SIZE(c->interface))
 		c->interface[c->next_interface_id] = NULL;
 
@@ -350,10 +319,12 @@ static int gfs_do_config(struct usb_configuration *c)
 
 
 #ifdef CONFIG_USB_FUNCTIONFS_ETH
+
 static int eth_bind_config(struct usb_configuration *c, u8 ethaddr[ETH_ALEN])
 {
 	return can_support_ecm(c->cdev->gadget)
 		? ecm_bind_config(c, ethaddr)
 		: geth_bind_config(c, ethaddr);
 }
+
 #endif
-- 
1.7.1

[PATCHv6 7/7] USB: gadget: g_multi: moved strings handling code to composite

[Michal Nazarewicz]

This patch removes some of the string registration from the
Multifunction Composite Gadget as composite layer can handle
the iManufacturer and iProduct for us.

This also adds the "needs_serial" so that composite layer will
issue a warning if user space fails to provide the iSerialNumber
module parameter.

Signed-off-by: Michal Nazarewicz <m.nazarewicz@samsung.com>
Signed-off-by: Kyungmin Park <kyungmin.park@samsung.com>
---
 drivers/usb/gadget/multi.c |   19 +++----------------
 1 files changed, 3 insertions(+), 16 deletions(-)

diff --git a/drivers/usb/gadget/multi.c b/drivers/usb/gadget/multi.c
index 36d67a3..ca51661 100644
--- a/drivers/usb/gadget/multi.c
+++ b/drivers/usb/gadget/multi.c
@@ -121,8 +121,6 @@ static const struct usb_descriptor_header *otg_desc[] = {
 
 
 enum {
-	MULTI_STRING_MANUFACTURER_IDX,
-	MULTI_STRING_PRODUCT_IDX,
 #ifdef CONFIG_USB_G_MULTI_RNDIS
 	MULTI_STRING_RNDIS_CONFIG_IDX,
 #endif
@@ -131,11 +129,7 @@ enum {
 #endif
 };
 
-static char manufacturer[50];
-
 static struct usb_string strings_dev[] = {
-	[MULTI_STRING_MANUFACTURER_IDX].s = manufacturer,
-	[MULTI_STRING_PRODUCT_IDX].s      = DRIVER_DESC,
 #ifdef CONFIG_USB_G_MULTI_RNDIS
 	[MULTI_STRING_RNDIS_CONFIG_IDX].s = "Multifunction with RNDIS",
 #endif
@@ -314,20 +308,11 @@ static int __ref multi_bind(struct usb_composite_dev *cdev)
 		device_desc.bcdDevice = cpu_to_le16(0x0300 | 0x0099);
 	}
 
-	/* allocate string descriptor numbers */
-	snprintf(manufacturer, sizeof manufacturer, "%s %s with %s",
-	         init_utsname()->sysname, init_utsname()->release,
-	         gadget->name);
-
+	/* allocate string IDs */
 	status = usb_string_ids_tab(cdev, strings_dev);
 	if (unlikely(status < 0))
 		goto fail2;
 
-	device_desc.iManufacturer =
-		strings_dev[MULTI_STRING_MANUFACTURER_IDX].id;
-	device_desc.iProduct      =
-		strings_dev[MULTI_STRING_PRODUCT_IDX].id;
-
 	/* register configurations */
 	status = rndis_config_register(cdev);
 	if (unlikely(status < 0))
@@ -370,6 +355,8 @@ static struct usb_composite_driver multi_driver = {
 	.strings	= dev_strings,
 	.bind		= multi_bind,
 	.unbind		= __exit_p(multi_unbind),
+	.iProduct	= DRIVER_DESC,
+	.needs_serial	= 1,
 };
 
 
-- 
1.7.1

Re: [PATCHv6 4/7] USB: gadget: composite: Better string override handling

[David Brownell]

OK, I guess.  This version does not have the
hard-wired serial number some previous versions had.

However, I can't put the time in for a real review
and test session, so I'll have to hope this doesn't
have any subtle bugs such review would uncover.

- Dave

Re: [PATCHv6 4/7] USB: gadget: composite: Better string override handling

[Michał Nazarewicz]

On Thu, 05 Aug 2010 22:58:06 +0200, David Brownell <david-b@pacbell.net> wrote:

> OK, I guess.  This version does not have the
> hard-wired serial number some previous versions had.

Great! Thanks!  At least it seems my patch makes sense now. :)

-- 
Best regards,                                        _     _
| Humble Liege of Serenely Enlightened Majesty of  o' \,=./ `o
| Computer Science,  Michał "mina86" Nazarewicz       (o o)
+----[mina86*mina86.com]---[mina86*jabber.org]----ooO--(_)--Ooo--

Re: [PATCHv6 1/7] USB: gadget: storage_common: fixed warning building mass storage function

[Andy Shevchenko]

On Thu, Aug 5, 2010 at 1:27 PM, Michal Nazarewicz
<m.nazarewicz@samsung.com> wrote:
> The "nofua" attribute is used in the File-Storage Gadget but the
> functions handling it are defined in storage_common.c (which may be
> not a bad thing if anyone will want to port tho "nofua" attribute to
> mass storage function) which causes the following warnings:
>
> drivers/usb/gadget/storage_common.c:718: \
>  warning: ‘fsg_show_nofua’ defined but not used
> drivers/usb/gadget/storage_common.c:782: \
>  warning: ‘fsg_store_nofua’ defined but not used
>
> Adding __maybe_unused fixes this issue.
>
> Signed-off-by: Michal Nazarewicz <m.nazarewicz@samsung.com>
> Cc: Andy Shevchenko <andy.shevchenko@gmail.com>
> ---
>  drivers/usb/gadget/storage_common.c |   10 +++++-----
>  1 files changed, 5 insertions(+), 5 deletions(-)
>
> diff --git a/drivers/usb/gadget/storage_common.c b/drivers/usb/gadget/storage_common.c
> index 484acfb..3379cc3 100644
> --- a/drivers/usb/gadget/storage_common.c
> +++ b/drivers/usb/gadget/storage_common.c
> @@ -715,8 +715,8 @@ static ssize_t fsg_show_ro(struct device *dev, struct device_attribute *attr,
>                                  : curlun->initially_ro);
>  }
>
> -static ssize_t fsg_show_nofua(struct device *dev, struct device_attribute *attr,
> -                             char *buf)
> +static ssize_t __maybe_unused
> +fsg_show_nofua(struct device *dev, struct device_attribute *attr, char *buf)
>  {
>        struct fsg_lun  *curlun = fsg_lun_from_dev(dev);
>
> @@ -779,9 +779,9 @@ static ssize_t fsg_store_ro(struct device *dev, struct device_attribute *attr,
>        return rc;
>  }
>
> -static ssize_t fsg_store_nofua(struct device *dev,
> -                              struct device_attribute *attr,
> -                              const char *buf, size_t count)
> +static ssize_t __maybe_unused
> +fsg_store_nofua(struct device *dev, struct device_attribute *attr,
> +               const char *buf, size_t count)
>  {
>        struct fsg_lun  *curlun = fsg_lun_from_dev(dev);
>        unsigned long   nofua;
> --
> 1.7.1
>
>

Acked-by: Andy Shevchenko <andy.shevchenko@gmail.com>

-- 
With Best Regards,
Andy Shevchenko