[U-Boot] [PATCH 1/4] OneNAND: Move largepage_memorybased

This moves "struct nand_bbt_descr largepage_memorybased" into "onenand_default_bbt" as that's the only place where this is used.
This also removes an entry from .data section. (For me, this section disappears after relocation).
Signed-off-by: Marek Vasut marek.vasut@gmail.com --- drivers/mtd/onenand/onenand_bbt.c | 25 ++++++++++++------------- 1 files changed, 12 insertions(+), 13 deletions(-)
diff --git a/drivers/mtd/onenand/onenand_bbt.c b/drivers/mtd/onenand/onenand_bbt.c index 1354877..c5d3905 100644 --- a/drivers/mtd/onenand/onenand_bbt.c +++ b/drivers/mtd/onenand/onenand_bbt.c @@ -227,19 +227,6 @@ int onenand_scan_bbt(struct mtd_info *mtd, struct nand_bbt_descr *bd) return ret; }
-/* - * Define some generic bad / good block scan pattern which are used - * while scanning a device for factory marked good / bad blocks. - */ -static uint8_t scan_ff_pattern[] = { 0xff, 0xff }; - -static struct nand_bbt_descr largepage_memorybased = { - .options = 0, - .offs = 0, - .len = 2, - .pattern = scan_ff_pattern, -}; - /** * onenand_default_bbt - [OneNAND Interface] Select a default bad block table for the device * @param mtd MTD device structure @@ -252,6 +239,18 @@ int onenand_default_bbt(struct mtd_info *mtd) struct onenand_chip *this = mtd->priv; struct bbm_info *bbm;
+ /* + * Define some generic bad / good block scan pattern which are used + * while scanning a device for factory marked good / bad blocks. + */ + uint8_t scan_ff_pattern[] = { 0xff, 0xff }; + struct nand_bbt_descr largepage_memorybased = { + .options = 0, + .offs = 0, + .len = 2, + .pattern = scan_ff_pattern, + }; + this->bbm = malloc(sizeof(struct bbm_info)); if (!this->bbm) return -ENOMEM;

Signed-off-by: Marek Vasut marek.vasut@gmail.com --- arch/arm/lib/board.c | 6 ++++++ common/cmd_onenand.c | 6 ++++++ 2 files changed, 12 insertions(+), 0 deletions(-)
diff --git a/arch/arm/lib/board.c b/arch/arm/lib/board.c index 5f2dfd0..07995ba 100644 --- a/arch/arm/lib/board.c +++ b/arch/arm/lib/board.c @@ -695,6 +695,9 @@ void board_init_r (gd_t *id, ulong dest_addr) #if defined(CONFIG_CMD_I2C) extern void i2c_reloc(void); #endif +#if defined(CONFIG_CMD_ONENAND) + extern void onenand_reloc(void); +#endif #endif
gd = id; @@ -724,6 +727,9 @@ void board_init_r (gd_t *id, ulong dest_addr) #if defined(CONFIG_CMD_I2C) i2c_reloc(); #endif +#if defined(CONFIG_CMD_ONENAND) + onenand_reloc(); +#endif #endif /* !defined(CONFIG_RELOC_FIXUP_WORKS) */
#ifdef CONFIG_LOGBUFFER diff --git a/common/cmd_onenand.c b/common/cmd_onenand.c index 83d967b..fe84c3b 100644 --- a/common/cmd_onenand.c +++ b/common/cmd_onenand.c @@ -525,6 +525,12 @@ static cmd_tbl_t cmd_onenand_sub[] = { U_BOOT_CMD_MKENT(markbad, CONFIG_SYS_MAXARGS, 0, do_onenand_markbad, "", ""), };
+#ifndef CONFIG_RELOC_FIXUP_WORKS +void onenand_reloc(void) { + fixup_cmdtable(cmd_onenand_sub, ARRAY_SIZE(cmd_onenand_sub)); +} +#endif + static int do_onenand(cmd_tbl_t * cmdtp, int flag, int argc, char * const argv[]) { cmd_tbl_t *c;

