All of lore.kernel.org
 help / color / mirror / Atom feed
* [U-Boot] [PATCH 1/4] Armada100: Ethernet support for Marvell gplugD
@ 2011-07-08  6:22 Ajay Bhargav
  2011-07-08  7:47 ` Wolfgang Denk
  2011-07-08 15:19 ` Prafulla Wadaskar
  0 siblings, 2 replies; 14+ messages in thread
From: Ajay Bhargav @ 2011-07-08  6:22 UTC (permalink / raw)
  To: u-boot

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
 
 # 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
+
+
 /*
  * Main Power Management (MPMU) Registers
  * Refer Datasheet Appendix A.8
@@ -117,5 +120,41 @@ struct armd1apb1_registers {
 	u32 ac97;	/*0x084*/
 };
 
+/*
+ * APB2 Clock Reset/Control Registers
+ * Refer Datasheet Appendix A.11
+ */
+struct armd1apb2_registers {
+	u32 pad1[0x01C - 0x000];
+	u32 ssp1_clkrst;		/* 0x01C */
+	u32 ssp2_clkrst;		/* 0x020 */
+	u32 pad2[0x04C - 0x020 - 4];
+	u32 ssp3_clkrst;                /* 0x04C */
+	u32 pad3[0x058 - 0x04C - 4];
+	u32 ssp4_clkrst;                /* 0x058 */
+	u32 ssp5_clkrst;                /* 0x05C */
+};
+
+/*
+ * CPU Interface Registers
+ * Refer Datasheet Appendix A.2
+ */
+struct armd1cpuif_registers {
+	u32 chipid;             /* 0x000 */
+	u32 pad1[0x008 - 0x000 - 4];
+	u32 cpu_conf;           /* 0x008 */
+	u32 pad2[0x01C - 0x008 - 4];
+	u32 mcb_conf;           /* 0x01C */
+	u32 pad3[0x024 - 0x01C - 4];
+	u32 swbranch_add;       /* 0x024 */
+	u32 pref_count0ctrl;    /* 0x028 */
+	u32 pref_count1ctrl;    /* 0x02C */
+	u32 pref_count2ctrl;    /* 0x030 */
+	u32 pref_count0;        /* 0x034 */
+	u32 pref_count1;        /* 0x038 */
+	u32 pref_count2;        /* 0x03C */
+	u32 mc_conf;            /* 0x040 */
+};
+
 #endif /* CONFIG_ARMADA100 */
 #endif /* _ASM_ARCH_ARMADA100_H */
diff --git a/arch/arm/include/asm/arch-armada100/gpio.h b/arch/arm/include/asm/arch-armada100/gpio.h
new file mode 100644
index 0000000..e13c2da
--- /dev/null
+++ b/arch/arm/include/asm/arch-armada100/gpio.h
@@ -0,0 +1,81 @@
+/**************************************************************************
+ *
+ * Copyright (c) 2009, 2010 Marvell International Ltd.
+ *
+ * This file is part of GNU program.
+ *
+ * GNU program is free software: you can redistribute it and/or modify it
+ * under the terms of the GNU General Public License as published by the
+ * Free Software Foundation, either version 2 of the License, or (at your
+ * option) any later version.
+ *
+ * GNU program is distributed in the hope that it will be useful, but
+ * WITHOUT ANY WARRANTY; without even the implied warranty of
+ * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the GNU General
+ * Public License for more details.
+ *
+ * You should have received a copy of the GNU General Public License along
+ * with this program.
+ *
+ * If not, see http://www.gnu.org/licenses/old-licenses/gpl-2.0.html
+ *
+ *************************************************************************/
+
+#ifndef __PXA_GPIO_H
+#define __PXA_GPIO_H
+
+
+#define GPIO_REGS_VIRT (0xD4019000)
+
+#define BANK_OFF(n)     (((n) < 3) ? (n) << 2 : 0x100 + (((n) - 3) << 2))
+#define GPIO_REG(x)     (GPIO_REGS_VIRT + (x))
+
+#define NR_BUILTIN_GPIO (128)
+
+#define GPIO_bit(gpio)  (1 << ((gpio) & 0x1f))
+
+#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)
+
+#define GPIO_SET	1
+#define GPIO_CLR	0
+
+static inline int gpio_get_value(unsigned gpio)
+{
+	if (gpio < NR_BUILTIN_GPIO)
+		return readl(GPLR(gpio)) & readl(GPIO_bit(gpio));
+	else
+		panic("Invalid GPIO pin %u\n", gpio);
+}
+
+static inline void gpio_set_value(unsigned gpio, int value)
+{
+	if (gpio < NR_BUILTIN_GPIO) {
+		if (value)
+			writel(GPIO_bit(gpio), GPSR(gpio));
+		else
+			writel(GPIO_bit(gpio), GPCR(gpio));
+	} else
+		panic("Invalid GPIO pin %u\n", gpio);
+}
+
+static inline int gpio_direction_output(unsigned gpio, int value)
+{
+	if (gpio < NR_BUILTIN_GPIO) {
+		writel(GPIO_bit(gpio), GSDR(gpio));
+
+		if (value)
+			writel(GPIO_bit(gpio), GPSR(gpio));
+		else
+			writel(GPIO_bit(gpio), GPCR(gpio));
+		return 0;
+	} else
+		panic("Invalid GPIO pin %u\n", gpio);
+	return 1;
+}
+
+#endif /* __PXA_GPIO_H */
diff --git a/arch/arm/include/asm/arch-armada100/mfp.h b/arch/arm/include/asm/arch-armada100/mfp.h
index d6e0494..da76b58 100644
--- a/arch/arm/include/asm/arch-armada100/mfp.h
+++ b/arch/arm/include/asm/arch-armada100/mfp.h
@@ -64,6 +64,25 @@
 #define MFP105_CI2C_SDA		(MFP_REG(0x1a4) | MFP_AF1 | MFP_DRIVE_MEDIUM)
 #define MFP106_CI2C_SCL		(MFP_REG(0x1a8) | MFP_AF1 | MFP_DRIVE_MEDIUM)
 
+/* Fast Ethernet */
+#define MFP086_ETH_TXCLK	(MFP_REG(0x158) | MFP_AF5 | MFP_DRIVE_MEDIUM)
+#define MFP087_ETH_TXEN		(MFP_REG(0x15C) | MFP_AF5 | MFP_DRIVE_MEDIUM)
+#define MFP088_ETH_TXDQ3	(MFP_REG(0x160) | MFP_AF5 | MFP_DRIVE_MEDIUM)
+#define MFP089_ETH_TXDQ2	(MFP_REG(0x164) | MFP_AF5 | MFP_DRIVE_MEDIUM)
+#define MFP090_ETH_TXDQ1	(MFP_REG(0x168) | MFP_AF5 | MFP_DRIVE_MEDIUM)
+#define MFP091_ETH_TXDQ0	(MFP_REG(0x16C) | MFP_AF5 | MFP_DRIVE_MEDIUM)
+#define MFP092_ETH_CRS		(MFP_REG(0x170) | MFP_AF5 | MFP_DRIVE_MEDIUM)
+#define MFP093_ETH_COL		(MFP_REG(0x174) | MFP_AF5 | MFP_DRIVE_MEDIUM)
+#define MFP094_ETH_RXCLK	(MFP_REG(0x178) | MFP_AF5 | MFP_DRIVE_MEDIUM)
+#define MFP095_ETH_RXER		(MFP_REG(0x17C) | MFP_AF5 | MFP_DRIVE_MEDIUM)
+#define MFP096_ETH_RXDQ3	(MFP_REG(0x180) | MFP_AF5 | MFP_DRIVE_MEDIUM)
+#define MFP097_ETH_RXDQ2	(MFP_REG(0x184) | MFP_AF5 | MFP_DRIVE_MEDIUM)
+#define MFP098_ETH_RXDQ1	(MFP_REG(0x188) | MFP_AF5 | MFP_DRIVE_MEDIUM)
+#define MFP099_ETH_RXDQ0	(MFP_REG(0x18C) | MFP_AF5 | MFP_DRIVE_MEDIUM)
+#define MFP100_ETH_MDC		(MFP_REG(0x190) | MFP_AF5 | MFP_DRIVE_MEDIUM)
+#define MFP101_ETH_MDIO		(MFP_REG(0x194) | MFP_AF5 | MFP_DRIVE_MEDIUM)
+#define MFP103_ETH_RXDV		(MFP_REG(0x19C) | MFP_AF5 | MFP_DRIVE_MEDIUM)
+
 /* More macros can be defined here... */
 
 #define MFP_PIN_MAX	117
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
+
 int board_early_init_f(void)
 {
 	u32 mfp_cfg[] = {
@@ -45,10 +60,34 @@ int board_early_init_f(void)
 		/* Enable Console on UART3 */
 		MFPO8_UART3_TXD,
 		MFPO9_UART3_RXD,
+
+		/* Ethernet PHY Interface */
+		MFP086_ETH_TXCLK,
+		MFP087_ETH_TXEN,
+		MFP088_ETH_TXDQ3,
+		MFP089_ETH_TXDQ2,
+		MFP090_ETH_TXDQ1,
+		MFP091_ETH_TXDQ0,
+		MFP092_ETH_CRS,
+		MFP093_ETH_COL,
+		MFP094_ETH_RXCLK,
+		MFP095_ETH_RXER,
+		MFP096_ETH_RXDQ3,
+		MFP097_ETH_RXDQ2,
+		MFP098_ETH_RXDQ1,
+		MFP099_ETH_RXDQ0,
+		MFP100_ETH_MDC,
+		MFP101_ETH_MDIO,
+		MFP103_ETH_RXDV,
+
 		MFP_EOC		/*End of configuration*/
 	};
 	/* configure MFP's */
 	mfp_config(mfp_cfg);
+
+	/* Assert PHY_RST# */
+	gpio_direction_output(PHY_RST, GPIO_CLR);
+
 	return 0;
 }
 
