[PATCH net v2 0/2] mv88e6xxx fixed link fixes

[PATCH net v2 0/2] mv88e6xxx fixed link fixes

[Andrew Lunn]

Recent changes for how the MAC is configured broke fixed links, as
used by CPU/DSA ports, and for SFPs when phylink cannot be used. The
first fix is unchanged from v1. The second fix takes a different
solution than v1. If a CPU or DSA port is known to have a PHYLINK
instance, configure the port down before instantiating the PHYLINK, so
it is in the down state as expected by PHYLINK.

Andrew Lunn (2):
  net: dsa: mv88e6xxx: Configure MAC when using fixed link
  net: dsa: Down cpu/dsa ports phylink will control

 drivers/net/dsa/mv88e6xxx/chip.c | 5 +++--
 net/dsa/port.c                   | 7 ++++++-
 2 files changed, 9 insertions(+), 3 deletions(-)

-- 
2.26.0

[PATCH net v2 1/2] net: dsa: mv88e6xxx: Configure MAC when using fixed link

[Andrew Lunn]

The 88e6185 is reporting it has detected a PHY, when a port is
connected to an SFP. As a result, the fixed-phy configuration is not
being applied. That then breaks packet transfer, since the port is
reported as being down.

Add additional conditions to check the interface mode, and if it is
fixed always configure the port on link up/down, independent of the
PPU status.

Fixes: 30c4a5b0aad8 ("net: mv88e6xxx: use resolved link config in mac_link_up()")
Signed-off-by: Andrew Lunn <andrew@lunn.ch>
---
 drivers/net/dsa/mv88e6xxx/chip.c | 5 +++--
 1 file changed, 3 insertions(+), 2 deletions(-)

diff --git a/drivers/net/dsa/mv88e6xxx/chip.c b/drivers/net/dsa/mv88e6xxx/chip.c
index 221593261e8f..dd8a5666a584 100644
--- a/drivers/net/dsa/mv88e6xxx/chip.c
+++ b/drivers/net/dsa/mv88e6xxx/chip.c
@@ -709,7 +709,8 @@ static void mv88e6xxx_mac_link_down(struct dsa_switch *ds, int port,
 	ops = chip->info->ops;
 
 	mv88e6xxx_reg_lock(chip);
-	if (!mv88e6xxx_port_ppu_updates(chip, port) && ops->port_set_link)
+	if ((!mv88e6xxx_port_ppu_updates(chip, port) ||
+	     mode == MLO_AN_FIXED) && ops->port_set_link)
 		err = ops->port_set_link(chip, port, LINK_FORCED_DOWN);
 	mv88e6xxx_reg_unlock(chip);
 
@@ -731,7 +732,7 @@ static void mv88e6xxx_mac_link_up(struct dsa_switch *ds, int port,
 	ops = chip->info->ops;
 
 	mv88e6xxx_reg_lock(chip);
-	if (!mv88e6xxx_port_ppu_updates(chip, port)) {
+	if (!mv88e6xxx_port_ppu_updates(chip, port) || mode == MLO_AN_FIXED) {
 		/* FIXME: for an automedia port, should we force the link
 		 * down here - what if the link comes up due to "other" media
 		 * while we're bringing the port up, how is the exclusivity
-- 
2.26.0

[PATCH net v2 2/2] net: dsa: Down cpu/dsa ports phylink will control

[Andrew Lunn]

DSA and CPU ports can be configured in two ways. By default, the
driver should configure such ports to there maximum bandwidth. For
most use cases, this is suficient. When this default is insufficient,
a phylink instance can be bound to such ports, and phylink will
configure the port, e.g. based on fixed-link properties. phylink
assumes the port is initially down. Given that the driver should of
already configured it to its maximum speed, ask the driver to down
the port before instantiating the phylink instance.

Fixes: 30c4a5b0aad8 ("net: mv88e6xxx: use resolved link config in mac_link_up()")
Signed-off-by: Andrew Lunn <andrew@lunn.ch>
---
 net/dsa/port.c | 7 ++++++-
 1 file changed, 6 insertions(+), 1 deletion(-)

diff --git a/net/dsa/port.c b/net/dsa/port.c
index 231b2d494f1c..a58fdd362574 100644
--- a/net/dsa/port.c
+++ b/net/dsa/port.c
@@ -670,11 +670,16 @@ int dsa_port_link_register_of(struct dsa_port *dp)
 {
 	struct dsa_switch *ds = dp->ds;
 	struct device_node *phy_np;
+	int port = dp->index;
 
 	if (!ds->ops->adjust_link) {
 		phy_np = of_parse_phandle(dp->dn, "phy-handle", 0);
-		if (of_phy_is_fixed_link(dp->dn) || phy_np)
+		if (of_phy_is_fixed_link(dp->dn) || phy_np) {
+			if (ds->ops->phylink_mac_link_down)
+				ds->ops->phylink_mac_link_down(ds, port,
+					MLO_AN_FIXED, PHY_INTERFACE_MODE_NA);
 			return dsa_port_phylink_register(dp);
+		}
 		return 0;
 	}
 
-- 
2.26.0

Re: [PATCH net v2 2/2] net: dsa: Down cpu/dsa ports phylink will control

[Sergei Shtylyov]

Hello!

   Only a couple grammar bugs... :-)

On 14.04.2020 3:34, Andrew Lunn wrote:

> DSA and CPU ports can be configured in two ways. By default, the
> driver should configure such ports to there maximum bandwidth. For
> most use cases, this is suficient. When this default is insufficient,

    Sufficient.

> a phylink instance can be bound to such ports, and phylink will
> configure the port, e.g. based on fixed-link properties. phylink
> assumes the port is initially down. Given that the driver should of

    Should have?

> already configured it to its maximum speed, ask the driver to down
> the port before instantiating the phylink instance.
> 
> Fixes: 30c4a5b0aad8 ("net: mv88e6xxx: use resolved link config in mac_link_up()")
> Signed-off-by: Andrew Lunn <andrew@lunn.ch>
[...]

MBR, Sergei

Re: [PATCH net v2 0/2] mv88e6xxx fixed link fixes

[David Miller]

From: Andrew Lunn <andrew@lunn.ch>
Date: Tue, 14 Apr 2020 02:34:37 +0200

> Recent changes for how the MAC is configured broke fixed links, as
> used by CPU/DSA ports, and for SFPs when phylink cannot be used. The
> first fix is unchanged from v1. The second fix takes a different
> solution than v1. If a CPU or DSA port is known to have a PHYLINK
> instance, configure the port down before instantiating the PHYLINK, so
> it is in the down state as expected by PHYLINK.

Series applied with Sergei's grammar fixes to the commit message of
patch #2.

Thanks!