[U-Boot] [RFC 0/4] MXC: Add NAND support for i.MX31

Hi all,
This series adds NAND support for i.MX31 using the mxc_nand that was added for i.MX27. The same NAND Flash Controller is used in i.MX31.
I've done some limited run-time testing on the Litekit using small page NAND and it seems to work.
I have embedded a question in patch #2 and #4, this relates to where the 16 bit SoC specific auto detection code shall be placed.
Once I've received input on the open question I will clean up the series and do the necessary MAKEALL tests (not done at the moment), add signed-off-by and re-submit.
On a separate note, the Linux version of mxc_nand.c is undergoing updates (don't think the patches have been accepted yet) and unfortunately it's not just a matter of copying the new version since the uboot version was converted to use readw(&host->regs->register) instead of readw(host->regs + REGISTER). But in any case, there may be things to import to u-boot.
Regards, Magnus
Magnus Lilja (4): MX31: Add struct definition for clock control module in i.MX31. mxc_nand: Update driver to work with i.MX31. MX31: Activate NAND support for i.MX31 Litekit board. MXC: Reorganize 16 bit nand detection.
drivers/mtd/nand/mxc_nand.c | 4 +-- include/asm-arm/arch-mx27/imx-regs.h | 13 ++++++++ include/asm-arm/arch-mx31/mx31-regs.h | 54 +++++++++++++++++++++++++++++++++ include/configs/imx31_litekit.h | 11 +++++++ 4 files changed, 79 insertions(+), 3 deletions(-)

Comment: The struct is called system_control_registers only because the mxc_nand.c uses that name. For i.MX31 these registers are called "Clock Control Registers" so the struct name should be clock_control_registers, while for i.MX27 they are called "System Control Registers". --- include/asm-arm/arch-mx31/mx31-regs.h | 41 +++++++++++++++++++++++++++++++++ 1 files changed, 41 insertions(+), 0 deletions(-)
diff --git a/include/asm-arm/arch-mx31/mx31-regs.h b/include/asm-arm/arch-mx31/mx31-regs.h index 51b02a2..86c9975 100644 --- a/include/asm-arm/arch-mx31/mx31-regs.h +++ b/include/asm-arm/arch-mx31/mx31-regs.h @@ -24,6 +24,47 @@ #ifndef __ASM_ARCH_MX31_REGS_H #define __ASM_ARCH_MX31_REGS_H
+#if !(defined(__KERNEL_STRICT_NAMES) || defined(__ASSEMBLY__)) +#include <asm/types.h> + +/* Clock control module registers */ +struct system_control_regs { + u32 ccmr; + u32 pdr0; + u32 pdr1; + u32 rcsr; + u32 mpctl; + u32 upctl; + u32 spctl; + u32 cosr; + u32 cgr0; + u32 cgr1; + u32 cgr2; + u32 wimr0; + u32 ldc; + u32 dcvr0; + u32 dcvr1; + u32 dcvr2; + u32 dcvr3; + u32 ltr0; + u32 ltr1; + u32 ltr2; + u32 ltr3; + u32 ltbr0; + u32 ltbr1; + u32 pmcr0; + u32 pmcr1; + u32 pdr2; +}; + +#define IMX_SYSTEM_CTL_BASE (0x53F80000) + +/* Bit definitions for RCSR register in CCM */ +#define CCM_RCSR_NF16B (1 << 31) +#define CCM_RCSR_NFMS (1 << 30) + +#endif + #define __REG(x) (*((volatile u32 *)(x))) #define __REG16(x) (*((volatile u16 *)(x))) #define __REG8(x) (*((volatile u8 *)(x)))

Comment: Given how mxc_nand.c looks like (it was written with i.MX27 in mind), this is the straight forward way of adding i.MX31 support. Personally I don't like the #ifdef's and prefer the solution presented in a later patch in this series. --- drivers/mtd/nand/mxc_nand.c | 8 +++++++- 1 files changed, 7 insertions(+), 1 deletions(-)
diff --git a/drivers/mtd/nand/mxc_nand.c b/drivers/mtd/nand/mxc_nand.c index eb0323f..3e4254a 100644 --- a/drivers/mtd/nand/mxc_nand.c +++ b/drivers/mtd/nand/mxc_nand.c @@ -871,9 +871,15 @@ int board_nand_init(struct nand_chip *this) writew(0x4, &host->regs->nfc_wrprot);
/* NAND bus width determines access funtions used by upper layer */ +#ifdef CONFIG_MX27 if (readl(&sc_regs->fmcr) & NF_16BIT_SEL) this->options |= NAND_BUSWIDTH_16; - +#elif defined(CONFIG_MX31) + if (readl(&sc_regs->rcsr) & CCM_RCSR_NF16B) + this->options |= NAND_BUSWIDTH_16; +#else +#warning "No autodetection of 8/16 bit NAND" +#endif host->pagesize_2k = 0;
return err;

--- include/configs/imx31_litekit.h | 11 +++++++++++ 1 files changed, 11 insertions(+), 0 deletions(-)
diff --git a/include/configs/imx31_litekit.h b/include/configs/imx31_litekit.h index 6131008..7e8ddbb 100644 --- a/include/configs/imx31_litekit.h +++ b/include/configs/imx31_litekit.h @@ -89,6 +89,7 @@ #define CONFIG_CMD_PING #define CONFIG_CMD_SPI #define CONFIG_CMD_DATE +#define CONFIG_CMD_NAND
#define CONFIG_BOOTDELAY 3
@@ -174,4 +175,14 @@ #undef CONFIG_CMD_MTDPARTS #define CONFIG_JFFS2_DEV "nor0"
+/* + * NAND + */ +#define CONFIG_NAND_MXC +#define CONFIG_MXC_NAND_REGS_BASE NFC_BASE_ADDR +#define CONFIG_SYS_MAX_NAND_DEVICE 1 +#define CONFIG_SYS_NAND_BASE NFC_BASE_ADDR +#define CONFIG_JFFS2_NAND +#define CONFIG_MXC_NAND_HWECC + #endif /* __CONFIG_H */

Alternative solution for supporting 16 bit NAND detection for the i.MX27 and i.MX31 SoCs. This moves the SoC specific code to the SoC header file leaving mxc_nand.c free from #ifdef's (in this respect).
Question: Is this approach acceptable/preferred over having #ifdef's for different SoCs in mxc_nand.c? --- drivers/mtd/nand/mxc_nand.c | 12 ++---------- include/asm-arm/arch-mx27/imx-regs.h | 13 +++++++++++++ include/asm-arm/arch-mx31/mx31-regs.h | 13 +++++++++++++ 3 files changed, 28 insertions(+), 10 deletions(-)
diff --git a/drivers/mtd/nand/mxc_nand.c b/drivers/mtd/nand/mxc_nand.c index 3e4254a..45d0024 100644 --- a/drivers/mtd/nand/mxc_nand.c +++ b/drivers/mtd/nand/mxc_nand.c @@ -808,8 +808,6 @@ static void mxc_nand_command(struct mtd_info *mtd, unsigned command,
int board_nand_init(struct nand_chip *this) { - struct system_control_regs *sc_regs = - (struct system_control_regs *)IMX_SYSTEM_CTL_BASE; struct mtd_info *mtd; uint16_t tmp; int err = 0; @@ -871,15 +869,9 @@ int board_nand_init(struct nand_chip *this) writew(0x4, &host->regs->nfc_wrprot);
/* NAND bus width determines access funtions used by upper layer */ -#ifdef CONFIG_MX27 - if (readl(&sc_regs->fmcr) & NF_16BIT_SEL) - this->options |= NAND_BUSWIDTH_16; -#elif defined(CONFIG_MX31) - if (readl(&sc_regs->rcsr) & CCM_RCSR_NF16B) + if (mxc_nand_is_16bit()) this->options |= NAND_BUSWIDTH_16; -#else -#warning "No autodetection of 8/16 bit NAND" -#endif + host->pagesize_2k = 0;
return err; diff --git a/include/asm-arm/arch-mx27/imx-regs.h b/include/asm-arm/arch-mx27/imx-regs.h index d36a6da..b88fe51 100644 --- a/include/asm-arm/arch-mx27/imx-regs.h +++ b/include/asm-arm/arch-mx27/imx-regs.h @@ -516,4 +516,17 @@ struct iim_regs { #define IIM0_SCC_KEY 11 #define IIM1_SUID 1
+#ifndef __ASSEMBLY__ +static inline int mxc_nand_is_16bit(void) +{ + struct system_control_regs *sc_regs = + (struct system_control_regs *)IMX_SYSTEM_CTL_BASE; + + if (readl(&sc_regs->rcsr) & CCM_RCSR_NF16B) + return 1; + else + return 0; +} +#endif + #endif /* _IMX_REGS_H */ diff --git a/include/asm-arm/arch-mx31/mx31-regs.h b/include/asm-arm/arch-mx31/mx31-regs.h index 86c9975..613c632 100644 --- a/include/asm-arm/arch-mx31/mx31-regs.h +++ b/include/asm-arm/arch-mx31/mx31-regs.h @@ -63,6 +63,19 @@ struct system_control_regs { #define CCM_RCSR_NF16B (1 << 31) #define CCM_RCSR_NFMS (1 << 30)
+#include <asm/io.h> + +static inline int mxc_nand_is_16bit(void) +{ + struct system_control_regs *sc_regs = + (struct system_control_regs *)IMX_SYSTEM_CTL_BASE; + + if (readl(&sc_regs->rcsr) & CCM_RCSR_NF16B) + return 1; + else + return 0; +} + #endif
#define __REG(x) (*((volatile u32 *)(x)))

On Sun, Nov 08, 2009 at 11:55:39AM +0100, Magnus Lilja wrote:
Alternative solution for supporting 16 bit NAND detection for the i.MX27 and i.MX31 SoCs. This moves the SoC specific code to the SoC header file leaving mxc_nand.c free from #ifdef's (in this respect).
OTOH, it moves more NAND stuff out of the NAND driver.
I guess it depends on how many more such alternatives you think will be added in the future.
diff --git a/drivers/mtd/nand/mxc_nand.c b/drivers/mtd/nand/mxc_nand.c index 3e4254a..45d0024 100644 --- a/drivers/mtd/nand/mxc_nand.c +++ b/drivers/mtd/nand/mxc_nand.c @@ -808,8 +808,6 @@ static void mxc_nand_command(struct mtd_info *mtd, unsigned command,
int board_nand_init(struct nand_chip *this) {
- struct system_control_regs *sc_regs =
struct mtd_info *mtd; uint16_t tmp; int err = 0;(struct system_control_regs *)IMX_SYSTEM_CTL_BASE;
@@ -871,15 +869,9 @@ int board_nand_init(struct nand_chip *this) writew(0x4, &host->regs->nfc_wrprot);
/* NAND bus width determines access funtions used by upper layer */ -#ifdef CONFIG_MX27
- if (readl(&sc_regs->fmcr) & NF_16BIT_SEL)
this->options |= NAND_BUSWIDTH_16;
-#elif defined(CONFIG_MX31)
- if (readl(&sc_regs->rcsr) & CCM_RCSR_NF16B)
- if (mxc_nand_is_16bit()) this->options |= NAND_BUSWIDTH_16;
-#else -#warning "No autodetection of 8/16 bit NAND" -#endif
host->pagesize_2k = 0;
return err;
diff --git a/include/asm-arm/arch-mx27/imx-regs.h b/include/asm-arm/arch-mx27/imx-regs.h index d36a6da..b88fe51 100644 --- a/include/asm-arm/arch-mx27/imx-regs.h +++ b/include/asm-arm/arch-mx27/imx-regs.h @@ -516,4 +516,17 @@ struct iim_regs { #define IIM0_SCC_KEY 11 #define IIM1_SUID 1
+#ifndef __ASSEMBLY__ +static inline int mxc_nand_is_16bit(void) +{
- struct system_control_regs *sc_regs =
(struct system_control_regs *)IMX_SYSTEM_CTL_BASE;
- if (readl(&sc_regs->rcsr) & CCM_RCSR_NF16B)
return 1;
- else
return 0;
+} +#endif
#endif /* _IMX_REGS_H */ diff --git a/include/asm-arm/arch-mx31/mx31-regs.h b/include/asm-arm/arch-mx31/mx31-regs.h index 86c9975..613c632 100644 --- a/include/asm-arm/arch-mx31/mx31-regs.h +++ b/include/asm-arm/arch-mx31/mx31-regs.h @@ -63,6 +63,19 @@ struct system_control_regs { #define CCM_RCSR_NF16B (1 << 31) #define CCM_RCSR_NFMS (1 << 30)
+#include <asm/io.h>
+static inline int mxc_nand_is_16bit(void) +{
- struct system_control_regs *sc_regs =
(struct system_control_regs *)IMX_SYSTEM_CTL_BASE;
- if (readl(&sc_regs->rcsr) & CCM_RCSR_NF16B)
return 1;
- else
return 0;
+}
#endif
It looks like you put the MX31 version in the MX27 file as well...
-Scott

Scott Wood skrev:
On Sun, Nov 08, 2009 at 11:55:39AM +0100, Magnus Lilja wrote:
Alternative solution for supporting 16 bit NAND detection for the i.MX27 and i.MX31 SoCs. This moves the SoC specific code to the SoC header file leaving mxc_nand.c free from #ifdef's (in this respect).
OTOH, it moves more NAND stuff out of the NAND driver.
I guess it depends on how many more such alternatives you think will be added in the future.
I'll figure something out that keeps the NAND stuff in mxc_nand.c and repost.
diff --git a/drivers/mtd/nand/mxc_nand.c b/drivers/mtd/nand/mxc_nand.c index 3e4254a..45d0024 100644 --- a/drivers/mtd/nand/mxc_nand.c +++ b/drivers/mtd/nand/mxc_nand.c @@ -808,8 +808,6 @@ static void mxc_nand_command(struct mtd_info *mtd, unsigned command,
int board_nand_init(struct nand_chip *this) {
- struct system_control_regs *sc_regs =
struct mtd_info *mtd; uint16_t tmp; int err = 0;(struct system_control_regs *)IMX_SYSTEM_CTL_BASE;
@@ -871,15 +869,9 @@ int board_nand_init(struct nand_chip *this) writew(0x4, &host->regs->nfc_wrprot);
/* NAND bus width determines access funtions used by upper layer */ -#ifdef CONFIG_MX27
- if (readl(&sc_regs->fmcr) & NF_16BIT_SEL)
this->options |= NAND_BUSWIDTH_16;
-#elif defined(CONFIG_MX31)
- if (readl(&sc_regs->rcsr) & CCM_RCSR_NF16B)
- if (mxc_nand_is_16bit()) this->options |= NAND_BUSWIDTH_16;
-#else -#warning "No autodetection of 8/16 bit NAND" -#endif
host->pagesize_2k = 0;
return err;
diff --git a/include/asm-arm/arch-mx27/imx-regs.h b/include/asm-arm/arch-mx27/imx-regs.h index d36a6da..b88fe51 100644 --- a/include/asm-arm/arch-mx27/imx-regs.h +++ b/include/asm-arm/arch-mx27/imx-regs.h @@ -516,4 +516,17 @@ struct iim_regs { #define IIM0_SCC_KEY 11 #define IIM1_SUID 1
+#ifndef __ASSEMBLY__ +static inline int mxc_nand_is_16bit(void) +{
- struct system_control_regs *sc_regs =
(struct system_control_regs *)IMX_SYSTEM_CTL_BASE;
- if (readl(&sc_regs->rcsr) & CCM_RCSR_NF16B)
return 1;
- else
return 0;
+} +#endif
#endif /* _IMX_REGS_H */ diff --git a/include/asm-arm/arch-mx31/mx31-regs.h b/include/asm-arm/arch-mx31/mx31-regs.h index 86c9975..613c632 100644 --- a/include/asm-arm/arch-mx31/mx31-regs.h +++ b/include/asm-arm/arch-mx31/mx31-regs.h @@ -63,6 +63,19 @@ struct system_control_regs { #define CCM_RCSR_NF16B (1 << 31) #define CCM_RCSR_NFMS (1 << 30)
+#include <asm/io.h>
+static inline int mxc_nand_is_16bit(void) +{
- struct system_control_regs *sc_regs =
(struct system_control_regs *)IMX_SYSTEM_CTL_BASE;
- if (readl(&sc_regs->rcsr) & CCM_RCSR_NF16B)
return 1;
- else
return 0;
+}
#endif
It looks like you put the MX31 version in the MX27 file as well...
Heh, thanks. Will take of that.
Thanks for the feedback
/Magnus
participants (2)
-
Magnus Lilja
-
Scott Wood