Re: [PATCH net-next v4 02/10] net: ethernet: ravb: Move programming of gPTP timer interval

[sashiko-bot@kernel.org]

Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:
- [High] Incorrect wait condition for GTI loading completion causes a read-modify-write race on GCCR.
--

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

net: ethernet: ravb: Move programming of gPTP timer interval

This commit moves the programming of the gPTP timer interval from the
main RAVB logic into the gPTP initialization function. It also executes
this operation while holding the gPTP registers lock and attempts to
wait for the hardware to acknowledge the programming.

> diff --git a/drivers/net/ethernet/renesas/ravb_ptp.c b/drivers/net/ethernet/renesas/ravb_ptp.c
> index 43218bc15b151..157fb78209c78 100644
> --- a/drivers/net/ethernet/renesas/ravb_ptp.c
> +++ b/drivers/net/ethernet/renesas/ravb_ptp.c

[ ... ]

> @@ -321,11 +321,20 @@ void ravb_ptp_init(struct net_device *ndev, struct platform_device *pdev)
>  
>  	priv->ptp.info = ravb_ptp_info;
>  
> -	priv->ptp.default_addend = ravb_read(ndev, GTI);
> +	priv->ptp.default_addend = priv->gti_tiv;
>  	priv->ptp.current_addend = priv->ptp.default_addend;
>  
>  	spin_lock_irqsave(&priv->lock, flags);
> +
> +	/* Set gPTP Timer Increment Value. */
> +	ravb_write(ndev, priv->ptp.default_addend, GTI);
> +
> +	/* Request GTI loading. */
> +	ravb_modify(ndev, GCCR, GCCR_LTI, GCCR_LTI);
> +
> +	/* Wait for GIT loading to complete. */
>  	ravb_wait(ndev, GCCR, GCCR_TCR, GCCR_TCR_NOREQ);

[Severity: High]
Does this wait on the correct bits for GTI loading? The comment mentions
waiting for the load to complete, which is triggered by GCCR_LTI, but the
code polls GCCR_TCR instead.

Since the wait condition on GCCR_TCR is likely already met (no TCR request
active), this wait will return immediately.

> +
>  	ravb_modify(ndev, GCCR, GCCR_TCSS, GCCR_TCSS_ADJGPTP);

If the hardware is still processing the LTI request, could this read-modify-write
operation cause a race condition?

It might read the still-active GCCR_LTI bit and write it back to 1,
erroneously re-triggering the loading and potentially corrupting the gPTP
hardware state machine during normal interface configuration.

>  	spin_unlock_irqrestore(&priv->lock, flags);
>

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