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