Hi Joe, Tom,

On Wed, Mar 17, 2021 at 4:14 PM Lukasz Majewski lukma@denx.de wrote:

...

Some Marvell switch devices are dual chip ones, like mv88e6020, which use direct MDIO addressing to access its ports' registers. Such approach allows connecting two such devices in a single MDIO bus with simple addressing scheme.

Signed-off-by: Lukasz Majewski lukma@denx.de

drivers/net/phy/mv88e61xx.c | 42 +++++++++++++++++++++++++++++++++++++ 1 file changed, 42 insertions(+)

diff --git a/drivers/net/phy/mv88e61xx.c b/drivers/net/phy/mv88e61xx.c index 7eff37b24499..69a1dd8f1859 100644 --- a/drivers/net/phy/mv88e61xx.c +++ b/drivers/net/phy/mv88e61xx.c @@ -202,6 +202,7 @@ struct mv88e61xx_phy_priv { 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 */

•   u8 direct_access;          /* Access switch device directly
  

*/ };

static inline int smi_cmd(int cmd, int addr, int reg) @@ -928,6 +929,40 @@ static int mv88e61xx_priv_reg_offs_pre_init(struct phy_device *phydev) return -ENODEV; }

+static int mv88e61xx_check_addressing(struct phy_device *phydev) +{

•   if (!CONFIG_IS_ENABLED(OF_CONTROL))
  

•           return 0;
  

•   /*
  

•    * Some devices - like mv88e6020 are dual chip - i.e. two
  

•    * such devices can be directly accessed via SMI bus.
  

•    * The addressing depends on R0_LED/ADDR4 pin value duing
  

•    * bootstrap.
  

•    *
  

•    * This means that there is no need for indirect access.
  

•    */
  

•   struct mv88e61xx_phy_priv *priv = phydev->priv;
  

•   /*
  

•    * As this function is called very early and hence the
  

phydev

•    * is not yet initialized we use aliast and DTS to asses if
  

•    * device shall be directly accessed or not.
  

•    */
  

•   ofnode sw0;
  

•   int ret;
  

•   sw0 = ofnode_get_aliases_node("switch0");
  

•   if (!ofnode_valid(sw0))
  

•           return -ENODEV;
  

•   ret = ofnode_device_is_compatible(sw0, "marvell,mv88e6020");
  

•   if (ret)
  

•           priv->direct_access = 1;
  

•   return 0;
  

+}

