Re: [PATCH net-next v5 01/10] net: ethernet: ravb: Remove gPTP control from WoL setup and restore

[sashiko-bot@kernel.org]

Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:
- [Medium] ravb: gPTP hardware left active during WoL suspend
--

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

net: ethernet: ravb: Remove gPTP control from WoL setup and restore

This commit removes the explicit gPTP hardware stop and start calls from the
Wake-on-LAN setup and restore functions. The commit message indicates this is
redundant because the network device is closed and re-opened during system
suspend and resume.

>     This makes the special case to stop and start it when resuming from WoL
>     redundant. As the ndev will always be closed and re-opened when
>     resuming the system.

[Severity: Medium]
Is this assumption accurate for Wake-on-LAN?

Looking at ravb_suspend(), the device explicitly bypasses ravb_close() when 
WoL is enabled:

ravb_suspend()
{
	...
	if (priv->wol_enabled) {
		ret = ravb_wol_setup(ndev);
		rtnl_unlock();
		return ret;
	}

	ret = ravb_close(ndev);
	...
}

> diff --git a/drivers/net/ethernet/renesas/ravb_main.c b/drivers/net/ethernet/renesas/ravb_main.c
> index ea1c7e536791e..36173a09e2fdb 100644
> --- a/drivers/net/ethernet/renesas/ravb_main.c
> +++ b/drivers/net/ethernet/renesas/ravb_main.c
> @@ -3182,9 +3182,6 @@ static int ravb_wol_setup(struct net_device *ndev)
>  	/* Enable MagicPacket */
>  	ravb_modify(ndev, ECMR, ECMR_MPDE, ECMR_MPDE);
>  
> -	if (priv->info->ccc_gac)
> -		ravb_ptp_stop(ndev);
> -

[Severity: Medium]
Because ravb_close() is bypassed in ravb_suspend() as noted above, does 
removing ravb_ptp_stop() here leave the gPTP hardware active and consuming 
power during a WoL suspend?

>  	return enable_irq_wake(priv->emac_irq);
>  }
>  
> @@ -3204,9 +3201,6 @@ static int ravb_wol_restore(struct net_device *ndev)
>  	if (error)
>  		return error;
>  
> -	if (priv->info->ccc_gac)
> -		ravb_ptp_init(ndev, priv->pdev);
> -

[Severity: Medium]
Since the gPTP hardware may not have been stopped during suspend, will 
removing ravb_ptp_init() here cause problems upon resume, or leave the PTP 
clock in an unexpected state?

>  	if (info->nc_queues)
>  		napi_enable(&priv->napi[RAVB_NC]);
>  	napi_enable(&priv->napi[RAVB_BE]);

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