Re: [PATCH net-next v8 02/15] dt-bindings: dpll: add SiTime SiT95316 clock generator
From: netdev-bot+sashiko@kernel.org
Date: 2026-09-08 12:45:12
Also in:
linux-devicetree, lkml
Thank you for your contribution! Sashiko AI review found 3 potential issue(s) to consider: Critical: 0 · High: 0 · Medium: 1 · Low: 2 - [Medium] New DT property `sitime,pll-fvco` (Documentation/devicetree/bindings/dpll/sitime,sit95316.yaml:51) omits… - [Low] Concern (assessed invalid): the MAINTAINERS section "SITIME SIT9531X DPLL DRIVER" added at MAINTAINERS:25160 lists only the… - [Low] Concern (assessed invalid): the schema does not bound DPLL channel count (`dpll-types` has no maxItems: 4) nor per-variant… -- dt-bindings: dpll: add SiTime SiT95316 clock generator Adds a new DT binding for the SiTime SiT95316/SiT95317 I2C clock generators, described as a DPLL provider with four PLLs, eight selectable inputs and up to twelve outputs, plus two vendor properties (sitime,pll-fvco and sitime,output-pll-map). Also adds a MAINTAINERS entry for the new binding.
quoted hunk ↗ jump to hunk
diff --git a/Documentation/devicetree/bindings/dpll/sitime,sit95316.yaml b/Documentation/devicetree/bindings/dpll/sitime,sit95316.yaml new file mode 100644 index 000000000000..b3cf9d5a8267 --- /dev/null +++ b/Documentation/devicetree/bindings/dpll/sitime,sit95316.yaml@@ -0,0 +1,171 @@
[ ... ]
+ sitime,pll-fvco: + $ref: /schemas/types.yaml#/definitions/uint64-array + minItems: 4 + maxItems: 4 + description: + Per-PLL VCO frequency in Hz for PLLA, PLLB, PLLC, PLLD. Neither + standard frequency suffix fits. The VCO bands are 4915.2 MHz to + 5898.24 MHz and 6875 MHz to 7812.5 MHz, so every valid value + exceeds the 32 bits "-hz" allows, and the band edges are not + whole megahertz either, so "-mhz" cannot express them. Hence a + plain uint64-array. Override where the standard
[Severity: Medium]
Is the statement that "every valid value exceeds the 32 bits "-hz" allows"
correct? A "-hz" property can carry 64-bit values, so the stated reason for
dropping the unit suffix does not seem to hold.
The peer binding in the same directory does exactly that:
Documentation/devicetree/bindings/dpll/microchip,zl30731.yaml
supported-frequencies-hz = /bits/ 64 <10000 10000000>;
And this very file does it too, in its own examples:
supported-frequencies-hz = /bits/ 64 <156250000>;
Documentation/devicetree/bindings/opp/opp-v2-base.yaml also documents this
explicitly:
opp-hz:
description:
Frequency in Hz, expressed as a 64-bit big-endian integer.
Since the values are already in Hz, could the property be named
sitime,pll-fvco-hz while keeping
$ref: /schemas/types.yaml#/definitions/uint64-array
and the 32-bit rationale dropped from the description? Once merged, the
suffix-less spelling becomes DT ABI and cannot be renamed later.
+ Fvco = Fref * DIVN derivation does not match the running VCO -- + for example a PLL operating in INTSYNC mode. A value of 0 keeps + the register-derived computation for that PLL.
[ ... ] -- Sashiko AI review · https://netdev-ai.bots.linux.dev/sashiko/#/patchset/20260902214030.20955-1-arouhi%40sitime.com