[PATCH v2 0/3] Fix Versa3 clock mapping

[PATCH v2 0/3] Fix Versa3 clock mapping

[Biju Das]

According to Table 3. ("Output Source") in the 5P35023 datasheet,
the output clock mapping should be 0=REF, 1=SE1, 2=SE2, 3=SE3,
4=DIFF1, 5=DIFF2. But the code uses inverse.

This patch series aims to document clock-output-names in bindings and
fix the mapping in driver.

v1->v2:
 * Updated binding commit description to make it clear it fixes
   "assigned-clock-rates" in the example based on 5P35023 datasheet.

Biju Das (3):
  dt-bindings: clock: versaclock3: Document clock-output-names
  clk: vc3: Fix output clock mapping
  arm64: dts: renesas: rz-smarc-common: Use versa3 clk for audio mclk

 .../bindings/clock/renesas,5p35023.yaml       | 14 ++--
 .../boot/dts/renesas/rz-smarc-common.dtsi     | 14 ++--
 arch/arm64/boot/dts/renesas/rzg2l-smarc.dtsi  | 23 +++++++
 arch/arm64/boot/dts/renesas/rzg2lc-smarc.dtsi | 23 +++++++
 arch/arm64/boot/dts/renesas/rzg2ul-smarc.dtsi | 27 ++++++++
 drivers/clk/clk-versaclock3.c                 | 68 +++++++++----------
 6 files changed, 124 insertions(+), 45 deletions(-)

-- 
2.25.1

[PATCH v2 1/3] dt-bindings: clock: versaclock3: Document clock-output-names

[Biju Das]

Document clock-output-names property and fix the "assigned-clock-rates"
for each clock output in the example based on Table 3. ("Output Source")
in the 5P35023 datasheet(ie: {REF,SE1,SE2,SE3,DIFF1,DIFF2}).

While at it, replace clocks phandle in the example from x1_x2->x1 as
X2 is a different 32768 kHz crystal.

Suggested-by: Geert Uytterhoeven <geert+renesas@glider.be>
Closes: https://lore.kernel.org/all/CAMuHMdUHD+bEco=WYTYWsTAyRt3dTQQt4Xpaejss0Y2ZpLCMNg@mail.gmail.com/
Fixes: a03d23f860eb ("dt-bindings: clock: Add Renesas versa3 clock generator bindings")
Signed-off-by: Biju Das <biju.das.jz@bp.renesas.com>
---
v1->v2:
 * Updated commit description to make it clear it fixes
   "assigned-clock-rates" in the example based on 5P35023 datasheet.
---
 .../devicetree/bindings/clock/renesas,5p35023.yaml | 14 ++++++++++----
 1 file changed, 10 insertions(+), 4 deletions(-)

diff --git a/Documentation/devicetree/bindings/clock/renesas,5p35023.yaml b/Documentation/devicetree/bindings/clock/renesas,5p35023.yaml
index 839648e753d4..db8d01b291dd 100644
--- a/Documentation/devicetree/bindings/clock/renesas,5p35023.yaml
+++ b/Documentation/devicetree/bindings/clock/renesas,5p35023.yaml
@@ -49,6 +49,9 @@ properties:
     $ref: /schemas/types.yaml#/definitions/uint8-array
     maxItems: 37
 
+  clock-output-names:
+    maxItems: 6
+
 required:
   - compatible
   - reg
@@ -68,7 +71,7 @@ examples:
             reg = <0x68>;
             #clock-cells = <1>;
 
-            clocks = <&x1_x2>;
+            clocks = <&x1>;
 
             renesas,settings = [
                 80 00 11 19 4c 02 23 7f 83 19 08 a9 5f 25 24 bf
@@ -76,11 +79,14 @@ examples:
                 80 b0 45 c4 95
             ];
 
