Thread (6 messages) flat view 6 messages, 1 author, 1d ago

Re: [PATCH net-next v8 3/3] dpll: zl3073x: add PTP clock support

From: Ivan Vecera <ivecera@redhat.com>
Date: 2026-08-16 14:53:17
Also in: lkml

Sashiko comments with my replies:

 > Should PTP_1588_CLOCK be a hard dependency here?

Yes. This was explicitly requested by Jakub in the v2 review [1].

[1] https://lore.kernel.org/netdev/20260722135848.2d401ada@kernel.org/ (local)

 > Does settime64() need to compensate for the deferred ToD load?
 > [...] does this leave the clock up to a second behind the requested
 > time on every clock_settime()?

settime64 is a coarse "slam the clock" operation. gettimex64() does
not return stale values — it calls tod_ready_wait() which blocks
until the pending WR_NEXT_1HZ completes. The PTP servo handles any
residual offset in subsequent adjtime calls.

 > Also, ts.tv_nsec is written raw. If a caller passes a non-zero
 > tv_nsec, does the ToD second boundary end up displaced from the
 > DPLL 1 Hz / output 1PPS phase?

Yes, a non-zero tv_nsec displaces the ToD second boundary from the
1PPS edge. This matches the hardware behavior — settime64 sets
the ToD counter to whatever value is requested. Phase coherence
between ToD and outputs is maintained by adjtime which routes
sub-second adjustments through phase step or TIE write.

 > Is this error return reachable after the seconds are already
 > committed? [...] Does the caller then get a failure for an
 > adjustment whose seconds part is already applied?

If tod_ready_wait fails, we cannot be certain that WR_NEXT_1HZ was
actually applied. Returning the error is the safer choice — masking
it would silently report success when the adjustment may not have
been applied at all.

 > Does the same pending WR_NEXT_1HZ hazard exist across callback
 > invocations? [...]
 > Would calling zl3073x_chan_tod_ready_wait() unconditionally before
 > the phase step / TIE write cover this?

Phase step and TIE write operate on independent hardware blocks
(output phase adjustment and DPLL input phase respectively) and do
not conflict with a pending ToD write. Any subsequent ToD operation
goes through tod_ready_wait which waits for the pending write to
complete. The cross-callback sequence described (settime64 followed
immediately by adjphase) does not occur in practice — these are
fundamentally different operations used at different stages of PTP
clock management.

 > zl3073x_dpll_alloc() and zl3073x_dpll_free() are moved verbatim
 > [...] Could it be dropped or split into a separate
 > no-functional-change patch?

The move is needed because the new PTP registration helper
zl3073x_dpll_ptp_register() must be defined before
zl3073x_dpll_alloc() which references ptp_info. Splitting this
into a separate patch was done in v3 at Jakub's request but was
squashed back when the refactored helper was dropped along with
perout support.

Thanks,
Ivan
Keyboard shortcuts
hback out one level
jnext message in thread
kprevious message in thread
ldrill in
Escclose help / fold thread tree
?toggle this help