[PATCH v1 0/6] Provide support for mv88e6020 Marvell switch

[PATCH v1 0/6] Provide support for mv88e6020 Marvell switch

[Lukasz Majewski]

This patch set provides support for mv88e6020 switch. It is a dual chip
device, with direct access to port register on SMI bus. However, PHYs
are accessed indirectly.


Lukasz Majewski (6):
  net: mv88e61xx: Add support for checking addressing mode
  net: mv88e61xx: Configure PHY ports to also pass packets between them
  net: mv88e61xx: Clear temporary structs before using them in
    get_phy_id()
  net: mv88e61xx: Directly access the switch chip
  net: mv88e61xx: Set proper offset when R0_LED/ADDR4 is set on
    bootstrap
  net: mv88e61xx: Reset switch PHYs when bootstrapped to !NO_CPU

 drivers/net/phy/mv88e61xx.c | 158 ++++++++++++++++++++++++++++++++----
 1 file changed, 142 insertions(+), 16 deletions(-)

-- 
2.20.1

[PATCH v1 1/6] net: mv88e61xx: Add support for checking addressing mode

[Lukasz Majewski]

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

[PATCH v1 2/6] net: mv88e61xx: Configure PHY ports to also pass packets between them

[Lukasz Majewski]

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

[PATCH v1 3/6] net: mv88e61xx: Clear temporary structs before using them in get_phy_id()

[Lukasz Majewski]

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

[PATCH v1 4/6] net: mv88e61xx: Directly access the switch chip

[Lukasz Majewski]

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

[PATCH v1 5/6] net: mv88e61xx: Set proper offset when R0_LED/ADDR4 is set on bootstrap

[Lukasz Majewski]

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

[PATCH v1 6/6] net: mv88e61xx: Reset switch PHYs when bootstrapped to !NO_CPU

[Lukasz Majewski]

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@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

[PATCH v1 1/6] net: mv88e61xx: Add support for checking addressing mode

[Ramon Fried]

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>

[PATCH v1 2/6] net: mv88e61xx: Configure PHY ports to also pass packets between them

[Ramon Fried]

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>

[PATCH v1 3/6] net: mv88e61xx: Clear temporary structs before using them in get_phy_id()

[Ramon Fried]

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>

[PATCH v1 4/6] net: mv88e61xx: Directly access the switch chip

[Ramon Fried]

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>

[PATCH v1 5/6] net: mv88e61xx: Set proper offset when R0_LED/ADDR4 is set on bootstrap

[Ramon Fried]

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>

[PATCH v1 6/6] net: mv88e61xx: Reset switch PHYs when bootstrapped to !NO_CPU

[Ramon Fried]

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>

Re: [PATCH v1 1/6] net: mv88e61xx: Add support for checking addressing mode

[Lukasz Majewski]

[-- Attachment #1: Type: text/plain, Size: 3914 bytes --]

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

[-- Attachment #2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 488 bytes --]

Re: [PATCH v1 3/6] net: mv88e61xx: Clear temporary structs before using them in get_phy_id()

[Marek Vasut]

On 6/1/23 12:00, Lukasz Majewski 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>
> Reviewed-by: Ramon Fried <rfried.dev@gmail.com>
> ---
> 
>   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 4aee83551beb..c19c3dfa8b6d 100644
> --- a/drivers/net/phy/mv88e61xx.c
> +++ b/drivers/net/phy/mv88e61xx.c
> @@ -1213,6 +1213,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));

struct mii_dev temp_mii = { 0 }; etc

should work all the same, no need for memset.

[PATCH v1 3/6] net: mv88e61xx: Clear temporary structs before using them in get_phy_id()

[Lukasz Majewski]

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

Signed-off-by: Lukasz Majewski <lukma@denx.de>
Reviewed-by: Ramon Fried <rfried.dev@gmail.com>
---

 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 4aee83551beb..c19c3dfa8b6d 100644
--- a/drivers/net/phy/mv88e61xx.c
+++ b/drivers/net/phy/mv88e61xx.c
@@ -1213,6 +1213,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.30.2