[U-Boot] [patch] dm355evm NAND support

[U-Boot] [patch] dm355evm NAND support

[David Brownell]

This is the second half of my DM355 EVM support patches, adding
the NAND support now that the 4-bit ECC is merged:

 - Kick in NAND support, enabling
     * the DaVinci NAND driver
     * its 4-bit ECC support
     * MTD_DEVICE (newish, should be automatic!)
     * 64-bit printf (newish, should be automatic!)
     * saving the environment in one NAND block
 - Configure for the 2GB SLC flash normally shipped with the EVM
     * move all sizing info to the end
     * LARGEPAGE option is gone
     * use first block for environment (it's otherwise unused)
     * ... if small page NAND is used, use 1 MByte bootloader area
 - Enable default 5 second bootdelay

And a build fix:  set_bit()/clear_bit() param types changed.

Supporting 2GB MLC chips would need (a) 256K blocks (b) bigger
malloc heap and (c) 4K pages ... with that last requiring NAND
core changes, to support 80 bytes of ECC data.

Signed-off-by: David Brownell <dbrownell@users.sourceforge.net>
---
DIFFERS FROM SANDEEP'S PATCH:  (a) 64-bit VSPRINTf, (b) no MLC hooks,
(c) environment in block 0, which would otherwise be wasted, (d) no
dependency on dubious "remove SZ_* symbols" patches, (e) buildfix

 board/davinci/dm355evm/dm355evm.c  |    4 +--
 include/configs/davinci_dm355evm.h |   37 +++++++++++++++++++++--------------
 2 files changed, 25 insertions(+), 16 deletions(-)

--- a/board/davinci/dm355evm/dm355evm.c
+++ b/board/davinci/dm355evm/dm355evm.c
@@ -92,8 +92,8 @@ int board_eth_init(bd_t *bis)
 static void nand_dm355evm_select_chip(struct mtd_info *mtd, int chip)
 {
 	struct nand_chip	*this = mtd->priv;
-	u32			wbase = (u32) this->IO_ADDR_W;
-	u32			rbase = (u32) this->IO_ADDR_R;
+	unsigned long		wbase = (unsigned long) this->IO_ADDR_W;
+	unsigned long		rbase = (unsigned long) this->IO_ADDR_R;
 
 	if (chip == 1) {
 		__set_bit(14, &wbase);
--- a/include/configs/davinci_dm355evm.h
+++ b/include/configs/davinci_dm355evm.h
@@ -29,6 +29,7 @@
 #define CONFIG_SYS_NO_FLASH		/* that is, no *NOR* flash */
 #define CONFIG_SYS_CONSOLE_INFO_QUIET
 #define CONFIG_DISPLAY_CPUINFO
+#define CONFIG_SYS_64BIT_VSPRINTF	/* needed for nand_util.c */
 
 /* SoC Configuration */
 #define CONFIG_ARM926EJS				/* arm926ejs CPU */
@@ -66,11 +67,10 @@
 #define CONFIG_SYS_I2C_SLAVE		0x10	/* SMBus host address */
 
 /* NAND: socketed, two chipselects, normally 2 GBytes */
-/* NYET -- #define CONFIG_NAND_DAVINCI */
-#define CONFIG_SYS_NAND_HW_ECC
+#define CONFIG_NAND_DAVINCI
+#define CONFIG_SYS_NAND_4BIT_HW_ECC_OOBFIRST
 #define CONFIG_SYS_NAND_USE_FLASH_BBT
 
-#define CONFIG_SYS_NAND_LARGEPAGE
 #define CONFIG_SYS_NAND_BASE_LIST	{ 0x02000000, }
 /* socket has two chipselects, nCE0 gated by address BIT(14) */
 #define CONFIG_SYS_MAX_NAND_DEVICE	1
@@ -95,16 +95,14 @@
 
 #ifdef CONFIG_NAND_DAVINCI
 #define CONFIG_CMD_MTDPARTS
+#define CONFIG_MTD_DEVICE
 #define CONFIG_MTD_PARTITIONS
 #define CONFIG_CMD_NAND
 #define CONFIG_CMD_UBI
 #define CONFIG_RBTREE
+#define CONFIG_ENV_IS_IN_NAND
 #endif
 
-/* TEMPORARY -- no safe place to save env, yet */
-#define CONFIG_ENV_IS_NOWHERE
-#undef CONFIG_CMD_SAVEENV
-
 #ifdef CONFIG_USB_DAVINCI
 #define CONFIG_MUSB_HCD
 #define CONFIG_CMD_USB
@@ -130,9 +128,7 @@
 #define CONFIG_SYS_PROMPT_HUSH_PS2	"> "
 #define CONFIG_SYS_LONGHELP
 
-#define CONFIG_ENV_SIZE		SZ_16K
-
-/* NYET -- #define CONFIG_BOOTDELAY	5 */
+#define CONFIG_BOOTDELAY	5
 #define CONFIG_BOOTCOMMAND \
 		"dhcp;bootm"
 #define CONFIG_BOOTARGS \
@@ -174,14 +170,27 @@
  */
 #define MTDIDS_DEFAULT		"nand0=davinci_nand.0"
 
-#ifdef CONFIG_SYS_NAND_LARGEPAGE
-/*  Use same layout for 128K/256K blocks; allow some bad blocks */
+/* default: 2GByte SLC; large page (2KB), 128KB blocks */
+#define CONFIG_SYS_NAND_BLOCKSIZE	(128 * 1024)
+#define CONFIG_SYS_NAND_PAGE_2K
+#define NAND_LARGE_PAGE
+
+/* No support _yet_ for OOB layouts needed by U-Boot + 4-bit ECC with:
+ * (a) 512 byte small page chips ... driver just needs new oob struct
+ * (b) 4K large page chips, MLC ... NAND core can't handle 80 bytes ECC
+ */
+
+#ifdef NAND_LARGE_PAGE
+/* Use same layout for 128K/256K blocks; allow for some bad blocks */
 #define PART_BOOT		"2m(bootloader)ro,"
-#else
+#else /* small page */
 /* Assume 16K erase blocks; allow a few bad ones. */
-#define PART_BOOT		"512k(bootloader)ro,"
+#define PART_BOOT		"1m(bootloader)ro,"
 #endif
 
+#define CONFIG_ENV_OFFSET	0		/* block 0 not used by UBL */
+#define CONFIG_ENV_SIZE		(16 * 1024)
+
 #define PART_KERNEL		"4m(kernel),"	/* kernel + initramfs */
 #define PART_REST		"-(filesystem)"
 

[U-Boot] [patch] dm355evm NAND support

[Tom]

David Brownell wrote:
> This is the second half of my DM355 EVM support patches, adding
> the NAND support now that the 4-bit ECC is merged:
> 
>  - Kick in NAND support, enabling
>      * the DaVinci NAND driver
>      * its 4-bit ECC support
>      * MTD_DEVICE (newish, should be automatic!)
>      * 64-bit printf (newish, should be automatic!)
>      * saving the environment in one NAND block
>  - Configure for the 2GB SLC flash normally shipped with the EVM
>      * move all sizing info to the end
>      * LARGEPAGE option is gone
>      * use first block for environment (it's otherwise unused)
>      * ... if small page NAND is used, use 1 MByte bootloader area
>  - Enable default 5 second bootdelay
> 
> And a build fix:  set_bit()/clear_bit() param types changed.
> 
> Supporting 2GB MLC chips would need (a) 256K blocks (b) bigger
> malloc heap and (c) 4K pages ... with that last requiring NAND
> core changes, to support 80 bytes of ECC data.
> 
> Signed-off-by: David Brownell <dbrownell@users.sourceforge.net>
> ---
> DIFFERS FROM SANDEEP'S PATCH:  (a) 64-bit VSPRINTf, (b) no MLC hooks,
> (c) environment in block 0, which would otherwise be wasted, (d) no
> dependency on dubious "remove SZ_* symbols" patches, (e) buildfix
> 
>  board/davinci/dm355evm/dm355evm.c  |    4 +--
>  include/configs/davinci_dm355evm.h |   37 +++++++++++++++++++++--------------
>  2 files changed, 25 insertions(+), 16 deletions(-)
> 
> --- a/board/davinci/dm355evm/dm355evm.c
> +++ b/board/davinci/dm355evm/dm355evm.c
> @@ -92,8 +92,8 @@ int board_eth_init(bd_t *bis)
>  static void nand_dm355evm_select_chip(struct mtd_info *mtd, int chip)
>  {
>  	struct nand_chip	*this = mtd->priv;
> -	u32			wbase = (u32) this->IO_ADDR_W;
> -	u32			rbase = (u32) this->IO_ADDR_R;
> +	unsigned long		wbase = (unsigned long) this->IO_ADDR_W;
> +	unsigned long		rbase = (unsigned long) this->IO_ADDR_R;
>  

Nak.
I have already ack-ed Sandeep's patch that contains this
fix for the warning.  Please check with him.

In general it is better to break patches that do multiple things into
multiple patches.  When you resubmit, please break this patch into its
logical parts :
1. NAND
2. Environment
3. Bootdelay

Tom

>  	if (chip == 1) {
>  		__set_bit(14, &wbase);
> --- a/include/configs/davinci_dm355evm.h
> +++ b/include/configs/davinci_dm355evm.h
> @@ -29,6 +29,7 @@
>  #define CONFIG_SYS_NO_FLASH		/* that is, no *NOR* flash */
>  #define CONFIG_SYS_CONSOLE_INFO_QUIET
>  #define CONFIG_DISPLAY_CPUINFO
> +#define CONFIG_SYS_64BIT_VSPRINTF	/* needed for nand_util.c */
>  
>  /* SoC Configuration */
>  #define CONFIG_ARM926EJS				/* arm926ejs CPU */
> @@ -66,11 +67,10 @@
>  #define CONFIG_SYS_I2C_SLAVE		0x10	/* SMBus host address */
>  
>  /* NAND: socketed, two chipselects, normally 2 GBytes */
> -/* NYET -- #define CONFIG_NAND_DAVINCI */
> -#define CONFIG_SYS_NAND_HW_ECC
> +#define CONFIG_NAND_DAVINCI
> +#define CONFIG_SYS_NAND_4BIT_HW_ECC_OOBFIRST
>  #define CONFIG_SYS_NAND_USE_FLASH_BBT
>  
> -#define CONFIG_SYS_NAND_LARGEPAGE
>  #define CONFIG_SYS_NAND_BASE_LIST	{ 0x02000000, }
>  /* socket has two chipselects, nCE0 gated by address BIT(14) */
>  #define CONFIG_SYS_MAX_NAND_DEVICE	1
> @@ -95,16 +95,14 @@
>  
>  #ifdef CONFIG_NAND_DAVINCI
>  #define CONFIG_CMD_MTDPARTS
> +#define CONFIG_MTD_DEVICE
>  #define CONFIG_MTD_PARTITIONS
>  #define CONFIG_CMD_NAND
>  #define CONFIG_CMD_UBI
>  #define CONFIG_RBTREE
> +#define CONFIG_ENV_IS_IN_NAND
>  #endif
>  
> -/* TEMPORARY -- no safe place to save env, yet */
> -#define CONFIG_ENV_IS_NOWHERE
> -#undef CONFIG_CMD_SAVEENV
> -
>  #ifdef CONFIG_USB_DAVINCI
>  #define CONFIG_MUSB_HCD
>  #define CONFIG_CMD_USB
> @@ -130,9 +128,7 @@
>  #define CONFIG_SYS_PROMPT_HUSH_PS2	"> "
>  #define CONFIG_SYS_LONGHELP
>  
> -#define CONFIG_ENV_SIZE		SZ_16K
> -
> -/* NYET -- #define CONFIG_BOOTDELAY	5 */
> +#define CONFIG_BOOTDELAY	5
>  #define CONFIG_BOOTCOMMAND \
>  		"dhcp;bootm"
>  #define CONFIG_BOOTARGS \
> @@ -174,14 +170,27 @@
>   */
>  #define MTDIDS_DEFAULT		"nand0=davinci_nand.0"
>  
> -#ifdef CONFIG_SYS_NAND_LARGEPAGE
> -/*  Use same layout for 128K/256K blocks; allow some bad blocks */
> +/* default: 2GByte SLC; large page (2KB), 128KB blocks */
> +#define CONFIG_SYS_NAND_BLOCKSIZE	(128 * 1024)
> +#define CONFIG_SYS_NAND_PAGE_2K
> +#define NAND_LARGE_PAGE
> +
> +/* No support _yet_ for OOB layouts needed by U-Boot + 4-bit ECC with:
> + * (a) 512 byte small page chips ... driver just needs new oob struct
> + * (b) 4K large page chips, MLC ... NAND core can't handle 80 bytes ECC
> + */
> +
> +#ifdef NAND_LARGE_PAGE
> +/* Use same layout for 128K/256K blocks; allow for some bad blocks */
>  #define PART_BOOT		"2m(bootloader)ro,"
> -#else
> +#else /* small page */
>  /* Assume 16K erase blocks; allow a few bad ones. */
> -#define PART_BOOT		"512k(bootloader)ro,"
> +#define PART_BOOT		"1m(bootloader)ro,"
>  #endif
>  
> +#define CONFIG_ENV_OFFSET	0		/* block 0 not used by UBL */
> +#define CONFIG_ENV_SIZE		(16 * 1024)
> +
>  #define PART_KERNEL		"4m(kernel),"	/* kernel + initramfs */
>  #define PART_REST		"-(filesystem)"
>  
> _______________________________________________
> U-Boot mailing list
> U-Boot at lists.denx.de
> http://lists.denx.de/mailman/listinfo/u-boot

[U-Boot] [patch] dm355evm NAND support

[Paulraj, Sandeep]

> 
> David Brownell wrote:
> > This is the second half of my DM355 EVM support patches, adding
> > the NAND support now that the 4-bit ECC is merged:
> >
> >  - Kick in NAND support, enabling
> >      * the DaVinci NAND driver
> >      * its 4-bit ECC support
> >      * MTD_DEVICE (newish, should be automatic!)
> >      * 64-bit printf (newish, should be automatic!)
> >      * saving the environment in one NAND block
> >  - Configure for the 2GB SLC flash normally shipped with the EVM
> >      * move all sizing info to the end
> >      * LARGEPAGE option is gone
> >      * use first block for environment (it's otherwise unused)
> >      * ... if small page NAND is used, use 1 MByte bootloader area
> >  - Enable default 5 second bootdelay
> >
> > And a build fix:  set_bit()/clear_bit() param types changed.
> >
> > Supporting 2GB MLC chips would need (a) 256K blocks (b) bigger
> > malloc heap and (c) 4K pages ... with that last requiring NAND
> > core changes, to support 80 bytes of ECC data.
> >
> > Signed-off-by: David Brownell <dbrownell@users.sourceforge.net>
> > ---
> > DIFFERS FROM SANDEEP'S PATCH:  (a) 64-bit VSPRINTf, (b) no MLC hooks,
> > (c) environment in block 0, which would otherwise be wasted, (d) no
> > dependency on dubious "remove SZ_* symbols" patches, (e) buildfix
> >
> >  board/davinci/dm355evm/dm355evm.c  |    4 +--
> >  include/configs/davinci_dm355evm.h |   37 +++++++++++++++++++++--------
> ------
> >  2 files changed, 25 insertions(+), 16 deletions(-)
> >
> > --- a/board/davinci/dm355evm/dm355evm.c
> > +++ b/board/davinci/dm355evm/dm355evm.c
> > @@ -92,8 +92,8 @@ int board_eth_init(bd_t *bis)
> >  static void nand_dm355evm_select_chip(struct mtd_info *mtd, int chip)
> >  {
> >  	struct nand_chip	*this = mtd->priv;
> > -	u32			wbase = (u32) this->IO_ADDR_W;
> > -	u32			rbase = (u32) this->IO_ADDR_R;
> > +	unsigned long		wbase = (unsigned long) this->IO_ADDR_W;
> > +	unsigned long		rbase = (unsigned long) this->IO_ADDR_R;
> >
> 
> Nak.
> I have already ack-ed Sandeep's patch that contains this
> fix for the warning.  Please check with him.
That is correct, I did not add it to my tree because you ACK'ed this patch only after I sent a pull request. So obviously I cannot add a patch that has been ACK'ed to an already existing pull request.
This will be part of my next pull request which will have a similar fix for DM365 and hopefully the EMAC support for DM365 which should result in a fully functional DM365 EVM support.
> 
> In general it is better to break patches that do multiple things into
> multiple patches.  When you resubmit, please break this patch into its
> logical parts :
> 1. NAND
> 2. Environment
> 3. Bootdelay
> 
> Tom

If the u-boot-ti tree or the u-boot-arm tree is checked, all the above features which are being added are already in both trees.
When Tom sends a pull request to Wolfgang it should become part of Wolfgang's tree as well.

Afcourse it does not have the 64 bit VSPRINTf for which I was going to submit a patch anyway.

Thanks,
Sandeep

[U-Boot] [patch] dm355evm NAND support

[David Brownell]

On Monday 05 October 2009, Tom wrote:

> In general it is better to break patches that do multiple things into
> multiple patches.  When you resubmit, please break this patch into its
> logical parts :
> 1. NAND
> 2. Environment

Hmm, my *original* patch necessarily disabled both
because there was no persistent store to hold the
site-appropriate environment without NAND support.
I was waiting for the 4-bit ECC stuff to merge.

So I don't see those two as being cleanly separable.


> 3. Bootdelay

That makes some sense.  Though again, the original
reason for disabling it was the absense of persistent
storage to hold 


However, there's evidently been some other stuff
going on since I prepared this patch, so I'll follow
up to Sandeep's post.

- Dave

[U-Boot] [patch] dm355evm NAND support

[David Brownell]

On Monday 05 October 2009, Paulraj, Sandeep wrote:

> > I have already ack-ed Sandeep's patch that contains this
> > fix for the warning.  Please check with him.
>
> That is correct, I did not add it to my tree because you ACK'ed
> this patch only after I sent a pull request. So obviously I cannot
> add a patch that has been ACK'ed to an already existing pull request.

A "pull <this ID>" request wouldn't have been changed by
adding another commit to that tree.  You could however
have sent an updated pull request, with both.

That would result in a tree that *builds* properly...

 
> This will be part of my next pull request which will have a similar
> fix for DM365 and hopefully the EMAC support for DM365 which should
> result in a fully functional DM365 EVM support.  

That would be nice.  I'll still want the updated CPLD bits,
which pass SRST through from the JTAG adapter though; that
is obviously not a U-Boot issue.  ;)


> > In general it is better to break patches that do multiple things into
> > multiple patches.  When you resubmit, please break this patch into its
> > logical parts :
> > 1. NAND
> > 2. Environment
> > 3. Bootdelay
> > 
> > Tom
> 
> If the u-boot-ti tree or the u-boot-arm tree is checked, all the above 
> features which are being added are already in both trees. 

I guess that happened after I prepared the patch but before I sent
it in.  I'll look; there were some differences still.  Notably to
store the environment in the otherwise-unused block zero, and work
better with the small-page NANDs I've got handy.


> When Tom sends a pull request to Wolfgang it should become part of
> Wolfgang's tree as well. 
> 
> Afcourse it does not have the 64 bit VSPRINTf for which I was
> going to submit a patch anyway. 

That's important ... it doesn't work right without that patch.
When you erase or protect blocks, the diagnostics are broken
since they give bogus addresses.

- Dave

[U-Boot] [patch] dm355evm NAND support

[Paulraj, Sandeep]

> 
> > > I have already ack-ed Sandeep's patch that contains this
> > > fix for the warning.  Please check with him.
> >
> > That is correct, I did not add it to my tree because you ACK'ed
> > this patch only after I sent a pull request. So obviously I cannot
> > add a patch that has been ACK'ed to an already existing pull request.
> 
> A "pull <this ID>" request wouldn't have been changed by
> adding another commit to that tree.  You could however
> have sent an updated pull request, with both.

In hindsight yes. But since Tom had already acknowledged the request I chose not to send an update request 

> 
> That would result in a tree that *builds* properly...
> 
> 
> > This will be part of my next pull request which will have a similar
> > fix for DM365 and hopefully the EMAC support for DM365 which should
> > result in a fully functional DM365 EVM support.
> 
> That would be nice.  I'll still want the updated CPLD bits,
> which pass SRST through from the JTAG adapter though; that
> is obviously not a U-Boot issue.  ;)
> 
> 
> > > In general it is better to break patches that do multiple things into
> > > multiple patches.  When you resubmit, please break this patch into its
> > > logical parts :
> > > 1. NAND
> > > 2. Environment
> > > 3. Bootdelay
> > >
> > > Tom
> >
> > If the u-boot-ti tree or the u-boot-arm tree is checked, all the above
> > features which are being added are already in both trees.
> 
> I guess that happened after I prepared the patch but before I sent
> it in.  I'll look; there were some differences still.  Notably to
> store the environment in the otherwise-unused block zero,
That is because most users who have used existing TI binaries have the env stored at 0x3c0000. So if the update the U-Boot binary, they will still get all their env back.
IIRC this was a customer requirement of ours.

> and work
> better with the small-page NANDs I've got handy.
That is saw and I agree with you
> 
> 
> > When Tom sends a pull request to Wolfgang it should become part of
> > Wolfgang's tree as well.
> >
> > Afcourse it does not have the 64 bit VSPRINTf for which I was
> > going to submit a patch anyway.
> 
> That's important ... it doesn't work right without that patch.
> When you erase or protect blocks, the diagnostics are broken
> since they give bogus addresses.
I have noticed that. Also while trying to erase, if we come across a bad block, we will get something like "Bad Block erasing at 0x00000000".
All symptoms of the absence of the option
I was going to send a patch.
> 
> - Dave

Thanks,
Sandeep

[U-Boot] [patch] dm355evm NAND support

[David Brownell]

On Monday 05 October 2009, Paulraj, Sandeep wrote:
> 
> > I guess that happened after I prepared the patch but before I sent
> > it in. ?I'll look; there were some differences still. ?Notably to
> > store the environment in the otherwise-unused block zero,
>
> That is because most users who have used existing TI binaries have
> the env stored at 0x3c0000. So if the update the U-Boot binary, they
> will still get all their env back.  
> IIRC this was a customer requirement of ours.

I don't follow.  Those existing binaries also used the
deprecated "infix" OOB scheme, right?  The one which
trashes the manufacturer bad block markers, and is not
compatible with what U-Boot and Linux have agreed to use.

Which means that block becomes unreadable anyway.  There's
no point in trying to preserve it...

- Dave