This series adds a DPLL subsystem driver for the SiTime SiT95316 and
SiT95317 I2C clock generators. Each device integrates four PLLs with
automatic reference selection and on-chip TDC phase-offset measurement,
and is used for synchronization in telecom, networking, and data-center
timing.

The series contains the device-tree binding, the driver under
drivers/dpll/sit9531x/, and the MAINTAINERS entry.

v1: https://lore.kernel.org/netdev/20260511211143.19792-1-arouhi@sitime.com/
v2: https://lore.kernel.org/netdev/20260520191943.73938-1-arouhi@sitime.com/
v3: https://lore.kernel.org/netdev/20260731180951.65725-1-arouhi@sitime.com/
v4: https://lore.kernel.org/netdev/20260806232439.27551-1-arouhi@sitime.com/
v5: https://lore.kernel.org/netdev/20260810230439.22866-1-arouhi@sitime.com/
v6: https://lore.kernel.org/netdev/20260812175337.18155-1-arouhi@sitime.com/
v7: https://lore.kernel.org/netdev/20260815221919.64226-1-arouhi@sitime.com/

The driver was a single 5.6k-line patch through v7. Vadim and Ivan asked
for it to be split into logical parts, with the first zl3073x series as
the model, so v8 introduces it feature by feature instead:

  1-2   bindings: vendor prefix, then the device schema
  3     basic support: paged regmap, variant detection, probe
  4     DPLL types and pin properties from system firmware
  5     register the DPLL devices and pins, and keep their state
  6     input pin state on a DPLL, with the pin-state contract
  7     input pin priority
  8     pin frequency, both directions
  9     output pin state (mute)
  10    output phase adjust
  11    esync on outputs
  12    phase offset through the TDC
  13    fractional frequency offset of the selected reference
  14    the inter-PLL sync net as a pair of pins
  15    optional device-tree overrides for two board facts

The two bindings patches come first, so the driver never matches on a
compatible string before the schema that describes it is in the tree.

Each of the thirteen driver patches was built on its own, with W=1 on
x86_64 and i386, and is sparse clean. Each one also links: no patch
calls something that a later patch introduces, so the module builds at
every step of the series and a bisect cannot land on a tree that fails
to compile.

The split makes no functional change of its own. The tree this series
ends at compiles to objects byte-identical to the tree the single patch
produced once the fixes below are applied to it.

Changes in v8:

  - Split the driver patch into the thirteen patches listed above.

  - Advertise automatic mode only. v7 mapped DPLL_MODE_MANUAL onto the
    chip's free-run bit and then refused DPLL_PIN_STATE_CONNECTED on
    input pins, so it offered the mode whose one purpose is manual
    selection and refused the only request that mode exists for.
    Free-run selects no input at all, so on this device it is a lock
    state, not a selection mode. v8 advertises AUTOMATIC only and
    refuses anything else from .mode_set(), which now clears the outer
    loop disable bit so a PLL left free-running by the loaded
    configuration returns to selecting from the priority table.
    Free-run is reported through lock status as UNLOCKED.

    The device can also pin one reference while the loop keeps running,
    so real MANUAL support is possible later. It is left out here
    because the mode mapping becomes ABI once merged, and it belongs in
    the same series as the userspace that needs it rather than being
    approximated now.

  - State the pin-state contract once, above the pin ops, and let the
    two selection-role pins share one implementation of it. The driver
    had five ops tables and five hand-written encodings of the same
    state machine, and every review round has found another place where
    two of them disagreed. This closes several findings at once:
    CONNECTED reported while the outer loop was disabled, membership
    taken from the shared-receiver refcount rather than from the
    priority table, SELECTABLE on an INTSYNC destination userspace had
    disconnected, .prio_set() inserting a source that was never in the
    table, and .prio_remove() returning 0 when the disconnect would
    have emptied it. The last two are now -EINVAL and -EBUSY.

  - Report LOCKED_HO_ACQ only once the holdover estimate is valid. It
    was asserted whenever the loop was locked, without reading the bit
    that says the device has accumulated a frequency estimate good
    enough to fall back on. That bit is distinct from the one that says
    the PLL has already switched over to holdover.

  - Fix the 32-bit build. The output phase adjust reduced a negative
    request modulo the output period with % on two u64 values, leaving
    the module with an undefined __umoddi3; it now uses div64_u64_rem(),
    which the rest of the arithmetic there already uses. Reported by
    Jakub Kicinski.

  - Correct the register model comment: the page selector is at 0xFF and
    a page holds 256 registers, as the definitions right below the
    comment already said. Reported by Ivan Vecera.

  - Type ordinary clock outputs as DPLL_PIN_TYPE_EXT instead of
    DPLL_PIN_TYPE_GNSS, which claimed a GNSS-disciplined timing port.
    Advertise a phase-adjust range on outputs only, and drop the input
    pins' .phase_adjust_get with it: inputs have no .phase_adjust_set,
    so a non-zero range promised something every set would refuse.

  - Always issue the loop lock in the programming commit, so a transient
    I2C error cannot leave the chip in PRG_CMD with its loops open. Undo
    the global enable when the INTSYNC enable fails partway. Take
    multiop_lock before testing the selected reference in the frequency
    offset getter, as the sibling phase-offset getter already does.

  - Latch every read of the chip's debug block the way the phase read
    does. That block returns a sample of an internal measurement tap,
    selected by a read code and latched by reading a trigger register.
    The generic helper read the trigger once where the phase path reads
    it three times, and a single read returns the previous latch. On
    hardware this showed as a frequency offset that would not move while
    a phase measurement of the same PLL tracked a temperature step
    cleanly.

  - Report which PLLs came up and which EEPROM profile was loaded, and
    warn when a PLL drives outputs without having reached its active
    state, or when the profile's stored and computed CRCs disagree. In
    both cases every register still reads back a plausible value, so
    nothing about the failure is visible until the measurements come out
    wrong.

  - Say what the phase offset getter measures, next to the callback: the
    loop's residual error sampled with the loop closed, which is the
    quantity the ABI asks for -- not the vendor's open-loop
    phase-difference procedure, which would need the digital loop filter
    frozen and would leave the PLL undisciplined until something
    released it. A netlink read must not do that.

  - Drop fifteen register definitions that have never had a caller.
    Eleven describe that open-loop sequence and the PLL restart path,
    neither of which the driver performs. The other four -- the PLL page
    macro and the three DIVN2 fields -- had no user in any patch of the
    split, so there was no honest patch to introduce them in. Each will
    come back with its first user.

  - Binding: put the example VCO frequency inside a band. 6251500000 sat
    in the gap between the device's two bands, which the property's own
    description rules out; it is now 6900000000, which a shipping
    profile on our SiT95316 board runs. This is the only change to the
    binding since v6 -- the property and its description are unchanged
    -- so Krzysztof's Reviewed-by is carried across it.

