Marvell MV88E61XX Switch Driver support

Chips supported:-
1. 88E6161 6 port gbe swtich with 5 integrated PHYs
2. 88E6165 6 port gbe swtich with 5 integrated PHYs
2. 88E6132 3 port gbe swtich with 2 integrated PHYs Platform specific configuration supported

Note: This driver is supported and tested against kirkwood egiga interface

Contributors:
Yotam Admon <[email protected]>
Michael Blostein <[email protected]

Reviewed by: Ronen Shitrit <[email protected]>

Signed-off-by Prafulla Wadaskar <[email protected]>

---
Changelog:-
v2: updated as per review comments for v1 removed other two drivers form earlier patch debug_prints removed
driver moved to drivers/net/

v3: updated as per review comments for v2
miiphy interface used, platform specific dependency resolved Chip id detection and printing added
common code forked out
some cosmetic and magic number fixes

v4: updated as per review comments for v3 mv88e61xx.h added
platform specific configuration support added some more documentation references provided cleaned rgmii delay enable related code

-----Original Message-----
From: Jean-Christophe PLAGNIOL-VILLARD [mailto:plagnioj at jcrosoft.com]
Sent: Friday, April 17, 2009 1:06 PM
To: Prafulla Wadaskar
Cc: u-boot at lists.denx.de; Ashish Karkare; Ronen Shitrit; Ben Warren
Subject: Re: [U-Boot] [PATCH v4] Marvell MV88E61XX Switch Driver support

Quoted 2 lines · Prafulla Wadaskar

Chips supported:-
1. 88E6161 6 port gbe swtich with 5 integrated PHYs 2.

88E6165 6 port

Quoted 1 line

gbe swtich with 5 integrated PHYs 2. 88E6132 3 port gbe

swtich with 2

Quoted 10 lines

integrated PHYs Platform specific configuration supported

Note: This driver is supported and tested against kirkwood egiga interface

Contributors:
Yotam Admon <[email protected]>
Michael Blostein <[email protected]

Reviewed by: Ronen Shitrit <[email protected]>

Quoted 8 lines

Signed-off-by Prafulla Wadaskar <[email protected]>

---
Changelog:-
v2: updated as per review comments for v1 removed other two drivers form earlier patch debug_prints removed driver moved to drivers/net/

v3: updated as per review comments for v2 miiphy interface used, platform specific dependency resolved Chip id detection and

printing

Quoted 2 lines

added common code forked out some cosmetic and magic number fixes

Quoted 1 line

v4: updated as per review comments for v3 mv88e61xx.h added

platform

Quoted 2 lines

specific configuration support added some more documentation references provided cleaned rgmii delay enable related code

this version looks better

I've just 2 comments on it

1) drivers/net/phy/ is for phylib and drivers as a switch can be consider as a multiple phy port please move it there

2) the Vlan init is not so clear for people who need to maintain or update the code as the Marvell's switch datasheets are all under NDA

Best Regards,
J.

Chips supported:-
1. 88E6161 6 port gbe swtich with 5 integrated PHYs
2. 88E6165 6 port gbe swtich with 5 integrated PHYs
2. 88E6132 3 port gbe swtich with 2 integrated PHYs Platform specific configuration supported default and router port vlan config supported

Note: This driver is supported and tested against
kirkwood egiga interface

Contributors:
Yotam Admon <[email protected]>
Michael Blostein <[email protected]

Reviewed by: Ronen Shitrit <[email protected]>

Signed-off-by Prafulla Wadaskar <[email protected]>

---
Changelog:-
v2: updated as per review comments for v1
removed other two drivers form earlier patch
debug_prints removed
driver moved to drivers/net/

v3: updated as per review comments for v2
miiphy interface used, platform specific dependency resolved Chip id detection and printing added
common code forked out
some cosmetic and magic number fixes

v4: updated as per review comments for v3
mv88e61xx.h added
platform specific configuration support added
some more documentation references provided
cleaned rgmii delay enable related code

