[PATCH RFC 1/3] dt-bindings: clock: Add Renesas versa3 clock generator bindings

[PATCH RFC 1/3] dt-bindings: clock: Add Renesas versa3 clock generator bindings

[Biju Das]

Document Renesas versa3 clock generator(5P35023) bindings.

The 5P35023 is a VersaClock programmable clock generator and
is designed for low-power, consumer, and high-performance PCI
Express applications. The 5P35023 device is a three PLL
architecture design, and each PLL is individually programmable
and allowing for up to 6 unique frequency outputs.

Signed-off-by: Biju Das <biju.das.jz@bp.renesas.com>
---
 .../bindings/clock/renesas,versaclock3.yaml   | 135 ++++++++++++++++++
 1 file changed, 135 insertions(+)
 create mode 100644 Documentation/devicetree/bindings/clock/renesas,versaclock3.yaml

diff --git a/Documentation/devicetree/bindings/clock/renesas,versaclock3.yaml b/Documentation/devicetree/bindings/clock/renesas,versaclock3.yaml
new file mode 100644
index 000000000000..f45b8da73ec3
--- /dev/null
+++ b/Documentation/devicetree/bindings/clock/renesas,versaclock3.yaml
@@ -0,0 +1,135 @@
+# SPDX-License-Identifier: (GPL-2.0-only OR BSD-2-Clause)
+%YAML 1.2
+---
+$id: http://devicetree.org/schemas/clock/renesas,versaclock3.yaml#
+$schema: http://devicetree.org/meta-schemas/core.yaml#
+
+title: Renesas VersaClock 3 programmable I2C clock generators
+
+description: |
+  The 5P35023 is a VersaClock programmable clock generator and
+  is designed for low-power, consumer, and high-performance PCI
+  express applications. The 5P35023 device is a three PLL
+  architecture design, and each PLL is individually programmable
+  and allowing for up to 6 unique frequency outputs.
+
+  An internal OTP memory allows the user to store the configuration
+  in the device. After power up, the user can change the device register
+  settings through the I2C interface when I2C mode is selected.
+
+  The driver can read a full register map from the DT, and will use that
+  register map to initialize the attached part (via I2C) when the system
+  boots. Any configuration not supported by the common clock framework
+  must be done via the full register map, including optimized settings.
+
+  Link to datasheet: https://www.renesas.com/us/en/products/clocks-timing/
+                     clock-generation/programmable-clocks/
+                     5p35023-versaclock-3s-programmable-clock-generator
+
+maintainers:
+  - Biju Das <biju.das.jz@bp.renesas.com>
+
+properties:
+  compatible:
+    enum:
+      - renesas,5p35023
+
+  reg:
+    description: I2C device address
+    maxItems: 1
+
+  '#clock-cells':
+    const: 1
+
+  clock-names:
+    oneOf:
+      - items:
+          - const: x1
+      - items:
+          - const: clkin
+
+  clocks:
+    maxItems: 1
+
+  renesas,settings:
+    description: Optional, complete register map of the device.
+      Optimized settings for the device must be provided in full
+      and are written during initialization.
+    $ref: /schemas/types.yaml#/definitions/uint8-array
+    minItems: 37
+
+  assigned-clocks:
+    minItems: 6
+
+  assigned-clock-rates:
+    minItems: 6
+
+  renesas,clock-divider-read-only:
+    description: Flag for setting divider in read only mode.
+    $ref: /schemas/types.yaml#/definitions/uint32-array
+    minItems: 5
+
+  renesas,clock-flags:
+    description: Flags used in common clock frame work for configuring
+      clk outputs. See include/linux/clk-provider.h
+    $ref: /schemas/types.yaml#/definitions/uint32-array
+    minItems: 6
+
+required:
+  - compatible
+  - reg
+  - '#clock-cells'
+  - clocks
+  - clock-names
+
+additionalProperties: false
+
+examples:
+  - |
+    /* 24MHz crystal */
+    x1_x2: xtal {
+      compatible = "fixed-clock";
+      #clock-cells = <0>;
+      clock-frequency = <24000000>;
+    };
+
+    i2c@0 {
+        reg = <0x0 0x100>;
+        #address-cells = <1>;
+        #size-cells = <0>;
+
+        versa3: clock-generator@68 {
+            compatible = "renesas,5p35023";
+            reg = <0x68>;
+            #clock-cells = <1>;
+
+            clocks = <&x1_x2>;
+            clock-names = "x1";
+
+            renesas,settings = [
+                80 00 11 19 4c 02 23 7f 83 19 08 a9 5f 25 24 bf
+                00 14 7a e1 00 00 00 00 01 55 59 bb 3f 30 90 b6
+                80 b0 45 c4 95
+            ];
+
+            assigned-clocks = <&versa3 0>,
+                              <&versa3 1>,
+                              <&versa3 2>,
+                              <&versa3 3>,
+                              <&versa3 4>,
+                              <&versa3 5>;
+            assigned-clock-rates = <12288000>, <25000000>,
+                                   <12000000>, <11289600>,
+                                   <11289600>, <24000000>;
+            renesas,clock-divider-read-only = <1>, <1>, <1>, <1>, <1>;
+            renesas,clock-flags = <2176>, <2176>, <2176>, <2052>,
+                                  <2176>, <2048>;
+        };
+    };
+
+    /* Consumer referencing the versa 3 */
+    consumer {
+        /* ... */
+        clocks = <&versa3 3>;
+        /* ... */
+    };
-- 
2.25.1

Re: [PATCH RFC 1/3] dt-bindings: clock: Add Renesas versa3 clock generator bindings

[Krzysztof Kozlowski]

On 20/02/2023 14:13, Biju Das wrote:
> Document Renesas versa3 clock generator(5P35023) bindings.
> 
> The 5P35023 is a VersaClock programmable clock generator and
> is designed for low-power, consumer, and high-performance PCI
> Express applications. The 5P35023 device is a three PLL
> architecture design, and each PLL is individually programmable
> and allowing for up to 6 unique frequency outputs.
> 
> Signed-off-by: Biju Das <biju.das.jz@bp.renesas.com>
> ---
>  .../bindings/clock/renesas,versaclock3.yaml   | 135 ++++++++++++++++++
>  1 file changed, 135 insertions(+)
>  create mode 100644 Documentation/devicetree/bindings/clock/renesas,versaclock3.yaml
> 
> diff --git a/Documentation/devicetree/bindings/clock/renesas,versaclock3.yaml b/Documentation/devicetree/bindings/clock/renesas,versaclock3.yaml
> new file mode 100644
> index 000000000000..f45b8da73ec3
> --- /dev/null
> +++ b/Documentation/devicetree/bindings/clock/renesas,versaclock3.yaml

Filename usually is based on the compatible. Why these two are so different?


> @@ -0,0 +1,135 @@
> +# SPDX-License-Identifier: (GPL-2.0-only OR BSD-2-Clause)
> +%YAML 1.2
> +---
> +$id: http://devicetree.org/schemas/clock/renesas,versaclock3.yaml#
> +$schema: http://devicetree.org/meta-schemas/core.yaml#
> +
> +title: Renesas VersaClock 3 programmable I2C clock generators
> +
> +description: |
> +  The 5P35023 is a VersaClock programmable clock generator and
> +  is designed for low-power, consumer, and high-performance PCI
> +  express applications. The 5P35023 device is a three PLL
> +  architecture design, and each PLL is individually programmable
> +  and allowing for up to 6 unique frequency outputs.
> +
> +  An internal OTP memory allows the user to store the configuration
> +  in the device. After power up, the user can change the device register
> +  settings through the I2C interface when I2C mode is selected.
> +
> +  The driver can read a full register map from the DT, and will use that
> +  register map to initialize the attached part (via I2C) when the system
> +  boots. Any configuration not supported by the common clock framework
> +  must be done via the full register map, including optimized settings.
> +
> +  Link to datasheet: https://www.renesas.com/us/en/products/clocks-timing/
> +                     clock-generation/programmable-clocks/
> +                     5p35023-versaclock-3s-programmable-clock-generator

I think link should not be wrapped. Start in next line and just make it
long.

While touching this, please keep the same order of entries as in
example-schema, so maintainers go after title.

> +
> +maintainers:
> +  - Biju Das <biju.das.jz@bp.renesas.com>
> +
> +properties:
> +  compatible:
> +    enum:
> +      - renesas,5p35023
> +
> +  reg:
> +    description: I2C device address

Drop description, it's obvious.

> +    maxItems: 1
> +
> +  '#clock-cells':
> +    const: 1
> +
> +  clock-names:
> +    oneOf:
> +      - items:
> +          - const: x1
> +      - items:
> +          - const: clkin

This should be specific, not one or another. Why do you have two
entirely different clock inputs?

(and if this stays, then just items: - enum: [])


> +
> +  clocks:
> +    maxItems: 1
> +
> +  renesas,settings:
> +    description: Optional, complete register map of the device.
> +      Optimized settings for the device must be provided in full
> +      and are written during initialization.
> +    $ref: /schemas/types.yaml#/definitions/uint8-array
> +    minItems: 37

maxItems instead... but I am not sure that we allow register settings in
DT in general.

> +
> +  assigned-clocks:
> +    minItems: 6

Drop.

> +
> +  assigned-clock-rates:
> +    minItems: 6

Drop.

> +
> +  renesas,clock-divider-read-only:
> +    description: Flag for setting divider in read only mode.

Flag? Then type: boolean.

> +    $ref: /schemas/types.yaml#/definitions/uint32-array
> +    minItems: 5

This is broken...

> +
> +  renesas,clock-flags:
> +    description: Flags used in common clock frame work for configuring
> +      clk outputs. See include/linux/clk-provider.h

