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

[Ivan Vecera <ivecera@redhat.com>]

On 8/5/26 1:07 AM, Vadim Fedorenko wrote:
> On 03/08/2026 15:06, Ivan Vecera wrote:
>> Add low-level DPLL channel operations for ToD read/write/adjust,
>> output phase step, delta frequency offset write and TIE (Time
>> Interval Error) write. These serve as building blocks for the PTP
>> clock callbacks added in the next patch.
>>
>> ToD operations use a wait-before-write pattern to avoid blocking
>> after each operation.
>>
>> The tod_ready_wait helper selects the poll timeout based on the
>> current ToD command - write operations use a longer timeout (1000 ms)
>> than reads (30 ms).
>>
>> The ToD read captures system timestamps (ptp_system_timestamp) around
>> the HW command and completion poll to support cross-timestamping.
>>
>> The TIE write operation provides sub-picosecond resolution phase
>> adjustment for modes where the DPLL is tracking a reference
>> (AUTO and REFLOCK).
>>
>> Add output step-time mask to struct zl3073x_dev and
>> zl3073x_dev_out_is_stepped() helper to check if an output
>> participates in step-time operations.
>>
>> Reviewed-by: Petr Oros <poros@redhat.com>
>> Tested-by: Chris du Quesnay <Chris.duQuesnay@microchip.com>
>> Signed-off-by: Ivan Vecera <ivecera@redhat.com>
>> ---
>>   drivers/dpll/zl3073x/chan.c | 310 +++++++++++++++++++++++++++++++++++-
>>   drivers/dpll/zl3073x/chan.h |  32 ++++
>>   drivers/dpll/zl3073x/core.c |  13 ++
>>   drivers/dpll/zl3073x/core.h |  23 +++
>>   drivers/dpll/zl3073x/regs.h |  52 ++++++
>>   5 files changed, 428 insertions(+), 2 deletions(-)
>>
>> diff --git a/drivers/dpll/zl3073x/chan.c b/drivers/dpll/zl3073x/chan.c
>> index 4ec2cf53dad468..79874a9fdb4962 100644
>> --- a/drivers/dpll/zl3073x/chan.c
>> +++ b/drivers/dpll/zl3073x/chan.c
>> @@ -3,6 +3,7 @@
>>   #include <linux/cleanup.h>
>>   #include <linux/delay.h>
>>   #include <linux/dev_printk.h>
>> +#include <linux/ptp_clock_kernel.h>
>>   #include <linux/string.h>
>>   #include <linux/types.h>
>> @@ -162,8 +163,8 @@ int zl3073x_chan_nco_mode_set(struct zl3073x_dev 
>> *zldev, u8 index)
>>    * @zldev: pointer to zl3073x_dev structure
>>    * @index: DPLL channel index to fetch state for
>>    *
>> - * Reads the mode_refsel register and reference priority registers for
>> - * the given DPLL channel and stores the raw values for later use.
>> + * Reads the mode_refsel, status and reference priority registers for
>> + * the given DPLL channel and stores the values for later use.
>>    *
>>    * Return: 0 on success, <0 on error
>>    */
>> @@ -234,6 +235,311 @@ const struct zl3073x_chan 
>> *zl3073x_chan_state_get(struct zl3073x_dev *zldev,
>>       return &zldev->chan[index];
>>   }
>> +/**
>> + * zl3073x_chan_tod_ready_wait - wait for ToD semaphore to clear
>> + * @zldev: pointer to zl3073x device
>> + * @ch: DPLL channel index
>> + *
>> + * Polls the ToD control register until the semaphore bit is cleared,
>> + * indicating the device has completed the previous ToD operation.
>> + *
>> + * Return: 0 on success, -EBUSY if semaphore not cleared, <0 on error
>> + */
>> +int zl3073x_chan_tod_ready_wait(struct zl3073x_dev *zldev, u8 ch)
>> +{
>> +    unsigned int timeout;
>> +    u8 tod_ctrl;
>> +    int rc;
>> +
>> +    rc = zl3073x_read_u8(zldev, ZL_REG_DPLL_TOD_CTRL(ch), &tod_ctrl);
>> +    if (rc)
>> +        return rc;
>> +
>> +    switch (FIELD_GET(ZL_DPLL_TOD_CTRL_CMD, tod_ctrl)) {
>> +    case ZL_DPLL_TOD_CTRL_CMD_WR_NEXT_1HZ:
>> +        timeout = ZL_POLL_TOD_WR_TIMEOUT_US;
>> +        break;
>> +    default:
>> +        timeout = ZL_POLL_TOD_RD_TIMEOUT_US;
>> +        break;
>> +    }
> 
> there are 3 cmds defined, but FIELD_GET(ZL_DPLL_TOD_CTRL_CMD) can return
> up to 16 possible values. I would explicitly put defined commands in
> cases and make default to ENOTSUPP..

Hi Vadim,
the ZL_DPLL_TOD_CTRL_CMD bits are never filled by firmware (only the
semaphore bit) so the driver knows what it writes. So such check is not
necessary but if you want it I can add something like:

...
     case ZL_DPLL_TOD_CTRL_CMD_RD_CURRENT:
     case ZL_DPLL_TOD_CTRL_CMD_RD_NEXT_1HZ:
         timeout = ZL_POLL_TOD_RD_TIMEOUT_US;
         break;
     default:
         WARN_ON(1); /* this is really unexpected */
         return -ENOTSUPP;
...

Let me know.

Thanks,
Ivan