@@ -58,5 +97,43 @@ int board_init(void)
 	gd->bd->bi_arch_number = MACH_TYPE_SHEEVAD;
 	/* adress of boot parameters */
 	gd->bd->bi_boot_params = armd1_sdram_base(0) + 0x100;
+
 	return 0;
 }
+
+#ifdef CONFIG_PXA_ETH
+int board_eth_init(bd_t *bis)
+{
+	/* Deassert PHY_RST# */
+	gpio_set_value(PHY_RST, GPIO_SET);
+
+	/* Enable clock of ethernet controller */
+	writel(FE_CLK_RST | FE_CLK_ENA, FEC_CLK_ADR);
+	return pxa_fec_initialize();
+}
+
+/* Configure and initialize PHY chip 88E3015 */
+void reset_phy(void)
+{
+	u16 phy_adr;
+	const char *name = "pxa-fec0";
+
+	if (miiphy_set_current_dev(name))
+		return;
+
+	/* command to read PHY dev address */
+	if (miiphy_read(name, 0xff, 0xff, (u16 *) &phy_adr)) {
+		printf("Err..%s could not read PHY dev address\n",
+				__func__);
+		return;
+	}
+
+	/* Set Ethernet LED in TX blink mode */
+	miiphy_write(name, phy_adr, PHY_LED_MAN_REG, 0x00);
+	miiphy_write(name, phy_adr, PHY_LED_PAR_SEL_REG, PHY_LED_VAL);
+
+	/* reset the phy */
+	miiphy_reset(name, phy_adr);
+	printf("88E3015 Initialized on %s\n", name);
+}
+#endif /* CONFIG_PXA_ETH */
diff --git a/drivers/net/Makefile b/drivers/net/Makefile
index 819b197..125e7c4 100644
--- a/drivers/net/Makefile
+++ b/drivers/net/Makefile
@@ -84,6 +84,7 @@ COBJS-$(CONFIG_TSI108_ETH) += tsi108_eth.o
 COBJS-$(CONFIG_ULI526X) += uli526x.o
 COBJS-$(CONFIG_VSC7385_ENET) += vsc7385.o
 COBJS-$(CONFIG_XILINX_EMACLITE) += xilinx_emaclite.o