These are not bindings, so why do you non-bindings constants as
bindings? They can change anytime. Choose one:
1. Drop entire property,
2. Make it a proper binding property, so an ABI and explain why this is
DT specific. None of clock providers have to do it, so you need here
good explanation.

> +    $ref: /schemas/types.yaml#/definitions/uint32-array
> +    minItems: 6

maxItems instead

> +
> +required:
> +  - compatible
> +  - reg
> +  - '#clock-cells'
> +  - clocks
> +  - clock-names
> +
> +additionalProperties: false
> +
> +examples:
> +  - |
> +    /* 24MHz crystal */
> +    x1_x2: xtal {
> +      compatible = "fixed-clock";
> +      #clock-cells = <0>;
> +      clock-frequency = <24000000>;
> +    };

Drop this part, obvious.

> +
> +    i2c@0 {
> +        reg = <0x0 0x100>;

Just i2c { and drop reg

> +        #address-cells = <1>;
> +        #size-cells = <0>;
> +
> +        versa3: clock-generator@68 {
> +            compatible = "renesas,5p35023";
> +            reg = <0x68>;
> +            #clock-cells = <1>;
> +
> +            clocks = <&x1_x2>;
> +            clock-names = "x1";
> +
> +            renesas,settings = [
> +                80 00 11 19 4c 02 23 7f 83 19 08 a9 5f 25 24 bf
> +                00 14 7a e1 00 00 00 00 01 55 59 bb 3f 30 90 b6
> +                80 b0 45 c4 95
> +            ];
> +
> +            assigned-clocks = <&versa3 0>,
> +                              <&versa3 1>,
> +                              <&versa3 2>,
> +                              <&versa3 3>,
> +                              <&versa3 4>,
> +                              <&versa3 5>;
> +            assigned-clock-rates = <12288000>, <25000000>,
> +                                   <12000000>, <11289600>,
> +                                   <11289600>, <24000000>;
> +            renesas,clock-divider-read-only = <1>, <1>, <1>, <1>, <1>;
> +            renesas,clock-flags = <2176>, <2176>, <2176>, <2052>,
> +                                  <2176>, <2048>;
> +        };
> +    };
> +
> +    /* Consumer referencing the versa 3 */
> +    consumer {
> +        /* ... */
> +        clocks = <&versa3 3>;
> +        /* ... */

Drop consumer. Do you see it anywhere else?

Best regards,
Krzysztof

RE: [PATCH RFC 1/3] dt-bindings: clock: Add Renesas versa3 clock generator bindings

[Biju Das]

Hi Krzysztof Kozlowski,

Thanks for the feedback.

> -----Original Message-----
> From: Krzysztof Kozlowski <krzysztof.kozlowski@linaro.org>
> Sent: Wednesday, February 22, 2023 9:34 AM
> To: Biju Das <biju.das.jz@bp.renesas.com>; Michael Turquette
> <mturquette@baylibre.com>; Stephen Boyd <sboyd@kernel.org>; Rob Herring
> <robh+dt@kernel.org>; Krzysztof Kozlowski
> <krzysztof.kozlowski+dt@linaro.org>
> Cc: Geert Uytterhoeven <geert+renesas@glider.be>; linux-renesas-
> soc@vger.kernel.org; linux-clk@vger.kernel.org; devicetree@vger.kernel.org;
> Fabrizio Castro <fabrizio.castro.jz@renesas.com>
> Subject: Re: [PATCH RFC 1/3] dt-bindings: clock: Add Renesas versa3 clock
> generator bindings
> 
> On 20/02/2023 14:13, Biju Das wrote:
> > Document Renesas versa3 clock generator(5P35023) bindings.
> >
> > The 5P35023 is a VersaClock programmable clock generator and is
> > designed for low-power, consumer, and high-performance PCI Express
> > applications. The 5P35023 device is a three PLL architecture design,
> > and each PLL is individually programmable and allowing for up to 6
> > unique frequency outputs.
> >
> > Signed-off-by: Biju Das <biju.das.jz@bp.renesas.com>
> > ---
> >  .../bindings/clock/renesas,versaclock3.yaml   | 135 ++++++++++++++++++
> >  1 file changed, 135 insertions(+)
> >  create mode 100644
> > Documentation/devicetree/bindings/clock/renesas,versaclock3.yaml
> >
> > diff --git
> > a/Documentation/devicetree/bindings/clock/renesas,versaclock3.yaml
> > b/Documentation/devicetree/bindings/clock/renesas,versaclock3.yaml
> > new file mode 100644
> > index 000000000000..f45b8da73ec3
> > --- /dev/null
> > +++ b/Documentation/devicetree/bindings/clock/renesas,versaclock3.yaml
> 
> Filename usually is based on the compatible. Why these two are so different?

Versa3 clock generator has the following variants.

	5P35023, 5L35021, 5L35023 and 5P35021

RZ/G2L SMARC EVK uses 5P35023. So I used generic one as file name.
And added compatible for specific one.

> 
> 
> > +title: Renesas VersaClock 3 programmable I2C clock generators
> > +
> > +description: |
> > +  The 5P35023 is a VersaClock programmable clock generator and
> > +  is designed for low-power, consumer, and high-performance PCI
> > +  express applications. The 5P35023 device is a three PLL
> > +  architecture design, and each PLL is individually programmable
> > +  and allowing for up to 6 unique frequency outputs.
> > +
> > +  An internal OTP memory allows the user to store the configuration
> > + in the device. After power up, the user can change the device
> > + register  settings through the I2C interface when I2C mode is selected.
> > +
> > +  The driver can read a full register map from the DT, and will use
> > + that  register map to initialize the attached part (via I2C) when
> > + the system  boots. Any configuration not supported by the common
> > + clock framework  must be done via the full register map, including
> optimized settings.
> > +
> > +  Link to datasheet: https://www.renesas.com/us/en/products/clocks-
> timing/
> > +                     clock-generation/programmable-clocks/
> > +
> > + 5p35023-versaclock-3s-programmable-clock-generator
> 
> I think link should not be wrapped. Start in next line and just make it
> long.

OK.

> 
> While touching this, please keep the same order of entries as in example-
> schema, so maintainers go after title.

Agreed.

> 
> > +
> > +maintainers:
> > +  - Biju Das <biju.das.jz@bp.renesas.com>
> > +
> > +properties:
> > +  compatible:
> > +    enum:
> > +      - renesas,5p35023
> > +
> > +  reg:
> > +    description: I2C device address
> 
> Drop description, it's obvious.
OK.

> 
> > +    maxItems: 1
> > +
> > +  '#clock-cells':
> > +    const: 1
> > +
> > +  clock-names:
> > +    oneOf:
> > +      - items:
> > +          - const: x1
> > +      - items:
> > +          - const: clkin
> 
> This should be specific, not one or another. Why do you have two entirely
> different clock inputs?

Reference input can be Crystal oscillator interface input(x1) or differential
clock input pin(clkin)

> 
> (and if this stays, then just items: - enum: [])


OK, I will use enum.
> 
> 
> > +
> > +  clocks:
> > +    maxItems: 1
> > +
> > +  renesas,settings:
> > +    description: Optional, complete register map of the device.
> > +      Optimized settings for the device must be provided in full
> > +      and are written during initialization.
> > +    $ref: /schemas/types.yaml#/definitions/uint8-array
> > +    minItems: 37
> 
> maxItems instead... but I am not sure that we allow register settings in DT
> in general.

Agreed. I guess it is allowed [1]
[1] https://lore.kernel.org/linux-renesas-soc/833d3837892f0879233695636291af97a746e584.1643968653.git.michal.simek@xilinx.com/

> 
> > +
> > +  assigned-clocks:
> > +    minItems: 6
> 
> Drop.

OK.

> 
> > +
> > +  assigned-clock-rates:
> > +    minItems: 6
> 
> Drop.

OK.

> 
> > +
> > +  renesas,clock-divider-read-only:
> > +    description: Flag for setting divider in read only mode.
> 
> Flag? Then type: boolean.

Agreed.
> 
> > +    $ref: /schemas/types.yaml#/definitions/uint32-array
> > +    minItems: 5
> 
> This is broken...
OK you mean maxItems. Based on Boolean type I will update this
> 
> > +
> > +  renesas,clock-flags:
> > +    description: Flags used in common clock frame work for configuring
> > +      clk outputs. See include/linux/clk-provider.h
> 
> These are not bindings, so why do you non-bindings constants as bindings?
> They can change anytime. Choose one:
> 1. Drop entire property,
> 2. Make it a proper binding property, so an ABI and explain why this is DT
> specific. None of clock providers have to do it, so you need here good
> explanation.

I will choose 2 and will explain as user should be allowed to
configure the output clock from clk generator, so that it has flexibility
for
1) changing the rates (propagate rate change up one level)
2) fixed always 
3) don't gate the ouput clk at all.

> 
> > +    $ref: /schemas/types.yaml#/definitions/uint32-array
> > +    minItems: 6
> 
> maxItems instead

OK.

