[PATCH v3 7/8] net: add MV88E61xx DSA driver
Tim Harvey
tharvey at gateworks.com
Tue Jun 21 01:37:45 CEST 2022
t On Mon, Jun 20, 2022 at 4:58 AM Vladimir Oltean
<vladimir.oltean at nxp.com> wrote:
>
> On Mon, May 23, 2022 at 11:25:48AM -0700, Tim Harvey wrote:
> > Add a DSA driver for the MV88E61xx compatible GbE Ethernet switches.
> >
> > Cc: Marek BehĂșn <marek.behun at nic.cz>
> > Cc: Vladimir Oltean <vladimir.oltean at nxp.com>
> > Signed-off-by: Tim Harvey <tharvey at gateworks.com>
> > Reviewed-by: Vladimir Oltean <vladimir.oltean at nxp.com>
> > ---
> > v3:
> > - Added Vladimir's rb tag
> > v2:
> > - rebase on v2022.07-rc1 (use ofnode_get_phy_node)
> > - remove unused commented out fields from struct
> > - remove unused PORT_MASK macro
> > - remove phy from priv struct name
> > - refactor code from original drivers/net/phy/mv88e61xx with
> > suggestions from review to consolidate some functions
> > into mv88e61xx_dsa_port_enable
> > - remove unecessary skiping of disabling of CPU port
> > - remove unecessary dev_set_parent_priv
> > - remove unnecessary static init flag
> > - replace debug with a dev_warn if switch device-id unsupported
> > - remove unnecessary xmit/recv functions as we rely on the fact that
> > only a single port is active instead of mangling packets
>
> This looks good, but my opinion remains that we can rename mv88e61xx to
> mv88e6xxx for consistency with Linux. Users will know that the drivers
> are expected to support the same hardware models (even if the compatible
> list is now incomplete and does not cover an actual 61xx device).
>
Ok, since you're asking for additional changes requiring a v4 I'll
rename it to mvee86xxx as well.
> > ---
> > drivers/net/Kconfig | 7 +
> > drivers/net/Makefile | 1 +
> > drivers/net/mv88e61xx.c | 843 ++++++++++++++++++++++++++++++++++++++++
> > 3 files changed, 851 insertions(+)
> > create mode 100644 drivers/net/mv88e61xx.c
> >
> > diff --git a/drivers/net/Kconfig b/drivers/net/Kconfig
> > index 7fe0e00649cf..edb1a0898986 100644
> > --- a/drivers/net/Kconfig
> > +++ b/drivers/net/Kconfig
> > @@ -433,6 +433,13 @@ config LPC32XX_ETH
> > depends on ARCH_LPC32XX
> > default y
> >
> > +config MV88E61XX
> > + bool "Marvell MV88E61xx GbE switch DSA driver"
> > + depends on DM_DSA && DM_MDIO
> > + help
> > + This driver implements a DSA switch driver for the MV88E61xx family
> > + of GbE switches using the MDIO interface
> > +
> > config MVGBE
> > bool "Marvell Orion5x/Kirkwood network interface support"
> > depends on ARCH_KIRKWOOD || ARCH_ORION5X
> > diff --git a/drivers/net/Makefile b/drivers/net/Makefile
> > index 69fb3bbbf7cb..36b4c279430a 100644
> > --- a/drivers/net/Makefile
> > +++ b/drivers/net/Makefile
> > @@ -59,6 +59,7 @@ obj-$(CONFIG_MEDIATEK_ETH) += mtk_eth.o
> > obj-$(CONFIG_MPC8XX_FEC) += mpc8xx_fec.o
> > obj-$(CONFIG_MT7620_ETH) += mt7620-eth.o
> > obj-$(CONFIG_MT7628_ETH) += mt7628-eth.o
> > +obj-$(CONFIG_MV88E61XX) += mv88e61xx.o
> > obj-$(CONFIG_MVGBE) += mvgbe.o
> > obj-$(CONFIG_MVMDIO) += mvmdio.o
> > obj-$(CONFIG_MVNETA) += mvneta.o
> > diff --git a/drivers/net/mv88e61xx.c b/drivers/net/mv88e61xx.c
> > new file mode 100644
> > index 000000000000..514835bf03b9
> > --- /dev/null
> > +++ b/drivers/net/mv88e61xx.c
> > @@ -0,0 +1,843 @@
> > +// SPDX-License-Identifier: GPL-2.0+
> > +/*
> > + * (C) Copyright 2022
> > + * Gateworks Corporation <http://www.gateworks.com/>
> > + * Tim Harvey <tharvey at gateworks.com>
> > + *
> > + * (C) Copyright 2015
> > + * Elecsys Corporation <http://www.elecsyscorp.com/>
> > + * Kevin Smith <kevin.smith at elecsyscorp.com>
> > + *
> > + * Original driver:
> > + * (C) Copyright 2009
> > + * Marvell Semiconductor <http://www.marvell.com/>
> > + * Prafulla Wadaskar <prafulla at marvell.com>
> > + */
> > +
> > +/*
> > + * DSA driver for mv88e61xx ethernet switches.
> > + *
> > + * This driver configures the mv88e61xx for basic use as a DSA switch.
> > + *
> > + * This driver was adapted from drivers/net/phy/mv88e61xx and tested
> > + * on the mv88e6176 via an SGMII interface.
> > + */
> > +
> > +#include <bitfield.h>
> > +#include <common.h>
> > +#include <miiphy.h>
>
> Alphabetic ordering please.
>
ok
> > +#include <dm/device.h>
> > +#include <dm/device_compat.h>
> > +#include <dm/device-internal.h>
> > +#include <dm/lists.h>
> > +#include <dm/of_extra.h>
> > +#include <linux/delay.h>
> > +#include <net/dsa.h>
> > +
> > +/* Device addresses */
> > +#define DEVADDR_PHY(p) (p)
> > +#define DEVADDR_SERDES 0x0F
> > +
> > +/* SMI indirection registers for multichip addressing mode */
> > +#define SMI_CMD_REG 0x00
> > +#define SMI_DATA_REG 0x01
> > +
> > +/* Global registers */
> > +#define GLOBAL1_STATUS 0x00
> > +#define GLOBAL1_CTRL 0x04
> > +#define GLOBAL1_MON_CTRL 0x1A
> > +
> > +/* Global 2 registers */
> > +#define GLOBAL2_REG_PHY_CMD 0x18
> > +#define GLOBAL2_REG_PHY_DATA 0x19
> > +#define GLOBAL2_REG_SCRATCH 0x1A
> > +
> > +/* Port registers */
> > +#define PORT_REG_STATUS 0x00
> > +#define PORT_REG_PHYS_CTRL 0x01
> > +#define PORT_REG_SWITCH_ID 0x03
> > +#define PORT_REG_CTRL 0x04
> > +#define PORT_REG_VLAN_MAP 0x06
> > +#define PORT_REG_VLAN_ID 0x07
> > +#define PORT_REG_LED_CTRL 0x16
> > +
> > +/* Phy registers */
> > +#define PHY_REG_CTRL1 0x10
> > +#define PHY_REG_STATUS1 0x11
> > +#define PHY_REG_PAGE 0x16
> > +
> > +/* Serdes registers */
> > +#define SERDES_REG_CTRL_1 0x10
> > +
> > +/* Phy page numbers */
> > +#define PHY_PAGE_COPPER 0
> > +#define PHY_PAGE_SERDES 1
> > +
> > +/* Register fields */
> > +#define GLOBAL1_CTRL_SWRESET BIT(15)
> > +
> > +#define GLOBAL1_MON_CTRL_CPUDEST_SHIFT 4
> > +#define GLOBAL1_MON_CTRL_CPUDEST_WIDTH 4
> > +
> > +#define PORT_REG_STATUS_SPEED_SHIFT 8
> > +#define PORT_REG_STATUS_SPEED_10 0
> > +#define PORT_REG_STATUS_SPEED_100 1
> > +#define PORT_REG_STATUS_SPEED_1000 2
> > +
> > +#define PORT_REG_STATUS_CMODE_MASK 0xF
> > +#define PORT_REG_STATUS_CMODE_100BASE_X 0x8
> > +#define PORT_REG_STATUS_CMODE_1000BASE_X 0x9
> > +#define PORT_REG_STATUS_CMODE_SGMII 0xa
> > +
> > +#define PORT_REG_PHYS_CTRL_RGMII_DELAY_RXCLK BIT(15)
> > +#define PORT_REG_PHYS_CTRL_RGMII_DELAY_TXCLK BIT(14)
> > +#define PORT_REG_PHYS_CTRL_PCS_AN_EN BIT(10)
> > +#define PORT_REG_PHYS_CTRL_PCS_AN_RST BIT(9)
> > +#define PORT_REG_PHYS_CTRL_FC_VALUE BIT(7)
> > +#define PORT_REG_PHYS_CTRL_FC_FORCE BIT(6)
> > +#define PORT_REG_PHYS_CTRL_LINK_VALUE BIT(5)
> > +#define PORT_REG_PHYS_CTRL_LINK_FORCE BIT(4)
> > +#define PORT_REG_PHYS_CTRL_DUPLEX_VALUE BIT(3)
> > +#define PORT_REG_PHYS_CTRL_DUPLEX_FORCE BIT(2)
> > +#define PORT_REG_PHYS_CTRL_SPD1000 BIT(1)
> > +#define PORT_REG_PHYS_CTRL_SPD100 BIT(0)
> > +#define PORT_REG_PHYS_CTRL_SPD_MASK (BIT(1) | BIT(0))
> > +
> > +#define PORT_REG_CTRL_PSTATE_SHIFT 0
> > +#define PORT_REG_CTRL_PSTATE_WIDTH 2
> > +
> > +#define PORT_REG_VLAN_ID_DEF_VID_SHIFT 0
> > +#define PORT_REG_VLAN_ID_DEF_VID_WIDTH 12
> > +
> > +#define PORT_REG_VLAN_MAP_TABLE_SHIFT 0
> > +#define PORT_REG_VLAN_MAP_TABLE_WIDTH 11
> > +
> > +#define SERDES_REG_CTRL_1_FORCE_LINK BIT(10)
> > +
> > +/* Field values */
> > +#define PORT_REG_CTRL_PSTATE_DISABLED 0
> > +#define PORT_REG_CTRL_PSTATE_FORWARD 3
> > +
> > +#define PHY_REG_CTRL1_ENERGY_DET_OFF 0
> > +#define PHY_REG_CTRL1_ENERGY_DET_SENSE_PULSE 1
> > +#define PHY_REG_CTRL1_ENERGY_DET_SENSE_ONLY 2
> > +#define PHY_REG_CTRL1_ENERGY_DET_SENSE_XMIT 3
> > +
> > +/* PHY Status Register */
> > +#define PHY_REG_STATUS1_SPEED 0xc000
> > +#define PHY_REG_STATUS1_GBIT 0x8000
> > +#define PHY_REG_STATUS1_100 0x4000
> > +#define PHY_REG_STATUS1_DUPLEX 0x2000
> > +#define PHY_REG_STATUS1_SPDDONE 0x0800
> > +#define PHY_REG_STATUS1_LINK 0x0400
> > +#define PHY_REG_STATUS1_ENERGY 0x0010
> > +
> > +/*
> > + * Macros for building commands for indirect addressing modes. These are valid
> > + * for both the indirect multichip addressing mode and the PHY indirection
> > + * required for the writes to any PHY register.
> > + */
> > +#define SMI_BUSY BIT(15)
> > +#define SMI_CMD_CLAUSE_22 BIT(12)
> > +#define SMI_CMD_CLAUSE_22_OP_READ (2 << 10)
> > +#define SMI_CMD_CLAUSE_22_OP_WRITE (1 << 10)
> > +
> > +#define SMI_CMD_READ (SMI_BUSY | SMI_CMD_CLAUSE_22 | \
> > + SMI_CMD_CLAUSE_22_OP_READ)
> > +#define SMI_CMD_WRITE (SMI_BUSY | SMI_CMD_CLAUSE_22 | \
> > + SMI_CMD_CLAUSE_22_OP_WRITE)
> > +
> > +#define SMI_CMD_ADDR_SHIFT 5
> > +#define SMI_CMD_ADDR_WIDTH 5
> > +#define SMI_CMD_REG_SHIFT 0
> > +#define SMI_CMD_REG_WIDTH 5
> > +
> > +/* Defines for Scratch and Misc Registers */
> > +#define SCRATCH_UPDATE BIT(15)
> > +#define SCRATCH_ADDR_SHIFT 8
> > +#define SCRATCH_ADDR_WIDTH 7
> > +
> > +/* Scratch registers */
> > +#define SCRATCH_ADDR_GPIO0DIR 0x62 /* GPIO[7:0] direction 1=input */
> > +#define SCRATCH_ADDR_GPIO1DIR 0x63 /* GPIO[14:8] direction 1=input */
> > +#define SCRATCH_ADDR_GPIO0DATA 0x64 /* GPIO[7:0] data */
> > +#define SCRATCH_ADDR_GPIO1DATA 0x65 /* GPIO[14:8] data */
> > +#define SCRATCH_ADDR_GPIO0CTRL 0x68 /* GPIO[1:0] control */
> > +#define SCRATCH_ADDR_GPIO1CTRL 0x69 /* GPIO[3:2] control */
> > +
> > +/* ID register values for different switch models */
> > +#define PORT_SWITCH_ID_6020 0x0200
> > +#define PORT_SWITCH_ID_6070 0x0700
> > +#define PORT_SWITCH_ID_6071 0x0710
> > +#define PORT_SWITCH_ID_6096 0x0980
> > +#define PORT_SWITCH_ID_6097 0x0990
> > +#define PORT_SWITCH_ID_6172 0x1720
> > +#define PORT_SWITCH_ID_6176 0x1760
> > +#define PORT_SWITCH_ID_6220 0x2200
> > +#define PORT_SWITCH_ID_6240 0x2400
> > +#define PORT_SWITCH_ID_6250 0x2500
> > +#define PORT_SWITCH_ID_6352 0x3520
> > +
> > +struct mv88e61xx_priv {
> > + int smi_addr;
> > + int id;
> > + int port_count; /* Number of switch ports */
> > + int port_reg_base; /* Base of the switch port registers */
> > + u8 global1; /* Offset of Switch Global 1 registers */
> > + u8 global2; /* Offset of Switch Global 2 registers */
> > + u8 phy_ctrl1_en_det_shift; /* 'EDet' bit field offset */
> > + u8 phy_ctrl1_en_det_width; /* Width of 'EDet' bit field */
> > + u8 phy_ctrl1_en_det_ctrl; /* 'EDet' control value */
> > +};
> > +
> > +static inline int smi_cmd(int cmd, int addr, int reg)
> > +{
> > + cmd = bitfield_replace(cmd, SMI_CMD_ADDR_SHIFT, SMI_CMD_ADDR_WIDTH,
> > + addr);
> > + cmd = bitfield_replace(cmd, SMI_CMD_REG_SHIFT, SMI_CMD_REG_WIDTH, reg);
> > + return cmd;
> > +}
> > +
> > +static inline int smi_cmd_read(int addr, int reg)
> > +{
> > + return smi_cmd(SMI_CMD_READ, addr, reg);
> > +}
> > +
> > +static inline int smi_cmd_write(int addr, int reg)
> > +{
> > + return smi_cmd(SMI_CMD_WRITE, addr, reg);
> > +}
> > +
> > +/* Wait for the current SMI indirect command to complete */
> > +static int mv88e61xx_smi_wait(struct udevice *dev, int smi_addr)
> > +{
> > + int val;
> > + u32 timeout = 100;
> > +
> > + do {
> > + val = dm_mdio_read(dev->parent, smi_addr, MDIO_DEVAD_NONE, SMI_CMD_REG);
> > + if (val >= 0 && (val & SMI_BUSY) == 0)
> > + return 0;
> > +
> > + mdelay(1);
> > + } while (--timeout);
> > +
> > + dev_err(dev, "SMI busy timeout\n");
> > + return -ETIMEDOUT;
> > +}
> > +
> > +/*
> > + * The mv88e61xx has three types of addresses: the smi bus address, the device
> > + * address, and the register address. The smi bus address distinguishes it on
> > + * the smi bus from other PHYs or switches. The device address determines
> > + * which on-chip register set you are reading/writing (the various PHYs, their
> > + * associated ports, or global configuration registers). The register address
> > + * is the offset of the register you are reading/writing.
> > + *
> > + * When the mv88e61xx is hardware configured to have address zero, it behaves in
> > + * single-chip addressing mode, where it responds to all SMI addresses, using
> > + * the smi address as its device address. This obviously only works when this
> > + * is the only chip on the SMI bus. This allows the driver to access device
> > + * registers without using indirection. When the chip is configured to a
> > + * non-zero address, it only responds to that SMI address and requires indirect
> > + * writes to access the different device addresses.
> > + */
> > +static int mv88e61xx_reg_read(struct udevice *dev, int addr, int reg)
> > +{
> > + struct mv88e61xx_priv *priv = dev_get_priv(dev);
> > + int smi_addr = priv->smi_addr;
> > + int res;
> > +
> > + /* In single-chip mode, the device can be addressed directly */
> > + if (smi_addr == 0)
> > + return dm_mdio_read(dev->parent, addr, MDIO_DEVAD_NONE, reg);
> > +
> > + /* Wait for the bus to become free */
> > + res = mv88e61xx_smi_wait(dev, smi_addr);
> > + if (res < 0)
> > + return res;
> > +
> > + /* Issue the read command */
> > + res = dm_mdio_write(dev->parent, smi_addr, MDIO_DEVAD_NONE, SMI_CMD_REG,
> > + smi_cmd_read(addr, reg));
> > + if (res < 0)
> > + return res;
> > +
> > + /* Wait for the read command to complete */
> > + res = mv88e61xx_smi_wait(dev, smi_addr);
> > + if (res < 0)
> > + return res;
> > +
> > + /* Read the data */
> > + res = dm_mdio_read(dev->parent, smi_addr, MDIO_DEVAD_NONE, SMI_DATA_REG);
> > + if (res < 0)
> > + return res;
> > +
> > + return bitfield_extract(res, 0, 16);
> > +}
> > +
> > +/* See the comment above mv88e61xx_reg_read */
> > +static int mv88e61xx_reg_write(struct udevice *dev, int addr, int reg, u16 val)
> > +{
> > + struct mv88e61xx_priv *priv = dev_get_priv(dev);
> > + int smi_addr = priv->smi_addr;
> > + int res;
> > +
> > + /* In single-chip mode, the device can be addressed directly */
> > + if (smi_addr == 0)
> > + return dm_mdio_write(dev->parent, addr, MDIO_DEVAD_NONE, reg, val);
> > +
> > + /* Wait for the bus to become free */
> > + res = mv88e61xx_smi_wait(dev, smi_addr);
> > + if (res < 0)
> > + return res;
> > +
> > + /* Set the data to write */
> > + res = dm_mdio_write(dev->parent, smi_addr, MDIO_DEVAD_NONE,
> > + SMI_DATA_REG, val);
> > + if (res < 0)
> > + return res;
> > +
> > + /* Issue the write command */
> > + res = dm_mdio_write(dev->parent, smi_addr, MDIO_DEVAD_NONE, SMI_CMD_REG,
> > + smi_cmd_write(addr, reg));
> > + if (res < 0)
> > + return res;
> > +
> > + /* Wait for the write command to complete */
> > + res = mv88e61xx_smi_wait(dev, smi_addr);
> > + if (res < 0)
> > + return res;
> > +
> > + return 0;
> > +}
> > +
> > +static int mv88e61xx_phy_wait(struct udevice *dev)
> > +{
> > + struct mv88e61xx_priv *priv = dev_get_priv(dev);
> > + int val;
> > + u32 timeout = 100;
> > +
> > + do {
> > + val = mv88e61xx_reg_read(dev, priv->global2, GLOBAL2_REG_PHY_CMD);
> > + if (val >= 0 && (val & SMI_BUSY) == 0)
> > + return 0;
> > +
> > + mdelay(1);
> > + } while (--timeout);
> > +
> > + return -ETIMEDOUT;
> > +}
> > +
> > +static int mv88e61xx_phy_read_indirect(struct udevice *dev, int phyad, int devad, int reg)
> > +{
> > + struct mv88e61xx_priv *priv = dev_get_priv(dev);
> > + int res;
> > +
> > + /* Issue command to read */
> > + res = mv88e61xx_reg_write(dev, priv->global2,
> > + GLOBAL2_REG_PHY_CMD,
> > + smi_cmd_read(phyad, reg));
> > +
> > + /* Wait for data to be read */
> > + res = mv88e61xx_phy_wait(dev);
> > + if (res < 0)
> > + return res;
> > +
> > + /* Read retrieved data */
> > + return mv88e61xx_reg_read(dev, priv->global2,
> > + GLOBAL2_REG_PHY_DATA);
> > +}
> > +
> > +static int mv88e61xx_phy_write_indirect(struct udevice *dev, int phyad,
> > + int devad, int reg, u16 data)
> > +{
> > + struct mv88e61xx_priv *priv = dev_get_priv(dev);
> > + int res;
> > +
> > + /* Set the data to write */
> > + res = mv88e61xx_reg_write(dev, priv->global2,
> > + GLOBAL2_REG_PHY_DATA, data);
> > + if (res < 0)
> > + return res;
> > + /* Issue the write command */
> > + res = mv88e61xx_reg_write(dev, priv->global2,
> > + GLOBAL2_REG_PHY_CMD,
> > + smi_cmd_write(phyad, reg));
> > + if (res < 0)
> > + return res;
> > +
> > + /* Wait for command to complete */
> > + return mv88e61xx_phy_wait(dev);
> > +}
> > +
> > +/* Wrapper function to make calls to phy_read_indirect simpler */
> > +static int mv88e61xx_phy_read(struct udevice *dev, int phy, int reg)
> > +{
> > + return mv88e61xx_phy_read_indirect(dev, DEVADDR_PHY(phy),
> > + MDIO_DEVAD_NONE, reg);
> > +}
> > +
> > +/* Wrapper function to make calls to phy_write_indirect simpler */
> > +static int mv88e61xx_phy_write(struct udevice *dev, int phy, int reg, u16 val)
> > +{
> > + return mv88e61xx_phy_write_indirect(dev, DEVADDR_PHY(phy),
> > + MDIO_DEVAD_NONE, reg, val);
> > +}
> > +
> > +static int mv88e61xx_port_read(struct udevice *dev, u8 port, u8 reg)
> > +{
> > + struct mv88e61xx_priv *priv = dev_get_priv(dev);
> > +
> > + return mv88e61xx_reg_read(dev, priv->port_reg_base + port, reg);
> > +}
> > +
> > +static int mv88e61xx_port_write(struct udevice *dev, u8 port, u8 reg, u16 val)
> > +{
> > + struct mv88e61xx_priv *priv = dev_get_priv(dev);
> > +
> > + return mv88e61xx_reg_write(dev, priv->port_reg_base + port, reg, val);
> > +}
> > +
> > +static int mv88e61xx_set_page(struct udevice *dev, u8 phy, u8 page)
> > +{
> > + return mv88e61xx_phy_write(dev, phy, PHY_REG_PAGE, page);
> > +}
> > +
> > +static int mv88e61xx_get_switch_id(struct udevice *dev)
> > +{
> > + int res;
> > +
> > + res = mv88e61xx_port_read(dev, 0, PORT_REG_SWITCH_ID);
> > + if (res < 0)
> > + return res;
> > + return res & 0xfff0;
> > +}
> > +
> > +static bool mv88e61xx_6352_family(struct udevice *dev)
> > +{
> > + struct mv88e61xx_priv *priv = dev_get_priv(dev);
> > +
> > + switch (priv->id) {
> > + case PORT_SWITCH_ID_6172:
> > + case PORT_SWITCH_ID_6176:
> > + case PORT_SWITCH_ID_6240:
> > + case PORT_SWITCH_ID_6352:
> > + return true;
> > + }
> > + return false;
> > +}
> > +
> > +static int mv88e61xx_get_cmode(struct udevice *dev, u8 port)
> > +{
> > + int res;
> > +
> > + res = mv88e61xx_port_read(dev, port, PORT_REG_STATUS);
> > + if (res < 0)
> > + return res;
> > + return res & PORT_REG_STATUS_CMODE_MASK;
> > +}
> > +
> > +static int mv88e61xx_switch_reset(struct udevice *dev)
> > +{
> > + struct mv88e61xx_priv *priv = dev_get_priv(dev);
> > + int time;
> > + int val;
> > + u8 port;
> > +
> > + /* Disable all ports */
> > + for (port = 0; port < priv->port_count; port++) {
> > + val = mv88e61xx_port_read(dev, port, PORT_REG_CTRL);
> > + if (val < 0)
> > + return val;
> > + val = bitfield_replace(val, PORT_REG_CTRL_PSTATE_SHIFT,
> > + PORT_REG_CTRL_PSTATE_WIDTH,
> > + PORT_REG_CTRL_PSTATE_DISABLED);
> > + val = mv88e61xx_port_write(dev, port, PORT_REG_CTRL, val);
> > + if (val < 0)
> > + return val;
> > + }
> > +
> > + /* Wait 2 ms for queues to drain */
> > + udelay(2000);
> > +
> > + /* Reset switch */
> > + val = mv88e61xx_reg_read(dev, priv->global1, GLOBAL1_CTRL);
> > + if (val < 0)
> > + return val;
> > + val |= GLOBAL1_CTRL_SWRESET;
> > + val = mv88e61xx_reg_write(dev, priv->global1, GLOBAL1_CTRL, val);
> > + if (val < 0)
> > + return val;
> > +
> > + /* Wait up to 1 second for switch reset complete */
> > + for (time = 1000; time; time--) {
> > + val = mv88e61xx_reg_read(dev, priv->global1, GLOBAL1_CTRL);
> > + if (val >= 0 && ((val & GLOBAL1_CTRL_SWRESET) == 0))
> > + break;
> > + udelay(1000);
> > + }
> > + if (!time)
> > + return -ETIMEDOUT;
> > +
> > + return 0;
> > +}
> > +
> > +static int mv88e61xx_serdes_init(struct udevice *dev)
> > +{
> > + int val;
> > +
> > + val = mv88e61xx_set_page(dev, DEVADDR_SERDES, PHY_PAGE_SERDES);
> > + if (val < 0)
> > + return val;
> > +
> > + /* Power up serdes module */
> > + val = mv88e61xx_phy_read(dev, DEVADDR_SERDES, MII_BMCR);
> > + if (val < 0)
> > + return val;
> > + val &= ~(BMCR_PDOWN);
> > + val = mv88e61xx_phy_write(dev, DEVADDR_SERDES, MII_BMCR, val);
> > + if (val < 0)
> > + return val;
> > +
> > + return 0;
> > +}
> > +
> > +static int mv88e61xx_fixed_port_setup(struct udevice *dev, u8 port)
> > +{
> > + struct dsa_pdata *dsa_pdata = dev_get_uclass_plat(dev);
> > + struct mv88e61xx_priv *priv = dev_get_priv(dev);
> > + int val;
> > +
> > + val = mv88e61xx_port_read(dev, port, PORT_REG_PHYS_CTRL);
> > + if (val < 0)
> > + return val;
> > +
> > + val &= ~(PORT_REG_PHYS_CTRL_SPD_MASK |
> > + PORT_REG_PHYS_CTRL_FC_VALUE |
> > + PORT_REG_PHYS_CTRL_FC_FORCE);
> > + val |= PORT_REG_PHYS_CTRL_FC_FORCE |
> > + PORT_REG_PHYS_CTRL_DUPLEX_VALUE |
> > + PORT_REG_PHYS_CTRL_DUPLEX_FORCE;
> > +
> > + if (priv->id == PORT_SWITCH_ID_6071) {
> > + val |= PORT_REG_PHYS_CTRL_SPD100;
> > + } else {
> > + val |= PORT_REG_PHYS_CTRL_PCS_AN_EN |
> > + PORT_REG_PHYS_CTRL_PCS_AN_RST |
> > + PORT_REG_PHYS_CTRL_SPD1000;
> > + }
> > +
> > + if (port == dsa_pdata->cpu_port)
> > + val |= PORT_REG_PHYS_CTRL_LINK_VALUE |
> > + PORT_REG_PHYS_CTRL_LINK_FORCE;
> > +
> > + return mv88e61xx_port_write(dev, port, PORT_REG_PHYS_CTRL, val);
> > +}
> > +
> > +/*
> > + * This function is used to pre-configure the required register
> > + * offsets, so that the indirect register access to the PHY registers
> > + * is possible. This is necessary to be able to read the PHY ID
> > + * while driver probing or in get_phy_id(). The globalN register
> > + * offsets must be initialized correctly for a detected switch,
> > + * otherwise detection of the PHY ID won't work!
> > + */
> > +static int mv88e61xx_priv_reg_offs_pre_init(struct udevice *dev)
> > +{
> > + struct mv88e61xx_priv *priv = dev_get_priv(dev);
> > +
> > + /*
> > + * Initial 'port_reg_base' value must be an offset of existing
> > + * port register, then reading the ID should succeed. First, try
> > + * to read via port registers with device address 0x10 (88E6096
> > + * and compatible switches).
> > + */
> > + priv->port_reg_base = 0x10;
> > + priv->id = mv88e61xx_get_switch_id(dev);
> > + if (priv->id != 0xfff0) {
> > + priv->global1 = 0x1B;
> > + priv->global2 = 0x1C;
> > + return 0;
> > + }
> > +
> > + /*
> > + * Now try via port registers with device address 0x08
> > + * (88E6020 and compatible switches).
> > + */
> > + priv->port_reg_base = 0x08;
> > + priv->id = mv88e61xx_get_switch_id(dev);
> > + if (priv->id != 0xfff0) {
> > + priv->global1 = 0x0F;
> > + priv->global2 = 0x07;
> > + return 0;
> > + }
> > +
> > + dev_warn(dev, "%s Unknown ID 0x%x\n", __func__, priv->id);
> > +
> > + return -ENODEV;
> > +}
> > +
> > +static int mv88e61xx_mdio_read(struct udevice *dev, int addr, int devad, int reg)
> > +{
> > + return mv88e61xx_phy_read_indirect(dev->parent, DEVADDR_PHY(addr),
> > + MDIO_DEVAD_NONE, reg);
> > +}
> > +
> > +static int mv88e61xx_mdio_write(struct udevice *dev, int addr, int devad,
> > + int reg, u16 val)
> > +{
> > + return mv88e61xx_phy_write_indirect(dev->parent, DEVADDR_PHY(addr),
> > + MDIO_DEVAD_NONE, reg, val);
> > +}
> > +
> > +static const struct mdio_ops mv88e61xx_mdio_ops = {
> > + .read = mv88e61xx_mdio_read,
> > + .write = mv88e61xx_mdio_write,
> > +};
> > +
> > +static int mv88e61xx_mdio_bind(struct udevice *dev)
> > +{
> > + char name[32];
> > + static int num_devices;
> > +
> > + sprintf(name, "mv88e61xx-mdio-%d", num_devices++);
> > + device_set_name(dev, name);
> > +
> > + return 0;
> > +}
> > +
> > +U_BOOT_DRIVER(mv88e61xx_mdio) = {
> > + .name = "mv88e61xx_mdio",
> > + .id = UCLASS_MDIO,
> > + .ops = &mv88e61xx_mdio_ops,
> > + .bind = mv88e61xx_mdio_bind,
> > + .priv_auto = sizeof(struct mv88e61xx_priv),
> > + .plat_auto = sizeof(struct mdio_perdev_priv),
> > +};
> > +
> > +static int mv88e61xx_dsa_port_enable(struct udevice *dev, int port, struct phy_device *phy)
> > +{
> > + struct dsa_pdata *dsa_pdata = dev_get_uclass_plat(dev);
> > + struct mv88e61xx_priv *priv = dev_get_priv(dev);
> > + int val, ret;
> > +
> > + dev_dbg(dev, "%s P%d phy:0x%08x %s\n", __func__, port,
> > + phy->phy_id, phy_string_for_interface(phy->interface));
> > +
> > + /*
> > + * Enable energy-detect sensing on PHY, used to determine when a PHY
> > + * port is physically connected
> > + */
> > + val = mv88e61xx_phy_read(dev, port, PHY_REG_CTRL1);
> > + if (val < 0)
> > + return val;
> > + val = bitfield_replace(val, priv->phy_ctrl1_en_det_shift,
> > + priv->phy_ctrl1_en_det_width,
> > + priv->phy_ctrl1_en_det_ctrl);
> > + val = mv88e61xx_phy_write(dev, port, PHY_REG_CTRL1, val);
> > + if (val < 0)
> > + return val;
> > +
> > + /* enable port */
> > + val = mv88e61xx_port_read(dev, port, PORT_REG_CTRL);
> > + if (val < 0)
> > + return val;
> > + val = bitfield_replace(val, PORT_REG_CTRL_PSTATE_SHIFT,
> > + PORT_REG_CTRL_PSTATE_WIDTH,
> > + PORT_REG_CTRL_PSTATE_FORWARD);
> > + val = mv88e61xx_port_write(dev, port, PORT_REG_CTRL, val);
> > + if (val < 0)
> > + return val;
> > +
> > + /* configure RGMII delays for fixed link */
> > + if (phy->phy_id == PHY_FIXED_ID) {
> > + /* Physical Control register: Table 62 */
> > + val = mv88e61xx_port_read(dev, port, PORT_REG_PHYS_CTRL);
> > + val &= ~(PORT_REG_PHYS_CTRL_RGMII_DELAY_RXCLK ||
> > + PORT_REG_PHYS_CTRL_RGMII_DELAY_TXCLK);
> > + if (phy->interface == PHY_INTERFACE_MODE_RGMII_ID ||
> > + phy->interface == PHY_INTERFACE_MODE_RGMII_RXID)
> > + val |= PORT_REG_PHYS_CTRL_RGMII_DELAY_RXCLK;
> > + if (phy->interface == PHY_INTERFACE_MODE_RGMII_ID ||
> > + phy->interface == PHY_INTERFACE_MODE_RGMII_TXID)
> > + val |= PORT_REG_PHYS_CTRL_RGMII_DELAY_TXCLK;
> > + val |= BIT(5) | BIT(4); /* Force link */
> > + ret = mv88e61xx_port_write(dev, port, PORT_REG_PHYS_CTRL, val);
> > + if (ret < 0)
> > + return ret;
> > +
> > + ret = mv88e61xx_fixed_port_setup(dev, port);
> > + if (ret < 0)
> > + return ret;
> > + }
> > +
> > + if (port == dsa_pdata->cpu_port) {
> > + /* Set CPUDest for cpu port */
> > + val = mv88e61xx_reg_read(dev, priv->global1, GLOBAL1_MON_CTRL);
> > + if (val < 0)
> > + return val;
> > + val = bitfield_replace(val, GLOBAL1_MON_CTRL_CPUDEST_SHIFT,
> > + GLOBAL1_MON_CTRL_CPUDEST_WIDTH, port);
> > + val = mv88e61xx_reg_write(dev, priv->global1, GLOBAL1_MON_CTRL, val);
> > + if (val < 0)
> > + return val;
> > +
> > + if (mv88e61xx_6352_family(dev)) {
> > + /* initialize serdes */
> > + val = mv88e61xx_get_cmode(dev, dsa_pdata->cpu_port);
> > + if (val < 0)
> > + return val;
> > + if (val == PORT_REG_STATUS_CMODE_100BASE_X ||
> > + val == PORT_REG_STATUS_CMODE_1000BASE_X ||
> > + val == PORT_REG_STATUS_CMODE_SGMII) {
> > + val = mv88e61xx_serdes_init(dev);
> > + if (val < 0)
> > + return val;
> > + }
> > + }
> > + }
> > +
> > + return 0;
> > +}
> > +
> > +static void mv88e61xx_dsa_port_disable(struct udevice *dev, int port, struct phy_device *phy)
> > +{
> > + int val;
> > +
> > + dev_dbg(dev, "%s P%d phy:0x%x\n", __func__, port, phy->phy_id);
> > +
> > + val = mv88e61xx_port_read(dev, port, PORT_REG_CTRL);
> > + if (val < 0)
> > + return;
> > + val = bitfield_replace(val, PORT_REG_CTRL_PSTATE_SHIFT,
> > + PORT_REG_CTRL_PSTATE_WIDTH,
> > + PORT_REG_CTRL_PSTATE_DISABLED);
> > + val = mv88e61xx_port_write(dev, port, PORT_REG_CTRL, val);
> > + if (val < 0)
> > + return;
> > +}
> > +
> > +static const struct dsa_ops mv88e61xx_dsa_ops = {
> > + .port_enable = mv88e61xx_dsa_port_enable,
> > + .port_disable = mv88e61xx_dsa_port_disable,
>
> You can drop "dsa" from these names so they become "mv88e6xxx_port_enable"
> and "mv88e6xxx_port_disable". In Marvell terminology, a DSA port is a
> technical term for a cascade port, and this is not what this is about.
>
ok
> > +};
> > +
> > +/* bind and probe the switch mdios */
> > +static int mv88e61xx_dsa_probe_mdio(struct udevice *dev)
>
> And in general you can drop the "dsa" word from most function names, it
> makes them longer with not a lot of benefit.
>
ok
> > +{
> > + struct udevice *pdev;
> > + ofnode node, mdios;
> > + const char *name;
> > + int ret;
> > +
> > + /* bind phy ports of mdios child node to mv88e61xx_mdio device */
> > + mdios = dev_read_subnode(dev, "mdios");
> > + if (ofnode_valid(mdios)) {
> > + ofnode_for_each_subnode(node, mdios) {
> > + name = ofnode_get_name(node);
> > + ret = device_bind_driver_to_node(dev,
> > + "mv88e61xx_mdio",
> > + name, node, &pdev);
> > + if (ret) {
> > + dev_err(dev, "failed to bind %s: %d\n", name, ret);
> > + continue;
> > + }
> > +
> > + /* need to probe it as there is no compatible to do so */
> > + ret = uclass_get_device_by_ofnode(UCLASS_MDIO, node, &pdev);
> > + if (ret) {
> > + dev_err(dev, "failed to probe %s: %d\n", name, ret);
> > + continue;
> > + }
>
> What do you do with this pdev once you get it? Are you missing a device_probe() call?
> Also, why "pdev" and not "dev"? What does the "p" stand for?
struct udevice *dev is passed into the function so I use pdev to
iterate over the ports in the mdios node so 'pdev' means 'port' here.
I do not need to do anything with pdev but I must use
uclass_get_device_by_ofnode() to probe it and that function requires
it. I don't need to call device_probe because
uclass_get_device_by_ofnode does it for me
>
> > + }
> > + }
> > +
> > + /* bind the mdios node to the parent mdio driver */
> > + name = ofnode_get_name(mdios);
> > + ret = device_bind_driver_to_node(dev,
> > + "mv88e61xx_mdio",
> > + name, mdios, &pdev);
> > + if (ret)
> > + dev_err(dev, "failed to bind %s: %d\n", name, ret);
> > +
> > + /* need to probe it as there is no compatible to do so */
> > + ret = uclass_get_device_by_ofnode(UCLASS_MDIO, mdios, &pdev);
> > + if (ret)
> > + dev_err(dev, "failed to probe %s: %d\n", name, ret);
> > +
> > + return ret;
>
> I think this after the "if (ofnode_valid(mdios))" check is all stray
> code that should be removed. There is no udevice that should be bound to
> the "mdios" node, that is just a container.
yes, your correct that code is not needed - will remove
>
> That, plus you should try to reduce the indentation by one level by
> exiting early if (!ofnode_valid(mdio)) (and not with an error, it is
> completely valid to not have an MDIO controller described in DT).
>
ok, good idea
Thanks for the review!
Tim
> > +}
> > +
> > +static int mv88e61xx_dsa_probe(struct udevice *dev)
> > +{
> > + struct mv88e61xx_priv *priv = dev_get_priv(dev);
> > + struct udevice *master = dsa_get_master(dev);
> > + int ret;
> > +
> > + if (!master)
> > + return -ENODEV;
> > +
> > + dev_dbg(dev, "%s master:%s\n", __func__, master->name);
> > +
> > + /* probe internal mdio bus */
> > + ret = mv88e61xx_dsa_probe_mdio(dev);
> > + if (ret)
> > + return ret;
> > +
> > + ret = mv88e61xx_priv_reg_offs_pre_init(dev);
> > + if (ret)
> > + return ret;
> > +
> > + dev_dbg(dev, "ID=0x%x PORT_BASE=0x%02x GLOBAL1=0x%02x GLOBAL2=0x%02x\n",
> > + priv->id, priv->port_reg_base, priv->global1, priv->global2);
> > + switch (priv->id) {
> > + case PORT_SWITCH_ID_6096:
> > + case PORT_SWITCH_ID_6097:
> > + case PORT_SWITCH_ID_6172:
> > + case PORT_SWITCH_ID_6176:
> > + case PORT_SWITCH_ID_6240:
> > + case PORT_SWITCH_ID_6352:
> > + priv->port_count = 11;
> > + priv->phy_ctrl1_en_det_shift = 8;
> > + priv->phy_ctrl1_en_det_width = 2;
> > + priv->phy_ctrl1_en_det_ctrl =
> > + PHY_REG_CTRL1_ENERGY_DET_SENSE_XMIT;
> > + break;
> > + case PORT_SWITCH_ID_6020:
> > + case PORT_SWITCH_ID_6070:
> > + case PORT_SWITCH_ID_6071:
> > + case PORT_SWITCH_ID_6220:
> > + case PORT_SWITCH_ID_6250:
> > + priv->port_count = 7;
> > + priv->phy_ctrl1_en_det_shift = 14;
> > + priv->phy_ctrl1_en_det_width = 1;
> > + priv->phy_ctrl1_en_det_ctrl =
> > + PHY_REG_CTRL1_ENERGY_DET_SENSE_PULSE;
> > + break;
> > + default:
> > + return -ENODEV;
> > + }
> > +
> > + ret = mv88e61xx_switch_reset(dev);
> > + if (ret < 0)
> > + return ret;
> > +
> > + dsa_set_tagging(dev, 0, 0);
> > +
> > + return 0;
> > +}
> > +
> > +static const struct udevice_id mv88e61xx_ids[] = {
> > + { .compatible = "marvell,mv88e6085" },
> > + { }
> > +};
> > +
> > +U_BOOT_DRIVER(mv88e61xx) = {
> > + .name = "mv88e61xx",
> > + .id = UCLASS_DSA,
> > + .of_match = mv88e61xx_ids,
> > + .probe = mv88e61xx_dsa_probe,
> > + .ops = &mv88e61xx_dsa_ops,
> > + .priv_auto = sizeof(struct mv88e61xx_priv),
> > +};
> > --
> > 2.17.1
> >
More information about the U-Boot
mailing list