diff mbox

[U-Boot,03/12] da850: add NOR boot mode support

Message ID 1312299792-16415-4-git-send-email-nagabhushana.netagunte@ti.com
State Superseded
Headers show

Commit Message

nagabhushana.netagunte@ti.com Aug. 2, 2011, 3:43 p.m. UTC 
From: Nagabhushana Netagunte <nagabhushana.netagunte@ti.com>

Add an option to use NOR boot mode in configuration file and
correspanding pin-mux support in board file.

Signed-off-by: Sudhakar Rajashekhara <sudhakar.raj@ti.com>
Signed-off-by: Nagabhushana Netagunte <nagabhushana.netagunte@ti.com>
---
 board/davinci/da8xxevm/da850evm.c |   51 +++++++++++++++++++++++++++++++++++++
 include/configs/da850evm.h        |   19 +++++++++++++
 2 files changed, 70 insertions(+), 0 deletions(-)

Comments

Wolfgang Denk Aug. 2, 2011, 4:12 p.m. UTC | #1 
Dear nagabhushana.netagunte@ti.com,

In message <1312299792-16415-4-git-send-email-nagabhushana.netagunte@ti.com> you wrote:
> From: Nagabhushana Netagunte <nagabhushana.netagunte@ti.com>
> 
> Add an option to use NOR boot mode in configuration file and
> correspanding pin-mux support in board file.
> 
> Signed-off-by: Sudhakar Rajashekhara <sudhakar.raj@ti.com>
> Signed-off-by: Nagabhushana Netagunte <nagabhushana.netagunte@ti.com>
...
>  #define CONFIG_DRIVER_TI_EMAC
>  #define CONFIG_USE_SPIFLASH
> +#undef CONFIG_USE_NAND
> +#undef CONFIG_USE_NOR

Please do not undef what is not defined anyway.  If you want to add
comments to the user, then use C comments for this purpose.

Also, please note that none of the CONFIG_ options listed here
(CONFIG_DRIVER_TI_EMAC, CONFIG_USE_SPIFLASH, CONFIG_USE_NAND, or
CONFIG_USE_NOR) are documented.  Please add appropriate documentation
to the README file.

>  /*
>   * SoC Configuration
> @@ -129,6 +131,23 @@
>  #define CONFIG_NET_MULTI
>  #endif
>  
> +#ifdef CONFIG_USE_NOR
> +#define CONFIG_ENV_IS_IN_FLASH
> +#undef CONFIG_SYS_NO_FLASH

Please do not undef what is not defined anyway.  Please fix globally.

> +#define CONFIG_SYS_FLASH_SECT_SZ	(128 << 10) /* 128KB */
> +#define CONFIG_ENV_OFFSET		(CONFIG_SYS_FLASH_SECT_SZ * 3)
> +#define CONFIG_ENV_SIZE			(128 << 10)

Are you absolutely sure that you need 128 KiB of environment data?
Keep in mind that such a big environment will _considerably_ slow down
booting - and in all practical situations I have seen so far the
actual environment size was in the order of a few KiB only - I don;t
even remember any board with more than 10 KiB.

Best regards,

Wolfgang Denk
nagabhushana.netagunte@ti.com Aug. 9, 2011, 2:03 p.m. UTC | #2 
Hi Denk,

Thanks for comments. My response is in-lined.

Regards,
Nag

On Tue, Aug 02, 2011 at 21:42:28, Wolfgang Denk wrote:
> Dear nagabhushana.netagunte@ti.com,
> 
> In message <1312299792-16415-4-git-send-email-nagabhushana.netagunte@ti.com> you wrote:
> > From: Nagabhushana Netagunte <nagabhushana.netagunte@ti.com>
> > 
> > Add an option to use NOR boot mode in configuration file and 
> > correspanding pin-mux support in board file.
> > 
> > Signed-off-by: Sudhakar Rajashekhara <sudhakar.raj@ti.com>
> > Signed-off-by: Nagabhushana Netagunte <nagabhushana.netagunte@ti.com>
> ...
> >  #define CONFIG_DRIVER_TI_EMAC
> >  #define CONFIG_USE_SPIFLASH
> > +#undef CONFIG_USE_NAND
> > +#undef CONFIG_USE_NOR
> 
> Please do not undef what is not defined anyway.  If you want to add comments to the user, then use C comments for this purpose.
> 

Will do that.

> Also, please note that none of the CONFIG_ options listed here (CONFIG_DRIVER_TI_EMAC, CONFIG_USE_SPIFLASH, CONFIG_USE_NAND, or
> CONFIG_USE_NOR) are documented.  Please add appropriate documentation to the README file.
> 

Will add a new patch to add appropriate documentation.

