From: Wolfgang Denk <wd@denx.de>
Date: Sun, 09 Sep 2007 00:53:02 +0200
Subject: [U-Boot-Users] [PATCH 1/5] OneNAND support
In-Reply-To: Your message of "Fri, 07 Sep 2007 09:57:23 +0900."
	<20070907005723.GA19727@party>
Message-ID: <20070908225302.CA6D324653@gemini.denx.de>
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

In message <20070907005723.GA19727@party> you wrote:
> [PATCH] OneNAND support
> 
> This patch enables the OneNAND at U-boot

I cannot comment much on this code as I have no access to any hardware
with such a device, but here are a few formal comments:

> +U_BOOT_CMD (onenand, 6, 1, do_onenand,
> +	    "onenand - OneNAND sub-system\n",
> +	    "info   - show available OneNAND devices\n"
> +	    "onenand read[.oob] addr ofs len - read data at ofs with len to addr\n"
> +	    "onenand write addr ofs len - write data at ofs with len from addr\n"
> +	    "onenand erase block start-end - erase block from start to end\n"
> +	    "onenand erase saddr eaddr - erase block start addr to end addr\n"
> +	    "onenand block[.oob] addr block [page] [len] - "
> +	    "read data with (block [, page]) to addr\n"

This line is probably missing some additional indentation?

> +	    "onenand unlock start-end - unlock block from start to end\n"
> +	    "onenand save bootloader addr - save bootloader at addr\n");

I'm not happy with the parameter format of the "erase"  and  "unlock"
commands  -  using  an  address  specification  of "start-end" breaks
existing conventions in U-Boot. Please  use  two  separate  arguments
instead.

Also, I don;t see what  the  difference  between  "erase  block"  and
"erase"  is  -  can  we  avoid  "erase block" all together? If not, I
recommend to  change  it  into  "eraseblock"  (which  could  then  be
shortened to "eraseb") to avoid too different command formats.


After changing this, you have my ACK.

Thanks a lot.

Best regards,

Wolfgang Denk

-- 
DENX Software Engineering GmbH,     MD: Wolfgang Denk & Detlev Zundel
HRB 165235 Munich, Office: Kirchenstr.5, D-82194 Groebenzell, Germany
Phone: (+49)-8142-66989-10 Fax: (+49)-8142-66989-80 Email: wd at denx.de
In the beginning there was nothing.
And the Lord said "Let There Be Light!"
And still there was nothing, but at least now you could see it.