> 
> > +
> > +required:
> > +  - compatible
> > +  - reg
> > +  - '#clock-cells'
> > +  - clocks
> > +  - clock-names
> > +
> > +additionalProperties: false
> > +
> > +examples:
> > +  - |
> > +    /* 24MHz crystal */
> > +    x1_x2: xtal {
> > +      compatible = "fixed-clock";
> > +      #clock-cells = <0>;
> > +      clock-frequency = <24000000>;
> > +    };
> 
> Drop this part, obvious.
OK.
> 
> > +
> > +    i2c@0 {
> > +        reg = <0x0 0x100>;
> 
> Just i2c { and drop reg

Agreed.
> 
> > +        #address-cells = <1>;
> > +        #size-cells = <0>;
> > +
> > +        versa3: clock-generator@68 {
> > +            compatible = "renesas,5p35023";
> > +            reg = <0x68>;
> > +            #clock-cells = <1>;
> > +
> > +            clocks = <&x1_x2>;
> > +            clock-names = "x1";
> > +
> > +            renesas,settings = [
> > +                80 00 11 19 4c 02 23 7f 83 19 08 a9 5f 25 24 bf
> > +                00 14 7a e1 00 00 00 00 01 55 59 bb 3f 30 90 b6
> > +                80 b0 45 c4 95
> > +            ];
> > +
> > +            assigned-clocks = <&versa3 0>,
> > +                              <&versa3 1>,
> > +                              <&versa3 2>,
> > +                              <&versa3 3>,
> > +                              <&versa3 4>,
> > +                              <&versa3 5>;
> > +            assigned-clock-rates = <12288000>, <25000000>,
> > +                                   <12000000>, <11289600>,
> > +                                   <11289600>, <24000000>;
> > +            renesas,clock-divider-read-only = <1>, <1>, <1>, <1>, <1>;
> > +            renesas,clock-flags = <2176>, <2176>, <2176>, <2052>,
> > +                                  <2176>, <2048>;
> > +        };
> > +    };
> > +
> > +    /* Consumer referencing the versa 3 */
> > +    consumer {
> > +        /* ... */
> > +        clocks = <&versa3 3>;
> > +        /* ... */
> 
> Drop consumer. Do you see it anywhere else?

No, will drop.

Cheers,
Biju

Re: [PATCH RFC 1/3] dt-bindings: clock: Add Renesas versa3 clock generator bindings

[Geert Uytterhoeven]

Hi Biju,

On Wed, Mar 8, 2023 at 3:39 PM Biju Das <biju.das.jz@bp.renesas.com> wrote:
> > From: Krzysztof Kozlowski <krzysztof.kozlowski@linaro.org>
> > On 20/02/2023 14:13, Biju Das wrote:
> > > Document Renesas versa3 clock generator(5P35023) bindings.
> > >
> > > The 5P35023 is a VersaClock programmable clock generator and is
> > > designed for low-power, consumer, and high-performance PCI Express
> > > applications. The 5P35023 device is a three PLL architecture design,
> > > and each PLL is individually programmable and allowing for up to 6
> > > unique frequency outputs.
> > >
> > > Signed-off-by: Biju Das <biju.das.jz@bp.renesas.com>

> > > --- /dev/null
> > > +++ b/Documentation/devicetree/bindings/clock/renesas,versaclock3.yaml

> > > +  clock-names:
> > > +    oneOf:
> > > +      - items:
> > > +          - const: x1
> > > +      - items:
> > > +          - const: clkin
> >
> > This should be specific, not one or another. Why do you have two entirely
> > different clock inputs?
>
> Reference input can be Crystal oscillator interface input(x1) or differential
> clock input pin(clkin)

I believe that's purely a hardware feature, which does not need any
software configuration?
I.e. logically, there's just a single clock input, i.e. no need for clock-names.

Gr{oetje,eeting}s,

                        Geert

--
Geert Uytterhoeven -- There's lots of Linux beyond ia32 -- geert@linux-m68k.org

In personal conversations with technical people, I call myself a hacker. But
when I'm talking to journalists I just say "programmer" or something like that.
                                -- Linus Torvalds

RE: [PATCH RFC 1/3] dt-bindings: clock: Add Renesas versa3 clock generator bindings

[Biju Das]

Hi Geert,

Thanks for the feedback.

> Subject: Re: [PATCH RFC 1/3] dt-bindings: clock: Add Renesas versa3 clock
> generator bindings
> 
> Hi Biju,
> 
> On Wed, Mar 8, 2023 at 3:39 PM Biju Das <biju.das.jz@bp.renesas.com> wrote:
> > > From: Krzysztof Kozlowski <krzysztof.kozlowski@linaro.org> On
> > > 20/02/2023 14:13, Biju Das wrote:
> > > > Document Renesas versa3 clock generator(5P35023) bindings.
> > > >
> > > > The 5P35023 is a VersaClock programmable clock generator and is
> > > > designed for low-power, consumer, and high-performance PCI Express
> > > > applications. The 5P35023 device is a three PLL architecture
> > > > design, and each PLL is individually programmable and allowing for
> > > > up to 6 unique frequency outputs.
> > > >
> > > > Signed-off-by: Biju Das <biju.das.jz@bp.renesas.com>
> 
> > > > --- /dev/null
> > > > +++ b/Documentation/devicetree/bindings/clock/renesas,versaclock3.
> > > > +++ yaml
> 
> > > > +  clock-names:
> > > > +    oneOf:
> > > > +      - items:
> > > > +          - const: x1
> > > > +      - items:
> > > > +          - const: clkin
> > >
> > > This should be specific, not one or another. Why do you have two
> > > entirely different clock inputs?
> >
> > Reference input can be Crystal oscillator interface input(x1) or
> > differential clock input pin(clkin)
> 
> I believe that's purely a hardware feature, which does not need any software
> configuration?
> I.e. logically, there's just a single clock input, i.e. no need for clock-
> names.

OK. Agreed. Will remove clock-names.

Cheers,
Biju

Re: [PATCH RFC 1/3] dt-bindings: clock: Add Renesas versa3 clock generator bindings

[Krzysztof Kozlowski]

On 08/03/2023 15:39, Biju Das wrote:

>>> +++ b/Documentation/devicetree/bindings/clock/renesas,versaclock3.yaml
>>
>> Filename usually is based on the compatible. Why these two are so different?
> 
> Versa3 clock generator has the following variants.
> 
> 	5P35023, 5L35021, 5L35023 and 5P35021
> 
> RZ/G2L SMARC EVK uses 5P35023. So I used generic one as file name.
> And added compatible for specific one.

And what about other devices? Since you do not add them, just keep
compatible as filename.

>>
>>> +
>>> +  clocks:
>>> +    maxItems: 1
>>> +
>>> +  renesas,settings:
>>> +    description: Optional, complete register map of the device.
>>> +      Optimized settings for the device must be provided in full
>>> +      and are written during initialization.
>>> +    $ref: /schemas/types.yaml#/definitions/uint8-array
>>> +    minItems: 37
>>
>> maxItems instead... but I am not sure that we allow register settings in DT
>> in general.
> 
> Agreed. I guess it is allowed [1]
> [1] https://lore.kernel.org/linux-renesas-soc/833d3837892f0879233695636291af97a746e584.1643968653.git.michal.simek@xilinx.com/

I don't see Rob's review on this, so what do you prove exactly?

> 
>>
>>> +
>>> +  assigned-clocks:
>>> +    minItems: 6
>>
>> Drop.
> 
> OK.
> 
>>
>>> +
>>> +  assigned-clock-rates:
>>> +    minItems: 6
>>
>> Drop.
> 
> OK.
> 
>>
>>> +
>>> +  renesas,clock-divider-read-only:
>>> +    description: Flag for setting divider in read only mode.
>>
>> Flag? Then type: boolean.
> 
> Agreed.
>>
>>> +    $ref: /schemas/types.yaml#/definitions/uint32-array
>>> +    minItems: 5
>>
>> This is broken...
> OK you mean maxItems. Based on Boolean type I will update this

I mean, it does not match the description. Maybe I just don't understand
here something, but flag is boolean. Anyway, minItems means you can have
million of items, so was it intended?

>>
>>> +
>>> +  renesas,clock-flags:
>>> +    description: Flags used in common clock frame work for configuring
>>> +      clk outputs. See include/linux/clk-provider.h
>>
>> These are not bindings, so why do you non-bindings constants as bindings?
>> They can change anytime. Choose one:
>> 1. Drop entire property,
>> 2. Make it a proper binding property, so an ABI and explain why this is DT
>> specific. None of clock providers have to do it, so you need here good
>> explanation.
> 
> I will choose 2 and will explain as user should be allowed to
> configure the output clock from clk generator, so that it has flexibility
> for
> 1) changing the rates (propagate rate change up one level)
> 2) fixed always 
> 3) don't gate the ouput clk at all.

User's choice is task for user-space, so not a good explanation for DT.

Best regards,
Krzysztof

RE: [PATCH RFC 1/3] dt-bindings: clock: Add Renesas versa3 clock generator bindings

[Biju Das]

Hi Krzysztof Kozlowski,

Thanks for the feedback.

> Subject: Re: [PATCH RFC 1/3] dt-bindings: clock: Add Renesas versa3 clock
> generator bindings
> 
> On 08/03/2023 15:39, Biju Das wrote:
> 
> >>> +++ b/Documentation/devicetree/bindings/clock/renesas,versaclock3.ya
> >>> +++ ml
> >>
> >> Filename usually is based on the compatible. Why these two are so
> different?
> >
> > Versa3 clock generator has the following variants.
> >
> > 	5P35023, 5L35021, 5L35023 and 5P35021
> >
> > RZ/G2L SMARC EVK uses 5P35023. So I used generic one as file name.
> > And added compatible for specific one.
> 
> And what about other devices? Since you do not add them, just keep
> compatible as filename.

OK.

> 
> >>
> >>> +
> >>> +  clocks:
> >>> +    maxItems: 1
> >>> +
> >>> +  renesas,settings:
> >>> +    description: Optional, complete register map of the device.
> >>> +      Optimized settings for the device must be provided in full
> >>> +      and are written during initialization.
> >>> +    $ref: /schemas/types.yaml#/definitions/uint8-array
> >>> +    minItems: 37
> >>
> >> maxItems instead... but I am not sure that we allow register settings
> >> in DT in general.
> >
> > Agreed. I guess it is allowed [1]
> > [1]
> 
> I don't see Rob's review on this, so what do you prove exactly?

