[PATCH v2 0/4] clk: si5351: add option to adjust PLL without glitches

[PATCH v2 0/4] clk: si5351: add option to adjust PLL without glitches

[Alvin Šipraga]

From: Alvin Šipraga <alsi@bang-olufsen.dk>

This series intends to address a problem I had when using the Si5351A as
a runtime adjustable audio bit clock. The basic issue is that the driver
in its current form unconditionally resets the PLL whenever adjusting
its rate. But this reset causes an unwanted ~1.4 ms LOW signal glitch in
the clock output.

As a remedy, a new property is added to control the reset behaviour of
the PLLs more precisely. In the process I also converted the bindings to
YAML.

Changes:

v1 -> v2:

- address Rob's comments on the two dt-bindings patches
- new patch to correct the clock node names in the only upstream device
  tree using si5351

Alvin Šipraga (4):
  dt-bindings: clock: si5351: convert to yaml
  ARM: dts: dove-cubox: fix si5351 node names
  dt-bindings: clock: si5351: add PLL reset mode property
  clk: si5351: allow PLLs to be adjusted without reset

 .../bindings/clock/silabs,si5351.txt          | 126 --------
 .../bindings/clock/silabs,si5351.yaml         | 277 ++++++++++++++++++
 arch/arm/boot/dts/marvell/dove-cubox.dts      |   4 +-
 drivers/clk/clk-si5351.c                      |  47 ++-
 include/linux/platform_data/si5351.h          |   2 +
 5 files changed, 325 insertions(+), 131 deletions(-)
 delete mode 100644 Documentation/devicetree/bindings/clock/silabs,si5351.txt
 create mode 100644 Documentation/devicetree/bindings/clock/silabs,si5351.yaml

-- 
2.42.0

[PATCH v2 1/4] dt-bindings: clock: si5351: convert to yaml

[Alvin Šipraga]

From: Alvin Šipraga <alsi@bang-olufsen.dk>

The following additional properties are described:

  - clock-names
  - clock-frequency of the clkout child nodes

In order to suppress warnings from the DT schema validator, the clkout
child nodes are prescribed names clkout@[0-7] rather than clkout[0-7].
The latter form is still admissible but the example has been changed to
use the former.

The example is refined as follows:

  - correct the usage of property pll-master -> silabs,pll-master
  - give an example of how the silabs,pll-reset property can be used

I made myself maintainer of the file as I cannot presume that anybody
else wants the responsibility.

Cc: Sebastian Hesselbarth <sebastian.hesselbarth@gmail.com>
Cc: Rabeeh Khoury <rabeeh@solid-run.com>
Signed-off-by: Alvin Šipraga <alsi@bang-olufsen.dk>
---
 .../bindings/clock/silabs,si5351.txt          | 126 ---------
 .../bindings/clock/silabs,si5351.yaml         | 253 ++++++++++++++++++
 2 files changed, 253 insertions(+), 126 deletions(-)
 delete mode 100644 Documentation/devicetree/bindings/clock/silabs,si5351.txt
 create mode 100644 Documentation/devicetree/bindings/clock/silabs,si5351.yaml