Dear Vasut,
On 3 October 2010 02:33, Marek Vasut marek.vasut@gmail.com wrote:
Signed-off-by: Marek Vasut marek.vasut@gmail.com
arch/arm/lib/board.c | 6 ++++++ common/cmd_onenand.c | 6 ++++++ 2 files changed, 12 insertions(+), 0 deletions(-)
diff --git a/arch/arm/lib/board.c b/arch/arm/lib/board.c index 5f2dfd0..07995ba 100644 --- a/arch/arm/lib/board.c +++ b/arch/arm/lib/board.c @@ -695,6 +695,9 @@ void board_init_r (gd_t *id, ulong dest_addr) #if defined(CONFIG_CMD_I2C) extern void i2c_reloc(void); #endif +#if defined(CONFIG_CMD_ONENAND)
- extern void onenand_reloc(void);
+#endif #endif
gd = id; @@ -724,6 +727,9 @@ void board_init_r (gd_t *id, ulong dest_addr) #if defined(CONFIG_CMD_I2C) i2c_reloc(); #endif +#if defined(CONFIG_CMD_ONENAND)
- onenand_reloc();
+#endif #endif /* !defined(CONFIG_RELOC_FIXUP_WORKS) */
#ifdef CONFIG_LOGBUFFER diff --git a/common/cmd_onenand.c b/common/cmd_onenand.c index 83d967b..fe84c3b 100644 --- a/common/cmd_onenand.c +++ b/common/cmd_onenand.c @@ -525,6 +525,12 @@ static cmd_tbl_t cmd_onenand_sub[] = { U_BOOT_CMD_MKENT(markbad, CONFIG_SYS_MAXARGS, 0, do_onenand_markbad, "", ""), };
+#ifndef CONFIG_RELOC_FIXUP_WORKS +void onenand_reloc(void) {
- fixup_cmdtable(cmd_onenand_sub, ARRAY_SIZE(cmd_onenand_sub));
+} +#endif
static int do_onenand(cmd_tbl_t * cmdtp, int flag, int argc, char * const argv[]) { cmd_tbl_t *c; -- 1.7.1
I've prepared same patch. :)
Acked-by: Minkyu Kang mk7.kang@samsung.com
Thanks Minkyu Kang

Dne So 2. října 2010 20:28:19 Minkyu Kang napsal(a):
Dear Vasut,
On 3 October 2010 02:33, Marek Vasut marek.vasut@gmail.com wrote:
Signed-off-by: Marek Vasut marek.vasut@gmail.com
arch/arm/lib/board.c | 6 ++++++ common/cmd_onenand.c | 6 ++++++ 2 files changed, 12 insertions(+), 0 deletions(-)
diff --git a/arch/arm/lib/board.c b/arch/arm/lib/board.c index 5f2dfd0..07995ba 100644 --- a/arch/arm/lib/board.c +++ b/arch/arm/lib/board.c @@ -695,6 +695,9 @@ void board_init_r (gd_t *id, ulong dest_addr) #if defined(CONFIG_CMD_I2C) extern void i2c_reloc(void); #endif +#if defined(CONFIG_CMD_ONENAND)
extern void onenand_reloc(void);
+#endif #endif
gd = id;
@@ -724,6 +727,9 @@ void board_init_r (gd_t *id, ulong dest_addr) #if defined(CONFIG_CMD_I2C) i2c_reloc(); #endif +#if defined(CONFIG_CMD_ONENAND)
onenand_reloc();
+#endif #endif /* !defined(CONFIG_RELOC_FIXUP_WORKS) */
#ifdef CONFIG_LOGBUFFER diff --git a/common/cmd_onenand.c b/common/cmd_onenand.c index 83d967b..fe84c3b 100644 --- a/common/cmd_onenand.c +++ b/common/cmd_onenand.c @@ -525,6 +525,12 @@ static cmd_tbl_t cmd_onenand_sub[] = { U_BOOT_CMD_MKENT(markbad, CONFIG_SYS_MAXARGS, 0, do_onenand_markbad, "", ""), };
+#ifndef CONFIG_RELOC_FIXUP_WORKS +void onenand_reloc(void) {
fixup_cmdtable(cmd_onenand_sub, ARRAY_SIZE(cmd_onenand_sub));
+} +#endif
static int do_onenand(cmd_tbl_t * cmdtp, int flag, int argc, char * const argv[]) { cmd_tbl_t *c; -- 1.7.1
I've prepared same patch. :)
Oh, I'm sorry, I haven't noticed. Please merge whichever you find more suitable.
Cheers
Acked-by: Minkyu Kang mk7.kang@samsung.com
Thanks Minkyu Kang

Dear Marek Vasut,
On 3 October 2010 03:59, Marek Vasut marek.vasut@gmail.com wrote:
Dne So 2. října 2010 20:28:19 Minkyu Kang napsal(a):
Dear Vasut,
On 3 October 2010 02:33, Marek Vasut marek.vasut@gmail.com wrote:
Signed-off-by: Marek Vasut marek.vasut@gmail.com
arch/arm/lib/board.c | 6 ++++++ common/cmd_onenand.c | 6 ++++++ 2 files changed, 12 insertions(+), 0 deletions(-)
diff --git a/arch/arm/lib/board.c b/arch/arm/lib/board.c index 5f2dfd0..07995ba 100644 --- a/arch/arm/lib/board.c +++ b/arch/arm/lib/board.c @@ -695,6 +695,9 @@ void board_init_r (gd_t *id, ulong dest_addr) #if defined(CONFIG_CMD_I2C) extern void i2c_reloc(void); #endif +#if defined(CONFIG_CMD_ONENAND)
- extern void onenand_reloc(void);
+#endif #endif
gd = id; @@ -724,6 +727,9 @@ void board_init_r (gd_t *id, ulong dest_addr) #if defined(CONFIG_CMD_I2C) i2c_reloc(); #endif +#if defined(CONFIG_CMD_ONENAND)
- onenand_reloc();
+#endif #endif /* !defined(CONFIG_RELOC_FIXUP_WORKS) */
#ifdef CONFIG_LOGBUFFER diff --git a/common/cmd_onenand.c b/common/cmd_onenand.c index 83d967b..fe84c3b 100644 --- a/common/cmd_onenand.c +++ b/common/cmd_onenand.c @@ -525,6 +525,12 @@ static cmd_tbl_t cmd_onenand_sub[] = { U_BOOT_CMD_MKENT(markbad, CONFIG_SYS_MAXARGS, 0, do_onenand_markbad, "", ""), };
+#ifndef CONFIG_RELOC_FIXUP_WORKS +void onenand_reloc(void) {
- fixup_cmdtable(cmd_onenand_sub, ARRAY_SIZE(cmd_onenand_sub));
+} +#endif
static int do_onenand(cmd_tbl_t * cmdtp, int flag, int argc, char * const argv[]) { cmd_tbl_t *c; -- 1.7.1
I've prepared same patch. :)
Oh, I'm sorry, I haven't noticed. Please merge whichever you find more suitable.
No No.. I mean.. prepared at my local tree. I didn't send it to mailing list. I just acked your patch :)
Thanks. Minkyu Kang.

Dear Marek Vasut,
In message 201010022059.53303.marek.vasut@gmail.com you wrote:
I've prepared same patch. :)
Oh, I'm sorry, I haven't noticed. Please merge whichever you find more suitable.
NAK!
I want to see this fixed at the base, not by adding workarounds.
Best regards,
Wolfgang Denk

Dne Ne 3. října 2010 20:21:33 Wolfgang Denk napsal(a):
Dear Marek Vasut,
In message 201010022059.53303.marek.vasut@gmail.com you wrote:
I've prepared same patch. :)
Oh, I'm sorry, I haven't noticed. Please merge whichever you find more suitable.
NAK!
I want to see this fixed at the base, not by adding workarounds.
Best regards,
Wolfgang Denk
And what's the suggestion? The rest of similar cases were fixed this way too.

Dear Marek Vasut,
In message 201010032124.29154.marek.vasut@gmail.com you wrote:
Oh, I'm sorry, I haven't noticed. Please merge whichever you find more suitable.
NAK!
I want to see this fixed at the base, not by adding workarounds.
Best regards,
Wolfgang Denk
And what's the suggestion? The rest of similar cases were fixed this way too.
That was before the issue was solved, at least for PPC.
For the rest, plase read the "ARM relocation" thread.
Best regards,
Wolfgang Denk

Hello.
On 02-10-2010 21:33, Marek Vasut wrote:
Signed-off-by: Marek Vasut marek.vasut@gmail.com
[...]
diff --git a/common/cmd_onenand.c b/common/cmd_onenand.c index 83d967b..fe84c3b 100644 --- a/common/cmd_onenand.c +++ b/common/cmd_onenand.c @@ -525,6 +525,12 @@ static cmd_tbl_t cmd_onenand_sub[] = { U_BOOT_CMD_MKENT(markbad, CONFIG_SYS_MAXARGS, 0, do_onenand_markbad, "", ""), };
+#ifndef CONFIG_RELOC_FIXUP_WORKS +void onenand_reloc(void) {
{ should be on a separate line.
WBR, Sergei

Dne Ne 3. října 2010 13:19:21 Sergei Shtylyov napsal(a):
Hello.
On 02-10-2010 21:33, Marek Vasut wrote:
Signed-off-by: Marek Vasut marek.vasut@gmail.com
[...]
diff --git a/common/cmd_onenand.c b/common/cmd_onenand.c index 83d967b..fe84c3b 100644 --- a/common/cmd_onenand.c +++ b/common/cmd_onenand.c @@ -525,6 +525,12 @@ static cmd_tbl_t cmd_onenand_sub[] = {
U_BOOT_CMD_MKENT(markbad, CONFIG_SYS_MAXARGS, 0, do_onenand_markbad, "", ""),
};
+#ifndef CONFIG_RELOC_FIXUP_WORKS +void onenand_reloc(void) {
{ should be on a separate line.
WBR, Sergei
Fixed in my tree, thanks!

Dear Marek Vasut,
In message 201010031625.00339.marek.vasut@gmail.com you wrote:
+#ifndef CONFIG_RELOC_FIXUP_WORKS +void onenand_reloc(void) {
{ should be on a separate line.
WBR, Sergei
Fixed in my tree, thanks!
Keep in mind that "your tree" is totally uninteresting to anybody else. You are not the responsible custodian for OneNAND code, so you gotta repost patches.
Thanks.
Wolfgang Denk

This allows to specify where the OneNAND IPL should load the U-Boot binary. The purpose of CONFIG_SYS_LOAD_ADDR is different I believe.
On PXA, this is needed with OneNAND memories that expose only first 1kb of data, which is insufficient for SDRAM init. U-Boot can then be copied into SRAM.
Signed-off-by: Marek Vasut marek.vasut@gmail.com --- include/configs/apollon.h | 1 + onenand_ipl/onenand_boot.c | 4 ++-- 2 files changed, 3 insertions(+), 2 deletions(-)
diff --git a/include/configs/apollon.h b/include/configs/apollon.h index c1295de..d34e24d 100644 --- a/include/configs/apollon.h +++ b/include/configs/apollon.h @@ -199,6 +199,7 @@
/* default load address */ #define CONFIG_SYS_LOAD_ADDR (OMAP2420_SDRC_CS0) +#define CONFIG_SYS_IPL_LOAD_ADDR (OMAP2420_SDRC_CS0)
/* The 2420 has 12 GP timers, they can be driven by the SysClk (12/13/19.2) * or by 32KHz clk, or from external sig. This rate is divided by a local diff --git a/onenand_ipl/onenand_boot.c b/onenand_ipl/onenand_boot.c index 22baebb..2a0c7ce 100644 --- a/onenand_ipl/onenand_boot.c +++ b/onenand_ipl/onenand_boot.c @@ -33,11 +33,11 @@ void start_oneboot(void) { uchar *buf;
- buf = (uchar *) CONFIG_SYS_LOAD_ADDR; + buf = (uchar *) CONFIG_SYS_IPL_LOAD_ADDR;
onenand_read_block(buf);
- ((init_fnc_t *)CONFIG_SYS_LOAD_ADDR)(); + ((init_fnc_t *)CONFIG_SYS_IPL_LOAD_ADDR)();
/* should never come here */ }

Acked-by: Kyungmin Park kyungmin.park@samsung.com
On Sun, Oct 3, 2010 at 2:33 AM, Marek Vasut marek.vasut@gmail.com wrote:
This allows to specify where the OneNAND IPL should load the U-Boot binary. The purpose of CONFIG_SYS_LOAD_ADDR is different I believe.
On PXA, this is needed with OneNAND memories that expose only first 1kb of data, which is insufficient for SDRAM init. U-Boot can then be copied into SRAM.
Signed-off-by: Marek Vasut marek.vasut@gmail.com
include/configs/apollon.h | 1 + onenand_ipl/onenand_boot.c | 4 ++-- 2 files changed, 3 insertions(+), 2 deletions(-)
diff --git a/include/configs/apollon.h b/include/configs/apollon.h index c1295de..d34e24d 100644 --- a/include/configs/apollon.h +++ b/include/configs/apollon.h @@ -199,6 +199,7 @@
/* default load address */ #define CONFIG_SYS_LOAD_ADDR (OMAP2420_SDRC_CS0) +#define CONFIG_SYS_IPL_LOAD_ADDR (OMAP2420_SDRC_CS0)
/* The 2420 has 12 GP timers, they can be driven by the SysClk (12/13/19.2) * or by 32KHz clk, or from external sig. This rate is divided by a local diff --git a/onenand_ipl/onenand_boot.c b/onenand_ipl/onenand_boot.c index 22baebb..2a0c7ce 100644 --- a/onenand_ipl/onenand_boot.c +++ b/onenand_ipl/onenand_boot.c @@ -33,11 +33,11 @@ void start_oneboot(void) { uchar *buf;
- buf = (uchar *) CONFIG_SYS_LOAD_ADDR;
- buf = (uchar *) CONFIG_SYS_IPL_LOAD_ADDR;
onenand_read_block(buf);
- ((init_fnc_t *)CONFIG_SYS_LOAD_ADDR)();
- ((init_fnc_t *)CONFIG_SYS_IPL_LOAD_ADDR)();
/* should never come here */ } -- 1.7.1
U-Boot mailing list U-Boot@lists.denx.de http://lists.denx.de/mailman/listinfo/u-boot

Dne Ne 3. října 2010 08:40:55 Kyungmin Park napsal(a):
Acked-by: Kyungmin Park kyungmin.park@samsung.com
Added in my tree to commit message, Thanks!
On Sun, Oct 3, 2010 at 2:33 AM, Marek Vasut marek.vasut@gmail.com wrote:
This allows to specify where the OneNAND IPL should load the U-Boot binary. The purpose of CONFIG_SYS_LOAD_ADDR is different I believe.
On PXA, this is needed with OneNAND memories that expose only first 1kb of data, which is insufficient for SDRAM init. U-Boot can then be copied into SRAM.
Signed-off-by: Marek Vasut marek.vasut@gmail.com
include/configs/apollon.h | 1 + onenand_ipl/onenand_boot.c | 4 ++-- 2 files changed, 3 insertions(+), 2 deletions(-)
diff --git a/include/configs/apollon.h b/include/configs/apollon.h index c1295de..d34e24d 100644 --- a/include/configs/apollon.h +++ b/include/configs/apollon.h @@ -199,6 +199,7 @@
/* default load address */ #define CONFIG_SYS_LOAD_ADDR (OMAP2420_SDRC_CS0) +#define CONFIG_SYS_IPL_LOAD_ADDR (OMAP2420_SDRC_CS0)
/* The 2420 has 12 GP timers, they can be driven by the SysClk (12/13/19.2) * or by 32KHz clk, or from external sig. This rate is divided by a local diff --git a/onenand_ipl/onenand_boot.c b/onenand_ipl/onenand_boot.c index 22baebb..2a0c7ce 100644 --- a/onenand_ipl/onenand_boot.c +++ b/onenand_ipl/onenand_boot.c @@ -33,11 +33,11 @@ void start_oneboot(void) { uchar *buf;
buf = (uchar *) CONFIG_SYS_LOAD_ADDR;
buf = (uchar *) CONFIG_SYS_IPL_LOAD_ADDR; onenand_read_block(buf);
((init_fnc_t *)CONFIG_SYS_LOAD_ADDR)();
((init_fnc_t *)CONFIG_SYS_IPL_LOAD_ADDR)(); /* should never come here */
}
1.7.1
U-Boot mailing list U-Boot@lists.denx.de http://lists.denx.de/mailman/listinfo/u-boot

There apparantly is no reason for having "onenand_read_page" abstracted. Besides, it's static data which causes trouble.
Signed-off-by: Marek Vasut marek.vasut@gmail.com --- onenand_ipl/onenand_read.c | 9 +++------ 1 files changed, 3 insertions(+), 6 deletions(-)
diff --git a/onenand_ipl/onenand_read.c b/onenand_ipl/onenand_read.c index 8d0df81..008d73a 100644 --- a/onenand_ipl/onenand_read.c +++ b/onenand_ipl/onenand_read.c @@ -37,8 +37,6 @@ extern void *memcpy32(void *dest, void *src, int size); #endif
-int (*onenand_read_page)(ulong block, ulong page, u_char *buf, int pagesize); - /* read a page with ECC */ static int generic_onenand_read_page(ulong block, ulong page, u_char * buf, int pagesize) @@ -122,8 +120,6 @@ int onenand_read_block(unsigned char *buf) int pagesize, erasesize, erase_shift; int page_is_4KiB = 0;
- onenand_read_page = generic_onenand_read_page; - onenand_generic_init(&page_is_4KiB, &page);
if (page_is_4KiB) { @@ -139,10 +135,11 @@ int onenand_read_block(unsigned char *buf)
/* NOTE: you must read page from page 1 of block 0 */ /* read the block page by page */ + for (block = 0; block < nblocks; block++) { for (; page < ONENAND_PAGES_PER_BLOCK; page++) { - if (onenand_read_page(block, page, buf + offset, - pagesize)) { + if (generic_onenand_read_page(block, page, + buf + offset, pagesize)) { /* This block is bad. Skip it * and read next block */ offset -= page * pagesize;

Hi,
No it's used another place. that's reason not static function pointer. I'll update it soon.
Thank you, Kyungmin Park
On Sun, Oct 3, 2010 at 2:33 AM, Marek Vasut marek.vasut@gmail.com wrote:
There apparantly is no reason for having "onenand_read_page" abstracted. Besides, it's static data which causes trouble.
Signed-off-by: Marek Vasut marek.vasut@gmail.com
onenand_ipl/onenand_read.c | 9 +++------ 1 files changed, 3 insertions(+), 6 deletions(-)
diff --git a/onenand_ipl/onenand_read.c b/onenand_ipl/onenand_read.c index 8d0df81..008d73a 100644 --- a/onenand_ipl/onenand_read.c +++ b/onenand_ipl/onenand_read.c @@ -37,8 +37,6 @@ extern void *memcpy32(void *dest, void *src, int size); #endif
-int (*onenand_read_page)(ulong block, ulong page, u_char *buf, int pagesize);
/* read a page with ECC */ static int generic_onenand_read_page(ulong block, ulong page, u_char * buf, int pagesize) @@ -122,8 +120,6 @@ int onenand_read_block(unsigned char *buf) int pagesize, erasesize, erase_shift; int page_is_4KiB = 0;
- onenand_read_page = generic_onenand_read_page;
onenand_generic_init(&page_is_4KiB, &page);
if (page_is_4KiB) { @@ -139,10 +135,11 @@ int onenand_read_block(unsigned char *buf)
/* NOTE: you must read page from page 1 of block 0 */ /* read the block page by page */
for (block = 0; block < nblocks; block++) { for (; page < ONENAND_PAGES_PER_BLOCK; page++) {
- if (onenand_read_page(block, page, buf + offset,
- pagesize)) {
- if (generic_onenand_read_page(block, page,
- buf + offset, pagesize)) {
/* This block is bad. Skip it * and read next block */ offset -= page * pagesize; -- 1.7.1
U-Boot mailing list U-Boot@lists.denx.de http://lists.denx.de/mailman/listinfo/u-boot

Dne Ne 3. října 2010 08:40:19 Kyungmin Park napsal(a):
Hi,
No it's used another place. that's reason not static function pointer. I'll update it soon.
Hey, my problem is this code hangs my CPU. If I apply this patch, it doesn't happen anymore.
Where is this used and what's your proposed change?
Thank you in advance
Thank you, Kyungmin Park
On Sun, Oct 3, 2010 at 2:33 AM, Marek Vasut marek.vasut@gmail.com wrote:
There apparantly is no reason for having "onenand_read_page" abstracted. Besides, it's static data which causes trouble.
Signed-off-by: Marek Vasut marek.vasut@gmail.com
onenand_ipl/onenand_read.c | 9 +++------ 1 files changed, 3 insertions(+), 6 deletions(-)
diff --git a/onenand_ipl/onenand_read.c b/onenand_ipl/onenand_read.c index 8d0df81..008d73a 100644 --- a/onenand_ipl/onenand_read.c +++ b/onenand_ipl/onenand_read.c @@ -37,8 +37,6 @@ extern void *memcpy32(void *dest, void *src, int size); #endif
-int (*onenand_read_page)(ulong block, ulong page, u_char *buf, int pagesize); - /* read a page with ECC */ static int generic_onenand_read_page(ulong block, ulong page, u_char * buf, int pagesize) @@ -122,8 +120,6 @@ int onenand_read_block(unsigned char *buf) int pagesize, erasesize, erase_shift; int page_is_4KiB = 0;
onenand_read_page = generic_onenand_read_page;
onenand_generic_init(&page_is_4KiB, &page); if (page_is_4KiB) {
@@ -139,10 +135,11 @@ int onenand_read_block(unsigned char *buf)
/* NOTE: you must read page from page 1 of block 0 */ /* read the block page by page */
for (block = 0; block < nblocks; block++) { for (; page < ONENAND_PAGES_PER_BLOCK; page++) {
if (onenand_read_page(block, page, buf + offset,
pagesize)) {
if (generic_onenand_read_page(block, page,
buf + offset, pagesize)) { /* This block is bad. Skip it * and read next block */ offset -= page * pagesize;
-- 1.7.1
U-Boot mailing list U-Boot@lists.denx.de http://lists.denx.de/mailman/listinfo/u-boot

Dne Ne 3. října 2010 16:27:04 Marek Vasut napsal(a):
Dne Ne 3. října 2010 08:40:19 Kyungmin Park napsal(a):
Hi,
No it's used another place. that's reason not static function pointer. I'll update it soon.
Hey, my problem is this code hangs my CPU. If I apply this patch, it doesn't happen anymore.
Where is this used and what's your proposed change?
btw. who's the OneNAND maintainer (aka. who's supposed to pick up these changes ?)
Thank you in advance
Thank you, Kyungmin Park
On Sun, Oct 3, 2010 at 2:33 AM, Marek Vasut marek.vasut@gmail.com wrote:
There apparantly is no reason for having "onenand_read_page" abstracted. Besides, it's static data which causes trouble.
Signed-off-by: Marek Vasut marek.vasut@gmail.com
onenand_ipl/onenand_read.c | 9 +++------ 1 files changed, 3 insertions(+), 6 deletions(-)
diff --git a/onenand_ipl/onenand_read.c b/onenand_ipl/onenand_read.c index 8d0df81..008d73a 100644 --- a/onenand_ipl/onenand_read.c +++ b/onenand_ipl/onenand_read.c @@ -37,8 +37,6 @@
extern void *memcpy32(void *dest, void *src, int size); #endif
-int (*onenand_read_page)(ulong block, ulong page, u_char *buf, int pagesize); -
/* read a page with ECC */ static int generic_onenand_read_page(ulong block, ulong page,
u_char * buf, int pagesize)
@@ -122,8 +120,6 @@ int onenand_read_block(unsigned char *buf)
int pagesize, erasesize, erase_shift; int page_is_4KiB = 0;
onenand_read_page = generic_onenand_read_page;
onenand_generic_init(&page_is_4KiB, &page); if (page_is_4KiB) {
@@ -139,10 +135,11 @@ int onenand_read_block(unsigned char *buf)
/* NOTE: you must read page from page 1 of block 0 */ /* read the block page by page */
for (block = 0; block < nblocks; block++) { for (; page < ONENAND_PAGES_PER_BLOCK; page++) {
if (onenand_read_page(block, page, buf +
offset, - pagesize)) {
if (generic_onenand_read_page(block, page,
buf + offset, pagesize)) { /* This block is bad. Skip it * and read next block */ offset -= page * pagesize;
-- 1.7.1
U-Boot mailing list U-Boot@lists.denx.de http://lists.denx.de/mailman/listinfo/u-boot

Hi,
There's no OneNAND tree but all patches are mainlined by Scott Wood's tree.
And related with patch. I wonder why it's problem with your board?
As it's function pointer. It should be located the data section and it's assigned at runtime.
Maybe your board runs the code at OneNAND directly. It's the reason do you need to fixup.
I'll prepare the patches to fix this issue. So don't delete the codes.
Thank you, Kyungmin Park
-----Original Message----- From: Marek Vasut [mailto:marek.vasut@gmail.com] Sent: Monday, October 04, 2010 8:34 AM To: Kyungmin Park Cc: u-boot@lists.denx.de Subject: Re: [U-Boot] [PATCH 4/4] OneNAND: Use generic_onenand_read_page in IPL
Dne Ne 3. října 2010 16:27:04 Marek Vasut napsal(a):
Dne Ne 3. října 2010 08:40:19 Kyungmin Park napsal(a):
Hi,
No it's used another place. that's reason not static function pointer. I'll update it soon.
Hey, my problem is this code hangs my CPU. If I apply this patch, it doesn't happen anymore.
Where is this used and what's your proposed change?
btw. who's the OneNAND maintainer (aka. who's supposed to pick up these changes ?)
Thank you in advance
Thank you, Kyungmin Park
On Sun, Oct 3, 2010 at 2:33 AM, Marek Vasut marek.vasut@gmail.com wrote:
There apparantly is no reason for having "onenand_read_page" abstracted. Besides, it's static data which causes trouble.
Signed-off-by: Marek Vasut marek.vasut@gmail.com
onenand_ipl/onenand_read.c | 9 +++------ 1 files changed, 3 insertions(+), 6 deletions(-)
diff --git a/onenand_ipl/onenand_read.c b/onenand_ipl/onenand_read.c index 8d0df81..008d73a 100644 --- a/onenand_ipl/onenand_read.c +++ b/onenand_ipl/onenand_read.c @@ -37,8 +37,6 @@
extern void *memcpy32(void *dest, void *src, int size); #endif
-int (*onenand_read_page)(ulong block, ulong page, u_char *buf, int pagesize); -
/* read a page with ECC */ static int generic_onenand_read_page(ulong block, ulong page,
u_char * buf, int pagesize)
@@ -122,8 +120,6 @@ int onenand_read_block(unsigned char *buf)
int pagesize, erasesize, erase_shift; int page_is_4KiB = 0;
onenand_read_page = generic_onenand_read_page;
onenand_generic_init(&page_is_4KiB, &page); if (page_is_4KiB) {
@@ -139,10 +135,11 @@ int onenand_read_block(unsigned char *buf)
/* NOTE: you must read page from page 1 of block 0 */ /* read the block page by page */
for (block = 0; block < nblocks; block++) { for (; page < ONENAND_PAGES_PER_BLOCK; page++) {
if (onenand_read_page(block, page, buf +
offset, - pagesize)) {
if (generic_onenand_read_page(block, page,
buf + offset, pagesize)) { /* This block is bad. Skip it * and read next block */ offset -= page * pagesize;
-- 1.7.1
U-Boot mailing list U-Boot@lists.denx.de http://lists.denx.de/mailman/listinfo/u-boot

Dne Po 4. října 2010 01:45:16 Kyungmin Park napsal(a):
Hi,
There's no OneNAND tree but all patches are mainlined by Scott Wood's tree.
And related with patch. I wonder why it's problem with your board?
As it's function pointer. It should be located the data section and it's assigned at runtime.
The data section isn't writable, right ?
Maybe your board runs the code at OneNAND directly. It's the reason do you need to fixup.
Yes, it runs the code directly from the first 1kb window in OneNAND.
I'll prepare the patches to fix this issue. So don't delete the codes.
Thanks!
Thank you, Kyungmin Park
-----Original Message----- From: Marek Vasut [mailto:marek.vasut@gmail.com] Sent: Monday, October 04, 2010 8:34 AM To: Kyungmin Park Cc: u-boot@lists.denx.de Subject: Re: [U-Boot] [PATCH 4/4] OneNAND: Use generic_onenand_read_page in IPL
Dne Ne 3. října 2010 16:27:04 Marek Vasut napsal(a):
Dne Ne 3. října 2010 08:40:19 Kyungmin Park napsal(a):
Hi,
No it's used another place. that's reason not static function pointer. I'll update it soon.
Hey, my problem is this code hangs my CPU. If I apply this patch, it doesn't happen anymore.
Where is this used and what's your proposed change?
btw. who's the OneNAND maintainer (aka. who's supposed to pick up these changes ?)
Thank you in advance
Thank you, Kyungmin Park
On Sun, Oct 3, 2010 at 2:33 AM, Marek Vasut marek.vasut@gmail.com wrote:
There apparantly is no reason for having "onenand_read_page" abstracted. Besides, it's static data which causes trouble.
Signed-off-by: Marek Vasut marek.vasut@gmail.com
onenand_ipl/onenand_read.c | 9 +++------ 1 files changed, 3 insertions(+), 6 deletions(-)
diff --git a/onenand_ipl/onenand_read.c b/onenand_ipl/onenand_read.c index 8d0df81..008d73a 100644 --- a/onenand_ipl/onenand_read.c +++ b/onenand_ipl/onenand_read.c @@ -37,8 +37,6 @@
extern void *memcpy32(void *dest, void *src, int size); #endif
-int (*onenand_read_page)(ulong block, ulong page, u_char *buf, int pagesize); -
/* read a page with ECC */ static int generic_onenand_read_page(ulong block, ulong page,
u_char * buf, int pagesize)
@@ -122,8 +120,6 @@ int onenand_read_block(unsigned char *buf)
int pagesize, erasesize, erase_shift; int page_is_4KiB = 0;
onenand_read_page = generic_onenand_read_page;
onenand_generic_init(&page_is_4KiB, &page); if (page_is_4KiB) {
@@ -139,10 +135,11 @@ int onenand_read_block(unsigned char *buf)
/* NOTE: you must read page from page 1 of block 0 */ /* read the block page by page */
for (block = 0; block < nblocks; block++) { for (; page < ONENAND_PAGES_PER_BLOCK; page++) {
if (onenand_read_page(block, page, buf +
offset, - pagesize)) {
if (generic_onenand_read_page(block, page,
buf + offset, pagesize)) { /* This block is bad. Skip it * and read next block */ offset -= page * pagesize;
-- 1.7.1
U-Boot mailing list U-Boot@lists.denx.de http://lists.denx.de/mailman/listinfo/u-boot

Dear Marek Vasut,
In message 201010040224.35419.marek.vasut@gmail.com you wrote:
The data section isn't writable, right ?
The data section is (officially) writable only after relocation. This rule has the situation in mind that U-Boot starts runnng from some form of ROM (like NOR flash). If loaded to RAM by some other preloader then the data segmentis axctually writable, but you still have to keep in mind that you have only a minimal stack and no valid BSS.
Best regards,
Wolfgang Denk

Dne Po 4. října 2010 01:45:16 Kyungmin Park napsal(a):
Hi,
There's no OneNAND tree but all patches are mainlined by Scott Wood's tree.
And related with patch. I wonder why it's problem with your board?
As it's function pointer. It should be located the data section and it's assigned at runtime.
Maybe your board runs the code at OneNAND directly. It's the reason do you need to fixup.
I'll prepare the patches to fix this issue. So don't delete the codes.
Hi,
any update ?
Thanks
Thank you, Kyungmin Park
-----Original Message----- From: Marek Vasut [mailto:marek.vasut@gmail.com] Sent: Monday, October 04, 2010 8:34 AM To: Kyungmin Park Cc: u-boot@lists.denx.de Subject: Re: [U-Boot] [PATCH 4/4] OneNAND: Use generic_onenand_read_page in IPL
Dne Ne 3. října 2010 16:27:04 Marek Vasut napsal(a):
Dne Ne 3. října 2010 08:40:19 Kyungmin Park napsal(a):
Hi,
No it's used another place. that's reason not static function pointer. I'll update it soon.
Hey, my problem is this code hangs my CPU. If I apply this patch, it doesn't happen anymore.
Where is this used and what's your proposed change?
btw. who's the OneNAND maintainer (aka. who's supposed to pick up these changes ?)
Thank you in advance
Thank you, Kyungmin Park
On Sun, Oct 3, 2010 at 2:33 AM, Marek Vasut marek.vasut@gmail.com wrote:
There apparantly is no reason for having "onenand_read_page" abstracted. Besides, it's static data which causes trouble.
Signed-off-by: Marek Vasut marek.vasut@gmail.com
onenand_ipl/onenand_read.c | 9 +++------ 1 files changed, 3 insertions(+), 6 deletions(-)
diff --git a/onenand_ipl/onenand_read.c b/onenand_ipl/onenand_read.c index 8d0df81..008d73a 100644 --- a/onenand_ipl/onenand_read.c +++ b/onenand_ipl/onenand_read.c @@ -37,8 +37,6 @@
extern void *memcpy32(void *dest, void *src, int size); #endif
-int (*onenand_read_page)(ulong block, ulong page, u_char *buf, int pagesize); -
/* read a page with ECC */ static int generic_onenand_read_page(ulong block, ulong page,
u_char * buf, int pagesize)
@@ -122,8 +120,6 @@ int onenand_read_block(unsigned char *buf)
int pagesize, erasesize, erase_shift; int page_is_4KiB = 0;
onenand_read_page = generic_onenand_read_page;
onenand_generic_init(&page_is_4KiB, &page); if (page_is_4KiB) {
@@ -139,10 +135,11 @@ int onenand_read_block(unsigned char *buf)
/* NOTE: you must read page from page 1 of block 0 */ /* read the block page by page */
for (block = 0; block < nblocks; block++) { for (; page < ONENAND_PAGES_PER_BLOCK; page++) {
if (onenand_read_page(block, page, buf +
offset, - pagesize)) {
if (generic_onenand_read_page(block, page,
buf + offset, pagesize)) { /* This block is bad. Skip it * and read next block */ offset -= page * pagesize;
-- 1.7.1
U-Boot mailing list U-Boot@lists.denx.de http://lists.denx.de/mailman/listinfo/u-boot

On Sat, 2 Oct 2010 19:33:59 +0200 Marek Vasut marek.vasut@gmail.com wrote:
There apparantly is no reason for having "onenand_read_page" abstracted. Besides, it's static data which causes trouble.
Why does static data cause trouble?
I don't think purging the code of all instances of static data is reasonable.
-Scott

Dne So 2. října 2010 19:33:56 Marek Vasut napsal(a):
This moves "struct nand_bbt_descr largepage_memorybased" into "onenand_default_bbt" as that's the only place where this is used.
This also removes an entry from .data section. (For me, this section disappears after relocation).
Scott, can you please check this and the other three patches? Thanks
Signed-off-by: Marek Vasut marek.vasut@gmail.com
drivers/mtd/onenand/onenand_bbt.c | 25 ++++++++++++------------- 1 files changed, 12 insertions(+), 13 deletions(-)
diff --git a/drivers/mtd/onenand/onenand_bbt.c b/drivers/mtd/onenand/onenand_bbt.c index 1354877..c5d3905 100644 --- a/drivers/mtd/onenand/onenand_bbt.c +++ b/drivers/mtd/onenand/onenand_bbt.c @@ -227,19 +227,6 @@ int onenand_scan_bbt(struct mtd_info *mtd, struct nand_bbt_descr *bd) return ret; }
-/*
- Define some generic bad / good block scan pattern which are used
- while scanning a device for factory marked good / bad blocks.
- */
-static uint8_t scan_ff_pattern[] = { 0xff, 0xff };
-static struct nand_bbt_descr largepage_memorybased = {
- .options = 0,
- .offs = 0,
- .len = 2,
- .pattern = scan_ff_pattern,
-};
/**
- onenand_default_bbt - [OneNAND Interface] Select a default bad block
table for the device * @param mtd MTD device structure @@ -252,6 +239,18 @@ int onenand_default_bbt(struct mtd_info *mtd) struct onenand_chip *this = mtd->priv; struct bbm_info *bbm;
- /*
* Define some generic bad / good block scan pattern which are used
* while scanning a device for factory marked good / bad blocks.
*/
- uint8_t scan_ff_pattern[] = { 0xff, 0xff };
- struct nand_bbt_descr largepage_memorybased = {
.options = 0,
.offs = 0,
.len = 2,
.pattern = scan_ff_pattern,
- };
- this->bbm = malloc(sizeof(struct bbm_info)); if (!this->bbm) return -ENOMEM;

On Sat, 2 Oct 2010 19:33:56 +0200 Marek Vasut marek.vasut@gmail.com wrote:
This moves "struct nand_bbt_descr largepage_memorybased" into "onenand_default_bbt" as that's the only place where this is used.
This also removes an entry from .data section. (For me, this section disappears after relocation).
Signed-off-by: Marek Vasut marek.vasut@gmail.com
drivers/mtd/onenand/onenand_bbt.c | 25 ++++++++++++------------- 1 files changed, 12 insertions(+), 13 deletions(-)
diff --git a/drivers/mtd/onenand/onenand_bbt.c b/drivers/mtd/onenand/onenand_bbt.c index 1354877..c5d3905 100644 --- a/drivers/mtd/onenand/onenand_bbt.c +++ b/drivers/mtd/onenand/onenand_bbt.c @@ -227,19 +227,6 @@ int onenand_scan_bbt(struct mtd_info *mtd, struct nand_bbt_descr *bd) return ret; }
-/*
- Define some generic bad / good block scan pattern which are used
- while scanning a device for factory marked good / bad blocks.
- */
-static uint8_t scan_ff_pattern[] = { 0xff, 0xff };
-static struct nand_bbt_descr largepage_memorybased = {
- .options = 0,
- .offs = 0,
- .len = 2,
- .pattern = scan_ff_pattern,
-};
/**
- onenand_default_bbt - [OneNAND Interface] Select a default bad block table for the device
- @param mtd MTD device structure
@@ -252,6 +239,18 @@ int onenand_default_bbt(struct mtd_info *mtd) struct onenand_chip *this = mtd->priv; struct bbm_info *bbm;
- /*
* Define some generic bad / good block scan pattern which are used
* while scanning a device for factory marked good / bad blocks.
*/
- uint8_t scan_ff_pattern[] = { 0xff, 0xff };
- struct nand_bbt_descr largepage_memorybased = {
.options = 0,
.offs = 0,
.len = 2,
.pattern = scan_ff_pattern,
- };
- this->bbm = malloc(sizeof(struct bbm_info)); if (!this->bbm) return -ENOMEM;
NACK, you're taking the address of stack data and using it after the stack frame goes away.
What problem are you trying to solve?
-Scott

Dne Po 4. října 2010 19:59:14 Scott Wood napsal(a):
On Sat, 2 Oct 2010 19:33:56 +0200
Marek Vasut marek.vasut@gmail.com wrote:
This moves "struct nand_bbt_descr largepage_memorybased" into "onenand_default_bbt" as that's the only place where this is used.
This also removes an entry from .data section. (For me, this section disappears after relocation).
Signed-off-by: Marek Vasut marek.vasut@gmail.com
drivers/mtd/onenand/onenand_bbt.c | 25 ++++++++++++------------- 1 files changed, 12 insertions(+), 13 deletions(-)
diff --git a/drivers/mtd/onenand/onenand_bbt.c b/drivers/mtd/onenand/onenand_bbt.c index 1354877..c5d3905 100644 --- a/drivers/mtd/onenand/onenand_bbt.c +++ b/drivers/mtd/onenand/onenand_bbt.c @@ -227,19 +227,6 @@ int onenand_scan_bbt(struct mtd_info *mtd, struct nand_bbt_descr *bd)
return ret;
}
-/*
- Define some generic bad / good block scan pattern which are used
- while scanning a device for factory marked good / bad blocks.
- */
-static uint8_t scan_ff_pattern[] = { 0xff, 0xff };
-static struct nand_bbt_descr largepage_memorybased = {
- .options = 0,
- .offs = 0,
- .len = 2,
- .pattern = scan_ff_pattern,
-};
/**
- onenand_default_bbt - [OneNAND Interface] Select a default bad block
table for the device * @param mtd MTD device structure
@@ -252,6 +239,18 @@ int onenand_default_bbt(struct mtd_info *mtd)
struct onenand_chip *this = mtd->priv; struct bbm_info *bbm;
/*
* Define some generic bad / good block scan pattern which are used
* while scanning a device for factory marked good / bad blocks.
*/
uint8_t scan_ff_pattern[] = { 0xff, 0xff };
struct nand_bbt_descr largepage_memorybased = {
.options = 0,
.offs = 0,
.len = 2,
.pattern = scan_ff_pattern,
};
this->bbm = malloc(sizeof(struct bbm_info)); if (!this->bbm)
return -ENOMEM;
NACK, you're taking the address of stack data and using it after the stack frame goes away.
The .data section doesn't exist at 0x0 (where the stuff is linked) when this code is executed in my case.
What problem are you trying to solve?
-Scott

On Mon, 4 Oct 2010 23:15:11 +0200 Marek Vasut marek.vasut@gmail.com wrote:
Dne Po 4. října 2010 19:59:14 Scott Wood napsal(a):
NACK, you're taking the address of stack data and using it after the stack frame goes away.
The .data section doesn't exist at 0x0 (where the stuff is linked) when this code is executed in my case.
So fix the linking. :-)
-Scott

Dne Po 4. října 2010 23:18:51 Scott Wood napsal(a):
On Mon, 4 Oct 2010 23:15:11 +0200
Marek Vasut marek.vasut@gmail.com wrote:
Dne Po 4. října 2010 19:59:14 Scott Wood napsal(a):
NACK, you're taking the address of stack data and using it after the stack frame goes away.
The .data section doesn't exist at 0x0 (where the stuff is linked) when this code is executed in my case.
So fix the linking. :-)
I'd gladly accept any suggestion or if you could point me in a correct direction.
-Scott

Hi,
I tested it with the latest u-boot codes. and without OneNAND patch, it's boot and working well.
Of course I used the relocation method at u-boot.
Thank you, Kyungmin Park
On Tue, Oct 5, 2010 at 7:31 AM, Marek Vasut marek.vasut@gmail.com wrote:
Dne Po 4. října 2010 23:18:51 Scott Wood napsal(a):
On Mon, 4 Oct 2010 23:15:11 +0200
Marek Vasut marek.vasut@gmail.com wrote:
Dne Po 4. října 2010 19:59:14 Scott Wood napsal(a):
NACK, you're taking the address of stack data and using it after the stack frame goes away.
The .data section doesn't exist at 0x0 (where the stuff is linked) when this code is executed in my case.
So fix the linking. :-)
I'd gladly accept any suggestion or if you could point me in a correct direction.
-Scott
U-Boot mailing list U-Boot@lists.denx.de http://lists.denx.de/mailman/listinfo/u-boot

Dne Út 5. října 2010 04:05:57 Kyungmin Park napsal(a):
Hi,
I tested it with the latest u-boot codes. and without OneNAND patch, it's boot and working well.
Of course I used the relocation method at u-boot.
Well is it necessary to expose this stuff so much anyway ? Why can't it be in that function?
Besides, in my case, the .data doesn't exist at the place where it was before relocation after the relocation is done. With this patch though, I have no problems even in that case.
Cheers
Thank you, Kyungmin Park
On Tue, Oct 5, 2010 at 7:31 AM, Marek Vasut marek.vasut@gmail.com wrote:
Dne Po 4. října 2010 23:18:51 Scott Wood napsal(a):
On Mon, 4 Oct 2010 23:15:11 +0200
Marek Vasut marek.vasut@gmail.com wrote:
Dne Po 4. října 2010 19:59:14 Scott Wood napsal(a):
NACK, you're taking the address of stack data and using it after the stack frame goes away.
The .data section doesn't exist at 0x0 (where the stuff is linked) when this code is executed in my case.
So fix the linking. :-)
I'd gladly accept any suggestion or if you could point me in a correct direction.
-Scott
U-Boot mailing list U-Boot@lists.denx.de http://lists.denx.de/mailman/listinfo/u-boot
participants (6)
-
Kyungmin Park
-
Marek Vasut
-
Minkyu Kang
-
Scott Wood
-
Sergei Shtylyov
-
Wolfgang Denk