[U-Boot] [PATCH v1 3/5] usb, g_dnl: make iSerialNumber board configurable

[Lukasz Majewski]

Hi Heiko,

> Hello Lukasz,
> 
> Am 22.10.2013 14:37, schrieb Lukasz Majewski:
> > Hi Heiko,
> >
> >> add the possibility to set the iSerialNumber board specific.
> >> Therefore the CONFIG_G_DNL_SERIAL_STRING is introduced, which
> >> defines the maximum length of the iSerialNumber string.
> >> The new function g_dnl_set_serialnumber() must called from
> >> g_dnl_bind_fixup(), to setup the iSerialNumber string.
> >>
> >> Signed-off-by: Heiko Schocher<hs at denx.de>
> >> Cc: Marek Vasut<marek.vasut at gmail.com>
> >> Cc: Lukasz Majewski<l.majewski at samsung.com>
> >> Cc: Kyungmin Park<kyungmin.park at samsung.com>
> >> ---
> >>   drivers/usb/gadget/g_dnl.c | 28 ++++++++++++++++++++++++++++
> >>   include/g_dnl.h            |  3 +++
> >>   2 files changed, 31 insertions(+)
> >>
> >> diff --git a/drivers/usb/gadget/g_dnl.c
> >> b/drivers/usb/gadget/g_dnl.c index 40868c0..5f09d66 100644
> >> --- a/drivers/usb/gadget/g_dnl.c
> >> +++ b/drivers/usb/gadget/g_dnl.c
> >> @@ -39,8 +39,21 @@
> >>
> >>   static const char shortname[] = "usb_dnl_";
> >>   static const char product[] = "USB download gadget";
> >> +#if defined(CONFIG_G_DNL_SERIAL_STRING)
> >
> > I don't like the #if defined preprocessor directives here. The
> > g_dnl.c code is a generic code, to "glue" all functions together.
> >
> >> +#define STRING_SERIAL  3
> >> +static char g_dnl_serial[CONFIG_G_DNL_SERIAL_STRING + 1];
> >> +#endif
> >>   static const char manufacturer[] = CONFIG_G_DNL_MANUFACTURER;
> >>
> >> +#if defined(CONFIG_G_DNL_SERIAL_STRING)
> >> +void g_dnl_set_serialnumber(char *s)
> >> +{
> >> +	memset(g_dnl_serial, 0, CONFIG_G_DNL_SERIAL_STRING + 1);
> >> +	if (strlen(s)<= CONFIG_G_DNL_SERIAL_STRING)
> >> +		strncpy(g_dnl_serial, s, strlen(s));
> >> +}
> >> +#endif
> >> +
> >>   static struct usb_device_descriptor device_desc = {
> >>   	.bLength = sizeof device_desc,
> >>   	.bDescriptorType = USB_DT_DEVICE,
> >> @@ -52,6 +65,9 @@ static struct usb_device_descriptor device_desc
> >> = { .idVendor = __constant_cpu_to_le16(CONFIG_G_DNL_VENDOR_NUM),
> >>   	.idProduct =
> >> __constant_cpu_to_le16(CONFIG_G_DNL_PRODUCT_NUM), .iProduct =
> >> STRING_PRODUCT, +#if defined(CONFIG_G_DNL_SERIAL_STRING)
> >
> > I think that #if defined(CONFIG_G_DNL_SERIAL_STRING) can be easily
> > removed, since the iSerialNumber is a valid member of struct
> > usb_descriptor.
> >
> > In my opinion, instead of defining #ifdefs, we can define some
> > "default" iSerialNumber for all devices.
> 
> Ok! How would look like the default serial number?

Some devices have default serial number set to 0 or 0xFFFFFFFF. Maybe we
can start with the 0 value?

> 
> > Then this value can be overridden by board when needed by call to
> > g_dnl_set_serialnumber() function
> 
> Ok. Will change it in this direction, thanks for the review!

No problem. You are welcome.

> 
> bye,
> Heiko

-- 
Best regards,

Lukasz Majewski

Samsung R&D Institute Poland (SRPOL) | Linux Platform Group