[U-Boot] [U-Boot, 3/9] nand: add a hw specific subcommand to the nand command.

[Heiko Schocher]

Hello Scott,

Scott Wood wrote:
> On Wed, 3 Aug 2011 07:14:51 +0200
> Heiko Schocher <hs at denx.de> wrote:
> 
>> Hello Scott,
>>
>> Scott Wood wrote:
>>> Sorry for the delay, didn't see this the first time around -- just
>>> noticed it in patchwork.
>> No problem. Thanks for the review!
>>
>>> On Sat, Jul 16, 2011 at 12:06:44AM -0000, Heiko Schocher wrote:
>>>> @@ -739,6 +743,10 @@ U_BOOT_CMD(
>>>>  	"nand env.oob set off|partition - set enviromnent offset\n"
>>>>  	"nand env.oob get - get environment offset"
>>>>  #endif
>>>> +#ifdef CONFIG_CMD_NAND_HWFUNC
>>>> +	"\n"
>>>> +	"nand hwfunc " CONFIG_CMD_NAND_HWFUNC "\n"
>>>> +#endif
>>> Why not just define hardware-specific commands at whatever scope they
>>> makes sense (board code, controller driver, etc.), rather than add a
>>> generic hook here?
>> Hmm... because if I define a hw specific command for example at
>> soc scope, it is not a nand subcommand ... and I like to have all
>> nand commands (I use it for switching between different read/write
>> functions) accessable under "nand ..." from the u-boot shell ...
>> I don;t like to have all over the code randomised some nand commands ...
> 
> What if there are multiple such commands?  You'd end up with "nand hwfunc

Then we would have a (as hwfunc is a nand subcommand) hwfunc
subcommand(s) ...

> foo <args>", which is getting a bit unwieldy.  Having only one might

Why? (I don;t know, if we get really more then one hwfunc...), but
if we define all over the code some (board, soc, arch,...) specific
nand commands, we lose track ... and with the "nand hwfunc" we have
one place where we have to look ...

> actually be worse: what does a plain "nand hwfunc" do?

A plain "nand hwfunc" should print the help message (if we have more
subcommands under "nand hwfunc"), if we have only one, it returns
as usual in uboot, actual state ... for example on the cam_enc_4xx board:

cam_enc_4xx> nand
nand - NAND sub-system

Usage:
nand info - show available NAND devices
nand device [dev] - show or set current device
nand read - addr off|partition size
nand write - addr off|partition size
    read/write 'size' bytes starting at offset 'off'
    to/from memory address 'addr', skipping bad blocks.
nand erase[.spread] [clean] off size - erase 'size' bytes from offset 'off'
    With '.spread', erase enough for given file size, otherwise,
    'size' includes skipped bad blocks.
nand erase.part [clean] partition - erase entire mtd partition'
nand erase.chip [clean] - erase entire chip'
nand bad - show bad blocks
nand dump[.oob] off - dump page
nand scrub off size | scrub.part partition | scrub.chip
    really clean NAND erasing bad blocks (UNSAFE)
nand markbad off [...] - mark bad block(s) at offset (UNSAFE)
nand biterr off - make a bit error at offset (UNSAFE)
nand hwfunc [rbl/uboot]
         switch between rbl/uboot nand read/write functions

cam_enc_4xx> nand hwfunc
uboot
cam_enc_4xx>

> What if there are multiple commands, but which belong in different places
> (e.g. one defined by the board, one defined by the controller driver)?

Maybe something like this?

nand hwfunc arch ...
nand hwfunc board ...
nand hwfunc ctrl ...
nand hwfunc soc ...
[...]

each hwfunc subcommand added through an define ...

bye,
Heiko
-- 
DENX Software Engineering GmbH,     MD: Wolfgang Denk & Detlev Zundel
HRB 165235 Munich, Office: Kirchenstr.5, D-82194 Groebenzell, Germany