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<h...@denx.de>
> >> Cc: Marek Vasut<marek.va...@gmail.com>
> >> Cc: Lukasz Majewski<l.majew...@samsung.com>
> >> Cc: Kyungmin Park<kyungmin.p...@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
_______________________________________________
U-Boot mailing list
U-Boot@lists.denx.de
http://lists.denx.de/mailman/listinfo/u-boot
