[U-Boot] [PATCH v2] Marvell MV88E61XX Switch Driver support
Prafulla Wadaskar
prafulla at marvell.com
Thu Apr 9 13:26:44 CEST 2009
> -----Original Message-----
> From: Ben Warren [mailto:biggerbadderben at gmail.com]
> Sent: Thursday, April 09, 2009 4:46 AM
> To: Prafulla Wadaskar
> Cc: u-boot at 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 <yotam at marvell.com>
> > Michael Blostein <michaelbl at marvell.com
> >
> > Reviewed by: Ronen Shitrit <rshitrit at marvell.com>
> > Signed-off-by: Prafulla Wadaskar <prafulla at 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 <prafulla at 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 */
> >
>
>
More information about the U-Boot
mailing list