Re: [PATCH net-next v7 2/3] dpll: zl3073x: add channel ToD, phase step and TIE operations

On 8/13/26 11:21 PM, Vadim Fedorenko wrote:
> On 11/08/2026 14:46, Ivan Vecera wrote:
> 
> [...]
> 
>> + * zl3073x_chan_tod_adjust - atomic ToD read-modify-write with 
>> rollover guard
>> + * @zldev: pointer to zl3073x device
>> + * @ch: DPLL channel index
>> + * @delta: time adjustment to apply
>> + *
>> + * Reads the next-Hz ToD and current ToD, then checks whether enough 
>> time
>> + * remains before the next 1 Hz rollover to safely complete the write.
>> + * Re-reads if the 1 Hz tick crossed between the two reads or if less
>> + * than 20 ms remains before the next rollover. Applies @delta and 
>> writes
>> + * the result back.
> 
> you keep 20ms threshold based on the avg read/write transaction time (I
> assume), but it may happen, that this call will be preempted in between
> reads and write, and 20ms will not be enough before 1Hz rollover. Have
> you though about such option?

Yes, this is a possibility. The 20 ms threshold provides margin for
the write sequence (ready-wait + three register writes + command)
which normally completes in single-digit milliseconds, but a
scheduling delay could consume the remaining margin.

If that happens, the hardware applies the written value at the
following 1 Hz edge instead of the intended one, resulting in a
1 second ToD offset. The PTP servo detects and corrects this in the
next cycle. There is no atomic ToD update command in the hardware,
so this trade-off is inherent to the device's register interface.

I tried to run 1000x times loop with comparing jiffies after ToD read
and after ToD write. The testptp that adjusted ToD has been pinned to
single CPU and parallel stress CPU program on the same CPU during
testing. The average time for write-tod operation was 9ms with maximum
of 10ms.

> 
>> + *
>> + * Context: Caller must serialize all zl3073x_chan_tod_* calls 
>> externally.
>> + * Return: 0 on success, <0 on error
>> + */
>> +int zl3073x_chan_tod_adjust(struct zl3073x_dev *zldev, u8 ch,
>> +                struct timespec64 delta)
>> +{
>> +#define ZL_TOD_MAX_RETRIES    20
>> +    static const long threshold_ns = 20 * NSEC_PER_MSEC;
>> +    struct timespec64 ts_next, ts_cur, diff;
>> +    int rc, i;
>> +
>> +    for (i = 0; i < ZL_TOD_MAX_RETRIES; i++) {
>> +        rc = zl3073x_chan_tod_read(zldev, ch, true, &ts_next, NULL);
>> +        if (rc)
>> +            return rc;
>> +
>> +        rc = zl3073x_chan_tod_read(zldev, ch, false, &ts_cur, NULL);
>> +        if (rc)
>> +            return rc;
>> +
>> +        /* Ensure the 1 Hz tick did not cross between the two reads
>> +         * and that enough margin remains to complete the write.
>> +         */
>> +        diff = timespec64_sub(ts_next, ts_cur);
>> +        if (diff.tv_sec > 0 ||
>> +            (!diff.tv_sec && diff.tv_nsec >= threshold_ns))
>> +            break;
>> +    }
>> +    if (i == ZL_TOD_MAX_RETRIES) {
>> +        dev_warn(zldev->dev,
>> +             "DPLL%u ToD adjust failed to get stable margin\n",
>> +             ch);
>> +        return -EBUSY;
>> +    }
>> +
>> +    /* Apply delta to the next-Hz ToD */
>> +    ts_next = timespec64_add(ts_next, delta);
>> +    if (!timespec64_valid(&ts_next))
> 
> maybe timespec64_valid_settod() will be better as the code is about
> modifying ToD?

Good point. Will switch to timespec64_valid_settod() in v8.

Ivan