diff --git a/Documentation/devicetree/bindings/clock/silabs,si5351.txt b/Documentation/devicetree/bindings/clock/silabs,si5351.txt
deleted file mode 100644
index bfda6af76bee..000000000000
--- a/Documentation/devicetree/bindings/clock/silabs,si5351.txt
+++ /dev/null
@@ -1,126 +0,0 @@
-Binding for Silicon Labs Si5351a/b/c programmable i2c clock generator.
-
-Reference
-[1] Si5351A/B/C Data Sheet
-    https://www.skyworksinc.com/-/media/Skyworks/SL/documents/public/data-sheets/Si5351-B.pdf
-
-The Si5351a/b/c are programmable i2c clock generators with up to 8 output
-clocks. Si5351a also has a reduced pin-count package (MSOP10) where only
-3 output clocks are accessible. The internal structure of the clock
-generators can be found in [1].
-
-==I2C device node==
-
-Required properties:
-- compatible: shall be one of the following:
-	"silabs,si5351a" - Si5351a, QFN20 package
-	"silabs,si5351a-msop" - Si5351a, MSOP10 package
-	"silabs,si5351b" - Si5351b, QFN20 package
-	"silabs,si5351c" - Si5351c, QFN20 package
-- reg: i2c device address, shall be 0x60 or 0x61.
-- #clock-cells: from common clock binding; shall be set to 1.
-- clocks: from common clock binding; list of parent clock
-  handles, shall be xtal reference clock or xtal and clkin for
-  si5351c only. Corresponding clock input names are "xtal" and
-  "clkin" respectively.
-- #address-cells: shall be set to 1.
-- #size-cells: shall be set to 0.
-
-Optional properties:
-- silabs,pll-source: pair of (number, source) for each pll. Allows
-  to overwrite clock source of pll A (number=0) or B (number=1).
-
-==Child nodes==
-
-Each of the clock outputs can be overwritten individually by
-using a child node to the I2C device node. If a child node for a clock
-output is not set, the eeprom configuration is not overwritten.
-
-Required child node properties:
-- reg: number of clock output.
-
-Optional child node properties:
-- silabs,clock-source: source clock of the output divider stage N, shall be
-  0 = multisynth N
-  1 = multisynth 0 for output clocks 0-3, else multisynth4
-  2 = xtal
-  3 = clkin (si5351c only)
-- silabs,drive-strength: output drive strength in mA, shall be one of {2,4,6,8}.
-- silabs,multisynth-source: source pll A(0) or B(1) of corresponding multisynth
-  divider.
-- silabs,pll-master: boolean, multisynth can change pll frequency.
-- silabs,pll-reset: boolean, clock output can reset its pll.
-- silabs,disable-state : clock output disable state, shall be
-  0 = clock output is driven LOW when disabled
-  1 = clock output is driven HIGH when disabled
-  2 = clock output is FLOATING (HIGH-Z) when disabled
-  3 = clock output is NEVER disabled
-
-==Example==
-
-/* 25MHz reference crystal */
-ref25: ref25M {
-	compatible = "fixed-clock";
-	#clock-cells = <0>;
-	clock-frequency = <25000000>;
-};
-
-i2c-master-node {
-
-	/* Si5351a msop10 i2c clock generator */
-	si5351a: clock-generator@60 {
-		compatible = "silabs,si5351a-msop";
-		reg = <0x60>;
-		#address-cells = <1>;
-		#size-cells = <0>;
-		#clock-cells = <1>;
-
-		/* connect xtal input to 25MHz reference */
-		clocks = <&ref25>;
-		clock-names = "xtal";
-
-		/* connect xtal input as source of pll0 and pll1 */
-		silabs,pll-source = <0 0>, <1 0>;
-
-		/*
-		 * overwrite clkout0 configuration with:
-		 * - 8mA output drive strength
-		 * - pll0 as clock source of multisynth0
-		 * - multisynth0 as clock source of output divider
-		 * - multisynth0 can change pll0
-		 * - set initial clock frequency of 74.25MHz
-		 */
-		clkout0 {
-			reg = <0>;
-			silabs,drive-strength = <8>;
-			silabs,multisynth-source = <0>;
-			silabs,clock-source = <0>;
-			silabs,pll-master;
-			clock-frequency = <74250000>;
-		};
-
-		/*
-		 * overwrite clkout1 configuration with:
-		 * - 4mA output drive strength
-		 * - pll1 as clock source of multisynth1
-		 * - multisynth1 as clock source of output divider
-		 * - multisynth1 can change pll1
-		 */
-		clkout1 {
-			reg = <1>;
-			silabs,drive-strength = <4>;
-			silabs,multisynth-source = <1>;
-			silabs,clock-source = <0>;
-			pll-master;
-		};
-
-		/*
-		 * overwrite clkout2 configuration with:
-		 * - xtal as clock source of output divider
-		 */
-		clkout2 {
-			reg = <2>;
-			silabs,clock-source = <2>;
-		};
-	};
-};
diff --git a/Documentation/devicetree/bindings/clock/silabs,si5351.yaml b/Documentation/devicetree/bindings/clock/silabs,si5351.yaml
new file mode 100644
index 000000000000..400c8cec2a3a
--- /dev/null
+++ b/Documentation/devicetree/bindings/clock/silabs,si5351.yaml
@@ -0,0 +1,253 @@
+# SPDX-License-Identifier: (GPL-2.0-only OR BSD-2-Clause)
+%YAML 1.2
+---
+$id: http://devicetree.org/schemas/clock/silabs,si5351.yaml#
+$schema: http://devicetree.org/meta-schemas/core.yaml#
+
+title: Silicon Labs Si5351A/B/C programmable I2C clock generators
+
+description: |
+  The Silicon Labs Si5351A/B/C are programmable I2C clock generators with up to
+  8 outputs. Si5351A also has a reduced pin-count package (10-MSOP) where only 3
+  output clocks are accessible. The internal structure of the clock generators
+  can be found in [1].
+
+  [1] Si5351A/B/C Data Sheet
+      https://www.skyworksinc.com/-/media/Skyworks/SL/documents/public/data-sheets/Si5351-B.pdf
+
+maintainers:
+  - Alvin Šipraga <alsi@bang-olufsen.dk>
+
+properties:
+  compatible:
+    enum:
+      - silabs,si5351a      # Si5351A, 20-QFN package
+      - silabs,si5351a-msop # Si5351A, 10-MSOP package
+      - silabs,si5351b      # Si5351B, 20-QFN package
+      - silabs,si5351c      # Si5351C, 20-QFN package
+
+  reg:
+    enum:
+      - 0x60
+      - 0x61
+
+  "#address-cells":
+    const: 1
+
+  "#size-cells":
+    const: 0
+
+  "#clock-cells":
+    const: 1
+
+  silabs,pll-source:
+    $ref: /schemas/types.yaml#/definitions/uint32-matrix
+    description: |
+      A list of cell pairs containing a PLL index and its source. Allows to
+      overwrite clock source of the internal PLLs.
+    minItems: 1
+    items:
+      items:
+        - description: PLL A (0) or PLL B (1)
+          enum: [ 0, 1 ]
+        - description: PLL source, XTAL (0) or CLKIN (1, Si5351C only).
+          enum: [ 0, 1 ]
+
+patternProperties:
+  "^clkout@[0-7]$":
+    type: object
+
+    properties:
+      reg:
+        $ref: /schemas/types.yaml#/definitions/uint32
+        description: Clock output number.
+
+      clock-frequency: true
+
+      silabs,clock-source:
+        $ref: /schemas/types.yaml#/definitions/uint32
+        description: |
+          Source clock of the this output's divider stage.
+
+          0 - use multisynth N for this output, where N is the output number
+          1 - use either multisynth 0 (if output number is 0-3) or multisynth 4
+              (otherwise) for this output
+          2 - use XTAL for this output
+          3 - use CLKIN for this output (Si5351C only)
+
+      silabs,drive-strength:
+        $ref: /schemas/types.yaml#/definitions/uint32
+        enum: [ 2, 4, 6, 8 ]
+        description: Output drive strength in mA.
+
+      silabs,multisynth-source:
+        $ref: /schemas/types.yaml#/definitions/uint32
+        enum: [ 0, 1 ]
+        description: |
+          Source PLL A (0) or B (1) for the corresponding multisynth divider.
+
+      silabs,pll-master:
+        type: boolean
+        description: |
+          The frequency of the source PLL is allowed to be changed by the
+          multisynth when setting the rate of this clock output.
+
+      silabs,pll-reset:
+        type: boolean
+        description: Reset the source PLL when enabling this clock output.
+
+      silabs,disable-state:
+        $ref: /schemas/types.yaml#/definitions/uint32
+        enum: [ 0, 1, 2, 3 ]
+        description: |
+          Clock output disable state. The state can be one of:
+
+          0 - clock output is driven LOW when disabled
+          1 - clock output is driven HIGH when disabled
+          2 - clock output is FLOATING (HIGH-Z) when disabled
+          3 - clock output is never disabled
+
+    allOf:
+      - if:
+          properties:
+            compatible:
+              contains:
+                const: silabs,si5351a-msop
+        then:
+          properties:
+            reg:
+              minimum: 0
+              maximum: 2
+        else:
+          properties:
+            reg:
+              minimum: 0
+              maximum: 7
+
+      - if:
+          properties:
+            compatible:
+              contains:
+                const: silabs,si5351c
+        then:
+          properties:
+            silabs,clock-source:
+              enum: [ 0, 1, 2, 3 ]
+        else:
+          properties:
+            silabs,clock-source:
+              enum: [ 0, 1, 2 ]
+    required:
+      - reg
+
+    additionalProperties: false
+
+allOf:
+  - $ref: /schemas/clock/clock.yaml
+  - if:
+      properties:
+        compatible:
+          contains:
+            enum:
+              - silabs,si5351a
+              - silabs,si5351a-msop
+              - silabs,si5351b
+    then:
+      properties:
+        clocks:
+          minItems: 1
+          maxItems: 1
+        clock-names:
+          items:
+            - const: xtal
+
+  - if:
+      properties:
+        compatible:
+          contains:
+            const: silabs,si5351c
+    then:
+      properties:
+        clocks:
+          minItems: 1
+          maxItems: 2
+        clock-names:
+          minItems: 1
+          items:
+            - const: xtal
+            - const: clkin
+
+required:
+  - reg
+  - "#address-cells"
+  - "#size-cells"
+  - "#clock-cells"
+  - clocks
+  - clock-names
+
+unevaluatedProperties: false
+
+examples:
+  - |
+    i2c {
+      #address-cells = <1>;
+      #size-cells = <0>;
+
+      clock-generator@60 {
+        compatible = "silabs,si5351a-msop";
+        reg = <0x60>;
+        #address-cells = <1>;
+        #size-cells = <0>;
+        #clock-cells = <1>;
+
+        /* Connect XTAL input to 25MHz reference */
+        clocks = <&ref25>;
+        clock-names = "xtal";
+
+        /* Use XTAL input as source of PLL0 and PLL1 */
+        silabs,pll-source = <0 0>, <1 0>;
+
+        /*
+         * Overwrite CLK0 configuration with:
+         * - 8 mA output drive strength
+         * - PLL0 as clock source of multisynth 0
+         * - Multisynth 0 as clock source of output divider
+         * - Multisynth 0 can change PLL0
+         * - Set initial clock frequency of 74.25MHz
+         */
+        clkout@0 {
+          reg = <0>;
+          silabs,drive-strength = <8>;
+          silabs,multisynth-source = <0>;
+          silabs,clock-source = <0>;
+          silabs,pll-master;
+          clock-frequency = <74250000>;
+        };
+
+        /*
+         * Overwrite CLK1 configuration with:
+         * - 4 mA output drive strength
+         * - PLL1 as clock source of multisynth 1
+         * - Multisynth 1 as clock source of output divider
+         * - Multisynth 1 can change PLL1
+         * - Reset PLL1 when enabling this clock output
+         */
+        clkout@1 {
+          reg = <1>;
+          silabs,drive-strength = <4>;
+          silabs,multisynth-source = <1>;
+          silabs,clock-source = <0>;
+          silabs,pll-master;
+          silabs,pll-reset;
+        };
+
+        /*
+         * Overwrite CLK2 configuration with:
+         * - XTAL as clock source of output divider
+         */
+        clkout@2 {
+          reg = <2>;
+          silabs,clock-source = <2>;
+        };
+      };
+    };
-- 
2.42.0

