Hi all,
Marek, did you see the onenand_ipl/onenand_read.c at u-boot? It's already implemented and used for OneNAND boot.
How do you think migrate this file into generic SPL framework?
Thank you, Kyungmin Park
-----Original Message----- From: Marek Vasut [mailto:marek.vasut@gmail.com] Sent: Thursday, November 03, 2011 9:16 AM To: Scott Wood Cc: u-boot@lists.denx.de; Albert ARIBAUD; Kyungmin Park Subject: Re: [PATCH 3/4 V2] OneNAND: Add simple OneNAND SPL
On 11/01/2011 05:54 PM, Marek Vasut wrote:
+static void spl_onenand_get_geometry(struct spl_onenand_data *data) +{
[...]
- /* The page can be either 2k or 4k, avoid using DIV_ROUND_UP. */
- if (data.pagesize == 2048) {
total_pages = len / 2048;page = offset / 2048;total_pages += !!(len & 2047);- } else if (data.pagesize == 4096) {
total_pages = len / 4096;page = offset / 4096;total_pages += !!(len & 4095);- }
What's wrong with DIV_ROUND_UP? It should produce smaller code than what you've done here...
It pulls in aeabi_*div* functions, which won't fit into block 0 of Onenand.
- for (; page <= total_pages; page++) {
block = page / ONENAND_PAGES_PER_BLOCK;rpage = page & (ONENAND_PAGES_PER_BLOCK - 1);ret = spl_onenand_read_page(block, rpage, addr,
data.pagesize);
if (ret) {total_pages++;err |= 1;} elseaddr += data.pagesize / 4;- }
As discussed, please retain the existing block-skipping semantics. And if you do skip a block, that's not an error to be propagated upward (as opposed to something like an uncorrectable ECC error).
If one side of an if statement requires braces, both sides should have them.
diff --git a/include/onenand_uboot.h b/include/onenand_uboot.h index 92279d5..fcb50ff 100644 --- a/include/onenand_uboot.h +++ b/include/onenand_uboot.h @@ -16,6 +16,8 @@
#include <linux/types.h>
+#ifndef CONFIG_SPL_BUILD
Please use a space rather than a tab after #define, #ifndef, etc.
/* Forward declarations */ struct mtd_info; struct mtd_oob_ops;
@@ -52,4 +54,10 @@ extern int flexonenand_set_boundary(struct mtd_info *mtd, int die,
extern void s3c64xx_onenand_init(struct mtd_info *); extern void s3c64xx_set_width_regs(struct onenand_chip *);
+#else
+int spl_onenand_load_image(uint32_t dst, uint32_t offset, uint32_t
len);
+#endif
Why does this need to be ifdeffed at all? We normally don't ifdef header declarations.
-Scott