Subject: [PATCH v9 1/2] dt-bindings: Add binding for Renesas 8T49N241
Date: Fri,  4 Feb 2022 10:57:38 +0100	[thread overview]
Message-ID: <833d3837892f0879233695636291af97a746e584.1643968653.git.michal.simek@xilinx.com> (raw)
In-Reply-To: <cover.1643968653.git.michal.simek@xilinx.com>

From: Alex Helms <alexander.helms.jy@renesas.com>

Renesas 8T49N241 has 4 outputs, 1 integral and 3 fractional dividers.
The 8T49N241 accepts up to two differential or single-ended input clocks
and a fundamental-mode crystal input. The internal PLL can lock to either
of the input reference clocks or to the crystal to behave as a frequency
synthesizer.

Signed-off-by: Alex Helms <alexander.helms.jy@renesas.com>
Reviewed-by: Rob Herring <robh@kernel.org>
Reviewed-by: Geert Uytterhoeven <geert+renesas@glider.be>
Signed-off-by: Michal Simek <michal.simek@xilinx.com>
---
> 
> >
> >>
> >>> +
> >>> +  assigned-clocks:
> >>> +    minItems: 6
> >>
> >> Drop.
> >
> > OK.
> >
> >>
> >>> +
> >>> +  assigned-clock-rates:
> >>> +    minItems: 6
> >>
> >> Drop.
> >
> > OK.
> >
> >>
> >>> +
> >>> +  renesas,clock-divider-read-only:
> >>> +    description: Flag for setting divider in read only mode.
> >>
> >> Flag? Then type: boolean.
> >
> > Agreed.
> >>
> >>> +    $ref: /schemas/types.yaml#/definitions/uint32-array
> >>> +    minItems: 5
> >>
> >> This is broken...
> > OK you mean maxItems. Based on Boolean type I will update this
> 
> I mean, it does not match the description. Maybe I just don't understand
> here something, but flag is boolean. Anyway, minItems means you can have
> million of items, so was it intended?

It is a mistake.

> 
> >>
> >>> +
> >>> +  renesas,clock-flags:
> >>> +    description: Flags used in common clock frame work for configuring
> >>> +      clk outputs. See include/linux/clk-provider.h
> >>
> >> These are not bindings, so why do you non-bindings constants as bindings?
> >> They can change anytime. Choose one:
> >> 1. Drop entire property,
> >> 2. Make it a proper binding property, so an ABI and explain why this
> >> is DT specific. None of clock providers have to do it, so you need
> >> here good explanation.
> >
> > I will choose 2 and will explain as user should be allowed to
> > configure the output clock from clk generator, so that it has
> > flexibility for
> > 1) changing the rates (propagate rate change up one level)
> > 2) fixed always
> > 3) don't gate the ouput clk at all.
> 
> User's choice is task for user-space, so not a good explanation for DT.

I don't think clock generator HW has any business with user space.

It is clk generator HW specific. Clk generator is vital component which
provides clocks to the system. We are providing some hardware feature which
is exposed as dt properties.

Like clock output is fixed rate clock or dynamic rate clock/

Cheers,
Biju

Re: [PATCH RFC 1/3] dt-bindings: clock: Add Renesas versa3 clock generator bindings

[Krzysztof Kozlowski]

On 08/03/2023 19:55, Biju Das wrote:
> 
> Renesas 8T49N241 has 4 outputs, 1 integral and 3 fractional dividers.
> The 8T49N241 accepts up to two differential or single-ended input clocks
> and a fundamental-mode crystal input. The internal PLL can lock to either
> of the input reference clocks or to the crystal to behave as a frequency
> synthesizer.
> 
> Signed-off-by: Alex Helms <alexander.helms.jy@renesas.com>
> Reviewed-by: Rob Herring <robh@kernel.org>

Ah, indeed, fine then.

>>
>>>>
>>>>> +
>>>>> +  renesas,clock-flags:
>>>>> +    description: Flags used in common clock frame work for configuring
>>>>> +      clk outputs. See include/linux/clk-provider.h
>>>>
>>>> These are not bindings, so why do you non-bindings constants as bindings?
>>>> They can change anytime. Choose one:
>>>> 1. Drop entire property,
>>>> 2. Make it a proper binding property, so an ABI and explain why this
>>>> is DT specific. None of clock providers have to do it, so you need
>>>> here good explanation.
>>>
>>> I will choose 2 and will explain as user should be allowed to
>>> configure the output clock from clk generator, so that it has
>>> flexibility for
>>> 1) changing the rates (propagate rate change up one level)
>>> 2) fixed always
>>> 3) don't gate the ouput clk at all.
>>
>> User's choice is task for user-space, so not a good explanation for DT.
> 
> I don't think clock generator HW has any business with user space.
> 
> It is clk generator HW specific. Clk generator is vital component which
> provides clocks to the system. 

Every clock controller is vital...

> We are providing some hardware feature which
> is exposed as dt properties.
> 
> Like clock output is fixed rate clock or dynamic rate clock/

OK, I wait then for proper description which will explain and justify this.


Best regards,
Krzysztof

RE: [PATCH RFC 1/3] dt-bindings: clock: Add Renesas versa3 clock generator bindings

[Biju Das]

Hi Krzysztof Kozlowski,

Thanks for the feedback.

> Subject: Re: [PATCH RFC 1/3] dt-bindings: clock: Add Renesas versa3 clock
> generator bindings
> 
> On 08/03/2023 19:55, Biju Das wrote:
> >
> > Renesas 8T49N241 has 4 outputs, 1 integral and 3 fractional dividers.
> > The 8T49N241 accepts up to two differential or single-ended input
> > clocks and a fundamental-mode crystal input. The internal PLL can lock
> > to either of the input reference clocks or to the crystal to behave as
> > a frequency synthesizer.
> >
> > Signed-off-by: Alex Helms <alexander.helms.jy@renesas.com>
> > Reviewed-by: Rob Herring <robh@kernel.org>
> 
> Ah, indeed, fine then.
> 
> >>
> >>>>
> >>>>> +
> >>>>> +  renesas,clock-flags:
> >>>>> +    description: Flags used in common clock frame work for
> configuring
> >>>>> +      clk outputs. See include/linux/clk-provider.h
> >>>>
> >>>> These are not bindings, so why do you non-bindings constants as
> bindings?
> >>>> They can change anytime. Choose one:
> >>>> 1. Drop entire property,
> >>>> 2. Make it a proper binding property, so an ABI and explain why
> >>>> this is DT specific. None of clock providers have to do it, so you
> >>>> need here good explanation.
> >>>
> >>> I will choose 2 and will explain as user should be allowed to
> >>> configure the output clock from clk generator, so that it has
> >>> flexibility for
> >>> 1) changing the rates (propagate rate change up one level)
> >>> 2) fixed always
> >>> 3) don't gate the ouput clk at all.
> >>
> >> User's choice is task for user-space, so not a good explanation for DT.
> >
> > I don't think clock generator HW has any business with user space.
> >
> > It is clk generator HW specific. Clk generator is vital component
> > which provides clocks to the system.
> 
> Every clock controller is vital...
> 
> > We are providing some hardware feature which is exposed as dt
> > properties.
> >
> > Like clock output is fixed rate clock or dynamic rate clock/
> 
> OK, I wait then for proper description which will explain and justify this.

Here it is, Please let me know is it ok?

renesas,output-clock-fixed-rate-mode:
    type: boolean
    description:
      In output clock fixed rate mode, the output clock frequency is always
      fixed and the hardware will use the values from the OTP or full register
	map initialized during boot.
      If not given, the output clock rate is not fixed.
    maxItems: 6

Cheers,
Biju

Re: [PATCH RFC 1/3] dt-bindings: clock: Add Renesas versa3 clock generator bindings

[Krzysztof Kozlowski]

On 09/03/2023 08:57, Biju Das wrote:
>>> It is clk generator HW specific. Clk generator is vital component
>>> which provides clocks to the system.
>>
>> Every clock controller is vital...
>>
>>> We are providing some hardware feature which is exposed as dt
>>> properties.
>>>
>>> Like clock output is fixed rate clock or dynamic rate clock/
>>
>> OK, I wait then for proper description which will explain and justify this.
> 
> Here it is, Please let me know is it ok?
> 
> renesas,output-clock-fixed-rate-mode:
>     type: boolean
>     description:
>       In output clock fixed rate mode, the output clock frequency is always
>       fixed and the hardware will use the values from the OTP or full register
> 	map initialized during boot.
>       If not given, the output clock rate is not fixed.
>     maxItems: 6

boolean is scalar, not array, so no maxItems. If the frequency is taken
from OTP or register map, why they cannot also provide information the
clock is fixed?

> 
> Cheers,
> Biju

Best regards,
Krzysztof

RE: [PATCH RFC 1/3] dt-bindings: clock: Add Renesas versa3 clock generator bindings

[Biju Das]

