net: Adds Fast Ethernet Controller driver for Armada100

drivers/net/armada100_fec.c new file +5 -0

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

drivers/net/armada100_fec.c line 697

Quoted 2 lines · On Wednesday, August 24, 2011 09:07:18 Ajay Bhargav wrote:

drivers/net/armada100_fec.c new file +2 -0

+       darmdfec->p_rxdesc = (struct rx_desc *) memalign(PKTALIGN,
+                       ARMDFEC_RXQ_DESC_ALIGNED_SIZE * RINGSZ + 1);

memalign() returns a void*, so you shouldnt need to cast its return value (you do this a couple of times)

drivers/net/armada100_fec.c line 724

Quoted 2 lines

drivers/net/armada100_fec.c new file +2 -0

+	/* Read mac from env if available */
+	eth_getenv_enetaddr("ethaddr", dev->enetaddr);

you shouldnt need to do this. the higher layers will take care of this for you when you set write_hwaddr

also, it seems like some of my previous feedback wasnt addressed ?

Quoted 2 lines · Mike Frysinger

Quoted 1 line · On Wednesday, August 24, 2011 09:07:18 Ajay Bhargav wrote:

+ darmdfec->p_rxdesc = (struct rx_desc *)

memalign(PKTALIGN,

Quoted 1 line

Quoted 1 line

+ ARMDFEC_RXQ_DESC_ALIGNED_SIZE * RINGSZ +

1);

Quoted 2 lines

memalign() returns a void*, so you shouldnt need to cast its return

value

Quoted 6 lines

(you do this a couple of times)

drivers/net/armada100_fec.c line 724

Quoted 2 lines

drivers/net/armada100_fec.c new file +2 -0

+	/* Read mac from env if available */
+	eth_getenv_enetaddr("ethaddr", dev->enetaddr);

you shouldnt need to do this. the higher layers will take care of

this for

Quoted 3 lines

you when you set write_hwaddr

also, it seems like some of my previous feedback wasnt addressed ?

I have exactly the same feeling :-(

Ajay, please go through the feedback, if you don't understand something, just
ask instead of hoping we won't notice ... we will, we see everything ;-)

Cheers

Quoted 34 lines · ----- "Marek Vasut

Quoted 2 lines · Mike Frysinger

Quoted 1 line · On Wednesday, August 24, 2011 09:07:18 Ajay Bhargav wrote:

+ darmdfec->p_rxdesc = (struct rx_desc *)

memalign(PKTALIGN,

Quoted 1 line

Quoted 1 line

+ ARMDFEC_RXQ_DESC_ALIGNED_SIZE * RINGSZ +

1);

Quoted 1 line

memalign() returns a void*, so you shouldnt need to cast its return

value

Quoted 6 lines

(you do this a couple of times)

drivers/net/armada100_fec.c line 724

Quoted 2 lines

drivers/net/armada100_fec.c new file +2 -0

+	/* Read mac from env if available */
+	eth_getenv_enetaddr("ethaddr", dev->enetaddr);

you shouldnt need to do this. the higher layers will take care of

this for

Quoted 3 lines

you when you set write_hwaddr

also, it seems like some of my previous feedback wasnt addressed ?

I have exactly the same feeling :-(

Ajay, please go through the feedback, if you don't understand something, just
ask instead of hoping we won't notice ... we will, we see everything ;-)

Quoted 1 line

Cheers

I did go through your feedbacks.. My apologies if I missed. Marek, Its good for me that you guys are monitoring :) it will surely help me to write a better code :)

Thanks,
Ajay Bhargav

Review

Quoted 2 lines · On Wednesday, August 24, 2011 09:07:18 Ajay Bhargav wrote:

(quoted diff) +2 -0

+       darmdfec->p_rxdesc = (struct rx_desc *) memalign(PKTALIGN,
+                       ARMDFEC_RXQ_DESC_ALIGNED_SIZE * RINGSZ +

1);

memalign() returns a void*, so you shouldnt need to cast its return value (you
do this a couple of times)

drivers/net/armada100_fec.c line 724

Quoted 2 lines

drivers/net/armada100_fec.c new file +2 -0

+	/* Read mac from env if available */
+	eth_getenv_enetaddr("ethaddr", dev->enetaddr);

you shouldnt need to do this. the higher layers will take care of this for
you when you set write_hwaddr

also, it seems like some of my previous feedback wasnt addressed ?

drivers/net/armada100_fec.c line 610

Quoted 3 lines

drivers/net/armada100_fec.c new file +2 -0

+     while (cmd_sts & BUF_OWNED_BY_DMA) {
...
+     };

no semi-colon needed

drivers/net/armada100_fec.c line 718

Quoted 4 lines

drivers/net/armada100_fec.c new file +3 -0

+int armada100_fec_initialize()
+{
...
+     darmdfec->regs = (void *) ARMD1_FEC_BASE;

make the reg base a parameter to armada100_fec_initialize()

drivers/net/armada100_fec.c

Quoted 8 lines

drivers/net/armada100_fec.c new file +8 -0

+#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");

drivers/net/armada100_fec.c line 749

Quoted 7 lines

drivers/net/armada100_fec.c new file +7 -0

+             return -1;
+     } else {
+             debug("PHY detected at addr %d\n", phy_adr);
+             miiphy_write(dev->name, PHY_ADR_REQ, PHY_ADR_REQ,
+                          (u16) phy_adr);
+     }
+#endif

this should be done in the armdfec_init() func, not the initialize func
-mike

Quoted 7 lines · ----- "Mike Frysinger

drivers/net/armada100_fec.c line 724

