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

[Lukasz Majewski]

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 :-).


-- 
Best regards,

Lukasz Majewski

Samsung Poland R&D Center | Linux Platform Group