> >  /*
> >   * SoC Configuration
> > @@ -129,6 +131,23 @@
> >  #define CONFIG_NET_MULTI
> >  #endif
> >  
> > +#ifdef CONFIG_USE_NOR
> > +#define CONFIG_ENV_IS_IN_FLASH
> > +#undef CONFIG_SYS_NO_FLASH
> 
> Please do not undef what is not defined anyway.  Please fix globally.
> 
> > +#define CONFIG_SYS_FLASH_SECT_SZ	(128 << 10) /* 128KB */
> > +#define CONFIG_ENV_OFFSET		(CONFIG_SYS_FLASH_SECT_SZ * 3)
> > +#define CONFIG_ENV_SIZE			(128 << 10)
> 
> Are you absolutely sure that you need 128 KiB of environment data?
> Keep in mind that such a big environment will _considerably_ slow down booting - and in all practical situations I have seen so far the actual environment size was in the order of a few KiB only - I don;t even remember any board with more than 10 KiB.
> 

I agree with you that 128 KiB is huge. It is 128KiB because sector size for
NOR flash is 128KiB. So, decision was to reserve one complete sector for ENV
Data.

> Best regards,
> 
> Wolfgang Denk
> 
> -- 
> DENX Software Engineering GmbH,     MD: Wolfgang Denk & Detlev Zundel
> HRB 165235 Munich, Office: Kirchenstr.5, D-82194 Groebenzell, Germany
> Phone: (+49)-8142-66989-10 Fax: (+49)-8142-66989-80 Email: wd@denx.de
> A list is only as strong as its weakest link.            -- Don Knuth
>
Detlev Zundel Aug. 10, 2011, 10:50 a.m. UTC | #3 
Hi Nag,

[...]

>> > +#define CONFIG_SYS_FLASH_SECT_SZ	(128 << 10) /* 128KB */
>> > +#define CONFIG_ENV_OFFSET		(CONFIG_SYS_FLASH_SECT_SZ * 3)
>> > +#define CONFIG_ENV_SIZE			(128 << 10)
>> 
>> Are you absolutely sure that you need 128 KiB of environment data?
>> Keep in mind that such a big environment will _considerably_ slow
>> down booting - and in all practical situations I have seen so far
>> the actual environment size was in the order of a few KiB only - I
>> don;t even remember any board with more than 10 KiB.
>> 
>
> I agree with you that 128 KiB is huge. It is 128KiB because sector size for
> NOR flash is 128KiB. So, decision was to reserve one complete sector for ENV
> Data.

It's pretty common to have such large sectors.  So using one sector for
the environment is of course pretty common.  From the sector size
however we use only a fraction for the _actual_ data, as the environment
functions have a runtime dependency on the size of the environment
_data_ (think about calculating the CRC).  So if you simply reduce the
environment (data) size, you will get faster runtime practically for
free.  So keep the flash layout but reduce the environment size.

Cheers
  Detlev
nagabhushana.netagunte@ti.com Aug. 17, 2011, 12:43 p.m. UTC | #4 
Hi Detlev,

Thanks for your valuable and detailed comments, they make sense. I will make appropriate changes and reduce the env size.

Best Regards,
-Nag

On Wed, Aug 10, 2011 at 16:20:51, Detlev Zundel wrote:
> Hi Nag,
> 
> [...]
> 
> >> > +#define CONFIG_SYS_FLASH_SECT_SZ	(128 << 10) /* 128KB */
> >> > +#define CONFIG_ENV_OFFSET		(CONFIG_SYS_FLASH_SECT_SZ * 3)
> >> > +#define CONFIG_ENV_SIZE			(128 << 10)
> >> 
> >> Are you absolutely sure that you need 128 KiB of environment data?
> >> Keep in mind that such a big environment will _considerably_ slow 
> >> down booting - and in all practical situations I have seen so far the 
> >> actual environment size was in the order of a few KiB only - I don;t 
> >> even remember any board with more than 10 KiB.
> >> 
> >
> > I agree with you that 128 KiB is huge. It is 128KiB because sector 
> > size for NOR flash is 128KiB. So, decision was to reserve one complete 
> > sector for ENV Data.
> 
> It's pretty common to have such large sectors.  So using one sector for the environment is of course pretty common.  From the sector size however we use only a fraction for the _actual_ data, as the environment functions have a runtime dependency on the size of the environment _data_ (think about calculating the CRC).  So if you simply reduce the environment (data) size, you will get faster runtime practically for free.  So keep the flash layout but reduce the environment size.
> 
> Cheers
>   Detlev
> 
> --
> The GNU GPL makes sense in terms of its purpose: freedom and social solidarity.  Trying to understand it in terms of the goals and values of open source is like trying understand a CD drive's retractable drawer as a cupholder.  You can use it for that, but that is not what it was
> designed for.                      -- Richard Stallman
> --
> DENX Software Engineering GmbH,      MD: Wolfgang Denk & Detlev Zundel
> HRB 165235 Munich,  Office: Kirchenstr.5, D-82194 Groebenzell, Germany
> Phone: (+49)-8142-66989-40 Fax: (+49)-8142-66989-80 Email: dzu@denx.de
>
diff mbox

Patch

diff --git a/board/davinci/da8xxevm/da850evm.c b/board/davinci/da8xxevm/da850evm.c
index 73eaa48..d3d965c 100644
--- a/board/davinci/da8xxevm/da850evm.c
+++ b/board/davinci/da8xxevm/da850evm.c
@@ -105,6 +105,55 @@  const struct pinmux_config nand_pins[] = {
 	{ pinmux(12), 1, 5 },
 	{ pinmux(12), 1, 6 }
 };
