public inbox for u-boot@lists.denx.de
 help / color / mirror / Atom feed
From: Scott Wood <scottwood@freescale.com>
To: u-boot@lists.denx.de
Subject: [U-Boot] [U-Boot, 3/9] nand: add a hw specific subcommand to the nand command.
Date: Thu, 4 Aug 2011 14:42:35 -0500	[thread overview]
Message-ID: <4E3AF62B.8080605@freescale.com> (raw)
In-Reply-To: <4E3A30D7.3020104@denx.de>

On 08/04/2011 12:40 AM, Heiko Schocher wrote:
> Scott Wood wrote:
>> 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) ...

The question was rhetorical, as the next sentence indicates -- unless 
you had a different answer, of course.

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

The "hwfunc" doesn't really tell the user anything, it's an internal 
implementation detail of where in the code the command is implemented. 
It's verbosity without benefit.

> (I don;t know, if we get really more then one hwfunc...)

You later say a plain "nand hwfunc" would print a help message, so that 
plus one real command makes two...

>, 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 ...

More like one thing to grep for, and you'll still need to limit the 
files grepped so you don't get irrelevant results from other boards or 
controllers.  Why not just search those files for U_BOOT_CMD?

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

"actual state" of what?  We haven't gotten to the point of actually 
defining a command yet.

> nand hwfunc [rbl/uboot]
>           switch between rbl/uboot nand read/write functions
>
> cam_enc_4xx>  nand hwfunc
> uboot

Again, I think "nand hwfunc" conveys little information about what it's 
actually going to do, versus something like:

=> nandrbl
off

At least "nand hwfunc type" or "nand hwfunc rbl" would be more 
informative, but I question the benefit that the verbosity buys us.

Why do you need alternative implementations of read/write functions, BTW?

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

So now it's "nand hwfunc ctrl type rbl"?  Just to avoid grepping for 
U_BOOT_CMD?

Or by "one place to look" are you talking about as a user in the help 
output?  If you insist on the command appearing as a proper "nand" 
subcommand, how about dropping "hwfunc" and letting other pieces of code 
register on a chain of handlers?  Anything that isn't recognized gets 
passed on to the next link in the chain.

-Scott

  reply	other threads:[~2011-08-04 19:42 UTC|newest]

Thread overview: 24+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2011-07-16 10:06 [U-Boot] [PATCH 0/9] arm, davinci: add support for dm368 based cam_enc_4xx board Heiko Schocher
2011-07-16 10:06 ` [U-Boot] [PATCH 1/9] mkimage: add UBL header support for booting davinci cpus Heiko Schocher
2011-07-28 14:52   ` Wolfgang Denk
2011-07-16 10:06 ` [U-Boot] [PATCH 2/9] arm, arm926: fix missing symbols in NAND_SPL mode Heiko Schocher
2011-07-16 10:06 ` [U-Boot] [PATCH 3/9] nand: add a hw specific subcommand to the nand command Heiko Schocher
2011-08-02 20:19   ` [U-Boot] [U-Boot, " Scott Wood
2011-08-03  5:14     ` Heiko Schocher
2011-08-03 15:56       ` Scott Wood
2011-08-04  5:40         ` Heiko Schocher
2011-08-04 19:42           ` Scott Wood [this message]
2011-08-04 20:14             ` Wolfgang Denk
2011-08-04 20:28               ` Scott Wood
2011-08-04 20:46                 ` Wolfgang Denk
2011-07-16 10:06 ` [U-Boot] [PATCH 4/9] arm, davinci: add header files for dm365 Heiko Schocher
2011-10-05 14:28   ` [U-Boot] [PATCH v2 06/10] " Heiko Schocher
2011-07-16 10:06 ` [U-Boot] [PATCH 5/9] arm, davinci: add lowlevel function for dm365 soc Heiko Schocher
2011-07-16 10:06 ` [U-Boot] [PATCH 6/9] arm926ejs, davinci: add cpuinfo for dm365 Heiko Schocher
2011-10-05 14:29   ` [U-Boot] [PATCH v2 07/10] arm, davinci: add lowlevel function for dm365 soc Heiko Schocher
2011-10-05 14:29   ` [U-Boot] [PATCH v2 08/10] arm926ejs, davinci: add cpuinfo for dm365 Heiko Schocher
2011-07-16 10:06 ` [U-Boot] [PATCH 7/9] arm926ejs, davinci: add missing spi defines " Heiko Schocher
2011-07-16 10:06 ` [U-Boot] [PATCH 8/9] spl, nand: add 4bit HW ecc oob first nand_read_page function Heiko Schocher
2011-08-02 20:49   ` [U-Boot] [U-Boot, " Scott Wood
2011-07-16 10:06 ` [U-Boot] [PATCH 9/9] arm, davinci: add cam_enc_4xx support Heiko Schocher
2011-10-05 14:30   ` [U-Boot] [PATCH v2 10/10] " Heiko Schocher

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=4E3AF62B.8080605@freescale.com \
    --to=scottwood@freescale.com \
    --cc=u-boot@lists.denx.de \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox