Re: [PATCH net-next v3 2/3] ptp: Add driver for R-Car Gen4

[Niklas Söderlund]

Hi Vadim,

Thanks for your feedback.

On 2026-07-01 22:47:16 +0100, Vadim Fedorenko wrote:
> On 01/07/2026 10:06, Niklas Söderlund wrote:
> > Add driver for the gPTP timer found on R-Car Gen4 devices. The timer is
> > system-wide and shared by different Ethernet devices on each Gen4
> > platform. The operation of the timer is however not completely in
> > depended of the systems Ethernet devices.
> > 
> >    - On R-Car S4 is gated by the RSWITCH Ethernet module clock.
> > 
> >    - On R-Car V4H is gated by the RTSN Ethernet module clock.
> > 
> >    - On R-Car V4M is gated by its own module clock, the system have
> >      neither RTSN or RSWITCH device. But the module clock is the same as
> >      RTSN on V4H and the documentation referees to it as tsn (EtherTSN).
> > 
> > The gPTP device do have its own register space on all three platforms.
> > But on S4 and V4H it will share its clock and reset property with
> > RSWITCH or RTSN, respectively.
> > 
> > Signed-off-by: Niklas Söderlund <niklas.soderlund+renesas@xxxxxxxxxxxx>
> 
> [...]
> 
> > +static int ptp_rcar_gen4_adjfine(struct ptp_clock_info *ptp, long scaled_ppm)
> > +{
> > +	struct ptp_rcar_gen4_priv *priv = ptp_to_priv(ptp);
> > +	s64 addend = priv->default_addend;
> > +	bool neg_adj = scaled_ppm < 0;
> > +	unsigned long flags;
> > +	s64 diff;
> > +
> > +	if (neg_adj)
> > +		scaled_ppm = -scaled_ppm;
> > +	diff = div_s64(addend * scaled_ppm_to_ppb(scaled_ppm), NSEC_PER_SEC);
> > +	addend = neg_adj ? addend - diff : addend + diff;
> > +
> > +	spin_lock_irqsave(&priv->lock, flags);
> > +	iowrite32(addend, priv->base + PTPTIVC0_REG);
> 
> how are you so sure that addend will always fit into s32? It looks like
> it may go over in some cases, no?

Indeed, if the adjustment is more then 32ns per tick it will go over. 
The register is defined as,

        /* Default timer increment in ns.
         * bit[31:27] - integer
         * bit[26:0]  - decimal

I will add a check for this clamping the value before writing it to the 
register.

> 
> > +	spin_unlock_irqrestore(&priv->lock, flags);
> > +
> > +	return 0;
> > +}
> 

-- 
Kind Regards,
Niklas Söderlund