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

[Ali Rouhi]

On Mon, May 12, 2026, Conor Dooley wrote:
> This is not relevant information in a binding FYI.

Agreed — removed the Linux-specific paragraph from the description.

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

Done — reset-gpios now just says "GPIO connected to the chip's
active-low reset pin (RESETB)."

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

Reworded to describe hardware behavior: "Asserted when the device
detects a status change such as lock acquisition or loss."

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

That was an oversight on my part. v2 adds allOf/$ref to
dpll-device.yaml, switches to unevaluatedProperties, and includes
input-pins/output-pins sub-nodes in the second example.

Thanks for the review.

Ali


On Tue, May 12, 2026 at 1:15 PM Ali Rouhi <rouhi.ali@xxxxxxxxx> wrote:
>
> On Tue, May 12, 2026 at 10:52 AM Conor Dooley <conor@xxxxxxxxxx> wrote:
> > This is not relevant information in a binding FYI.
>
> Agreed — removed the Linux-specific paragraph from the description.
>
> > Description here shouldn't talk about what linux drivers do
> > with the property. Remove the second sentence please.
>
> Done — reset-gpios now just says "GPIO connected to the chip's
> active-low reset pin (RESETB)."
>
> > Same here. If you even converted "uses it" to "should" or "must"
> > then it'd be fine.
>
> Reworded to describe hardware behavior: "Asserted when the device
> detects a status change such as lock acquisition or loss."
>
> > Can you explain why there's no reference to dpll-device.yaml and
> > none of the properties involved are used?
>
> That was an oversight on my part. v2 adds allOf/$ref to
> dpll-device.yaml, switches to unevaluatedProperties, and includes
> input-pins/output-pins sub-nodes in the second example.
>
> Thanks Conor for the review.
>
> Ali
>
> On Tue, May 12, 2026 at 10:52 AM Conor Dooley <conor@xxxxxxxxxx> wrote:
>>
>> 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@xxxxxxxxxx>
>> > ---
>> >  .../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@xxxxxxxxxx>
>> > +
>> > +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
>> >