Re: [PATCH v2 1/4] dt-bindings: clock: si5351: convert to yaml

[Rob Herring]

On Wed, Oct 04, 2023 at 08:35:27AM +0200, Alvin Šipraga wrote:
> From: Alvin Šipraga <alsi@bang-olufsen.dk>
> 
> The following additional properties are described:
> 
>   - clock-names
>   - clock-frequency of the clkout child nodes
> 
> In order to suppress warnings from the DT schema validator, the clkout
> child nodes are prescribed names clkout@[0-7] rather than clkout[0-7].
> The latter form is still admissible but the example has been changed to
> use the former.
> 
> The example is refined as follows:
> 
>   - correct the usage of property pll-master -> silabs,pll-master
>   - give an example of how the silabs,pll-reset property can be used
> 
> I made myself maintainer of the file as I cannot presume that anybody
> else wants the responsibility.
> 
> Cc: Sebastian Hesselbarth <sebastian.hesselbarth@gmail.com>
> Cc: Rabeeh Khoury <rabeeh@solid-run.com>
> Signed-off-by: Alvin Šipraga <alsi@bang-olufsen.dk>
> ---
>  .../bindings/clock/silabs,si5351.txt          | 126 ---------
>  .../bindings/clock/silabs,si5351.yaml         | 253 ++++++++++++++++++
>  2 files changed, 253 insertions(+), 126 deletions(-)
>  delete mode 100644 Documentation/devicetree/bindings/clock/silabs,si5351.txt
>  create mode 100644 Documentation/devicetree/bindings/clock/silabs,si5351.yaml

[...]

> diff --git a/Documentation/devicetree/bindings/clock/silabs,si5351.yaml b/Documentation/devicetree/bindings/clock/silabs,si5351.yaml
> new file mode 100644
> index 000000000000..400c8cec2a3a
> --- /dev/null
> +++ b/Documentation/devicetree/bindings/clock/silabs,si5351.yaml
> @@ -0,0 +1,253 @@
> +# SPDX-License-Identifier: (GPL-2.0-only OR BSD-2-Clause)
> +%YAML 1.2
> +---
> +$id: http://devicetree.org/schemas/clock/silabs,si5351.yaml#
> +$schema: http://devicetree.org/meta-schemas/core.yaml#
> +
> +title: Silicon Labs Si5351A/B/C programmable I2C clock generators
> +
> +description: |
> +  The Silicon Labs Si5351A/B/C are programmable I2C clock generators with up to
> +  8 outputs. Si5351A also has a reduced pin-count package (10-MSOP) where only 3
> +  output clocks are accessible. The internal structure of the clock generators
> +  can be found in [1].
> +
> +  [1] Si5351A/B/C Data Sheet
> +      https://www.skyworksinc.com/-/media/Skyworks/SL/documents/public/data-sheets/Si5351-B.pdf
> +
> +maintainers:
> +  - Alvin Šipraga <alsi@bang-olufsen.dk>
> +
> +properties:
> +  compatible:
> +    enum:
> +      - silabs,si5351a      # Si5351A, 20-QFN package
> +      - silabs,si5351a-msop # Si5351A, 10-MSOP package
> +      - silabs,si5351b      # Si5351B, 20-QFN package
> +      - silabs,si5351c      # Si5351C, 20-QFN package
> +
> +  reg:
> +    enum:
> +      - 0x60
> +      - 0x61
> +
> +  "#address-cells":
> +    const: 1
> +
> +  "#size-cells":
> +    const: 0
> +
> +  "#clock-cells":
> +    const: 1
> +
> +  silabs,pll-source:
> +    $ref: /schemas/types.yaml#/definitions/uint32-matrix
> +    description: |
> +      A list of cell pairs containing a PLL index and its source. Allows to
> +      overwrite clock source of the internal PLLs.
> +    minItems: 1

