[U-Boot] [PATCH v3 1/7] dfu:usb: Support for g_dnl composite download gadget.

[Lukasz Majewski <l.majewski@samsung.com>]

Hi Mike,

> On Tuesday 31 July 2012 02:36:57 Lukasz Majewski wrote:
> > --- /dev/null
> > +++ b/drivers/usb/gadget/g_dnl.c
> >
> > +static const struct usb_descriptor_header *otg_desc[] = {
> > +	(struct usb_descriptor_header *) &(struct
> > usb_otg_descriptor){
> 
> since you're just casting away things, you could just use (void*) to
> make it a bit more compact
> 
> also, can this be:
> static const struct usb_descriptor_header * const otg_desc[]
> (notice the 2nd const)
> 

After looking into the code I can say that otg_support is not needed
(at least for this gadget) in u-boot. Will be removed.

> > +static struct usb_gadget_strings *g_dnl_composite_strings[] = {
> 
> here too:
> static struct usb_gadget_strings * const g_dnl_composite_strings[] = {

This const is anyway discarded by compiler (GCC 4.6.1) when .string
field of struct usb_composite_driver is initialized. 

> 
> > +static int g_dnl_do_config(struct usb_configuration *c)
> > +{
> > +	int ret = -1;
> > +	char *s = (char *) c->cdev->driver->name;
> 
> pretty sure this should be:
> 	const char *s = c->cdev->driver->name;

Yes, you are right. Corrected.
> 
> > +static int g_dnl_bind(struct usb_composite_dev *cdev)
> > +{
> > +	int			gcnum;
> > +	int id, ret;
> > +	struct usb_gadget	*gadget = cdev->gadget;
> 
> can we stick to a single space for indentation between the type and
> the variable name please ?

Corrected.
> 
> > +int g_dnl_register(const char *type)
> > +{
> > +	int ret;
> > +	/* We only allow "dfu" atm, so 3 should be enough */
> > +	static char name[sizeof(shortname) + 3];
> > +
> > +	if (!strcmp(type, "dfu")) {
> > +		strcpy(name, shortname);
> > +		strcat(name, type);
> 
> if u-boot had stpcpy(), we could do:
> 	strcpy(stpcpy(name, shortname), type);
> 
> > --- /dev/null
> > +++ b/include/g_dnl.h
> >
> > +/* USB initialization declaration - board specific*/
> 
> needs a space before that "*/"

Corrected.

> -mike



-- 
Best regards,

Lukasz Majewski

Samsung Poland R&D Center | Linux Platform Group