From mboxrd@z Thu Jan  1 00:00:00 1970
From: Lukasz Majewski <l.majewski@samsung.com>
Date: Wed, 04 Jul 2012 17:13:57 +0200
Subject: [U-Boot] [PATCH 4/7] dfu: MMC specific routines for DFU
	operation
In-Reply-To: <201207041638.20184.marex@denx.de>
References: <1341308291-14663-1-git-send-email-l.majewski@samsung.com>
	<201207040001.27997.marex@denx.de>
	<20120704111032.3d3325e7@lmajewski.digital.local>
	<201207041638.20184.marex@denx.de>
Message-ID: <20120704171357.01fce1bd@lmajewski.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

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