Patchwork [U-Boot,v2] nand_spl: Fix large page nand_command()

login
register
mail settings
Submitter Alex
Date April 6, 2011, 8:01 p.m.
Message ID <4D9CC6B0.6020608@dawning.com>
Download mbox | patch
Permalink /patch/90064/
State Accepted
Commit 65a9db7be0868be91ba81b9b5bf821de82e6d9b0
Headers show

Comments

Alex - April 6, 2011, 8:01 p.m.
Sorry, I screwed up the settings in Thunderbird, hopefully I fixed the 
white space/name issues now.

About the patch not applying, I used git format-patch and git apply to
test the stuff locally, was that wrong?

Here is a revised patch, I hope this is better.

- Alex

From: Alex Waterman <awaterman@dawning.com>
Date: Wed, 6 Apr 2011 15:09:57 -0400
Subject: [PATCH] nand_spl: Fix large page nand_command()

This patch changes the large page nand_command() routine to use a word
offset instead of a byte offset. The 'offs' argument gets divided by 2
so that the offset passed to nand_command() is still by byte offset.
Originally, the offset was not shifted and when too high an offset was
requested the nand chip would attempt to read non-existent data.

Changes for v2:

 - Moved the offset calculation to outside of the OOB emulation code.
 - Hopefully no more whitespace mangling.

Signed-off-by: Alex Waterman <awaterman@dawning.com>
---
 nand_spl/nand_boot.c |    4 ++++
 1 files changed, 4 insertions(+), 0 deletions(-)

-- 1.7.3.1
Scott Wood - April 11, 2011, 6:58 p.m.
On Wed, Apr 06, 2011 at 04:01:52PM -0400, Alex Waterman wrote:
> Sorry, I screwed up the settings in Thunderbird, hopefully I fixed the 
> white space/name issues now.
> 
> About the patch not applying, I used git format-patch and git apply to
> test the stuff locally, was that wrong?
> 
> Here is a revised patch, I hope this is better.
> 
> - Alex

Applied to u-boot-nand-flash

For future reference, anything which is not supposed to be part
of the permanent changelog (including a description of changes
from the previous version) should go below the --- line.

-Scott
Wolfgang Denk - May 1, 2011, 2:43 p.m.
Dear Alex Waterman,

In message <4D9CC6B0.6020608@dawning.com> you wrote:
>
> This patch changes the large page nand_command() routine to use a word
> offset instead of a byte offset. The 'offs' argument gets divided by 2
> so that the offset passed to nand_command() is still by byte offset.
> Originally, the offset was not shifted and when too high an offset was
> requested the nand chip would attempt to read non-existent data.
> 
> Changes for v2:
> 
>  - Moved the offset calculation to outside of the OOB emulation code.
>  - Hopefully no more whitespace mangling.
> 
> Signed-off-by: Alex Waterman <awaterman@dawning.com>
> ---
>  nand_spl/nand_boot.c |    4 ++++
>  1 files changed, 4 insertions(+), 0 deletions(-)

Unfortunately this patch breaks building of the canyonlands_nand and
glacier_nand configurations:

Configuring for canyonlands_nand - Board: canyonlands, Options: CANYONLANDS,NAND_U_BOOT,SYS_TEXT_BASE=0x01000000
ppc_6xx-ld: section .resetvec [e3003ffc -> e3003fff] overlaps section .bss.ndfc_cs [e3003ff4 -> e3004003]
make[1]: *** [/work/wd/tmp-ppc/nand_spl/u-boot-spl] Error 1
make: *** [nand_spl] Error 2
make: *** Waiting for unfinished jobs....
ppc_6xx-size: '/work/wd/tmp-ppc/u-boot': No such file

