* [U-Boot] [PATCH v2] Marvell MV88E61XX Switch Driver support
@ 2009-04-08 18:30 Prafulla Wadaskar
2009-04-08 22:32 ` Andrew Dyer
` (3 more replies)
0 siblings, 4 replies; 9+ messages in thread
From: Prafulla Wadaskar @ 2009-04-08 18:30 UTC (permalink / raw)
To: u-boot
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@marvell.com>
Michael Blostein <michaelbl at marvell.com
Reviewed by: Ronen Shitrit <rshitrit@marvell.com>
Signed-off-by: Prafulla Wadaskar <prafulla@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
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@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
+
+/* 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));
+ 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, 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;
+
+ /* 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);
+
+ 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 */
+ }
+
+ /*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);
+
+ printf(MV88E61XX_NAME " Initialized\n");
+ return 0;
+}
+
+#endif /* CONFIG_SWITCH_MV88E61XX */
--
1.5.3.3
^ permalink raw reply related [flat|nested] 9+ messages in thread
* [U-Boot] [PATCH v2] Marvell MV88E61XX Switch Driver support
2009-04-08 18:30 [U-Boot] [PATCH v2] Marvell MV88E61XX Switch Driver support Prafulla Wadaskar
@ 2009-04-08 22:32 ` Andrew Dyer
2009-04-08 22:38 ` Wolfgang Denk
2009-04-09 10:10 ` Prafulla Wadaskar
2009-04-08 23:12 ` Andy Fleming
` (2 subsequent siblings)
3 siblings, 2 replies; 9+ messages in thread
From: Andrew Dyer @ 2009-04-08 22:32 UTC (permalink / raw)
To: u-boot
On Wed, Apr 8, 2009 at 1:30 PM, Prafulla Wadaskar <prafulla@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@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 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@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.
^ permalink raw reply [flat|nested] 9+ messages in thread
* [U-Boot] [PATCH v2] Marvell MV88E61XX Switch Driver support
2009-04-08 22:32 ` Andrew Dyer
@ 2009-04-08 22:38 ` Wolfgang Denk
2009-04-09 10:10 ` Prafulla Wadaskar
1 sibling, 0 replies; 9+ messages in thread
From: Wolfgang Denk @ 2009-04-08 22:38 UTC (permalink / raw)
To: u-boot
Dear Andrew Dyer,
In message <c166aa9f0904081532r7583585esc5cdcc384382d581@mail.gmail.com> you wrote:
>
> > v2: updated as per review comments by Wolfgand Denk
>
> It's always good to spell the name of the guy with commit access right :-)
He. I'm not Russell King.
Best regards,
Wolfgang Denk
--
DENX Software Engineering GmbH, MD: Wolfgang Denk & Detlev Zundel
HRB 165235 Munich, Office: Kirchenstr.5, D-82194 Groebenzell, Germany
Phone: (+49)-8142-66989-10 Fax: (+49)-8142-66989-80 Email: wd at denx.de
"This is a test of the Emergency Broadcast System. If this had been an
actual emergency, do you really think we'd stick around to tell you?"
^ permalink raw reply [flat|nested] 9+ messages in thread
* [U-Boot] [PATCH v2] Marvell MV88E61XX Switch Driver support
2009-04-08 18:30 [U-Boot] [PATCH v2] Marvell MV88E61XX Switch Driver support Prafulla Wadaskar
2009-04-08 22:32 ` Andrew Dyer
@ 2009-04-08 23:12 ` Andy Fleming
2009-04-09 10:13 ` Prafulla Wadaskar
2009-04-08 23:16 ` Ben Warren
2009-04-09 6:31 ` Norbert van Bolhuis
3 siblings, 1 reply; 9+ messages in thread
From: Andy Fleming @ 2009-04-08 23:12 UTC (permalink / raw)
To: u-boot
On Wed, Apr 8, 2009 at 1:30 PM, Prafulla Wadaskar <prafulla@marvell.com> wrote:
> --- /dev/null
> +++ b/drivers/net/mv88e61xx.c
> @@ -0,0 +1,291 @@
> +/*
> + * (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>
> +
> +#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
Is this discoverable at runtime? What if there's a system that
supports using multiple different types of MV88E61xx? I know it's a
bit of a crazy idea, but board designers like to screw around with
software developers like that.
> +
> +#define MV88E61XX_PHY_TIMEOUT ? ? ? ? ?100000
> +#define MV88E61XX_MAX_PORTS_NUM ? ? ? ? ? ? ? ?0x6
Is this a limitation of the 88e61xx architecture, or just the max
number on all of the currently shipping versions?
> +
> +#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
This sort of thing is discouraged. Why does this driver need to know
about the ethernet controller? Perhaps function pointers are needed?
Andy
^ permalink raw reply [flat|nested] 9+ messages in thread
* [U-Boot] [PATCH v2] Marvell MV88E61XX Switch Driver support
2009-04-08 18:30 [U-Boot] [PATCH v2] Marvell MV88E61XX Switch Driver support Prafulla Wadaskar
2009-04-08 22:32 ` Andrew Dyer
2009-04-08 23:12 ` Andy Fleming
@ 2009-04-08 23:16 ` Ben Warren
2009-04-09 11:26 ` Prafulla Wadaskar
2009-04-09 6:31 ` Norbert van Bolhuis
3 siblings, 1 reply; 9+ messages in thread
From: Ben Warren @ 2009-04-08 23:16 UTC (permalink / raw)
To: u-boot
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@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 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
> 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@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
> +
> +/* 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.
> +#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.
> +/* 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
> +{
> + 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?
> + /* 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?
> +
> + /* 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
> +
> + 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
> + /*
> + * 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?
> + return 0;
> +}
> +
> +#endif /* CONFIG_SWITCH_MV88E61XX */
>
^ permalink raw reply [flat|nested] 9+ messages in thread
* [U-Boot] [PATCH v2] Marvell MV88E61XX Switch Driver support
2009-04-08 18:30 [U-Boot] [PATCH v2] Marvell MV88E61XX Switch Driver support Prafulla Wadaskar
` (2 preceding siblings ...)
2009-04-08 23:16 ` Ben Warren
@ 2009-04-09 6:31 ` Norbert van Bolhuis
3 siblings, 0 replies; 9+ messages in thread
From: Norbert van Bolhuis @ 2009-04-09 6:31 UTC (permalink / raw)
To: u-boot
Hi Prafulla,
We have a custom designed board with the M88E6185 and M88E6131
switch. The u-boot patches are very usefull, even though I already
finished some code to initialize the switches.
In our application it's more easy to "fully" provision the switches
when the linux kernel is up and running.
So I wonder:
Is "M88E61XX switch driver support" planned for the linux kernel ?
In other words: is there any chance similar patches will be
released for the linux kernel any time soon ?
---
N. van Bolhuis.
^ permalink raw reply [flat|nested] 9+ messages in thread
* [U-Boot] [PATCH v2] Marvell MV88E61XX Switch Driver support
2009-04-08 22:32 ` Andrew Dyer
2009-04-08 22:38 ` Wolfgang Denk
@ 2009-04-09 10:10 ` Prafulla Wadaskar
1 sibling, 0 replies; 9+ messages in thread
From: Prafulla Wadaskar @ 2009-04-09 10:10 UTC (permalink / raw)
To: u-boot
> -----Original Message-----
> From: Andrew Dyer [mailto:amdyer at gmail.com]
> Sent: Thursday, April 09, 2009 4:03 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
>
> On Wed, Apr 8, 2009 at 1:30 PM, Prafulla Wadaskar
> <prafulla@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.
Welcome...
>
> >
> > 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 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@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.
Thanks..miiphy is used for the same
>
> > +
> > +/* 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.
It is more egiga driver specific,
you need to read phy address programmed for respective egga port on
Gbe controller to establish communication,
this settings is done in egiga driver, but there is no generic interface to read this
To make it more generic I have used miiphy api
i.e.
miiphy_read(name, 0xEE, 0xEE, &mii_dev_adr);
Values 0xEE are dummy to identify phy_address read.
Hope this will help,
we can define special interface in future if this becomes generic usecase
>
> > + 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.
Corrected....
Code factored too..
>
> > + 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?
It should.... I have added detection logic for 88E6123 too
>
> > + /* 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?
I am sorry at this moment, I will replace them latter since I don't have enough data
>
> What if I don't want RGMII delay as I'm using GMII on my platform?
For GMII/MII above settings not required...
I have encapsulated them with CONFIG_SYS_RGMII_MODE
>
> > +
> > + 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
These lines removed, by default switch detects speed normaly
> our case we want GMII, not RGMII. On another project we
> might want ports 4 & 5 to use the copper phy.
In this case this driver many need updates, or we need to provide such interface.
Any was current objective of this driver it to power up Phy and configure switch in managed mode. Also there are several h/s settings that go on the board which configures switch on power on Reset.
Thanks for feedback, I have updated the driver, will post v3 soon
Regards..
Prafulla . .
>
> > +
> > + 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.
>
^ permalink raw reply [flat|nested] 9+ messages in thread
* [U-Boot] [PATCH v2] Marvell MV88E61XX Switch Driver support
2009-04-08 23:12 ` Andy Fleming
@ 2009-04-09 10:13 ` Prafulla Wadaskar
0 siblings, 0 replies; 9+ messages in thread
From: Prafulla Wadaskar @ 2009-04-09 10:13 UTC (permalink / raw)
To: u-boot
> -----Original Message-----
> From: Andy Fleming [mailto:afleming at gmail.com]
> Sent: Thursday, April 09, 2009 4:43 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
>
> On Wed, Apr 8, 2009 at 1:30 PM, Prafulla Wadaskar
> <prafulla@marvell.com> wrote:
> > --- /dev/null
> > +++ b/drivers/net/mv88e61xx.c
> > @@ -0,0 +1,291 @@
> > +/*
> > + * (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>
> > +
> > +#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
>
>
> Is this discoverable at runtime? What if there's a system that
> supports using multiple different types of MV88E61xx? I know it's a
> bit of a crazy idea, but board designers like to screw around with
> software developers like that.
Yes... I have done it, you will find it in PATCH v3
>
>
> > +
> > +#define MV88E61XX_PHY_TIMEOUT ? ? ? ? ?100000
> > +#define MV88E61XX_MAX_PORTS_NUM ? ? ? ? ? ? ? ?0x6
>
> Is this a limitation of the 88e61xx architecture, or just the max
> number on all of the currently shipping versions?
No this represents how many maximum ports there are in 6161/6165 and those are 6
>
> > +
> > +#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
>
> This sort of thing is discouraged. Why does this driver need to know
> about the ethernet controller? Perhaps function pointers are needed?
>
Yes this is removed and the interface is addressed through miiphy_read/write
Thanks ...
Regards..
Prafulla . .
> Andy
>
^ permalink raw reply [flat|nested] 9+ messages in thread
* [U-Boot] [PATCH v2] Marvell MV88E61XX Switch Driver support
2009-04-08 23:16 ` Ben Warren
@ 2009-04-09 11:26 ` Prafulla Wadaskar
0 siblings, 0 replies; 9+ messages in thread
From: Prafulla Wadaskar @ 2009-04-09 11:26 UTC (permalink / raw)
To: u-boot
> -----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@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 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@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 */
> >
>
>
^ permalink raw reply [flat|nested] 9+ messages in thread
end of thread, other threads:[~2009-04-09 11:26 UTC | newest]
Thread overview: 9+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2009-04-08 18:30 [U-Boot] [PATCH v2] Marvell MV88E61XX Switch Driver support Prafulla Wadaskar
2009-04-08 22:32 ` Andrew Dyer
2009-04-08 22:38 ` Wolfgang Denk
2009-04-09 10:10 ` Prafulla Wadaskar
2009-04-08 23:12 ` Andy Fleming
2009-04-09 10:13 ` Prafulla Wadaskar
2009-04-08 23:16 ` Ben Warren
2009-04-09 11:26 ` Prafulla Wadaskar
2009-04-09 6:31 ` Norbert van Bolhuis
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox