[U-Boot] [PATCH v3] Marvell MV88E61XX Switch Driver support

Prafulla Wadaskar prafulla at marvell.com
Tue Apr 14 16:41:27 CEST 2009


Dear Jean,

Thanks for your comments
Please see my inlined reply

________________________________________
From: Jean-Christophe PLAGNIOL-VILLARD [plagnioj at jcrosoft.com]
Sent: Tuesday, April 14, 2009 3:49 AM
To: Prafulla Wadaskar
Cc: u-boot at 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:-
> 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
>
> Note: This driver is supported and tested against
> kirkwood egiga interface
>
> 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 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 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>
> +#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
> + * 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 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, &reg);
> +             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);
> +             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 <--> MV88E61XX_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);
> +
> +                     /*
> +                      * 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);
> +                     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);
> +     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
> + */
> +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, &reg);
> +             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, &reg);
> +     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);
> +     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.


More information about the U-Boot mailing list