+            clock-output-names = "ref", "se1", "se2", "se3",
+                                 "diff1", "diff2";
+
             assigned-clocks = <&versa3 0>, <&versa3 1>,
                               <&versa3 2>, <&versa3 3>,
                               <&versa3 4>, <&versa3 5>;
-            assigned-clock-rates = <12288000>, <25000000>,
-                                   <12000000>, <11289600>,
-                                   <11289600>, <24000000>;
+            assigned-clock-rates = <24000000>, <11289600>,
+                                   <11289600>, <12000000>,
+                                   <25000000>, <12288000>;
         };
     };
-- 
2.25.1

Re: [PATCH v2 1/3] dt-bindings: clock: versaclock3: Document clock-output-names

[Conor Dooley]

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

On Thu, Aug 17, 2023 at 10:08:08AM +0100, Biju Das wrote:
> Document clock-output-names property and fix the "assigned-clock-rates"
> for each clock output in the example based on Table 3. ("Output Source")
> in the 5P35023 datasheet(ie: {REF,SE1,SE2,SE3,DIFF1,DIFF2}).
> 
> While at it, replace clocks phandle in the example from x1_x2->x1 as
> X2 is a different 32768 kHz crystal.
> 
> Suggested-by: Geert Uytterhoeven <geert+renesas@glider.be>
> Closes: https://lore.kernel.org/all/CAMuHMdUHD+bEco=WYTYWsTAyRt3dTQQt4Xpaejss0Y2ZpLCMNg@mail.gmail.com/
> Fixes: a03d23f860eb ("dt-bindings: clock: Add Renesas versa3 clock generator bindings")
> Signed-off-by: Biju Das <biju.das.jz@bp.renesas.com>

Acked-by: Conor Dooley <conor.dooley@microchip.com>

Thanks,
Conor.

> ---
> v1->v2:
>  * Updated commit description to make it clear it fixes
>    "assigned-clock-rates" in the example based on 5P35023 datasheet.
> ---
>  .../devicetree/bindings/clock/renesas,5p35023.yaml | 14 ++++++++++----
>  1 file changed, 10 insertions(+), 4 deletions(-)
> 
> diff --git a/Documentation/devicetree/bindings/clock/renesas,5p35023.yaml b/Documentation/devicetree/bindings/clock/renesas,5p35023.yaml
> index 839648e753d4..db8d01b291dd 100644
> --- a/Documentation/devicetree/bindings/clock/renesas,5p35023.yaml
> +++ b/Documentation/devicetree/bindings/clock/renesas,5p35023.yaml
> @@ -49,6 +49,9 @@ properties:
>      $ref: /schemas/types.yaml#/definitions/uint8-array
>      maxItems: 37
>  
> +  clock-output-names:
> +    maxItems: 6
> +
>  required:
>    - compatible
>    - reg
> @@ -68,7 +71,7 @@ examples:
>              reg = <0x68>;
>              #clock-cells = <1>;
>  
> -            clocks = <&x1_x2>;
> +            clocks = <&x1>;
>  
>              renesas,settings = [
>                  80 00 11 19 4c 02 23 7f 83 19 08 a9 5f 25 24 bf
> @@ -76,11 +79,14 @@ examples:
>                  80 b0 45 c4 95
>              ];
>  
> +            clock-output-names = "ref", "se1", "se2", "se3",
> +                                 "diff1", "diff2";
> +
>              assigned-clocks = <&versa3 0>, <&versa3 1>,
>                                <&versa3 2>, <&versa3 3>,
>                                <&versa3 4>, <&versa3 5>;
> -            assigned-clock-rates = <12288000>, <25000000>,
> -                                   <12000000>, <11289600>,
> -                                   <11289600>, <24000000>;
> +            assigned-clock-rates = <24000000>, <11289600>,
> +                                   <11289600>, <12000000>,
> +                                   <25000000>, <12288000>;
>          };
>      };
> -- 
> 2.25.1
> 

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

