Re: [PATCH net v3 1/4] net: stmmac: propagate PTP init failures in stmmac_setup_ptp()

[sashiko-bot@kernel.org]

Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:
- [High] Propagating stmmac_init_timestamping() return value causes network interface open/resume to fail on hardware without PTP support.
--

commit 9136103951651df9ef9668b2743b64e0da023618
Author: Lorenzo Bianconi <lorenzo.bianconi@oss.qualcomm.com>

net: stmmac: propagate PTP init failures in stmmac_setup_ptp()

This patch modifies stmmac_setup_ptp() to return an error code instead of
swallowing PTP setup failures. This ensures that failures like a broken
reference clock prevent the schedule from getting stuck during taprio offload.

> diff --git a/drivers/net/ethernet/stmicro/stmmac/stmmac_main.c b/drivers/net/ethernet/stmicro/stmmac/stmmac_main.c
> index d576059c04df9..47295845371aa 100644
> --- a/drivers/net/ethernet/stmicro/stmmac/stmmac_main.c
> +++ b/drivers/net/ethernet/stmicro/stmmac/stmmac_main.c
> @@ -937,18 +937,27 @@ static int stmmac_init_timestamping(struct stmmac_priv *priv)
>  	return 0;
>  }
>  
> -static void stmmac_setup_ptp(struct stmmac_priv *priv)
> +static int stmmac_setup_ptp(struct stmmac_priv *priv)
>  {
>  	int ret;
>  
>  	ret = clk_prepare_enable(priv->plat->clk_ptp_ref);
> -	if (ret < 0)
> +	if (ret < 0) {
>  		netdev_warn(priv->dev,
>  			    "failed to enable PTP reference clock: %pe\n",
>  			    ERR_PTR(ret));
> +		return ret;
> +	}
> +
> +	ret = stmmac_init_timestamping(priv);
> +	if (ret) {
> +		clk_disable_unprepare(priv->plat->clk_ptp_ref);
> +		return ret;
> +	}

[Severity: High]
Does this unexpectedly propagate a failure when the MAC hardware lacks PTP
capabilities?

In stmmac_init_timestamping(), if the hardware does not support PTP, it
returns -EOPNOTSUPP:

	if (!(priv->dma_cap.time_stamp || priv->dma_cap.atime_stamp)) {
		netdev_info(priv->dev, "PTP not supported by HW\n");
		return -EOPNOTSUPP;
	}

By returning this error here, will it prevent devices without PTP support
from initializing correctly?

> +
> +	stmmac_ptp_register(priv);
>  
> -	if (stmmac_init_timestamping(priv) == 0)
> -		stmmac_ptp_register(priv);
> +	return 0;
>  }
>  
>  static void stmmac_release_ptp(struct stmmac_priv *priv)
> @@ -4153,10 +4162,12 @@ static int __stmmac_open(struct net_device *dev,
>  	ret = stmmac_hw_setup(dev);
>  	if (ret < 0) {
>  		netdev_err(priv->dev, "%s: Hw setup failed\n", __func__);
> -		goto init_error;
> +		return ret;
>  	}
>  
> -	stmmac_setup_ptp(priv);
> +	ret = stmmac_setup_ptp(priv);
> +	if (ret)
> +		goto ptp_error;

[Severity: High]
Could propagating the -EOPNOTSUPP return value cause the network interface
bring-up to fail completely?

Prior to this patch, stmmac_setup_ptp() ignored the return value, allowing
non-PTP hardware to seamlessly skip PTP setup. Since the error is now handled,
it seems like bringing up the interface (e.g., via ip link set up) will jump
to ptp_error and abort the open process for hardware without PTP.

[ ... ]
> @@ -8318,13 +8330,12 @@ int stmmac_resume(struct device *dev)
>  	ret = stmmac_hw_setup(ndev);
>  	if (ret < 0) {
>  		netdev_err(priv->dev, "%s: Hw setup failed\n", __func__);
> -		stmmac_legacy_serdes_power_down(priv);
> -		mutex_unlock(&priv->lock);
> -		rtnl_unlock();
> -		return ret;
> +		goto error_unlock;
>  	}
>  
> -	stmmac_init_timestamping(priv);
> +	ret = stmmac_init_timestamping(priv);
> +	if (ret)
> +		goto error_stop_dma;

[Severity: High]
Does this introduce a similar issue during system resume?

If stmmac_init_timestamping() returns -EOPNOTSUPP for hardware lacking PTP,
this looks like it will abort the resume process and leave the interface
broken after the system wakes up.

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20260902-stmmac-est-reapply-after-open-v3-0-e72a6df5a7ef@oss.qualcomm.com?part=1