| Message ID | 1331121854-20494-5-git-send-email-amit.virdi@st.com |
|---|---|
| State | Superseded |
| Delegated to: | Stefan Roese |
| Headers | show |
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
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
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
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