> -----Original Message-----
> From: Krzysztof Kozlowski <krzysztof.kozlowski@linaro.org>
> Sent: Thursday, March 9, 2023 9:14 AM
> To: Biju Das <biju.das.jz@bp.renesas.com>; Michael Turquette
> <mturquette@baylibre.com>; Stephen Boyd <sboyd@kernel.org>; Rob Herring
> <robh+dt@kernel.org>; Krzysztof Kozlowski
> <krzysztof.kozlowski+dt@linaro.org>
> Cc: Geert Uytterhoeven <geert+renesas@glider.be>; linux-renesas-
> soc@vger.kernel.org; linux-clk@vger.kernel.org; devicetree@vger.kernel.org;
> Fabrizio Castro <fabrizio.castro.jz@renesas.com>
> Subject: Re: [PATCH RFC 1/3] dt-bindings: clock: Add Renesas versa3 clock
> generator bindings
> 
> On 09/03/2023 08:57, Biju Das wrote:
> >>> It is clk generator HW specific. Clk generator is vital component
> >>> which provides clocks to the system.
> >>
> >> Every clock controller is vital...
> >>
> >>> We are providing some hardware feature which is exposed as dt
> >>> properties.
> >>>
> >>> Like clock output is fixed rate clock or dynamic rate clock/
> >>
> >> OK, I wait then for proper description which will explain and justify
> this.
> >
> > Here it is, Please let me know is it ok?
> >
> > renesas,output-clock-fixed-rate-mode:
> >     type: boolean
> >     description:
> >       In output clock fixed rate mode, the output clock frequency is
> always
> >       fixed and the hardware will use the values from the OTP or full
> register
> > 	map initialized during boot.
> >       If not given, the output clock rate is not fixed.
> >     maxItems: 6
> 
> boolean is scalar, not array, so no maxItems. If the frequency is taken from
> OTP or register map, why they cannot also provide information the clock is
> fixed?

OK, I will make an array property instead. From HW perspective each clock output from the
Clock generator is controllable ie, fixed rate or dynamic rate.

If all the output clocks are fixed rate one, then frequency is taken from OTP or
register map. But if any one clock output generates dynamic rate, then it uses
dynamic settings.

Cheers,
Biju

Re: [PATCH RFC 1/3] dt-bindings: clock: Add Renesas versa3 clock generator bindings

[Krzysztof Kozlowski]

On 09/03/2023 10:18, Biju Das wrote:
> 
> 
>> -----Original Message-----
>> From: Krzysztof Kozlowski <krzysztof.kozlowski@linaro.org>
>> Sent: Thursday, March 9, 2023 9:14 AM
>> To: Biju Das <biju.das.jz@bp.renesas.com>; Michael Turquette
>> <mturquette@baylibre.com>; Stephen Boyd <sboyd@kernel.org>; Rob Herring
>> <robh+dt@kernel.org>; Krzysztof Kozlowski
>> <krzysztof.kozlowski+dt@linaro.org>
>> Cc: Geert Uytterhoeven <geert+renesas@glider.be>; linux-renesas-
>> soc@vger.kernel.org; linux-clk@vger.kernel.org; devicetree@vger.kernel.org;
>> Fabrizio Castro <fabrizio.castro.jz@renesas.com>
>> Subject: Re: [PATCH RFC 1/3] dt-bindings: clock: Add Renesas versa3 clock
>> generator bindings
>>
>> On 09/03/2023 08:57, Biju Das wrote:
>>>>> It is clk generator HW specific. Clk generator is vital component
>>>>> which provides clocks to the system.
>>>>
>>>> Every clock controller is vital...
>>>>
>>>>> We are providing some hardware feature which is exposed as dt
>>>>> properties.
>>>>>
>>>>> Like clock output is fixed rate clock or dynamic rate clock/
>>>>
>>>> OK, I wait then for proper description which will explain and justify
>> this.
>>>
>>> Here it is, Please let me know is it ok?
>>>
>>> renesas,output-clock-fixed-rate-mode:
>>>     type: boolean
>>>     description:
>>>       In output clock fixed rate mode, the output clock frequency is
>> always
>>>       fixed and the hardware will use the values from the OTP or full
>> register
>>> 	map initialized during boot.
>>>       If not given, the output clock rate is not fixed.
>>>     maxItems: 6
>>
>> boolean is scalar, not array, so no maxItems. If the frequency is taken from
>> OTP or register map, why they cannot also provide information the clock is
>> fixed?
> 
> OK, I will make an array property instead. From HW perspective each clock output from the
> Clock generator is controllable ie, fixed rate or dynamic rate.
> 
> If all the output clocks are fixed rate one, then frequency is taken from OTP or
> register map. But if any one clock output generates dynamic rate, then it uses
> dynamic settings.

Second try, same question, let me know if it is not clear:

"why they cannot also provide information the clock is fixed?"

Best regards,
Krzysztof

RE: [PATCH RFC 1/3] dt-bindings: clock: Add Renesas versa3 clock generator bindings

[Biju Das]

> -----Original Message-----
> From: Krzysztof Kozlowski <krzysztof.kozlowski@linaro.org>
> Sent: Thursday, March 9, 2023 9:44 AM
> To: Biju Das <biju.das.jz@bp.renesas.com>; Michael Turquette
> <mturquette@baylibre.com>; Stephen Boyd <sboyd@kernel.org>; Rob Herring
> <robh+dt@kernel.org>; Krzysztof Kozlowski
> <krzysztof.kozlowski+dt@linaro.org>
> Cc: Geert Uytterhoeven <geert+renesas@glider.be>; linux-renesas-
> soc@vger.kernel.org; linux-clk@vger.kernel.org; devicetree@vger.kernel.org;
> Fabrizio Castro <fabrizio.castro.jz@renesas.com>
> Subject: Re: [PATCH RFC 1/3] dt-bindings: clock: Add Renesas versa3 clock
> generator bindings
> 
> On 09/03/2023 10:18, Biju Das wrote:
> >
> >
> >> -----Original Message-----
> >> From: Krzysztof Kozlowski <krzysztof.kozlowski@linaro.org>
> >> Sent: Thursday, March 9, 2023 9:14 AM
> >> To: Biju Das <biju.das.jz@bp.renesas.com>; Michael Turquette
> >> <mturquette@baylibre.com>; Stephen Boyd <sboyd@kernel.org>; Rob
> >> Herring <robh+dt@kernel.org>; Krzysztof Kozlowski
> >> <krzysztof.kozlowski+dt@linaro.org>
> >> Cc: Geert Uytterhoeven <geert+renesas@glider.be>; linux-renesas-
> >> soc@vger.kernel.org; linux-clk@vger.kernel.org;
> >> devicetree@vger.kernel.org; Fabrizio Castro
> >> <fabrizio.castro.jz@renesas.com>
> >> Subject: Re: [PATCH RFC 1/3] dt-bindings: clock: Add Renesas versa3
> >> clock generator bindings
> >>
> >> On 09/03/2023 08:57, Biju Das wrote:
> >>>>> It is clk generator HW specific. Clk generator is vital component
> >>>>> which provides clocks to the system.
> >>>>
> >>>> Every clock controller is vital...
> >>>>
> >>>>> We are providing some hardware feature which is exposed as dt
> >>>>> properties.
> >>>>>
> >>>>> Like clock output is fixed rate clock or dynamic rate clock/
> >>>>
> >>>> OK, I wait then for proper description which will explain and
> >>>> justify
> >> this.
> >>>
> >>> Here it is, Please let me know is it ok?
> >>>
> >>> renesas,output-clock-fixed-rate-mode:
> >>>     type: boolean
> >>>     description:
> >>>       In output clock fixed rate mode, the output clock frequency is
> >> always
> >>>       fixed and the hardware will use the values from the OTP or
> >>> full
> >> register
> >>> 	map initialized during boot.
> >>>       If not given, the output clock rate is not fixed.
> >>>     maxItems: 6
> >>
> >> boolean is scalar, not array, so no maxItems. If the frequency is
> >> taken from OTP or register map, why they cannot also provide
> >> information the clock is fixed?
> >
> > OK, I will make an array property instead. From HW perspective each
> > clock output from the Clock generator is controllable ie, fixed rate or
> dynamic rate.
> >
> > If all the output clocks are fixed rate one, then frequency is taken
> > from OTP or register map. But if any one clock output generates
> > dynamic rate, then it uses dynamic settings.
> 
> Second try, same question, let me know if it is not clear:
> 
> "why they cannot also provide information the clock is fixed?"

This information we are providing through dt.

It is a complex clock generator which provides 6 HW clock outputs.
The 6 HW clock outputs can be individually controllable to generate
Either fixed frequency or dynamic frequency.

 Output clk1 "diff2",
 Output clk2 "diff1",
 Output clk3 "se3",
 Output clk4 "se2",
 Output clk5 "se1",
 Output clk6 "ref"

I want to make "Output clk4" from clock generator as dynamic frequency one
And make other clock frequency from clock generator as fixed one.

How do you describe this in dt? Please share your thoughts.

Cheers,
Biju

Re: [PATCH RFC 1/3] dt-bindings: clock: Add Renesas versa3 clock generator bindings

[Krzysztof Kozlowski]