v5: updated as per review comments for v3 code moved to drivers/net/phy/
vlan_init changed to vlan_config and platform specific

 drivers/net/phy/Makefile    |    1 +
 drivers/net/phy/mv88e61xx.c |  313 +++++++++++++++++++++++++++++++++++++++++++
 drivers/net/phy/mv88e61xx.h |   73 ++++++++++
 3 files changed, 387 insertions(+), 0 deletions(-)
 create mode 100644 drivers/net/phy/mv88e61xx.c
 create mode 100644 drivers/net/phy/mv88e61xx.h

drivers/net/phy/Makefile +1 -0

index 4fe3b05..3b92614 100644
@@ -26,6 +26,7 @@ include $(TOPDIR)/config.mk
 LIB	:= $(obj)libphy.a
 
 COBJS-$(CONFIG_BITBANGMII) += miiphybb.o
+COBJS-$(CONFIG_MV88E61XX_SWITCH) += mv88e61xx.o
 
 COBJS	:= $(COBJS-y)
 SRCS	:= $(COBJS:.o=.c)

drivers/net/phy/mv88e61xx.c new file +34 -0

new file mode 100644
index 0000000..8ba2291
@@ -0,0 +1,313 @@
+/*
+ * (C) Copyright 2009
+ * Marvell Semiconductor <www.marvell.com>
+ * Prafulla Wadaskar <[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 <miiphy.h>
+#include "mv88e61xx.h"
+
+/* Chip Address mode
+ * The Switch support two modes of operation
+ * 1. single chip mode and
+ * 2. Multi-chip mode
+ * Refer section 9.2 &9.3 in chip datasheet-02 for more details
+ *

drivers/net/phy/mv88e61xx.c new file +44 -0

+	u32 reg = 0;
+	u32 timeout = MV88E61XX_PHY_TIMEOUT;
+
+	/* Poll till SMIBusy bit is clear */
+	do {
+		miiphy_read(name, devaddr, 0x0, &reg);
+		if (timeout-- == 0) {
+			printf("SMI busy timeout\n");
+			return -1;
+		}
+	} while (reg & BIT15);
+	return 0;
+}
+
+static void mv88e61xx_wr_phy(char *name, u32 phy_adr, u32 reg_ofs, u16 data)
+{
+	u16 reg;
+	u32 mii_dev_addr;
+
+	/* command to read PHY dev address */
+	if (!miiphy_read(name, 0xEE, 0xEE, &mii_dev_addr)) {
+		printf("Error..could not read PHY dev address\n");
+		return;
+	}
+	mv88e61xx_busychk_multic(mii_dev_addr);
+	/* Write data to Switch indirect data register */
+	miiphy_write(name, mii_dev_addr, 0x1, data);
+	/* Write command to Switch indirect command register (write) */
+	miiphy_write(name, mii_dev_addr, 0x0,
+		     reg_ofs | (phy_adr << 5) | BIT10 | BIT12 | BIT15);
+}
+
+static void mv88e61xx_rd_phy(char *name, u32 phy_adr, u32 reg_ofs, u16 * data)
+{
+	u16 reg;
+	u32 mii_dev_addr;
+
+	/* command to read PHY dev address */
+	if (!miiphy_read(name, 0xEE, 0xEE, &mii_dev_addr)) {
+		printf("Error..could not read PHY dev address\n");
+		return;
+	}
+	mv88e61xx_busychk_multic(mii_dev_addr);
+	/* Write command to Switch indirect command register (read) */

drivers/net/phy/mv88e61xx.c new file +115 -0

+{
+	u32 prt;
+	u16 reg;
+	char *name = swconfig->name;
+	u32 cpu_port = swconfig->cpuport;
+	u32 port_mask = swconfig->ports_enabled;
+	enum mv88f61xx_cfg_vlan vlancfg = swconfig->vlancfg;
+
+	/* be sure all ports are disabled */
+	for (prt = 0; prt < max_prtnum; prt++) {
+		mv88e61xx_rd_phy(name, ports_ofs + prt, MV88E61XX_PRT_CTRL_REG,
+				 &reg);
+		reg &= ~0x3;
+		mv88e61xx_wr_phy(name, ports_ofs + prt, MV88E61XX_PRT_CTRL_REG,
+				 reg);
+	}
+	/* Set CPU port VID to 0x1 */
+	mv88e61xx_rd_phy(name, (ports_ofs + cpu_port), MV88E61XX_PRT_VID_REG,
+			 &reg);
+	reg &= ~0xfff;
+	reg |= 0x1;
+	mv88e61xx_wr_phy(name, (ports_ofs + cpu_port), MV88E61XX_PRT_VID_REG,
+			 reg);
+
+	/* Setting  Port default priority for all ports to zero */
+	for (prt = 0; prt < max_prtnum; prt++) {
+		mv88e61xx_rd_phy(name, ports_ofs + prt, MV88E61XX_PRT_VID_REG,
+				 &reg);
+		reg &= ~0xc000;
+		mv88e61xx_wr_phy(name, ports_ofs + prt, MV88E61XX_PRT_VID_REG,
+				 reg);
+	}
+	/* Setting VID and VID map for all ports except CPU port */
+	for (prt = 0; prt < max_prtnum; prt++) {
+		/* only for enabled ports */
+		if ((1 << prt) & port_mask) {
+			/* skip CPU port */
+			if (prt == cpu_port)
+				continue;
+
+			/*
+			 *  set Ports VLAN Mapping.
+			 *      port prt <--> cpu_port VLAN #prt+1.
+			 */
+			mv88e61xx_rd_phy(name, ports_ofs + prt,
+					 MV88E61XX_PRT_VID_REG, &reg);
+			reg &= ~0x0fff;
+			reg |= (prt + 1);
+			mv88e61xx_wr_phy(name, ports_ofs + prt,
+					 MV88E61XX_PRT_VID_REG, reg);
+
+			mv88e61xx_rd_phy(name, ports_ofs + prt,
+					 MV88E61XX_PRT_VMAP_REG, &reg);
+			if (vlancfg == MV88E61XX_VLANCFG_DEFAULT) {
+				/*
+				 * all any port can send frames to all other ports
+				 * ref: sec 3.2.1.1 of datasheet
+				 */
+				reg |= 0x03f;
+				reg &= ~(1 << prt);
+			} else if (vlancfg == MV88E61XX_VLANCFG_ROUTER) {
+				/*
+				 * all other ports can send frames to CPU port only
+				 * ref: sec 3.2.1.2 of datasheet
+				 */
+				reg &= ~((1 << max_prtnum) - 1);
+				reg |= (1 << cpu_port);
+			}
+			mv88e61xx_wr_phy(name, ports_ofs + prt,
+					 MV88E61XX_PRT_VMAP_REG, reg);
+		}
+	}
+	/* Set Vlan map table for cpu_port to see all ports */
+	mv88e61xx_rd_phy(name, (ports_ofs + cpu_port), MV88E61XX_PRT_VMAP_REG,
+			 &reg);
+	reg &= ~((1 << max_prtnum) - 1);
+	reg |= port_mask & ~(1 << cpu_port);
+	mv88e61xx_wr_phy(name, (ports_ofs + cpu_port), MV88E61XX_PRT_VMAP_REG,
+			 reg);
+
+	/*
+	 * enable only appropriate ports to forwarding mode
+	 * and disable the others
+	 */
+	for (prt = 0; prt < max_prtnum; prt++) {
+		if ((1 << prt) & port_mask) {
+			mv88e61xx_rd_phy(name, ports_ofs + prt,
+					 MV88E61XX_PRT_CTRL_REG, &reg);
+			reg |= 0x3;
+			mv88e61xx_wr_phy(name, ports_ofs + prt,
+					 MV88E61XX_PRT_CTRL_REG, reg);
+		} else {
+			/* Disable port */
+			mv88e61xx_rd_phy(name, ports_ofs + prt,
+					 MV88E61XX_PRT_CTRL_REG, &reg);
+			reg &= ~0x3;
+			mv88e61xx_wr_phy(name, ports_ofs + prt,
+					 MV88E61XX_PRT_CTRL_REG, reg);
+		}
+	}
+}
+
+/*
+ * Make sure SMIBusy bit cleared before another
+ * SMI operation can take place
+ */
+static int mv88361xx_busychk(char *name)
+{
+	u32 reg = 0;
+	u32 timeout = MV88E61XX_PHY_TIMEOUT;
+	do {
+		mv88e61xx_rd_phy(name, MV88E61XX_GLB2REG_DEVADR,
+				 MV88E61XX_PHY_CMD, (u16 *) & reg);
+		if (timeout-- == 0) {
+			printf("SMI busy timeout\n");

drivers/net/phy/mv88e61xx.c new file +90 -0

+{
+	u32 prt;
+	u16 reg;
+	char *idstr;
+	char *name = swconfig->name;
+
+	if (miiphy_set_current_dev(name)) {
+		printf("%s failed\n", __FUNCTION__);
+		return -1;
+	}
+
+	if (swconfig->cpuport < 4) {
+		swconfig->cpuport = 5;
+		printf("Invalid cpu port config, using default port5\n");
+	}
+
+	mv88e61xx_rd_phy(name, MV88E61XX_PRT_OFST, PHY_PHYIDR2, &reg);
+	reg &= 0xfff0;
+	if (reg == 0x1610)
+		idstr = "88E6161";
+	if (reg == 0x1650)
+		idstr = "88E6165";
+	if (reg == 0x1210) {
+		idstr = "88E6123";
+		/* ports 2,3,4 not available */
+		swconfig->ports_enabled &= 0x023;
+	}
+
+	/* Port based VLANs configuration */
+	if ((swconfig->vlancfg == MV88E61XX_VLANCFG_DEFAULT)
+	    || (swconfig->vlancfg == MV88E61XX_VLANCFG_ROUTER))
+		mv88e61xx_vlan_config(swconfig, MV88E61XX_MAX_PORTS_NUM,
+				      MV88E61XX_PRT_OFST);
+	else {
+		printf("Unsupported mode %s failed\n", __FUNCTION__);
+		return -1;
+	}
+
+	if (swconfig->rgmii_delay == MV88E61XX_RGMII_DELAY_EN) {
+		/*
+		 * Enable RGMII delay on Tx and Rx for CPU port
+		 * Ref: sec 9.5 of chip datasheet-02
+		 */
+		mv88e61xx_wr_phy(name, MV88E61XX_PRT_OFST + 5,
+				 MV88E61XX_RGMII_TIMECTRL_REG, 0x18);
+		mv88e61xx_wr_phy(name, MV88E61XX_PRT_OFST + 4,
+				 MV88E61XX_RGMII_TIMECTRL_REG, 0xc1e7);
+	}
+
+	for (prt = 0; prt < MV88E61XX_MAX_PORTS_NUM; prt++) {
+		if (prt != swconfig->cpuport) {
+			/* Write Copper Specific control reg1 (0x14) for-
+			 * Enable Phy power up
+			 * Energy Detect on (sense&Xmit NLP Periodically
+			 * reset other settings default
+			 */
+			mv88e61xx_wr_phy(name, MV88E61XX_GLB2REG_DEVADR,
+					 MV88E61XX_PHY_DATA, 0x3360);
+			mv88e61xx_wr_phy(name, MV88E61XX_GLB2REG_DEVADR,
+					 MV88E61XX_PHY_CMD,
+					 (0x9410 | (prt << 5)));
+
+			if (mv88361xx_busychk(name))
+				return -1;
+
+			/* Write PHY ctrl reg (0x0) to apply
+			 * Phy reset (BIT15=low)
+			 * reset other default values
+			 */
+			mv88e61xx_wr_phy(name, MV88E61XX_GLB2REG_DEVADR,
+					 MV88E61XX_PHY_DATA, 0x1140);
+			mv88e61xx_wr_phy(name, MV88E61XX_GLB2REG_DEVADR,
+					 MV88E61XX_PHY_CMD,
+					 (0x9400 | (prt << 5)));
+
+			if (mv88361xx_busychk(name))
+				return -1;
+		}
+
+		/*Program port state */
+		mv88e61xx_rd_phy(name, MV88E61XX_PRT_OFST + prt,
+				 MV88E61XX_PRT_CTRL_REG, &reg);
+		mv88e61xx_wr_phy(name, MV88E61XX_PRT_OFST + prt,
+				 MV88E61XX_PRT_CTRL_REG,
+				 reg | (swconfig->portstate & 0x03));
+	}
+
+	printf("%s Initialized on %s\n", idstr, name);
+	return 0;
+}

drivers/net/phy/mv88e61xx.h new file +73 -1

new file mode 100644
index 0000000..400a4ad
@@ -0,0 +1,73 @@
+/*
+ * (C) Copyright 2009
+ * Marvell Semiconductor <www.marvell.com>
+ * Prafulla Wadaskar <[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
+ */
+
+#ifndef _MV88E61XX_H
+#define _MV88E61XX_H
+
+#define PORT(_x)		(1 << _x)
+#define MV88E61XX_CPU_PORT		0x5
+#define MV88E61XX_MAX_PORTS_NUM		0x6
+
+#define MV88E61XX_PHY_TIMEOUT		100000
+
+#define MV88E61XX_PRT_STS_REG		0x1
+#define MV88E61XX_PRT_CTRL_REG		0x4
+#define MV88E61XX_PRT_VMAP_REG		0x6
+#define MV88E61XX_PRT_VID_REG		0x7
+
+#define MV88E61XX_PRT_OFST		0x10
+#define MV88E61XX_PHY_CMD		0x18
+#define MV88E61XX_PHY_DATA		0x19
+#define MV88E61XX_RGMII_TIMECTRL_REG	0x1A
+#define MV88E61XX_GLB2REG_DEVADR	0x1C
+
+/* constants for switch configuration */
+enum mv88f61xx_cfg_vlan {
+	MV88E61XX_VLANCFG_DEFAULT,
+	MV88E61XX_VLANCFG_ROUTER
+};
+
+enum mv88f61xx_cfg_rgmiid {
+	MV88E61XX_RGMII_DELAY_DIS,
+	MV88E61XX_RGMII_DELAY_EN
+};
+
+enum mv88f61xx_cfg_prtstt {
+	MV88E61XX_PORTSTT_DISABLED,
+	MV88E61XX_PORTSTT_BLOCKING,
+	MV88E61XX_PORTSTT_LEARNING,
+	MV88E61XX_PORTSTT_FORWARDING
+};
+
+/* switch configuration structure */
+struct mv88f61xx_config {
+	char *name;
+	enum mv88f61xx_cfg_vlan vlancfg;
+	enum mv88f61xx_cfg_rgmiid rgmii_delay;
+	enum mv88f61xx_cfg_prtstt portstate;
+	u32 ports_enabled;
+	u8 cpuport;
+};
+
+#endif /* _MV88E61XX_H */
--

1.5.3.4

Quoted 12 lines

drivers/net/phy/mv88e61xx.c new file +12 -0

+#include <common.h>
+#include <miiphy.h>
+#include "mv88e61xx.h"
+
+/* Chip Address mode
+ * The Switch support two modes of operation
+ * 1. single chip mode and
+ * 2. Multi-chip mode
+ * Refer section 9.2 &9.3 in chip datasheet-02 for more details
+ *
+ * By default single chip mode is configured
+ * multichip mode operation can be configured in board header  */ 

Review

Quoted 6 lines

(quoted diff) +5 -0

+#ifndef CONFIG_MV88E61XX_MULTICHIP_ADRMODE
+#define mv88e61xx_wr_phy miiphy_write #define mv88e61xx_rd_phy 
+miiphy_read #else
+
+static int mv88e61xx_busychk_multic(u32 devaddr) {

I mentioned this previously - if you're going to alias functions please do it in a header (I see that you have one now).

Quoted 1 line

+static void mv88e61xx_vlan_config(struct mv88f61xx_config

*swconfig,

drivers/net/phy/mv88e61xx.c line 98

Quoted 2 lines

drivers/net/phy/mv88e61xx.c new file +1 -0

+				  u32 max_prtnum, u32 ports_ofs)

Please rename these functions to make clear that you're configuring port-based VLANs. Maybe 'mv88e61xx_port_vlan_config()' or something like that.

drivers/net/phy/mv88e61xx.c

Quoted 5 lines

drivers/net/phy/mv88e61xx.c new file +4 -0

+
+/*
+ * Marvell 88E61XX Switch initialization  */ int 
+mv_switch_88e61xx_init(struct mv88f61xx_config *swconfig)

Please add this function's prototype to include/netdev.h

Quoted 1 line

1.5.3.4

Thanks for all the hard work. Address these issues and I'll pull it into the net branch.

drivers/net/phy/mv88e61xx.h new file +58 -0

+/* Chip Address mode
+ * The Switch support two modes of operation
+ * 1. single chip mode and
+ * 2. Multi-chip mode
+ * Refer section 9.2 &9.3 in chip datasheet-02 for more details
+ *
+ * By default single chip mode is configured
+ * multichip mode operation can be configured in board header
+ */
+#ifndef CONFIG_MV88E61XX_MULTICHIP_ADRMODE
+#define mv88e61xx_wr_phy miiphy_write
+#define mv88e61xx_rd_phy miiphy_read
+#else
+
+static int mv88e61xx_busychk_multic(u32 devaddr)
+{
+	u32 reg = 0;
+	u32 timeout = MV88E61XX_PHY_TIMEOUT;
+
+	/* Poll till SMIBusy bit is clear */
+	do {
+		miiphy_read(name, devaddr, 0x0, &reg);
+		if (timeout-- == 0) {
+			printf("SMI busy timeout\n");
+			return -1;
+		}
+	} while (reg & BIT15);
+	return 0;
+}
+
+static void mv88e61xx_wr_phy(char *name, u32 phy_adr, u32 reg_ofs, u16 data)
+{
+	u16 reg;
+	u32 mii_dev_addr;
+
+	/* command to read PHY dev address */
+	if (!miiphy_read(name, 0xEE, 0xEE, &mii_dev_addr)) {
+		printf("Error..could not read PHY dev address\n");
+		return;
+	}
+	mv88e61xx_busychk_multic(mii_dev_addr);
+	/* Write data to Switch indirect data register */
+	miiphy_write(name, mii_dev_addr, 0x1, data);
+	/* Write command to Switch indirect command register (write) */
+	miiphy_write(name, mii_dev_addr, 0x0,
+		     reg_ofs | (phy_adr << 5) | BIT10 | BIT12 | BIT15);
+}
+
+static void mv88e61xx_rd_phy(char *name, u32 phy_adr, u32 reg_ofs, u16 * data)
+{
+	u16 reg;
+	u32 mii_dev_addr;
+
+	/* command to read PHY dev address */
+	if (!miiphy_read(name, 0xEE, 0xEE, &mii_dev_addr)) {
+		printf("Error..could not read PHY dev address\n");
+		return;
+	}

drivers/net/phy/mv88e61xx.h new file +1 -0

+#endif /* _MV88E61XX_H */

include/netdev.h +0 -0

index 2794ddd..e6832de 100644

-----Original Message-----
From: Detlev Zundel [mailto:dzu at denx.de]
Sent: Thursday, April 23, 2009 6:00 PM
To: Ben Warren
Cc: Prafulla Wadaskar; u-boot at lists.denx.de; Ashish Karkare; Prabhanjan Sarnaik; Ronen Shitrit
Subject: Re: [U-Boot] [PATCH v6] Marvell MV88E61XX Switch Driver support

Hi all,

just an interesting side note:

Statistics from v1 patch:

 board/Marvell/common/mv88e1116.c |   72 +++++++
 board/Marvell/common/mv88e60xx.c |  409 
+++++++++++++++++++++++++++++++++++++
 board/Marvell/common/mv88e61xx.c |  414 
++++++++++++++++++++++++++++++++++++++
 3 files changed, 895 insertions(+), 0 deletions(-)  create 

mode 100644 board/Marvell/common/mv88e1116.c create mode 100644 board/Marvell/common/mv88e60xx.c create mode 100644 board/Marvell/common/mv88e61xx.c

Statistics from v6 patch:

 drivers/net/phy/Makefile    |    1 +
 drivers/net/phy/mv88e61xx.c |  244 
+++++++++++++++++++++++++++++++++++++++++++
 drivers/net/phy/mv88e61xx.h |  143 +++++++++++++++++++++++++
 include/netdev.h            |    1 +
 4 files changed, 389 insertions(+), 0 deletions(-)  create 

mode 100644 drivers/net/phy/mv88e61xx.c create mode 100644 drivers/net/phy/mv88e61xx.h

Now even without looking at the details, if this isn't a *pretty* good example of what showing code and a review process can do, then I really don't know.

-----Original Message-----
From: Ben Warren [mailto:biggerbadderben at gmail.com]
Sent: Thursday, April 23, 2009 11:00 AM
To: Prafulla Wadaskar
Cc: u-boot at lists.denx.de; Ashish Karkare; Prabhanjan Sarnaik; Ronen Shitrit
Subject: Re: [U-Boot] [PATCH v6] Marvell MV88E61XX Switch Driver support

Hi Prafulla,

So close...

Quoted 1 line

+static void mv88e61xx_rd_phy(char *name, u32 phy_adr, u32

reg_ofs, u16 * data)

Quoted 13 lines

drivers/net/phy/mv88e61xx.h new file +13 -0

+{
+	u16 reg;
+	u32 mii_dev_addr;
+
+	/* command to read PHY dev address */
+	if (!miiphy_read(name, 0xEE, 0xEE, &mii_dev_addr)) {
+		printf("Error..could not read PHY dev address\n");
+		return;
+	}
+	mv88e61xx_busychk_multic(mii_dev_addr);
+	/* Write command to Switch indirect command register (read) */
+	miiphy_write(name, mii_dev_addr, 0x0,
+		     reg_ofs | (phy_adr << 5) | BIT10 | BIT12 | BIT15);

drivers/net/phy/mv88e61xx.h line 142

Quoted 7 lines

drivers/net/phy/mv88e61xx.h new file +6 -0

+	mv88e61xx_busychk_multic(mii_dev_addr);
+	/* Read data from Switch indirect data register */
+	miiphy_read(name, mii_dev_addr, 0x1, (u16 *) & data);
+}
+#endif /* CONFIG_MV88E61XX_MULTICHIP_ADRMODE */
+

I apologize for not being more clear here.

non-trivial
functions in header files. What I was hoping you'd do was put something
like this in the header file:

static void mv88e61xx_wr_phy(char *name, u32 phy_adr, u32 reg_ofs, u16 data);
static void mv88e61xx_rd_phy(char *name, u32 phy_adr, u32 reg_ofs, u16 * data);

/* Helpful comment here */
#ifndef CONFIG_MV88E61XX_MULTICHIP_ADRMODE
#define WRITE_PHY miiphy_write
#define READ_PHY miiphy_read
#else
#define WRITE_PHY mv88e61xx_wr_phy
#define READ_PHY mv88e61xx_rd_phy
#endif

and then use these macros throughout. You'll agree that it's much easier
to follow.