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

On 12/08/2026 07:57, Ivan Vecera wrote:
> 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.

> 
> Thanks,
> Ivan
>