The AI review's use-after-free report on the pin properties remains a
false positive: dpll_pin_alloc() calls dpll_pin_prop_dup(), which copies
freq_supported with kmemdup() and the labels with kstrdup(), so the core
owns its copies and the driver has to free its own.

Ali Rouhi (2):
  dt-bindings: vendor-prefixes: add SiTime Corporation
  dt-bindings: dpll: add SiTime SiT95316 clock generator

Oleg Zadorozhnyi (13):
  dpll: add basic SiTime SiT9531x support
  dpll: sit9531x: read DPLL types and pin properties from system
    firmware
  dpll: sit9531x: register DPLL devices and pins
  dpll: sit9531x: implement input pin state on a DPLL
  dpll: sit9531x: add support to get and set priority on input pins
  dpll: sit9531x: add support to get and set frequency on pins
  dpll: sit9531x: implement output pin state on a DPLL
  dpll: sit9531x: add support to adjust output phase
  dpll: sit9531x: add support to get and set esync on pins
  dpll: sit9531x: add support to get phase offset on the connected input
    pin
  dpll: sit9531x: add support to get fractional frequency offset
  dpll: sit9531x: model the inter-PLL sync net as a pair of pins
  dpll: sit9531x: allow the device tree to override two board facts

 .../bindings/dpll/sitime,sit95316.yaml        |  171 +
 .../devicetree/bindings/vendor-prefixes.yaml  |    2 +
 MAINTAINERS                                   |    7 +
 drivers/dpll/Kconfig                          |    2 +
 drivers/dpll/Makefile                         |    1 +
 drivers/dpll/sit9531x/Kconfig                 |   17 +
 drivers/dpll/sit9531x/Makefile                |    4 +
 drivers/dpll/sit9531x/core.c                  | 3317 +++++++++++++++++
 drivers/dpll/sit9531x/core.h                  |  381 ++
 drivers/dpll/sit9531x/dpll.c                  | 1335 +++++++
 drivers/dpll/sit9531x/dpll.h                  |   69 +
 drivers/dpll/sit9531x/prop.c                  |  406 ++
 drivers/dpll/sit9531x/prop.h                  |   39 +
 drivers/dpll/sit9531x/regs.h                  |  377 ++
 14 files changed, 6128 insertions(+)
 create mode 100644 Documentation/devicetree/bindings/dpll/sitime,sit95316.yaml
 create mode 100644 drivers/dpll/sit9531x/Kconfig
 create mode 100644 drivers/dpll/sit9531x/Makefile
 create mode 100644 drivers/dpll/sit9531x/core.c
 create mode 100644 drivers/dpll/sit9531x/core.h
 create mode 100644 drivers/dpll/sit9531x/dpll.c
 create mode 100644 drivers/dpll/sit9531x/dpll.h
 create mode 100644 drivers/dpll/sit9531x/prop.c
 create mode 100644 drivers/dpll/sit9531x/prop.h
 create mode 100644 drivers/dpll/sit9531x/regs.h