static int mv88e61xx_probe(struct phy_device *phydev) { struct mii_dev *smi_wrapper; @@ -982,6 +1017,8 @@ static int mv88e61xx_probe(struct phy_device *phydev)

    phydev->priv = priv;

•   mv88e61xx_check_addressing(phydev);
  

•   res = mv88e61xx_priv_reg_offs_pre_init(phydev);
    if (res < 0)
            return res;
  

@@ -1197,6 +1234,11 @@ int get_phy_id(struct mii_dev *bus, int smi_addr, int devad, u32 *phy_id) temp_phy.priv = &temp_priv; temp_mii.priv = &temp_phy;

•   mv88e61xx_check_addressing(&temp_phy);
  

•   /* For direct access the phy address equals to smi_addr */
  

•   if (temp_priv.direct_access)
  

•           temp_phy.addr = smi_addr;
  

•   /*
     * get_phy_id() can be called by framework before mv88e61xx
  

driver * probing, in this case the global register offsets are not -- 2.20.1

Reviewed-by: Ramon Fried rfried.dev@gmail.com

Could this patch series be pulled to u-boot-net tree (or directly to -master branch) ?

Best regards,

Lukasz Majewski

--

DENX Software Engineering GmbH, Managing Director: Wolfgang Denk HRB 165235 Munich, Office: Kirchenstr.5, D-82194 Groebenzell, Germany Phone: (+49)-8142-66989-59 Fax: (+49)-8142-66989-80 Email: lukma@denx.de

On Wed, Mar 17, 2021 at 4:14 PM Lukasz Majewski lukma@denx.de wrote:

After this change PHY ports are able to pass packets between them (and also to CPU port).

The Kconfig variable - CONFIG_MV88E61XX_PHY_PORTS - is used to get the PHY ports of the switch and generate proper mask.

Signed-off-by: Lukasz Majewski lukma@denx.de

drivers/net/phy/mv88e61xx.c | 9 +++++++-- 1 file changed, 7 insertions(+), 2 deletions(-)

diff --git a/drivers/net/phy/mv88e61xx.c b/drivers/net/phy/mv88e61xx.c index 69a1dd8f1859..14074c0b82fb 100644 --- a/drivers/net/phy/mv88e61xx.c +++ b/drivers/net/phy/mv88e61xx.c @@ -873,14 +873,19 @@ static int mv88e61xx_phy_setup(struct phy_device *phydev, u8 phy)

static int mv88e61xx_phy_config_port(struct phy_device *phydev, u8 phy) {

•   struct mv88e61xx_phy_priv *priv = phydev->priv;
  

•   u16 port_mask;
    int val;
  
    val = mv88e61xx_port_enable(phydev, phy);
    if (val < 0)
            return val;
  
  

•   val = mv88e61xx_port_set_vlan(phydev, phy,
  

•                   1 << CONFIG_MV88E61XX_CPU_PORT);
  

•   port_mask = PORT_MASK(priv->port_count) & CONFIG_MV88E61XX_PHY_PORTS;
  

•   port_mask &= ~(1 << phy);
  

•   port_mask |= (1 << CONFIG_MV88E61XX_CPU_PORT);
  

•   val = mv88e61xx_port_set_vlan(phydev, phy, port_mask);
    if (val < 0)
            return val;
  
  

-- 2.20.1

Reviewed-by: Ramon Fried rfried.dev@gmail.com

On Wed, Mar 17, 2021 at 4:14 PM Lukasz Majewski lukma@denx.de wrote:

Those automatically created structures can have random value. However, mv88e61xx driver assumes that those are zeroed.

Signed-off-by: Lukasz Majewski lukma@denx.de

drivers/net/phy/mv88e61xx.c | 4 ++++ 1 file changed, 4 insertions(+)

diff --git a/drivers/net/phy/mv88e61xx.c b/drivers/net/phy/mv88e61xx.c index 14074c0b82fb..d8116530700d 100644 --- a/drivers/net/phy/mv88e61xx.c +++ b/drivers/net/phy/mv88e61xx.c @@ -1230,6 +1230,10 @@ int get_phy_id(struct mii_dev *bus, int smi_addr, int devad, u32 *phy_id) struct mii_dev temp_mii; int val;

•   memset(&temp_phy, 0, sizeof(temp_phy));
  

•   memset(&temp_priv, 0, sizeof(temp_priv));
  

•   memset(&temp_mii, 0, sizeof(temp_mii));
  

•   /*
     * Buid temporary data structures that the chip reading code needs to
     * read the ID
  

-- 2.20.1

Reviewed-by: Ramon Fried rfried.dev@gmail.com

On Wed, Mar 17, 2021 at 4:14 PM Lukasz Majewski lukma@denx.de wrote:

The mv88e6020 is accessed in a direct way (i.e. with direct read and write to mdio bus). The only necessary indirection is required when accessing its PHY registers.

Signed-off-by: Lukasz Majewski lukma@denx.de

drivers/net/phy/mv88e61xx.c | 15 ++++++++++----- 1 file changed, 10 insertions(+), 5 deletions(-)

diff --git a/drivers/net/phy/mv88e61xx.c b/drivers/net/phy/mv88e61xx.c index d8116530700d..3d846b89fd12 100644 --- a/drivers/net/phy/mv88e61xx.c +++ b/drivers/net/phy/mv88e61xx.c @@ -269,8 +269,11 @@ static int mv88e61xx_reg_read(struct phy_device *phydev, int dev, int reg) int smi_addr = priv->smi_addr; int res;

•   /* In single-chip mode, the device can be addressed directly */
  

•   if (smi_addr == 0)
  

•   /*
  

•    * In single-chip or dual-chip (like mv88e6020) mode, the device can
  

•    * be addressed directly.
  

•    */
  

•   if (smi_addr == 0 || priv->direct_access)
            return mdio_bus->read(mdio_bus, dev, MDIO_DEVAD_NONE, reg);
  
    /* Wait for the bus to become free */
  

@@ -306,11 +309,13 @@ static int mv88e61xx_reg_write(struct phy_device *phydev, int dev, int reg, int smi_addr = priv->smi_addr; int res;

•   /* In single-chip mode, the device can be addressed directly */
  

•   if (smi_addr == 0) {
  

•   /*
  

•    * In single-chip or dual-chip (like mv88e6020) mode, the device can
  

•    * be addressed directly.
  

•    */
  

•   if (smi_addr == 0 || priv->direct_access)
            return mdio_bus->write(mdio_bus, dev, MDIO_DEVAD_NONE, reg,
                            val);
  

•   }
  
    /* Wait for the bus to become free */
    res = mv88e61xx_smi_wait(mdio_bus, smi_addr);
  

-- 2.20.1

Reviewed-by: Ramon Fried rfried.dev@gmail.com

On Wed, Mar 17, 2021 at 4:14 PM Lukasz Majewski lukma@denx.de wrote:

The mv88e61xx driver need to be adjusted to handle situation when switch MDIO addresses are switched by offset (0x10 in this case).

Signed-off-by: Lukasz Majewski lukma@denx.de

drivers/net/phy/mv88e61xx.c | 25 ++++++++++++++++++------- 1 file changed, 18 insertions(+), 7 deletions(-)

diff --git a/drivers/net/phy/mv88e61xx.c b/drivers/net/phy/mv88e61xx.c index 3d846b89fd12..325d5b56135f 100644 --- a/drivers/net/phy/mv88e61xx.c +++ b/drivers/net/phy/mv88e61xx.c @@ -45,7 +45,6 @@ #define PORT_MASK(port_count) ((1 << (port_count)) - 1)

/* Device addresses */ -#define DEVADDR_PHY(p) (p) #define DEVADDR_SERDES 0x0F

/* SMI indirection registers for multichip addressing mode */ @@ -414,7 +413,7 @@ static int mv88e61xx_phy_write_indirect(struct mii_dev *smi_wrapper, int dev, /* Wrapper function to make calls to phy_read_indirect simpler */ static int mv88e61xx_phy_read(struct phy_device *phydev, int phy, int reg) {

•   return mv88e61xx_phy_read_indirect(phydev->bus, DEVADDR_PHY(phy),
  

•   return mv88e61xx_phy_read_indirect(phydev->bus, phydev->addr,
                                       MDIO_DEVAD_NONE, reg);
  

}

@@ -422,7 +421,7 @@ static int mv88e61xx_phy_read(struct phy_device *phydev, int phy, int reg) static int mv88e61xx_phy_write(struct phy_device *phydev, int phy, int reg, u16 val) {

•   return mv88e61xx_phy_write_indirect(phydev->bus, DEVADDR_PHY(phy),
  

•   return mv88e61xx_phy_write_indirect(phydev->bus, phydev->addr,
                                        MDIO_DEVAD_NONE, reg, val);
  

}

@@ -926,12 +925,21 @@ static int mv88e61xx_priv_reg_offs_pre_init(struct phy_device *phydev) /* * Now try via port registers with device address 0x08 * (88E6020 and compatible switches).

•    *
  

•    * When R0_LED/ADDR4 is set during bootstrap, one needs
  

•    * to add 0x10 offset to switch addresses.
  

•    *
  

•    * The phydev->addr is set according to device tree address
  

•    * of MDIO accessible device:
  

•    *
  

•    * When on board RO_LED/ADDR4 = 1 -> 0x10
  

•    *                              0 -> 0x0
     */
  

•   priv->port_reg_base = 0x08;
  

•   priv->port_reg_base = 0x08 + phydev->addr;
    priv->id = mv88e61xx_get_switch_id(phydev);
    if (priv->id != 0xfff0) {
  

•           priv->global1 = 0x0F;
  

•           priv->global2 = 0x07;
  

•           priv->global1 = 0x0F + phydev->addr;
  

•           priv->global2 = 0x07 + phydev->addr;
            return 0;
    }
  
  

@@ -1090,7 +1098,10 @@ static int mv88e61xx_phy_config(struct phy_device *phydev)

    for (i = 0; i < priv->port_count; i++) {
            if ((1 << i) & CONFIG_MV88E61XX_PHY_PORTS) {

•                   phydev->addr = i;
  

•                   if (phydev->addr)
  

•                           phydev->addr += i;
  

•                   else
  

•                           phydev->addr = i;
  
                    res = mv88e61xx_phy_enable(phydev, i);
                    if (res < 0) {
  

-- 2.20.1

Reviewed-by: Ramon Fried rfried.dev@gmail.com

On Wed, Mar 17, 2021 at 4:14 PM Lukasz Majewski lukma@denx.de wrote:

Some devices, when configured in bootstrap to 'no cpu' mode require PHY manual reset to get them operational and responding to reading their ID registers.

Without this step - the PHYLIB probing will fail.

In more details - the bootstrap configuration from switch must be read. The value of CONFIG Data1 (0x71) of Scratch and Misc register is read to check if 'no_cpu' and 'addr4' bits were set.

Signed-off-by: Lukasz Majewski lukma@denx.de

---

drivers/net/phy/mv88e61xx.c | 63 +++++++++++++++++++++++++++++++++++-- 1 file changed, 61 insertions(+), 2 deletions(-)

diff --git a/drivers/net/phy/mv88e61xx.c b/drivers/net/phy/mv88e61xx.c index 325d5b56135f..1fa821ca162b 100644 --- a/drivers/net/phy/mv88e61xx.c +++ b/drivers/net/phy/mv88e61xx.c @@ -202,6 +202,17 @@ struct mv88e61xx_phy_priv { u8 phy_ctrl1_en_det_width; /* Width of 'EDet' bit field */ u8 phy_ctrl1_en_det_ctrl; /* 'EDet' control value */ u8 direct_access; /* Access switch device directly */

•   /*
  

•    * Bootstrap configuration:
  

•    *
  

•    * If addr4 = 1 device is accessible from 0x10 address on MDIO bus.
  

•    */
  

•   u8 addr4;
  

•   /*
  

•    * If no_cpu = 1 switch is automatically setup, otherwise PHY reset is
  

•    * required from CPU for normal operation.
  

•    */
  

•   u8 no_cpu;
  

};

static inline int smi_cmd(int cmd, int addr, int reg) @@ -1235,6 +1246,33 @@ int phy_mv88e61xx_init(void) return 0; }

+static int mv88e61xx_read_bootstrap(struct phy_device *phydev) +{

•   struct mv88e61xx_phy_priv *priv = phydev->priv;
  

•   struct mii_dev *mdio_bus = priv->mdio_bus;
  

•   int val;
  

•   /* mv88e6020 - ID = 0x0200 (REG 3 on non PHY port) */
  

•   if (priv->id == PORT_SWITCH_ID_6020) {
  

•           /* Prepare to read scratch and misc register */
  

•           mdio_bus->write(mdio_bus, priv->global2, 0,
  

•                           0x1a /*MV_SCRATCH_MISC*/,
  

•                           (0x71 /*MV_CONFIG_DATA1*/ << 8));
  

•           val = mdio_bus->read(mdio_bus, priv->global2, 0,
  

•                                0x1a /*MV_SCRATCH_MISC*/);
  

•           if (val & (1 << 0))
  

•                   priv->no_cpu = 1;
  

•           if (val & (1 << 4))
  

•                   priv->addr4 = 1;
  

•           debug("mv88e6020: no_cpu=%d addr4=%d\n", priv->no_cpu,
  

•                 priv->addr4);
  

•   }
  

•   return 0;
  

+}

/*

• Overload weak get_phy_id definition since we need non-standard functions
• to read PHY registers

@@ -1274,13 +1312,34 @@ int get_phy_id(struct mii_dev *bus, int smi_addr, int devad, u32 *phy_id) if (val < 0) return val;

•   val = mv88e61xx_phy_read_indirect(&temp_mii, 0, devad, MII_PHYSID1);
  

•   mv88e61xx_read_bootstrap(&temp_phy);
  

•   /*
  

•    * When switch is configured to work with CPU (i.e. NO_CPU == 0), PHYs
  

•    * require reset (to at least single one) to have its registers
  

•    * accessible.
  

•    */
  

•   if (!temp_priv.no_cpu && temp_priv.id == PORT_SWITCH_ID_6020) {
  

•           /* Reset PHY */
  

•           val = mv88e61xx_phy_read_indirect(&temp_mii, temp_phy.addr,
  

•                                             devad, MII_BMCR);
  

•           if (val & BMCR_PDOWN)
  

•                   val &= ~BMCR_PDOWN;
  

•           mv88e61xx_phy_write_indirect(&temp_mii, temp_phy.addr, devad,
  

•                                        MII_BMCR, val);
  

•   }
  

•   /* Read PHY_ID */
  

•   val = mv88e61xx_phy_read_indirect(&temp_mii, temp_phy.addr, devad,
  

•                                     MII_PHYSID1);
    if (val < 0)
            return -EIO;
  
    *phy_id = val << 16;
  
  

•   val = mv88e61xx_phy_read_indirect(&temp_mii, 0, devad, MII_PHYSID2);
  

•   val = mv88e61xx_phy_read_indirect(&temp_mii, temp_phy.addr, devad,
  

•                                     MII_PHYSID2);
    if (val < 0)
            return -EIO;
  
  

-- 2.20.1

Reviewed-by: Ramon Fried rfried.dev@gmail.com