+#elif defined(CONFIG_USE_NOR)
+/* NOR pin muxer settings */
+const struct pinmux_config nor_pins[] = {
+	{ pinmux(5), 1, 6 },
+	{ pinmux(6), 1, 6 },
+	{ pinmux(7), 1, 0 },
+	{ pinmux(7), 1, 4 },
+	{ pinmux(7), 1, 5 },
+	{ pinmux(8), 1, 0 },
+	{ pinmux(8), 1, 1 },
+	{ pinmux(8), 1, 2 },
+	{ pinmux(8), 1, 3 },
+	{ pinmux(8), 1, 4 },
+	{ pinmux(8), 1, 5 },
+	{ pinmux(8), 1, 6 },
+	{ pinmux(8), 1, 7 },
+	{ pinmux(9), 1, 0 },
+	{ pinmux(9), 1, 1 },
+	{ pinmux(9), 1, 2 },
+	{ pinmux(9), 1, 3 },
+	{ pinmux(9), 1, 4 },
+	{ pinmux(9), 1, 5 },
+	{ pinmux(9), 1, 6 },
+	{ pinmux(9), 1, 7 },
+	{ pinmux(10), 1, 0 },
+	{ pinmux(10), 1, 1 },
+	{ pinmux(10), 1, 2 },
+	{ pinmux(10), 1, 3 },
+	{ pinmux(10), 1, 4 },
+	{ pinmux(10), 1, 5 },
+	{ pinmux(10), 1, 6 },
+	{ pinmux(10), 1, 7 },
+	{ pinmux(11), 1, 0 },
+	{ pinmux(11), 1, 1 },
+	{ pinmux(11), 1, 2 },
+	{ pinmux(11), 1, 3 },
+	{ pinmux(11), 1, 4 },
+	{ pinmux(11), 1, 5 },
+	{ pinmux(11), 1, 6 },
+	{ pinmux(11), 1, 7 },
+	{ pinmux(12), 1, 0 },
+	{ pinmux(12), 1, 1 },
+	{ pinmux(12), 1, 2 },
+	{ pinmux(12), 1, 3 },
+	{ pinmux(12), 1, 4 },
+	{ pinmux(12), 1, 5 },
+	{ pinmux(12), 1, 6 },
+	{ pinmux(12), 1, 7 }
+};
 #endif
 
 #ifdef CONFIG_DRIVER_TI_EMAC_USE_RMII
@@ -122,6 +171,8 @@  static const struct pinmux_resource pinmuxes[] = {
 	PINMUX_ITEM(i2c_pins),
 #ifdef CONFIG_NAND_DAVINCI
 	PINMUX_ITEM(nand_pins),
+#elif defined(CONFIG_USE_NOR)
+	PINMUX_ITEM(nor_pins),
 #endif
 };
 
diff --git a/include/configs/da850evm.h b/include/configs/da850evm.h
index fdcc6e3..53d63ab 100644
--- a/include/configs/da850evm.h
+++ b/include/configs/da850evm.h
@@ -28,6 +28,8 @@ 
  */
 #define CONFIG_DRIVER_TI_EMAC
 #define CONFIG_USE_SPIFLASH
+#undef CONFIG_USE_NAND
+#undef CONFIG_USE_NOR
 
 /*
  * SoC Configuration
@@ -129,6 +131,23 @@ 
 #define CONFIG_NET_MULTI
 #endif
 
+#ifdef CONFIG_USE_NOR
+#define CONFIG_ENV_IS_IN_FLASH
+#undef CONFIG_SYS_NO_FLASH
+#define CONFIG_FLASH_CFI_DRIVER
+#define CONFIG_SYS_FLASH_CFI
+#define CONFIG_SYS_FLASH_PROTECTION
+#define CONFIG_SYS_MAX_FLASH_BANKS	1 /* max number of flash banks */
+#define CONFIG_SYS_FLASH_SECT_SZ	(128 << 10) /* 128KB */
+#define CONFIG_ENV_OFFSET		(CONFIG_SYS_FLASH_SECT_SZ * 3)
+#define CONFIG_ENV_SIZE			(128 << 10)
+#define CONFIG_SYS_FLASH_BASE		DAVINCI_ASYNC_EMIF_DATA_CE2_BASE
+#define PHYS_FLASH_SIZE			(8 << 20) /* Flash size 8MB */
+#define CONFIG_SYS_MAX_FLASH_SECT ((PHYS_FLASH_SIZE/CONFIG_SYS_FLASH_SECT_SZ)\
+	       + 3)
+#define CONFIG_ENV_SECT_SIZE		CONFIG_SYS_FLASH_SECT_SZ
+#endif
+
 #ifdef CONFIG_USE_SPIFLASH
 #undef CONFIG_ENV_IS_IN_FLASH
 #undef CONFIG_ENV_IS_IN_NAND