mirror of https://lore.kernel.org/lkml/
 help / color / mirror / Atom feed
* [PATCH net-next v11 00/13] dpll: add SiTime SiT9531x DPLL clock driver
@ 2026-09-30 23:37 Ali Rouhi
  2026-09-30 23:37 ` [PATCH net-next v11 01/13] dt-bindings: dpll: allow hex unit addresses on output pins Ali Rouhi
                   ` (12 more replies)
  0 siblings, 13 replies; 26+ messages in thread
From: Ali Rouhi @ 2026-09-30 23:37 UTC (permalink / raw)
  To: jiri
  Cc: vadim.fedorenko, arkadiusz.kubalewski, ivecera, kuba, pabeni,
	robh, krzk+dt, conor+dt, cjubran, Oleg.Zadorozhnyi, devicetree,
	netdev, linux-kernel

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/
v8: https://lore.kernel.org/netdev/20260902214030.20955-1-arouhi@sitime.com/
v9: https://lore.kernel.org/netdev/20260915000015.80480-1-arouhi@sitime.com/
v10: https://lore.kernel.org/netdev/20260921201108.42676-1-arouhi@sitime.com/

v10 was set to Changes Requested on the review of its binding patch, and
we asked for it to be dropped from patchwork rather than restored. This
is the promised replacement.

The review of v10 raised 82 points across the thirteen patches it
covered. Seventy-five are fixed here, five are addressed in part and
answered on the thread, one patch is dropped outright, and one point was
made moot by a change made for a different reason. Two of the findings
were real bugs; both are described below.

  1     bindings: allow hex unit addresses on DPLL output pins
  2     bindings: vendor prefix
  3     bindings: the device schema
  4     basic support: paged regmap, variant detection, probe
  5     DPLL types and pin properties from system firmware
  6     register the DPLL devices and pins, and keep their state
  7     input pin state and operational state on a DPLL
  8     input pin priority
  9     pin frequency, both directions
  10    output pin state (mute)
  11    output phase adjust
  12    phase offset through the TDC
  13    the inter-PLL sync net as a pair of pins

The three 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 ten driver patches builds and links on its own: no patch
calls something a later patch introduces, so a bisect cannot land on a
tree that fails to compile. That was re-checked for this posting patch
by patch with W=1 and with sparse. checkpatch --strict is clean except
for the "does MAINTAINERS need updating?" hint on patches 5 and 6, which
add files under drivers/dpll/sit9531x/ -- patch 4 already covers that
directory with an F: entry.

Five things are worth reading before the changelog.

The first is a change in what DPLL_A_PIN_STATE means in this driver,
and it is the structural change in v11.

Until v10 the input pin state reported what the device was doing: a pin
read back CONNECTED when the hardware had selected it. dpll.rst says
PIN_STATE is administrative intent and PIN_OPERSTATE is what the
hardware is doing, and the review was right that we had merged the two.
State now answers what userspace asked for -- SELECTABLE when the source
is in this PLL's priority table, DISCONNECTED when it is not -- and a
new .operstate_on_dpll_get on the physical inputs and on the inter-PLL
sync destination reports ACTIVE, NO_SIGNAL, QUAL_FAILED or STANDBY.

The active predicate deliberately requires more than the selection
register. It requires the loop to be locked, because that register holds
what the driver last wrote or the device last chose and not proof the
loop is using it; a free-running, frozen or unlocked PLL follows
nothing. And it requires the named lane to have signal.

That second condition carries a limitation worth stating plainly. When
the device fails over on its own to another source in the table, the
registers this driver reads do not name the source it moved to: the
selection register still names the lane that died. We therefore report
no pin as active rather than report the dead one, so on an autonomous
failover userspace gets notification that something changed but not
identity. That is a limitation of the driver rather than of the part --
there are status registers that report the reference a PLL is actually
locked to, and reading those is work for a later version.

