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

login
register
mail settings
Submitter Amit Virdi
Date March 7, 2012, 12:03 p.m.
Message ID <1331121854-20494-5-git-send-email-amit.virdi@st.com>
Download mbox | patch
Permalink /patch/145210/
State Superseded
Delegated to: Stefan Roese
Headers show

Comments

Amit Virdi - March 7, 2012, 12:03 p.m.
From: Vipin KUMAR <vipin.kumar@st.com>

Signed-off-by: Vipin Kumar <vipin.kumar@st.com>
Signed-off-by: Amit Virdi <amit.virdi@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(-)
Stefan Roese - March 7, 2012, 1:29 p.m.
On Wednesday 07 March 2012 13:03:53 Amit Virdi wrote:
> From: Vipin KUMAR <vipin.kumar@st.com>

Please find some comments below.

> Signed-off-by: Vipin Kumar <vipin.kumar@st.com>
> Signed-off-by: Amit Virdi <amit.virdi@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@denx.de
Amit Virdi - March 26, 2012, 11:41 a.m.
Hello Stefan,

On 3/7/2012 6:59 PM, Stefan Roese wrote:
> On Wednesday 07 March 2012 13:03:53 Amit Virdi wrote:
>> From: Vipin KUMAR<vipin.kumar@st.com>
>
> Please find some comments below.
>
>> Signed-off-by: Vipin Kumar<vipin.kumar@st.com>
>> Signed-off-by: Amit Virdi<amit.virdi@st.com>
>> 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.
>

ok

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

[...]

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

[...]

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

Yes Stefan, it is planned. There's a lot of cleanup that is needed in 
the SPEAr platform code. I shall be sending another patchset after this 
patchset that adds new functionality and does the cleanup.

Can you accept this patch for the time being?

>>
>> +/* 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.
>

Sure!


Thanks
Amit Virdi
Stefan Roese - March 26, 2012, 11:51 a.m.
Hi Amit,

On Monday 26 March 2012 13:41:09 Amit Virdi wrote:
> >> +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.
> 
> Yes Stefan, it is planned. There's a lot of cleanup that is needed in
> the SPEAr platform code. I shall be sending another patchset after this
> patchset that adds new functionality and does the cleanup.
> 
> Can you accept this patch for the time being?

I can, yes. Only speaking for myself though.

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@denx.de

Patch

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)
 
 #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
+}
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
+}
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
+}
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
+}
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 */
+
 /* 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