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 KUMARvipin.kumar@st.com
Please find some comments below.
Signed-off-by: Vipin Kumarvipin.kumar@st.com Signed-off-by: Amit Virdiamit.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