Re: [PATCH net-next v5 05/10] net: ethernet: ravb: Propagate error from ptp_clock_register()

[sashiko-bot@kernel.org]

Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:
- [Medium] net: ravb: ethtool timestamping capabilities are incorrectly suppressed when interface is down
--

commit bba7bddbe4fcaec0df4875259d8239664ffaa696
Author: Niklas Söderlund <niklas.soderlund+renesas@ragnatech.se>

net: ethernet: ravb: Propagate error from ptp_clock_register()

If the PTP clock registration failed, the error is silently ignored. This
commit reworks callers of ravb_ptp_init() to handle and propagate the error.
It also updates how the PHC index is handled.

Link: https://sashiko.dev/#/patchset/20260610102432.3538432-2-niklas.soderlund+renesas@ragnatech.se?part=1

> diff --git a/drivers/net/ethernet/renesas/ravb_main.c b/drivers/net/ethernet/renesas/ravb_main.c
> --- a/drivers/net/ethernet/renesas/ravb_main.c
> +++ b/drivers/net/ethernet/renesas/ravb_main.c
> @@ -1774,7 +1774,7 @@ static int ravb_get_ts_info(struct net_device *ndev,
>  	struct ravb_private *priv = netdev_priv(ndev);
>  	const struct ravb_hw_info *hw_info = priv->info;
>  
> -	if (hw_info->gptp || hw_info->ccc_gac) {
> +	if (priv->ptp.clock && (hw_info->gptp || hw_info->ccc_gac)) {

[Severity: Medium]
Does this new condition incorrectly suppress the static ethtool timestamping 
capabilities when the interface is administratively down?

If a userspace tool calls ethtool -T on the interface while it is in the down
state, priv->ptp.clock will be NULL since the clock is only active while the 
interface is open. By checking priv->ptp.clock here, ravb_get_ts_info() 
completely skips reporting static capabilities like SOF_TIMESTAMPING_TX_SOFTWARE 
and SOF_TIMESTAMPING_TX_HARDWARE.

Could the priv->ptp.clock check be moved so that it only guards the
info->phc_index assignment below, rather than the entire capabilities block?

>  		info->so_timestamping =
>  			SOF_TIMESTAMPING_TX_SOFTWARE |
>  			SOF_TIMESTAMPING_TX_HARDWARE |
> @@ -1785,7 +1785,7 @@ static int ravb_get_ts_info(struct net_device *ndev,
>  			(1 << HWTSTAMP_FILTER_NONE) |
>  			(1 << HWTSTAMP_FILTER_PTP_V2_L2_EVENT) |
>  			(1 << HWTSTAMP_FILTER_ALL);
> -		info->phc_index = READ_ONCE(priv->ptp.phc_index);
> +		info->phc_index = ptp_clock_index(priv->ptp.clock);
>  	}

[ ... ]

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20260921175037.4084310-1-niklas.soderlund+renesas@ragnatech.se?part=5