The minimum is 1 by default (can't have 0).

> +    items:
> +      items:
> +        - description: PLL A (0) or PLL B (1)
> +          enum: [ 0, 1 ]
> +        - description: PLL source, XTAL (0) or CLKIN (1, Si5351C only).
> +          enum: [ 0, 1 ]
> +
> +patternProperties:
> +  "^clkout@[0-7]$":
> +    type: object
> +
> +    properties:
> +      reg:
> +        $ref: /schemas/types.yaml#/definitions/uint32

reg already has a type. Drop.

> +        description: Clock output number.
> +
> +      clock-frequency: true
> +
> +      silabs,clock-source:
> +        $ref: /schemas/types.yaml#/definitions/uint32
> +        description: |
> +          Source clock of the this output's divider stage.
> +
> +          0 - use multisynth N for this output, where N is the output number
> +          1 - use either multisynth 0 (if output number is 0-3) or multisynth 4
> +              (otherwise) for this output
> +          2 - use XTAL for this output
> +          3 - use CLKIN for this output (Si5351C only)
> +
> +      silabs,drive-strength:
> +        $ref: /schemas/types.yaml#/definitions/uint32
> +        enum: [ 2, 4, 6, 8 ]
> +        description: Output drive strength in mA.
> +
> +      silabs,multisynth-source:
> +        $ref: /schemas/types.yaml#/definitions/uint32
> +        enum: [ 0, 1 ]
> +        description: |

Don't need '|' if no formatting to preserve.

> +          Source PLL A (0) or B (1) for the corresponding multisynth divider.
> +
> +      silabs,pll-master:
> +        type: boolean
> +        description: |
> +          The frequency of the source PLL is allowed to be changed by the
> +          multisynth when setting the rate of this clock output.
> +
> +      silabs,pll-reset:
> +        type: boolean
> +        description: Reset the source PLL when enabling this clock output.
> +
> +      silabs,disable-state:
> +        $ref: /schemas/types.yaml#/definitions/uint32
> +        enum: [ 0, 1, 2, 3 ]
> +        description: |
> +          Clock output disable state. The state can be one of:
> +
> +          0 - clock output is driven LOW when disabled
> +          1 - clock output is driven HIGH when disabled
> +          2 - clock output is FLOATING (HIGH-Z) when disabled
> +          3 - clock output is never disabled
> +
> +    allOf:
> +      - if:
> +          properties:
> +            compatible:
> +              contains:
> +                const: silabs,si5351a-msop
> +        then:
> +          properties:
> +            reg:
> +              minimum: 0

The minimum is already 0. Drop.

> +              maximum: 2
> +        else:
> +          properties:
> +            reg:
> +              minimum: 0
> +              maximum: 7
> +
> +      - if:
> +          properties:
> +            compatible:
> +              contains:
> +                const: silabs,si5351c
> +        then:
> +          properties:
> +            silabs,clock-source:
> +              enum: [ 0, 1, 2, 3 ]
> +        else:
> +          properties:
> +            silabs,clock-source:
> +              enum: [ 0, 1, 2 ]
> +    required:
> +      - reg
> +
> +    additionalProperties: false

Move this next to 'type: object'

> +
> +allOf:
> +  - $ref: /schemas/clock/clock.yaml

Don't need this.

> +  - if:
> +      properties:
> +        compatible:
> +          contains:
> +            enum:
> +              - silabs,si5351a
> +              - silabs,si5351a-msop
> +              - silabs,si5351b

Isn't this just the 'else' for the next one? Or more parts are coming?

> +    then:
> +      properties:
> +        clocks:
> +          minItems: 1
> +          maxItems: 1
> +        clock-names:
> +          items:
> +            - const: xtal
> +
> +  - if:
> +      properties:
> +        compatible:
> +          contains:
> +            const: silabs,si5351c
> +    then:
> +      properties:
> +        clocks:
> +          minItems: 1
> +          maxItems: 2
> +        clock-names:
> +          minItems: 1
> +          items:
> +            - const: xtal
> +            - const: clkin

Define clocks and clock-names at the top level and just use 
minItems/maxItems in the if/then schemas.

> +
> +required:
> +  - reg
> +  - "#address-cells"
> +  - "#size-cells"
> +  - "#clock-cells"
> +  - clocks
> +  - clock-names
> +
> +unevaluatedProperties: false
> +
> +examples:
> +  - |
> +    i2c {
> +      #address-cells = <1>;
> +      #size-cells = <0>;
> +
> +      clock-generator@60 {
> +        compatible = "silabs,si5351a-msop";
> +        reg = <0x60>;
> +        #address-cells = <1>;
> +        #size-cells = <0>;
> +        #clock-cells = <1>;
> +
> +        /* Connect XTAL input to 25MHz reference */
> +        clocks = <&ref25>;
> +        clock-names = "xtal";
> +
> +        /* Use XTAL input as source of PLL0 and PLL1 */
> +        silabs,pll-source = <0 0>, <1 0>;
> +
> +        /*
> +         * Overwrite CLK0 configuration with:
> +         * - 8 mA output drive strength
> +         * - PLL0 as clock source of multisynth 0
> +         * - Multisynth 0 as clock source of output divider
> +         * - Multisynth 0 can change PLL0
> +         * - Set initial clock frequency of 74.25MHz
> +         */
> +        clkout@0 {
> +          reg = <0>;
> +          silabs,drive-strength = <8>;
> +          silabs,multisynth-source = <0>;
> +          silabs,clock-source = <0>;
> +          silabs,pll-master;
> +          clock-frequency = <74250000>;
> +        };
> +
> +        /*
> +         * Overwrite CLK1 configuration with:
> +         * - 4 mA output drive strength
> +         * - PLL1 as clock source of multisynth 1
> +         * - Multisynth 1 as clock source of output divider
> +         * - Multisynth 1 can change PLL1
> +         * - Reset PLL1 when enabling this clock output
> +         */
> +        clkout@1 {
> +          reg = <1>;
> +          silabs,drive-strength = <4>;
> +          silabs,multisynth-source = <1>;
> +          silabs,clock-source = <0>;
> +          silabs,pll-master;
> +          silabs,pll-reset;
> +        };
> +
> +        /*
> +         * Overwrite CLK2 configuration with:
> +         * - XTAL as clock source of output divider
> +         */
> +        clkout@2 {
> +          reg = <2>;
> +          silabs,clock-source = <2>;
> +        };
> +      };
> +    };
> -- 
> 2.42.0
> 

Re: [PATCH v2 1/4] dt-bindings: clock: si5351: convert to yaml

[Alvin Šipraga]

On Wed, Oct 04, 2023 at 09:39:37AM -0500, Rob Herring wrote:
> > +      silabs,multisynth-source:
> > +        $ref: /schemas/types.yaml#/definitions/uint32
> > +        enum: [ 0, 1 ]
> > +        description: |
> 
> Don't need '|' if no formatting to preserve.

I thought the line would be too long otherwise.
Column width is 80 in dt-schema as well, right?

> 
> > +          Source PLL A (0) or B (1) for the corresponding multisynth divider.
> > +

[...]

> > +  - if:
> > +      properties:
> > +        compatible:
> > +          contains:
> > +            enum:
> > +              - silabs,si5351a
> > +              - silabs,si5351a-msop
> > +              - silabs,si5351b
> 
> Isn't this just the 'else' for the next one? Or more parts are coming?

Not sure if more parts are coming - these are the only ones I am aware of. But I
have not checked thoroughly. I thought it better to be explicit, but I will
change the next one to an else: in v3 unless you change your mind.

> 
> > +    then:
> > +      properties:
> > +        clocks:
> > +          minItems: 1
> > +          maxItems: 1
> > +        clock-names:
> > +          items:
> > +            - const: xtal
> > +
> > +  - if:
> > +      properties:
> > +        compatible:
> > +          contains:
> > +            const: silabs,si5351c
> > +    then:
> > +      properties:
> > +        clocks:
> > +          minItems: 1
> > +          maxItems: 2
> > +        clock-names:
> > +          minItems: 1
> > +          items:
> > +            - const: xtal
> > +            - const: clkin
> 
> Define clocks and clock-names at the top level and just use 
> minItems/maxItems in the if/then schemas.

I was trying to imply here that it is invalid to specify clkin for the former
three part types - only for the si5351c. If I specify both in the top-level
clock-names:items then it would allow something like this:

  clk {
    compatible = "silabs,si5351a-msop";
    clocks = <&ref25>;
    clock-names = "clkin"; /* not OK - Si5351A-MSOP only supports XTAL */
  };

Kind regards,
Alvin

Re: [PATCH v2 1/4] dt-bindings: clock: si5351: convert to yaml

[Rob Herring]

On Wed, Oct 4, 2023 at 4:40 PM Alvin Šipraga <ALSI@bang-olufsen.dk> wrote:
>
> On Wed, Oct 04, 2023 at 09:39:37AM -0500, Rob Herring wrote:
> > > +      silabs,multisynth-source:
> > > +        $ref: /schemas/types.yaml#/definitions/uint32
> > > +        enum: [ 0, 1 ]
> > > +        description: |
> >
> > Don't need '|' if no formatting to preserve.
>
> I thought the line would be too long otherwise.
> Column width is 80 in dt-schema as well, right?

Yes, and up to 100 is fine as an exception.

> >
> > > +          Source PLL A (0) or B (1) for the corresponding multisynth divider.

But this doesn't look like it is over 80. Maybe if you put after
'description:' on the same line, but that's not what I said. It can
still be on the next line. No '|' just means the line endings aren't
fixed. Not important now, but if we were to generate pretty
documentation from the schemas, then it would matter.

>
> [...]
>
> > > +  - if:
> > > +      properties:
> > > +        compatible:
> > > +          contains:
> > > +            enum:
> > > +              - silabs,si5351a
> > > +              - silabs,si5351a-msop
> > > +              - silabs,si5351b
> >
> > Isn't this just the 'else' for the next one? Or more parts are coming?
>
> Not sure if more parts are coming - these are the only ones I am aware of. But I
> have not checked thoroughly. I thought it better to be explicit, but I will
> change the next one to an else: in v3 unless you change your mind.
>
> >
> > > +    then:
> > > +      properties:
> > > +        clocks:
> > > +          minItems: 1
> > > +          maxItems: 1
> > > +        clock-names:
> > > +          items:
> > > +            - const: xtal
> > > +
> > > +  - if:
> > > +      properties:
> > > +        compatible:
> > > +          contains:
> > > +            const: silabs,si5351c
> > > +    then:
> > > +      properties:
> > > +        clocks:
> > > +          minItems: 1
> > > +          maxItems: 2
> > > +        clock-names:
> > > +          minItems: 1
> > > +          items:
> > > +            - const: xtal
> > > +            - const: clkin
> >
> > Define clocks and clock-names at the top level and just use
> > minItems/maxItems in the if/then schemas.
>
> I was trying to imply here that it is invalid to specify clkin for the former
> three part types - only for the si5351c. If I specify both in the top-level
> clock-names:items then it would allow something like this:
>
>   clk {
>     compatible = "silabs,si5351a-msop";
>     clocks = <&ref25>;
>     clock-names = "clkin"; /* not OK - Si5351A-MSOP only supports XTAL */
>   };

What I'm saying will work. There are lots of examples in the tree. The
top-level defines the full array and then the if/then schema just sets
the max size to 1 so that the clkin entry is not allowed.

properties:
  clock-names:
    minItems: 1
    items:
      - const: xtal
      - const: clkin

if:
  properties:
    compatible:
      contains:
        const: silabs,si5351a-msop
then:
  properties:
    clock-names:
      maxItems: 1

(and then "minItems: 2" for the cases with 2 clocks)

Rob

Re: [PATCH v2 1/4] dt-bindings: clock: si5351: convert to yaml

[Alvin Šipraga]

On Tue, Oct 10, 2023 at 09:50:03AM -0500, Rob Herring wrote:
> On Wed, Oct 4, 2023 at 4:40 PM Alvin Šipraga <ALSI@bang-olufsen.dk> wrote:
> >
> > On Wed, Oct 04, 2023 at 09:39:37AM -0500, Rob Herring wrote:
> > > > +      silabs,multisynth-source:
> > > > +        $ref: /schemas/types.yaml#/definitions/uint32
> > > > +        enum: [ 0, 1 ]
> > > > +        description: |
> > >
> > > Don't need '|' if no formatting to preserve.
> >
> > I thought the line would be too long otherwise.
> > Column width is 80 in dt-schema as well, right?
> 
> Yes, and up to 100 is fine as an exception.
> 
> > >
> > > > +          Source PLL A (0) or B (1) for the corresponding multisynth divider.
> 
> But this doesn't look like it is over 80. Maybe if you put after
> 'description:' on the same line, but that's not what I said. It can
> still be on the next line. No '|' just means the line endings aren't
> fixed. Not important now, but if we were to generate pretty
> documentation from the schemas, then it would matter.

Ah I see. OK, I removed the | now and it works fine. :)