That register is now written as well as read. Until v10 the driver
rebuilt a PLL's priority table and left the device's active selection
alone, so a table write could leave the selection naming a source the
table no longer listed, or one that had lost signal, and the PLL
pointing at a reference that had just been disconnected. v11 picks the
selection alongside the table, following what automatic mode is defined
to do -- the highest-priority input with signal -- with one exception:
when the write only reorders sources below the one in use, the
selection stays, so a change low in the table cannot pull a PLL off a
healthy reference.

We believe this is what was behind a re-selection failure Carolina
Jubran reported against v9 and again against v10, where a DPLL kept
following an input removed from its priority table until that input was
removed from every DPLL on the device. The inputs are shared receivers,
so removing one everywhere powers it down; the device moved then, but
not on the table write alone.

The second is the fractional frequency offset patch, which is dropped
and not replaced.

Patch 12 of v10 advertised BIT(DPLL_FFO_PIN_DEVICE) on every input pin
and published (running - configured) / configured. The review pointed
out that this is not the quantity the uAPI defines for that attribute.
In the pin-parent-device nest the attribute is the offset between the
pin and its parent DPLL device; the offset of the device's own output
from nominal belongs to PIN_TYPE_INT_NCO, and zl3073x follows that
split. What we published was the offset of the loop from what the
configuration asked for, measured against the local XO -- a real
quantity, but a different one. Two drivers answering the same netlink
read with different physical quantities is precisely what the attribute
exists to prevent, so the patch is withdrawn rather than argued. The
finding was correct on the ABI and correct about the code. Offering the
right quantity means measuring the reference against the DPLL rather
than against the XO, which this part exposes through the TDC and not
through the dividers; that is future work, not a v11 fix.

Nothing in v11 advertises DPLL_FFO_PIN_DEVICE.

The third is that patch 14 of v10 is dropped with both of the
device-tree properties it carried, which is the other reason the series
is shorter.

sitime,output-pll-map is gone because the routing it described is
discoverable. Each PLL page holds OUT_MAP_LO/OUT_MAP_HI naming the slots
that PLL drives, the driver already reads them, and it never writes
them, so the routing is fixed by the configuration loaded from NVM at
boot and no DPLL call changes it. The property was kept on the strength
of a claim that those bitmaps can be ambiguous; they are not, and device
tree is for what firmware cannot discover.

sitime,pll-fvco is gone because the driver derives the VCO frequency
from the registers, and that derivation reproduces every rate these
parts are configured for. Keeping a property to override a value the
device already answers for is the same mistake in a different place.

The fourth is two driver fixes that the removal of sitime,output-pll-map
made necessary, and which the DT override had been masking.

Once the output-to-PLL association comes from OUT_MAP_LO/OUT_MAP_HI, the
bit order of those bitmaps matters, and on PLLC and PLLD it is reversed
relative to PLLA and PLLB. Our register map defines OUTPUT_ENABLE_PLLx
identically on all four pages and does not say which bit is which
output, which is why the driver had assumed the natural order on all
four. The map is being corrected separately. The driver now indexes the
bit accordingly, and it accumulates the claims from all four PLLs
instead of stopping at the first match, so a slot claimed twice is
warned about rather than silently taken by whichever page was read
first.

The second is the forced Hi-Z path, which was asserting the override on
the CLKP leg and on the master output power-up but not on CLKN. For a
differential output that leaves one leg still driven by a mute the
caller was told had succeeded. Both legs are now forced.

The fifth is that a PLL the loaded configuration builds without the
phase-flush feature has nothing to fire after a frequency or delay
change, and was previously left with its output dividers counting from
wherever they were. Such a PLL is now restarted instead, which restarts
its dividers from the PLL phase, as the documented procedure does.

