Re: [PATCH net-next v2 4/6] net: ftgmac100: Require phy-mode for AST2700

[sashiko-bot@kernel.org]

Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:
- [Medium] net: unreachable phy-mode validation for AST2700 due to NULL mii_bus
--

commit 73c3274c3dceb52c26bd7913efb395eafee8af8d
Author: Jacky Chou <jacky_chou@aspeedtech.com>

net: ftgmac100: Require phy-mode for AST2700

This commit modifies the PHY interface initialization for the AST2700 SoC.
It mandates the presence of the "phy-mode" device tree property and will
fail probing if it is missing, instead of silently defaulting to RGMII.

> diff --git a/drivers/net/ethernet/faraday/ftgmac100.c b/drivers/net/ethernet/faraday/ftgmac100.c
> index b835472da3604..2d029d1262e6f 100644
> --- a/drivers/net/ethernet/faraday/ftgmac100.c
> +++ b/drivers/net/ethernet/faraday/ftgmac100.c
> @@ -1521,8 +1521,13 @@ static int ftgmac100_mii_probe(struct net_device *netdev)
>  
>  	/* Default to RGMII. It's a gigabit part after all */
>  	err = of_get_phy_mode(np, &phy_intf);
> -	if (err)
> +	if (err) {
> +		if (priv->mac_id == FTGMAC100_AST2700) {
> +			dev_err(priv->dev, "phy-mode is required for this SoC\n");
> +			return -EINVAL;
> +		}

[Severity: Medium]
Is this new AST2700 check unreachable in practice?

A few lines earlier in ftgmac100_mii_probe(), the function unconditionally
returns -ENODEV if priv->mii_bus is NULL:

	if (!priv->mii_bus) {
		dev_err(priv->dev, "No MDIO bus available\n");
		return -ENODEV;
	}

Looking at ftgmac100_probe(), the driver skips calling ftgmac100_setup_mdio()
for the AST2700:

	if (priv->mac_id == FTGMAC100_FARADAY ||
	    priv->mac_id == FTGMAC100_AST2400 ||
	    priv->mac_id == FTGMAC100_AST2500) {
		err = ftgmac100_setup_mdio(netdev);
		if (err)
			return err;
	}

Because ftgmac100_setup_mdio() is skipped, does priv->mii_bus remain NULL for
the AST2700, causing ftgmac100_mii_probe() to always return -ENODEV before 
ever reaching the newly added phy-mode validation logic?

>  		phy_intf = PHY_INTERFACE_MODE_RGMII;
> +	}
>  
>  	/* Aspeed only supports these. I don't know about other IP
>  	 * block vendors so I'm going to just let them through for

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20261006-support_ast2700-v2-0-36de51fb8066@aspeedtech.com?part=4