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

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

[Magnus Lilja]

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(-)

[U-Boot] [RFC 1/4] MX31: Add struct definition for clock control module in i.MX31.

[Magnus Lilja]

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)))
-- 
1.5.6

[U-Boot] [RFC 2/4] mxc_nand: Update driver to work with i.MX31.

[Magnus Lilja]

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;
-- 
1.5.6

[U-Boot] [RFC 3/4] MX31: Activate NAND support for i.MX31 Litekit board.

[Magnus Lilja]

---
 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 */
-- 
1.5.6

[U-Boot] [RFC 4/4] MXC: Reorganize 16 bit nand detection.

[Magnus Lilja]

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)))
-- 
1.5.6

[U-Boot] [RFC 4/4] MXC: Reorganize 16 bit nand detection.

[Scott Wood]

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 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

It looks like you put the MX31 version in the MX27 file as well...

-Scott

[U-Boot] [RFC 4/4] MXC: Reorganize 16 bit nand detection.

[Magnus Lilja]

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 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
> 
> 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