Quoted 2 lines · On Wednesday, August 24, 2011 09:07:18 Ajay Bhargav wrote:

drivers/net/armada100_fec.c new file +2 -0

+	/* Read mac from env if available */
+	eth_getenv_enetaddr("ethaddr", dev->enetaddr);

you shouldnt need to do this. the higher layers will take care of this for
you when you set write_hwaddr

I do not have a hardware storage for MAC on my controller. write_hwaddr is not needed for me.

Quoted 6 lines

drivers/net/armada100_fec.c line 718

Quoted 4 lines

drivers/net/armada100_fec.c new file +3 -0

+int armada100_fec_initialize()
+{
...
+     darmdfec->regs = (void *) ARMD1_FEC_BASE;

make the reg base a parameter to armada100_fec_initialize()

This driver is for Armada100 series and base address is same for the whole series, so i did not feel passing it as a parameter. Can you please tell me if there is any specific reason for the same?

This patch adds support for Fast Ethernet Controller driver for Armada100 series.

Signed-off-by Ajay Bhargav <[email protected]>

---

drivers/net/armada100_fec.c new file +56 -0

new file mode 100644
index 0000000..e36dca6
@@ -0,0 +1,761 @@
+/*
+ * (C) Copyright 2011
+ * eInfochips Ltd. <www.einfochips.com>
+ * Written-by: Ajay Bhargav <[email protected]>
+ *
+ * (C) Copyright 2010
+ * Marvell Semiconductor <www.marvell.com>
+ * Contributor: Mahavir Jain <[email protected]>
+ *
+ * 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 <asm/types.h>
+#include <asm/byteorder.h>
+#include <linux/err.h>
+#include <linux/mii.h>
+#include <asm/io.h>
+#include <asm/arch/armada100.h>
+#include "armada100_fec.h"
+
+#define  PHY_ADR_REQ     0xFF	/* Magic number to read/write PHY address */
+
+#ifdef DEBUG
+static int eth_dump_regs(struct eth_device *dev)
+{
+	struct armdfec_device *darmdfec = to_darmdfec(dev);
+	struct armdfec_reg *regs = darmdfec->regs;
+	unsigned int i = 0;
+
+	printf("\noffset: phy_adr, value: 0x%x\n", readl(&regs->phyadr));
+	printf("offset: smi, value: 0x%x\n", readl(&regs->smi));
+	for (i = 0x400; i <= 0x4e4; i += 4)
+		printf("offset: 0x%x, value: 0x%x\n",
+			i, readl(ARMD1_FEC_BASE + i));
+	return 0;

drivers/net/armada100_fec.c new file +24 -0

+			break;
+		udelay(PHY_WAIT_MICRO_SECONDS);
+	}
+	return !timeout;
+}
+
+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 armdfec_device *darmdfec = to_darmdfec(dev);
+	struct armdfec_reg *regs = darmdfec->regs;
+	u32 val, reg_data;
+
+	if (phy_addr == PHY_ADR_REQ && phy_reg == PHY_ADR_REQ) {
+		reg_data = readl(&regs->phyadr);
+		*value = (u16) (reg_data & 0x1f);
+		return 0;
+	}
+
+	/* check parameters */
+	if (phy_addr > PHY_MASK) {
+		printf("Err..(%s) Invalid phy address: 0x%X\n",
+				__func__, phy_addr);

drivers/net/armada100_fec.c new file +59 -0

+
+	/* now wait for the data to be valid */
+	if (armdfec_phy_timeout((u32)&regs->smi, SMI_R_VALID, TRUE)) {
+		val = readl(&regs->smi);
+		printf("Err (%s) PHY Read timeout, val=0x%x\n", __func__, val);
+		return -1;
+	}
+	val = readl(&regs->smi);
+	*value = val & 0xffff;
+
+	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 armdfec_device *darmdfec = to_darmdfec(dev);
+	struct armdfec_reg *regs = darmdfec->regs;
+
+	if (phy_addr == PHY_ADR_REQ && phy_reg == PHY_ADR_REQ) {
+		clrsetbits_le32(&regs->phyadr, 0x1f, value & 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 */
+	if (armdfec_phy_timeout((u32)&regs->smi, SMI_BUSY, FALSE)) {
+		printf("Error (%s) PHY busy timeout\n",	__func__);
+		return -1;
+	}
+
+	writel(phy_addr << 16 | phy_reg << 21 | SMI_OP_W | (value & 0xffff),
+			&regs->smi);
+	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 armdfec_device *darmdfec = to_darmdfec(dev);
+	struct armdfec_reg *regs = darmdfec->regs;
+	int delay;
+	int maxretries = 40;
+
+	do {

drivers/net/armada100_fec.c new file +4 -0

+
+	if (maxretries <= 0)
+		printf("%s : DMA Stuck\n", __func__);
+}

drivers/net/armada100_fec.c new file +14 -0

+int armada100_fec_register()
+{
+	struct armdfec_device *darmdfec;
+	struct eth_device *dev;
+	int phy_adr;
+
+	darmdfec = malloc(sizeof(struct armdfec_device));
+	if (!darmdfec)
+		goto error;
+
+	memset(darmdfec, 0, sizeof(struct armdfec_device));
+
+	darmdfec->htpr = memalign(8, HASH_ADDR_TABLE_SIZE);
+	if (!darmdfec->htpr)

drivers/net/armada100_fec.c new file +5 -0

+	if (!darmdfec->p_rxbuf)
+		goto error;
+
+	darmdfec->p_aligned_txbuf = memalign(8, PKTSIZE_ALIGN);
+	if (!darmdfec->p_aligned_txbuf)