[PATCH net-next 0/4] net: altera-tse: Cleanup init sequence

[PATCH net-next 0/4] net: altera-tse: Cleanup init sequence

[Maxime Chevallier]

When Altera TSE was converted to phylink, the PCS and phylink creation
were added after register_netdev(), which is wrong as this may race
with .ndo_open() once the netdev is registered.

This series makes so that we register the netdev once all resources are
cleanly initialised, that includes PCS and phylink creation as well as a
few other operations such as reading the IP version.

No errors were found in the wild, so this series doesn't target net, but
given that we fix some racy-ness, a point could be made to send that to
net.

This series doesn't introduce functional changes, however the internal
mii_bus for PCS configuration is renamed.

Maxime Chevallier (4):
  net: altera-tse: Set platform drvdata before registering netdev
  net: altera-tse: Read core revision before registering netdev
  net: altera-tse: Don't use netdev name for the PCS mdio bus
  net: altera-tse: Init PCS and phylink before registering netdev

 drivers/net/ethernet/altera/altera_tse_main.c | 37 +++++++++----------
 1 file changed, 18 insertions(+), 19 deletions(-)

-- 
2.49.0

[PATCH net-next 1/4] net: altera-tse: Set platform drvdata before registering netdev

[Maxime Chevallier]

We don't have to wait until netdev is registered before setting it as the
pdev's drvdata. Move it at netdev alloc time.

Signed-off-by: Maxime Chevallier <maxime.chevallier@bootlin.com>
---
 drivers/net/ethernet/altera/altera_tse_main.c | 3 +--
 1 file changed, 1 insertion(+), 2 deletions(-)

diff --git a/drivers/net/ethernet/altera/altera_tse_main.c b/drivers/net/ethernet/altera/altera_tse_main.c
index 3f6204de9e6b..6ba1249f027d 100644
--- a/drivers/net/ethernet/altera/altera_tse_main.c
+++ b/drivers/net/ethernet/altera/altera_tse_main.c
@@ -1150,6 +1150,7 @@ static int altera_tse_probe(struct platform_device *pdev)
 	}
 
 	SET_NETDEV_DEV(ndev, &pdev->dev);
+	platform_set_drvdata(pdev, ndev);
 
 	priv = netdev_priv(ndev);
 	priv->device = &pdev->dev;
@@ -1394,8 +1395,6 @@ static int altera_tse_probe(struct platform_device *pdev)
 		goto err_register_netdev;
 	}
 
-	platform_set_drvdata(pdev, ndev);
-
 	priv->revision = ioread32(&priv->mac_dev->megacore_revision);
 
 	if (netif_msg_probe(priv))
-- 
2.49.0

[PATCH net-next 2/4] net: altera-tse: Read core revision before registering netdev

[Maxime Chevallier]

The core revision is used in .ndo_open(), so we have to populate it
before regstering the netdev.

Signed-off-by: Maxime Chevallier <maxime.chevallier@bootlin.com>
---
 drivers/net/ethernet/altera/altera_tse_main.c | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/drivers/net/ethernet/altera/altera_tse_main.c b/drivers/net/ethernet/altera/altera_tse_main.c
index 6ba1249f027d..c74b1c11d759 100644
--- a/drivers/net/ethernet/altera/altera_tse_main.c
+++ b/drivers/net/ethernet/altera/altera_tse_main.c
@@ -1388,6 +1388,8 @@ static int altera_tse_probe(struct platform_device *pdev)
 	spin_lock_init(&priv->tx_lock);
 	spin_lock_init(&priv->rxdma_irq_lock);
 
+	priv->revision = ioread32(&priv->mac_dev->megacore_revision);
+
 	netif_carrier_off(ndev);
 	ret = register_netdev(ndev);
 	if (ret) {
@@ -1395,8 +1397,6 @@ static int altera_tse_probe(struct platform_device *pdev)
 		goto err_register_netdev;
 	}
 
