Re: [PATCH 1/3] usb: gadget: iSerialNumber for FunctionFS

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

 



On Wed, 18 Jan 2012 12:31:37 +0100, Andrzej Pietrasiewicz <andrzej.p@xxxxxxxxxxx> wrote:
iSerialNumber as a module parameter

Signed-off-by: Andrzej Pietrasiewicz <andrzej.p@xxxxxxxxxxx>
Signed-off-by: Kyungmin Park <kyungmin.park@xxxxxxxxxxx>

What version is it based on?  It does not seem to apply on top of 3.2, and
in fact is completely redundant since composite.c already includes iSerialNumber
module parameter (along with idVendor and idProduct which are not present in
g_ffs.c from 3.2).

---
 drivers/usb/gadget/g_ffs.c |   16 ++++++++++++++--
 1 files changed, 14 insertions(+), 2 deletions(-)

diff --git a/drivers/usb/gadget/g_ffs.c b/drivers/usb/gadget/g_ffs.c
index 3c2f0a4..1b337bc 100644
--- a/drivers/usb/gadget/g_ffs.c
+++ b/drivers/usb/gadget/g_ffs.c
@@ -55,6 +55,7 @@ MODULE_LICENSE("GPL");
static unsigned short gfs_vendor_id    = 0x1d6b;	/* Linux Foundation */
 static unsigned short gfs_product_id   = 0x0105;	/* FunctionFS Gadget */
+static char *gfs_serial_p;
static struct usb_device_descriptor gfs_dev_desc = {
 	.bLength		= sizeof gfs_dev_desc,
@@ -69,7 +70,7 @@ static struct usb_device_descriptor gfs_dev_desc = {
 	/* .bcdDevice		= f(hardware) */
 	/* .iManufacturer	= DYNAMIC */
 	/* .iProduct		= DYNAMIC */
-	/* NO SERIAL NUMBER */
+	/* .iSerialNumber	= DYNAMIC */
 	.bNumConfigurations	= 1,
 };
@@ -86,6 +87,8 @@ 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(usb_serial,   gfs_serial_p,                 charp,  0644);
+GFS_MODULE_PARAM_DESC(usb_serial, iSerialNumber);
@@ -108,16 +111,21 @@ static const struct usb_descriptor_header *gfs_otg_desc[] = {
 enum {
 	GFS_STRING_MANUFACTURER_IDX,
 	GFS_STRING_PRODUCT_IDX,
+	GFS_STRING_SERIAL_IDX,
 	GFS_STRING_FIRST_CONFIG_IDX,
 };
-static       char gfs_manufacturer[50];
+#define STRING_LEN 50
+
+static       char gfs_manufacturer[STRING_LEN];
+static       char gfs_serial[STRING_LEN] = {'\0'};

I'm fairly certain that the initialisation is redundant.
Also "" would IMO look more natural.

 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,
+	[GFS_STRING_SERIAL_IDX].s = gfs_serial,
 #ifdef CONFIG_USB_FUNCTIONFS_RNDIS
 	{ .s = "FunctionFS + RNDIS" },
 #endif
@@ -251,6 +259,9 @@ static int gfs_bind(struct usb_composite_dev *cdev)
 	snprintf(gfs_manufacturer, sizeof gfs_manufacturer, "%s %s with %s",
 		 init_utsname()->sysname, init_utsname()->release,
 		 cdev->gadget->name);
+	if (gfs_serial_p)
+		strncpy(gfs_serial, gfs_serial_p, STRING_LEN);
+	gfs_serial[STRING_LEN - 1] = '\0';

My taste would be to use sizeof(gfs_serial) rather then macro (it would also
match the rest of the code).

Also, this copying is not needed at all, since you can just do
“gfs_strings[GFS_STRING_SERIAL_IDX].s = gfs_serial_p;” and get rid of the
“gfs_serial” array all-together (gfs_strings[GFS_STRING_SERIAL_IDX].s may
be initialised to a pointer to a NUL byte).

	ret = usb_string_ids_tab(cdev, gfs_strings);
 	if (unlikely(ret < 0))
@@ -258,6 +269,7 @@ static int gfs_bind(struct usb_composite_dev *cdev)
	gfs_dev_desc.iManufacturer = gfs_strings[GFS_STRING_MANUFACTURER_IDX].id;
 	gfs_dev_desc.iProduct      = gfs_strings[GFS_STRING_PRODUCT_IDX].id;
+	gfs_dev_desc.iSerialNumber = gfs_strings[GFS_STRING_SERIAL_IDX].id;

Wrap it around “if (gfs_serial_p)” as well.  No point in setting it if the string
is empty.

 	ret = functionfs_bind(gfs_ffs_data, cdev);
 	if (unlikely(ret < 0))

--
Best regards,                                         _     _
.o. | Liege of Serenely Enlightened Majesty of      o' \,=./ `o
..o | Computer Science,  Michał “mina86” Nazarewicz    (o o)
ooo +----<email/xmpp: mpn@xxxxxxxxxx>--------------ooO--(_)--Ooo--
--
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