Re: [PATCH 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 1 potential issue(s) to consider:
- [High] Incorrect unconditional advertisement of 5G/10G MAC capabilities and 10GBASER/XGMII interface modes for older ENETC v1 hardware.
--

commit afb9f1b607c70d3a6a854e15aaa034f99fb49f5c
Author: Claudiu Manoil <claudiu.manoil@nxp.com>

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

The ENETC pseudo-MACs are proprietary internal links that do not implement
any standard MII interface, so restrict their supported PHY interface modes
to PHY_INTERFACE_MODE_INTERNAL only. This also expands the supported
interface modes and MAC capabilities for standalone ENETC.

> diff --git a/drivers/net/ethernet/freescale/enetc/enetc_pf_common.c b/drivers/net/ethernet/freescale/enetc/enetc_pf_common.c
> index 3597cb81a7ccb..adf79a99a8c81 100644
> --- a/drivers/net/ethernet/freescale/enetc/enetc_pf_common.c
> +++ b/drivers/net/ethernet/freescale/enetc/enetc_pf_common.c

[ ... ]

> @@ -400,25 +400,40 @@ int enetc_phylink_create(struct enetc_ndev_priv *priv, struct device_node *node,
>  {
>  	struct enetc_pf *pf = enetc_si_priv(priv->si);
>  	struct phylink *phylink;
> +	unsigned long mac_caps;
>  	int err;
>  
>  	pf->phylink_config.dev = &priv->ndev->dev;
>  	pf->phylink_config.type = PHYLINK_NETDEV;
> -	pf->phylink_config.mac_capabilities = MAC_ASYM_PAUSE | MAC_SYM_PAUSE |
> -		MAC_10 | MAC_100 | MAC_1000 | MAC_2500FD;
>  
>  	__set_bit(PHY_INTERFACE_MODE_INTERNAL,
>  		  pf->phylink_config.supported_interfaces);
> -	__set_bit(PHY_INTERFACE_MODE_SGMII,
> -		  pf->phylink_config.supported_interfaces);
> -	__set_bit(PHY_INTERFACE_MODE_1000BASEX,
> -		  pf->phylink_config.supported_interfaces);
> -	__set_bit(PHY_INTERFACE_MODE_2500BASEX,
> -		  pf->phylink_config.supported_interfaces);
> -	__set_bit(PHY_INTERFACE_MODE_USXGMII,
> -		  pf->phylink_config.supported_interfaces);
> -	phy_interface_set_rgmii(pf->phylink_config.supported_interfaces);
>  
> +	mac_caps = MAC_ASYM_PAUSE | MAC_SYM_PAUSE;
> +	if (!enetc_is_pseudo_mac(priv->si)) {
> +		mac_caps |= MAC_10 | MAC_100 | MAC_1000FD | MAC_2500FD |
> +			    MAC_5000FD | MAC_10000FD;
> +
> +		__set_bit(PHY_INTERFACE_MODE_SGMII,
> +			  pf->phylink_config.supported_interfaces);
> +		__set_bit(PHY_INTERFACE_MODE_1000BASEX,
> +			  pf->phylink_config.supported_interfaces);
> +		__set_bit(PHY_INTERFACE_MODE_2500BASEX,
> +			  pf->phylink_config.supported_interfaces);
> +		__set_bit(PHY_INTERFACE_MODE_USXGMII,
> +			  pf->phylink_config.supported_interfaces);
> +		__set_bit(PHY_INTERFACE_MODE_10GBASER,
> +			  pf->phylink_config.supported_interfaces);
> +		__set_bit(PHY_INTERFACE_MODE_XGMII,
> +			  pf->phylink_config.supported_interfaces);

[Severity: High]
Will this unconditionally enable 5G and 10G speeds for older ENETC v1
hardware?

If a 10G capable PHY is attached via USXGMII to an ENETC v1 port, it appears
the link will negotiate at 10Gbps instead of being capped at 2.5Gbps.

If that happens, enetc_sched_speed_set() doesn't seem to handle SPEED_5000 or
SPEED_10000:

drivers/net/ethernet/freescale/enetc/enetc_qos.c:enetc_sched_speed_set() {
	...
	switch (speed) {
	case SPEED_1000:
		pspeed = ENETC_PMR_PSPEED_1000M;
		break;
	case SPEED_2500:
		pspeed = ENETC_PMR_PSPEED_2500M;
		break;
	case SPEED_100:
		pspeed = ENETC_PMR_PSPEED_100M;
		break;
	case SPEED_10:
	default:
		pspeed = ENETC_PMR_PSPEED_10M;
	}
	...
}

This would cause it to fall through to 10 Mbps and break QoS scheduling.

Additionally, enetc_mac_config() doesn't appear to have support for
configuring the hardware for PHY_INTERFACE_MODE_10GBASER or
PHY_INTERFACE_MODE_XGMII which are now being advertised.

Should these new capabilities be restricted to newer hardware revisions?

> +		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);

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