[U-Boot] [PATCH] ARM DaVinci: Adding CONFIG options for NAND ALE and CLE
The CLE and ALE for DaVinci DM644x is not the same as DM646x. This patch adds 2 CONFIG_ options to the config.h header files to all the DM6446 based boards. These values are then used by the driver. This has been tested on the DM644x, DM355, DM357 and DM365. Support for the latter 3 will be added soon.
Signed-off-by: Sandeep Paulraj s-paulraj@ti.com --- drivers/mtd/nand/davinci_nand.c | 6 +++--- include/asm-arm/arch-davinci/nand_defs.h | 9 ++++----- include/configs/davinci_dvevm.h | 2 ++ include/configs/davinci_schmoogie.h | 2 ++ include/configs/davinci_sffsdr.h | 2 ++ include/configs/davinci_sonata.h | 2 ++ 6 files changed, 15 insertions(+), 8 deletions(-)
diff --git a/drivers/mtd/nand/davinci_nand.c b/drivers/mtd/nand/davinci_nand.c index a974667..dcc0f39 100644 --- a/drivers/mtd/nand/davinci_nand.c +++ b/drivers/mtd/nand/davinci_nand.c @@ -54,13 +54,13 @@ static void nand_davinci_hwcontrol(struct mtd_info *mtd, int cmd, unsigned int c struct nand_chip *this = mtd->priv; u_int32_t IO_ADDR_W = (u_int32_t)this->IO_ADDR_W;
- IO_ADDR_W &= ~(MASK_ALE|MASK_CLE); + IO_ADDR_W &= ~(CONFIG_MASK_ALE | CONFIG_MASK_CLE);
if (ctrl & NAND_CTRL_CHANGE) { if ( ctrl & NAND_CLE ) - IO_ADDR_W |= MASK_CLE; + IO_ADDR_W |= CONFIG_MASK_CLE; if ( ctrl & NAND_ALE ) - IO_ADDR_W |= MASK_ALE; + IO_ADDR_W |= CONFIG_MASK_ALE; this->IO_ADDR_W = (void __iomem *) IO_ADDR_W; }
diff --git a/include/asm-arm/arch-davinci/nand_defs.h b/include/asm-arm/arch-davinci/nand_defs.h index 187d3c3..a2d01b0 100644 --- a/include/asm-arm/arch-davinci/nand_defs.h +++ b/include/asm-arm/arch-davinci/nand_defs.h @@ -28,11 +28,10 @@
#include <asm/arch/hardware.h>
-#define MASK_CLE 0x10 -#define MASK_ALE 0x0a - -#define NAND_CE0CLE ((volatile u_int8_t *)(CONFIG_SYS_NAND_BASE + 0x10)) -#define NAND_CE0ALE ((volatile u_int8_t *)(CONFIG_SYS_NAND_BASE + 0x0a)) +#define NAND_CE0CLE ((volatile u_int8_t *)(CONFIG_SYS_NAND_BASE + \ + CONFIG_MASK_CLE)) +#define NAND_CE0ALE ((volatile u_int8_t *)(CONFIG_SYS_NAND_BASE + \ + CONFIG_MASK_ALE)) #define NAND_CE0DATA ((volatile u_int8_t *)CONFIG_SYS_NAND_BASE)
typedef struct { diff --git a/include/configs/davinci_dvevm.h b/include/configs/davinci_dvevm.h index fae430b..1fe3e19 100644 --- a/include/configs/davinci_dvevm.h +++ b/include/configs/davinci_dvevm.h @@ -128,6 +128,8 @@ #define CONFIG_SYS_NAND_BASE 0x02000000 #define CONFIG_SYS_NAND_HW_ECC #define CONFIG_SYS_MAX_NAND_DEVICE 1 /* Max number of NAND devices */ +#define CONFIG_MASK_CLE 0x10 +#define CONFIG_MASK_ALE 0x0a #define CONFIG_ENV_OFFSET 0x0 /* Block 0--not used by bootcode */ #define DEF_BOOTM "" #elif defined(CONFIG_SYS_USE_NOR) diff --git a/include/configs/davinci_schmoogie.h b/include/configs/davinci_schmoogie.h index 923e477..40df3c3 100644 --- a/include/configs/davinci_schmoogie.h +++ b/include/configs/davinci_schmoogie.h @@ -90,6 +90,8 @@ #define CONFIG_SYS_NAND_BASE 0x02000000 #define CONFIG_SYS_NAND_HW_ECC #define CONFIG_SYS_MAX_NAND_DEVICE 1 /* Max number of NAND devices */ +#define CONFIG_MASK_CLE 0x10 +#define CONFIG_MASK_ALE 0x0a #define CONFIG_ENV_OFFSET 0x0 /* Block 0--not used by bootcode */ /*=====================*/ /* Board related stuff */ diff --git a/include/configs/davinci_sffsdr.h b/include/configs/davinci_sffsdr.h index 73a59db..5d693f8 100644 --- a/include/configs/davinci_sffsdr.h +++ b/include/configs/davinci_sffsdr.h @@ -86,6 +86,8 @@ #define CONFIG_SYS_NAND_BASE 0x02000000 #define CONFIG_SYS_NAND_HW_ECC #define CONFIG_SYS_MAX_NAND_DEVICE 1 /* Max number of NAND devices */ +#define CONFIG_MASK_CLE 0x10 +#define CONFIG_MASK_ALE 0x0a #define CONFIG_ENV_OFFSET 0x0 /* Block 0--not used by bootcode */ /* I2C switch definitions for PCA9543 chip */ #define CONFIG_SYS_I2C_PCA9543_ADDR 0x70 diff --git a/include/configs/davinci_sonata.h b/include/configs/davinci_sonata.h index 70d2c7d..6699b3a 100644 --- a/include/configs/davinci_sonata.h +++ b/include/configs/davinci_sonata.h @@ -123,6 +123,8 @@ #define CONFIG_SYS_NAND_BASE 0x02000000 #define CONFIG_SYS_NAND_HW_ECC #define CONFIG_SYS_MAX_NAND_DEVICE 1 /* Max number of NAND devices */ +#define CONFIG_MASK_CLE 0x10 +#define CONFIG_MASK_ALE 0x0a #define CONFIG_ENV_OFFSET 0x0 /* Block 0--not used by bootcode */ #define DEF_BOOTM "" #elif defined(CONFIG_SYS_USE_NOR)
On Tue, 2009-04-28 at 16:33 -0400, s-paulraj@ti.com wrote:
The CLE and ALE for DaVinci DM644x is not the same as DM646x. This patch adds 2 CONFIG_ options to the config.h header files to all the DM6446 based boards. These values are then used by the driver. This has been tested on the DM644x, DM355, DM357 and DM365. Support for the latter 3 will be added soon.
Signed-off-by: Sandeep Paulraj s-paulraj@ti.com
drivers/mtd/nand/davinci_nand.c | 6 +++--- include/asm-arm/arch-davinci/nand_defs.h | 9 ++++----- include/configs/davinci_dvevm.h | 2 ++ include/configs/davinci_schmoogie.h | 2 ++ include/configs/davinci_sffsdr.h | 2 ++ include/configs/davinci_sonata.h | 2 ++ 6 files changed, 15 insertions(+), 8 deletions(-)
diff --git a/drivers/mtd/nand/davinci_nand.c b/drivers/mtd/nand/davinci_nand.c index a974667..dcc0f39 100644 --- a/drivers/mtd/nand/davinci_nand.c +++ b/drivers/mtd/nand/davinci_nand.c @@ -54,13 +54,13 @@ static void nand_davinci_hwcontrol(struct mtd_info *mtd, int cmd, unsigned int c struct nand_chip *this = mtd->priv; u_int32_t IO_ADDR_W = (u_int32_t)this->IO_ADDR_W;
- IO_ADDR_W &= ~(MASK_ALE|MASK_CLE);
IO_ADDR_W &= ~(CONFIG_MASK_ALE | CONFIG_MASK_CLE);
if (ctrl & NAND_CTRL_CHANGE) { if ( ctrl & NAND_CLE )
IO_ADDR_W |= MASK_CLE;
IO_ADDR_W |= CONFIG_MASK_CLE;
IO_ADDR_W |= MASK_ALE;
IO_ADDR_W |= CONFIG_MASK_ALE;
When I modified this code in the MV kernel, I remember having to use += and -= instead of |= and &=. The reason is tha DM6467 CLE/ALE mask has large enough that the bits overlaps IO_ADDR_W.
Steve, My intention was to change that when I add the DM6467 Support which I am working on even as we speak.
I am aware of it.
Thanks, Sandeep
-----Original Message----- From: Steve Chen [mailto:schen@mvista.com] Sent: Tuesday, April 28, 2009 4:54 PM To: Paulraj, Sandeep Cc: u-boot@lists.denx.de; davinci-linux-open-source@linux.davincidsp.com Subject: Re: [PATCH] ARM DaVinci: Adding CONFIG options for NAND ALE and CLE
On Tue, 2009-04-28 at 16:33 -0400, s-paulraj@ti.com wrote:
The CLE and ALE for DaVinci DM644x is not the same as DM646x. This patch adds 2 CONFIG_ options to the config.h header files to all the DM6446
based
boards. These values are then used by the driver. This has been tested
on the
DM644x, DM355, DM357 and DM365. Support for the latter 3 will be added
soon.
Signed-off-by: Sandeep Paulraj s-paulraj@ti.com
drivers/mtd/nand/davinci_nand.c | 6 +++--- include/asm-arm/arch-davinci/nand_defs.h | 9 ++++----- include/configs/davinci_dvevm.h | 2 ++ include/configs/davinci_schmoogie.h | 2 ++ include/configs/davinci_sffsdr.h | 2 ++ include/configs/davinci_sonata.h | 2 ++ 6 files changed, 15 insertions(+), 8 deletions(-)
diff --git a/drivers/mtd/nand/davinci_nand.c
b/drivers/mtd/nand/davinci_nand.c
index a974667..dcc0f39 100644 --- a/drivers/mtd/nand/davinci_nand.c +++ b/drivers/mtd/nand/davinci_nand.c @@ -54,13 +54,13 @@ static void nand_davinci_hwcontrol(struct mtd_info
*mtd, int cmd, unsigned int c
struct nand_chip *this = mtd->priv; u_int32_t IO_ADDR_W = (u_int32_t)this->IO_ADDR_W;
- IO_ADDR_W &= ~(MASK_ALE|MASK_CLE);
IO_ADDR_W &= ~(CONFIG_MASK_ALE | CONFIG_MASK_CLE);
if (ctrl & NAND_CTRL_CHANGE) { if ( ctrl & NAND_CLE )
IO_ADDR_W |= MASK_CLE;
IO_ADDR_W |= CONFIG_MASK_CLE;
IO_ADDR_W |= MASK_ALE;
IO_ADDR_W |= CONFIG_MASK_ALE;When I modified this code in the MV kernel, I remember having to use += and -= instead of |= and &=. The reason is tha DM6467 CLE/ALE mask has large enough that the bits overlaps IO_ADDR_W.
On Tue, Apr 28, 2009 at 04:33:14PM -0400, s-paulraj@ti.com wrote:
diff --git a/include/configs/davinci_dvevm.h b/include/configs/davinci_dvevm.h index fae430b..1fe3e19 100644 --- a/include/configs/davinci_dvevm.h +++ b/include/configs/davinci_dvevm.h @@ -128,6 +128,8 @@ #define CONFIG_SYS_NAND_BASE 0x02000000 #define CONFIG_SYS_NAND_HW_ECC #define CONFIG_SYS_MAX_NAND_DEVICE 1 /* Max number of NAND devices */ +#define CONFIG_MASK_CLE 0x10 +#define CONFIG_MASK_ALE 0x0a
Make it CONFIG_SYS_DAVINCI_NAND_CLE/ALE.
Also, no tab after #define.
-Scott
Scott, I will resubmit after i take care of your comments.
Thanks, Sandeep ________________________________________ From: Scott Wood [scottwood@freescale.com] Sent: Tuesday, April 28, 2009 5:22 PM To: Paulraj, Sandeep Cc: u-boot@lists.denx.de; davinci-linux-open-source@linux.davincidsp.com Subject: Re: [U-Boot] [PATCH] ARM DaVinci: Adding CONFIG options for NAND ALE and CLE
On Tue, Apr 28, 2009 at 04:33:14PM -0400, s-paulraj@ti.com wrote:
diff --git a/include/configs/davinci_dvevm.h b/include/configs/davinci_dvevm.h index fae430b..1fe3e19 100644 --- a/include/configs/davinci_dvevm.h +++ b/include/configs/davinci_dvevm.h @@ -128,6 +128,8 @@ #define CONFIG_SYS_NAND_BASE 0x02000000 #define CONFIG_SYS_NAND_HW_ECC #define CONFIG_SYS_MAX_NAND_DEVICE 1 /* Max number of NAND devices */ +#define CONFIG_MASK_CLE 0x10 +#define CONFIG_MASK_ALE 0x0a
Make it CONFIG_SYS_DAVINCI_NAND_CLE/ALE.
Also, no tab after #define.
-Scott
On Tuesday 28 April 2009, s-paulraj@ti.com wrote:
The CLE and ALE for DaVinci DM644x is not the same as DM646x. This patch adds 2 CONFIG_ options to the config.h header files to all the DM6446 based boards. These values are then used by the driver.
I had been thinking that the davinci_nand driver should probably change to let board_nand_init() really become a board-specific function...
It would call a driver routine to init the callback functions and set up private data so that for example the 2GByte NAND could be properly treated as a single device with two chips, instead of two separate devices. (As Linux does; I'm just assuming that mechanism works right in U-Boot.)
Other private data could include CLE/ALE masks, if needed. I may well have missed something, but I thought that the ROM on all chips used the same mask values when booting from NAND flash... but that non-boot chips might end up using different values, if they wanted. (Burst memory access would work better if toggling low address bits didn't affect ALE/CLE, etc.)
Alternatively ... how about just letting those values be defined in the headers if the boot-from-NAND defaults shouldn't be used?
That is, the u-boot davinci_nand.c code could
#ifndef CONFIG_SYS_DAVINCI_NAND_CLE ... #define it to the default 0x10 ... #endif
I think the convention for these things (toplevel README) is that since they're hardware-specific, CONFIG_SYS_* is the prefix not CONFIG_* for those symbols.
Related: MASK_ALE should not be 0x0a by default, but 0x08; right? That's what newer docs say; I think the old stuff was just a bug. In Linux, 0x08 works just fine.
And for that matter, include/asm-arm/arch-davinci/nand_defs.h is starting to look almost un-necessary after those cleanup patches I sent. :)
This has been tested on the DM644x, DM355, DM357 and DM365. Support for the latter 3 will be added soon.
I'm glad to see TI's latest U-Boot work becoming public...
However, this won't apply on top of the cleanup patches I just sent, which remove the NAND_CE0{ALE,CLE,DATA} symbols in favor of the relevant nand_chip pointer. As you know, that change is important for support of the EVM boards which include the socketed 2 GByte NAND chips, with multiple chip select lines.
- Dave
Dave,
Please see inline.
-----Original Message----- From: David Brownell [mailto:david-b@pacbell.net] Sent: Tuesday, April 28, 2009 5:51 PM To: Paulraj, Sandeep; u-boot@lists.denx.de Cc: davinci-linux-open-source@linux.davincidsp.com Subject: Re: [PATCH] ARM DaVinci: Adding CONFIG options for NAND ALE and CLE
On Tuesday 28 April 2009, s-paulraj@ti.com wrote:
The CLE and ALE for DaVinci DM644x is not the same as DM646x. This patch adds 2 CONFIG_ options to the config.h header files to all the DM6446
based
boards. These values are then used by the driver.
I had been thinking that the davinci_nand driver should probably change to let board_nand_init() really become a board-specific function...
It would call a driver routine to init the callback functions and set up private data so that for example the 2GByte NAND could be properly treated as a single device with two chips, instead of two separate devices. (As Linux does; I'm just assuming that mechanism works right in U-Boot.)
[Sandeep] At the U-Boot level do we really need this mechanism?
Other private data could include CLE/ALE masks, if needed.
[Sandeep] This is the way we do it in the kernel. Again does U-boot really need this much sophistication.
I may well have missed something, but I thought that the ROM on all chips used the same mask values when booting from NAND flash... but that non-boot chips might end up using different values, if they wanted. (Burst memory access would work better if toggling low address bits didn't affect ALE/CLE, etc.)
Alternatively ... how about just letting those values be defined in the headers if the boot-from-NAND defaults shouldn't be used?
That is, the u-boot davinci_nand.c code could
#ifndef CONFIG_SYS_DAVINCI_NAND_CLE ... #define it to the default 0x10 ... #endif
[Sandeep] It is my personal opinion that we should try to avoid such #ifdefs in the driver. I would just define all these in the config.h file. Let me know your opinion. We will need such a #ifndef for the CLE as well so I would just put both the values in the config.h rather than have 2 #ifndefs in the NAND driver
I think the convention for these things (toplevel README) is that since they're hardware-specific, CONFIG_SYS_* is the prefix not CONFIG_* for those symbols.
[Sandeep] will do
Related: MASK_ALE should not be 0x0a by default, but 0x08; right?
[Sandeep] That is what I use in the internal LSP releases That's what newer docs say; I think the old stuff
was just a bug. In Linux, 0x08 works just fine.
[Sandeep] Yes that is correct. I was going to send an e-mail tomorrow to see if anyone had any objections to me changing this value
And for that matter, include/asm-arm/arch-davinci/nand_defs.h is starting to look almost un-necessary after those cleanup patches I sent. :)
[Sandeep] yes I noticed that.
This has been tested on the DM644x, DM355, DM357 and DM365. Support for the latter 3 will be added soon.
[Sandeep] Its ready, it just a matter of getting the patches to apply over other patches in a form acceptable to everybody.
I'm glad to see TI's latest U-Boot work becoming public...
[Sandeep] I don't know why version 1.3.4 has not become public yet but it will be soon.
However, this won't apply on top of the cleanup patches I just sent, which remove the NAND_CE0{ALE,CLE,DATA} symbols in favor of the relevant nand_chip pointer. As you know, that change is important for support of the EVM boards which include the socketed 2 GByte NAND chips, with multiple chip select lines.
[Sandeep] yes and I will send an updated patch tomorrow.
- Dave
Hi Sandeep,
On Tuesday 28 April 2009, Paulraj, Sandeep wrote:
I had been thinking that the davinci_nand driver should probably change to let board_nand_init() really become a board-specific function...
It would call a driver routine to init the callback functions and set up private data so that for example the 2GByte NAND could be properly treated as a single device with two chips, instead of two separate devices. (As Linux does; I'm just assuming that mechanism works right in U-Boot.)
[Sandeep] At the U-Boot level do we really need this mechanism?
I think it will improve Linux interop.
For example, the MTDPART support in U-Boot provides a string that can be passed on the kernel command line to ensure that U-Boot and Linux use the same partitioning.
But ... that won't work very well if Linux sees one big device, while U-Boot sees two smaller ones. That "mtdpart=..." command line string won't work for Linux.
Other private data could include CLE/ALE masks, if needed.
[Sandeep] This is the way we do it in the kernel. Again does U-boot really need this much sophistication.
Erm, the 2.6.30 code (and davinci-git) passes the masks in through platform_data; they're not fixed at compile time.
That is, the u-boot davinci_nand.c code could
#ifndef CONFIG_SYS_DAVINCI_NAND_CLE ... #define it to the default 0x10 ... #endif
[Sandeep] It is my personal opinion that we should try to avoid such #ifdefs in the driver. I would just define all these in the config.h file. Let me know your opinion.
The place to *really* avoid #ifdefs is inside C functions. I could agree that putting such defaults into the DaVinci "nand.h" header might be prettier.
But I'd like to keep the config.h files free of such stuff. Especially, free of stuff that doesn't change from board to board (except in unusual cases).
We will need such a #ifndef for the CLE as well so I would just put both the values in the config.h rather than have 2 #ifndefs in the NAND driver
Well, "nand.h" would be better. Especially considering that board using a non-default values are uncommon.
Related: MASK_ALE should not be 0x0a by default, but 0x08; right?
[Sandeep] That is what I use in the internal LSP releases
That's what newer docs say; I think the old stuffwas just a bug. In Linux, 0x08 works just fine.
[Sandeep] Yes that is correct. I was going to send an e-mail tomorrow to see if anyone had any objections to me changing this value
Nah ... just fixit. ;)
- Dave
participants (5)
-
David Brownell -
Paulraj, Sandeep -
s-paulraj＠ti.com
-
Scott Wood -
Steve Chen