[PATCH net-next 0/5] External MDIO support for mv88e6xxx

[PATCH net-next 0/5] External MDIO support for mv88e6xxx

[Andrew Lunn]

The mv88e6390 family of switches has two MDIO busses, one internal to
the switch and a second one for external usage. Older generations of switches
have a single MDIO bus, which is used both internally and externally.

Refactor the existing MDIO driver code to allow for multiple MDIO
busses, and implement the second MDIO bus on mv88e6390.

This is a rewrite of a patch previously submitted as part of "Batch
3". It has been broken up into 5 smaller patches. A compatible string
is now used in the device tree to indicate the external MDIO bus.

Andrew Lunn (5):
  net: dsa: mv88e6xxx: Abstract mv88e6165 PHY operations
  net: dsa: mv88e6xxx: Pass mii_bus to all PHY operations
  net: dsa: mv88e6xxx: Add mdio private structure
  net: dsa: mv88e6xxx: Support multiple MDIO busses
  net: dsa: mv88e6xxx: Implement the 6390 external MDIO bus

 .../devicetree/bindings/net/dsa/marvell.txt        |  41 ++++-
 drivers/net/dsa/mv88e6xxx/chip.c                   | 169 ++++++++++++++++-----
 drivers/net/dsa/mv88e6xxx/global2.c                |  18 ++-
 drivers/net/dsa/mv88e6xxx/global2.h                |  12 +-
 drivers/net/dsa/mv88e6xxx/mv88e6xxx.h              |  25 +--
 5 files changed, 206 insertions(+), 59 deletions(-)

-- 
2.11.0

[PATCH net-next 1/5] net: dsa: mv88e6xxx: Abstract mv88e6165 PHY operations

[Andrew Lunn]

The mv88e6165 family has the internal PHYs mapped directly onto the
SMI register space as the switch. So the registers can be read
directly. Put a wrapper around this, in preparation for changing the
signature in order to support the external MDIO bus of the 6390.

Signed-off-by: Andrew Lunn <andrew@lunn.ch>
---
 drivers/net/dsa/mv88e6xxx/chip.c | 24 ++++++++++++++++++------
 1 file changed, 18 insertions(+), 6 deletions(-)

diff --git a/drivers/net/dsa/mv88e6xxx/chip.c b/drivers/net/dsa/mv88e6xxx/chip.c
index c7e08e13bb54..374d887e8256 100644
--- a/drivers/net/dsa/mv88e6xxx/chip.c
+++ b/drivers/net/dsa/mv88e6xxx/chip.c
@@ -222,6 +222,18 @@ int mv88e6xxx_write(struct mv88e6xxx_chip *chip, int addr, int reg, u16 val)
 	return 0;
 }
 
