[PATCH v2 net-next 0/2] Ocelot phylink fixes

This series addresses a regression reported by Horatiu which introduced
by the ocelot conversion to phylink: there are broken device trees in
the wild, and the driver fails to probe the entire switch when a port
fails to probe, which it previously did not do.

Continue probing even when some ports fail to initialize properly.

Horatiu Vultur (1):
  net: mscc: ocelot: be able to reuse a devlink_port after teardown

Vladimir Oltean (1):
  net: mscc: ocelot: allow probing to continue with ports that fail to
    register

 drivers/net/ethernet/mscc/ocelot_net.c     | 1 +
 drivers/net/ethernet/mscc/ocelot_vsc7514.c | 7 ++++---
 2 files changed, 5 insertions(+), 3 deletions(-)

-- 
2.25.1

From: Horatiu Vultur <horatiu.vultur@microchip.com>

There are cases where we would like to continue probing the switch even
if one port has failed to probe. When that happens, we need to
unregister a devlink_port of type DEVLINK_PORT_FLAVOUR_PHYSICAL and
re-register it of type DEVLINK_PORT_FLAVOUR_UNUSED.

This is fine, except when calling devlink_port_attrs_set on a structure
on which devlink_port_register has been previously called, there is a
WARN_ON in devlink_port_attrs_set that devlink_port->devlink must be
NULL.

So don't assume that the memory behind dlp is clean when calling
ocelot_port_devlink_init, just zero-initialize it.

Signed-off-by: Horatiu Vultur <horatiu.vultur@microchip.com>
Signed-off-by: Vladimir Oltean <vladimir.oltean@nxp.com>
---
 drivers/net/ethernet/mscc/ocelot_net.c | 1 +
 1 file changed, 1 insertion(+)

diff --git a/drivers/net/ethernet/mscc/ocelot_net.c b/drivers/net/ethernet/mscc/ocelot_net.c
index 9adf7dfb5389..9745c29c09ef 100644
--- a/drivers/net/ethernet/mscc/ocelot_net.c
+++ b/drivers/net/ethernet/mscc/ocelot_net.c

@@ -174,6 +174,7 @@ int ocelot_port_devlink_init(struct ocelot *ocelot, int port,
 	struct devlink *dl = ocelot->devlink;
 	struct devlink_port_attrs attrs = {};
 
+	memset(dlp, 0, sizeof(*dlp));
 	memcpy(attrs.switch_id.id, &ocelot->base_mac, id_len);
 	attrs.switch_id.id_len = id_len;
 	attrs.phys.port_number = port;

-- 
2.25.1

The existing ocelot device trees, like ocelot_pcb123.dts for example,
have SERDES ports (ports 4 and higher) that do not have status = "disabled";
but on the other hand do not have a phy-handle or a fixed-link either.

So from the perspective of phylink, they have broken DT bindings.

Since the blamed commit, probing for the entire switch will fail when
such a device tree binding is encountered on a port. There used to be
this piece of code which skipped ports without a phy-handle:

	phy_node = of_parse_phandle(portnp, "phy-handle", 0);
	if (!phy_node)
		continue;

but now it is gone.

Anyway, fixed-link setups are a thing which should work out of the box
with phylink, so it would not be in the best interest of the driver to
add that check back.

Instead, let's look at what other drivers do. Since commit 86f8b1c01a0a
("net: dsa: Do not make user port errors fatal"), DSA continues after a
switch port fails to register, and works only with the ports that
succeeded.

We can achieve the same behavior in ocelot by unregistering the devlink
port for ports where ocelot_port_phylink_create() failed (called via
ocelot_probe_port), and clear the bit in devlink_ports_registered for
that port. This will make the next iteration reconsider the port that
failed to probe as an unused port, and re-register a devlink port of
type UNUSED for it. No other cleanup should need to be performed, since
ocelot_probe_port() should be self-contained when it fails.

Fixes: e6e12df625f2 ("net: mscc: ocelot: convert to phylink")
Reported-and-tested-by: Horatiu Vultur <horatiu.vultur@microchip.com>
Signed-off-by: Vladimir Oltean <vladimir.oltean@nxp.com>
---
 drivers/net/ethernet/mscc/ocelot_vsc7514.c | 7 ++++---
 1 file changed, 4 insertions(+), 3 deletions(-)

diff --git a/drivers/net/ethernet/mscc/ocelot_vsc7514.c b/drivers/net/ethernet/mscc/ocelot_vsc7514.c
index f553eb871087..bfb540591c1f 100644
--- a/drivers/net/ethernet/mscc/ocelot_vsc7514.c
+++ b/drivers/net/ethernet/mscc/ocelot_vsc7514.c

@@ -978,14 +978,15 @@ static int mscc_ocelot_init_ports(struct platform_device *pdev,
 			of_node_put(portnp);
 			goto out_teardown;
 		}
-		devlink_ports_registered |= BIT(port);
 
 		err = ocelot_probe_port(ocelot, port, target, portnp);
 		if (err) {
-			of_node_put(portnp);
-			goto out_teardown;
+			ocelot_port_devlink_teardown(ocelot, port);
+			continue;
 		}
 
+		devlink_ports_registered |= BIT(port);
+
 		ocelot_port = ocelot->ports[port];
 		priv = container_of(ocelot_port, struct ocelot_port_private,
 				    port);

-- 
2.25.1

Hello:

This series was applied to netdev/net-next.git (refs/heads/master):

On Thu, 19 Aug 2021 19:49:56 +0300 you wrote:

This series addresses a regression reported by Horatiu which introduced
by the ocelot conversion to phylink: there are broken device trees in
the wild, and the driver fails to probe the entire switch when a port
fails to probe, which it previously did not do.

Continue probing even when some ports fail to initialize properly.

[...]

Here is the summary with links:
  - [v2,net-next,1/2] net: mscc: ocelot: be able to reuse a devlink_port after teardown
    https://git.kernel.org/netdev/net-next/c/b5e33a157158
  - [v2,net-next,2/2] net: mscc: ocelot: allow probing to continue with ports that fail to register
    https://git.kernel.org/netdev/net-next/c/5c8bb71dbdf8

You are awesome, thank you!
--
Deet-doot-dot, I am a bot.
https://korg.docs.kernel.org/patchwork/pwbot.html