On 09/03/2023 10:53, Biju Das wrote:
> 
> 
>> -----Original Message-----
>> From: Krzysztof Kozlowski <krzysztof.kozlowski@linaro.org>
>> Sent: Thursday, March 9, 2023 9:44 AM
>> To: Biju Das <biju.das.jz@bp.renesas.com>; Michael Turquette
>> <mturquette@baylibre.com>; Stephen Boyd <sboyd@kernel.org>; Rob Herring
>> <robh+dt@kernel.org>; Krzysztof Kozlowski
>> <krzysztof.kozlowski+dt@linaro.org>
>> Cc: Geert Uytterhoeven <geert+renesas@glider.be>; linux-renesas-
>> soc@vger.kernel.org; linux-clk@vger.kernel.org; devicetree@vger.kernel.org;
>> Fabrizio Castro <fabrizio.castro.jz@renesas.com>
>> Subject: Re: [PATCH RFC 1/3] dt-bindings: clock: Add Renesas versa3 clock
>> generator bindings
>>
>> On 09/03/2023 10:18, Biju Das wrote:
>>>
>>>
>>>> -----Original Message-----
>>>> From: Krzysztof Kozlowski <krzysztof.kozlowski@linaro.org>
>>>> Sent: Thursday, March 9, 2023 9:14 AM
>>>> To: Biju Das <biju.das.jz@bp.renesas.com>; Michael Turquette
>>>> <mturquette@baylibre.com>; Stephen Boyd <sboyd@kernel.org>; Rob
>>>> Herring <robh+dt@kernel.org>; Krzysztof Kozlowski
>>>> <krzysztof.kozlowski+dt@linaro.org>
>>>> Cc: Geert Uytterhoeven <geert+renesas@glider.be>; linux-renesas-
>>>> soc@vger.kernel.org; linux-clk@vger.kernel.org;
>>>> devicetree@vger.kernel.org; Fabrizio Castro
>>>> <fabrizio.castro.jz@renesas.com>
>>>> Subject: Re: [PATCH RFC 1/3] dt-bindings: clock: Add Renesas versa3
>>>> clock generator bindings
>>>>
>>>> On 09/03/2023 08:57, Biju Das wrote:
>>>>>>> It is clk generator HW specific. Clk generator is vital component
>>>>>>> which provides clocks to the system.
>>>>>>
>>>>>> Every clock controller is vital...
>>>>>>
>>>>>>> We are providing some hardware feature which is exposed as dt
>>>>>>> properties.
>>>>>>>
>>>>>>> Like clock output is fixed rate clock or dynamic rate clock/
>>>>>>
>>>>>> OK, I wait then for proper description which will explain and
>>>>>> justify
>>>> this.
>>>>>
>>>>> Here it is, Please let me know is it ok?
>>>>>
>>>>> renesas,output-clock-fixed-rate-mode:
>>>>>     type: boolean
>>>>>     description:
>>>>>       In output clock fixed rate mode, the output clock frequency is
>>>> always
>>>>>       fixed and the hardware will use the values from the OTP or
>>>>> full
>>>> register
>>>>> 	map initialized during boot.
>>>>>       If not given, the output clock rate is not fixed.
>>>>>     maxItems: 6
>>>>
>>>> boolean is scalar, not array, so no maxItems. If the frequency is
>>>> taken from OTP or register map, why they cannot also provide
>>>> information the clock is fixed?
>>>
>>> OK, I will make an array property instead. From HW perspective each
>>> clock output from the Clock generator is controllable ie, fixed rate or
>> dynamic rate.
>>>
>>> If all the output clocks are fixed rate one, then frequency is taken
>>> from OTP or register map. But if any one clock output generates
>>> dynamic rate, then it uses dynamic settings.
>>
>> Second try, same question, let me know if it is not clear:
>>
>> "why they cannot also provide information the clock is fixed?"
> 
> This information we are providing through dt.

No, you are not. We just discuss it. If we do not agree, you are not
going to provide information through DT.

> 
> It is a complex clock generator which provides 6 HW clock outputs.
> The 6 HW clock outputs can be individually controllable to generate
> Either fixed frequency or dynamic frequency.

Ah, indeed. 6 clock outputs prohibits configuring this from OTP. If only
there were 5 outputs then it would be possible...

> 
>  Output clk1 "diff2",
>  Output clk2 "diff1",
>  Output clk3 "se3",
>  Output clk4 "se2",
>  Output clk5 "se1",
>  Output clk6 "ref"
> 
> I want to make "Output clk4" from clock generator as dynamic frequency one
> And make other clock frequency from clock generator as fixed one.
> 
> How do you describe this in dt? Please share your thoughts.

Read from OTP or registers.

Best regards,
Krzysztof

RE: [PATCH RFC 1/3] dt-bindings: clock: Add Renesas versa3 clock generator bindings

[Biju Das]

Hi Krzysztof Kozlowski,

> Subject: Re: [PATCH RFC 1/3] dt-bindings: clock: Add Renesas versa3 clock
> generator bindings
> 
> On 09/03/2023 10:53, Biju Das wrote:
> >
> >
> >> -----Original Message-----
> >> From: Krzysztof Kozlowski <krzysztof.kozlowski@linaro.org>
> >> Sent: Thursday, March 9, 2023 9:44 AM
> >> To: Biju Das <biju.das.jz@bp.renesas.com>; Michael Turquette
> >> <mturquette@baylibre.com>; Stephen Boyd <sboyd@kernel.org>; Rob
> >> Herring <robh+dt@kernel.org>; Krzysztof Kozlowski
> >> <krzysztof.kozlowski+dt@linaro.org>
> >> Cc: Geert Uytterhoeven <geert+renesas@glider.be>; linux-renesas-
> >> soc@vger.kernel.org; linux-clk@vger.kernel.org;
> >> devicetree@vger.kernel.org; Fabrizio Castro
> >> <fabrizio.castro.jz@renesas.com>
> >> Subject: Re: [PATCH RFC 1/3] dt-bindings: clock: Add Renesas versa3
> >> clock generator bindings
> >>
> >> On 09/03/2023 10:18, Biju Das wrote:
> >>>
> >>>
> >>>> -----Original Message-----
> >>>> From: Krzysztof Kozlowski <krzysztof.kozlowski@linaro.org>
> >>>> Sent: Thursday, March 9, 2023 9:14 AM
> >>>> To: Biju Das <biju.das.jz@bp.renesas.com>; Michael Turquette
> >>>> <mturquette@baylibre.com>; Stephen Boyd <sboyd@kernel.org>; Rob
> >>>> Herring <robh+dt@kernel.org>; Krzysztof Kozlowski
> >>>> <krzysztof.kozlowski+dt@linaro.org>
> >>>> Cc: Geert Uytterhoeven <geert+renesas@glider.be>; linux-renesas-
> >>>> soc@vger.kernel.org; linux-clk@vger.kernel.org;
> >>>> devicetree@vger.kernel.org; Fabrizio Castro
> >>>> <fabrizio.castro.jz@renesas.com>
> >>>> Subject: Re: [PATCH RFC 1/3] dt-bindings: clock: Add Renesas versa3
> >>>> clock generator bindings
> >>>>
> >>>> On 09/03/2023 08:57, Biju Das wrote:
> >>>>>>> It is clk generator HW specific. Clk generator is vital
> >>>>>>> component which provides clocks to the system.
> >>>>>>
> >>>>>> Every clock controller is vital...
> >>>>>>
> >>>>>>> We are providing some hardware feature which is exposed as dt
> >>>>>>> properties.
> >>>>>>>
> >>>>>>> Like clock output is fixed rate clock or dynamic rate clock/
> >>>>>>
> >>>>>> OK, I wait then for proper description which will explain and
> >>>>>> justify
> >>>> this.
> >>>>>
> >>>>> Here it is, Please let me know is it ok?
> >>>>>
> >>>>> renesas,output-clock-fixed-rate-mode:
> >>>>>     type: boolean
> >>>>>     description:
> >>>>>       In output clock fixed rate mode, the output clock frequency
> >>>>> is
> >>>> always
> >>>>>       fixed and the hardware will use the values from the OTP or
> >>>>> full
> >>>> register
> >>>>> 	map initialized during boot.
> >>>>>       If not given, the output clock rate is not fixed.
> >>>>>     maxItems: 6
> >>>>
> >>>> boolean is scalar, not array, so no maxItems. If the frequency is
> >>>> taken from OTP or register map, why they cannot also provide
> >>>> information the clock is fixed?
> >>>
> >>> OK, I will make an array property instead. From HW perspective each
> >>> clock output from the Clock generator is controllable ie, fixed rate
> >>> or
> >> dynamic rate.
> >>>
> >>> If all the output clocks are fixed rate one, then frequency is taken
> >>> from OTP or register map. But if any one clock output generates
> >>> dynamic rate, then it uses dynamic settings.
> >>
> >> Second try, same question, let me know if it is not clear:
> >>
> >> "why they cannot also provide information the clock is fixed?"
> >
> > This information we are providing through dt.
> 
> No, you are not. We just discuss it. If we do not agree, you are not going
> to provide information through DT.
> 
> >
> > It is a complex clock generator which provides 6 HW clock outputs.
> > The 6 HW clock outputs can be individually controllable to generate
> > Either fixed frequency or dynamic frequency.
> 
> Ah, indeed. 6 clock outputs prohibits configuring this from OTP. If only
> there were 5 outputs then it would be possible...
> 
> >
> >  Output clk1 "diff2",
> >  Output clk2 "diff1",
> >  Output clk3 "se3",
> >  Output clk4 "se2",
> >  Output clk5 "se1",
> >  Output clk6 "ref"
> >
> > I want to make "Output clk4" from clock generator as dynamic frequency
> > one And make other clock frequency from clock generator as fixed one.
> >
> > How do you describe this in dt? Please share your thoughts.
> 
> Read from OTP or registers.

Looks no other option. So will drop this property and match the one from OTP with full regmap.

Cheers,
Biju

Re: [PATCH RFC 1/3] dt-bindings: clock: Add Renesas versa3 clock generator bindings

[Geert Uytterhoeven]

Hi Biju,