+COBJS-$(CONFIG_PXA_ETH) += pxa168_eth.o
 
 COBJS	:= $(sort $(COBJS-y))
 SRCS	:= $(COBJS:.o=.c)
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
@@ -0,0 +1,815 @@
+/**************************************************************************
+ *
+ * Copyright (c) 2009, 2010 Marvell International Ltd.
+ *
+ * This file is part of GNU program.
+ *
+ * GNU program is free software: you can redistribute it and/or modify it
+ * under the terms of the GNU General Public License as published by the
+ * Free Software Foundation, either version 2 of the License, or (at your
+ * option) any later version.
+ *
+ * GNU program is distributed in the hope that it will be useful, but
+ * WITHOUT ANY WARRANTY; without even the implied warranty of
+ * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the GNU General
+ * Public License for more details.
+ *
+ * You should have received a copy of the GNU General Public License along
+ * with this program.
+ *
+ * If not, see http://www.gnu.org/licenses/old-licenses/gpl-2.0.html
+ *
+ *************************************************************************/
+
+/*
+ * (C) Copyright 2010
+ * Marvell Semiconductor <www.marvell.com>
+ * Written-by: Mahavir Jain <mjain@marvell.com>
+ *
+ * See file CREDITS for list of people who contributed to this
+ * project.
+ *
+ * This program is free software; you can redistribute it and/or
+ * modify it under the terms of the GNU General Public License as
+ * published by the Free Software Foundation; either version 2 of
+ * the License, or (at your option) any later version.
+ *
+ * This program is distributed in the hope that it will be useful,
+ * but WITHOUT ANY WARRANTY; without even the implied warranty of
+ * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the
+ * GNU General Public License for more details.
+ *
+ * You should have received a copy of the GNU General Public License
+ * along with this program; if not, write to the Free Software
+ * Foundation, Inc., 51 Franklin Street, Fifth Floor, Boston,
+ * MA 02110-1301 USA
+ */
+
+#include <common.h>
+#include <net.h>
+#include <malloc.h>
+#include <miiphy.h>
+#include <netdev.h>
+#include <linux/types.h>
+#include <asm/byteorder.h>
+#include <linux/err.h>
+#include <linux/mii.h>
+#include <asm/io.h>
+#include "pxa168_eth.h"
+
+#define  PHY_ADR_REQ     0xFF	/* Magic number to read/write PHY address */
+
+#ifdef ETH_DUMP_REGS
+static int eth_dump_regs(struct eth_device *dev)
+{
+	struct pxa_device *dpxa = to_dpxa(dev);
+	struct pxa_reg *regs = dpxa->regs;
+	unsigned int i = 0;
+
+	printf("\noffset: phy_adr, value: 0x%x\n", PXA_RD(regs->phyadr));
+	printf("offset: smi, value: 0x%x\n", PXA_RD(regs->smi));
+	for (i = 0x400; i <= 0x4e4; i += 4)
+		printf("offset: 0x%x, value: 0x%x\n",
+			i, readl(PXA_FEC_BASE + i));
+	return 0;
+}
+#endif
+
+static int smi_reg_read(const char *devname, u8 phy_addr, u8 phy_reg,
+			u16 *value)
+{
+	struct eth_device *dev = eth_get_dev_by_name(devname);
+	struct pxa_device *dpxa = to_dpxa(dev);
+	struct pxa_reg *regs = dpxa->regs;
+	u32 val, reg_data;
+	int i = 0;
+
+	if (phy_addr == PHY_ADR_REQ && phy_reg == PHY_ADR_REQ) {
+		reg_data = PXA_RD(regs->phyadr);
+		*value = (u16) ((reg_data >> (5 * PORT_NUM)) & 0x1f);
+		return 0;
+	}
+
+	/* check parameters */
+	if (phy_addr > PHY_MASK) {
+		printf("Err..(%s) Invalid phy address\n", __func__);
+		return -EINVAL;
+	}
+	if (phy_reg > PHY_MASK) {
+		printf("Err..(%s) Invalid register offset\n",
+			__func__);
+		return -EINVAL;
+	}
+
+	/* wait for the SMI register to become available */
+	for (i = 0; (val = PXA_RD(regs->smi)) & SMI_BUSY; i++) {
+
+		if (i == PHY_WAIT_ITERATIONS) {
+			printf("Error (%s) PHY busy timeout\n",
+				__func__);
+			return -1;
+		}
+		udelay(PHY_WAIT_MICRO_SECONDS);
+	}
+
+	PXA_WR(((phy_addr << 16) | (phy_reg << 21) | SMI_OP_R), regs->smi);
+
+	/* now wait for the data to be valid */
+	for (i = 0; !((val = PXA_RD(regs->smi)) & SMI_R_VALID); i++) {
+		if (i == PHY_WAIT_ITERATIONS) {
+			printf("Error (%s) PHY Read timeout, val=0x%x\n",
+				__func__, val);
+			return -1;
+		}
+		udelay(PHY_WAIT_MICRO_SECONDS);
+	}
+	*value = val & 0xffff;
+	debug("%s:(adr 0x%x, off 0x%x) value= %04x\n", __func__,
+		phy_addr, phy_reg, *value);
+
+	return 0;
+}
+
+static int smi_reg_write(const char *devname,
+	 u8 phy_addr, u8 phy_reg, u16 value)
+{
+	struct eth_device *dev = eth_get_dev_by_name(devname);
+	struct pxa_device *dpxa = to_dpxa(dev);
+	struct pxa_reg *regs = dpxa->regs;
+	u32 reg_data;
+	int addr_shift = 5 * PORT_NUM;
+	int i;
+
+	if (phy_addr == PHY_ADR_REQ && phy_reg == PHY_ADR_REQ) {
+		reg_data = PXA_RD(regs->phyadr);
+		reg_data &= ~(0x1f << addr_shift);
+		reg_data |= (value & 0x1f) << addr_shift;
+		PXA_WR(reg_data, regs->phyadr);
+		return 0;
+	}
+
+	/* check parameters */
+	if (phy_addr > PHY_MASK) {
+		printf("Err..(%s) Invalid phy address\n", __func__);
+		return -EINVAL;
+	}
+	if (phy_reg > PHY_MASK) {
+		printf("Err..(%s) Invalid register offset\n",
+		       __func__);
+		return -EINVAL;
+	}
+
+	/* wait for the SMI register to become available */
+	for (i = 0; PXA_RD(regs->smi) & SMI_BUSY; i++) {
+		if (i == PHY_WAIT_ITERATIONS) {
+			printf("Error (%s) PHY busy timeout\n",
+			       __func__);
+			return -1;
+		}
+		udelay(PHY_WAIT_MICRO_SECONDS);
+	}
+
+	PXA_WR(((phy_addr << 16) | (phy_reg << 21) | SMI_OP_W |
+		(value & 0xffff)), regs->smi);
+	debug("%s:(adr 0x%x, off 0x%x) value= %04x\n", __func__,
+		phy_addr, phy_reg, value);
+	return 0;
+}
+
+/* Abort any transmit and receive operations and put DMA
+ * in idle state. AT and AR bits are cleared upon entering
+ * in IDLE state. So poll those bits to verify operation.
+ */
+static void abortDMA(struct eth_device *dev)
+{
+	struct pxa_device *dpxa = to_dpxa(dev);
+	struct pxa_reg *regs = dpxa->regs;
+	int delay;
+	int maxRetries = 40;
+
+	do {
+		PXA_WR(SDMA_CMD_AR | SDMA_CMD_AT, regs->sdma_cmd);
+		udelay(100);
+
+		delay = 10;
+		while ((PXA_RD(regs->sdma_cmd) &
+			(SDMA_CMD_AR | SDMA_CMD_AT))
+			&& delay-- > 0) {
+			udelay(10);
+		}
+	} while (maxRetries-- > 0 && delay <= 0);
+
+	if (maxRetries <= 0)
+		printf("%s : DMA Stuck\n", __func__);
+}
+
+static inline u32 nibble_swapping_32_bit(u32 x)
+{
+	return (((x) & 0xf0f0f0f0) >> 4) | (((x) & 0x0f0f0f0f) << 4);
+}
+
+static inline u32 nibble_swapping_16_bit(u32 x)
+{
+	return (((x) & 0x0000f0f0) >> 4) | (((x) & 0x00000f0f) << 4);
+}
+
+static inline u32 flip_4_bits(u32 x)
+{
+	return (((x) & 0x01) << 3) | (((x) & 0x002) << 1)
+		| (((x) & 0x04) >> 1) | (((x) & 0x008) >> 3);
+}
+
+/*
+ * ----------------------------------------------------------------------------
+ * This function will calculate the hash function of the address.
+ * depends on the hash mode and hash size.
+ * Inputs
+ * macH             - the 2 most significant bytes of the MAC address.
+ * macL             - the 4 least significant bytes of the MAC address.
+ * Outputs
+ * return the calculated entry.
+ */
+static u32 hash_function(u32 macH, u32 macL)
+{
+	u32 hashResult;
+	u32 addrH;
+	u32 addrL;
+	u32 addr0;
+	u32 addr1;
+	u32 addr2;
+	u32 addr3;
+	u32 addrHSwapped;
+	u32 addrLSwapped;
+
+	addrH = nibble_swapping_16_bit(macH);
+	addrL = nibble_swapping_32_bit(macL);
+
+	addrHSwapped = flip_4_bits(addrH & 0xf)
+		+ ((flip_4_bits((addrH >> 4) & 0xf)) << 4)
+		+ ((flip_4_bits((addrH >> 8) & 0xf)) << 8)
+		+ ((flip_4_bits((addrH >> 12) & 0xf)) << 12);
+
+	addrLSwapped = flip_4_bits(addrL & 0xf)
+		+ ((flip_4_bits((addrL >> 4) & 0xf)) << 4)
+		+ ((flip_4_bits((addrL >> 8) & 0xf)) << 8)
+		+ ((flip_4_bits((addrL >> 12) & 0xf)) << 12)
+		+ ((flip_4_bits((addrL >> 16) & 0xf)) << 16)
+		+ ((flip_4_bits((addrL >> 20) & 0xf)) << 20)
+		+ ((flip_4_bits((addrL >> 24) & 0xf)) << 24)
+		+ ((flip_4_bits((addrL >> 28) & 0xf)) << 28);
+
+	addrH = addrHSwapped;
+	addrL = addrLSwapped;
+
+	addr0 = (addrL >> 2) & 0x03f;
+	addr1 = (addrL & 0x003) | ((addrL >> 8) & 0x7f) << 2;
+	addr2 = (addrL >> 15) & 0x1ff;
+	addr3 = ((addrL >> 24) & 0x0ff) | ((addrH & 1) << 8);
+
+	hashResult = (addr0 << 9) | (addr1 ^ addr2 ^ addr3);
+	hashResult = hashResult & 0x07ff;
+	return hashResult;
+}
+
+/*
+ * ----------------------------------------------------------------------------
+ * This function will add an entry to the address table.
+ * depends on the hash mode and hash size that was initialized.
+ * Inputs
+ * macH - the 2 most significant bytes of the MAC address.
+ * macL - the 4 least significant bytes of the MAC address.
+ * skip - if 1, skip this address.
+ * rd   - the RD field in the address table.
+ * Outputs
+ * address table entry is added.
+ * 0 if success.
+ * -ENOSPC if table full
+ */
+static int add_del_hash_entry(struct pxa_device *dpxa, u32 macH,
+			      u32 macL, u32 rd, u32 skip, int del)
+{
+	struct addr_table_entry_t *entry, *start;
+	u32 newHi;
+	u32 newLo;
+	u32 i;
+	u8 *last;
+
+	newLo = (((macH >> 4) & 0xf) << 15)
+		| (((macH >> 0) & 0xf) << 11)
+		| (((macH >> 12) & 0xf) << 7)
+		| (((macH >> 8) & 0xf) << 3)
+		| (((macL >> 20) & 0x1) << 31)
+		| (((macL >> 16) & 0xf) << 27)
+		| (((macL >> 28) & 0xf) << 23)
+		| (((macL >> 24) & 0xf) << 19)
+		| (skip << hteSkip) | (rd << hteRDBit)
+		| hteValid;
+
+	newHi = (((macL >> 4) & 0xf) << 15)
+		| (((macL >> 0) & 0xf) << 11)
+		| (((macL >> 12) & 0xf) << 7)
+		| (((macL >> 8) & 0xf) << 3)
+		| (((macL >> 21) & 0x7) << 0);
+
+	/*
+	 * Pick the appropriate table, start scanning for free/reusable
+	 * entries@the index obtained by hashing the specified MAC address
+	 */
+	start = (struct addr_table_entry_t *) (dpxa->htpr);
+	entry = start + hash_function(macH, macL);
+	for (i = 0; i < HOP_NUMBER; i++) {
+		if (!(entry->lo & hteValid)) {
+			break;
+		} else {
+			/* if same address put in same position */
+			if (((entry->lo & 0xfffffff8) ==
+			     (newLo & 0xfffffff8))
+			    && (entry->hi == newHi)) {
+				break;
+			}
+		}
+		if (entry == start + 0x7ff)
+			entry = start;
+		else
+			entry++;
+	}
+
+	if (((entry->lo & 0xfffffff8) != (newLo & 0xfffffff8)) &&
+		(entry->hi != newHi) && del)
+		return 0;
+
+	if (i == HOP_NUMBER) {
+		if (!del) {
+			printf("%s: table section is full\n", __FILE__);
+			return -ENOSPC;
+		} else {
+			return 0;
+		}
+	}
+
+	/*
+	 * Update the selected entry
+	 */
+	if (del) {
+		entry->hi = 0;
+		entry->lo = 0;
+	} else {
+		entry->hi = newHi;
+		entry->lo = newLo;
+	}
+
+	last = (u8 *) entry;
+	last = last + sizeof(*entry);
+
+	return 0;
+}
+
+/*
+ * ----------------------------------------------------------------------------
+ *  Create an addressTable entry from MAC address info
+ *  found in the specifed net_device struct
+ *
+ *  Input : pointer to ethernet interface network device structure
+ *  Output : N/A
+ */
+static void update_hash_table_mac_address(struct pxa_device *dpxa,
+					  u8 *oaddr, u8 *addr)
+{
+	u32 macH;
+	u32 macL;
+
+	/* Delete old entry */
+	if (oaddr) {
+		macH = (oaddr[0] << 8) | oaddr[1];
+		macL = (oaddr[2] << 24) | (oaddr[3] << 16) |
+			(oaddr[4] << 8) | oaddr[5];
+		add_del_hash_entry(dpxa, macH, macL, 1, 0, HASH_DELETE);
+	}
+
+	/* Add new entry */
+	macH = (addr[0] << 8) | addr[1];
+	macL = (addr[2] << 24) | (addr[3] << 16) | (addr[4] << 8) | addr[5];
+	add_del_hash_entry(dpxa, macH, macL, 1, 0, HASH_ADD);
+}
+
+/* Address Table Initialization */
+static void init_hashtable(struct eth_device *dev)
+{
+	struct pxa_device *dpxa = to_dpxa(dev);
+	struct pxa_reg *regs = dpxa->regs;
+	memset(dpxa->htpr, 0, HASH_ADDR_TABLE_SIZE);
+	PXA_WR((u32) dpxa->htpr, regs->htpr);
+}
+
+static int setPortConfigExt(struct eth_device *dev, int mtu)
+{
+	struct pxa_device *dpxa = to_dpxa(dev);
+	struct pxa_reg *regs = dpxa->regs;
+	int mtuSize;
+
+	/* 64 should work but does not -- dhcp packets NEVER get transmitted. */
+	if ((mtu > MAX_PKT_SIZE) || (mtu < 64))
+		return -EINVAL;
+
+	/* add source/dest mac addr (12) + pid (2) + crc (4) */
+	mtu += ETH_EXTRA_HEADER;
+	if (mtu <= 1518)
+		mtuSize = PCXR_MFL_1518;
+	else if (mtu <= 1536)
+		mtuSize = PCXR_MFL_1536;
+	else if (mtu <= 2048)
+		mtuSize = PCXR_MFL_2048;
+	else
+		mtuSize = PCXR_MFL_64K;
+
+	/* Extended Port Configuration */
+	PXA_WR(PCXR_2BSM |	/* Two byte suffix aligns IP hdr */
+		PCXR_DSCP_EN |	/* Enable DSCP in IP */
+		mtuSize | PCXR_FLP |	/* do not force link pass */
+		PCXR_TX_HIGH_PRI,	/* Transmit - high priority queue */
+		regs->pconf_ext);
+
+	/* subtract source/dest mac addr (12) + pid (2) + crc (4) */
+	mtu -= ETH_EXTRA_HEADER;
+	return 0;
+}
+
+/* This detects PHY chip from address 0-31 by reading PHY status
+ * registers. PHY chip can be connected at any of this address.
+ */
+static int ethernet_phy_detect(struct eth_device *dev)
+{
+	u32 val;
+	u16 tmp, mii_status;
+	u8 addr;
+
+	for (addr = 0; addr < 32; addr++) {
+		if (miiphy_read(dev->name, addr, MII_BMSR, &mii_status)
+			!= 0)
+			/* try next phy */
+			continue;
+
+		/* invalid MII status. More validation required here... */
+		if (mii_status == 0 || mii_status == 0xffff)
+			/* try next phy */
+			continue;
+
+		if (miiphy_read(dev->name, addr, MII_PHYSID1, &tmp) != 0)
+			/* try next phy */
+			continue;
+
+		val = tmp << 16;
+		if (miiphy_read(dev->name, addr, MII_PHYSID2, &tmp) != 0)
+			/* try next phy */
+			continue;
+
+		val |= tmp;
+
+		if ((val & 0xfffffff0) != 0)
+			return addr;
+	}
+	return -1;
+}
+
+static void pxa_init_rx_desc_ring(struct pxa_device *dpxa)
+{
+	struct rx_desc *p_rx_desc;
+	int i;
+
+	/* initialize the Rx descriptors ring */
+	p_rx_desc = dpxa->p_rxdesc;
+	for (i = 0; i < RINGSZ; i++) {
+		p_rx_desc->cmd_sts = BUF_OWNED_BY_DMA | RX_EN_INT;
+		p_rx_desc->buf_size = PKTSIZE_ALIGN;
+		p_rx_desc->byte_cnt = 0;
+		p_rx_desc->buf_ptr = dpxa->p_rxbuf + i * PKTSIZE_ALIGN;
+		if (i == (RINGSZ - 1))
+			p_rx_desc->nxtdesc_p = dpxa->p_rxdesc;
+		else {
+			p_rx_desc->nxtdesc_p = (struct rx_desc *)
+			    ((u32) p_rx_desc + PXA_RXQ_DESC_ALIGNED_SIZE);
+			p_rx_desc = p_rx_desc->nxtdesc_p;
+		}
+	}
+	dpxa->p_rxdesc_curr = dpxa->p_rxdesc;
+}
+
+static int pxa_init(struct eth_device *dev)
+{
+	struct pxa_device *dpxa = to_dpxa(dev);
+	struct pxa_reg *regs = dpxa->regs;
+	u32 val;
+
+	pxa_init_rx_desc_ring(dpxa);
+
+	/* Disable interrupts */
+	PXA_WR(0, regs->im);
+	PXA_WR(0, regs->ic);
+	/* Write to ICR to clear interrupts. */
+	PXA_WR(0, regs->iwc);
+
+	/*Abort any transmit and receive operations and put DMA
+	 *in idle state.
+	 */
+	abortDMA(dev);
+
+	/* Initialize address hash table */
+	init_hashtable(dev);
+
+	/* SDMA configuration */
+	PXA_WR(SDCR_BSZ8 |	/* Burst size = 32 bytes */
+		SDCR_RIFB |	/* Rx interrupt on frame */
+		SDCR_BLMT |	/* Little endian transmit */
+		SDCR_BLMR |	/* Little endian receive */
+		SDCR_RC_MAX_RETRANS,	/* Max retransmit count */
+		regs->sdma_conf);
+	/* Port Configuration */
+	PXA_WR(PCR_HS, regs->pconf);	/* Hash size is 1/2kb */
+	setPortConfigExt(dev, 1500);
+
+	update_hash_table_mac_address(dpxa, dev->enetaddr, dev->enetaddr);
+
+	/* Update TX and RX queue descriptor register */
+	PXA_WR((u32) dpxa->p_txdesc, regs->txcdp[TXQ]);
+	PXA_WR((u32) dpxa->p_rxdesc, regs->rxfdp[RXQ]);
+	PXA_WR((u32) dpxa->p_rxdesc_curr, regs->rxcdp[RXQ]);
+
+	/* Enable Interrupts */
+	PXA_WR(ALL_INTS, regs->im);
+
+	/* Enable Ethernet Port */
+	val = PXA_RD(regs->pconf);
+	val |= PCR_EN;
+	PXA_WR(val, regs->pconf);
+
+	/* Enable RX DMA engine */
+	val = PXA_RD(regs->sdma_cmd);
+	val |= SDMA_CMD_ERD;
+	PXA_WR(val, regs->sdma_cmd);
+
+#ifdef ETH_DUMP_REGS
+	eth_dump_regs(dev);
+#endif
+
+#if (defined(CONFIG_MII) || defined(CONFIG_CMD_MII)) \
+			&& defined(CONFIG_SYS_FAULT_ECHO_LINK_DOWN)
+	/* Wait up to 5s for the link status */
+	for (i = 0; i < 5; i++) {
+		u16 phy_adr;
+
+		miiphy_read(dev->name, 0xFF, 0xFF, &phy_adr);
+		/* Return if we get link up */
+		if (miiphy_link(dev->name, phy_adr))
+			return 0;
+		udelay(1000000);
+	}
+
+	printf("No link on %s\n", dev->name);
+	return -1;
+#endif
+	return 0;
+}
+
+static int pxa_halt(struct eth_device *dev)
+{
+	struct pxa_device *dpxa = to_dpxa(dev);
+	struct pxa_reg *regs = dpxa->regs;
+	u32 val;
+	/* Stop RX DMA */
+	val = PXA_RD(regs->sdma_cmd);
+	val &= ~SDMA_CMD_ERD;
+	PXA_WR(val, regs->sdma_cmd);
+
+	/* Abort any transmit and receive operations and put DMA
+	 * in idle state.
+	 */
+	abortDMA(dev);
+
+	/* Disable interrupts */
+	PXA_WR(0, regs->im);
+	PXA_WR(0, regs->ic);
+	PXA_WR(0, regs->iwc);
+
+	/* Disable Port */
+	val = PXA_RD(regs->pconf);
+	val &= ~PCR_EN;
+	PXA_WR(val, regs->pconf);
+
+	return 0;
+}
+
+static int pxa_send(struct eth_device *dev, void *dataptr,
+		    int datasize)
+{
+	struct pxa_device *dpxa = to_dpxa(dev);
+	struct pxa_reg *regs = dpxa->regs;
+	struct tx_desc *p_txdesc = dpxa->p_txdesc;
+	void *p = (void *) dataptr;
+	u32 cmd_sts;
+
+	/* Copy buffer if it's misaligned */
+	if ((u32) dataptr & 0x07) {
+		if (datasize > PKTSIZE_ALIGN) {
+			printf("Non-aligned data too large (%d)\n",
+				datasize);
+			return -1;
+		}
+
+		memcpy(dpxa->p_aligned_txbuf, p, datasize);
+		p = dpxa->p_aligned_txbuf;
+	}
+
+	p_txdesc->cmd_sts = TX_ZERO_PADDING | TX_GEN_CRC;
+	p_txdesc->cmd_sts |= TX_FIRST_DESC | TX_LAST_DESC;
+	p_txdesc->cmd_sts |= BUF_OWNED_BY_DMA;
+	p_txdesc->cmd_sts |= TX_EN_INT;
+	p_txdesc->buf_ptr = (u8 *) p;
+	p_txdesc->byte_cnt = datasize;
+
+	/* Apply send command using high priority TX queue */
+	PXA_WR((u32) p_txdesc, regs->txcdp[TXQ]);
+	PXA_WR(SDMA_CMD_TXDL | SDMA_CMD_TXDH | SDMA_CMD_ERD,
+		regs->sdma_cmd);
+
+	/*
+	 * wait for packet xmit completion
+	 */
+	cmd_sts = readl(&p_txdesc->cmd_sts);
+	while (cmd_sts & BUF_OWNED_BY_DMA) {
+		/* return fail if error is detected */
+		if ((cmd_sts & (TX_ERROR | TX_LAST_DESC)) ==
+			(TX_ERROR | TX_LAST_DESC)) {
+			printf("Err..(%s) in xmit packet\n", __func__);
+			return -1;
+		}
+		cmd_sts = readl(&p_txdesc->cmd_sts);
+	};
+	return 0;
+}
+
+static int pxa_recv(struct eth_device *dev)
+{
+	struct pxa_device *dpxa = to_dpxa(dev);
+	struct rx_desc *p_rxdesc_curr = dpxa->p_rxdesc_curr;
+	u32 cmd_sts;
+	u32 timeout = 0;
+
+	/* wait untill rx packet available or timeout */
+	do {
+		if (timeout <
+			(PHY_WAIT_ITERATIONS * PHY_WAIT_MICRO_SECONDS))
+			timeout++;
+		else {
+			debug("%s time out...\n", __func__);
+			return -1;
+		}
+	} while (readl(&p_rxdesc_curr->cmd_sts) & BUF_OWNED_BY_DMA);
+
+	if (p_rxdesc_curr->byte_cnt != 0) {
+		debug
+			("%s: Received %d byte Packet @ 0x%x (cmd_sts= %08x)\n",
+			__func__, (u32) p_rxdesc_curr->byte_cnt,
+			(u32) p_rxdesc_curr->buf_ptr,
+			(u32) p_rxdesc_curr->cmd_sts);
+	}
+
+	/*
+	 * In case received a packet without first/last bits on
+	 * OR the error summary bit is on,
+	 * the packets needs to be dropeed.
+	 */
+	cmd_sts = readl(&p_rxdesc_curr->cmd_sts);
+
+	if ((cmd_sts & (RX_FIRST_DESC | RX_LAST_DESC))
+		!= (RX_FIRST_DESC | RX_LAST_DESC)) {
+
+		printf("Err..(%s) Dropping packet spread on"
+			" multiple descriptors\n", __func__);
+
+	} else if (cmd_sts & RX_ERROR) {
+
+		printf("Err..(%s) Dropping packet with errors\n",
+			__func__);
+
+	} else {
+		/* !!! call higher layer processing */
+		debug("%s: Sending Received packet to"
+			" upper layer (NetReceive)\n", __func__);
+
+		/* let the upper layer handle the packet, subtract offset
+		 * as two dummy bytes are added in received buffer see
+		 * PORT_CONFIG_EXT register bit TWO_Byte_Stuff_Mode bit.
+		 */
+		NetReceive((p_rxdesc_curr->buf_ptr + RX_BUF_OFFSET),
+			   (int) (p_rxdesc_curr->byte_cnt -
+				  RX_BUF_OFFSET));
+	}
+	/*
+	 * free these descriptors and point next in the ring
+	 */
+	p_rxdesc_curr->cmd_sts = BUF_OWNED_BY_DMA | RX_EN_INT;
+	p_rxdesc_curr->buf_size = PKTSIZE_ALIGN;
+	p_rxdesc_curr->byte_cnt = 0;
+
+	writel((unsigned int) p_rxdesc_curr->nxtdesc_p, &(dpxa->p_rxdesc_curr));
+
+	return 0;
+}
+
+int pxa_fec_initialize()
+{
+	struct pxa_device *dpxa;
+	struct eth_device *dev;
+	int phy_adr;
+	char *s = "ethaddr";
+
+	dpxa = malloc(sizeof(struct pxa_device));
+	if (!dpxa)
+		goto error1;
+
+	memset(dpxa, 0, sizeof(struct pxa_device));
+
+	dpxa->htpr = memalign(8, HASH_ADDR_TABLE_SIZE);
+	if (!dpxa->htpr)
+		goto error2;
+
+	dpxa->p_rxdesc =
+		(struct rx_desc *) memalign(PKTALIGN,
+					PXA_RXQ_DESC_ALIGNED_SIZE
+					* RINGSZ + 1);
+	if (!dpxa->p_rxdesc)
+		goto error3;
+
+	dpxa->p_rxbuf = (u8 *) memalign(PKTALIGN, RINGSZ
+						* PKTSIZE_ALIGN + 1);
+	if (!dpxa->p_rxbuf)
+		goto error4;
+
+	dpxa->p_aligned_txbuf = memalign(8, PKTSIZE_ALIGN);
+	if (!dpxa->p_aligned_txbuf)
+		goto error5;
+
+	dpxa->p_txdesc = (struct tx_desc *)
+		memalign(PKTALIGN, sizeof(struct tx_desc) + 1);
+	if (!dpxa->p_txdesc) {
+		free(dpxa->p_aligned_txbuf);
+error5:
+		free(dpxa->p_rxbuf);
+error4:
+		free(dpxa->p_rxdesc);
+error3:
+		free(dpxa->htpr);
+error2:
+		free(dpxa);
+error1:
+		printf("Err.. %s Failed to allocate memory\n",
+			__func__);
+		return -1;
+	}
+
+	dev = &dpxa->dev;
+	/* Assign PXA Fast Ethernet Controller Base Address */
+	dpxa->regs = (void *) PXA_FEC_BASE;
+
+	/* must be less than NAMESIZE (16) */
+	strcpy(dev->name, "pxa-fec0");
+
+	while (!eth_getenv_enetaddr(s, dev->enetaddr)) {
+		/* Generate Random Private MAC addr if
+		 * not set - TODO */
+		dev->enetaddr[0] = 0x00;
+		dev->enetaddr[1] = 0x50;
+		dev->enetaddr[2] = 0x43;
+		dev->enetaddr[3] = 0x11;
+		dev->enetaddr[4] = 0x22;
+		dev->enetaddr[5] = 0x33;
+		eth_setenv_enetaddr(s, dev->enetaddr);
+	}
+
+	dev->init = (void *) pxa_init;
+	dev->halt = (void *) pxa_halt;
+	dev->send = (void *) pxa_send;
+	dev->recv = (void *) pxa_recv;
+
+	eth_register(dev);
+#if defined(CONFIG_MII) || defined(CONFIG_CMD_MII)
+	miiphy_register(dev->name, smi_reg_read, smi_reg_write);
+
+#if defined(CONFIG_PHY_BASE_ADR)
+	miiphy_write(dev->name, PHY_ADR_REQ, PHY_ADR_REQ,
+		     (u16) CONFIG_PHY_BASE_ADR);
+#else
+	/* Search phy address from range 0-31 */
+	phy_adr = ethernet_phy_detect(dev);
+	if (phy_adr < 0) {
+		printf("Error: PHY not detected at address range 0-31\n");
+		return -1;
+	} else {
+		debug("PHY detected@addr %d\n", phy_adr);
+		miiphy_write(dev->name, PHY_ADR_REQ, PHY_ADR_REQ,
+			     (u16) phy_adr);
+	}
+#endif
+#endif
+	return 0;
+}
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
@@ -0,0 +1,239 @@
+/**************************************************************************
+ *
+ * Copyright (c) 2009, 2010 Marvell International Ltd.
+ *
+ * This file is part of GNU program.
+ *
+ * GNU program is free software: you can redistribute it and/or modify it
+ * under the terms of the GNU General Public License as published by the
+ * Free Software Foundation, either version 2 of the License, or (at your
+ * option) any later version.
+ *
+ * GNU program is distributed in the hope that it will be useful, but
+ * WITHOUT ANY WARRANTY; without even the implied warranty of
+ * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the GNU General
+ * Public License for more details.
+ *
+ * You should have received a copy of the GNU General Public License along
+ * with this program.
+ *
+ * If not, see http://www.gnu.org/licenses/old-licenses/gpl-2.0.html
+ *
+ *************************************************************************/
+
+/*
+ * (C) Copyright 2010
+ * Marvell Semiconductor <www.marvell.com>
+ * Written-by: Mahavir Jain <mjain@marvell.com>
+ *
+ * See file CREDITS for list of people who contributed to this
+ * project.
+ *
+ * This program is free software; you can redistribute it and/or
+ * modify it under the terms of the GNU General Public License as
+ * published by the Free Software Foundation; either version 2 of
+ * the License, or (at your option) any later version.
+ *
+ * This program is distributed in the hope that it will be useful,
+ * but WITHOUT ANY WARRANTY; without even the implied warranty of
+ * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the
+ * GNU General Public License for more details.
+ *
+ * You should have received a copy of the GNU General Public License
+ * along with this program; if not, write to the Free Software
+ * Foundation, Inc., 51 Franklin Street, Fifth Floor, Boston,
+ * MA 02110-1301 USA
+ */
+
+#ifndef __ETH_H__
+#define __ETH_H__
+
+#define PXA_FEC_BASE		0xC0800000
+#define PORT_NUM		0x0
+
+/* RX & TX descriptor command */
+#define BUF_OWNED_BY_DMA        (1<<31)
+
+/* RX descriptor status */
+#define RX_EN_INT               (1<<23)
+#define RX_FIRST_DESC           (1<<17)
+#define RX_LAST_DESC            (1<<16)
+#define RX_ERROR                (1<<15)
+
+/* TX descriptor command */
+#define TX_EN_INT               (1<<23)
+#define TX_GEN_CRC              (1<<22)
+#define TX_ZERO_PADDING         (1<<18)
+#define TX_FIRST_DESC           (1<<17)
+#define TX_LAST_DESC            (1<<16)
+#define TX_ERROR                (1<<15)
+
+/* smi register */
+#define SMI_BUSY                (1<<28)	/* 0 - Write, 1 - Read  */
+#define SMI_R_VALID             (1<<27)	/* 0 - Write, 1 - Read  */
+#define SMI_OP_W                (0<<26)	/* Write operation      */
+#define SMI_OP_R                (1<<26)	/* Read operation */
+
+#define HASH_ADD                0
+#define HASH_DELETE             1
+#define HASH_ADDR_TABLE_SIZE    0x4000	/* 16K (1/2K address - PCR_HS == 1) */
+#define HOP_NUMBER              12
+
+#define PHY_WAIT_ITERATIONS     1000	/* 1000 iterations * 10uS = 10mS max */
+#define PHY_WAIT_MICRO_SECONDS  10
+
+#define ETH_HW_IP_ALIGN         2	/* hw aligns IP header */
+#define ETH_EXTRA_HEADER        (6+6+2+4)
+					/* dest+src addr+protocol id+crc */
+#define MAX_PKT_SIZE            1536
+
+
+/* Bit definitions of the SDMA Config Reg */
+#define SDCR_BSZ_OFF            12
+#define SDCR_BSZ8               (3<<SDCR_BSZ_OFF)
+#define SDCR_BSZ4               (2<<SDCR_BSZ_OFF)
+#define SDCR_BSZ2               (1<<SDCR_BSZ_OFF)
+#define SDCR_BSZ1               (0<<SDCR_BSZ_OFF)
+#define SDCR_BLMR               (1<<6)
+#define SDCR_BLMT               (1<<7)
+#define SDCR_RIFB               (1<<9)
+#define SDCR_RC_OFF             2
+#define SDCR_RC_MAX_RETRANS     (0xf << SDCR_RC_OFF)
+
+/* SDMA_CMD */
+#define SDMA_CMD_AT             (1<<31)
+#define SDMA_CMD_TXDL           (1<<24)
+#define SDMA_CMD_TXDH           (1<<23)
+#define SDMA_CMD_AR             (1<<15)
+#define SDMA_CMD_ERD            (1<<7)
+
+
+/* Bit definitions of the Port Config Reg */
+#define PCR_HS                  (1<<12)
+#define PCR_EN                  (1<<7)
+#define PCR_PM                  (1<<0)
+
+/* Bit definitions of the Port Config Extend Reg */
+#define PCXR_2BSM               (1<<28)
+#define PCXR_DSCP_EN            (1<<21)
+#define PCXR_MFL_1518           (0<<14)
+#define PCXR_MFL_1536           (1<<14)
+#define PCXR_MFL_2048           (2<<14)
+#define PCXR_MFL_64K            (3<<14)
+#define PCXR_FLP                (1<<11)
+#define PCXR_PRIO_TX_OFF        3
+#define PCXR_TX_HIGH_PRI        (7<<PCXR_PRIO_TX_OFF)
+
+/*
+ *  * Bit definitions of the Interrupt Cause Reg
+ *   * and Interrupt MASK Reg is the same
+ *    */
+#define ICR_RXBUF               (1<<0)
+#define ICR_TXBUF_H             (1<<2)
+#define ICR_TXBUF_L             (1<<3)
+#define ICR_TXEND_H             (1<<6)
+#define ICR_TXEND_L             (1<<7)
+#define ICR_RXERR               (1<<8)
+#define ICR_TXERR_H             (1<<10)
+#define ICR_TXERR_L             (1<<11)
+#define ICR_TX_UDR              (1<<13)
+#define ICR_MII_CH              (1<<28)
+
+#define ALL_INTS (ICR_TXBUF_H  | ICR_TXBUF_L  | ICR_TX_UDR |\
+				ICR_TXERR_H  | ICR_TXERR_L |\
+				ICR_TXEND_H  | ICR_TXEND_L |\
+				ICR_RXBUF | ICR_RXERR  | ICR_MII_CH)
+
+#define PHY_MASK               0x0000001f
+
+#define to_dpxa(_kd) container_of(_kd, struct pxa_device, dev)
+/* Size of a Tx/Rx descriptor used in chain list data structure */
+#define PXA_RXQ_DESC_ALIGNED_SIZE \
+	(((sizeof(struct rx_desc) / PKTALIGN) + 1) * PKTALIGN)
+
+#define PXA_WR(val, adr)	writel(val, &adr)
+#define PXA_RD(adr)		readl(&adr)
+#define RX_BUF_OFFSET		0x2
+#define RXQ			0x0	/* RX Queue 0 */
+#define TXQ			0x1	/* TX Queue 1 */
+
+struct addr_table_entry_t {
+	u32 lo;
+	u32 hi;
+};
+
+/* Bit fields of a Hash Table Entry */
+enum hash_table_entry {
+	hteValid = 1,
+	hteSkip = 2,
+	hteRD = 4,
+	hteRDBit = 2
+};
+
+struct tx_desc {
+	u32 cmd_sts;		/* Command/status field */
+	u16 reserved;
+	u16 byte_cnt;		/* buffer byte count */
+	u8 *buf_ptr;		/* pointer to buffer for this descriptor */
+	struct tx_desc *nextdesc_p;	/* Pointer to next descriptor */
+};
+
+struct rx_desc {
+	u32 cmd_sts;		/* Descriptor command status */
+	u16 byte_cnt;		/* Descriptor buffer byte count */
+	u16 buf_size;		/* Buffer size */
+	u8 *buf_ptr;		/* Descriptor buffer pointer */
+	struct rx_desc *nxtdesc_p;	/* Next descriptor pointer */
+};
+
+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];
+};
+
+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 */
+};
+
+#endif				/* __ETH_H__ */
diff --git a/include/configs/gplugd.h b/include/configs/gplugd.h
index cc14f49..d2b91d7 100644
--- a/include/configs/gplugd.h
+++ b/include/configs/gplugd.h
@@ -62,8 +62,26 @@
 #define CONFIG_CMD_I2C
 #define CONFIG_CMD_AUTOSCRIPT
 #undef CONFIG_CMD_FPGA