-	priv->revision = ioread32(&priv->mac_dev->megacore_revision);
-
 	if (netif_msg_probe(priv))
 		dev_info(&pdev->dev, "Altera TSE MAC version %d.%d at 0x%08lx irq %d/%d\n",
 			 (priv->revision >> 8) & 0xff,
-- 
2.49.0

[PATCH net-next 3/4] net: altera-tse: Don't use netdev name for the PCS mdio bus

[Maxime Chevallier]

The PCS mdio bus must be created before registering the net_device. To
do that, we musn't depend on the netdev name to create the mdio bus
name. Let's use the device's name instead.

Signed-off-by: Maxime Chevallier <maxime.chevallier@bootlin.com>
---
 drivers/net/ethernet/altera/altera_tse_main.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/drivers/net/ethernet/altera/altera_tse_main.c b/drivers/net/ethernet/altera/altera_tse_main.c
index c74b1c11d759..a601ba57190e 100644
--- a/drivers/net/ethernet/altera/altera_tse_main.c
+++ b/drivers/net/ethernet/altera/altera_tse_main.c
@@ -1404,7 +1404,7 @@ static int altera_tse_probe(struct platform_device *pdev)
 			 (unsigned long) control_port->start, priv->rx_irq,
 			 priv->tx_irq);
 