Changes in v11:

  - Pin state and operational state are split, as described above. The
    input pin state callbacks answer from the driver's own priority
    model; the new operstate callbacks answer from the device.

  - Priority is kept by the driver per source and per PLL, independent
    of whether the source is currently in the table. Setting one input's
    priority no longer perturbs any other input's, and a pin reports the
    same priority whether it is connected or not. The hardware table is
    rebuilt from those priorities: members are ordered by configured
    priority, ties broken by the order the table already held, packed
    from slot 0, and the remaining slots filled with the code for no
    source. That also guarantees each source occupies exactly one slot.
    Four separate findings about the old free-slot search dissolved with
    it.

  - The device's active selection is written with the table rather than
    left alone, so a rebuilt table can no longer leave a PLL named onto
    a source that is gone or dead. Described above.

  - The inter-PLL sync disable path had a real bug: on full success it
    fell through into the block that restores the global enable bit,
    re-asserting the net it had just torn down while returning success.
    The success path now skips the restore, which is reached only from
    the error gotos.

  - The Hi-Z rollback had a real bug: it always cleared the override bit
    and never saved what the slot held on entry, so unwinding a failed
    request could un-mute a pad that was already muted, by an earlier
    request or by the loaded profile. The write now reads both the state
    and the override first and restores them in the reverse of the order
    they went on. The mute itself is now ordered value first and override
    second, so enabling the override while the state bit still holds what
    the profile left there cannot pin the pad driven for the width of an
    I2C transfer. A rollback that fails is logged as leaving the override
    half applied.

  - Binding. A new first patch widens dpll-device.yaml's output-pins
    pattern from ^pin@[0-9]+$ to ^pin@[0-9a-f]+$, so a device with more
    than ten outputs can describe the rest; it carries a Fixes: tag. The
    device schema now bounds pin reg per variant, so dt_binding_check
    rejects an output node the SiT95317 does not bond out, and the
    example exercises the widened pattern with a pin@a. clocks now
    depends on clock-names, closing a hole where a node could carry both
    a clock phandle and clock-frequency. The argument about dtschema
    types moved out of a property description and into the commit
    message, where writing-bindings.rst wants it.

  - The one remaining read-modify-write accessor that could leave a
    stale cached page selector behind now drops the cache and logs, like
    the single-register accessors.

  - Manual-selection reporting covers GPIO_INPUT_FUNC_CTRL5 through 8 as
    well, so a reference pinned through any of those inputs is reported
    rather than half of them.

  - Comment and kernel-doc corrections throughout, including several
    where the text no longer described the code: the divider guard that
    still referred to a band clamp that no longer exists, the two mute
    comments that disagreed on what a mute does to the pad, and a
    kernel-doc field description that named the wrong slot.

  - Tags. Patch 2 keeps Conor's Acked-by. Patch 3 has changed again, so
    Krzysztof's Reviewed-by is still not carried across it.

Two rounds of this series were shaped by bench reports from Carolina
Jubran. The probe path that accepts a clock-frequency property when
firmware exposes no oscillator through the clock framework, now patch 4,
came from her report against v8; the input-handling work that went into
v10 -- emptying the priority table when the last reference is
disconnected, giving a meaning to every slot code including the input
pair this part does not have, and reporting connected as selected rather
than locked -- came from her report against v9. Both came by private
mail. v10 is being dropped rather than applied, so the credit is
repeated here.

Three items from earlier rounds are unchanged and are repeated here so
they are not re-raised.

The phase-adjust granularity stays at 1 ps rather than the 30 ps fine
step. The delays this device can reach are not a lattice of 30 ps: a
request is split between a coarse delay counted in VCO cycles and a
three-bit fine field of 30 ps steps, and the two are added, so the
spacing depends on the VCO period in force. Advertising 30 would name a
step the device does not have. A request is accepted at 1 ps and rounded
to the nearest delay the registers can hold, and the getter reports what
they hold rather than what was asked for, so a caller that needs the
exact figure reads it back.

A frequency request of 0 Hz is still refused with -EINVAL rather than
treated as a request to stop the output. Nothing in the ABI says zero
means off, and this device already has a mute control that says so
explicitly.

The u64 truncation in dpll_pin_freq_set() is still there and is still
not ours to fix in this series: the requested frequency is read as a u64
and validated through a helper that takes a u32, so a rate of U32_MAX +
1 + N is accepted as N against ranges that are themselves u64. That
affects every driver behind the interface. It will be posted as its own
patch against the core rather than buried here; this driver range-checks
its own input in the meantime.

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

