[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