> -----Original Message----- > From: Ben Warren [mailto:biggerbadder...@gmail.com] > Sent: Thursday, April 09, 2009 4:46 AM > To: Prafulla Wadaskar > Cc: u-boot@lists.denx.de; Ashish Karkare; Ronen Shitrit > Subject: Re: [U-Boot] [PATCH v2] Marvell MV88E61XX Switch > Driver support > > Hi Prafulla, > Prafulla Wadaskar wrote: > > Chips supprted:- > > 1. 88E6161 6 port gbe swtich with 5 integrated PHYs > > 2. 88E6165 6 port gbe swtich with 5 integrated PHYs > > Note: This driver is supported and tested against > > kirkwood egiga interface, other interfaces can be added > > > > Contributors: > > Yotam Admon <yo...@marvell.com> > > Michael Blostein <michae...@marvell.com > > > > Reviewed by: Ronen Shitrit <rshit...@marvell.com> > > Signed-off-by: Prafulla Wadaskar <prafu...@marvell.com> > > --- > > Changelog:- > > v2: updated as per review comments by Wolfgand Denk > > removed other two drivers form earlier patch > > debug_prints removed > > driver moved to drivers/net/ > > > > drivers/net/Makefile | 1 + > > drivers/net/mv88e61xx.c | 291 > +++++++++++++++++++++++++++++++++++++++++++++++ > > 2 files changed, 292 insertions(+), 0 deletions(-) > > create mode 100644 drivers/net/mv88e61xx.c > > > > diff --git a/drivers/net/Makefile b/drivers/net/Makefile > > index f0c5654..7482327 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_SWITCH_MV88E61XX) += mv88e61xx.o > > > Please change this to CONFIG_MV88E61XX_SWITCH Changed....
> > 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..8167919 > > --- /dev/null > > +++ b/drivers/net/mv88e61xx.c > > @@ -0,0 +1,291 @@ > > +/* > > + * (C) Copyright 2009 > > + * Marvell Semiconductor <www.marvell.com> > > + * Prafulla Wadaskar <prafu...@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> > > + > > +#if defined (CONFIG_SWITCH_MV88E61XX) > > > This is not needed - the makefile takes care of the > conditional compilation Removed... > > + > > +/* 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) > > +#endif > > + > > +#if defined (CONFIG_88E6161) > > +#define MV88E61XX_NAME "88E6161" > > +#elif defined (CONFIG_88E6165) > > +#define MV88E61XX_NAME "88E6165" > > +#else > > +#define MV88E61XX_NAME "88E61XX" > > +#endif > > + > > > There doesn't appear to be any code difference between the different > chips. What's the point of differentiating them here? If it is > meaningful, you should at least #warning if none of the known-about > variants is mentioned. This part of code removed... Chip Id is read from PHY_Id reg and displayed now. > > +#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 > > + > > +#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 > > + > > +#ifdef CONFIG_KIRKWOOD_EGIGA > > +#define smi_wr_reg eth_smi_reg_write > > +#define smi_rd_reg eth_smi_reg_read > > +#else /* add new interface above this */ > > +#error Unsupported interface > > +#endif > > + > > > Please find a better way to do this. Board-specific code > doesn't belong > in a driver. this is now implemented using miiphy_read/write calls > > +/* Chip Address mode > > + * The Switch support two modes of operation > > + * 1. single chip mode and > > + * 2. 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 smi_wr_reg > > +#define mv88e61xx_rd_phy smi_rd_reg > > +#else > > +void mv88e61xx_wr_phy(u32 port, u32 phy_adr, u32 reg_ofs, u16 data) > > > If you're going to play games with macros & function names, > please do it > in a header file Void .. Removed.... > > +{ > > + u16 reg; > > + u32 smi_dev_addr; > > + > > + smi_dev_addr = KW_REG_READ(KW_ETH_PHY_ADDR_REG(port)); > > + do { > > + smi_rd_reg(port, smi_dev_addr, 0x0, ®); > > + } while ((reg & BIT15)); > > > Is there any possibility this (and all the others) will > become infinite > loops? No there is no such posibility, but at safer side I put there time out > > + /* Poll till SMIBusy bit is clear */ > > + smi_wr_reg(port, smi_dev_addr, 0x1, data); > > + /* Write data to Switch indirect data register */ > > + smi_wr_reg(port, smi_dev_addr, 0x0, > > + reg_ofs | (phy_adr << 5) | BIT10 | BIT12 | BIT15); > > + /* Write command to Switch indirect command register (write) */ > > +} > > + > > +void mv88e61xx_rd_phy(u32 port, u32 phy_adr, u32 reg_ofs, > u16 * data) > > +{ > > + u16 reg; > > + u32 smi_dev_addr; > > + > > + smi_dev_addr = KW_REG_READ(KW_ETH_PHY_ADDR_REG(port)); > > + do { > > + smi_rd_reg(port, smi_dev_addr, 0x0, ®); > > + } while ((reg & BIT15)); > > + /* Poll till SMIBusy bit is clear */ > > + smi_wr_reg(port, smi_dev_addr, 0x0, > > + reg_ofs | (phy_adr << 5) | BIT10 | BIT12 | BIT15); > > + /* Write command to Switch indirect command register (read) */ > > + do { > > + smi_rd_reg(port, smi_dev_addr, 0x0, ®); > > + } while ((reg & BIT15)); > > + /* Poll till SMIBusy bit is clear */ > > + smi_rd_reg(port, smi_dev_addr, 0x1, (u16 *) & data); > > + /* Read data from Switch indirect data register */ > > +} > > +#endif /* CONFIG_MV88E61XX_MULTICHIP_ADRMODE */ > > + > > +static void mv88e61xx_vlaninit(u32 port, 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(port, ports_ofs + prt, > MV88E61XX_PRT_CTRL_REG, > > + ®); > > + reg &= ~0x3; > > + mv88e61xx_wr_phy(port, ports_ofs + prt, > MV88E61XX_PRT_CTRL_REG, > > + reg); > > + } > > + /* Set CPU port VID to 0x1 */ > > + mv88e61xx_rd_phy(port, (ports_ofs + cpu_port), > MV88E61XX_PRT_VID_REG, > > + ®); > > + reg &= ~0xfff; > > + reg |= 0x1; > > + mv88e61xx_wr_phy(port, (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(port, ports_ofs + prt, > MV88E61XX_PRT_VID_REG, > > + ®); > > + reg &= ~0xc000; > > + mv88e61xx_wr_phy(port, 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(port, ports_ofs + prt, > > + MV88E61XX_PRT_VID_REG, ®); > > + reg &= ~0x0fff; > > + reg |= (prt + 1); > > + mv88e61xx_wr_phy(port, 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(port, ports_ofs + prt, > > + MV88E61XX_PRT_VMAP_REG, ®); > > + reg &= ~((1 << max_prtnum) - 1); > > + reg |= (1 << cpu_port); > > + mv88e61xx_wr_phy(port, ports_ofs + prt, > > + MV88E61XX_PRT_VMAP_REG, reg); > > + } > > + } > > + /* Set Vlan map table for MV88E61XX_CPU_PORT to see all ports */ > > + mv88e61xx_rd_phy(port, (ports_ofs + cpu_port), > MV88E61XX_PRT_VMAP_REG, > > + ®); > > + reg &= ~((1 << max_prtnum) - 1); > > + reg |= port_mask & ~(1 << cpu_port); > > + mv88e61xx_wr_phy(port, (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(port, ports_ofs + prt, > > + MV88E61XX_PRT_CTRL_REG, ®); > > + reg |= 0x3; > > + mv88e61xx_wr_phy(port, ports_ofs + prt, > > + MV88E61XX_PRT_CTRL_REG, reg); > > + } else { > > + /* Disable port */ > > + mv88e61xx_rd_phy(port, ports_ofs + prt, > > + MV88E61XX_PRT_CTRL_REG, ®); > > + reg &= ~0x3; > > + mv88e61xx_wr_phy(port, ports_ofs + prt, > > + MV88E61XX_PRT_CTRL_REG, reg); > > + } > > + } > > +} > > + > > +/* > > + * Marvell 88E61XX Switch initialization > > + */ > > +int mv_switch_88e61xx_init(u32 port) > > +{ > > + u32 prt; > > + u16 reg; > > + volatile u32 timeout; > > > Why is this marked volatile? Sorry .. Removed..... > > + > > + /* Init vlan */ > > + mv88e61xx_vlaninit(port, MV88E61XX_CPU_PORT, > MV88E61XX_MAX_PORTS_NUM, > > + MV88E61XX_PRT_OFST, MV88E61XX_ENABLED_PORTS); > > + > > + /* Enable RGMII delay on Tx and Rx for CPU port */ > > + mv88e61xx_wr_phy(port, 0x14, 0x1a, 0x81e7); > > + mv88e61xx_rd_phy(port, 0x15, 0x1a, ®); > > + mv88e61xx_wr_phy(port, 0x15, 0x1a, 0x18); > > + mv88e61xx_wr_phy(port, 0x14, 0x1a, 0xc1e7); > > > Please define these magic numbers somewhere I am sorry, At this moment I can't, since I do not have sufficient data. I got this info from h/w team, data sheets yet to update :-( I will do it latter.. > > + > > + for (prt = 0; prt < MV88E61XX_MAX_PORTS_NUM; prt++) { > > + if (prt != MV88E61XX_CPU_PORT) { > > + /*Enable Phy power up */ > > + mv88e61xx_wr_phy(port, > MV88E61XX_GLOBAL2_REG_DEVADR, > > + MV88E61XX_PHY_DATA, 0x3360); > > + mv88e61xx_wr_phy(port, > MV88E61XX_GLOBAL2_REG_DEVADR, > > + MV88E61XX_PHY_CMD, > > + (0x9410 | (prt << 5))); > > + > > > Same comment about magic numbers Put some comments there.. > > + /* > > + * Make sure SMIBusy bit cleared before another > > + * SMI operation can take place > > + */ > > + timeout = MV88E61XX_PHY_TIMEOUT; > > + do { > > + mv88e61xx_rd_phy(port, > > + > MV88E61XX_GLOBAL2_REG_DEVADR, > > + > MV88E61XX_PHY_CMD, ®); > > + if (timeout-- == 0) { > > + printf("SMI busy timeout\n"); > > + return -1; > > + } > > + } while (reg & BIT28); /* busy mask */ > > + > > + mv88e61xx_wr_phy(port, > MV88E61XX_GLOBAL2_REG_DEVADR, > > + MV88E61XX_PHY_DATA, 0x1140); > > + mv88e61xx_wr_phy(port, > MV88E61XX_GLOBAL2_REG_DEVADR, > > + MV88E61XX_PHY_CMD, > > + (0x9400 | (prt << 5))); > > + > > + /* > > + * Make sure SMIBusy bit cleared before another > > + * SMI operation can take place > > + */ > > + timeout = MV88E61XX_PHY_TIMEOUT; > > + do { > > + mv88e61xx_rd_phy(port, > > + > MV88E61XX_GLOBAL2_REG_DEVADR, > > + > MV88E61XX_PHY_CMD, ®); > > + if (timeout-- == 0) { > > + printf("SMI busy timeout\n"); > > + return -1; > > + } > > + } while (reg & BIT28); /* busy mask */ > > + } > > + > > + /*Enable port */ > > + mv88e61xx_wr_phy(port, MV88E61XX_PRT_OFST + > prt, 4, 0x7f); > > + } > > + /*Force CPU port to RGMII FDX 1000Base */ > > + mv88e61xx_wr_phy(port, MV88E61XX_PRT_OFST + > MV88E61XX_CPU_PORT, 1, > > + 0x3e); > > > Is RGMII the only mode this chip supports? If not, why are > you forcing > it so? > > + > > + printf(MV88E61XX_NAME " Initialized\n"); > > > Shouldn't this message include the port number? Added port name to the string I am sending v3 patch for the same Reagards.. Prafulla . . > > + return 0; > > +} > > + > > +#endif /* CONFIG_SWITCH_MV88E61XX */ > > > > _______________________________________________ U-Boot mailing list U-Boot@lists.denx.de http://lists.denx.de/mailman/listinfo/u-boot