-	snprintf(mrc.name, MII_BUS_ID_SIZE, "%s-pcs-mii", ndev->name);
+	snprintf(mrc.name, MII_BUS_ID_SIZE, "%s-pcs-mii", dev_name(&pdev->dev));
 	pcs_bus = devm_mdio_regmap_register(&pdev->dev, &mrc);
 	if (IS_ERR(pcs_bus)) {
 		ret = PTR_ERR(pcs_bus);
-- 
2.49.0

[PATCH net-next 4/4] net: altera-tse: Init PCS and phylink before registering netdev

[Maxime Chevallier]

register_netdev() must be done only once all resources are ready, as
they may be used in .ndo_open() immediately upon registration.

Move the lynx PCS and phylink initialisation before registerng the
netdevice. We also remove the call to netif_carrier_off(), as phylink
takes care of that.

Signed-off-by: Maxime Chevallier <maxime.chevallier@bootlin.com>
---
 drivers/net/ethernet/altera/altera_tse_main.c | 32 +++++++++----------
 1 file changed, 16 insertions(+), 16 deletions(-)

diff --git a/drivers/net/ethernet/altera/altera_tse_main.c b/drivers/net/ethernet/altera/altera_tse_main.c
index a601ba57190e..4ffa3edf1d0c 100644
--- a/drivers/net/ethernet/altera/altera_tse_main.c
+++ b/drivers/net/ethernet/altera/altera_tse_main.c
@@ -1390,20 +1390,6 @@ static int altera_tse_probe(struct platform_device *pdev)
 
 	priv->revision = ioread32(&priv->mac_dev->megacore_revision);
 
-	netif_carrier_off(ndev);
-	ret = register_netdev(ndev);
-	if (ret) {
-		dev_err(&pdev->dev, "failed to register TSE net device\n");
-		goto err_register_netdev;
-	}
-
-	if (netif_msg_probe(priv))
-		dev_info(&pdev->dev, "Altera TSE MAC version %d.%d at 0x%08lx irq %d/%d\n",
-			 (priv->revision >> 8) & 0xff,
-			 priv->revision & 0xff,
-			 (unsigned long) control_port->start, priv->rx_irq,
-			 priv->tx_irq);
-
 	snprintf(mrc.name, MII_BUS_ID_SIZE, "%s-pcs-mii", dev_name(&pdev->dev));
 	pcs_bus = devm_mdio_regmap_register(&pdev->dev, &mrc);
 	if (IS_ERR(pcs_bus)) {
@@ -1441,12 +1427,26 @@ static int altera_tse_probe(struct platform_device *pdev)
 		goto err_init_phylink;
 	}
 
+	ret = register_netdev(ndev);
+	if (ret) {
+		dev_err(&pdev->dev, "failed to register TSE net device\n");
+		goto err_register_netdev;
+	}
+
+	if (netif_msg_probe(priv))
+		dev_info(&pdev->dev, "Altera TSE MAC version %d.%d at 0x%08lx irq %d/%d\n",
+			 (priv->revision >> 8) & 0xff,
+			 priv->revision & 0xff,
+			 (unsigned long)control_port->start, priv->rx_irq,
+			 priv->tx_irq);
+
 	return 0;
+
+err_register_netdev:
+	phylink_destroy(priv->phylink);
 err_init_phylink:
 	lynx_pcs_destroy(priv->pcs);
 err_init_pcs:
-	unregister_netdev(ndev);
-err_register_netdev:
 	netif_napi_del(&priv->napi);
 	altera_tse_mdio_destroy(ndev);
 err_free_netdev:
-- 
2.49.0

Re: [PATCH net-next 1/4] net: altera-tse: Set platform drvdata before registering netdev

[Andrew Lunn]

On Thu, Oct 30, 2025 at 11:24:14AM +0100, Maxime Chevallier wrote:
> We don't have to wait until netdev is registered before setting it as the
> pdev's drvdata. Move it at netdev alloc time.
> 
> Signed-off-by: Maxime Chevallier <maxime.chevallier@bootlin.com>

Reviewed-by: Andrew Lunn <andrew@lunn.ch>

    Andrew

Re: [PATCH net-next 2/4] net: altera-tse: Read core revision before registering netdev

[Andrew Lunn]

On Thu, Oct 30, 2025 at 11:24:15AM +0100, Maxime Chevallier wrote:
> The core revision is used in .ndo_open(), so we have to populate it
> before regstering the netdev.

All that open does is:

	if ((priv->revision < 0xd00) || (priv->revision > 0xe00))
		netdev_warn(dev, "TSE revision %x\n", priv->revision);

So i agree this does not need a Fixes: tag.

But i do wounder why this is in open. The revision has already been
printed once in probe. Are values < 0xd00 or > 0xe00 significant? Is
this left over code and some actions that were previously here are now
gone?

Maybe a better fix is to remove this, and make revision local in
probe?

      Andrew

Re: [PATCH net-next 3/4] net: altera-tse: Don't use netdev name for the PCS mdio bus

[Andrew Lunn]

On Thu, Oct 30, 2025 at 11:24:16AM +0100, Maxime Chevallier wrote:
> The PCS mdio bus must be created before registering the net_device. To
> do that, we musn't depend on the netdev name to create the mdio bus
> name. Let's use the device's name instead.

The name does appear in /sys/bus/mdiobus, so there is a potential for
ABI breakage. But it seems unlikely.

Reviewed-by: Andrew Lunn <andrew@lunn.ch>

    Andrew

Re: [PATCH net-next 4/4] net: altera-tse: Init PCS and phylink before registering netdev

[Andrew Lunn]

On Thu, Oct 30, 2025 at 11:24:17AM +0100, Maxime Chevallier wrote:
> register_netdev() must be done only once all resources are ready, as
> they may be used in .ndo_open() immediately upon registration.
> 
> Move the lynx PCS and phylink initialisation before registerng the
> netdevice. We also remove the call to netif_carrier_off(), as phylink
> takes care of that.
> 
> Signed-off-by: Maxime Chevallier <maxime.chevallier@bootlin.com>

Reviewed-by: Andrew Lunn <andrew@lunn.ch>

    Andrew

Re: [PATCH net-next 2/4] net: altera-tse: Read core revision before registering netdev

[Maxime Chevallier]

Hi Andrew,

On 31/10/2025 17:15, Andrew Lunn wrote:
> On Thu, Oct 30, 2025 at 11:24:15AM +0100, Maxime Chevallier wrote:
>> The core revision is used in .ndo_open(), so we have to populate it
>> before regstering the netdev.
> 
> All that open does is:
> 
> 	if ((priv->revision < 0xd00) || (priv->revision > 0xe00))
> 		netdev_warn(dev, "TSE revision %x\n", priv->revision);
> 
> So i agree this does not need a Fixes: tag.
> 
> But i do wounder why this is in open. The revision has already been
> printed once in probe. Are values < 0xd00 or > 0xe00 significant? Is
> this left over code and some actions that were previously here are now
> gone?
> 
> Maybe a better fix is to remove this, and make revision local in
> probe?

You are correct. I was focusing on leaving the existing behaviour
untouched so I didn't dig any further, but now that you point this
out, we can definitely simplify that.

The revision is read from the priv->megacore_revsion address, and
stored into priv->revision.

The only other spot where this could have been useful is in
altera_tse_ethtool.c, .tse_get_drvinfo(), but here we actually
re-read the version from the registers...

So indeed we can keep that local to probe, print a warning for
unexpected version, and just drop priv->revision :)

Thanks, I'll respin with this.

Maxime

>       Andrew