On Thu, Mar 9, 2023 at 10:44 AM Krzysztof Kozlowski
<krzysztof.kozlowski@linaro.org> wrote:
> On 09/03/2023 10:18, Biju Das wrote:
> >> -----Original Message-----
> >> From: Krzysztof Kozlowski <krzysztof.kozlowski@linaro.org>
> >> On 09/03/2023 08:57, Biju Das wrote:
> >>>>> It is clk generator HW specific. Clk generator is vital component
> >>>>> which provides clocks to the system.
> >>>>
> >>>> Every clock controller is vital...
> >>>>
> >>>>> We are providing some hardware feature which is exposed as dt
> >>>>> properties.
> >>>>>
> >>>>> Like clock output is fixed rate clock or dynamic rate clock/
> >>>>
> >>>> OK, I wait then for proper description which will explain and justify
> >> this.
> >>>
> >>> Here it is, Please let me know is it ok?
> >>>
> >>> renesas,output-clock-fixed-rate-mode:
> >>>     type: boolean
> >>>     description:
> >>>       In output clock fixed rate mode, the output clock frequency is
> >> always
> >>>       fixed and the hardware will use the values from the OTP or full
> >> register
> >>>     map initialized during boot.
> >>>       If not given, the output clock rate is not fixed.
> >>>     maxItems: 6
> >>
> >> boolean is scalar, not array, so no maxItems. If the frequency is taken from
> >> OTP or register map, why they cannot also provide information the clock is
> >> fixed?
> >
> > OK, I will make an array property instead. From HW perspective each clock output from the
> > Clock generator is controllable ie, fixed rate or dynamic rate.
> >
> > If all the output clocks are fixed rate one, then frequency is taken from OTP or
> > register map. But if any one clock output generates dynamic rate, then it uses
> > dynamic settings.
>
> Second try, same question, let me know if it is not clear:
>
> "why they cannot also provide information the clock is fixed?"

What is the actual use case?
My understanding is:
  1. If the OTP is programmed, the clock generator will be configured
     from the OTP on power-on,
  2. The clock generator can be (re)configured from software.
     a. If the OTP is programmed, this is not needed,
     b. For critical clocks, you may want to prevent this.

Also, AFAIUI, "fixed frequency" or "dynamic frequency" is a policy,
and purely software? Or are there OTP bits to enforce this?

Perhaps you need a per-output "do-not-change-frequency" flag,
probably with a generic name, in the spirit of "regulator-always-on"
for regulators?

Now, if all the output clocks are fixed rate, you might want to describe
this in DTS using a set of fixed{,-factor-}-clocks?

Gr{oetje,eeting}s,

                        Geert

--
Geert Uytterhoeven -- There's lots of Linux beyond ia32 -- geert@linux-m68k.org

In personal conversations with technical people, I call myself a hacker. But
when I'm talking to journalists I just say "programmer" or something like that.
                                -- Linus Torvalds

Re: [PATCH RFC 1/3] dt-bindings: clock: Add Renesas versa3 clock generator bindings

[Krzysztof Kozlowski]

On 09/03/2023 10:58, Geert Uytterhoeven wrote:
>>>>> Here it is, Please let me know is it ok?
>>>>>
>>>>> renesas,output-clock-fixed-rate-mode:
>>>>>     type: boolean
>>>>>     description:
>>>>>       In output clock fixed rate mode, the output clock frequency is
>>>> always
>>>>>       fixed and the hardware will use the values from the OTP or full
>>>> register
>>>>>     map initialized during boot.
>>>>>       If not given, the output clock rate is not fixed.
>>>>>     maxItems: 6
>>>>
>>>> boolean is scalar, not array, so no maxItems. If the frequency is taken from
>>>> OTP or register map, why they cannot also provide information the clock is
>>>> fixed?
>>>
>>> OK, I will make an array property instead. From HW perspective each clock output from the
>>> Clock generator is controllable ie, fixed rate or dynamic rate.
>>>
>>> If all the output clocks are fixed rate one, then frequency is taken from OTP or
>>> register map. But if any one clock output generates dynamic rate, then it uses
>>> dynamic settings.
>>
>> Second try, same question, let me know if it is not clear:
>>
>> "why they cannot also provide information the clock is fixed?"
> 
> What is the actual use case?
> My understanding is:
>   1. If the OTP is programmed, the clock generator will be configured
>      from the OTP on power-on,
>   2. The clock generator can be (re)configured from software.
>      a. If the OTP is programmed, this is not needed,
>      b. For critical clocks, you may want to prevent this.
> 
> Also, AFAIUI, "fixed frequency" or "dynamic frequency" is a policy,
> and purely software? Or are there OTP bits to enforce this?
> 
> Perhaps you need a per-output "do-not-change-frequency" flag,
> probably with a generic name, in the spirit of "regulator-always-on"
> for regulators?
> 
> Now, if all the output clocks are fixed rate, you might want to describe
> this in DTS using a set of fixed{,-factor-}-clocks?
> 

I would also argue that fixed frequency is actually also dynamic
frequency, just with a limit to one frequency...

Best regards,
Krzysztof

Re: [PATCH RFC 1/3] dt-bindings: clock: Add Renesas versa3 clock generator bindings

[Geert Uytterhoeven]

Hi Krzysztof,

On Thu, Mar 9, 2023 at 11:17 AM Krzysztof Kozlowski
<krzysztof.kozlowski@linaro.org> wrote:
> On 09/03/2023 10:58, Geert Uytterhoeven wrote:
> >>>>> Here it is, Please let me know is it ok?
> >>>>>
> >>>>> renesas,output-clock-fixed-rate-mode:
> >>>>>     type: boolean
> >>>>>     description:
> >>>>>       In output clock fixed rate mode, the output clock frequency is
> >>>> always
> >>>>>       fixed and the hardware will use the values from the OTP or full
> >>>> register
> >>>>>     map initialized during boot.
> >>>>>       If not given, the output clock rate is not fixed.
> >>>>>     maxItems: 6
> >>>>
> >>>> boolean is scalar, not array, so no maxItems. If the frequency is taken from
> >>>> OTP or register map, why they cannot also provide information the clock is
> >>>> fixed?
> >>>
> >>> OK, I will make an array property instead. From HW perspective each clock output from the
> >>> Clock generator is controllable ie, fixed rate or dynamic rate.
> >>>
> >>> If all the output clocks are fixed rate one, then frequency is taken from OTP or
> >>> register map. But if any one clock output generates dynamic rate, then it uses
> >>> dynamic settings.
> >>
> >> Second try, same question, let me know if it is not clear:
> >>
> >> "why they cannot also provide information the clock is fixed?"
> >
> > What is the actual use case?
> > My understanding is:
> >   1. If the OTP is programmed, the clock generator will be configured
> >      from the OTP on power-on,
> >   2. The clock generator can be (re)configured from software.
> >      a. If the OTP is programmed, this is not needed,
> >      b. For critical clocks, you may want to prevent this.
> >
> > Also, AFAIUI, "fixed frequency" or "dynamic frequency" is a policy,
> > and purely software? Or are there OTP bits to enforce this?
> >
> > Perhaps you need a per-output "do-not-change-frequency" flag,
> > probably with a generic name, in the spirit of "regulator-always-on"
> > for regulators?

Actually we already have "assigned-clock{,-rate}s" properties for that.

> > Now, if all the output clocks are fixed rate, you might want to describe
> > this in DTS using a set of fixed{,-factor-}-clocks?
>
> I would also argue that fixed frequency is actually also dynamic
> frequency, just with a limit to one frequency...

Indeed.

Gr{oetje,eeting}s,

                        Geert

-- 
Geert Uytterhoeven -- There's lots of Linux beyond ia32 -- geert@linux-m68k.org

In personal conversations with technical people, I call myself a hacker. But
when I'm talking to journalists I just say "programmer" or something like that.
                                -- Linus Torvalds

RE: [PATCH RFC 1/3] dt-bindings: clock: Add Renesas versa3 clock generator bindings

[Biju Das]

Hi Geert Uytterhoeven,

Thanks for the feedback.

> Subject: Re: [PATCH RFC 1/3] dt-bindings: clock: Add Renesas versa3 clock
> generator bindings
> 
> Hi Krzysztof,
> 
> On Thu, Mar 9, 2023 at 11:17 AM Krzysztof Kozlowski
> <krzysztof.kozlowski@linaro.org> wrote:
> > On 09/03/2023 10:58, Geert Uytterhoeven wrote:
> > >>>>> Here it is, Please let me know is it ok?
> > >>>>>
> > >>>>> renesas,output-clock-fixed-rate-mode:
> > >>>>>     type: boolean
> > >>>>>     description:
> > >>>>>       In output clock fixed rate mode, the output clock
> > >>>>> frequency is
> > >>>> always
> > >>>>>       fixed and the hardware will use the values from the OTP or
> > >>>>> full
> > >>>> register
> > >>>>>     map initialized during boot.
> > >>>>>       If not given, the output clock rate is not fixed.
> > >>>>>     maxItems: 6
> > >>>>
> > >>>> boolean is scalar, not array, so no maxItems. If the frequency is
> > >>>> taken from OTP or register map, why they cannot also provide
> > >>>> information the clock is fixed?
> > >>>
> > >>> OK, I will make an array property instead. From HW perspective
> > >>> each clock output from the Clock generator is controllable ie, fixed
> rate or dynamic rate.
> > >>>
> > >>> If all the output clocks are fixed rate one, then frequency is
> > >>> taken from OTP or register map. But if any one clock output
> > >>> generates dynamic rate, then it uses dynamic settings.
> > >>
> > >> Second try, same question, let me know if it is not clear:
> > >>
> > >> "why they cannot also provide information the clock is fixed?"
> > >
> > > What is the actual use case?
> > > My understanding is:
> > >   1. If the OTP is programmed, the clock generator will be configured
> > >      from the OTP on power-on,
> > >   2. The clock generator can be (re)configured from software.
> > >      a. If the OTP is programmed, this is not needed,
> > >      b. For critical clocks, you may want to prevent this.
> > >
> > > Also, AFAIUI, "fixed frequency" or "dynamic frequency" is a policy,
> > > and purely software? Or are there OTP bits to enforce this?
> > >
> > > Perhaps you need a per-output "do-not-change-frequency" flag,
> > > probably with a generic name, in the spirit of "regulator-always-on"
> > > for regulators?
> 
> Actually we already have "assigned-clock{,-rate}s" properties for that.

