Re: [PATCH 14/20] usb/gadget: push iSerialNumber into gadgets

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



On 08/25/2012 12:07 AM, Michal Nazarewicz wrote:
--- a/drivers/usb/gadget/acm_ms.c
+++ b/drivers/usb/gadget/acm_ms.c
@@ -92,12 +92,14 @@ static const struct usb_descriptor_header *otg_desc[] = {

  #define STRING_MANUFACTURER_IDX		0
  #define STRING_PRODUCT_IDX		1
+#define STRING_PRODUCT_SERIAL		2

  static char manufacturer[50];

  static struct usb_string strings_dev[] = {
  	[STRING_MANUFACTURER_IDX].s = manufacturer,
  	[STRING_PRODUCT_IDX].s = DRIVER_DESC,
+	[STRING_PRODUCT_SERIAL].s = "",
  	{  } /* end of list */
  };

@@ -205,6 +207,11 @@ static int __init acm_ms_bind(struct usb_composite_dev *cdev)
  		goto fail1;

  	USB_GADGET_COMPOSITE_OVERWRITE_OPTIONS(device_desc);
+	if (iSerialNumber) {
+		strings_dev[STRING_PRODUCT_SERIAL].s = iSerialNumber;
+		device_desc.iSerialNumber =
+			strings_dev[STRING_PRODUCT_SERIAL].id;
+	}

Could we instead have USB_GADGET_COMPOSITE_OVERWRITE_OPTIONS handle
this?

Everything is possible.

This could be achieved by passing strings_dev and STRING_PRODUCT_SERIAL
to the macro.  It could also take care of "" initialisation if module
parameter was not given.

Argh. And for product and manuf you add another two variable to the
macro. And how do you achieve the "" initialization? This macro is
evaluated after the id has been assigned.
What I could live with is to add the string_dev variable to the macro
and enforce all Serial/Manu/Product index to be the same. The idea died once I saw g_ffs. So?

Alternatively, composite could require STRING_PRODUCT_SERIAL to always
be zero.  This way only strings_dev would have to be passed as
additional argument to the macro.

  	dev_info(&gadget->dev, "%s, version: " DRIVER_VERSION "\n",
  			DRIVER_DESC);
  	fsg_common_put(&fsg_common);

diff --git a/drivers/usb/gadget/g_ffs.c b/drivers/usb/gadget/g_ffs.c
index 563feb5..948414d 100644
--- a/drivers/usb/gadget/g_ffs.c
+++ b/drivers/usb/gadget/g_ffs.c
@@ -114,17 +114,33 @@ static const struct usb_descriptor_header *gfs_otg_desc[] = {
  	NULL
  };

+enum {
+#ifdef CONFIG_USB_FUNCTIONFS_RNDIS
+	GFFS_FFS_RNDIS_IDX,
+#endif
+
+#ifdef CONFIG_USB_FUNCTIONFS_ETH
+	GFFS_FFS_ECM_IDX,
+#endif
+
+#ifdef CONFIG_USB_FUNCTIONFS_GENERIC
+	GFFS_FFS_ECM_GEN_IDX,
+#endif
+	GFFS_SERIAL_IDX,
+};
+
  /* String IDs are assigned dynamically */
  static struct usb_string gfs_strings[] = {
  #ifdef CONFIG_USB_FUNCTIONFS_RNDIS
-	{ .s = "FunctionFS + RNDIS" },
+	[GFFS_FFS_RNDIS_IDX].s = "FunctionFS + RNDIS",
  #endif
  #ifdef CONFIG_USB_FUNCTIONFS_ETH
-	{ .s = "FunctionFS + ECM" },
+	[GFFS_FFS_ECM_IDX].s = "FunctionFS + ECM",
  #endif
  #ifdef CONFIG_USB_FUNCTIONFS_GENERIC
-	{ .s = "FunctionFS" },
+	[GFFS_FFS_ECM_IDX].s = "FunctionFS",
  #endif
+	[GFFS_SERIAL_IDX].s = "",
  	{  } /* end of list */
  };

@@ -380,6 +396,11 @@ static int gfs_bind(struct usb_composite_dev *cdev)
  			goto error_unbind;
  	}
  	USB_GADGET_COMPOSITE_OVERWRITE_OPTIONS(gfs_dev_desc);
+	if (iSerialNumber) {
+		gfs_strings[GFFS_SERIAL_IDX].s = iSerialNumber;
+		gfs_dev_desc.iSerialNumber = gfs_strings[GFFS_SERIAL_IDX].id;
+	}
+

Alternatively, you could just define GFFS_SERIAL_IDX as zero and change:

-		c->c.label			= gfs_strings[i].s;
-		c->c.iConfiguration		= gfs_strings[i].id;
+		c->c.label			= gfs_strings[i + 1].s;
+		c->c.iConfiguration		= gfs_strings[i + 1].id;

(In fact, moving ++i from the for loop just above c.label assignment
would make it even easier.)

And why does this look better? I had this but then decided to the enum
definition because it was more obvious. Ideally this should be done
function's bind code. But I did not want to do too much. And you do know, besides serial we add two more members here.

Sebastian
--
To unsubscribe from this list: send the line "unsubscribe linux-usb" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at  http://vger.kernel.org/majordomo-info.html


[Index of Archives]     [Linux Media]     [Linux Input]     [Linux Audio Users]     [Yosemite News]     [Linux Kernel]     [Linux SCSI]     [Old Linux USB Devel Archive]

  Powered by Linux