Re: [PATCH net-next v6 5/7] net: ftgmac100: Require phy-mode for AST2700

[sashiko-bot@kernel.org]

Thank you for your contribution! Sashiko AI review found 1 potential
issue to consider:
- [Medium] net: ftgmac100: unreachable AST2700 phy-mode check in
  ftgmac100_mii_probe()
--

commit 76d6a3664942a09583b0bf6c204d11a490a88f77
Author: Jacky Chou <jacky_chou@aspeedtech.com>

net: ftgmac100: Require phy-mode for AST2700

This commit adds a check in ftgmac100_mii_probe() to fail probe with
-EINVAL on AST2700 when the "phy-mode" device tree property is missing,
rather than defaulting to RGMII.

> diff --git a/drivers/net/ethernet/faraday/ftgmac100.c b/drivers/net/ethernet/faraday/ftgmac100.c
> index 9b7becb017262..5a578a62df673 100644
> --- a/drivers/net/ethernet/faraday/ftgmac100.c
> +++ b/drivers/net/ethernet/faraday/ftgmac100.c
> @@ -1526,8 +1526,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;
> +		}
>  		phy_intf = PHY_INTERFACE_MODE_RGMII;
> +	}

[Severity: Medium]
Can this FTGMAC100_AST2700 check in ftgmac100_mii_probe() ever be
reached?

ftgmac100_mii_probe() returns -ENODEV at the top of the function when
!priv->mii_bus, and ftgmac100_setup_mdio() is only called for Faraday,
AST2400, and AST2500:

drivers/net/ethernet/faraday/ftgmac100.c:ftgmac100_probe() {
    ...
	if (priv->mac_id == FTGMAC100_FARADAY ||
	    priv->mac_id == FTGMAC100_AST2400 ||
	    priv->mac_id == FTGMAC100_AST2500) {
		err = ftgmac100_setup_mdio(netdev);
    ...
}

For FTGMAC100_AST2700, priv->mii_bus is always NULL, so reaching
ftgmac100_mii_probe() always fails with "No MDIO bus available" before
of_get_phy_mode() is called.  Meanwhile, AST2700 device tree nodes with
use-ncsi or an mdio child node return from ftgmac100_probe_dt() without
verifying that phy-mode is present.  Should this check be placed in
ftgmac100_probe_dt() instead?

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20261112-support_ast2700-v6-0-238020bf46cf@aspeedtech.com?part=5