[U-Boot] [PATCH 04/25] SPEAr: Configure network support for spear SoCs

[Stefan Roese]

On Wednesday 07 March 2012 13:03:53 Amit Virdi wrote:
> From: Vipin KUMAR <vipin.kumar at st.com>

Please find some comments below.

> Signed-off-by: Vipin Kumar <vipin.kumar at st.com>
> Signed-off-by: Amit Virdi <amit.virdi at st.com>
> ---
>  arch/arm/include/asm/arch-spear/hardware.h |    1 +
>  board/spear/spear300/spear300.c            |   10 ++++++++++
>  board/spear/spear310/spear310.c            |   10 ++++++++++
>  board/spear/spear320/spear320.c            |   10 ++++++++++
>  board/spear/spear600/spear600.c            |   10 ++++++++++
>  include/configs/spear-common.h             |   14 ++++++++++++--
>  include/configs/spear3xx.h                 |    3 +++
>  7 files changed, 56 insertions(+), 2 deletions(-)
> 
> diff --git a/arch/arm/include/asm/arch-spear/hardware.h
> b/arch/arm/include/asm/arch-spear/hardware.h index a6517b2..2b9cb0e 100644
> --- a/arch/arm/include/asm/arch-spear/hardware.h
> +++ b/arch/arm/include/asm/arch-spear/hardware.h
> @@ -31,6 +31,7 @@
>  #define CONFIG_SPEAR_SYSCNTLBASE		(0xFCA00000)
>  #define CONFIG_SPEAR_TIMERBASE			(0xFC800000)
>  #define CONFIG_SPEAR_MISCBASE			(0xFCA80000)
> +#define CONFIG_SPEAR_ETHBASE			(0xE0800000)

I would prefer if you removed these unneeded parentheses here:

#define CONFIG_SPEAR_ETHBASE			0xE0800000

Perhaps best done by doing a cosmetic cleanup patch before the newly added 
defines. I know that checkpatch doesn't complain about this, but these 
parentheses really distract me. Not sure how other feel about it.

>  #define CONFIG_SYS_NAND_CLE			(1 << 16)
>  #define CONFIG_SYS_NAND_ALE			(1 << 17)
> diff --git a/board/spear/spear300/spear300.c
> b/board/spear/spear300/spear300.c index 32bcb77..3f7ccb8 100644
> --- a/board/spear/spear300/spear300.c
> +++ b/board/spear/spear300/spear300.c
> @@ -22,6 +22,7 @@
>   */
> 
>  #include <common.h>
> +#include <netdev.h>
>  #include <nand.h>
>  #include <asm/io.h>
>  #include <linux/mtd/fsmc_nand.h>
> @@ -57,3 +58,12 @@ int board_nand_init(struct nand_chip *nand)
>  #endif
>  	return -1;
>  }
> +
> +int board_eth_init(bd_t *bis)
> +{
> +#if defined(CONFIG_DESIGNWARE_ETH)
> +	return designware_initialize(0, CONFIG_SPEAR_ETHBASE, CONFIG_DW0_PHY);
> +#else
> +	return -1;
> +#endif
> +}

This routine is added multiple times in this patch. Perhaps this can be moved 
to a "common/" file instead?

> diff --git a/board/spear/spear310/spear310.c
> b/board/spear/spear310/spear310.c index 8b58218..8c5b5ba 100644
> --- a/board/spear/spear310/spear310.c
> +++ b/board/spear/spear310/spear310.c
> @@ -23,6 +23,7 @@
>   */
> 
>  #include <common.h>
> +#include <netdev.h>
>  #include <nand.h>
>  #include <asm/io.h>
>  #include <linux/mtd/fsmc_nand.h>
> @@ -58,3 +59,12 @@ int board_nand_init(struct nand_chip *nand)
>  #endif
>  	return -1;
>  }
> +
> +int board_eth_init(bd_t *bis)
> +{
> +#if defined(CONFIG_DESIGNWARE_ETH)
> +	return designware_initialize(0, CONFIG_SPEAR_ETHBASE, CONFIG_DW0_PHY);
> +#else
> +	return -1;
> +#endif
> +}

Here again.