Actually it is a good pointer. I can make use of this property.

Cheers,
Biju

RE: [PATCH RFC 1/3] dt-bindings: clock: Add Renesas versa3 clock generator bindings

[Biju Das]

Hi Geert Uytterhoeven,

Thanks for the feedback.

> Subject: Re: [PATCH RFC 1/3] dt-bindings: clock: Add Renesas versa3 clock
> generator bindings
> 
> Hi Biju,
> 
> On Thu, Mar 9, 2023 at 10:44 AM Krzysztof Kozlowski
> <krzysztof.kozlowski@linaro.org> wrote:
> > On 09/03/2023 10:18, Biju Das wrote:
> > >> -----Original Message-----
> > >> From: Krzysztof Kozlowski <krzysztof.kozlowski@linaro.org> On
> > >> 09/03/2023 08:57, Biju Das wrote:
> > >>>>> It is clk generator HW specific. Clk generator is vital
> > >>>>> component which provides clocks to the system.
> > >>>>
> > >>>> Every clock controller is vital...
> > >>>>
> > >>>>> We are providing some hardware feature which is exposed as dt
> > >>>>> properties.
> > >>>>>
> > >>>>> Like clock output is fixed rate clock or dynamic rate clock/
> > >>>>
> > >>>> OK, I wait then for proper description which will explain and
> > >>>> justify
> > >> this.
> > >>>
> > >>> Here it is, Please let me know is it ok?
> > >>>
> > >>> renesas,output-clock-fixed-rate-mode:
> > >>>     type: boolean
> > >>>     description:
> > >>>       In output clock fixed rate mode, the output clock frequency
> > >>> is
> > >> always
> > >>>       fixed and the hardware will use the values from the OTP or
> > >>> full
> > >> register
> > >>>     map initialized during boot.
> > >>>       If not given, the output clock rate is not fixed.
> > >>>     maxItems: 6
> > >>
> > >> boolean is scalar, not array, so no maxItems. If the frequency is
> > >> taken from OTP or register map, why they cannot also provide
> > >> information the clock is fixed?
> > >
> > > OK, I will make an array property instead. From HW perspective each
> > > clock output from the Clock generator is controllable ie, fixed rate or
> dynamic rate.
> > >
> > > If all the output clocks are fixed rate one, then frequency is taken
> > > from OTP or register map. But if any one clock output generates
> > > dynamic rate, then it uses dynamic settings.
> >
> > Second try, same question, let me know if it is not clear:
> >
> > "why they cannot also provide information the clock is fixed?"
> 
> What is the actual use case?
> My understanding is:
>   1. If the OTP is programmed, the clock generator will be configured
>      from the OTP on power-on,

Correct.

>   2. The clock generator can be (re)configured from software.
>      a. If the OTP is programmed, this is not needed,


Yes, But we miss some HW functionality.

Eg:
On RZ/G2L SMARC EVK, By default audio mclk is connected to
11.2896 MHz clk (se2 output from clock generator)  which is multiple of 44.1KHz.
and this clock is a non-critical clock.

48Khz playback/record is not possible with Audio codec, if we just use the
value from OTP.

But by changing parent of "se2 clock", it is possible to achieve
48 KHz playback.

>      b. For critical clocks, you may want to prevent this.

For caseb, Critical clocks we won't change its registers.
The reconfiguration is only for non-critical clocks.

> Also, AFAIUI, "fixed frequency" or "dynamic frequency" is a policy, and
> purely software? Or are there OTP bits to enforce this?

Nothing OTP bits related. 

> 
> Perhaps you need a per-output "do-not-change-frequency" flag, probably with
> a generic name, in the spirit of "regulator-always-on"
> for regulators?

Yes "do-not-change-frequency" flag for per-output is sensible one.

> Now, if all the output clocks are fixed rate, you might want to describe
> this in DTS using a set of fixed{,-factor-}-clocks?

On Ideal case, all the output clocks are fixed rate and use the value from OTP.

But cases like, non critical clocks we should be able to
change frequency of that particular clock output.

In audio playback case, it is just 1 bit for changing the parent,
After the initial reconfiguration.

Cheers,
Biju

RE: [PATCH RFC 0/3] Add Versa3 clock generator support

[Biju Das]

+ Rob , Krzysztof Kozlowski

> -----Original Message-----
> From: Biju Das <biju.das.jz@bp.renesas.com>
> Sent: Monday, February 20, 2023 1:13 PM
> To: Michael Turquette <mturquette@baylibre.com>; Stephen Boyd
> <sboyd@kernel.org>
> Cc: Biju Das <biju.das.jz@bp.renesas.com>; linux-clk@vger.kernel.org; Geert
> Uytterhoeven <geert+renesas@glider.be>; Kuninori Morimoto
> <kuninori.morimoto.gx@renesas.com>; Prabhakar Mahadev Lad
> <prabhakar.mahadev-lad.rj@bp.renesas.com>; Fabrizio Castro
> <fabrizio.castro.jz@renesas.com>; Mark Brown <broonie@kernel.org>; Takashi
> Iwai <tiwai@suse.de>; alsa-devel@alsa-project.org; linux-renesas-
> soc@vger.kernel.org
> Subject: [PATCH RFC 0/3] Add Versa3 clock generator support
> 
> The 5P35023 is a VersaClock programmable clock generator and it provides 6
> clk outputs {diff2, diff1, se3, se2, se1 and refin}.
> 
> It has an internal OTP memory allows the user to store the configuration in
> the device. After power up, the user can change the device register settings
> through the I2C interface when I2C mode is selected.
> 
> This driver is for overriding OTP default values during boot based on a full
> register map from DT, and also minimal support to change the parent of a
> output clock.
> 
> The motivation for developing this driver is for supporting 48KHz
> playback/record with audio codec on RZ/G2L SMARC EVK.
> 
> On RZ/G2L SMARC EVK, By default audio mclk is connected to
> 11.2896 MHz clk which is multiple of 44.1KHz.
> 
> Please see the below default OTP configuration of Dividers connected to
> output clocks.
> 
> DIV3 12.2880 MHz   DIFF2--> Audio clk2
> DIV5 11.2896 MHz   SE1  --> Audio clk1
> DIV5 11.2896 MHz   SE2  --> Audio mck
> DIV4 12      MHz   SE3  --> This clk Not used
> DIV1 25 MHz        DIFF1-->Ethernet clk
> Ref1-> 24MHz
> 
> With this setup, we won't be able to do 48KHz playback/record on audio
> codec, as mck is always connected to 11.2896MHz clk.
> 
> But by programming the i2c, we can make use of DIV4 to generate 12.2880 MHz
> and make that as parent of SE2 and there by supporting 48KHz
> playback/record.
> 
> A block diagram with modification can be find here[1]
> [1] https://paste.pics/a253ce7cdc8720c3b5eb6953b97b25ff 
> 
> DIV3 12.2880 MHz   DIFF2--> Audio clk2
> DIV5 11.2896 MHz   SE1  --> Audio clk1
> DIV5 11.2896 MHz | SE2  --> Audio mck
> DIV4 12.2880 MHz |
> DIV2 12      MHz   SE3  --> This clk Not used
> DIV1 25 MHz        DIFF1--> Ethernet clk
> Ref1-> 24MHz
> 
> The driver can read a full register map from the DT, and will use that
> register map to initialize the clk generator when the system boots.
> and later, based on sampling rate, it switches the parent of SE2 and support
> both 44.1 and 48 KHz playback/record at run time.
> 
> 48KHz playback
> 1f: f6 --> setting Div4 as clock source for se2 Read at address  0x10049C00
> : 0x300B4022 --> Setting Audio clk2 in SSI
>        pfd2                           1        1        0    24000000
>           pll2                        1        1        0   491519897
>              div4_mux                 1        1        0   491519897
>                 div4                  1        1        0    12287998
>                    se2_mux            1        1        0    12287998
>                       se2             1        1        0    12287998
> 
> 44.1KHz playback
> 1f: b6 --> setting Div5 as clock source for se2 Read at address  0x10049C00:
> 0x700B4022--> Setting Audio clk1 in SSI
>     pfd3_mux                          1        1        0    24000000
>        pfd3                           1        1        0      960000
>           pll3                        1        1        0   564480000
>              div5                     1        1        0    11289600
>                 se2_mux               1        1        0    11289600
>                    se2                1        1        0    11289600
> 
> Please provide your valuable comment for this patch series.
> 
> Biju Das (3):
>   dt-bindings: clock: Add Renesas versa3 clock generator bindings
>   drivers: clk: Add support for versa3 clock driver
>   arm64: dts: renesas: rzg2l-smarc: Use versa3 clk for audio mclk
> 
>  .../bindings/clock/renesas,versaclock3.yaml   |  135 ++
>  .../boot/dts/renesas/rz-smarc-common.dtsi     |    7 -
>  arch/arm64/boot/dts/renesas/rzg2l-smarc.dtsi  |   35 +
>  drivers/clk/Kconfig                           |    9 +
>  drivers/clk/Makefile                          |    1 +
>  drivers/clk/clk-versaclock3.c                 | 1134 +++++++++++++++++
>  6 files changed, 1314 insertions(+), 7 deletions(-)  create mode 100644
> Documentation/devicetree/bindings/clock/renesas,versaclock3.yaml
>  create mode 100644 drivers/clk/clk-versaclock3.c
> 
> --
> 2.25.1