Re: [PATCH v2 1/3] dt-bindings: clock: versaclock3: Document clock-output-names

[Geert Uytterhoeven]

Hi Biju,

On Thu, Aug 17, 2023 at 11:08 AM Biju Das <biju.das.jz@bp.renesas.com> wrote:
> Document clock-output-names property and fix the "assigned-clock-rates"
> for each clock output in the example based on Table 3. ("Output Source")
> in the 5P35023 datasheet(ie: {REF,SE1,SE2,SE3,DIFF1,DIFF2}).
>
> While at it, replace clocks phandle in the example from x1_x2->x1 as
> X2 is a different 32768 kHz crystal.
>
> Suggested-by: Geert Uytterhoeven <geert+renesas@glider.be>
> Closes: https://lore.kernel.org/all/CAMuHMdUHD+bEco=WYTYWsTAyRt3dTQQt4Xpaejss0Y2ZpLCMNg@mail.gmail.com/
> Fixes: a03d23f860eb ("dt-bindings: clock: Add Renesas versa3 clock generator bindings")
> Signed-off-by: Biju Das <biju.das.jz@bp.renesas.com>
> ---
> v1->v2:
>  * Updated commit description to make it clear it fixes
>    "assigned-clock-rates" in the example based on 5P35023 datasheet.

Thanks for your patch!
> ---
>  .../devicetree/bindings/clock/renesas,5p35023.yaml | 14 ++++++++++----
>  1 file changed, 10 insertions(+), 4 deletions(-)
>
> diff --git a/Documentation/devicetree/bindings/clock/renesas,5p35023.yaml b/Documentation/devicetree/bindings/clock/renesas,5p35023.yaml
> index 839648e753d4..db8d01b291dd 100644
> --- a/Documentation/devicetree/bindings/clock/renesas,5p35023.yaml
> +++ b/Documentation/devicetree/bindings/clock/renesas,5p35023.yaml
> @@ -49,6 +49,9 @@ properties:
>      $ref: /schemas/types.yaml#/definitions/uint8-array
>      maxItems: 37
>
> +  clock-output-names:
> +    maxItems: 6
> +

Why do you need clock-output-names?
The clock output names should be created by the driver (taking into
account the instance number, so it works with multiple instances).

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 v2 1/3] dt-bindings: clock: versaclock3: Document clock-output-names

[Biju Das]

Hi Geert Uytterhoeven,

Thanks for the feedback.

