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

Lukasz Majewski
2012-07-04T09:10:32Z

Hi Marek,

> Dear Tom Rini,
>
> > On Tue, Jul 03, 2012 at 11:29:31PM +0200, Marek Vasut wrote:
> > > Dear Lukasz Majewski,
> > >
> > > > Support for MMC storage devices to work with DFU framework.
> > > >
> > > > Signed-off-by: Lukasz Majewski <l.majewski@samsung.com>
> > > > Signed-off-by: Kyungmin Park <kyungmin.park@samsung.com>
> > > > Cc: Marek Vasut <marex@denx.de>
> > > > ---
> > >
> > > [...]
> > >
> > > > +
> > > > +#include <common.h>
> > > > +#include <malloc.h>
> > > > +#include <dfu.h>
> > > > +
> > > > +int dfu_write_medium_mmc(struct dfu_entity *dfu, void *buf,
> > > > long *len) +{
> > > > + ALLOC_CACHE_ALIGN_BUFFER(char, cmd_buf,
> > > > DFU_CMD_BUF_SIZE); +
> > > > + memset(cmd_buf, '\0', sizeof(cmd_buf));
> > > > +
> > > > + switch (dfu->layout) {
> > > > + case RAW_ADDR:
> > > > + sprintf(cmd_buf, "mmc write 0x%x %x %x",
> > > > (unsigned int) buf,
> > > > + dfu->data.mmc.lba_start,
> > > > dfu->data.mmc.lba_size);
> > > > + break;
> > > > + case FAT:
> > > > + sprintf(cmd_buf, "fatwrite mmc %d:%d 0x%x %s
> > > > %lx",
> > > > + dfu->data.mmc.dev, dfu->data.mmc.part,
> > > > + (unsigned int) buf, dfu->name, *len);
> > > > + break;
> > > > + default:
> > > > + printf("%s: Wrong layout!\n", __func__);
> > > > + }
> > > > +
> > > > + debug("%s: %s 0x%p\n", __func__, cmd_buf, cmd_buf);
> > > > + run_command(cmd_buf, 0);
> > >
> > > Holy Moly ... can we not make this into simple calls to those
> > > subsystems ? Instead invoking command is crazy ;-)
> >
> > Are they really simple? There's a few other places we do this, and
> > so long as it's documented that DFU depends on CONFIG_FAT_WRITE for
> > writing to fat and so forth.
>
> Well ain't it easier to call fat_write() or similar?
>
I've decided to use run_command on a purpose.

This call provides clean and reliable API. It is very unlikely that the
mmc write <dev> <addr> <start> <size> command will change (or any
other).
On the other hand the fields of struct mmc are changed from time to
time.

Moreover, mmc drivers are also a subject to change (like adding dw_mmc
recently).
Using run_command also takes the burden of mmc_init() related calls.

Of course the run_command's downside is the speed of execution. But is
it so important when one considers, the firmware update?

Side note: DFU uses only EP0 (for transfer and configuration), so this
is rather slow communication link.

I'm open for discussion.

--
Best regards,

Lukasz Majewski

Samsung Poland R&D Center | Linux Platform Group