-#undef CONFIG_CMD_NET
-#undef CONFIG_CMD_NFS
+
+/* Network Parameters */
+#define CONFIG_CMD_PING
+#define CONFIG_NET_MULTI
+#define CONFIG_IPADDR			192.168.9.51
+#define CONFIG_SERVERIP			192.168.9.91
+#define CONFIG_ETHADDR			"F0:AD:4E:00:32:9C"
+#define CONFIG_MII
+#define CONFIG_CMD_MII
+#define CONFIG_RESET_PHY_R
+#define CONFIG_PXA_ETH
+
+/* DHCP Support */
+#define CONFIG_CMD_DHCP
+#define CONFIG_BOOTP_DHCP_REQUEST_DELAY	50000
+#define CONFIG_BOOTP_SERVERIP
+
+/* Default Boot Parameters */
+#define CONFIG_ROOTPATH			"/tftpboot"
+#define CONFIG_SYS_IMG_NAME		"uImage"
 
 /*
  * mv-common.h should be defined after CMD configs since it used them
diff --git a/include/netdev.h b/include/netdev.h
index 6f0a971..378192d 100644
--- a/include/netdev.h
+++ b/include/netdev.h
@@ -94,6 +94,7 @@ int xilinx_emaclite_initialize (bd_t *bis, int base_addr);
 int sh_eth_initialize(bd_t *bis);
 int dm9000_initialize(bd_t *bis);
 int fecmxc_initialize(bd_t *bis);
+int pxa_fec_initialize(void);
 
 /* Boards with PCI network controllers can call this from their board_eth_init()
  * function to initialize whatever's on board.
-- 
1.7.0.4

^ permalink raw reply related	[flat|nested] 14+ messages in thread

* [U-Boot] [PATCH 1/4] Armada100: Ethernet support for Marvell gplugD
  2011-07-08  6:22 [U-Boot] [PATCH 1/4] Armada100: Ethernet support for Marvell gplugD Ajay Bhargav
@ 2011-07-08  7:47 ` Wolfgang Denk
  2011-07-08 15:19 ` Prafulla Wadaskar
  1 sibling, 0 replies; 14+ messages in thread
From: Wolfgang Denk @ 2011-07-08  7:47 UTC (permalink / raw)
  To: u-boot

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

-- 
DENX Software Engineering GmbH,     MD: Wolfgang Denk & Detlev Zundel
HRB 165235 Munich, Office: Kirchenstr.5, D-82194 Groebenzell, Germany
Phone: (+49)-8142-66989-10 Fax: (+49)-8142-66989-80 Email: wd at denx.de
"Don't worry about people stealing your ideas. If your ideas are  any
good, you'll have to ram them down people's throats."  - Howard Aiken

^ permalink raw reply	[flat|nested] 14+ messages in thread

* [U-Boot] [PATCH 1/4] Armada100: Ethernet support for Marvell gplugD
  2011-07-08  6:22 [U-Boot] [PATCH 1/4] Armada100: Ethernet support for Marvell gplugD Ajay Bhargav
  2011-07-08  7:47 ` Wolfgang Denk
@ 2011-07-08 15:19 ` Prafulla Wadaskar
  2011-07-11  4:42   ` Ajay Bhargav
  1 sibling, 1 reply; 14+ messages in thread
From: Prafulla Wadaskar @ 2011-07-08 15:19 UTC (permalink / raw)
  To: u-boot



> -----Original Message-----
> From: Ajay Bhargav [mailto:ajay.bhargav at einfochips.com]
> Sent: Friday, July 08, 2011 11:53 AM
> To: Prafulla Wadaskar
> Cc: u-boot at lists.denx.de; Ajay Bhargav
> Subject: [PATCH 1/4] Armada100: Ethernet support for Marvell gplugD
> 
> 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 +

Spit this patch as
1. gpio: Add GPIO driver for Armada100
2. net: Add Fast Ethernet controller driver for Armada100
3. Armada100: Enable 88E3015 PHY support on GplugD
4. Armada100: Enable Ethernet support on GplugD

Also please mentioned the dependency of this code i.e. Gplugd patch which is not yet mainlined.

Regards..
Prafulla . .

^ permalink raw reply	[flat|nested] 14+ messages in thread

* [U-Boot] [PATCH 1/4] Armada100: Ethernet support for Marvell gplugD
  2011-07-08 15:19 ` Prafulla Wadaskar
@ 2011-07-11  4:42   ` Ajay Bhargav
  0 siblings, 0 replies; 14+ messages in thread
From: Ajay Bhargav @ 2011-07-11  4:42 UTC (permalink / raw)
  To: u-boot

Hi Prafulla,

Thank you so much for comments. I am already working on it.

Regards,
Ajay Bhargav

----- Original Message -----
From: "Prafulla Wadaskar" <prafulla@marvell.com>
To: "Ajay Bhargav" <ajay.bhargav@einfochips.com>
Cc: u-boot at lists.denx.de, "Ashish Karkare" <akarkare@marvell.com>, "Prabhanjan Sarnaik" <sarnaik@marvell.com>
Sent: Friday, July 8, 2011 8:49:00 PM
Subject: RE: [PATCH 1/4] Armada100: Ethernet support for Marvell gplugD



> -----Original Message-----
> From: Ajay Bhargav [mailto:ajay.bhargav at einfochips.com]
> Sent: Friday, July 08, 2011 11:53 AM
> To: Prafulla Wadaskar
> Cc: u-boot at lists.denx.de; Ajay Bhargav
> Subject: [PATCH 1/4] Armada100: Ethernet support for Marvell gplugD
> 
> 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 +

Spit this patch as
1. gpio: Add GPIO driver for Armada100
2. net: Add Fast Ethernet controller driver for Armada100
3. Armada100: Enable 88E3015 PHY support on GplugD
4. Armada100: Enable Ethernet support on GplugD

Also please mentioned the dependency of this code i.e. Gplugd patch which is not yet mainlined.

Regards..
Prafulla . .

-- 
This message has been scanned for viruses and
dangerous content by Clean Mail Gateway, and is
believed to be clean.

*************************************************************************************************************************************************************
einfochips Business Disclaimer : This e-mail message and all attachments transmitted with it are intended solely for the use of the addressee and may contain legally privileged and confidential information. If the reader of this message is not the intended recipient, or an employee or agent responsible for delivering this message to the intended recipient, you are hereby notified that any dissemination, distribution, copying, or other use of this message or its attachments is strictly prohibited. If you have received this message in error, please notify the sender immediately by replying to this message and please delete it from your computer. Any views expressed in this message are those of the individual sender unless otherwise stated. Company has taken enough precautions to prevent the spread of viruses. However the company accepts no liability for any damage caused by any virus transmitted by this email.
*************************************************************************************************************************************************************

^ permalink raw reply	[flat|nested] 14+ messages in thread

* [U-Boot] [PATCH 1/4] Armada100: Ethernet support for Marvell gplugD
  2011-07-08 10:17   ` Wolfgang Denk
@ 2011-07-08 15:06     ` Prafulla Wadaskar
  0 siblings, 0 replies; 14+ messages in thread
From: Prafulla Wadaskar @ 2011-07-08 15:06 UTC (permalink / raw)
  To: u-boot



> -----Original Message-----
> From: Wolfgang Denk [mailto:wd at denx.de]
> Sent: Friday, July 08, 2011 3:48 PM
> To: Ajay Bhargav
> Cc: Prafulla Wadaskar; u-boot at lists.denx.de
> Subject: Re: [U-Boot] [PATCH 1/4] Armada100: Ethernet support for
> Marvell gplugD
> 
> Dear Ajay Bhargav,
> 
> In message
> <1107056334.46206.1310117537118.JavaMail.root@ahm.einfochips.com> you
> wrote:
> >
> > I need some more clarification on the GPIO part. Most of the
> > architectures i see define their own way of working with GPIO. There
> > is no generic framework for GPIO as i see. I followed similar style
> 
> Yes, there is.  See asm/gpio.h for the generic GPIO API and
> common/cmd_gpio.c for the generic gpio command that uses it.
> 

Hi Ajay
You should create gpio driver support for armada followed by its usage in your patches.

Regards..
Prafulla . .

^ permalink raw reply	[flat|nested] 14+ messages in thread

* [U-Boot] [PATCH 1/4] Armada100: Ethernet support for Marvell gplugD
  2011-07-08 12:48 ` Ajay Bhargav
@ 2011-07-08 14:59   ` Prafulla Wadaskar
  0 siblings, 0 replies; 14+ messages in thread
From: Prafulla Wadaskar @ 2011-07-08 14:59 UTC (permalink / raw)
  To: u-boot



> -----Original Message-----
> From: Ajay Bhargav [mailto:ajay.bhargav at einfochips.com]
> Sent: Friday, July 08, 2011 6:19 PM
> To: Wolfgang Denk
> Cc: Prafulla Wadaskar; u-boot at lists.denx.de
> Subject: Re: [U-Boot] [PATCH 1/4] Armada100: Ethernet support for
> Marvell gplugD
> 
> Dear Wolfgang,
> 
> > Bad examples are no excuse for adding more bad code.
> got it :)
> 
> > Some things only get noticed when they already exist.  But once
> > noticed, we should not repeat the same mistakes.
> 
> I wouldn't have tried making that mistake if it was mentioned that
> earlier implementations are wrong. I just thought its a simple way to
> do a complex thing :)

Hi Ajay
You may find similar implementation for other architectures also (old code).
It doesn't mean earlier implementation is wrong.

This is a development policy, we encourage c-struct for register definition for any new code submission.

Regards..
Prafulla . .

^ permalink raw reply	[flat|nested] 14+ messages in thread

* [U-Boot] [PATCH 1/4] Armada100: Ethernet support for Marvell gplugD
       [not found] <422945680.47818.1310129034861.JavaMail.root@ahm.einfochips.com>
@ 2011-07-08 12:48 ` Ajay Bhargav
  2011-07-08 14:59   ` Prafulla Wadaskar
  0 siblings, 1 reply; 14+ messages in thread
From: Ajay Bhargav @ 2011-07-08 12:48 UTC (permalink / raw)
  To: u-boot

Dear Wolfgang,

> Bad examples are no excuse for adding more bad code.
got it :)

> Some things only get noticed when they already exist.  But once
> noticed, we should not repeat the same mistakes.

I wouldn't have tried making that mistake if it was mentioned that
earlier implementations are wrong. I just thought its a simple way to
do a complex thing :)

I will get back with updated patch. Thanks for your time sir.

Regards,
Ajay Bhargav

^ permalink raw reply	[flat|nested] 14+ messages in thread

* [U-Boot] [PATCH 1/4] Armada100: Ethernet support for Marvell gplugD
  2011-07-08 12:03 ` Ajay Bhargav
@ 2011-07-08 12:39   ` Wolfgang Denk
  0 siblings, 0 replies; 14+ messages in thread
From: Wolfgang Denk @ 2011-07-08 12:39 UTC (permalink / raw)
  To: u-boot

Dear Ajay Bhargav,

In message <107544326.47582.1310126592414.JavaMail.root@ahm.einfochips.com> you wrote:
> 
> I did get your point sir, but...
...
> asm/arch-kirkwood/gpio.h)>

Bad examples are no excuse for adding more bad code.

Some things only get noticed when they already exist.  But once
noticed, we should not repeat the same mistakes.

Best regards,

Wolfgang Denk

-- 
DENX Software Engineering GmbH,     MD: Wolfgang Denk & Detlev Zundel
HRB 165235 Munich, Office: Kirchenstr.5, D-82194 Groebenzell, Germany
Phone: (+49)-8142-66989-10 Fax: (+49)-8142-66989-80 Email: wd at denx.de
Lots of folks confuse bad management with destiny.   -- Frank Hubbard

^ permalink raw reply	[flat|nested] 14+ messages in thread

* [U-Boot] [PATCH 1/4] Armada100: Ethernet support for Marvell gplugD
       [not found] <751999513.47571.1310126396909.JavaMail.root@ahm.einfochips.com>
@ 2011-07-08 12:03 ` Ajay Bhargav
  2011-07-08 12:39   ` Wolfgang Denk
  0 siblings, 1 reply; 14+ messages in thread
From: Ajay Bhargav @ 2011-07-08 12:03 UTC (permalink / raw)
  To: u-boot

Dear Wolfdang,

I did get your point sir, but...

> To me this looks different.

> - Do not use a "base address + offset" notation (with a declartion of
>  register offsets in your header files), but use C structs instead to
>  describe the regioster layout, and then use proper I/O accessor
>  functions on them.

asm/arch-kirkwood/gpio.h)>
#define GPIO_OFF(pin)           (((pin) >> 5) ? 0x0040 : 0x0000)
#define GPIO_OUT(pin)           (KW_GPIO0_BASE + GPIO_OFF(pin) + 0x00)
#define GPIO_IO_CONF(pin)       (KW_GPIO0_BASE + GPIO_OFF(pin) + 0x04)


#define GPIO_REG(x)     (GPIO_REGS_VIRT + (x))
#define GPLR(x)         GPIO_REG(BANK_OFF((x) >> 5) + 0x00)
#define GPDR(x)         GPIO_REG(BANK_OFF((x) >> 5) + 0x0c)

> - Do not implement your own GPIO framework, but instead use existing
>  code.

I did not tried anything on my own. I just tried similar things which were done
by others already. I hope I am able to highlight what i am trying to say.

Regards,
Ajay Bhargav

^ permalink raw reply	[flat|nested] 14+ messages in thread

* [U-Boot] [PATCH 1/4] Armada100: Ethernet support for Marvell gplugD
  2011-07-08 10:23 ` Ajay Bhargav
@ 2011-07-08 11:29   ` Wolfgang Denk
  0 siblings, 0 replies; 14+ messages in thread
From: Wolfgang Denk @ 2011-07-08 11:29 UTC (permalink / raw)
  To: u-boot

Dear Ajay Bhargav,

In message <867041231.46865.1310120597114.JavaMail.root@ahm.einfochips.com> you wrote:
> 
> Thanks for info, I have seen that. But i am not adding support for GPIO commands.

Like the gpio command, your code should use the same generic gpio API.

> In reference to 
> 
> >> +#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.
> 
> I have followed similar way of using GPIO as used by "drivers/gpio/kw_gpio.c" and defined in "include/asm/arch-kirkwood/gpio.h",

To me this looks different.

> so actually i am not adding GPIO support rather i wrote functions that are required by the Ethernet and other drivers.

You are not getting the point.  I'm asking for two things:

- Do not use a "base address + offset" notation (with a declartion of
  register offsets in your header files), but use C structs instead to
  describe the regioster layout, and then use proper I/O accessor
  functions on them.

- Do not implement your own GPIO framework, but instead use existing
  code.

Best regards,

Wolfgang Denk

-- 
DENX Software Engineering GmbH,     MD: Wolfgang Denk & Detlev Zundel
HRB 165235 Munich, Office: Kirchenstr.5, D-82194 Groebenzell, Germany
Phone: (+49)-8142-66989-10 Fax: (+49)-8142-66989-80 Email: wd at denx.de
Unix is like a toll road on which you have to stop every 50  feet  to
pay another nickel. But hey! You only feel 5 cents poorer each time.
                 - Larry Wall in <1992Aug13.192357.15731@netlabs.com>

^ permalink raw reply	[flat|nested] 14+ messages in thread

* [U-Boot] [PATCH 1/4] Armada100: Ethernet support for Marvell gplugD
       [not found] <1773826644.46838.1310120108778.JavaMail.root@ahm.einfochips.com>
@ 2011-07-08 10:23 ` Ajay Bhargav
  2011-07-08 11:29   ` Wolfgang Denk
  0 siblings, 1 reply; 14+ messages in thread
From: Ajay Bhargav @ 2011-07-08 10:23 UTC (permalink / raw)
  To: u-boot

Dear Wolfgang,

Thanks for info, I have seen that. But i am not adding support for GPIO commands.

In reference to 

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

I have followed similar way of using GPIO as used by "drivers/gpio/kw_gpio.c" and defined in "include/asm/arch-kirkwood/gpio.h",
so actually i am not adding GPIO support rather i wrote functions that are required by the Ethernet and other drivers.

Regards,
Ajay Bhargav

^ permalink raw reply	[flat|nested] 14+ messages in thread

* [U-Boot] [PATCH 1/4] Armada100: Ethernet support for Marvell gplugD
  2011-07-08  9:32 ` Ajay Bhargav
@ 2011-07-08 10:17   ` Wolfgang Denk
  2011-07-08 15:06     ` Prafulla Wadaskar
  0 siblings, 1 reply; 14+ messages in thread
From: Wolfgang Denk @ 2011-07-08 10:17 UTC (permalink / raw)
  To: u-boot

Dear Ajay Bhargav,

In message <1107056334.46206.1310117537118.JavaMail.root@ahm.einfochips.com> you wrote:
>
> I need some more clarification on the GPIO part. Most of the
> architectures i see define their own way of working with GPIO. There
> is no generic framework for GPIO as i see. I followed similar style

Yes, there is.  See asm/gpio.h for the generic GPIO API and
common/cmd_gpio.c for the generic gpio command that uses it.

> ----- Original Message -----
> From: "Wolfgang Denk" <wd@denx.de>
> To: "Ajay Bhargav" <ajay.bhargav@einfochips.com>
> Cc: prafulla at marvell.com, u-boot at lists.denx.de
> Sent: Friday, July 8, 2011 1:17:50 PM
> Subject: Re: [U-Boot] [PATCH 1/4] Armada100: Ethernet support for Marvell gplugD
...

And *please* STOP top posting / full quoting!!!

Best regards,

Wolfgang Denk

-- 
DENX Software Engineering GmbH,     MD: Wolfgang Denk & Detlev Zundel
HRB 165235 Munich, Office: Kirchenstr.5, D-82194 Groebenzell, Germany
Phone: (+49)-8142-66989-10 Fax: (+49)-8142-66989-80 Email: wd at denx.de
Life. Don't talk to me about life.      - Marvin the Paranoid Android

^ permalink raw reply	[flat|nested] 14+ messages in thread

* [U-Boot] [PATCH 1/4] Armada100: Ethernet support for Marvell gplugD
       [not found] <1707892633.46126.1310116683428.JavaMail.root@ahm.einfochips.com>
@ 2011-07-08  9:32 ` Ajay Bhargav
  2011-07-08 10:17   ` Wolfgang Denk
  0 siblings, 1 reply; 14+ messages in thread
From: Ajay Bhargav @ 2011-07-08  9:32 UTC (permalink / raw)
  To: u-boot

Dear Wolfgang,

I need some more clarification on the GPIO part. Most of the architectures i see define their own way of working with GPIO. There is no generic framework for GPIO as i see. I followed similar style used in drivers/gpio/kw_gpio.c they however used different names so its not so easy to make out that they are actually GPIO registers.

Regards,
Ajay Bhargav


----- Original Message -----
From: "Wolfgang Denk" <wd@denx.de>
To: "Ajay Bhargav" <ajay.bhargav@einfochips.com>
Cc: prafulla at marvell.com, u-boot at lists.denx.de
Sent: Friday, July 8, 2011 1:17:50 PM
Subject: Re: [U-Boot] [PATCH 1/4] Armada100: Ethernet support for Marvell gplugD

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

-- 
DENX Software Engineering GmbH,     MD: Wolfgang Denk & Detlev Zundel
HRB 165235 Munich, Office: Kirchenstr.5, D-82194 Groebenzell, Germany
Phone: (+49)-8142-66989-10 Fax: (+49)-8142-66989-80 Email: wd at denx.de
"Don't worry about people stealing your ideas. If your ideas are  any
good, you'll have to ram them down people's throats."  - Howard Aiken

-- 
This message has been scanned for viruses and
dangerous content by Clean Mail Gateway, and is
believed to be clean.

*************************************************************************************************************************************************************
einfochips Business Disclaimer : This e-mail message and all attachments transmitted with it are intended solely for the use of the addressee and may contain legally privileged and confidential information. If the reader of this message is not the intended recipient, or an employee or agent responsible for delivering this message to the intended recipient, you are hereby notified that any dissemination, distribution, copying, or other use of this message or its attachments is strictly prohibited. If you have received this message in error, please notify the sender immediately by replying to this message and please delete it from your computer. Any views expressed in this message are those of the individual sender unless otherwise stated. Company has taken enough precautions to prevent the spread of viruses. However the company accepts no liability for any damage caused by any virus transmitted by this email.
*************************************************************************************************************************************************************

^ permalink raw reply	[flat|nested] 14+ messages in thread

* [U-Boot] [PATCH 1/4] Armada100: Ethernet support for Marvell gplugD
       [not found] <1361369926.45813.1310113039881.JavaMail.root@ahm.einfochips.com>
@ 2011-07-08  8:17 ` Ajay Bhargav
  0 siblings, 0 replies; 14+ messages in thread
From: Ajay Bhargav @ 2011-07-08  8:17 UTC (permalink / raw)
  To: u-boot

Dear Wolfgang Denk,

Thank you so much for your comments. I will do the changes carefully.

Thanks & Regards,
Ajay Bhargav

----- Original Message -----
From: "Wolfgang Denk" <wd@denx.de>
To: "Ajay Bhargav" <ajay.bhargav@einfochips.com>
Cc: prafulla at marvell.com, u-boot at lists.denx.de
Sent: Friday, July 8, 2011 1:17:50 PM
Subject: Re: [U-Boot] [PATCH 1/4] Armada100: Ethernet support for Marvell gplugD

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

-- 
DENX Software Engineering GmbH,     MD: Wolfgang Denk & Detlev Zundel
HRB 165235 Munich, Office: Kirchenstr.5, D-82194 Groebenzell, Germany
Phone: (+49)-8142-66989-10 Fax: (+49)-8142-66989-80 Email: wd at denx.de
"Don't worry about people stealing your ideas. If your ideas are  any
good, you'll have to ram them down people's throats."  - Howard Aiken

^ permalink raw reply	[flat|nested] 14+ messages in thread

end of thread, other threads:[~2011-07-11  4:42 UTC | newest]

Thread overview: 14+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2011-07-08  6:22 [U-Boot] [PATCH 1/4] Armada100: Ethernet support for Marvell gplugD Ajay Bhargav
2011-07-08  7:47 ` Wolfgang Denk
2011-07-08 15:19 ` Prafulla Wadaskar
2011-07-11  4:42   ` Ajay Bhargav
     [not found] <1361369926.45813.1310113039881.JavaMail.root@ahm.einfochips.com>
2011-07-08  8:17 ` Ajay Bhargav
     [not found] <1707892633.46126.1310116683428.JavaMail.root@ahm.einfochips.com>
2011-07-08  9:32 ` Ajay Bhargav
2011-07-08 10:17   ` Wolfgang Denk
2011-07-08 15:06     ` Prafulla Wadaskar
     [not found] <1773826644.46838.1310120108778.JavaMail.root@ahm.einfochips.com>
2011-07-08 10:23 ` Ajay Bhargav
2011-07-08 11:29   ` Wolfgang Denk
     [not found] <751999513.47571.1310126396909.JavaMail.root@ahm.einfochips.com>
2011-07-08 12:03 ` Ajay Bhargav
2011-07-08 12:39   ` Wolfgang Denk
     [not found] <422945680.47818.1310129034861.JavaMail.root@ahm.einfochips.com>
2011-07-08 12:48 ` Ajay Bhargav
2011-07-08 14:59   ` Prafulla Wadaskar

This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.