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

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

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