diff mbox

[U-Boot] da850: add NOR boot mode support

Message ID 1311870338-18950-1-git-send-email-nagabhushana.netagunte@ti.com
State Changes Requested
Headers show

Commit Message

nagabhushana.netagunte@ti.com July 28, 2011, 4:25 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 |   50 +++++++++++++++++++++++++++++++++++++
 include/configs/da850evm.h        |   21 ++++++++++++++-
 2 files changed, 70 insertions(+), 1 deletions(-)

Comments

Detlev Zundel July 29, 2011, 8:24 a.m. UTC | #1 
Hi,

> 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 |   50 +++++++++++++++++++++++++++++++++++++
>  include/configs/da850evm.h        |   21 ++++++++++++++-
>  2 files changed, 70 insertions(+), 1 deletions(-)
>
> diff --git a/board/davinci/da8xxevm/da850evm.c b/board/davinci/da8xxevm/da850evm.c
> index 73eaa48..a77e438 100644
> --- a/board/davinci/da8xxevm/da850evm.c
> +++ b/board/davinci/da8xxevm/da850evm.c
> @@ -105,6 +105,54 @@ const struct pinmux_config nand_pins[] = {
>  	{ pinmux(12), 1, 5 },
>  	{ pinmux(12), 1, 6 }
>  };
> +#elif defined(CONFIG_SYS_USE_NOR)

Can we have at least a little comment explaining what this long magic
list does?  I for one don't have the slightest clue to why it has to be
like this.

> +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 +170,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..f0015e4 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_SYS_USE_NOR
>  
>  /*
>   * SoC Configuration
> @@ -129,6 +131,23 @@
>  #define CONFIG_NET_MULTI
>  #endif
>  
> +#ifdef CONFIG_SYS_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
> @@ -212,7 +231,7 @@
>  #endif
>  
>  #if !defined(CONFIG_USE_NAND) && \
> -	!defined(CONFIG_USE_NOR) && \
> +	!defined(CONFIG_SYS_USE_NOR) && \
>  	!defined(CONFIG_USE_SPIFLASH)
>  #define CONFIG_ENV_IS_NOWHERE
>  #define CONFIG_SYS_NO_FLASH

Also I am somewhat sceptical about the names of the defines in this
board config - there is for example the new CONFIG_SYS_USE_NOR and the
"old" CONFIG_USE_NAND and CONFIG_USE_SPIFLASH.  Looking into the README,
I see this:

,----[ README ]
| There are two classes of configuration variables:
| 
| * Configuration _OPTIONS_:
|   These are selectable by the user and have names beginning with
|   "CONFIG_".
| 
| * Configuration _SETTINGS_:
|   These depend on the hardware etc. and should not be meddled with if
|   you don't know what you're doing; they have names beginning with
|   "CONFIG_SYS_".
`----

If the names are chosen correctly, then CONFIG_SYS_USE_NOR is coupled to
the actual hardware, so either the hardware _has_ NOR flash, then it
must be set, or it _doesn't_ have NOR flash, then we don't set it.  But
it seems the options are there to give the user a choice what medium he
wants to use (for e.g. the environment).  So if this is correct, then
please strip the _SYS_ in them.

Cheers
  Detlev
nagabhushana.netagunte@ti.com Aug. 1, 2011, 9:42 a.m. UTC | #2 
Zundel,
Thanks for your valuable comments. My reply inline.

-Nag

On Fri, Jul 29, 2011 at 13:54:30, Detlev Zundel wrote:
> Hi,
> 
> > 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 |   50 +++++++++++++++++++++++++++++++++++++
> >  include/configs/da850evm.h        |   21 ++++++++++++++-
> >  2 files changed, 70 insertions(+), 1 deletions(-)
> >
> > diff --git a/board/davinci/da8xxevm/da850evm.c 
> > b/board/davinci/da8xxevm/da850evm.c
> > index 73eaa48..a77e438 100644
> > --- a/board/davinci/da8xxevm/da850evm.c
> > +++ b/board/davinci/da8xxevm/da850evm.c
> > @@ -105,6 +105,54 @@ const struct pinmux_config nand_pins[] = {
> >  	{ pinmux(12), 1, 5 },
> >  	{ pinmux(12), 1, 6 }
> >  };
> > +#elif defined(CONFIG_SYS_USE_NOR)
> 
> Can we have at least a little comment explaining what this long magic list does?  I for one don't have the slightest clue to why it has to be like this.
> 
Will add comments.
> > +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 +170,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..f0015e4 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_SYS_USE_NOR
> >  
> >  /*
> >   * SoC Configuration
> > @@ -129,6 +131,23 @@
> >  #define CONFIG_NET_MULTI
> >  #endif
> >  
> > +#ifdef CONFIG_SYS_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
> > @@ -212,7 +231,7 @@
> >  #endif
> >  
> >  #if !defined(CONFIG_USE_NAND) && \
> > -	!defined(CONFIG_USE_NOR) && \
> > +	!defined(CONFIG_SYS_USE_NOR) && \
> >  	!defined(CONFIG_USE_SPIFLASH)
> >  #define CONFIG_ENV_IS_NOWHERE
> >  #define CONFIG_SYS_NO_FLASH
> 
> Also I am somewhat sceptical about the names of the defines in this board config - there is for example the new CONFIG_SYS_USE_NOR and the "old" CONFIG_USE_NAND and CONFIG_USE_SPIFLASH.  Looking into the README, I see this:
> 
> ,----[ README ]
> | There are two classes of configuration variables:
> | 
> | * Configuration _OPTIONS_:
> |   These are selectable by the user and have names beginning with
> |   "CONFIG_".
> | 
> | * Configuration _SETTINGS_:
> |   These depend on the hardware etc. and should not be meddled with if
> |   you don't know what you're doing; they have names beginning with
> |   "CONFIG_SYS_".
> `----
> 
> If the names are chosen correctly, then CONFIG_SYS_USE_NOR is coupled to the actual hardware, so either the hardware _has_ NOR flash, then it must be set, or it _doesn't_ have NOR flash, then we don't set it.  But it seems the options are there to give the user a choice what medium he wants to use (for e.g. the environment).  So if this is correct, then please strip the _SYS_ in them.
> 
> Cheers
>   Detlev
> 
Will take care CONFIG name.
> --
> 14474011154664524427946373126085988481573677491474835889066354349131199152128
> If you know why this number is perfect - you're probably a mathematician...
> --
> 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..a77e438 100644
--- a/board/davinci/da8xxevm/da850evm.c
+++ b/board/davinci/da8xxevm/da850evm.c
@@ -105,6 +105,54 @@  const struct pinmux_config nand_pins[] = {
 	{ pinmux(12), 1, 5 },
 	{ pinmux(12), 1, 6 }
 };
+#elif defined(CONFIG_SYS_USE_NOR)
+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 +170,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..f0015e4 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_SYS_USE_NOR
 
 /*
  * SoC Configuration
@@ -129,6 +131,23 @@ 
 #define CONFIG_NET_MULTI
 #endif
 
+#ifdef CONFIG_SYS_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
@@ -212,7 +231,7 @@ 
 #endif
 
 #if !defined(CONFIG_USE_NAND) && \
-	!defined(CONFIG_USE_NOR) && \
+	!defined(CONFIG_SYS_USE_NOR) && \
 	!defined(CONFIG_USE_SPIFLASH)
 #define CONFIG_ENV_IS_NOWHERE
 #define CONFIG_SYS_NO_FLASH