> Subject: Re: [PATCH v2 1/3] dt-bindings: clock: versaclock3: Document
> clock-output-names
> 
> Hi Biju,
> 
> On Thu, Aug 17, 2023 at 11:08 AM Biju Das <biju.das.jz@bp.renesas.com>
> wrote:
> > Document clock-output-names property and fix the "assigned-clock-rates"
> > for each clock output in the example based on Table 3. ("Output
> > Source") in the 5P35023 datasheet(ie: {REF,SE1,SE2,SE3,DIFF1,DIFF2}).
> >
> > While at it, replace clocks phandle in the example from x1_x2->x1 as
> > X2 is a different 32768 kHz crystal.
> >
> > Suggested-by: Geert Uytterhoeven <geert+renesas@glider.be>
> > Closes:
> > Fixes: a03d23f860eb ("dt-bindings: clock: Add Renesas versa3 clock
> > generator bindings")
> > Signed-off-by: Biju Das <biju.das.jz@bp.renesas.com>
> > ---
> > v1->v2:
> >  * Updated commit description to make it clear it fixes
> >    "assigned-clock-rates" in the example based on 5P35023 datasheet.
> 
> Thanks for your patch!
> > ---
> >  .../devicetree/bindings/clock/renesas,5p35023.yaml | 14
> > ++++++++++----
> >  1 file changed, 10 insertions(+), 4 deletions(-)
> >
> > diff --git
> > a/Documentation/devicetree/bindings/clock/renesas,5p35023.yaml
> > b/Documentation/devicetree/bindings/clock/renesas,5p35023.yaml
> > index 839648e753d4..db8d01b291dd 100644
> > --- a/Documentation/devicetree/bindings/clock/renesas,5p35023.yaml
> > +++ b/Documentation/devicetree/bindings/clock/renesas,5p35023.yaml
> > @@ -49,6 +49,9 @@ properties:
> >      $ref: /schemas/types.yaml#/definitions/uint8-array
> >      maxItems: 37
> >
> > +  clock-output-names:
> > +    maxItems: 6
> > +
> 
> Why do you need clock-output-names?

I thought it will be useful information for a user, by looking at the example the name of clock-output-name and corresponding assigned-clocks and assigned-clock-rates.

See below, from this one can understand the relation between index and actual clock output.

  clock-output-names = "ref", "se1", "se2", "se3",
                       "diff1", "diff2";

  assigned-clocks = <&versa3 0>, <&versa3 1>,
                    <&versa3 2>, <&versa3 3>,
                    <&versa3 4>, <&versa3 5>;
  assigned-clock-rates = <24000000>, <11289600>,
                         <11289600>, <12000000>,
                         <25000000>, <12288000>;

> The clock output names should be created by the driver (taking into account
> the instance number, so it works with multiple instances).

OK, so shall I remove it from bindings then?

Cheers,
Biju

Re: [PATCH v2 1/3] dt-bindings: clock: versaclock3: Document clock-output-names

[Geert Uytterhoeven]

Hi Biju,

On Mon, Aug 21, 2023 at 10:11 AM Biju Das <biju.das.jz@bp.renesas.com> wrote:
> > Subject: Re: [PATCH v2 1/3] dt-bindings: clock: versaclock3: Document
> > clock-output-names
> > On Thu, Aug 17, 2023 at 11:08 AM Biju Das <biju.das.jz@bp.renesas.com>
> > wrote:
> > > Document clock-output-names property and fix the "assigned-clock-rates"
> > > for each clock output in the example based on Table 3. ("Output
> > > Source") in the 5P35023 datasheet(ie: {REF,SE1,SE2,SE3,DIFF1,DIFF2}).
> > >
> > > While at it, replace clocks phandle in the example from x1_x2->x1 as
> > > X2 is a different 32768 kHz crystal.
> > >
> > > Suggested-by: Geert Uytterhoeven <geert+renesas@glider.be>
> > > Closes:
> > > Fixes: a03d23f860eb ("dt-bindings: clock: Add Renesas versa3 clock
> > > generator bindings")
> > > Signed-off-by: Biju Das <biju.das.jz@bp.renesas.com>
> > > ---
> > > v1->v2:
> > >  * Updated commit description to make it clear it fixes
> > >    "assigned-clock-rates" in the example based on 5P35023 datasheet.
> >
> > Thanks for your patch!
> > > ---
> > >  .../devicetree/bindings/clock/renesas,5p35023.yaml | 14
> > > ++++++++++----
> > >  1 file changed, 10 insertions(+), 4 deletions(-)
> > >
> > > diff --git
> > > a/Documentation/devicetree/bindings/clock/renesas,5p35023.yaml
> > > b/Documentation/devicetree/bindings/clock/renesas,5p35023.yaml
> > > index 839648e753d4..db8d01b291dd 100644
> > > --- a/Documentation/devicetree/bindings/clock/renesas,5p35023.yaml
> > > +++ b/Documentation/devicetree/bindings/clock/renesas,5p35023.yaml
> > > @@ -49,6 +49,9 @@ properties:
> > >      $ref: /schemas/types.yaml#/definitions/uint8-array
> > >      maxItems: 37
> > >
> > > +  clock-output-names:
> > > +    maxItems: 6
> > > +
> >
> > Why do you need clock-output-names?
>
> I thought it will be useful information for a user, by looking at the example the name of clock-output-name and corresponding assigned-clocks and assigned-clock-rates.
>
> See below, from this one can understand the relation between index and actual clock output.
>
>   clock-output-names = "ref", "se1", "se2", "se3",
>                        "diff1", "diff2";
>
>   assigned-clocks = <&versa3 0>, <&versa3 1>,
>                     <&versa3 2>, <&versa3 3>,
>                     <&versa3 4>, <&versa3 5>;
>   assigned-clock-rates = <24000000>, <11289600>,
>                          <11289600>, <12000000>,
>                          <25000000>, <12288000>;
>
> > The clock output names should be created by the driver (taking into account
> > the instance number, so it works with multiple instances).
>
> OK, so shall I remove it from bindings then?

I think so, as it is not needed.

What is still missing (contrary to the Closes tag) is the mapping from
clock IDs to actual clock outputs.

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

[PATCH v2 3/3] arm64: dts: renesas: rz-smarc-common: Use versa3 clk for audio mclk

[Biju Das]

Currently audio mclk uses a fixed clk of 11.2896MHz (multiple of 44.1kHz).
Replace this fixed clk with the programmable versa3 clk that can provide
the clocking to support both 44.1kHz (with a clock of 11.2896MHz) and
48kHz (with a clock of 12.2880MHz), based on audio sampling rate for
playback and record.

Signed-off-by: Biju Das <biju.das.jz@bp.renesas.com>
---
v1->v2:
 * No change.
v1:
 * Added this patch as part of this series.
 * Replaced xtal->x1-clock and x1_x2->x1.
 * Added clock-output-names.
 * Updated clock-frequency = <400000> for RZ/G2UL i2c0
 * Updated assigned-clocks and assigned-clock-rates as per bindings.
 * Replaced mclk from '<&versa3 3>'->'<&versa3 2>'.
---
 .../boot/dts/renesas/rz-smarc-common.dtsi     | 14 +++++-----
 arch/arm64/boot/dts/renesas/rzg2l-smarc.dtsi  | 23 ++++++++++++++++
 arch/arm64/boot/dts/renesas/rzg2lc-smarc.dtsi | 23 ++++++++++++++++
 arch/arm64/boot/dts/renesas/rzg2ul-smarc.dtsi | 27 +++++++++++++++++++
 4 files changed, 80 insertions(+), 7 deletions(-)

diff --git a/arch/arm64/boot/dts/renesas/rz-smarc-common.dtsi b/arch/arm64/boot/dts/renesas/rz-smarc-common.dtsi
index a7594ba3a998..b7a3e6caa386 100644
--- a/arch/arm64/boot/dts/renesas/rz-smarc-common.dtsi
+++ b/arch/arm64/boot/dts/renesas/rz-smarc-common.dtsi
@@ -32,12 +32,6 @@ chosen {
 		stdout-path = "serial0:115200n8";
 	};
 
-	audio_mclock: audio_mclock {
-		compatible = "fixed-clock";
-		#clock-cells = <0>;
-		clock-frequency = <11289600>;
-	};
-
 	snd_rzg2l: sound {
 		compatible = "simple-audio-card";
 		simple-audio-card,format = "i2s";
@@ -55,7 +49,7 @@ cpu_dai: simple-audio-card,cpu {
 		};
 
 		codec_dai: simple-audio-card,codec {
-			clocks = <&audio_mclock>;
+			clocks = <&versa3 2>;
 			sound-dai = <&wm8978>;
 		};
 	};
@@ -76,6 +70,12 @@ vccq_sdhi1: regulator-vccq-sdhi1 {
 		gpios-states = <1>;
 		states = <3300000 1>, <1800000 0>;
 	};
+
+	x1: x1-clock {
+		compatible = "fixed-clock";
+		#clock-cells = <0>;
+		clock-frequency = <24000000>;
+	};
 };
 
 &audio_clk1 {
diff --git a/arch/arm64/boot/dts/renesas/rzg2l-smarc.dtsi b/arch/arm64/boot/dts/renesas/rzg2l-smarc.dtsi
index 68eab8e26bf2..186ca8f305db 100644
--- a/arch/arm64/boot/dts/renesas/rzg2l-smarc.dtsi
+++ b/arch/arm64/boot/dts/renesas/rzg2l-smarc.dtsi
@@ -105,6 +105,29 @@ &i2c3 {
 
 	status = "okay";
 
+	versa3: versa3@68 {
+		compatible = "renesas,5p35023";
+		reg = <0x68>;
+		#clock-cells = <1>;
+		clocks = <&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
+		];
+
+		clock-output-names = "ref", "se1", "se2", "se3",
+				     "diff1", "diff2";
+
+		assigned-clocks = <&versa3 0>, <&versa3 1>,
+				  <&versa3 2>, <&versa3 3>,
+				  <&versa3 4>, <&versa3 5>;
+		assigned-clock-rates = <24000000>, <11289600>,
+				       <11289600>, <12000000>,
+				       <25000000>, <12288000>;
+	};
+
 	wm8978: codec@1a {
 		compatible = "wlf,wm8978";
 		#sound-dai-cells = <0>;
diff --git a/arch/arm64/boot/dts/renesas/rzg2lc-smarc.dtsi b/arch/arm64/boot/dts/renesas/rzg2lc-smarc.dtsi
index 83fce96a2575..5abac6bc03c9 100644
--- a/arch/arm64/boot/dts/renesas/rzg2lc-smarc.dtsi
+++ b/arch/arm64/boot/dts/renesas/rzg2lc-smarc.dtsi
@@ -121,6 +121,29 @@ &i2c2 {
 
 	status = "okay";
 
+	versa3: versa3@68 {
+		compatible = "renesas,5p35023";
+		reg = <0x68>;
+		#clock-cells = <1>;
+		clocks = <&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
+		];
+
+		clock-output-names = "ref", "se1", "se2", "se3",
+				     "diff1", "diff2";
+
+		assigned-clocks = <&versa3 0>, <&versa3 1>,
+				  <&versa3 2>, <&versa3 3>,
+				  <&versa3 4>, <&versa3 5>;
+		assigned-clock-rates = <24000000>, <11289600>,
+				       <11289600>, <12000000>,
+				       <25000000>, <12288000>;
+	};
+
 	wm8978: codec@1a {
 		compatible = "wlf,wm8978";
 		#sound-dai-cells = <0>;
diff --git a/arch/arm64/boot/dts/renesas/rzg2ul-smarc.dtsi b/arch/arm64/boot/dts/renesas/rzg2ul-smarc.dtsi
index 8eb411aac80d..7e0a5814824e 100644
--- a/arch/arm64/boot/dts/renesas/rzg2ul-smarc.dtsi
+++ b/arch/arm64/boot/dts/renesas/rzg2ul-smarc.dtsi
@@ -20,6 +20,33 @@ &cpu_dai {
 	sound-dai = <&ssi1>;
 };
 
+&i2c0 {
+	clock-frequency = <400000>;
+
+	versa3: versa3@68 {
+		compatible = "renesas,5p35023";
+		reg = <0x68>;
+		#clock-cells = <1>;
+		clocks = <&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
+		];
+
+		clock-output-names = "ref", "se1", "se2", "se3",
+				     "diff1", "diff2";
+
+		assigned-clocks = <&versa3 0>, <&versa3 1>,
+				  <&versa3 2>, <&versa3 3>,
+				  <&versa3 4>, <&versa3 5>;
+		assigned-clock-rates = <24000000>, <11289600>,
+				       <11289600>, <12000000>,
+				       <25000000>, <12288000>;
+	};
+};
+
 &i2c1 {
 	wm8978: codec@1a {
 		compatible = "wlf,wm8978";
-- 
2.25.1