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

On 12/08/2026 12:00, Ivan Vecera wrote:

On 8/12/26 12:04 PM, Vadim Fedorenko wrote:

quoted

On 12/08/2026 07:57, Ivan Vecera wrote:

quoted

Sashiko findings. Replies inline.

 > Could a transient hardware error bring down the system here?
 >
 > If an I2C/SPI bus glitch causes the device to return 0xFF, the SEM
 > bit will be set and the CMD field will hit this default case. On
 > systems with panic_on_warn, using WARN_ON for validating external
 > hardware states turns recoverable bus errors into fatal kernel 
panics.

The SEM-first check already handles the most common bus glitch (0x00
return). For 0xFF: the CMD field is only written by the driver, never
by firmware, so an unknown CMD with SEM set indicates either a bus
error or firmware misbehavior that warrants attention. The switch
structure with WARN_ON in the default case was requested by Vadim
in his v4 review. Systems that enable panic_on_warn accept this
trade-off.

 > Will this sleep-based polling loop destroy the timestamp's precision?
 >
 > Should the postts be captured immediately after the trigger command
 > in zl3073x_chan_tod_ctrl() instead?

The hardware latches the ToD value when it processes the command,
which completes when the semaphore clears. The post-timestamp must
be taken after the semaphore clears to guarantee the window contains
the actual latch event. Moving it before the wait would risk the
timestamp window not containing the latch moment.

 > Could this loop exhaust its retries and return -EBUSY prematurely?
 >
 > The loop spins without an explicit wait [...] On fast SPI/I2C buses,
 > it will execute all 20 reads in a few milliseconds

Testing on I2C at both 100 kHz and 400 kHz bus speeds shows that a
single iteration of the loop body (two ToD reads, each involving a
ready-wait, command write, second ready-wait and data reads) takes
approximately 17-19 ms regardless of bus speed. The iteration time
is dominated by the device's internal processing, not bus transfer
time. With 20 retries the budget is 340-380 ms, well beyond the
20 ms margin window.

 > Is it safe to use WARN_ON to validate user-controlled input?
 >
 > Since delta_ns originates from the clock_adjtime syscall's tx.offset
 > (via the adjphase PTP callback) [...]

The PTP core already validates the input via getmaxphase, which
returns NSEC_PER_SEC - 1, rejecting values with magnitude >=
NSEC_PER_SEC before the driver callback is invoked. The WARN_ON is
a defensive check for a condition that should never be reached
through normal code paths, not user input validation.

AFAIR, the general rule is not to write defensive code in kernel if 
you know that the core has already validated inputs.

Yes, but this low-level helper zl3073x_chan_tie_write() is called from
multiple places and current code-paths are OK. But in future, if another
caller will be introduced or existing code will be refactored this WARN
immediately detects potential bug.

The same is also valid for WARN in zl3073x_chan_tod_ready_wait()... new
TOD command starts to be used but someone forget to update this function
accordingly.

That's a little bit weak and goes against "trust internal APIs". It's
currently called from adjtime and adjphase callbacks, what do you expect
to have in the future?