> 
> >
> > [...]
> >
> > > > +  - if:
> > > > +      properties:
> > > > +        compatible:
> > > > +          contains:
> > > > +            enum:
> > > > +              - silabs,si5351a
> > > > +              - silabs,si5351a-msop
> > > > +              - silabs,si5351b
> > >
> > > Isn't this just the 'else' for the next one? Or more parts are coming?
> >
> > Not sure if more parts are coming - these are the only ones I am aware of. But I
> > have not checked thoroughly. I thought it better to be explicit, but I will
> > change the next one to an else: in v3 unless you change your mind.
> >
> > >
> > > > +    then:
> > > > +      properties:
> > > > +        clocks:
> > > > +          minItems: 1
> > > > +          maxItems: 1
> > > > +        clock-names:
> > > > +          items:
> > > > +            - const: xtal
> > > > +
> > > > +  - if:
> > > > +      properties:
> > > > +        compatible:
> > > > +          contains:
> > > > +            const: silabs,si5351c
> > > > +    then:
> > > > +      properties:
> > > > +        clocks:
> > > > +          minItems: 1
> > > > +          maxItems: 2
> > > > +        clock-names:
> > > > +          minItems: 1
> > > > +          items:
> > > > +            - const: xtal
> > > > +            - const: clkin
> > >
> > > Define clocks and clock-names at the top level and just use
> > > minItems/maxItems in the if/then schemas.
> >
> > I was trying to imply here that it is invalid to specify clkin for the former
> > three part types - only for the si5351c. If I specify both in the top-level
> > clock-names:items then it would allow something like this:
> >
> >   clk {
> >     compatible = "silabs,si5351a-msop";
> >     clocks = <&ref25>;
> >     clock-names = "clkin"; /* not OK - Si5351A-MSOP only supports XTAL */
> >   };
> 
> What I'm saying will work. There are lots of examples in the tree. The
> top-level defines the full array and then the if/then schema just sets
> the max size to 1 so that the clkin entry is not allowed.
> 
> properties:
>   clock-names:
>     minItems: 1
>     items:
>       - const: xtal
>       - const: clkin
> 
> if:
>   properties:
>     compatible:
>       contains:
>         const: silabs,si5351a-msop
> then:
>   properties:
>     clock-names:
>       maxItems: 1
> 
> (and then "minItems: 2" for the cases with 2 clocks)