base-commit: d0ec95a8a4e79f2fd6063fc8932415db8c227689
-- 
2.43.0

On 9/2/26 11:40 PM, Ali Rouhi wrote:
> This series adds a DPLL subsystem driver for the SiTime SiT95316 and
> SiT95317 I2C clock generators. Each device integrates four PLLs with
> automatic reference selection and on-chip TDC phase-offset measurement,
> and is used for synchronization in telecom, networking, and data-center
> timing.
> 
> The series contains the device-tree binding, the driver under
> drivers/dpll/sit9531x/, and the MAINTAINERS entry.
> 
> v1: https://lore.kernel.org/netdev/20260511211143.19792-1-arouhi@sitime.com/
> v2: https://lore.kernel.org/netdev/20260520191943.73938-1-arouhi@sitime.com/
> v3: https://lore.kernel.org/netdev/20260731180951.65725-1-arouhi@sitime.com/
> v4: https://lore.kernel.org/netdev/20260806232439.27551-1-arouhi@sitime.com/
> v5: https://lore.kernel.org/netdev/20260810230439.22866-1-arouhi@sitime.com/
> v6: https://lore.kernel.org/netdev/20260812175337.18155-1-arouhi@sitime.com/
> v7: https://lore.kernel.org/netdev/20260815221919.64226-1-arouhi@sitime.com/
> 
> The driver was a single 5.6k-line patch through v7. Vadim and Ivan asked
> for it to be split into logical parts, with the first zl3073x series as
> the model, so v8 introduces it feature by feature instead:
> 
>   1-2   bindings: vendor prefix, then the device schema
>   3     basic support: paged regmap, variant detection, probe
>   4     DPLL types and pin properties from system firmware
>   5     register the DPLL devices and pins, and keep their state
>   6     input pin state on a DPLL, with the pin-state contract
>   7     input pin priority
>   8     pin frequency, both directions
>   9     output pin state (mute)
>   10    output phase adjust
>   11    esync on outputs
>   12    phase offset through the TDC
>   13    fractional frequency offset of the selected reference
>   14    the inter-PLL sync net as a pair of pins
>   15    optional device-tree overrides for two board facts
> 
> The two bindings patches come first, so the driver never matches on a
> compatible string before the schema that describes it is in the tree.
> 
> Each of the thirteen driver patches was built on its own, with W=1 on
> x86_64 and i386, and is sparse clean. Each one also links: no patch
> calls something that a later patch introduces, so the module builds at
> every step of the series and a bisect cannot land on a tree that fails
> to compile.
> 
> The split makes no functional change of its own. The tree this series
> ends at compiles to objects byte-identical to the tree the single patch
> produced once the fixes below are applied to it.
> 
> Changes in v8:
> 
>   - Split the driver patch into the thirteen patches listed above.
> 
>   - Advertise automatic mode only. v7 mapped DPLL_MODE_MANUAL onto the
>     chip's free-run bit and then refused DPLL_PIN_STATE_CONNECTED on
>     input pins, so it offered the mode whose one purpose is manual
>     selection and refused the only request that mode exists for.
>     Free-run selects no input at all, so on this device it is a lock
>     state, not a selection mode. v8 advertises AUTOMATIC only and
>     refuses anything else from .mode_set(), which now clears the outer
>     loop disable bit so a PLL left free-running by the loaded
>     configuration returns to selecting from the priority table.
>     Free-run is reported through lock status as UNLOCKED.
> 
>     The device can also pin one reference while the loop keeps running,
>     so real MANUAL support is possible later. It is left out here
>     because the mode mapping becomes ABI once merged, and it belongs in
>     the same series as the userspace that needs it rather than being
>     approximated now.
> 
>   - State the pin-state contract once, above the pin ops, and let the
>     two selection-role pins share one implementation of it. The driver
>     had five ops tables and five hand-written encodings of the same
>     state machine, and every review round has found another place where
>     two of them disagreed. This closes several findings at once:
>     CONNECTED reported while the outer loop was disabled, membership
>     taken from the shared-receiver refcount rather than from the
>     priority table, SELECTABLE on an INTSYNC destination userspace had
>     disconnected, .prio_set() inserting a source that was never in the
>     table, and .prio_remove() returning 0 when the disconnect would
>     have emptied it. The last two are now -EINVAL and -EBUSY.
> 
>   - Report LOCKED_HO_ACQ only once the holdover estimate is valid. It
>     was asserted whenever the loop was locked, without reading the bit
>     that says the device has accumulated a frequency estimate good
>     enough to fall back on. That bit is distinct from the one that says
>     the PLL has already switched over to holdover.
> 
>   - Fix the 32-bit build. The output phase adjust reduced a negative
>     request modulo the output period with % on two u64 values, leaving
>     the module with an undefined __umoddi3; it now uses div64_u64_rem(),
>     which the rest of the arithmetic there already uses. Reported by
>     Jakub Kicinski.
> 
>   - Correct the register model comment: the page selector is at 0xFF and
>     a page holds 256 registers, as the definitions right below the
>     comment already said. Reported by Ivan Vecera.
> 
>   - Type ordinary clock outputs as DPLL_PIN_TYPE_EXT instead of
>     DPLL_PIN_TYPE_GNSS, which claimed a GNSS-disciplined timing port.
>     Advertise a phase-adjust range on outputs only, and drop the input
>     pins' .phase_adjust_get with it: inputs have no .phase_adjust_set,
>     so a non-zero range promised something every set would refuse.
> 
>   - Always issue the loop lock in the programming commit, so a transient
>     I2C error cannot leave the chip in PRG_CMD with its loops open. Undo
>     the global enable when the INTSYNC enable fails partway. Take
>     multiop_lock before testing the selected reference in the frequency
>     offset getter, as the sibling phase-offset getter already does.
> 
>   - Latch every read of the chip's debug block the way the phase read
>     does. That block returns a sample of an internal measurement tap,
>     selected by a read code and latched by reading a trigger register.
>     The generic helper read the trigger once where the phase path reads
>     it three times, and a single read returns the previous latch. On
>     hardware this showed as a frequency offset that would not move while
>     a phase measurement of the same PLL tracked a temperature step
>     cleanly.
> 
>   - Report which PLLs came up and which EEPROM profile was loaded, and
>     warn when a PLL drives outputs without having reached its active
>     state, or when the profile's stored and computed CRCs disagree. In
>     both cases every register still reads back a plausible value, so
>     nothing about the failure is visible until the measurements come out
>     wrong.
> 
>   - Say what the phase offset getter measures, next to the callback: the
>     loop's residual error sampled with the loop closed, which is the
>     quantity the ABI asks for -- not the vendor's open-loop
>     phase-difference procedure, which would need the digital loop filter
>     frozen and would leave the PLL undisciplined until something
>     released it. A netlink read must not do that.
> 
>   - Drop fifteen register definitions that have never had a caller.
>     Eleven describe that open-loop sequence and the PLL restart path,
>     neither of which the driver performs. The other four -- the PLL page
>     macro and the three DIVN2 fields -- had no user in any patch of the
>     split, so there was no honest patch to introduce them in. Each will
>     come back with its first user.
> 
>   - Binding: put the example VCO frequency inside a band. 6251500000 sat
>     in the gap between the device's two bands, which the property's own
>     description rules out; it is now 6900000000, which a shipping
>     profile on our SiT95316 board runs. This is the only change to the
>     binding since v6 -- the property and its description are unchanged
>     -- so Krzysztof's Reviewed-by is carried across it.
> 
> The AI review's use-after-free report on the pin properties remains a
> false positive: dpll_pin_alloc() calls dpll_pin_prop_dup(), which copies
> freq_supported with kmemdup() and the labels with kstrdup(), so the core
> owns its copies and the driver has to free its own.
Clashiko had several comments, and some of them definitely look real.
Please note that you are supposed to handle such comments proactively,
see commit c82ff94592fb68f529afe63ca7f5ddb7dae4ba83

/P

> Clashiko had several comments, and some of them definitely look real.
> Please note that you are supposed to handle such comments proactively,
> see commit c82ff94592fb68f529afe63ca7f5ddb7dae4ba83

Thanks for the pointer.  We have gone through all fourteen per-patch reviews
and replied to each finding individually in the corresponding sub-threads:
the majority are fixed in the upcoming v9, a few are answered with the design
rationale, and one review suggestion (renaming a vendor property to a "-hz"
suffix) turned out to be rejected by dt_binding_check itself, which the reply
on patch 02 documents.  v9 with all of the fixes and a full changelog follows.

On the second half of that commit -- running LLM reviews in advance of the
post -- you are right that we had not been doing it, and a 15-patch series
drawing 86 findings is exactly the case the paragraph warns about.  We have
since set up Sashiko locally and now run it before posting rather than
after.  v9 has been through it twice over the whole series, and what it
found is folded into the patches rather than left for the bots to raise
again.  We intend to keep doing this for any further revision.