+static int mv88e6165_phy_read(struct mv88e6xxx_chip *chip, int addr,
+			      int reg, u16 *val)
+{
+	return mv88e6xxx_read(chip, addr, reg, val);
+}
+
+static int mv88e6165_phy_write(struct mv88e6xxx_chip *chip, int addr,
+			       int reg, u16 val)
+{
+	return mv88e6xxx_write(chip, addr, reg, val);
+}
+
 static int mv88e6xxx_phy_read(struct mv88e6xxx_chip *chip, int phy,
 			      int reg, u16 *val)
 {
@@ -3085,8 +3097,8 @@ static const struct mv88e6xxx_ops mv88e6097_ops = {
 static const struct mv88e6xxx_ops mv88e6123_ops = {
 	/* MV88E6XXX_FAMILY_6165 */
 	.set_switch_mac = mv88e6xxx_g2_set_switch_mac,
-	.phy_read = mv88e6xxx_read,
-	.phy_write = mv88e6xxx_write,
+	.phy_read = mv88e6165_phy_read,
+	.phy_write = mv88e6165_phy_write,
 	.port_set_link = mv88e6xxx_port_set_link,
 	.port_set_duplex = mv88e6xxx_port_set_duplex,
 	.port_set_speed = mv88e6185_port_set_speed,
@@ -3132,8 +3144,8 @@ static const struct mv88e6xxx_ops mv88e6131_ops = {
 static const struct mv88e6xxx_ops mv88e6161_ops = {
 	/* MV88E6XXX_FAMILY_6165 */
 	.set_switch_mac = mv88e6xxx_g2_set_switch_mac,
-	.phy_read = mv88e6xxx_read,
-	.phy_write = mv88e6xxx_write,
+	.phy_read = mv88e6165_phy_read,
+	.phy_write = mv88e6165_phy_write,
 	.port_set_link = mv88e6xxx_port_set_link,
 	.port_set_duplex = mv88e6xxx_port_set_duplex,
 	.port_set_speed = mv88e6185_port_set_speed,
@@ -3157,8 +3169,8 @@ static const struct mv88e6xxx_ops mv88e6161_ops = {
 static const struct mv88e6xxx_ops mv88e6165_ops = {
 	/* MV88E6XXX_FAMILY_6165 */
 	.set_switch_mac = mv88e6xxx_g2_set_switch_mac,
-	.phy_read = mv88e6xxx_read,
-	.phy_write = mv88e6xxx_write,
+	.phy_read = mv88e6165_phy_read,
+	.phy_write = mv88e6165_phy_write,
 	.port_set_link = mv88e6xxx_port_set_link,
 	.port_set_duplex = mv88e6xxx_port_set_duplex,
 	.port_set_speed = mv88e6185_port_set_speed,
-- 
2.11.0

Re: [PATCH net-next 1/5] net: dsa: mv88e6xxx: Abstract mv88e6165 PHY operations

[Vivien Didelot]

Andrew Lunn <andrew@lunn.ch> writes:

> The mv88e6165 family has the internal PHYs mapped directly onto the
> SMI register space as the switch. So the registers can be read
> directly. Put a wrapper around this, in preparation for changing the
> signature in order to support the external MDIO bus of the 6390.
>
> Signed-off-by: Andrew Lunn <andrew@lunn.ch>

Reviewed-by: Vivien Didelot <vivien.didelot@savoirfairelinux.com>

[PATCH net-next 2/5] net: dsa: mv88e6xxx: Pass mii_bus to all PHY operations

[Andrew Lunn]

In preparation for supporting multiple MDIO busses, pass the mii_bus
structure to all PHY operations. It will in future then be clear on
which MDIO bus the operation should be performed.

For reads/write from phylib, the mii_bus is readily available. However
some internal code also access the PHY, e.g. for EEE and SERDES. Make
this code use the one and only currently available MDIO bus.

Signed-off-by: Andrew Lunn <andrew@lunn.ch>
---
 drivers/net/dsa/mv88e6xxx/chip.c      | 42 +++++++++++++++++++++++++----------
 drivers/net/dsa/mv88e6xxx/global2.c   | 10 +++++----
 drivers/net/dsa/mv88e6xxx/global2.h   | 12 ++++++----
 drivers/net/dsa/mv88e6xxx/mv88e6xxx.h | 10 +++++----
 4 files changed, 50 insertions(+), 24 deletions(-)

diff --git a/drivers/net/dsa/mv88e6xxx/chip.c b/drivers/net/dsa/mv88e6xxx/chip.c
index 374d887e8256..dedccf201452 100644
--- a/drivers/net/dsa/mv88e6xxx/chip.c
+++ b/drivers/net/dsa/mv88e6xxx/chip.c
@@ -222,14 +222,16 @@ int mv88e6xxx_write(struct mv88e6xxx_chip *chip, int addr, int reg, u16 val)
 	return 0;
 }
 
-static int mv88e6165_phy_read(struct mv88e6xxx_chip *chip, int addr,
-			      int reg, u16 *val)
+static int mv88e6165_phy_read(struct mv88e6xxx_chip *chip,
+			      struct mii_bus *bus,
+			      int addr, int reg, u16 *val)
 {
 	return mv88e6xxx_read(chip, addr, reg, val);
 }
 
-static int mv88e6165_phy_write(struct mv88e6xxx_chip *chip, int addr,
-			       int reg, u16 val)
+static int mv88e6165_phy_write(struct mv88e6xxx_chip *chip,
+			       struct mii_bus *bus,
+			       int addr, int reg, u16 val)
 {
 	return mv88e6xxx_write(chip, addr, reg, val);
 }
@@ -238,22 +240,30 @@ static int mv88e6xxx_phy_read(struct mv88e6xxx_chip *chip, int phy,
 			      int reg, u16 *val)
 {
 	int addr = phy; /* PHY devices addresses start at 0x0 */
+	struct mii_bus *bus = chip->mdio_bus;
 
 	if (!chip->info->ops->phy_read)
 		return -EOPNOTSUPP;
 
-	return chip->info->ops->phy_read(chip, addr, reg, val);
+	if (!bus)
+		return -EOPNOTSUPP;
+
+	return chip->info->ops->phy_read(chip, bus, addr, reg, val);
 }
 
 static int mv88e6xxx_phy_write(struct mv88e6xxx_chip *chip, int phy,
 			       int reg, u16 val)
 {
 	int addr = phy; /* PHY devices addresses start at 0x0 */
+	struct mii_bus *bus = chip->mdio_bus;
 
 	if (!chip->info->ops->phy_write)
 		return -EOPNOTSUPP;
 
-	return chip->info->ops->phy_write(chip, addr, reg, val);
+	if (!bus)
+		return -EOPNOTSUPP;
+
+	return chip->info->ops->phy_write(chip, bus, addr, reg, val);
 }
 
 static int mv88e6xxx_phy_page_get(struct mv88e6xxx_chip *chip, int phy, u8 page)
@@ -623,8 +633,9 @@ static void mv88e6xxx_ppu_state_destroy(struct mv88e6xxx_chip *chip)
 	del_timer_sync(&chip->ppu_timer);
 }
 
-static int mv88e6xxx_phy_ppu_read(struct mv88e6xxx_chip *chip, int addr,
-				  int reg, u16 *val)
+static int mv88e6xxx_phy_ppu_read(struct mv88e6xxx_chip *chip,
+				  struct mii_bus *bus,
+				  int addr, int reg, u16 *val)
 {
 	int err;
 
@@ -637,8 +648,9 @@ static int mv88e6xxx_phy_ppu_read(struct mv88e6xxx_chip *chip, int addr,
 	return err;
 }
 
-static int mv88e6xxx_phy_ppu_write(struct mv88e6xxx_chip *chip, int addr,
-				   int reg, u16 val)
+static int mv88e6xxx_phy_ppu_write(struct mv88e6xxx_chip *chip,
+				   struct mii_bus *bus,
+				   int addr, int reg, u16 val)
 {
 	int err;
 
@@ -2897,8 +2909,11 @@ static int mv88e6xxx_mdio_read(struct mii_bus *bus, int phy, int reg)
 	if (phy >= mv88e6xxx_num_ports(chip))
 		return 0xffff;
 
+	if (!chip->info->ops->phy_read)
+		return -EOPNOTSUPP;
+
 	mutex_lock(&chip->reg_lock);
-	err = mv88e6xxx_phy_read(chip, phy, reg, &val);
+	err = chip->info->ops->phy_read(chip, bus, phy, reg, &val);
 	mutex_unlock(&chip->reg_lock);
 
 	return err ? err : val;
@@ -2912,8 +2927,11 @@ static int mv88e6xxx_mdio_write(struct mii_bus *bus, int phy, int reg, u16 val)
 	if (phy >= mv88e6xxx_num_ports(chip))
 		return 0xffff;
 
+	if (!chip->info->ops->phy_write)
+		return -EOPNOTSUPP;
+
 	mutex_lock(&chip->reg_lock);
-	err = mv88e6xxx_phy_write(chip, phy, reg, val);
+	err = chip->info->ops->phy_write(chip, bus, phy, reg, val);
 	mutex_unlock(&chip->reg_lock);
 
 	return err;
diff --git a/drivers/net/dsa/mv88e6xxx/global2.c b/drivers/net/dsa/mv88e6xxx/global2.c
index ead2e265c9ef..1f9a12a1fad9 100644
--- a/drivers/net/dsa/mv88e6xxx/global2.c
+++ b/drivers/net/dsa/mv88e6xxx/global2.c
@@ -501,8 +501,9 @@ static int mv88e6xxx_g2_smi_phy_cmd(struct mv88e6xxx_chip *chip, u16 cmd)
 	return mv88e6xxx_g2_smi_phy_wait(chip);
 }
 
-int mv88e6xxx_g2_smi_phy_read(struct mv88e6xxx_chip *chip, int addr, int reg,
-			      u16 *val)
+int mv88e6xxx_g2_smi_phy_read(struct mv88e6xxx_chip *chip,
+			      struct mii_bus *bus,
+			      int addr, int reg, u16 *val)
 {
 	u16 cmd = GLOBAL2_SMI_PHY_CMD_OP_22_READ_DATA | (addr << 5) | reg;
 	int err;
@@ -518,8 +519,9 @@ int mv88e6xxx_g2_smi_phy_read(struct mv88e6xxx_chip *chip, int addr, int reg,
 	return mv88e6xxx_g2_read(chip, GLOBAL2_SMI_PHY_DATA, val);
 }
 
-int mv88e6xxx_g2_smi_phy_write(struct mv88e6xxx_chip *chip, int addr, int reg,
-			       u16 val)
+int mv88e6xxx_g2_smi_phy_write(struct mv88e6xxx_chip *chip,
+			       struct mii_bus *bus,
+			       int addr, int reg, u16 val)
 {
 	u16 cmd = GLOBAL2_SMI_PHY_CMD_OP_22_WRITE_DATA | (addr << 5) | reg;
 	int err;
diff --git a/drivers/net/dsa/mv88e6xxx/global2.h b/drivers/net/dsa/mv88e6xxx/global2.h
index 2918f22388f7..00e635279ba1 100644
--- a/drivers/net/dsa/mv88e6xxx/global2.h
+++ b/drivers/net/dsa/mv88e6xxx/global2.h
@@ -23,10 +23,12 @@ static inline int mv88e6xxx_g2_require(struct mv88e6xxx_chip *chip)
 	return 0;
 }
 
-int mv88e6xxx_g2_smi_phy_read(struct mv88e6xxx_chip *chip, int addr, int reg,
-			      u16 *val);
-int mv88e6xxx_g2_smi_phy_write(struct mv88e6xxx_chip *chip, int addr, int reg,
-			       u16 val);
+int mv88e6xxx_g2_smi_phy_read(struct mv88e6xxx_chip *chip,
+			      struct mii_bus *bus,
+			      int addr, int reg, u16 *val);
+int mv88e6xxx_g2_smi_phy_write(struct mv88e6xxx_chip *chip,
+			       struct mii_bus *bus,
+			       int addr, int reg, u16 val);
 int mv88e6xxx_g2_set_switch_mac(struct mv88e6xxx_chip *chip, u8 *addr);
 
 int mv88e6xxx_g2_get_eeprom8(struct mv88e6xxx_chip *chip,
@@ -57,12 +59,14 @@ static inline int mv88e6xxx_g2_require(struct mv88e6xxx_chip *chip)
 }
 
 static inline int mv88e6xxx_g2_smi_phy_read(struct mv88e6xxx_chip *chip,
+					    struct mii_bus *bus,
 					    int addr, int reg, u16 *val)
 {
 	return -EOPNOTSUPP;
 }
 
 static inline int mv88e6xxx_g2_smi_phy_write(struct mv88e6xxx_chip *chip,
+					     struct mii_bus *bus,
 					     int addr, int reg, u16 val)
 {
 	return -EOPNOTSUPP;
diff --git a/drivers/net/dsa/mv88e6xxx/mv88e6xxx.h b/drivers/net/dsa/mv88e6xxx/mv88e6xxx.h
index ce8b43b14e96..7d75dd546bf7 100644
--- a/drivers/net/dsa/mv88e6xxx/mv88e6xxx.h
+++ b/drivers/net/dsa/mv88e6xxx/mv88e6xxx.h
@@ -758,10 +758,12 @@ struct mv88e6xxx_ops {
 
 	int (*set_switch_mac)(struct mv88e6xxx_chip *chip, u8 *addr);
 
-	int (*phy_read)(struct mv88e6xxx_chip *chip, int addr, int reg,
-			u16 *val);
-	int (*phy_write)(struct mv88e6xxx_chip *chip, int addr, int reg,
-			 u16 val);
+	int (*phy_read)(struct mv88e6xxx_chip *chip,
+			struct mii_bus *bus,
+			int addr, int reg, u16 *val);
+	int (*phy_write)(struct mv88e6xxx_chip *chip,
+			 struct mii_bus *bus,
+			 int addr, int reg, u16 val);
 
 	/* PHY Polling Unit (PPU) operations */
 	int (*ppu_enable)(struct mv88e6xxx_chip *chip);
-- 
2.11.0

[PATCH net-next 3/5] net: dsa: mv88e6xxx: Add mdio private structure

[Andrew Lunn]

Have the MDIO bus driver code allocate a private structure and make
the chip a member of it. This will allow us to add further members in
the future.

Signed-off-by: Andrew Lunn <andrew@lunn.ch>
---
 drivers/net/dsa/mv88e6xxx/chip.c      | 13 +++++++++----
 drivers/net/dsa/mv88e6xxx/mv88e6xxx.h |  4 ++++
 2 files changed, 13 insertions(+), 4 deletions(-)

diff --git a/drivers/net/dsa/mv88e6xxx/chip.c b/drivers/net/dsa/mv88e6xxx/chip.c
index dedccf201452..b0fd1432f4f3 100644
--- a/drivers/net/dsa/mv88e6xxx/chip.c
+++ b/drivers/net/dsa/mv88e6xxx/chip.c
@@ -2902,7 +2902,8 @@ static int mv88e6xxx_set_addr(struct dsa_switch *ds, u8 *addr)
 
 static int mv88e6xxx_mdio_read(struct mii_bus *bus, int phy, int reg)
 {
-	struct mv88e6xxx_chip *chip = bus->priv;
+	struct mv88e6xxx_mdio_bus *mdio_bus = bus->priv;
+	struct mv88e6xxx_chip *chip = mdio_bus->chip;
 	u16 val;
 	int err;
 
@@ -2921,7 +2922,8 @@ static int mv88e6xxx_mdio_read(struct mii_bus *bus, int phy, int reg)
 
 static int mv88e6xxx_mdio_write(struct mii_bus *bus, int phy, int reg, u16 val)
 {
-	struct mv88e6xxx_chip *chip = bus->priv;
+	struct mv88e6xxx_mdio_bus *mdio_bus = bus->priv;
+	struct mv88e6xxx_chip *chip = mdio_bus->chip;
 	int err;
 
 	if (phy >= mv88e6xxx_num_ports(chip))
@@ -2941,17 +2943,20 @@ static int mv88e6xxx_mdio_register(struct mv88e6xxx_chip *chip,
 				   struct device_node *np)
 {
 	static int index;
+	struct mv88e6xxx_mdio_bus *mdio_bus;
 	struct mii_bus *bus;
 	int err;
 
 	if (np)
 		chip->mdio_np = of_get_child_by_name(np, "mdio");
 
-	bus = devm_mdiobus_alloc(chip->dev);
+	bus = devm_mdiobus_alloc_size(chip->dev, sizeof(*mdio_bus));
 	if (!bus)
 		return -ENOMEM;
 
-	bus->priv = (void *)chip;
+	mdio_bus = bus->priv;
+	mdio_bus->chip = chip;
+
 	if (np) {
 		bus->name = np->full_name;
 		snprintf(bus->id, MII_BUS_ID_SIZE, "%s", np->full_name);
diff --git a/drivers/net/dsa/mv88e6xxx/mv88e6xxx.h b/drivers/net/dsa/mv88e6xxx/mv88e6xxx.h
index 7d75dd546bf7..6f7ddb594809 100644
--- a/drivers/net/dsa/mv88e6xxx/mv88e6xxx.h
+++ b/drivers/net/dsa/mv88e6xxx/mv88e6xxx.h
@@ -750,6 +750,10 @@ struct mv88e6xxx_bus_ops {
 	int (*write)(struct mv88e6xxx_chip *chip, int addr, int reg, u16 val);
 };
 
+struct mv88e6xxx_mdio_bus {
+	struct mv88e6xxx_chip *chip;
+};
+
 struct mv88e6xxx_ops {
 	int (*get_eeprom)(struct mv88e6xxx_chip *chip,
 			  struct ethtool_eeprom *eeprom, u8 *data);
-- 
2.11.0

[PATCH net-next 4/5] net: dsa: mv88e6xxx: Support multiple MDIO busses

[Andrew Lunn]

The mv88e6390 has multiple MDIO busses. Generalize the parsing of the
device tree to support multiple mdio nodes. The external mdio bus has
a compatible strings to indicate it is external.

Keep a linked list of busses, placing the external mdio bus at the
tail of the list. When within the driver an mdio bus is needed,
e.g. for EEE or SERDES, use the head of the list which should be the
internal bus.

Signed-off-by: Andrew Lunn <andrew@lunn.ch>
---
 .../devicetree/bindings/net/dsa/marvell.txt        |  41 +++++++-
 drivers/net/dsa/mv88e6xxx/chip.c                   | 110 +++++++++++++++------
 drivers/net/dsa/mv88e6xxx/mv88e6xxx.h              |  10 +-
 3 files changed, 126 insertions(+), 35 deletions(-)

diff --git a/Documentation/devicetree/bindings/net/dsa/marvell.txt b/Documentation/devicetree/bindings/net/dsa/marvell.txt
index b3dd6b40e0de..86a05563c8ae 100644
--- a/Documentation/devicetree/bindings/net/dsa/marvell.txt
+++ b/Documentation/devicetree/bindings/net/dsa/marvell.txt
@@ -26,8 +26,12 @@ Optional properties:
 - interrupt-controller	: Indicates the switch is itself an interrupt
 			  controller. This is used for the PHY interrupts.
 #interrupt-cells = <2>	: Controller uses two cells, number and flag
-- mdio			: container of PHY and devices on the switches MDIO
-			  bus
+- mdio			: Container of PHY and devices on the switches MDIO
+			  bus.
+- mdio?			: Container of PHYs and devices on the external MDIO
+			  bus. The node must contains a compatible string of
+			  "marvell,mv88e6xxx-mdio-external"
+
 Example:
 
        mdio {
@@ -53,3 +57,36 @@ Example:
                        };
                };
        };
+
+       mdio {
+               #address-cells = <1>;
+               #size-cells = <0>;
+               interrupt-parent = <&gpio0>;
+               interrupts = <27 IRQ_TYPE_LEVEL_LOW>;
+               interrupt-controller;
+               #interrupt-cells = <2>;
+
+               switch0: switch@0 {
+                       compatible = "marvell,mv88e6390";
+                       reg = <0>;
+		       reset-gpios = <&gpio5 1 GPIO_ACTIVE_LOW>;
+               };
+               mdio {
+                       #address-cells = <1>;
+                       #size-cells = <0>;
+                       switch1phy0: switch1phy0@0 {
+                               reg = <0>;
+                               interrupt-parent = <&switch0>;
+                               interrupts = <0 IRQ_TYPE_LEVEL_HIGH>;
+                       };
+               };
+
+               mdio1 {
+	       	       compatible = "marvell,mv88e6xxx-mdio-external";
+                       #address-cells = <1>;
+                       #size-cells = <0>;
+                       switch1phy9: switch1phy0@9 {
+                               reg = <9>;
+                       };
+               };
+       };
diff --git a/drivers/net/dsa/mv88e6xxx/chip.c b/drivers/net/dsa/mv88e6xxx/chip.c
index b0fd1432f4f3..5668e778ed1d 100644
--- a/drivers/net/dsa/mv88e6xxx/chip.c
+++ b/drivers/net/dsa/mv88e6xxx/chip.c
@@ -236,16 +236,29 @@ static int mv88e6165_phy_write(struct mv88e6xxx_chip *chip,
 	return mv88e6xxx_write(chip, addr, reg, val);
 }
 
+static struct mii_bus *mv88e6xxx_default_mdio_bus(struct mv88e6xxx_chip *chip)
+{
+	struct mv88e6xxx_mdio_bus *mdio_bus;
+
+	mdio_bus = list_first_entry(&chip->mdios, struct mv88e6xxx_mdio_bus,
+				    list);
+	if (!mdio_bus)
+		return NULL;
+
+	return mdio_bus->bus;
+}
+
 static int mv88e6xxx_phy_read(struct mv88e6xxx_chip *chip, int phy,
 			      int reg, u16 *val)
 {
 	int addr = phy; /* PHY devices addresses start at 0x0 */
-	struct mii_bus *bus = chip->mdio_bus;
+	struct mii_bus *bus;
 
-	if (!chip->info->ops->phy_read)
+	bus = mv88e6xxx_default_mdio_bus(chip);
+	if (!bus)
 		return -EOPNOTSUPP;
 
-	if (!bus)
+	if (!chip->info->ops->phy_read)
 		return -EOPNOTSUPP;
 
 	return chip->info->ops->phy_read(chip, bus, addr, reg, val);
@@ -255,12 +268,13 @@ static int mv88e6xxx_phy_write(struct mv88e6xxx_chip *chip, int phy,
 			       int reg, u16 val)
 {
 	int addr = phy; /* PHY devices addresses start at 0x0 */
-	struct mii_bus *bus = chip->mdio_bus;
+	struct mii_bus *bus;
 
-	if (!chip->info->ops->phy_write)
+	bus = mv88e6xxx_default_mdio_bus(chip);
+	if (!bus)
 		return -EOPNOTSUPP;
 
-	if (!bus)
+	if (!chip->info->ops->phy_write)
 		return -EOPNOTSUPP;
 
 	return chip->info->ops->phy_write(chip, bus, addr, reg, val);
@@ -2845,7 +2859,7 @@ static int mv88e6xxx_setup(struct dsa_switch *ds)
 	int i;
 
 	chip->ds = ds;
-	ds->slave_mii_bus = chip->mdio_bus;
+	ds->slave_mii_bus = mv88e6xxx_default_mdio_bus(chip);
 
 	mutex_lock(&chip->reg_lock);
 
@@ -2940,22 +2954,23 @@ static int mv88e6xxx_mdio_write(struct mii_bus *bus, int phy, int reg, u16 val)
 }
 
 static int mv88e6xxx_mdio_register(struct mv88e6xxx_chip *chip,
-				   struct device_node *np)
+				   struct device_node *np,
+				   bool external)
 {
 	static int index;
 	struct mv88e6xxx_mdio_bus *mdio_bus;
 	struct mii_bus *bus;
 	int err;
 
-	if (np)
-		chip->mdio_np = of_get_child_by_name(np, "mdio");
-
 	bus = devm_mdiobus_alloc_size(chip->dev, sizeof(*mdio_bus));
 	if (!bus)
 		return -ENOMEM;
 
 	mdio_bus = bus->priv;
+	mdio_bus->bus = bus;
 	mdio_bus->chip = chip;
+	INIT_LIST_HEAD(&mdio_bus->list);
+	mdio_bus->external = external;
 
 	if (np) {
 		bus->name = np->full_name;
@@ -2969,34 +2984,72 @@ static int mv88e6xxx_mdio_register(struct mv88e6xxx_chip *chip,
 	bus->write = mv88e6xxx_mdio_write;
 	bus->parent = chip->dev;
 
-	if (chip->mdio_np)
-		err = of_mdiobus_register(bus, chip->mdio_np);
+	if (np)
+		err = of_mdiobus_register(bus, np);
 	else
 		err = mdiobus_register(bus);
 	if (err) {
 		dev_err(chip->dev, "Cannot register MDIO bus (%d)\n", err);
-		goto out;
+		return err;
 	}
-	chip->mdio_bus = bus;
+
+	if (external)
+		list_add_tail(&mdio_bus->list, &chip->mdios);
+	else
+		list_add(&mdio_bus->list, &chip->mdios);
 
 	return 0;
+}
 
-out:
-	if (chip->mdio_np)
-		of_node_put(chip->mdio_np);
+static const struct of_device_id mv88e6xxx_mdio_external_match[] = {
+	{ .compatible = "marvell,mv88e6xxx-mdio-external",
+	  .data = (void *)true },
+	{ },
+};
 
-	return err;
+static int mv88e6xxx_mdios_register(struct mv88e6xxx_chip *chip,
+				    struct device_node *np)
+{
+	const struct of_device_id *match;
+	struct device_node *child;
+	int err;
+
+	/* Always register one mdio bus for the internal/default mdio
+	 * bus. This maybe represented in the device tree, but is
+	 * optional.
+	 */
+	child = of_get_child_by_name(np, "mdio");
+	err = mv88e6xxx_mdio_register(chip, child, false);
+	if (err)
+		return err;
+
+	/* Walk the device tree, and see if there are any other nodes
+	 * which say they are compatible with the external mdio
+	 * bus.
+	 */
+	for_each_available_child_of_node(np, child) {
+		match = of_match_node(mv88e6xxx_mdio_external_match, child);
+		if (match) {
+			err = mv88e6xxx_mdio_register(chip, child, true);
+			if (err)
+				return err;
+		}
+	}
+
+	return 0;
 }
 
-static void mv88e6xxx_mdio_unregister(struct mv88e6xxx_chip *chip)
+static void mv88e6xxx_mdios_unregister(struct mv88e6xxx_chip *chip)
 
 {
-	struct mii_bus *bus = chip->mdio_bus;
+	struct mv88e6xxx_mdio_bus *mdio_bus;
+	struct mii_bus *bus;
 
-	mdiobus_unregister(bus);
+	list_for_each_entry(mdio_bus, &chip->mdios, list) {
+		bus = mdio_bus->bus;
 
-	if (chip->mdio_np)
-		of_node_put(chip->mdio_np);
+		mdiobus_unregister(bus);
+	}
 }
 
 static int mv88e6xxx_get_eeprom_len(struct dsa_switch *ds)
@@ -4123,6 +4176,7 @@ static struct mv88e6xxx_chip *mv88e6xxx_alloc_chip(struct device *dev)
 	chip->dev = dev;
 
 	mutex_init(&chip->reg_lock);
+	INIT_LIST_HEAD(&chip->mdios);
 
 	return chip;
 }
@@ -4197,7 +4251,7 @@ static const char *mv88e6xxx_drv_probe(struct device *dsa_dev,
 
 	mv88e6xxx_phy_init(chip);
 
-	err = mv88e6xxx_mdio_register(chip, NULL);
+	err = mv88e6xxx_mdios_register(chip, NULL);
 	if (err)
 		goto free;
 
@@ -4398,7 +4452,7 @@ static int mv88e6xxx_probe(struct mdio_device *mdiodev)
 		}
 	}
 
-	err = mv88e6xxx_mdio_register(chip, np);
+	err = mv88e6xxx_mdios_register(chip, np);
 	if (err)
 		goto out_g2_irq;
 
@@ -4409,7 +4463,7 @@ static int mv88e6xxx_probe(struct mdio_device *mdiodev)
 	return 0;
 
 out_mdio:
-	mv88e6xxx_mdio_unregister(chip);
+	mv88e6xxx_mdios_unregister(chip);
 out_g2_irq:
 	if (mv88e6xxx_has(chip, MV88E6XXX_FLAG_G2_INT) && chip->irq > 0)
 		mv88e6xxx_g2_irq_free(chip);
@@ -4430,7 +4484,7 @@ static void mv88e6xxx_remove(struct mdio_device *mdiodev)
 
 	mv88e6xxx_phy_destroy(chip);
 	mv88e6xxx_unregister_switch(chip);
-	mv88e6xxx_mdio_unregister(chip);
+	mv88e6xxx_mdios_unregister(chip);
 
 	if (chip->irq > 0) {
 		if (mv88e6xxx_has(chip, MV88E6XXX_FLAG_G2_INT))
diff --git a/drivers/net/dsa/mv88e6xxx/mv88e6xxx.h b/drivers/net/dsa/mv88e6xxx/mv88e6xxx.h
index 6f7ddb594809..7d24add45e74 100644
--- a/drivers/net/dsa/mv88e6xxx/mv88e6xxx.h
+++ b/drivers/net/dsa/mv88e6xxx/mv88e6xxx.h
@@ -730,11 +730,8 @@ struct mv88e6xxx_chip {
 	/* set to size of eeprom if supported by the switch */
 	int		eeprom_len;
 
-	/* Device node for the MDIO bus */
-	struct device_node *mdio_np;
-
-	/* And the MDIO bus itself */
-	struct mii_bus *mdio_bus;
+	/* List of mdio busses */
+	struct list_head mdios;
 
 	/* There can be two interrupt controllers, which are chained
 	 * off a GPIO as interrupt source
@@ -751,7 +748,10 @@ struct mv88e6xxx_bus_ops {
 };
 
 struct mv88e6xxx_mdio_bus {
+	struct mii_bus *bus;
 	struct mv88e6xxx_chip *chip;
+	struct list_head list;
+	bool external;
 };
 
 struct mv88e6xxx_ops {
-- 
2.11.0

[PATCH net-next 5/5] net: dsa: mv88e6xxx: Implement the 6390 external MDIO bus

[Andrew Lunn]

With all the infrastructure in place, implement access to the external
MDIO bus on the 6390 family.

Signed-off-by: Andrew Lunn <andrew@lunn.ch>
---
 drivers/net/dsa/mv88e6xxx/global2.c   | 8 ++++++++
 drivers/net/dsa/mv88e6xxx/mv88e6xxx.h | 1 +
 2 files changed, 9 insertions(+)

diff --git a/drivers/net/dsa/mv88e6xxx/global2.c b/drivers/net/dsa/mv88e6xxx/global2.c
index 1f9a12a1fad9..353e26bea3c3 100644
--- a/drivers/net/dsa/mv88e6xxx/global2.c
+++ b/drivers/net/dsa/mv88e6xxx/global2.c
@@ -506,8 +506,12 @@ int mv88e6xxx_g2_smi_phy_read(struct mv88e6xxx_chip *chip,
 			      int addr, int reg, u16 *val)
 {
 	u16 cmd = GLOBAL2_SMI_PHY_CMD_OP_22_READ_DATA | (addr << 5) | reg;
+	struct mv88e6xxx_mdio_bus *mdio_bus = bus->priv;
 	int err;
 
+	if (mdio_bus->external)
+		cmd |= GLOBAL2_SMI_PHY_CMD_EXTERNAL;
+
 	err = mv88e6xxx_g2_smi_phy_wait(chip);
 	if (err)
 		return err;
@@ -524,8 +528,12 @@ int mv88e6xxx_g2_smi_phy_write(struct mv88e6xxx_chip *chip,
 			       int addr, int reg, u16 val)
 {
 	u16 cmd = GLOBAL2_SMI_PHY_CMD_OP_22_WRITE_DATA | (addr << 5) | reg;
+	struct mv88e6xxx_mdio_bus *mdio_bus = bus->priv;
 	int err;
 
+	if (mdio_bus->external)
+		cmd |= GLOBAL2_SMI_PHY_CMD_EXTERNAL;
+
 	err = mv88e6xxx_g2_smi_phy_wait(chip);
 	if (err)
 		return err;
diff --git a/drivers/net/dsa/mv88e6xxx/mv88e6xxx.h b/drivers/net/dsa/mv88e6xxx/mv88e6xxx.h
index 7d24add45e74..572d585dc1e2 100644
--- a/drivers/net/dsa/mv88e6xxx/mv88e6xxx.h
+++ b/drivers/net/dsa/mv88e6xxx/mv88e6xxx.h
@@ -387,6 +387,7 @@
 #define GLOBAL2_PTP_AVB_DATA	0x17
 #define GLOBAL2_SMI_PHY_CMD			0x18
 #define GLOBAL2_SMI_PHY_CMD_BUSY		BIT(15)
+#define GLOBAL2_SMI_PHY_CMD_EXTERNAL		BIT(13)
 #define GLOBAL2_SMI_PHY_CMD_MODE_22		BIT(12)
 #define GLOBAL2_SMI_PHY_CMD_OP_22_WRITE_DATA	((0x1 << 10) | \
 						 GLOBAL2_SMI_PHY_CMD_MODE_22 | \
-- 
2.11.0

Re: [PATCH net-next 5/5] net: dsa: mv88e6xxx: Implement the 6390 external MDIO bus

[Vivien Didelot]

Hi Andrew,

Sorry but the previous patches with a list of MDIO busses and refactored
PHY ops still seems too much complex to just toggle a bit.

We know which switch port has an external PHY attached to it, right?
So why not just something like this:

    struct mv88e6xxx_chip {
        ...
        u16 external_phys;
        ...
    }

    int mv88e6xxx_g2_smi_phy_read(struct mv88e6xxx_chip *chip, int phy,
                                  int reg, u16 *val)
    {
        u16 cmd = GLOBAL2_SMI_PHY_CMD_OP_22_READ_DATA | (addr << 5) | reg;
        int err;

        if (chip->external_phys & BIT(phy))
            cmd |= GLOBAL2_SMI_PHY_CMD_EXTERNAL;

        ...
    }


Thanks!

        Vivien

Re: [PATCH net-next 5/5] net: dsa: mv88e6xxx: Implement the 6390 external MDIO bus

[Andrew Lunn]

On Tue, Jan 24, 2017 at 12:48:02PM -0500, Vivien Didelot wrote:
> Hi Andrew,
> 
> Sorry but the previous patches with a list of MDIO busses and refactored
> PHY ops still seems too much complex to just toggle a bit.
> 
> We know which switch port has an external PHY attached to it, right?

No, not really. We have the full flexibility of phandles in the device
tree. The PHYs on the external bus might not even be connected to the
switch, they could be for the host interfaces, for example. And you
don't even need to use the internal PHYs. If you are using the SERDES
interfaces, you could have an external SGMII PHY connect to a
port. This would make sense with SFP cages, with a copper PHY inserted
into the cage. All ports of the 6390 allow this.

The hardware has two really separate MDIO busses. Both busses could
have a PHY on the same address. We should really model this as two
Linux MDIO devices.

It is actually a shame older chips don't have this. Early versions of
the ClearFog have a hardware design error. The external PHY on port6
is using address 0. So it clashes with the internal PHY on port0. Two
MDIO busses would solve this. And i actually expect quite a few
designs using the 6390 will put an external phy at address 0 and 1,
not 9 and 10. Few PHYs have that many strapping pins for address
selection.

     Andrew

Re: [PATCH net-next 5/5] net: dsa: mv88e6xxx: Implement the 6390 external MDIO bus

[Andrew Lunn]

> diff --git a/drivers/net/dsa/mv88e6xxx/mv88e6xxx.h b/drivers/net/dsa/mv88e6xxx/mv88e6xxx.h
> index 7d24add45e74..572d585dc1e2 100644
> --- a/drivers/net/dsa/mv88e6xxx/mv88e6xxx.h
> +++ b/drivers/net/dsa/mv88e6xxx/mv88e6xxx.h
> @@ -387,6 +387,7 @@
>  #define GLOBAL2_PTP_AVB_DATA	0x17
>  #define GLOBAL2_SMI_PHY_CMD			0x18
>  #define GLOBAL2_SMI_PHY_CMD_BUSY		BIT(15)
> +#define GLOBAL2_SMI_PHY_CMD_EXTERNAL		BIT(13)
>  #define GLOBAL2_SMI_PHY_CMD_MODE_22		BIT(12)
>  #define GLOBAL2_SMI_PHY_CMD_OP_22_WRITE_DATA	((0x1 << 10) | \

Hi Gregory

Please could you check if the 88E6341 has an external MDIO. Global 2,
register 0x18, bit 13.

Thanks
	Andrew

Re: [PATCH net-next 5/5] net: dsa: mv88e6xxx: Implement the 6390 external MDIO bus

[Gregory CLEMENT]

Hi Andrew,
 
 On mer., janv. 25 2017, Andrew Lunn <andrew@lunn.ch> wrote:

>> diff --git a/drivers/net/dsa/mv88e6xxx/mv88e6xxx.h b/drivers/net/dsa/mv88e6xxx/mv88e6xxx.h
>> index 7d24add45e74..572d585dc1e2 100644
>> --- a/drivers/net/dsa/mv88e6xxx/mv88e6xxx.h
>> +++ b/drivers/net/dsa/mv88e6xxx/mv88e6xxx.h
>> @@ -387,6 +387,7 @@
>>  #define GLOBAL2_PTP_AVB_DATA	0x17
>>  #define GLOBAL2_SMI_PHY_CMD			0x18
>>  #define GLOBAL2_SMI_PHY_CMD_BUSY		BIT(15)
>> +#define GLOBAL2_SMI_PHY_CMD_EXTERNAL		BIT(13)
>>  #define GLOBAL2_SMI_PHY_CMD_MODE_22		BIT(12)
>>  #define GLOBAL2_SMI_PHY_CMD_OP_22_WRITE_DATA	((0x1 << 10) | \
>
> Hi Gregory
>
> Please could you check if the 88E6341 has an external MDIO. Global 2,
> register 0x18, bit 13.

I confirm that 88E6341 has Global 2, register 0x18, bit 13 referred as
"External access"

Gregory

>
> Thanks
> 	Andrew

-- 
Gregory Clement, Free Electrons
Kernel, drivers, real-time and embedded Linux
development, consulting, training and support.
http://free-electrons.com

Re: [PATCH net-next 0/5] External MDIO support for mv88e6xxx

[David Miller]

From: Andrew Lunn <andrew@lunn.ch>
Date: Tue, 24 Jan 2017 14:53:46 +0100

> The mv88e6390 family of switches has two MDIO busses, one internal to
> the switch and a second one for external usage. Older generations of switches
> have a single MDIO bus, which is used both internally and externally.
> 
> Refactor the existing MDIO driver code to allow for multiple MDIO
> busses, and implement the second MDIO bus on mv88e6390.
> 
> This is a rewrite of a patch previously submitted as part of "Batch
> 3". It has been broken up into 5 smaller patches. A compatible string
> is now used in the device tree to indicate the external MDIO bus.

Series applied, thanks Andrew.

There seems to be a bit of inconsistency wrt. tab vs. space for
indentation in the binding files, just FYI.  I had to make some small
adjustments to this patch series in order to keep GIT from warning
when I applied this stuff.

Re: [PATCH net-next 0/5] External MDIO support for mv88e6xxx

[Vivien Didelot]

Hi David,

David Miller <davem@davemloft.net> writes:

> Series applied, thanks Andrew.
>
> There seems to be a bit of inconsistency wrt. tab vs. space for
> indentation in the binding files, just FYI.  I had to make some small
> adjustments to this patch series in order to keep GIT from warning
> when I applied this stuff.

Andrew just replied to my comment an hour ago, and I still have concerns
about the patchset. I didn't like the list_head of busses, and the 6352
family don't support the external mode. That needed to be addressed.

Why such an hurry to merge patches?

    Vivien

Re: [PATCH net-next 0/5] External MDIO support for mv88e6xxx

[David Miller]

From: Vivien Didelot <vivien.didelot@savoirfairelinux.com>
Date: Tue, 24 Jan 2017 16:05:08 -0500

> Hi David,
> 
> David Miller <davem@davemloft.net> writes:
> 
>> Series applied, thanks Andrew.
>>
>> There seems to be a bit of inconsistency wrt. tab vs. space for
>> indentation in the binding files, just FYI.  I had to make some small
>> adjustments to this patch series in order to keep GIT from warning
>> when I applied this stuff.
> 
> Andrew just replied to my comment an hour ago, and I still have concerns
> about the patchset. I didn't like the list_head of busses, and the 6352
> family don't support the external mode. That needed to be addressed.
> 
> Why such an hurry to merge patches?

Because my backlog sucks otherwise?

I can revert, or you can ask Andrew to post follow-on fixups to
satisfy your concerns.  Just because I applied it doesn't make it
the end of the world.

And maybe I should apply patches aggressively, as this lights a fire
under everyone's butt and people review and fix things more quickly.

:-)

Re: [PATCH net-next 0/5] External MDIO support for mv88e6xxx

[David Miller]

From: David Miller <davem@davemloft.net>
Date: Tue, 24 Jan 2017 16:07:50 -0500 (EST)

> From: Vivien Didelot <vivien.didelot@savoirfairelinux.com>
> Date: Tue, 24 Jan 2017 16:05:08 -0500
> 
>> Hi David,
>> 
>> David Miller <davem@davemloft.net> writes:
>> 
>>> Series applied, thanks Andrew.
>>>
>>> There seems to be a bit of inconsistency wrt. tab vs. space for
>>> indentation in the binding files, just FYI.  I had to make some small
>>> adjustments to this patch series in order to keep GIT from warning
>>> when I applied this stuff.
>> 
>> Andrew just replied to my comment an hour ago, and I still have concerns
>> about the patchset. I didn't like the list_head of busses, and the 6352
>> family don't support the external mode. That needed to be addressed.
>> 
>> Why such an hurry to merge patches?
> 
> Because my backlog sucks otherwise?

And if you don't believe it, 18 new patches arrived to netdev while
I was typing this.

Re: [PATCH net-next 0/5] External MDIO support for mv88e6xxx

[Vivien Didelot]

Hi David,

David Miller <davem@davemloft.net> writes:

>> Andrew just replied to my comment an hour ago, and I still have concerns
>> about the patchset. I didn't like the list_head of busses, and the 6352
>> family don't support the external mode. That needed to be addressed.
>> 
>> Why such an hurry to merge patches?
>
> Because my backlog sucks otherwise?
>
> I can revert, or you can ask Andrew to post follow-on fixups to
> satisfy your concerns.  Just because I applied it doesn't make it
> the end of the world.
>
> And maybe I should apply patches aggressively, as this lights a fire
> under everyone's butt and people review and fix things more quickly.

That has exactly the opposite effect. This makes people care less about
each other work because "it's gonna be merged in a few minutes anyway."

:-)

Re: [PATCH net-next 0/5] External MDIO support for mv88e6xxx

[David Miller]

From: Vivien Didelot <vivien.didelot@savoirfairelinux.com>
Date: Tue, 24 Jan 2017 16:24:55 -0500

> David Miller <davem@davemloft.net> writes:
> 
>> And maybe I should apply patches aggressively, as this lights a fire
>> under everyone's butt and people review and fix things more quickly.
> 
> That has exactly the opposite effect. This makes people care less about
> each other work because "it's gonna be merged in a few minutes anyway."

Your very actions here disagree with this hypothesis.  And, watch how
quickly Andrew will follow up.

:-)