Thanks Rob, your suggestion worked! I will send a new version now.

Kind regards,
Alvin

[PATCH v2 2/4] ARM: dts: dove-cubox: fix si5351 node names

[Alvin Šipraga]

From: Alvin Šipraga <alsi@bang-olufsen.dk>

Correct the device tree to conform with the bindings. The node name and
index should be separated with an @.

Suggested-by: Rob Herring <robh@kernel.org>
Signed-off-by: Alvin Šipraga <alsi@bang-olufsen.dk>
---
 arch/arm/boot/dts/marvell/dove-cubox.dts | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/arch/arm/boot/dts/marvell/dove-cubox.dts b/arch/arm/boot/dts/marvell/dove-cubox.dts
index bfde99486a87..bcaaf8320c45 100644
--- a/arch/arm/boot/dts/marvell/dove-cubox.dts
+++ b/arch/arm/boot/dts/marvell/dove-cubox.dts
@@ -101,7 +101,7 @@ si5351: clock-generator@60 {
 		/* connect xtal input as source of pll0 and pll1 */
 		silabs,pll-source = <0 0>, <1 0>;
 
-		clkout0 {
+		clkout@0 {
 			reg = <0>;
 			silabs,drive-strength = <8>;
 			silabs,multisynth-source = <0>;
@@ -109,7 +109,7 @@ clkout0 {
 			silabs,pll-master;
 		};
 
-		clkout2 {
+		clkout@2 {
 			reg = <2>;
 			silabs,drive-strength = <8>;
 			silabs,multisynth-source = <1>;
-- 
2.42.0

Re: [PATCH v2 2/4] ARM: dts: dove-cubox: fix si5351 node names

[Krzysztof Kozlowski]

On 04/10/2023 08:35, Alvin Šipraga wrote:
> From: Alvin Šipraga <alsi@bang-olufsen.dk>
> 
> Correct the device tree to conform with the bindings. The node name and
> index should be separated with an @.
> 
> Suggested-by: Rob Herring <robh@kernel.org>
> Signed-off-by: Alvin Šipraga <alsi@bang-olufsen.dk>
> ---

This is independent change, please do not mix it with this series. DTS
should go via ARM SOC, not drivers subsystem tree.

Best regards,
Krzysztof

Re: (subset) [PATCH v2 2/4] ARM: dts: dove-cubox: fix si5351 node names

[Krzysztof Kozlowski]

On Wed, 04 Oct 2023 08:35:28 +0200, Alvin Šipraga wrote:
> Correct the device tree to conform with the bindings. The node name and
> index should be separated with an @.
> 
> 

Applied, thanks!

[2/4] ARM: dts: dove-cubox: fix si5351 node names
      https://git.kernel.org/krzk/linux-dt/c/2df26223650027602d017368f4e8dc1eff90404e

Best regards,
-- 
Krzysztof Kozlowski <krzysztof.kozlowski@linaro.org>

[PATCH v2 3/4] dt-bindings: clock: si5351: add PLL reset mode property

[Alvin Šipraga]

From: Alvin Šipraga <alsi@bang-olufsen.dk>

For applications where the PLL must be adjusted without glitches in the
clock output(s), a new silabs,pll-reset-mode property is added. It
can be used to specify whether or not the PLL should be reset after
adjustment. Resetting is known to cause glitches.

For compatibility with older device trees, it must be assumed that the
default PLL reset mode is to unconditionally reset after adjustment.

Cc: Sebastian Hesselbarth <sebastian.hesselbarth@gmail.com>
Cc: Rabeeh Khoury <rabeeh@solid-run.com>
Cc: Jacob Siverskog <jacob@teenage.engineering>
Cc: Sergej Sawazki <sergej@taudac.com>
Signed-off-by: Alvin Šipraga <alsi@bang-olufsen.dk>
---
 .../bindings/clock/silabs,si5351.yaml         | 24 +++++++++++++++++++
 1 file changed, 24 insertions(+)

diff --git a/Documentation/devicetree/bindings/clock/silabs,si5351.yaml b/Documentation/devicetree/bindings/clock/silabs,si5351.yaml
index 400c8cec2a3a..f1be09b5c48c 100644
--- a/Documentation/devicetree/bindings/clock/silabs,si5351.yaml
+++ b/Documentation/devicetree/bindings/clock/silabs,si5351.yaml
@@ -53,6 +53,27 @@ properties:
         - description: PLL source, XTAL (0) or CLKIN (1, Si5351C only).
           enum: [ 0, 1 ]
 
+  silabs,pll-reset-mode:
+    $ref: /schemas/types.yaml#/definitions/uint32-matrix
+    minItems: 1
+    description: |
+      A list of cell pairs containing a PLL index and its reset mode.
+    items:
+      items:
+        - description: PLL A (0) or PLL B (1)
+          enum: [ 0, 1 ]
+        - description: |
+            Reset mode for the PLL. Mode can be one of:
+
+                0 - reset whenever PLL rate is adjusted (default mode)
+                1 - do not reset when PLL rate is adjusted
+
+            In mode 1, the PLL is only reset if the silabs,pll-reset is
+            specified in one of the clock output child nodes that also sources
+            the PLL. This mode may be preferable if output clocks are expected
+            to be adjusted without glitches.
+          enum: [ 0, 1 ]
+
 patternProperties:
   "^clkout@[0-7]$":
     type: object
@@ -207,6 +228,9 @@ examples:
         /* Use XTAL input as source of PLL0 and PLL1 */
         silabs,pll-source = <0 0>, <1 0>;
 
+        /* Don't reset PLL1 on rate adjustment */
+        silabs,pll-reset-mode = <1 1>;
+
         /*
          * Overwrite CLK0 configuration with:
          * - 8 mA output drive strength
-- 
2.42.0

[PATCH v2 4/4] clk: si5351: allow PLLs to be adjusted without reset

[Alvin Šipraga]

From: Alvin Šipraga <alsi@bang-olufsen.dk>

Introduce a new PLL reset mode flag which controls whether or not to
reset a PLL after adjusting its rate. The mode can be configured through
platform data or device tree.

Since commit 6dc669a22c77 ("clk: si5351: Add PLL soft reset"), the
driver unconditionally resets a PLL whenever its rate is adjusted.
The rationale was that a PLL reset was required to get three outputs
working at the same time. Before this change, the driver never reset the
PLLs.

Commit b26ff127c52c ("clk: si5351: Apply PLL soft reset before enabling
the outputs") subsequently introduced an option to reset the PLL when
enabling a clock output that sourced it. Here, the rationale was that
this is required to get a deterministic phase relationship between
multiple output clocks.

This clearly shows that it is useful to reset the PLLs in applications
where multiple clock outputs are used. However, the Si5351 also allows
for glitch-free rate adjustment of its PLLs if one avoids resetting the
PLL. In our audio application where a single Si5351 clock output is used
to supply a runtime adjustable bit clock, this unconditional PLL reset
behaviour introduces unwanted glitches in the clock output.

It would appear that the problem being solved in the former commit
may be solved by using the optional device tree property introduced in
the latter commit, obviating the need for an unconditional PLL reset
after rate adjustment. But it's not OK to break the default behaviour of
the driver, and it cannot be assumed that all device trees are using the
property introduced in the latter commit. Hence, the new behaviour is
made opt-in.

Cc: Sebastian Hesselbarth <sebastian.hesselbarth@gmail.com>
Cc: Rabeeh Khoury <rabeeh@solid-run.com>
Cc: Jacob Siverskog <jacob@teenage.engineering>
Cc: Sergej Sawazki <sergej@taudac.com>
Signed-off-by: Alvin Šipraga <alsi@bang-olufsen.dk>
---
 drivers/clk/clk-si5351.c             | 47 ++++++++++++++++++++++++++--
 include/linux/platform_data/si5351.h |  2 ++
 2 files changed, 46 insertions(+), 3 deletions(-)

diff --git a/drivers/clk/clk-si5351.c b/drivers/clk/clk-si5351.c
index 00fb9b09e030..95d7afb8cfc6 100644
--- a/drivers/clk/clk-si5351.c
+++ b/drivers/clk/clk-si5351.c
@@ -506,6 +506,8 @@ static int si5351_pll_set_rate(struct clk_hw *hw, unsigned long rate,
 {
 	struct si5351_hw_data *hwdata =
 		container_of(hw, struct si5351_hw_data, hw);
+	struct si5351_platform_data *pdata =
+		hwdata->drvdata->client->dev.platform_data;
 	u8 reg = (hwdata->num == 0) ? SI5351_PLLA_PARAMETERS :
 		SI5351_PLLB_PARAMETERS;
 
@@ -518,9 +520,10 @@ static int si5351_pll_set_rate(struct clk_hw *hw, unsigned long rate,
 		(hwdata->params.p2 == 0) ? SI5351_CLK_INTEGER_MODE : 0);
 
 	/* Do a pll soft reset on the affected pll */
-	si5351_reg_write(hwdata->drvdata, SI5351_PLL_RESET,
-			 hwdata->num == 0 ? SI5351_PLL_RESET_A :
-					    SI5351_PLL_RESET_B);
+	if (pdata->pll_reset[hwdata->num])
+		si5351_reg_write(hwdata->drvdata, SI5351_PLL_RESET,
+				 hwdata->num == 0 ? SI5351_PLL_RESET_A :
+						    SI5351_PLL_RESET_B);
 
 	dev_dbg(&hwdata->drvdata->client->dev,
 		"%s - %s: p1 = %lu, p2 = %lu, p3 = %lu, parent_rate = %lu, rate = %lu\n",
@@ -1222,6 +1225,44 @@ static int si5351_dt_parse(struct i2c_client *client,
 		}
 	}
 
+	/*
+	 * Parse PLL reset mode. For compatibility with older device trees, the
+	 * default is to always reset a PLL after setting its rate.
+	 */
+	pdata->pll_reset[0] = true;
+	pdata->pll_reset[1] = true;
+
+	of_property_for_each_u32(np, "silabs,pll-reset-mode", prop, p, num) {
+		if (num >= 2) {
+			dev_err(&client->dev,
+				"invalid pll %d on pll-reset-mode prop\n", num);
+			return -EINVAL;
+		}
+
+		p = of_prop_next_u32(prop, p, &val);
+		if (!p) {
+			dev_err(&client->dev,
+				"missing pll-reset-mode for pll %d\n", num);
+			return -EINVAL;
+		}
+
+		switch (val) {
+		case 0:
+			/* Reset PLL whenever its rate is adjusted */
+			pdata->pll_reset[num] = true;
+			break;
+		case 1:
+			/* Don't reset PLL whenever its rate is adjusted */
+			pdata->pll_reset[num] = false;
+			break;
+		default:
+			dev_err(&client->dev,
+				"invalid pll-reset-mode %d for pll %d\n", val,
+				num);
+			return -EINVAL;
+		}
+	}
+
 	/* per clkout properties */
 	for_each_child_of_node(np, child) {
 		if (of_property_read_u32(child, "reg", &num)) {
diff --git a/include/linux/platform_data/si5351.h b/include/linux/platform_data/si5351.h
index c71a2dd66143..5f412a615532 100644
--- a/include/linux/platform_data/si5351.h
+++ b/include/linux/platform_data/si5351.h
@@ -105,10 +105,12 @@ struct si5351_clkout_config {
  * @clk_xtal: xtal input clock
  * @clk_clkin: clkin input clock
  * @pll_src: array of pll source clock setting
+ * @pll_reset: array indicating if plls should be reset after setting the rate
  * @clkout: array of clkout configuration
  */
 struct si5351_platform_data {
 	enum si5351_pll_src pll_src[2];
+	bool pll_reset[2];
 	struct si5351_clkout_config clkout[8];
 };
 
-- 
2.42.0

Re: [PATCH v2 4/4] clk: si5351: allow PLLs to be adjusted without reset

[Sebastian Hesselbarth]

Hi Alvin,

thanks for the patch. In general, I am fine with the change as default behavior is to do what it did before.

So,
Acked-by: <sebastian.hesselbarth@gmail.com>
for the functional changes.

For the DT changes you'll need Rob, Stephen, Michael's approval more than mine.

However, as Jacob and Sergej already noticed on their patches, I cannot spend enough time for maintaining the driver anymore.

Is there anyone volunteering to pick maintainership up?

Regards,
  Sebastian

(Hopefully plain/text now)


Am 4. Oktober 2023 08:35:30 MESZ schrieb "Alvin Šipraga" <alvin@pqrs.dk>:
>From: Alvin Šipraga <alsi@bang-olufsen.dk>
>
>Introduce a new PLL reset mode flag which controls whether or not to
>reset a PLL after adjusting its rate. The mode can be configured through
>platform data or device tree.
>
>Since commit 6dc669a22c77 ("clk: si5351: Add PLL soft reset"), the
>driver unconditionally resets a PLL whenever its rate is adjusted.
>The rationale was that a PLL reset was required to get three outputs
>working at the same time. Before this change, the driver never reset the
>PLLs.
>
>Commit b26ff127c52c ("clk: si5351: Apply PLL soft reset before enabling
>the outputs") subsequently introduced an option to reset the PLL when
>enabling a clock output that sourced it. Here, the rationale was that
>this is required to get a deterministic phase relationship between
>multiple output clocks.
>
>This clearly shows that it is useful to reset the PLLs in applications
>where multiple clock outputs are used. However, the Si5351 also allows
>for glitch-free rate adjustment of its PLLs if one avoids resetting the
>PLL. In our audio application where a single Si5351 clock output is used
>to supply a runtime adjustable bit clock, this unconditional PLL reset
>behaviour introduces unwanted glitches in the clock output.
>
>It would appear that the problem being solved in the former commit
>may be solved by using the optional device tree property introduced in
>the latter commit, obviating the need for an unconditional PLL reset
>after rate adjustment. But it's not OK to break the default behaviour of
>the driver, and it cannot be assumed that all device trees are using the
>property introduced in the latter commit. Hence, the new behaviour is
>made opt-in.
>
>Cc: Sebastian Hesselbarth <sebastian.hesselbarth@gmail.com>
>Cc: Rabeeh Khoury <rabeeh@solid-run.com>
>Cc: Jacob Siverskog <jacob@teenage.engineering>
>Cc: Sergej Sawazki <sergej@taudac.com>
>Signed-off-by: Alvin Šipraga <alsi@bang-olufsen.dk>
>---
> drivers/clk/clk-si5351.c             | 47 ++++++++++++++++++++++++++--
> include/linux/platform_data/si5351.h |  2 ++
> 2 files changed, 46 insertions(+), 3 deletions(-)
>
>diff --git a/drivers/clk/clk-si5351.c b/drivers/clk/clk-si5351.c
>index 00fb9b09e030..95d7afb8cfc6 100644
>--- a/drivers/clk/clk-si5351.c
>+++ b/drivers/clk/clk-si5351.c
>@@ -506,6 +506,8 @@ static int si5351_pll_set_rate(struct clk_hw *hw, unsigned long rate,
> {
> 	struct si5351_hw_data *hwdata =
> 		container_of(hw, struct si5351_hw_data, hw);
>+	struct si5351_platform_data *pdata =
>+		hwdata->drvdata->client->dev.platform_data;
> 	u8 reg = (hwdata->num == 0) ? SI5351_PLLA_PARAMETERS :
> 		SI5351_PLLB_PARAMETERS;
> 
>@@ -518,9 +520,10 @@ static int si5351_pll_set_rate(struct clk_hw *hw, unsigned long rate,
> 		(hwdata->params.p2 == 0) ? SI5351_CLK_INTEGER_MODE : 0);
> 
> 	/* Do a pll soft reset on the affected pll */
>-	si5351_reg_write(hwdata->drvdata, SI5351_PLL_RESET,
>-			 hwdata->num == 0 ? SI5351_PLL_RESET_A :
>-					    SI5351_PLL_RESET_B);
>+	if (pdata->pll_reset[hwdata->num])
>+		si5351_reg_write(hwdata->drvdata, SI5351_PLL_RESET,
>+				 hwdata->num == 0 ? SI5351_PLL_RESET_A :
>+						    SI5351_PLL_RESET_B);
> 
> 	dev_dbg(&hwdata->drvdata->client->dev,
> 		"%s - %s: p1 = %lu, p2 = %lu, p3 = %lu, parent_rate = %lu, rate = %lu\n",
>@@ -1222,6 +1225,44 @@ static int si5351_dt_parse(struct i2c_client *client,
> 		}
> 	}
> 
>+	/*
>+	 * Parse PLL reset mode. For compatibility with older device trees, the
>+	 * default is to always reset a PLL after setting its rate.
>+	 */
>+	pdata->pll_reset[0] = true;
>+	pdata->pll_reset[1] = true;
>+
>+	of_property_for_each_u32(np, "silabs,pll-reset-mode", prop, p, num) {
>+		if (num >= 2) {
>+			dev_err(&client->dev,
>+				"invalid pll %d on pll-reset-mode prop\n", num);
>+			return -EINVAL;
>+		}
>+
>+		p = of_prop_next_u32(prop, p, &val);
>+		if (!p) {
>+			dev_err(&client->dev,
>+				"missing pll-reset-mode for pll %d\n", num);
>+			return -EINVAL;
>+		}
>+
>+		switch (val) {
>+		case 0:
>+			/* Reset PLL whenever its rate is adjusted */
>+			pdata->pll_reset[num] = true;
>+			break;
>+		case 1:
>+			/* Don't reset PLL whenever its rate is adjusted */
>+			pdata->pll_reset[num] = false;
>+			break;
>+		default:
>+			dev_err(&client->dev,
>+				"invalid pll-reset-mode %d for pll %d\n", val,
>+				num);
>+			return -EINVAL;
>+		}
>+	}
>+
> 	/* per clkout properties */
> 	for_each_child_of_node(np, child) {
> 		if (of_property_read_u32(child, "reg", &num)) {
>diff --git a/include/linux/platform_data/si5351.h b/include/linux/platform_data/si5351.h
>index c71a2dd66143..5f412a615532 100644
>--- a/include/linux/platform_data/si5351.h
>+++ b/include/linux/platform_data/si5351.h
>@@ -105,10 +105,12 @@ struct si5351_clkout_config {
>  * @clk_xtal: xtal input clock
>  * @clk_clkin: clkin input clock
>  * @pll_src: array of pll source clock setting
>+ * @pll_reset: array indicating if plls should be reset after setting the rate
>  * @clkout: array of clkout configuration
>  */
> struct si5351_platform_data {
> 	enum si5351_pll_src pll_src[2];
>+	bool pll_reset[2];
> 	struct si5351_clkout_config clkout[8];
> };
>