[U-Boot] [PATCH v2] Marvell MV88E61XX Switch Driver support
Andrew Dyer
amdyer at gmail.com
Thu Apr 9 00:32:30 CEST 2009
On Wed, Apr 8, 2009 at 1:30 PM, Prafulla Wadaskar <prafulla at marvell.com> 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
We are going to be using this chip in a project, so I'm happy to see
this code come by :-) A few comments sprinkled around below.
>
> 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
It's always good to spell the name of the guy with commit access right :-)
> 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
> 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)
> +
> +/* 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
> +
> +#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
As far as I understand it, "SMI" for the 88E6161 is really just the
physical MII layer with some changes in the register definitions.
Would it not make more sense to use the existing u-boot miiphy code in
common/miiphyutil.c to define the access functions? This allows for
platform code to register mii access functions and gets platform
specific stuff like this out of the driver.
> +
> +/* 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)
> +{
> + u16 reg;
> + u32 smi_dev_addr;
> +
> + smi_dev_addr = KW_REG_READ(KW_ETH_PHY_ADDR_REG(port));
if CONFIG_MV88E61XX_MULTICHIP_ADRMODE is defined you're going to
reference the KW_REG_READ and KW_ETH_PHY_ADDR_REG macros, which don't
seem to be defined in this file. I'm assuming this is platform
specific stuff.
> + do {
> + smi_rd_reg(port, smi_dev_addr, 0x0, ®);
> + } while ((reg & BIT15));
> + /* Poll till SMIBusy bit is clear */
How about a timeout here? I'm not a C style maven, but the extra ()
in the test could probably go away. Some constants for register names
and bit functions would help as well. This code appears several
times, so maybe could be factored out. Also the commenting style is a
bit random through the file - sometimes the comment is before the
action, other times afterwards.
> + 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));
ditto above regarding the KW* macros
> + 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);
> + }
Would this code (and similar code below) work on the 88E6123 which
(IIRC) only has ports 0, 1, and 5?
> + /* 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;
> +
> + /* 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);
^ -- lots of magic numbers above and below here -- v
In the middle of the above sequence you read the phy, but don't seem
to do anything with 'reg'. Is this required?
What if I don't want RGMII delay as I'm using GMII on my platform?
> +
> + 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)));
> +
> + /*
> + * 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 */
replicated status polling code, might want to factor it out.
> + }
> +
> + /*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);
This last write seems very platform specific, for example in our case
we want GMII, not RGMII. On another project we might want ports 4 & 5
to use the copper phy.
> +
> + printf(MV88E61XX_NAME " Initialized\n");
> + return 0;
> +}
> +
> +#endif /* CONFIG_SWITCH_MV88E61XX */
> --
> 1.5.3.3
>
> _______________________________________________
> U-Boot mailing list
> U-Boot at lists.denx.de
> http://lists.denx.de/mailman/listinfo/u-boot
>
--
Hardware, n.:
The parts of a computer system that can be kicked.
More information about the U-Boot
mailing list