[U-Boot] [PATCH v3 4/7] dfu: MMC specific routines for DFU operation

Lukasz Majewski l.majewski at samsung.com
Thu Aug 2 16:47:02 CEST 2012


Dear Mike Frysinger,

> On Tuesday 31 July 2012 02:37:00 Lukasz Majewski wrote:
> > --- /dev/null
> > +++ b/drivers/dfu/dfu_mmc.c
> >
> > +static int mmc_block_op(enum dfu_mmc_op op, struct dfu_entity *dfu,
> > +			void *buf, long *len)
> > +{
> > +	ALLOC_CACHE_ALIGN_BUFFER(char, cmd_buf, DFU_CMD_BUF_SIZE);
> 
> ugh, what ?  you're passing this string to run_command so there is no
> point in aligning it
Ok, I will change this.

> (not to mention the topic of u-boot code
> internally calling run_command() is seriously wrong.
We are all aware that this approach is a compromise. And this code
will be rewritten when new DM appears.

> 
> > +	memset(cmd_buf, '\0', sizeof(cmd_buf));
> > +
> > +	sprintf(cmd_buf, "mmc %s 0x%x %x %x",
> 
> that memset is pointless.  delete it.

Done.
> 
> > +static int mmc_file_op(enum dfu_mmc_op op, struct dfu_entity *dfu,
> > +			void *buf, long *len)
> > +{
> 
> came comments for this func as above
> 
> > +int dfu_fill_entity_mmc(struct dfu_entity *dfu, char* s)
> 
> "char *s", not "char* s".  please search all your patches for this
> mistake as it seems to have come up a lot.
Hmm, ./tools/checkpatch wasn't complaining about this... 

> -mike



-- 
Best regards,

Lukasz Majewski

Samsung Poland R&D Center | Linux Platform Group


More information about the U-Boot mailing list