Re: [PATCH net-next v3 1/2] net: lan743x: add RMII strap status detection for PCI11x1x

[Simon Horman]

From: 'Simon Horman' <horms@xxxxxxxxxx>

This is an AI-generated review of your patch. The human sending this
email has considered the AI review valid, or at least plausible.
Full review at: https://netdev-ai.bots.linux.dev/sashiko/
---
Subject: net: lan743x: add RMII strap status detection for PCI11x1x

This patch extends pci11x1x_strap_get_status() to read RMII strap bits
from STRAP_READ and initialise a new is_rmii_en flag on the adapter,
updating the netif_dbg() to print the selected interface as SGMII, RMII
or RGMII.

> diff --git a/drivers/net/ethernet/microchip/lan743x_main.c b/drivers/net/ethernet/microchip/lan743x_main.c
> index 1cdce35e1423..0798f3f1f435 100644
> --- a/drivers/net/ethernet/microchip/lan743x_main.c
> +++ b/drivers/net/ethernet/microchip/lan743x_main.c
> @@ -73,8 +74,15 @@ static void pci11x1x_strap_get_status(struct lan743x_adapter *adapter)
>  			adapter->is_sgmii_en = false;
>  		}
>  	}
> -	netif_dbg(adapter, drv, adapter->netdev,
> -		  "SGMII I/F %sable\n", adapter->is_sgmii_en ? "En" : "Dis");
> +
> +	if (!adapter->is_sgmii_en && strap & STRAP_READ_USE_RMII_EN_) {
> +		if (strap & STRAP_READ_RMII_EN_)
> +			adapter->is_rmii_en = true;
> +	}
> +
> +	netif_dbg(adapter, drv, adapter->netdev, "Selected I/F: %s\n",
> +		  adapter->is_sgmii_en ? "SGMII" :
> +		  adapter->is_rmii_en  ? "RMII"  : "RGMII");
>  }

[Low]
This isn't a bug, but does the new "Selected I/F: %s" debug print stay
consistent with the rest of the driver when is_rmii_en is true?

In lan743x_mdiobus_init() the non-SGMII branch still prints unconditionally:

	} else {
		dev_dbg(&adapter->pdev->dev, "RGMII operation\n");
		// Only C22 support when RGMII I/F
		adapter->mdiobus->read = lan743x_mdiobus_read_c22;
		adapter->mdiobus->write = lan743x_mdiobus_write_c22;
		adapter->mdiobus->name = "lan743x-mdiobus";

So an RMII-strapped board will report "Selected I/F: RMII" from
pci11x1x_strap_get_status() and then "RGMII operation" from
lan743x_mdiobus_init() in the same boot. The C22-only setup is correct
for both RGMII and RMII, so the only effect is the contradictory dmesg
line. Should this dev_dbg() also be updated to reflect the RMII case,
either in this patch or in the follow-up "net: lan743x: add support for
RMII interface"?