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