Oleg Zadorozhnyi (11):
  dt-bindings: dpll: allow hex unit addresses on output pins
  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 phase offset on the connected input
    pin
  dpll: sit9531x: model the inter-PLL sync net as a pair of pins

 .../devicetree/bindings/dpll/dpll-device.yaml |    2 +-
 .../bindings/dpll/sitime,sit95316.yaml        |  175 +
 .../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                  | 4887 +++++++++++++++++
 drivers/dpll/sit9531x/core.h                  |  434 ++
 drivers/dpll/sit9531x/dpll.c                  | 1448 +++++
 drivers/dpll/sit9531x/dpll.h                  |   69 +
 drivers/dpll/sit9531x/prop.c                  |  469 ++
 drivers/dpll/sit9531x/prop.h                  |   37 +
 drivers/dpll/sit9531x/regs.h                  |  412 ++
 15 files changed, 7965 insertions(+), 1 deletion(-)
 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: 4bb9710c6a68d35207f123aef55dcd50e7195ec5
-- 
2.43.0


^ permalink raw reply	[flat|nested] 26+ messages in thread
end of thread, other threads:[~2026-10-05  1:10 UTC | newest]

Thread overview: 26+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2026-09-30 23:37 [PATCH net-next v11 00/13] dpll: add SiTime SiT9531x DPLL clock driver Ali Rouhi
2026-09-30 23:37 ` [PATCH net-next v11 01/13] dt-bindings: dpll: allow hex unit addresses on output pins Ali Rouhi
2026-10-02  8:32   ` Krzysztof Kozlowski
2026-09-30 23:37 ` [PATCH net-next v11 03/13] dt-bindings: dpll: add SiTime SiT95316 clock generator Ali Rouhi
2026-10-05  1:09   ` netdev-bot+sashiko
2026-09-30 23:37 ` [PATCH net-next v11 02/13] dt-bindings: vendor-prefixes: add SiTime Corporation Ali Rouhi
2026-09-30 23:37 ` [PATCH net-next v11 04/13] dpll: add basic SiTime SiT9531x support Ali Rouhi
2026-10-05  1:09   ` netdev-bot+sashiko
2026-09-30 23:37 ` [PATCH net-next v11 05/13] dpll: sit9531x: read DPLL types and pin properties from system firmware Ali Rouhi
2026-10-05  1:09   ` netdev-bot+sashiko
2026-09-30 23:37 ` [PATCH net-next v11 06/13] dpll: sit9531x: register DPLL devices and pins Ali Rouhi
2026-10-05  1:09   ` netdev-bot+sashiko
2026-09-30 23:37 ` [PATCH net-next v11 07/13] dpll: sit9531x: implement input pin state on a DPLL Ali Rouhi
2026-10-05  1:10   ` netdev-bot+sashiko
2026-09-30 23:37 ` [PATCH net-next v11 08/13] dpll: sit9531x: add support to get and set priority on input pins Ali Rouhi
2026-10-05  1:10   ` netdev-bot+sashiko
2026-09-30 23:37 ` [PATCH net-next v11 10/13] dpll: sit9531x: implement output pin state on a DPLL Ali Rouhi
2026-10-05  1:10   ` netdev-bot+sashiko
2026-09-30 23:37 ` [PATCH net-next v11 09/13] dpll: sit9531x: add support to get and set frequency on pins Ali Rouhi
2026-10-05  1:10   ` netdev-bot+sashiko
2026-09-30 23:37 ` [PATCH net-next v11 12/13] dpll: sit9531x: add support to get phase offset on the connected input pin Ali Rouhi
2026-10-05  1:10   ` netdev-bot+sashiko
2026-09-30 23:37 ` [PATCH net-next v11 11/13] dpll: sit9531x: add support to adjust output phase Ali Rouhi
2026-10-05  1:10   ` netdev-bot+sashiko
2026-09-30 23:37 ` [PATCH net-next v11 13/13] dpll: sit9531x: model the inter-PLL sync net as a pair of pins Ali Rouhi
2026-10-05  1:10   ` netdev-bot+sashiko

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox
all inboxes | Powered by JetHome®