From mboxrd@z Thu Jan  1 00:00:00 1970
From: Lukasz Majewski <l.majewski@samsung.com>
Date: Thu, 02 Aug 2012 16:47:02 +0200
Subject: [U-Boot] [PATCH v3 4/7] dfu: MMC specific routines for DFU
	operation
In-Reply-To: <201208011900.13973.vapier@gentoo.org>
References: <1341308291-14663-1-git-send-email-l.majewski@samsung.com>
	<1343716623-8943-1-git-send-email-l.majewski@samsung.com>
	<1343716623-8943-5-git-send-email-l.majewski@samsung.com>
	<201208011900.13973.vapier@gentoo.org>
Message-ID: <20120802164702.4488a0e7@amdc308.digital.local>
List-Id: <u-boot.lists.denx.de>
MIME-Version: 1.0
Content-Type: text/plain; charset="us-ascii"
Content-Transfer-Encoding: 7bit
To: u-boot@lists.denx.de

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