> diff --git a/board/spear/spear320/spear320.c
> b/board/spear/spear320/spear320.c index 172ad35..b60acc2 100644
> --- a/board/spear/spear320/spear320.c
> +++ b/board/spear/spear320/spear320.c
> @@ -23,6 +23,7 @@
>   */
> 
>  #include <common.h>
> +#include <netdev.h>
>  #include <nand.h>
>  #include <asm/io.h>
>  #include <linux/mtd/fsmc_nand.h>
> @@ -58,3 +59,12 @@ int board_nand_init(struct nand_chip *nand)
>  #endif
>  	return -1;
>  }
> +
> +int board_eth_init(bd_t *bis)
> +{
> +#if defined(CONFIG_DESIGNWARE_ETH)
> +	return designware_initialize(0, CONFIG_SPEAR_ETHBASE, CONFIG_DW0_PHY);
> +#else
> +	return -1;
> +#endif
> +}

And again.

> diff --git a/board/spear/spear600/spear600.c
> b/board/spear/spear600/spear600.c index 7cf63d6..5a32b7f 100644
> --- a/board/spear/spear600/spear600.c
> +++ b/board/spear/spear600/spear600.c
> @@ -22,6 +22,7 @@
>   */
> 
>  #include <common.h>
> +#include <netdev.h>
>  #include <nand.h>
>  #include <asm/io.h>
>  #include <linux/mtd/fsmc_nand.h>
> @@ -52,3 +53,12 @@ int board_nand_init(struct nand_chip *nand)
>  #endif
>  	return -1;
>  }
> +
> +int board_eth_init(bd_t *bis)
> +{
> +#if defined(CONFIG_DESIGNWARE_ETH)
> +	return designware_initialize(0, CONFIG_SPEAR_ETHBASE, CONFIG_DW0_PHY);
> +#else
> +	return -1;
> +#endif
> +}

and again.

> diff --git a/include/configs/spear-common.h
> b/include/configs/spear-common.h index 8f4973a..3f52442 100644
> --- a/include/configs/spear-common.h
> +++ b/include/configs/spear-common.h
> @@ -27,6 +27,14 @@
>   * Common configurations used for both spear3xx as well as spear6xx
>   */
> 
> +/* Ethernet driver configuration */
> +#define CONFIG_MII
> +#define CONFIG_DESIGNWARE_ETH
> +#define CONFIG_DW_SEARCH_PHY
> +#define CONFIG_DW0_PHY				1
> +#define CONFIG_NET_MULTI
> +#define CONFIG_PHY_RESET_DELAY			(10000)		/* 
in usec */

Again, please remove these parentheses.

> +
>  /* USBD driver configuration */
>  #define CONFIG_DW_UDC
>  #define CONFIG_USB_DEVICE
> @@ -103,11 +111,13 @@
>  #define CONFIG_CMD_MEMORY
>  #define CONFIG_CMD_RUN
>  #define CONFIG_CMD_SAVES
> +#define CONFIG_CMD_NET
> +#define CONFIG_CMD_MII
> +#define CONFIG_CMD_PING
> +#define CONFIG_CMD_DHCP
> 
>  /* This must be included AFTER the definition of CONFIG_COMMANDS (if any)
> */ #include <config_cmd_default.h>
> -#undef CONFIG_CMD_NET
> -#undef CONFIG_CMD_NFS
> 
>  /*
>   * Default Environment Varible definitions
> diff --git a/include/configs/spear3xx.h b/include/configs/spear3xx.h
> index 2a86c21..035b321 100644
> --- a/include/configs/spear3xx.h
> +++ b/include/configs/spear3xx.h
> @@ -41,6 +41,9 @@
> 
>  #include <configs/spear-common.h>
> 
> +/* Ethernet driver configuration */
> +#define CONFIG_DW_ALTDESCRIPTOR			1
> +
>  /* Serial Configuration (PL011) */
>  #define CONFIG_SYS_SERIAL0			0xD0000000

Thanks,
Stefan

--
DENX Software Engineering GmbH,      MD: Wolfgang Denk & Detlev Zundel
HRB 165235 Munich,  Office: Kirchenstr.5, D-82194 Groebenzell, Germany
Phone: (+49)-8142-66989-0 Fax: (+49)-8142-66989-80 Email: office at denx.de