[U-Boot-Users] [PATCH 1/5] OneNAND support

Kyungmin Park kmpark at infradead.org
Mon Sep 10 02:48:31 CEST 2007


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

After committing the OneNAND patch, I will post the Apollon board patches which uses the OneNAND.
> 
> > +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?

Yes, I add the tap indentation as nand does

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

At code writing time, there are no strict rules to use address scheme, block scheme.
No problem, which one is used. Now temporally I removed it

Thank you,
Kyungmin Park

P.S. Now I just send the modified parts, after full comments, I will send it at once.

+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 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"
+);





More information about the U-Boot mailing list