Configuring for glacier_nand - Board: canyonlands, Options: GLACIER,NAND_U_BOOT,SYS_TEXT_BASE=0x01000000
ppc_6xx-ld: section .resetvec [e3003ffc -> e3003fff] overlaps section .bss.ndfc_cs [e3003ff4 -> e3004003]
make[1]: *** [/work/wd/tmp-ppc/nand_spl/u-boot-spl] Error 1
make: *** [nand_spl] Error 2
make: *** Waiting for unfinished jobs....
ppc_6xx-size: '/work/wd/tmp-ppc/u-boot': No such file


Git bisect says:

65a9db7be0868be91ba81b9b5bf821de82e6d9b0 is the first bad commit
commit 65a9db7be0868be91ba81b9b5bf821de82e6d9b0
Author: Alex Waterman <awaterman@dawning.com>
Date:   Wed Apr 6 16:01:52 2011 -0400

    nand_spl: Fix large page nand_command()


Can you please provide a fix?  Thanks.


Best regards,

Wolfgang Denk
Scott Wood - May 3, 2011, 4:08 p.m.
On Sun, 1 May 2011 16:43:30 +0200
Wolfgang Denk <wd@denx.de> wrote:

> Dear Alex Waterman,
> 
> In message <4D9CC6B0.6020608@dawning.com> you wrote:
> >
> > This patch changes the large page nand_command() routine to use a word
> > offset instead of a byte offset. The 'offs' argument gets divided by 2
> > so that the offset passed to nand_command() is still by byte offset.
> > Originally, the offset was not shifted and when too high an offset was
> > requested the nand chip would attempt to read non-existent data.
> > 
> > Changes for v2:
> > 
> >  - Moved the offset calculation to outside of the OOB emulation code.
> >  - Hopefully no more whitespace mangling.
> > 
> > Signed-off-by: Alex Waterman <awaterman@dawning.com>
> > ---
> >  nand_spl/nand_boot.c |    4 ++++
> >  1 files changed, 4 insertions(+), 0 deletions(-)
> 
> Unfortunately this patch breaks building of the canyonlands_nand and
> glacier_nand configurations:
> 
> Configuring for canyonlands_nand - Board: canyonlands, Options: CANYONLANDS,NAND_U_BOOT,SYS_TEXT_BASE=0x01000000
> ppc_6xx-ld: section .resetvec [e3003ffc -> e3003fff] overlaps section .bss.ndfc_cs [e3003ff4 -> e3004003]
> make[1]: *** [/work/wd/tmp-ppc/nand_spl/u-boot-spl] Error 1
> make: *** [nand_spl] Error 2
> make: *** Waiting for unfinished jobs....
> ppc_6xx-size: '/work/wd/tmp-ppc/u-boot': No such file
> 
> Configuring for glacier_nand - Board: canyonlands, Options: GLACIER,NAND_U_BOOT,SYS_TEXT_BASE=0x01000000
> ppc_6xx-ld: section .resetvec [e3003ffc -> e3003fff] overlaps section .bss.ndfc_cs [e3003ff4 -> e3004003]
> make[1]: *** [/work/wd/tmp-ppc/nand_spl/u-boot-spl] Error 1
> make: *** [nand_spl] Error 2
> make: *** Waiting for unfinished jobs....
> ppc_6xx-size: '/work/wd/tmp-ppc/u-boot': No such file

They build for me with GCC 4.5.1 -- probably right on the edge of the code
size limit.

-Scott

Patch

diff --git a/nand_spl/nand_boot.c b/nand_spl/nand_boot.c
index 76b8566..4a96878 100644
--- a/nand_spl/nand_boot.c
+++ b/nand_spl/nand_boot.c
@@ -90,6 +90,10 @@  static int nand_command(struct mtd_info *mtd, int block, int page, int offs, u8
 		cmd = NAND_CMD_READ0;
 	}
 
+	/* Shift the offset from byte addressing to word addressing. */
+	if (this->options & NAND_BUSWIDTH_16)
+		offs >>= 1;
+
 	/* Begin command latch cycle */
 	this->cmd_ctrl(mtd, cmd, NAND_CTRL_CLE | NAND_CTRL_CHANGE);
 	/* Set ALE and clear CLE to start address cycle */