Re: [PATCH RESEND v4 net-next 07/14] net: enetc: differentiate phylink capabilities for pseudo-MAC and standalone MAC

[sashiko-bot@kernel.org]

Thank you for your contribution! Sashiko AI review found 3 potential issue(s) to consider:

New issues:
- [High] The patch advertises `PHY_INTERFACE_MODE_10GBASER` as a supported interface to phylink despite the feature being incomplete and explicitly unsupported.

Pre-existing issues:
- [High] The hardware configuration function `enetc4_mac_config()` does not handle `PHY_INTERFACE_MODE_1000BASEX`, leading to runtime errors and an unconfigured MAC.
- [High] ENETC v4 fails to probe for any PCS-dependent PHY interface modes because `enetc4_pf_ops` lacks a `create_pcs` callback.
--

--- Patch [7]: [PATCH RESEND v4 net-next 07/14] net: enetc: differentiate phylink capabilities for pseudo-MAC and standalone MAC ---
commit 87cae554b380c00e9f19136c8479811aa8887e05
Author: Claudiu Manoil <claudiu.manoil@nxp.com>

net: enetc: differentiate phylink capabilities for pseudo-MAC and standalone MAC

This patch restricts ENETC pseudo-MACs to PHY_INTERFACE_MODE_INTERNAL while
configuring standard full-duplex speeds. For standalone ENETC (v4), it updates
the interface modes to include 10GBASER (as preparation) and explicit duplex
capabilities.

> diff --git a/drivers/net/ethernet/freescale/enetc/enetc4_pf.c b/drivers/net/ethernet/freescale/enetc/enetc4_pf.c
> index f24269a48c26b..75ee117e9b1df 100644
> --- a/drivers/net/ethernet/freescale/enetc/enetc4_pf.c
> +++ b/drivers/net/ethernet/freescale/enetc/enetc4_pf.c
> @@ -602,7 +602,6 @@ static void enetc4_mac_config(struct enetc_pf *pf, unsigned int mode,
>  		val |= IFMODE_SGMII;
>  		break;
>  	case PHY_INTERFACE_MODE_10GBASER:

[Severity: High]
This isn't a bug introduced by this patch, but does enetc4_mac_config() in 
drivers/net/ethernet/freescale/enetc/enetc4_pf.c fail to handle 
PHY_INTERFACE_MODE_1000BASEX?

If phylink configures the MAC for 1000BASEX, which is advertised as supported, 
it looks like it will fall through to the default case here and leave the MAC 
unconfigured:

    default:
        dev_err(priv->dev, "Unsupported PHY mode:%d\n", phy_mode);
        return;

> -	case PHY_INTERFACE_MODE_XGMII:
>  	case PHY_INTERFACE_MODE_USXGMII:
>  		val |= IFMODE_XGMII;
>  		break;
> diff --git a/drivers/net/ethernet/freescale/enetc/enetc_pf_common.c b/drivers/net/ethernet/freescale/enetc/enetc_pf_common.c
> index 3597cb81a7ccb..781b22198ca86 100644
> --- a/drivers/net/ethernet/freescale/enetc/enetc_pf_common.c
> +++ b/drivers/net/ethernet/freescale/enetc/enetc_pf_common.c
[ ... ]
> @@ -359,7 +359,8 @@ static bool enetc_port_has_pcs(struct enetc_pf *pf)
>  	return (pf->if_mode == PHY_INTERFACE_MODE_SGMII ||
>  		pf->if_mode == PHY_INTERFACE_MODE_1000BASEX ||
>  		pf->if_mode == PHY_INTERFACE_MODE_2500BASEX ||
> -		pf->if_mode == PHY_INTERFACE_MODE_USXGMII);
> +		pf->if_mode == PHY_INTERFACE_MODE_USXGMII ||
> +		pf->if_mode == PHY_INTERFACE_MODE_10GBASER);
>  }

[Severity: High]
This is a pre-existing issue, but does ENETC v4 fail to probe for any
PCS-dependent interface modes?

Since enetc_port_has_pcs() returns true for SGMII, 1000BASEX, 2500BASEX, 
USXGMII, and now 10GBASER, enetc_imdio_create() will be called during probe. 

However, enetc4_pf_ops in drivers/net/ethernet/freescale/enetc/enetc4_pf.c 
lacks a .create_pcs callback. Won't this trigger the check in 
enetc_imdio_create():

    if (!pf->ops->create_pcs) {
        dev_err(dev, "Creating PCS is not supported\n");
        return -EOPNOTSUPP;
    }

and cause the entire probe sequence to fail for these modes?

[ ... ]
> @@ -400,25 +401,42 @@ int enetc_phylink_create(struct enetc_ndev_priv *priv, struct device_node *node,
>  {
[ ... ]
> +		if (!is_enetc_rev1(priv->si)) {
> +			mac_caps |= MAC_5000FD | MAC_10000FD;
> +			__set_bit(PHY_INTERFACE_MODE_10GBASER,
> +				  pf->phylink_config.supported_interfaces);
> +		}

[Severity: High]
The commit message explicitly states that 10GBASE-R mode is not supported yet
and is added simply as preparation. 

Will advertising an unsupported interface mode to phylink in 
enetc_phylink_create() cause it to accept configurations for it, leading to a 
broken configuration state or a hard probe failure when it attempts to 
configure the MAC and PCS?

> +
> +		phy_interface_set_rgmii(pf->phylink_config.supported_interfaces);
> +	} else {
> +		mac_caps |= MAC_10FD | MAC_100FD | MAC_1000FD | MAC_2500FD |
> +			    MAC_5000FD | MAC_10000FD | MAC_20000FD |
> +			    MAC_25000FD;
> +	}
> +
> +	pf->phylink_config.mac_capabilities = mac_caps;
>  	phylink = phylink_create(&pf->phylink_config, of_fwnode_handle(node),
>  				 pf->if_mode, ops);
>  	if (IS_ERR(phylink)) {

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20260720014317.1059359-1-wei.fang@oss.nxp.com?part=7