Re: [PATCH net-next 2/3] dt-bindings: dpll: add SiTime SiT9531x clock generator

[Conor Dooley <conor@kernel.org>]

[-- Attachment #1: Type: text/plain, Size: 3635 bytes --]

On Mon, May 11, 2026 at 02:14:52PM -0700, Ali Rouhi wrote:
> Add device tree binding documentation for the SiTime SiT95316
> and SiT95317 DPLL clock generators.
> 
> Signed-off-by: Ali Rouhi <arouhi@sitime.com>
> ---
>  .../bindings/dpll/sitime,sit9531x.yaml        | 82 +++++++++++++++++++
>  1 file changed, 82 insertions(+)
>  create mode 100644 Documentation/devicetree/bindings/dpll/sitime,sit9531x.yaml
> 
> diff --git a/Documentation/devicetree/bindings/dpll/sitime,sit9531x.yaml b/Documentation/devicetree/bindings/dpll/sitime,sit9531x.yaml
> new file mode 100644
> index 000000000000..0b05f0de65b9
> --- /dev/null
> +++ b/Documentation/devicetree/bindings/dpll/sitime,sit9531x.yaml
> @@ -0,0 +1,82 @@
> +# SPDX-License-Identifier: (GPL-2.0-only OR BSD-2-Clause)
> +%YAML 1.2
> +---
> +$id: http://devicetree.org/schemas/dpll/sitime,sit9531x.yaml#
> +$schema: http://devicetree.org/meta-schemas/core.yaml#
> +
> +title: SiTime SiT9531x DPLL Clock Generator
> +
> +maintainers:
> +  - Ali Rouhi <arouhi@sitime.com>
> +
> +description: |
> +  The SiTime SiT95316 and SiT95317 are I2C-controlled programmable clock
> +  generators with integrated DPLL for synchronization applications.  Both
> +  variants contain four PLLs with automatic/manual reference selection,
> +  DCO frequency adjustment, and phase offset measurement via an on-chip
> +  TDC (Time-to-Digital Converter).
> +
> +  The SiT95317 provides 4 inputs and 8 outputs; the SiT95316 provides
> +  4 inputs and 12 outputs.


> +
> +  Runtime configuration (reference selection, frequency, phase) is managed
> +  through the kernel DPLL netlink subsystem; the device tree describes only
> +  the hardware wiring.

This is not relevant information in a binding FYI.

> +
> +properties:
> +  compatible:
> +    enum:
> +      - sitime,sit95316
> +      - sitime,sit95317
> +
> +  reg:
> +    maxItems: 1
> +
> +  reset-gpios:
> +    maxItems: 1
> +    description:
> +      GPIO connected to the chip's active-low reset pin.  If present, the
> +      driver holds the line deasserted at probe.  Optional; boards that do
> +      not route the reset line omit this property.

Description here shouldn't talk about what linux drivers do with the
property. Remove the second sentence please.

> +
> +  interrupts:
> +    maxItems: 1
> +    description:
> +      Interrupt from the chip's active-low INTRB output.  When wired, the
> +      driver uses it to trigger immediate status readback instead of
> +      relying solely on periodic polling.  Optional.

Same here. If you even converted "uses it" to "should" or "must" then
it'd be fine.

> +
> +required:
> +  - compatible
> +  - reg

Can you explain why there's no reference to dpll-device.yaml and none of
the properties involved are used?

Cheers,
Conor.

> +
> +additionalProperties: false
> +
> +examples:
> +  - |
> +    i2c {
> +        #address-cells = <1>;
> +        #size-cells = <0>;
> +
> +        clock-generator@68 {
> +            compatible = "sitime,sit95317";
> +            reg = <0x68>;
> +        };
> +    };
> +
> +  - |
> +    #include <dt-bindings/gpio/gpio.h>
> +    #include <dt-bindings/interrupt-controller/irq.h>
> +
> +    i2c {
> +        #address-cells = <1>;
> +        #size-cells = <0>;
> +
> +        clock-generator@68 {
> +            compatible = "sitime,sit95316";
> +            reg = <0x68>;
> +            reset-gpios = <&gpio 78 GPIO_ACTIVE_LOW>;
> +            interrupts = <12 IRQ_TYPE_LEVEL_LOW>;
> +        };
> +    };
> +...
> -- 
> 2.43.0
> 

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 228 bytes --]