Dear Jean,
Thanks for your comments Please see my inlined reply
________________________________________ From: Jean-Christophe PLAGNIOL-VILLARD [plagnioj@jcrosoft.com] Sent: Tuesday, April 14, 2009 3:49 AM To: Prafulla Wadaskar Cc: u-boot@lists.denx.de; Ashish Karkare; Ronen Shitrit Subject: Re: [U-Boot] [PATCH v3] Marvell MV88E61XX Switch Driver support
On 22:31 Thu 09 Apr , Prafulla Wadaskar wrote:
Chips supported:-
- 88E6161 6 port gbe swtich with 5 integrated PHYs
- 88E6165 6 port gbe swtich with 5 integrated PHYs
- 88E6132 3 port gbe swtich with 2 integrated PHYs
Note: This driver is supported and tested against kirkwood egiga interface
Contributors: Yotam Admon yotam@marvell.com Michael Blostein <michaelbl@marvell.com
Reviewed by: Ronen Shitrit rshitrit@marvell.com Signed-off-by: Prafulla Wadaskar prafulla@marvell.com
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
drivers/net/Makefile | 1 + drivers/net/mv88e61xx.c | 299 +++++++++++++++++++++++++++++++++++++++++++++++
the m88e61xx is a switch so have it in drivers/net/phy could make more sense Prafulla: This driver represents the mv88e61xx chips which uses miiphy interface provided externally so I think drivers/net/ it correct place for it. drivers/net/phy/ generally has phy read/write modules code like bitbang mii interface.
2 files changed, 300 insertions(+), 0 deletions(-) create mode 100644 drivers/net/mv88e61xx.c
diff --git a/drivers/net/Makefile b/drivers/net/Makefile index f0c5654..9e8787c 100644 --- a/drivers/net/Makefile +++ b/drivers/net/Makefile @@ -47,6 +47,7 @@ COBJS-$(CONFIG_MACB) += macb.o COBJS-$(CONFIG_MCFFEC) += mcffec.o mcfmii.o COBJS-$(CONFIG_MPC5xxx_FEC) += mpc5xxx_fec.o COBJS-$(CONFIG_MPC512x_FEC) += mpc512x_fec.o +COBJS-$(CONFIG_MV88E61XX_SWITCH) += mv88e61xx.o COBJS-$(CONFIG_NATSEMI) += natsemi.o COBJS-$(CONFIG_DRIVER_NE2000) += ne2000.o ne2000_base.o COBJS-$(CONFIG_DRIVER_AX88796L) += ax88796.o ne2000_base.o diff --git a/drivers/net/mv88e61xx.c b/drivers/net/mv88e61xx.c new file mode 100644 index 0000000..31a6fa6 --- /dev/null +++ b/drivers/net/mv88e61xx.c @@ -0,0 +1,299 @@ +/*
- (C) Copyright 2009
- Marvell Semiconductor <www.marvell.com>
- Prafulla Wadaskar prafulla@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 <miiphy.h>
+/* Enabled ports can be enabled in board header file */ +#if defined (CONFIG_MV88E61XX_ENABLED_PORTS) +#define MV88E61XX_ENABLED_PORTS CONFIG_MV88E61XX_ENABLED_PORTS +#else +#define MV88E61XX_ENABLED_PORTS (BIT0 | BIT1 | BIT2 | \
BIT3 | BIT4 | BIT5)
please avoid this BITx macrot Prafulla: OK, I will hardcode this value, but in my view it looks better (readable) using these macros provided those are available on other platforms, mostly it should be, how about renaming them as PORTx ?
+#endif
+#define MV88E61XX_PHY_TIMEOUT 100000 +#define MV88E61XX_MAX_PORTS_NUM 0x6
+/* CPU port can be configured in board header file */ +#if defined (CONFIG_MV88E61XX_CPU_PORT) +#define MV88E61XX_CPU_PORT CONFIG_MV88E61XX_CPU_PORT +#else +#define MV88E61XX_CPU_PORT 0x5 +#endif
this cpu port is normally configured by latch resistor you may could detect it Prafulla: There are two ports on 6165/6161 and one port on 6123 which can be used for CPU interface, this is decided by board h/w. we offer this configuration to board header file, default value is port5 Prafulla: I could not find "latch register" in the switch documentation, can you pls explain this?
+#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_GLOBAL2_REG_DEVADR 0x1C
GLOBAL2_REGISTER will be better Prafulla: Actually I has used similar name earlier (v1) but wolfgang suggested to use smaller names so I trimmed them and others too, I am planning to trim it further to MV88E61XX_GLB2REG_DEVADR in next release
+/* Chip Address mode
- The Switch support two modes of operation
- single chip mode and
- Multi-chip mode
- Refer chip documentation 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
+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, ®);if (timeout-- == 0) {printf("SMI busy timeout\n");return -1;}} while (reg & BIT15);return 0;+}
+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 */miiphy_read(name, 0xEE, 0xEE, &mii_dev_addr);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);+}
+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 */miiphy_read(name, 0xEE, 0xEE, &mii_dev_addr);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);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 */
a doc explaining how to us it will be nice Prafulla: I can give here reference to the relevant chapter in data sheet
+static void mv88e61xx_vlaninit(char *name, u32 cpu_port, u32 max_prtnum,
u32 ports_ofs, u32 port_mask)+{
u32 prt;u16 reg;/* 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 &= ~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 &= ~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 &= ~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 <--> MV88E61XX_CPU_PORT VLAN #prt+1.*/mv88e61xx_rd_phy(name, ports_ofs + prt,MV88E61XX_PRT_VID_REG, ®);reg &= ~0x0fff;reg |= (prt + 1);mv88e61xx_wr_phy(name, ports_ofs + prt,MV88E61XX_PRT_VID_REG, reg);/** Set Vlan map table for all ports to send only to* MV88E61XX_CPU_PORT*/mv88e61xx_rd_phy(name, ports_ofs + prt,MV88E61XX_PRT_VMAP_REG, ®);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 MV88E61XX_CPU_PORT to see all ports */mv88e61xx_rd_phy(name, (ports_ofs + cpu_port), MV88E61XX_PRT_VMAP_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 |= 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 &= ~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
- */
+int mv88361xx_busychk(char *name) +{
u32 reg = 0;u32 timeout = MV88E61XX_PHY_TIMEOUT;do {mv88e61xx_rd_phy(name, MV88E61XX_GLOBAL2_REG_DEVADR,MV88E61XX_PHY_CMD, ®);if (timeout-- == 0) {printf("SMI busy timeout\n");return -1;}} while (reg & BIT28); /* busy mask */return 0;+}
+/*
- Marvell 88E61XX Switch initialization
- */
+int mv_switch_88e61xx_init(char *name) +{
u32 prt;u16 reg;if (miiphy_set_current_dev(name)) {printf("%s failed\n", __FUNCTION__);return -1;}/* Init vlan */mv88e61xx_vlaninit(name, MV88E61XX_CPU_PORT, MV88E61XX_MAX_PORTS_NUM,MV88E61XX_PRT_OFST, MV88E61XX_ENABLED_PORTS);
this is board specific Prafulla : the objective of this driver it to power up switch h/w to enable data path for Ethernet controller in switch managed mode (CPU <-> all other ports). Prafulla: I think this is the functionality specific
+#ifdef CONFIG_SYS_RGMII_MODE
/* Enable RGMII delay on Tx and Rx for CPU port */mv88e61xx_wr_phy(name, 0x14, 0x1a, 0x81e7);mv88e61xx_rd_phy(name, 0x15, 0x1a, ®);mv88e61xx_wr_phy(name, 0x15, 0x1a, 0x18);mv88e61xx_wr_phy(name, 0x14, 0x1a, 0xc1e7);+#endif
this too Prafulla: this will be needed only if RGMII interface is used for this switch on any platform, so is needed here.
for (prt = 0; prt < MV88E61XX_MAX_PORTS_NUM; prt++) {if (prt != MV88E61XX_CPU_PORT) {/* 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_GLOBAL2_REG_DEVADR,MV88E61XX_PHY_DATA, 0x3360);mv88e61xx_wr_phy(name, MV88E61XX_GLOBAL2_REG_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_GLOBAL2_REG_DEVADR,MV88E61XX_PHY_DATA, 0x1140);mv88e61xx_wr_phy(name, MV88E61XX_GLOBAL2_REG_DEVADR,MV88E61XX_PHY_CMD,(0x9400 | (prt << 5)));if (mv88361xx_busychk(name))return -1;}/*Enable port */mv88e61xx_wr_phy(name, MV88E61XX_PRT_OFST + prt, 4, 0x7f);}
this too Prafulla : No :-) ... switch objective..., if I abstract your platform specific comments, there is nothing special in it to keep it in drivers/net/ Prafulla: so on any platform if this switch is used, this driver can be enabled, currently it provides copper specific init, can be extended further for fiber optics and other needs
mv88e61xx_rd_phy(name, MV88E61XX_PRT_OFST, PHY_PHYIDR2, ®);reg &= 0xfff0;if (reg == 0x1610)printf("88E6161");if (reg == 0x1650)printf("88E6165");if (reg == 0x1210)printf("88E6123");
by detecting the switch model you will capable to identify the numbers of ports and their capability Prafulla: sure, but which port to be used will be again question to be asked to board header
printf(" Initialized on %s\n", name);return 0;+}
Best Regards, J.