Dear Ajay Bhargav,
In message 1310106168-17166-1-git-send-email-ajay.bhargav@einfochips.com you wrote:
This patch adds ethernet support for GuruPlug-Display. tftpboot and and other network related commands now works.
Signed-off-by: Ajay Bhargav ajay.bhargav@einfochips.com
.gitignore | 1 + arch/arm/include/asm/arch-armada100/armada100.h | 39 ++ arch/arm/include/asm/arch-armada100/gpio.h | 81 +++ arch/arm/include/asm/arch-armada100/mfp.h | 19 + board/Marvell/gplugd/gplugd.c | 77 +++ drivers/net/Makefile | 1 + drivers/net/pxa168_eth.c | 815 +++++++++++++++++++++++ drivers/net/pxa168_eth.h | 239 +++++++ include/configs/gplugd.h | 22 +- include/netdev.h | 1 + 10 files changed, 1293 insertions(+), 2 deletions(-) create mode 100644 arch/arm/include/asm/arch-armada100/gpio.h create mode 100644 drivers/net/pxa168_eth.c create mode 100644 drivers/net/pxa168_eth.h
diff --git a/.gitignore b/.gitignore index 8ec3d06..806ab29 100644 --- a/.gitignore +++ b/.gitignore @@ -43,6 +43,7 @@
/include/generated/ /lib/asm-offsets.s +/include/configs/tags
This change is unrelated. Please submit separately.
# stgit generated dirs patches-* diff --git a/arch/arm/include/asm/arch-armada100/armada100.h b/arch/arm/include/asm/arch-armada100/armada100.h index d5d125a..b21e476 100644 --- a/arch/arm/include/asm/arch-armada100/armada100.h +++ b/arch/arm/include/asm/arch-armada100/armada100.h @@ -60,6 +60,9 @@ #define ARMD1_APMU_BASE 0xD4282800 #define ARMD1_CPU_BASE 0xD4282C00
+#define FEC_CLK_ADR 0xD42828FC
Only one blank line, please, at places like that.
--- /dev/null +++ b/arch/arm/include/asm/arch-armada100/gpio.h @@ -0,0 +1,81 @@ +/**************************************************************************
- Copyright (c) 2009, 2010 Marvell International Ltd.
...
Incorrect multiline comment style.
+#define GPIO_bit(gpio) (1 << ((gpio) & 0x1f))
Macro names must be all caps.
...
+#define GPLR(x) GPIO_REG(BANK_OFF((x) >> 5) + 0x00) +#define GPDR(x) GPIO_REG(BANK_OFF((x) >> 5) + 0x0c) +#define GPSR(x) GPIO_REG(BANK_OFF((x) >> 5) + 0x18) +#define GPCR(x) GPIO_REG(BANK_OFF((x) >> 5) + 0x24) +#define GSDR(x) GPIO_REG(BANK_OFF((x) >> 5) + 0x54) +#define GCDR(x) GPIO_REG(BANK_OFF((x) >> 5) + 0x60)
Please use a C struct to dsescribe the register layout.
+#define GPIO_SET 1 +#define GPIO_CLR 0 + +static inline int gpio_get_value(unsigned gpio)
Don't we have a generic GPIO framework we should use here?
diff --git a/board/Marvell/gplugd/gplugd.c b/board/Marvell/gplugd/gplugd.c index dc7d89d..81770fc 100644 --- a/board/Marvell/gplugd/gplugd.c +++ b/board/Marvell/gplugd/gplugd.c @@ -32,9 +32,24 @@ #include <mvmfp.h> #include <asm/arch/mfp.h> #include <asm/arch/armada100.h> +#include <asm/arch/gpio.h> +#include <miiphy.h>
+#ifdef CONFIG_PXA_ETH +#include <net.h> +#include <netdev.h> +#endif /* CONFIG_PXA_ETH */
DECLARE_GLOBAL_DATA_PTR;
+#define PHY_LED_PAR_SEL_REG 22 +#define PHY_LED_MAN_REG 25 +#define PHY_LED_VAL 0x5b /* LINK LED1, ACT LED2 */ +#define PHY_RST 104 /* GPIO104 - PHY RESET */ +#define RTC_IRQ 102 /* GPIO102 - RTC IRQ Input */ +#define FE_CLK_RST 0x1 +#define FE_CLK_ENA 0x8
Hm... is this cource file the right place for such #defines? Probably some should go into global hader files, other into the board config file?
diff --git a/drivers/net/pxa168_eth.c b/drivers/net/pxa168_eth.c new file mode 100644 index 0000000..f5251e9 --- /dev/null +++ b/drivers/net/pxa168_eth.c
...
This driver looks very much similar to what we already have in drivers/net/mvgbe.c.
Instead of re-implementing the same code again and again, please come up with a unified version.
diff --git a/drivers/net/pxa168_eth.h b/drivers/net/pxa168_eth.h new file mode 100644 index 0000000..29c8673 --- /dev/null +++ b/drivers/net/pxa168_eth.h
...
+struct pxa_reg {
- u32 phyadr;
- u8 pad1[0x010 - 0x00 - 4];
- u32 smi;
- u8 pad2[0x400 - 0x010 - 4];
- u32 pconf;
- u8 pad3[4];
- u32 pconf_ext;
- u8 pad4[4];
- u32 pcmd;
- u8 pad5[4];
- u32 pstatus;
- u8 pad6[4];
- u32 spar;
- u8 pad7[4];
- u32 htpr;
- u8 pad8[4];
- u32 fcsal;
- u8 pad9[4];
- u32 fcsah;
- u8 pad10[4];
- u32 sdma_conf;
- u8 pad11[4];
- u32 sdma_cmd;
- u8 pad12[4];
- u32 ic;
- u32 iwc;
- u32 im;
- u8 pad13[4];
- u32 *eth_idscpp[4];
- u32 eth_vlan_p;
- u8 pad14[0x480 - 0x470 - 4];
- struct rx_desc *rxfdp[4];
- u8 pad15[0x4a0 - 0x48c - 4];
- struct rx_desc *rxcdp[4];
- u8 pad16[0x4e0 - 0x4ac - 4];
- struct tx_desc *txcdp[2];
+};
Would it not be nice to have a few comments here what the entries are?
+struct pxa_device {
- struct eth_device dev;
- struct pxa_reg *regs;
- struct tx_desc *p_txdesc;
- struct rx_desc *p_rxdesc;
- struct rx_desc *p_rxdesc_curr;
- u8 *p_rxbuf;
- u8 *p_aligned_txbuf;
- u8 *htpr; /* hash pointer */
+};
Again, this should porobably be unified with drivers/net/mvgbe.h
+#define CONFIG_IPADDR 192.168.9.51 +#define CONFIG_SERVERIP 192.168.9.91 +#define CONFIG_ETHADDR "F0:AD:4E:00:32:9C"
NAK.
We don't allow such static initializations of network parameters.
Best regards,
Wolfgang Denk