| Message ID | 1341308291-14663-5-git-send-email-l.majewski@samsung.com |
|---|---|
| State | Changes Requested |
| Headers | show |
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 ;-) [...] Best regards, Marek Vasut
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.
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? Best regards, Marek Vasut
On Wed, Jul 04, 2012 at 12:01:27AM +0200, Marek Vasut wrote: > 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? Assuming that most of the logic in do_fat_fswrite is needed, no. And I think a good portion of it is, at first glance at least.
Dear Tom Rini, [...] > > > > > + 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? > > Assuming that most of the logic in do_fat_fswrite is needed, no. And I > think a good portion of it is, at first glance at least. Abstracting it out into a function won't cut it? Best regards, Marek Vasut
On Wed, Jul 04, 2012 at 12:31:14AM +0200, Marek Vasut wrote: > Dear Tom Rini, > > [...] > > > > > > > + 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? > > > > Assuming that most of the logic in do_fat_fswrite is needed, no. And I > > think a good portion of it is, at first glance at least. > > Abstracting it out into a function won't cut it? My inclination would be to say that seems a bit sillier than just using run_command(...) to call the existing command.
On 07/03/2012 04:33 PM, Tom Rini wrote: > On Wed, Jul 04, 2012 at 12:31:14AM +0200, Marek Vasut wrote: >> Dear Tom Rini, >> >> [...] >> >>>>>>> + 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? >>> >>> Assuming that most of the logic in do_fat_fswrite is needed, no. And I >>> think a good portion of it is, at first glance at least. >> >> Abstracting it out into a function won't cut it? > > My inclination would be to say that seems a bit sillier than just using > run_command(...) to call the existing command. Abstracting into a function definitely means the compiler will be able to type-check all the arguments to the function. Is the same true using run_command; I assume everything gets serialized to an array of strings and hence any validation is deferred until run-time then?
On Tue, Jul 03, 2012 at 05:07:04PM -0600, Stephen Warren wrote: > On 07/03/2012 04:33 PM, Tom Rini wrote: > > On Wed, Jul 04, 2012 at 12:31:14AM +0200, Marek Vasut wrote: > >> Dear Tom Rini, > >> > >> [...] > >> > >>>>>>> + 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? > >>> > >>> Assuming that most of the logic in do_fat_fswrite is needed, no. And I > >>> think a good portion of it is, at first glance at least. > >> > >> Abstracting it out into a function won't cut it? > > > > My inclination would be to say that seems a bit sillier than just using > > run_command(...) to call the existing command. > > Abstracting into a function definitely means the compiler will be able > to type-check all the arguments to the function. Is the same true using > run_command; I assume everything gets serialized to an array of strings > and hence any validation is deferred until run-time then? string is constructed with sprintf and that gives us some type checking. Only cast today is buf to an unsigned int. And the information either comes from tools that should/must have done sanity checking or from the 'dfu' command which again should (didn't verify that patch yet) have done sanity checking to get us this far.
On 07/03/2012 05:38 PM, Tom Rini wrote: > On Tue, Jul 03, 2012 at 05:07:04PM -0600, Stephen Warren wrote: >> On 07/03/2012 04:33 PM, Tom Rini wrote: >>> On Wed, Jul 04, 2012 at 12:31:14AM +0200, Marek Vasut wrote: >>>> Dear Tom Rini, >>>> >>>> [...] >>>> >>>>>>>>> + 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? >>>>> >>>>> Assuming that most of the logic in do_fat_fswrite is needed, no. And I >>>>> think a good portion of it is, at first glance at least. >>>> >>>> Abstracting it out into a function won't cut it? >>> >>> My inclination would be to say that seems a bit sillier than just using >>> run_command(...) to call the existing command. >> >> Abstracting into a function definitely means the compiler will be able >> to type-check all the arguments to the function. Is the same true using >> run_command; I assume everything gets serialized to an array of strings >> and hence any validation is deferred until run-time then? > > string is constructed with sprintf and that gives us some type checking. > Only cast today is buf to an unsigned int. And the information either > comes from tools that should/must have done sanity checking or from the > 'dfu' command which again should (didn't verify that patch yet) have > done sanity checking to get us this far. So that checks the parameters against the sprintf format string, but what checks that you chose the right format string for the command?
Dear Tom Rini, > On Wed, Jul 04, 2012 at 12:31:14AM +0200, Marek Vasut wrote: > > Dear Tom Rini, > > > > [...] > > > > > > > > > + 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? > > > > > > Assuming that most of the logic in do_fat_fswrite is needed, no. And I > > > think a good portion of it is, at first glance at least. > > > > Abstracting it out into a function won't cut it? > > My inclination would be to say that seems a bit sillier than just using > run_command(...) to call the existing command. Whose's syntax will likely soon change, breaking all this :-( Best regards, Marek Vasut
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.
Dear Lukasz Majewski, [...] > > > > 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. I'm afraid it might change with the driver model soon. > 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? But as Stephen pointed out, the type checking is much better when used as function. > Side note: DFU uses only EP0 (for transfer and configuration), so this > is rather slow communication link. I see > I'm open for discussion. Yes please, I think I started some bad flamewar in here :/ Best regards, Marek Vasut
Hi Marek, > Dear Lukasz Majewski, > > [...] > > > > > > 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. > > I'm afraid it might change with the driver model soon. You have probably more information than I about the driver model :-) Since I know u-boot this API was stable. If it changes, I will adjust the sprintf :-) > > > 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? > > But as Stephen pointed out, the type checking is much better when > used as function. Yes, I agree about the type check. Contrary, the cmd_mmc.c code is not checking the correctness of passed data. It performs strncmp, then simple_strtoul and with this parameter calls mmc->block_dev.block_read(). For this command such behavior is acceptable. > > > Side note: DFU uses only EP0 (for transfer and configuration), so > > this is rather slow communication link. > > I see > > > I'm open for discussion. > > Yes please, I think I started some bad flamewar in here :/ Maybe we come up with a better solution thanks to that :-).
On Tuesday 03 July 2012 20:13:37 Marek Vasut wrote: > Dear Tom Rini, > > On Wed, Jul 04, 2012 at 12:31:14AM +0200, Marek Vasut wrote: > > > Dear Tom Rini, > > > > > > > > + 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? > > > > > > > > Assuming that most of the logic in do_fat_fswrite is needed, no. And > > > > I think a good portion of it is, at first glance at least. > > > > > > Abstracting it out into a function won't cut it? > > > > My inclination would be to say that seems a bit sillier than just using > > run_command(...) to call the existing command. > > Whose's syntax will likely soon change, breaking all this :-( and would be pretty much impossible to detect at compile time. extending the API for code to call is the right answer ... abusing the shell at runtime from other code should be an absolute last resort. -mike
diff --git a/drivers/dfu/Makefile b/drivers/dfu/Makefile index 7736485..7b717bc 100644 --- a/drivers/dfu/Makefile +++ b/drivers/dfu/Makefile @@ -26,6 +26,7 @@ include $(TOPDIR)/config.mk LIB = $(obj)libdfu.o COBJS-$(CONFIG_DFU_FUNCTION) += dfu.o +COBJS-$(CONFIG_DFU_MMC) += dfu_mmc.o SRCS := $(COBJS-y:.o=.c) OBJS := $(addprefix $(obj),$(COBJS-y)) diff --git a/drivers/dfu/dfu_mmc.c b/drivers/dfu/dfu_mmc.c new file mode 100644 index 0000000..3151fbc --- /dev/null +++ b/drivers/dfu/dfu_mmc.c @@ -0,0 +1,126 @@ +/* + * dfu.c -- DFU back-end routines + * + * Copyright (C) 2012 Samsung Electronics + * authors: Andrzej Pietrasiewicz <andrzej.p@samsung.com> + * Lukasz Majewski <l.majewski@samsung.com> + * + * This program is free software; you can redistribute it and/or modify + * it under the terms of the GNU General Public License as published by + * the Free Software Foundation; either version 2 of the License, or + * (at your option) any later version. + * + * This program is distributed in the hope that it will be useful, + * but WITHOUT ANY WARRANTY; without even the implied warranty of + * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the + * GNU General Public License for more details. + * + * You should have received a copy of the GNU General Public License + * along with this program; if not, write to the Free Software + * Foundation, Inc., 59 Temple Place, Suite 330, Boston, MA 02111-1307 USA + */ + +#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); + + return 0; +} + +int dfu_read_medium_mmc(struct dfu_entity *dfu, void *buf, long *len) +{ + ALLOC_CACHE_ALIGN_BUFFER(char, cmd_buf, DFU_CMD_BUF_SIZE); + char *str_env = NULL; + int ret = 0; + + memset(cmd_buf, '\0', sizeof(cmd_buf)); + + switch (dfu->layout) { + case RAW_ADDR: + sprintf(cmd_buf, "mmc read 0x%x %x %x", (unsigned int) buf, + dfu->data.mmc.lba_start, dfu->data.mmc.lba_size); + + *len = dfu->data.mmc.lba_blk_size * dfu->data.mmc.lba_size; + break; + case FAT: + sprintf(cmd_buf, "fatload mmc %d:%d 0x%x %s", + dfu->data.mmc.dev, dfu->data.mmc.part, + (unsigned int) buf, dfu->name); + break; + default: + printf("%s: Wrong layout!\n", __func__); + } + + debug("%s: %s 0x%p\n", __func__, cmd_buf, cmd_buf); + + ret = run_command(cmd_buf, 0); + if (ret) { + puts("dfu: Read error!\n"); + return ret; + } + + if (dfu->layout != RAW_ADDR) { + str_env = getenv("filesize"); + if (str_env == NULL) { + puts("dfu: Wrong file size!\n"); + return -1; + } + + *len = simple_strtoul(str_env, NULL, 16); + } + return ret; +} + +int dfu_fill_entity_mmc(struct dfu_entity *dfu, char* s) +{ + char *st = NULL; + int n = 0; + + dfu->dev_type = MMC; + st = dfu_extract_token(&s, &n); + + if (!strncmp(st, "mmc", n)) { + dfu->layout = RAW_ADDR; + + dfu->data.mmc.lba_start = simple_strtoul(s, &s, 16); + dfu->data.mmc.lba_size = simple_strtoul(++s, &s, 16); + dfu->data.mmc.lba_blk_size = get_mmc_blk_size(dfu->dev_num); + + } else if (!strncmp(st, "fat", n)) { + dfu->layout = FAT; + + dfu->data.mmc.dev = simple_strtoul(s, &s, 10); + dfu->data.mmc.part = simple_strtoul(++s, &s, 10); + + } else { + printf("%s: Wrong memory layout!\n", __func__); + } + + dfu->read_medium = dfu_read_medium_mmc; + dfu->write_medium = dfu_write_medium_mmc; + + return 0; +}