[PATCH 0/3] arm64: dts: renesas: r8a77995: draak: add R-Car Sound support

[PATCH 0/3] arm64: dts: renesas: r8a77995: draak: add R-Car Sound support

[Kuninori Morimoto]

Hi Geert

These add R-Car Sound support for D3 draak board.
It is based on v5.13-rc1

Kuninori Morimoto (3):
  clk: renesas: r8a77995: Add ZA2 clock
  arm64: dts: renesas: r8a77995: add R-Car Sound support
  arm64: dts: renesas: r8a77995: draak: Add R-Car Sound support

 .../arm64/boot/dts/renesas/r8a77995-draak.dts | 103 ++++++++++
 arch/arm64/boot/dts/renesas/r8a77995.dtsi     | 176 ++++++++++++++++++
 drivers/clk/renesas/r8a77995-cpg-mssr.c       |   3 +
 3 files changed, 282 insertions(+)

-- 
2.25.1

[PATCH 1/3] clk: renesas: r8a77995: Add ZA2 clock

[Kuninori Morimoto]

From: Kuninori Morimoto <kuninori.morimoto.gx@renesas.com>

Implement support for the ZA2 clock which is needed
for R-Car Sound.

Signed-off-by: Kuninori Morimoto <kuninori.morimoto.gx@renesas.com>
---
 drivers/clk/renesas/r8a77995-cpg-mssr.c | 3 +++
 1 file changed, 3 insertions(+)

diff --git a/drivers/clk/renesas/r8a77995-cpg-mssr.c b/drivers/clk/renesas/r8a77995-cpg-mssr.c
index 9cfd00cf4e69..8fb84ed6fe08 100644
--- a/drivers/clk/renesas/r8a77995-cpg-mssr.c
+++ b/drivers/clk/renesas/r8a77995-cpg-mssr.c
@@ -35,6 +35,7 @@ enum clk_ids {
 	CLK_PLL0D2,
 	CLK_PLL0D3,
 	CLK_PLL0D5,
+	CLK_PLL0D24,
 	CLK_PLL1D2,
 	CLK_PE,
 	CLK_S0,
@@ -62,6 +63,7 @@ static const struct cpg_core_clk r8a77995_core_clks[] __initconst = {
 	DEF_FIXED(".pll0d2",   CLK_PLL0D2,         CLK_PLL0,       2, 1),
 	DEF_FIXED(".pll0d3",   CLK_PLL0D3,         CLK_PLL0,       3, 1),
 	DEF_FIXED(".pll0d5",   CLK_PLL0D5,         CLK_PLL0,       5, 1),
+	DEF_FIXED(".pll0d24",  CLK_PLL0D24,        CLK_PLL0,      24, 1),
 	DEF_FIXED(".pll1d2",   CLK_PLL1D2,         CLK_PLL1,       2, 1),
 	DEF_FIXED(".pe",       CLK_PE,             CLK_PLL0D3,     4, 1),
 	DEF_FIXED(".s0",       CLK_S0,             CLK_PLL1,       2, 1),
@@ -75,6 +77,7 @@ static const struct cpg_core_clk r8a77995_core_clks[] __initconst = {
 	DEF_RATE(".oco",       CLK_OCO,            8 * 1000 * 1000),
 
 	/* Core Clock Outputs */
+	DEF_FIXED("za2",       R8A77995_CLK_ZA2,   CLK_PLL0D24,    1, 1),
 	DEF_FIXED("z2",        R8A77995_CLK_Z2,    CLK_PLL0D3,     1, 1),
 	DEF_FIXED("ztr",       R8A77995_CLK_ZTR,   CLK_PLL1,       6, 1),
 	DEF_FIXED("zt",        R8A77995_CLK_ZT,    CLK_PLL1,       4, 1),
-- 
2.25.1

Re: [PATCH 1/3] clk: renesas: r8a77995: Add ZA2 clock

[Geert Uytterhoeven]

Hi Morimoto-san,

On Mon, May 17, 2021 at 2:36 AM Kuninori Morimoto
<kuninori.morimoto.gx@renesas.com> wrote:
> From: Kuninori Morimoto <kuninori.morimoto.gx@renesas.com>
>
> Implement support for the ZA2 clock which is needed
> for R-Car Sound.
>
> Signed-off-by: Kuninori Morimoto <kuninori.morimoto.gx@renesas.com>

Thanks for your patch!

> --- a/drivers/clk/renesas/r8a77995-cpg-mssr.c
> +++ b/drivers/clk/renesas/r8a77995-cpg-mssr.c
> @@ -35,6 +35,7 @@ enum clk_ids {
>         CLK_PLL0D2,
>         CLK_PLL0D3,
>         CLK_PLL0D5,
> +       CLK_PLL0D24,
>         CLK_PLL1D2,
>         CLK_PE,
>         CLK_S0,
> @@ -62,6 +63,7 @@ static const struct cpg_core_clk r8a77995_core_clks[] __initconst = {
>         DEF_FIXED(".pll0d2",   CLK_PLL0D2,         CLK_PLL0,       2, 1),
>         DEF_FIXED(".pll0d3",   CLK_PLL0D3,         CLK_PLL0,       3, 1),
>         DEF_FIXED(".pll0d5",   CLK_PLL0D5,         CLK_PLL0,       5, 1),
> +       DEF_FIXED(".pll0d24",  CLK_PLL0D24,        CLK_PLL0,      24, 1),
>         DEF_FIXED(".pll1d2",   CLK_PLL1D2,         CLK_PLL1,       2, 1),
>         DEF_FIXED(".pe",       CLK_PE,             CLK_PLL0D3,     4, 1),
>         DEF_FIXED(".s0",       CLK_S0,             CLK_PLL1,       2, 1),
> @@ -75,6 +77,7 @@ static const struct cpg_core_clk r8a77995_core_clks[] __initconst = {
>         DEF_RATE(".oco",       CLK_OCO,            8 * 1000 * 1000),
>
>         /* Core Clock Outputs */
> +       DEF_FIXED("za2",       R8A77995_CLK_ZA2,   CLK_PLL0D24,    1, 1),

This does not match the Hardware User's Manual (Rev. 2.20):
  1. ZA2 is not a fixed clock, but can be controlled through the ZA2CKCR
     register.  Adding support for that requires writing a custom clock
     driver.
     Of course we can consider it a fixed clock initially, and make it
     configurable later, when time permits.
  2. The parent clock is either PLL0D3 or S0, with a configurable
     post-divider of 2 or 4, yielding 200, 250, 400, or 500[*] MHz.
     Using plain PLL0D24 would mean a post-divider of 8, yielding
     125 MHz, which is not documented as a supported value.

[*] Using the default would mean:

    DEF_FIXED("za2", R8A77995_CLK_ZA2, CLK_PLL0D3, 4, 1);

>         DEF_FIXED("z2",        R8A77995_CLK_Z2,    CLK_PLL0D3,     1, 1),
>         DEF_FIXED("ztr",       R8A77995_CLK_ZTR,   CLK_PLL1,       6, 1),
>         DEF_FIXED("zt",        R8A77995_CLK_ZT,    CLK_PLL1,       4, 1),

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 1/3] clk: renesas: r8a77995: Add ZA2 clock

[Kuninori Morimoto]

Hi Geert

Thank you for your review

> > @@ -75,6 +77,7 @@ static const struct cpg_core_clk r8a77995_core_clks[] __initconst = {
> >         DEF_RATE(".oco",       CLK_OCO,            8 * 1000 * 1000),
> >
> >         /* Core Clock Outputs */
> > +       DEF_FIXED("za2",       R8A77995_CLK_ZA2,   CLK_PLL0D24,    1, 1),
> 
> This does not match the Hardware User's Manual (Rev. 2.20):
>   1. ZA2 is not a fixed clock, but can be controlled through the ZA2CKCR
>      register.  Adding support for that requires writing a custom clock
>      driver.
>      Of course we can consider it a fixed clock initially, and make it
>      configurable later, when time permits.
>   2. The parent clock is either PLL0D3 or S0, with a configurable
>      post-divider of 2 or 4, yielding 200, 250, 400, or 500[*] MHz.
>      Using plain PLL0D24 would mean a post-divider of 8, yielding
>      125 MHz, which is not documented as a supported value.
> 
> [*] Using the default would mean:
> 
>     DEF_FIXED("za2", R8A77995_CLK_ZA2, CLK_PLL0D3, 4, 1);

Oops, I had checked E3 block.
Thank you pointing it. will fix in v2


Thank you for your help !!

Best regards
---
Kuninori Morimoto

Re: [PATCH 1/3] clk: renesas: r8a77995: Add ZA2 clock

[Kuninori Morimoto]

Hi Geert

> > This does not match the Hardware User's Manual (Rev. 2.20):
> >   1. ZA2 is not a fixed clock, but can be controlled through the ZA2CKCR
> >      register.  Adding support for that requires writing a custom clock
> >      driver.
> >      Of course we can consider it a fixed clock initially, and make it
> >      configurable later, when time permits.
> >   2. The parent clock is either PLL0D3 or S0, with a configurable
> >      post-divider of 2 or 4, yielding 200, 250, 400, or 500[*] MHz.
> >      Using plain PLL0D24 would mean a post-divider of 8, yielding
> >      125 MHz, which is not documented as a supported value.
> > 
> > [*] Using the default would mean:
> > 
> >     DEF_FIXED("za2", R8A77995_CLK_ZA2, CLK_PLL0D3, 4, 1);

PLL0 * 1/3 = 1GHz.
And default ZA2 on D3 is 500MHz thus it will be below
but am I misunderstanding ?

	- DEF_FIXED("za2", R8A77995_CLK_ZA2, CLK_PLL0D3, 4, 1);
	+ DEF_FIXED("za2", R8A77995_CLK_ZA2, CLK_PLL0D3, 2, 1);

Thank you for your help !!

Best regards
---
Kuninori Morimoto

Re: [PATCH 1/3] clk: renesas: r8a77995: Add ZA2 clock

[Geert Uytterhoeven]

Hi Morimoto-san,

On Mon, May 24, 2021 at 2:57 AM Kuninori Morimoto
<kuninori.morimoto.gx@renesas.com> wrote:
> > > This does not match the Hardware User's Manual (Rev. 2.20):
> > >   1. ZA2 is not a fixed clock, but can be controlled through the ZA2CKCR
> > >      register.  Adding support for that requires writing a custom clock
> > >      driver.
> > >      Of course we can consider it a fixed clock initially, and make it
> > >      configurable later, when time permits.
> > >   2. The parent clock is either PLL0D3 or S0, with a configurable
> > >      post-divider of 2 or 4, yielding 200, 250, 400, or 500[*] MHz.
> > >      Using plain PLL0D24 would mean a post-divider of 8, yielding
> > >      125 MHz, which is not documented as a supported value.
> > >
> > > [*] Using the default would mean:
> > >
> > >     DEF_FIXED("za2", R8A77995_CLK_ZA2, CLK_PLL0D3, 4, 1);
>
> PLL0 * 1/3 = 1GHz.
> And default ZA2 on D3 is 500MHz thus it will be below
> but am I misunderstanding ?
>
>         - DEF_FIXED("za2", R8A77995_CLK_ZA2, CLK_PLL0D3, 4, 1);
>         + DEF_FIXED("za2", R8A77995_CLK_ZA2, CLK_PLL0D3, 2, 1);

Yes, /2 instead of /4. Sorry for the confusion.

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 2/3] arm64: dts: renesas: r8a77995: add R-Car Sound support

[Kuninori Morimoto]

From: Kuninori Morimoto <kuninori.morimoto.gx@renesas.com>

This patch adds R-Car Sound and Audio-DMAC support for D3.

1st note is that D3 doesn't have audio-clk-c, but is
required from driver. This patch uses null-clk for it.

2nd note is that D3 has SSI3/4 and SRC5/6 only, but driver
requres from SSI0/SRC0. This patch has disabled SSI/SRC for it.

Signed-off-by: Kuninori Morimoto <kuninori.morimoto.gx@renesas.com>
---
 arch/arm64/boot/dts/renesas/r8a77995.dtsi | 176 ++++++++++++++++++++++
 1 file changed, 176 insertions(+)

diff --git a/arch/arm64/boot/dts/renesas/r8a77995.dtsi b/arch/arm64/boot/dts/renesas/r8a77995.dtsi
index 2319271c881b..0950f39711eb 100644
--- a/arch/arm64/boot/dts/renesas/r8a77995.dtsi
+++ b/arch/arm64/boot/dts/renesas/r8a77995.dtsi
@@ -43,6 +43,33 @@ L2_CA53: cache-controller-1 {
 		};
 	};
 
+	/*
+	 * The external audio clocks are configured as 0 Hz fixed frequency
+	 * clocks by default.
+	 * Boards that provide audio clocks should override them.
+	 */
+	audio_clk_a: audio_clk_a {
+		compatible = "fixed-clock";
+		#clock-cells = <0>;
+		clock-frequency = <0>;
+	};
+
+	audio_clk_b: audio_clk_b {
+		compatible = "fixed-clock";
+		#clock-cells = <0>;
+		clock-frequency = <0>;
+	};
+
+	/*
+	 * R8A77995 doesn't have audio_clk_c, * but is required from driver.
+	 * Create NULL clock for it.
+	 */
+	null_clk: null_clk {
+		compatible = "fixed-clock";
+		#clock-cells = <0>;
+		clock-frequency = <0>;
+	};
+
 	extal_clk: extal {
 		compatible = "fixed-clock";
 		#clock-cells = <0>;
@@ -642,6 +669,48 @@ dmac2: dma-controller@e7310000 {
 			       <&ipmmu_ds1 22>, <&ipmmu_ds1 23>;
 		};
 
+		audma0: dma-controller@ec700000 {
+			compatible = "renesas,dmac-r8a77995",
+				     "renesas,rcar-dmac";
+			reg = <0 0xec700000 0 0x10000>;
+			interrupts = <GIC_SPI 350 IRQ_TYPE_LEVEL_HIGH>,
+				     <GIC_SPI 320 IRQ_TYPE_LEVEL_HIGH>,
+				     <GIC_SPI 321 IRQ_TYPE_LEVEL_HIGH>,
+				     <GIC_SPI 322 IRQ_TYPE_LEVEL_HIGH>,
+				     <GIC_SPI 323 IRQ_TYPE_LEVEL_HIGH>,
+				     <GIC_SPI 324 IRQ_TYPE_LEVEL_HIGH>,
+				     <GIC_SPI 325 IRQ_TYPE_LEVEL_HIGH>,
+				     <GIC_SPI 326 IRQ_TYPE_LEVEL_HIGH>,
+				     <GIC_SPI 327 IRQ_TYPE_LEVEL_HIGH>,
+				     <GIC_SPI 328 IRQ_TYPE_LEVEL_HIGH>,
+				     <GIC_SPI 329 IRQ_TYPE_LEVEL_HIGH>,
+				     <GIC_SPI 330 IRQ_TYPE_LEVEL_HIGH>,
+				     <GIC_SPI 331 IRQ_TYPE_LEVEL_HIGH>,
+				     <GIC_SPI 332 IRQ_TYPE_LEVEL_HIGH>,
+				     <GIC_SPI 333 IRQ_TYPE_LEVEL_HIGH>,
+				     <GIC_SPI 334 IRQ_TYPE_LEVEL_HIGH>,
+				     <GIC_SPI 335 IRQ_TYPE_LEVEL_HIGH>;
+			interrupt-names = "error",
+					"ch0", "ch1", "ch2", "ch3",
+					"ch4", "ch5", "ch6", "ch7",
+					"ch8", "ch9", "ch10", "ch11",
+					"ch12", "ch13", "ch14", "ch15";
+			clocks = <&cpg CPG_MOD 502>;
+			clock-names = "fck";
+			power-domains = <&sysc R8A77995_PD_ALWAYS_ON>;
+			resets = <&cpg 502>;
+			#dma-cells = <1>;
+			dma-channels = <16>;
+			iommus = <&ipmmu_mp 0>, <&ipmmu_mp 1>,
+				 <&ipmmu_mp 2>, <&ipmmu_mp 3>,
+				 <&ipmmu_mp 4>, <&ipmmu_mp 5>,
+				 <&ipmmu_mp 6>, <&ipmmu_mp 7>,
+				 <&ipmmu_mp 8>, <&ipmmu_mp 9>,
+				 <&ipmmu_mp 10>, <&ipmmu_mp 11>,
+				 <&ipmmu_mp 12>, <&ipmmu_mp 13>,
+				 <&ipmmu_mp 14>, <&ipmmu_mp 15>;
+		};
+
 		ipmmu_ds0: iommu@e6740000 {
 			compatible = "renesas,ipmmu-r8a77995";
 			reg = <0 0xe6740000 0 0x1000>;
@@ -1258,6 +1327,113 @@ prr: chipid@fff00044 {
 			compatible = "renesas,prr";
 			reg = <0 0xfff00044 0 4>;
 		};
+
+		rcar_sound: sound@ec500000 {
+			/*
+			 * #sound-dai-cells is required
+			 *
+			 * Single DAI : #sound-dai-cells = <0>;	<&rcar_sound>;
+			 * Multi  DAI : #sound-dai-cells = <1>;	<&rcar_sound N>;
+			 */
+			/*
+			 * #clock-cells is required for audio_clkout0/1/2/3
+			 *
+			 * clkout	: #clock-cells = <0>;	<&rcar_sound>;
+			 * clkout0/1/2/3: #clock-cells = <1>;	<&rcar_sound N>;
+			 */
+			compatible =  "renesas,rcar_sound-r8a77995", "renesas,rcar_sound-gen3";
+			reg =	<0 0xec500000 0 0x1000>, /* SCU */
+				<0 0xec5a0000 0 0x100>,  /* ADG */
+				<0 0xec540000 0 0x1000>, /* SSIU */
+				<0 0xec541000 0 0x280>,  /* SSI */
+				<0 0xec740000 0 0x200>;  /* Audio DMAC peri peri*/
+			reg-names = "scu", "adg", "ssiu", "ssi", "audmapp";
+
+			clocks = <&cpg CPG_MOD 1005>,
+				 <&cpg CPG_MOD 1011>, <&cpg CPG_MOD 1012>,
+				 <&cpg CPG_MOD 1025>, <&cpg CPG_MOD 1026>,
+				 <&cpg CPG_MOD 1020>, <&cpg CPG_MOD 1021>,
+				 <&cpg CPG_MOD 1020>, <&cpg CPG_MOD 1021>,
+				 <&cpg CPG_MOD 1019>, <&cpg CPG_MOD 1018>,
+				 <&audio_clk_a>, <&audio_clk_b>, <&null_clk>, /* clk_c is NULL */
+				 <&cpg CPG_CORE R8A77995_CLK_ZA2>;
+			clock-names = "ssi-all",
+				      "ssi.4", "ssi.3",
+				      "src.6", "src.5",
+				      "mix.1", "mix.0",
+				      "ctu.1", "ctu.0",
+				      "dvc.0", "dvc.1",
+				      "clk_a", "clk_b", "clk_c", "clk_i"; /* clk_c is NULL */
+			power-domains = <&sysc R8A77995_PD_ALWAYS_ON>;
+			resets = <&cpg 1005>,
+				 <&cpg 1011>, <&cpg 1012>;
+			reset-names = "ssi-all",
+				      "ssi.4", "ssi.3";
+			status = "disabled";
+
+			rcar_sound,ctu {
+				ctu00: ctu-0 { };
+				ctu01: ctu-1 { };
+				ctu02: ctu-2 { };
+				ctu03: ctu-3 { };
+				ctu10: ctu-4 { };
+				ctu11: ctu-5 { };
+				ctu12: ctu-6 { };
+				ctu13: ctu-7 { };
+			};
+
+			rcar_sound,dvc {
+				dvc0: dvc-0 {
+					dmas = <&audma0 0xbc>;
+					dma-names = "tx";
+				};
+				dvc1: dvc-1 {
+					dmas = <&audma0 0xbe>;
+					dma-names = "tx";
+				};
+			};
+
+			rcar_sound,mix {
+				mix0: mix-0 { };
+				mix1: mix-1 { };
+			};
+
+			rcar_sound,src {
+				src0: src-0 { status = "disabled"; };
+				src1: src-1 { status = "disabled"; };
+				src2: src-2 { status = "disabled"; };
+				src3: src-3 { status = "disabled"; };
+				src4: src-4 { status = "disabled"; };
+				src5: src-5 {
+					interrupts = <GIC_SPI 357 IRQ_TYPE_LEVEL_HIGH>;
+					dmas = <&audma0 0x8f>, <&audma0 0xb2>;
+					dma-names = "rx", "tx";
+				};
+				src6: src-6 {
+					interrupts = <GIC_SPI 358 IRQ_TYPE_LEVEL_HIGH>;
+					dmas = <&audma0 0x91>, <&audma0 0xb4>;
+					dma-names = "rx", "tx";
+				};
+			};
+
+			rcar_sound,ssi {
+				ssi0: ssi-0 { status = "disabled"; };
+				ssi1: ssi-1 { status = "disabled"; };
+				ssi2: ssi-2 { status = "disabled"; };
+				ssi3: ssi-3 {
+					interrupts = <GIC_SPI 373 IRQ_TYPE_LEVEL_HIGH>;
+					dmas = <&audma0 0x07>, <&audma0 0x08>,
+					       <&audma0 0x6f>, <&audma0 0x70>;
+					dma-names = "rx", "tx", "rxu", "txu";
+				};
+				ssi4: ssi-4 {
+					interrupts = <GIC_SPI 374 IRQ_TYPE_LEVEL_HIGH>;
+					dmas = <&audma0 0x09>, <&audma0 0x0a>,
+					       <&audma0 0x71>, <&audma0 0x72>;
+					dma-names = "rx", "tx", "rxu", "txu";
+				};
+			};
+		};
 	};
 
 	thermal-zones {
-- 
2.25.1

Re: [PATCH 2/3] arm64: dts: renesas: r8a77995: add R-Car Sound support

[Geert Uytterhoeven]

Hi Morimoto-san,

Thanks for your patch!

On Mon, May 17, 2021 at 2:36 AM Kuninori Morimoto
<kuninori.morimoto.gx@renesas.com> wrote:
> From: Kuninori Morimoto <kuninori.morimoto.gx@renesas.com>
>
> This patch adds R-Car Sound and Audio-DMAC support for D3.
>
> 1st note is that D3 doesn't have audio-clk-c, but is
> required from driver. This patch uses null-clk for it.
>
> 2nd note is that D3 has SSI3/4 and SRC5/6 only, but driver
> requres from SSI0/SRC0. This patch has disabled SSI/SRC for it.

I think it would be better to fix the driver instead.

>
> Signed-off-by: Kuninori Morimoto <kuninori.morimoto.gx@renesas.com>
> ---
>  arch/arm64/boot/dts/renesas/r8a77995.dtsi | 176 ++++++++++++++++++++++
>  1 file changed, 176 insertions(+)
>
> diff --git a/arch/arm64/boot/dts/renesas/r8a77995.dtsi b/arch/arm64/boot/dts/renesas/r8a77995.dtsi
> index 2319271c881b..0950f39711eb 100644
> --- a/arch/arm64/boot/dts/renesas/r8a77995.dtsi
> +++ b/arch/arm64/boot/dts/renesas/r8a77995.dtsi
> @@ -43,6 +43,33 @@ L2_CA53: cache-controller-1 {
>                 };
>         };
>
> +       /*
> +        * The external audio clocks are configured as 0 Hz fixed frequency
> +        * clocks by default.
> +        * Boards that provide audio clocks should override them.
> +        */
> +       audio_clk_a: audio_clk_a {

Please use alphabetical sort order when adding nodes without unit
addresses.

> +               compatible = "fixed-clock";
> +               #clock-cells = <0>;
> +               clock-frequency = <0>;
> +       };
> +
> +       audio_clk_b: audio_clk_b {
> +               compatible = "fixed-clock";
> +               #clock-cells = <0>;
> +               clock-frequency = <0>;
> +       };
> +
> +       /*
> +        * R8A77995 doesn't have audio_clk_c, * but is required from driver.
> +        * Create NULL clock for it.
> +        */
> +       null_clk: null_clk {
> +               compatible = "fixed-clock";
> +               #clock-cells = <0>;
> +               clock-frequency = <0>;
> +       };

Please do not add nodes not matching the hardware description, but fix
the driver instead. ("DT describes hardware, not software policy").

> +
>         extal_clk: extal {
>                 compatible = "fixed-clock";
>                 #clock-cells = <0>;
> @@ -642,6 +669,48 @@ dmac2: dma-controller@e7310000 {
>                                <&ipmmu_ds1 22>, <&ipmmu_ds1 23>;
>                 };
>
> +               audma0: dma-controller@ec700000 {

Please use numerical sort order when adding nodes with unit addresses.

> @@ -1258,6 +1327,113 @@ prr: chipid@fff00044 {
>                         compatible = "renesas,prr";
>                         reg = <0 0xfff00044 0 4>;
>                 };
> +
> +               rcar_sound: sound@ec500000 {

Please use numerical sort order when adding nodes with unit addresses.

> +                       /*
> +                        * #sound-dai-cells is required
> +                        *
> +                        * Single DAI : #sound-dai-cells = <0>; <&rcar_sound>;
> +                        * Multi  DAI : #sound-dai-cells = <1>; <&rcar_sound N>;
> +                        */
> +                       /*
> +                        * #clock-cells is required for audio_clkout0/1/2/3
> +                        *
> +                        * clkout       : #clock-cells = <0>;   <&rcar_sound>;
> +                        * clkout0/1/2/3: #clock-cells = <1>;   <&rcar_sound N>;
> +                        */
> +                       compatible =  "renesas,rcar_sound-r8a77995", "renesas,rcar_sound-gen3";
> +                       reg =   <0 0xec500000 0 0x1000>, /* SCU */
> +                               <0 0xec5a0000 0 0x100>,  /* ADG */
> +                               <0 0xec540000 0 0x1000>, /* SSIU */
> +                               <0 0xec541000 0 0x280>,  /* SSI */
> +                               <0 0xec740000 0 0x200>;  /* Audio DMAC peri peri*/
> +                       reg-names = "scu", "adg", "ssiu", "ssi", "audmapp";
> +
> +                       clocks = <&cpg CPG_MOD 1005>,
> +                                <&cpg CPG_MOD 1011>, <&cpg CPG_MOD 1012>,
> +                                <&cpg CPG_MOD 1025>, <&cpg CPG_MOD 1026>,
> +                                <&cpg CPG_MOD 1020>, <&cpg CPG_MOD 1021>,
> +                                <&cpg CPG_MOD 1020>, <&cpg CPG_MOD 1021>,
> +                                <&cpg CPG_MOD 1019>, <&cpg CPG_MOD 1018>,
> +                                <&audio_clk_a>, <&audio_clk_b>, <&null_clk>, /* clk_c is NULL */
> +                                <&cpg CPG_CORE R8A77995_CLK_ZA2>;
> +                       clock-names = "ssi-all",
> +                                     "ssi.4", "ssi.3",
> +                                     "src.6", "src.5",
> +                                     "mix.1", "mix.0",
> +                                     "ctu.1", "ctu.0",
> +                                     "dvc.0", "dvc.1",
> +                                     "clk_a", "clk_b", "clk_c", "clk_i"; /* clk_c is NULL */
> +                       power-domains = <&sysc R8A77995_PD_ALWAYS_ON>;
> +                       resets = <&cpg 1005>,
> +                                <&cpg 1011>, <&cpg 1012>;
> +                       reset-names = "ssi-all",
> +                                     "ssi.4", "ssi.3";

arch/arm64/boot/dts/renesas/r8a77995-draak.dt.yaml: sound@ec500000:
resets: [[4, 1005], [4, 1011], [4, 1012]] is too short
        From schema: Documentation/devicetree/bindings/sound/renesas,rsnd.yaml
arch/arm64/boot/dts/renesas/r8a77995-draak.dt.yaml: sound@ec500000:
reset-names: ['ssi-all', 'ssi.4', 'ssi.3'] is too short
        From schema: Documentation/devicetree/bindings/sound/renesas,rsnd.yaml

As the DTS is correct, the DT bindings should be updated.

> +                       rcar_sound,src {
> +                               src0: src-0 { status = "disabled"; };
> +                               src1: src-1 { status = "disabled"; };
> +                               src2: src-2 { status = "disabled"; };
> +                               src3: src-3 { status = "disabled"; };
> +                               src4: src-4 { status = "disabled"; };

Please drop nonexistent src channels.

> +                               src5: src-5 {
> +                                       interrupts = <GIC_SPI 357 IRQ_TYPE_LEVEL_HIGH>;
> +                                       dmas = <&audma0 0x8f>, <&audma0 0xb2>;
> +                                       dma-names = "rx", "tx";
> +                               };
> +                               src6: src-6 {
> +                                       interrupts = <GIC_SPI 358 IRQ_TYPE_LEVEL_HIGH>;
> +                                       dmas = <&audma0 0x91>, <&audma0 0xb4>;
> +                                       dma-names = "rx", "tx";
> +                               };
> +                       };
> +
> +                       rcar_sound,ssi {
> +                               ssi0: ssi-0 { status = "disabled"; };
> +                               ssi1: ssi-1 { status = "disabled"; };
> +                               ssi2: ssi-2 { status = "disabled"; };

Please drop nonexistent ssi channels.

> +                               ssi3: ssi-3 {
> +                                       interrupts = <GIC_SPI 373 IRQ_TYPE_LEVEL_HIGH>;
> +                                       dmas = <&audma0 0x07>, <&audma0 0x08>,
> +                                              <&audma0 0x6f>, <&audma0 0x70>;
> +                                       dma-names = "rx", "tx", "rxu", "txu";
> +                               };
> +                               ssi4: ssi-4 {
> +                                       interrupts = <GIC_SPI 374 IRQ_TYPE_LEVEL_HIGH>;
> +                                       dmas = <&audma0 0x09>, <&audma0 0x0a>,
> +                                              <&audma0 0x71>, <&audma0 0x72>;
> +                                       dma-names = "rx", "tx", "rxu", "txu";
> +                               };
> +                       };
> +               };
>         };
>
>         thermal-zones {

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 2/3] arm64: dts: renesas: r8a77995: add R-Car Sound support

[Kuninori Morimoto]

Hi Geert

> > From: Kuninori Morimoto <kuninori.morimoto.gx@renesas.com>
> >
> > This patch adds R-Car Sound and Audio-DMAC support for D3.
> >
> > 1st note is that D3 doesn't have audio-clk-c, but is
> > required from driver. This patch uses null-clk for it.
> >
> > 2nd note is that D3 has SSI3/4 and SRC5/6 only, but driver
> > requres from SSI0/SRC0. This patch has disabled SSI/SRC for it.
> 
> I think it would be better to fix the driver instead.

We already have this kind of SoC before.
And I guess it has these, but just not counnected.
It is Hardware side condition.

> > +       /*
> > +        * R8A77995 doesn't have audio_clk_c, * but is required from driver.
> > +        * Create NULL clock for it.
> > +        */
> > +       null_clk: null_clk {
> > +               compatible = "fixed-clock";
> > +               #clock-cells = <0>;
> > +               clock-frequency = <0>;
> > +       };
> 
> Please do not add nodes not matching the hardware description, but fix
> the driver instead. ("DT describes hardware, not software policy").

Yeah, I agree.
Will fix driver first, and repost SoC patch, again.

Thank you for your help !!

Best regards
---
Kuninori Morimoto

Re: [PATCH 2/3] arm64: dts: renesas: r8a77995: add R-Car Sound support

[Kuninori Morimoto]

Hi Geert

> Please use numerical sort order when adding nodes with unit addresses.

It seems r8a77995.dtsi numerical sort order is already broken.
Or am I misunderstanding ?

	...
	ipmmu_vi0: iommu@febd0000
	ipmmu_vp0: iommu@fe990000
=>	avb: ethernet@e6800000
=>	can0: can@e6c30000
	---- and ore ----


Thank you for your help !!

Best regards
---
Kuninori Morimoto

Re: [PATCH 2/3] arm64: dts: renesas: r8a77995: add R-Car Sound support

[Geert Uytterhoeven]

Hi Morimoto-san,

On Mon, May 24, 2021 at 7:43 AM Kuninori Morimoto
<kuninori.morimoto.gx@renesas.com> wrote:
> > Please use numerical sort order when adding nodes with unit addresses.
>
> It seems r8a77995.dtsi numerical sort order is already broken.
> Or am I misunderstanding ?
>
>         ...
>         ipmmu_vi0: iommu@febd0000
>         ipmmu_vp0: iommu@fe990000
> =>      avb: ethernet@e6800000
> =>      can0: can@e6c30000
>         ---- and ore ----

We use numerical sort order, but group similar entries together.
I.e. all iommu nodes are together, and their position is determined by
the unit address of the first iommu node.

I really should write a script to automate sorting, and propose that as
the gold standard...

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 2/3] arm64: dts: renesas: r8a77995: add R-Car Sound support

[Kuninori Morimoto]

Hi Geert

> >         ...
> >         ipmmu_vi0: iommu@febd0000
> >         ipmmu_vp0: iommu@fe990000
> > =>      avb: ethernet@e6800000
> > =>      can0: can@e6c30000
> >         ---- and ore ----
> 
> We use numerical sort order, but group similar entries together.
> I.e. all iommu nodes are together, and their position is determined by
> the unit address of the first iommu node.

Ahh, OK, I see.
So I guess my posted patch was not good order again ?

Thank you for your help !!

Best regards
---
Kuninori Morimoto

[PATCH 3/3] arm64: dts: renesas: r8a77995: draak: Add R-Car Sound support

[Kuninori Morimoto]

From: Kuninori Morimoto <kuninori.morimoto.gx@renesas.com>

This patch adds R-Car Sound support for D3 draak.

One note is that it is using different clock definition style
from ulcb/salvator boards to avoid verbose clocks settings
on rcar_sound node (see ulcb.dtsi  rcar_sound::clocks).

cs2000 and ADG are closs connected, and needs each other.
ulcb/salvator boards assume drivers are probed cs2000 -> rcar_sound.
This draak board assumes drivers are probed rcar_sound -> cs2000.

Signed-off-by: Kuninori Morimoto <kuninori.morimoto.gx@renesas.com>
---
 .../arm64/boot/dts/renesas/r8a77995-draak.dts | 103 ++++++++++++++++++
 1 file changed, 103 insertions(+)

diff --git a/arch/arm64/boot/dts/renesas/r8a77995-draak.dts b/arch/arm64/boot/dts/renesas/r8a77995-draak.dts
index 6783c3ad0856..591fad289802 100644
--- a/arch/arm64/boot/dts/renesas/r8a77995-draak.dts
+++ b/arch/arm64/boot/dts/renesas/r8a77995-draak.dts
@@ -6,6 +6,11 @@
  * Copyright (C) 2017 Glider bvba
  */
 
+/*
+ * This assumes...
+ *	SW60 : 2-1
+ */
+
 /dts-v1/;
 #include "r8a77995.dtsi"
 #include <dt-bindings/gpio/gpio.h>
@@ -156,11 +161,25 @@ adv7123_out: endpoint {
 		};
 	};
 
+	sound_card: sound {
+		compatible = "audio-graph-card";
+
+		dais = <&rsnd_port0	/* ak4613 */
+			/* HDMI is not yet supported */
+		>;
+	};
+
 	x12_clk: x12 {
 		compatible = "fixed-clock";
 		#clock-cells = <0>;
 		clock-frequency = <74250000>;
 	};
+
+	x19_clk: x19 {
+		compatible = "fixed-clock";
+		#clock-cells = <0>;
+		clock-frequency = <24576000>;
+	};
 };
 
 &avb {
@@ -347,6 +366,39 @@ eeprom@50 {
 		reg = <0x50>;
 		pagesize = <8>;
 	};
+
+	ak4613: codec@10 {
+		compatible = "asahi-kasei,ak4613";
+		#sound-dai-cells = <0>;
+		reg = <0x10>;
+		clocks = <&rcar_sound 0>; /* audio_clkout */
+
+		asahi-kasei,in1-single-end;
+		asahi-kasei,in2-single-end;
+		asahi-kasei,out1-single-end;
+		asahi-kasei,out2-single-end;
+		asahi-kasei,out3-single-end;
+		asahi-kasei,out4-single-end;
+		asahi-kasei,out5-single-end;
+		asahi-kasei,out6-single-end;
+
+		port {
+			ak4613_endpoint: endpoint {
+				remote-endpoint = <&rsnd_for_ak4613>;
+			};
+		};
+	};
+
+	cs2000: clk-multiplier@4f {
+		#clock-cells = <0>;
+		compatible = "cirrus,cs2000-cp";
+		reg = <0x4f>;
+		clocks = <&rcar_sound 1>, <&x19_clk>; /* audio_clkout_1, x19 */
+		clock-names = "clk_in", "ref_clk";
+
+		assigned-clocks = <&cs2000>;
+		assigned-clock-rates = <24576000>; /* 1/1 divide */
+	};
 };
 
 &i2c1 {
@@ -391,6 +443,46 @@ &ohci0 {
 	status = "okay";
 };
 
+&audio_clk_a {
+	/* same as cs2000 */
+	clock-frequency = <24576000>;
+};
+
+&audio_clk_b {
+	clock-frequency = <22579200>;
+};
+
+&rcar_sound {
+	pinctrl-0 = <&sound_pins>, <&sound_clk_pins>;
+	pinctrl-names = "default";
+
+	/* Single DAI */
+	#sound-dai-cells = <0>;
+
+	/* audio_clkout0/1 */
+	#clock-cells = <1>;
+	clock-frequency = <12288000 11289600>;
+
+	status = "okay";
+
+	ports {
+		rsnd_port0: port {
+			rsnd_for_ak4613: endpoint {
+				remote-endpoint = <&ak4613_endpoint>;
+				dai-format = "left_j";
+				bitclock-master = <&rsnd_for_ak4613>;
+				frame-master = <&rsnd_for_ak4613>;
+				playback = <&ssi3>, <&src5>, <&dvc0>;
+				capture  = <&ssi4>, <&src6>, <&dvc1>;
+			};
+		};
+	};
+};
+
+&ssi4 {
+	shared-pin;
+};
+
 &pfc {
 	avb0_pins: avb {
 		groups = "avb0_link", "avb0_mdio", "avb0_mii";
@@ -449,6 +541,17 @@ sdhi2_pins_uhs: sd2_uhs {
 		power-source = <1800>;
 	};
 
+	sound_pins: sound {
+		groups = "ssi34_ctrl", "ssi3_data", "ssi4_data_a";
+		function = "ssi";
+	};
+
+	sound_clk_pins: sound-clk {
+		groups = "audio_clk_a", "audio_clk_b",
+			 "audio_clkout", "audio_clkout1";
+		function = "audio_clk";
+	};
+
 	usb0_pins: usb0 {
 		groups = "usb0";
 		function = "usb0";
-- 
2.25.1

Re: [PATCH 3/3] arm64: dts: renesas: r8a77995: draak: Add R-Car Sound support

[Geert Uytterhoeven]

Hi Morimoto-san,

On Mon, May 17, 2021 at 2:37 AM Kuninori Morimoto
<kuninori.morimoto.gx@renesas.com> wrote:
> From: Kuninori Morimoto <kuninori.morimoto.gx@renesas.com>
>
> This patch adds R-Car Sound support for D3 draak.

Thanks for your patch!

> One note is that it is using different clock definition style
> from ulcb/salvator boards to avoid verbose clocks settings
> on rcar_sound node (see ulcb.dtsi  rcar_sound::clocks).
>
> cs2000 and ADG are closs connected, and needs each other.

cross connected?
need

> ulcb/salvator boards assume drivers are probed cs2000 -> rcar_sound.
> This draak board assumes drivers are probed rcar_sound -> cs2000.
>
> Signed-off-by: Kuninori Morimoto <kuninori.morimoto.gx@renesas.com>
> ---
>  .../arm64/boot/dts/renesas/r8a77995-draak.dts | 103 ++++++++++++++++++
>  1 file changed, 103 insertions(+)
>
> diff --git a/arch/arm64/boot/dts/renesas/r8a77995-draak.dts b/arch/arm64/boot/dts/renesas/r8a77995-draak.dts
> index 6783c3ad0856..591fad289802 100644
> --- a/arch/arm64/boot/dts/renesas/r8a77995-draak.dts
> +++ b/arch/arm64/boot/dts/renesas/r8a77995-draak.dts
> @@ -6,6 +6,11 @@
>   * Copyright (C) 2017 Glider bvba
>   */
>
> +/*
> + * This assumes...
> + *     SW60 : 2-1

Perhaps this comment should be extended (why is this SW60
setting needed?), and moved to the audio block below?

> + */
> +
>  /dts-v1/;
>  #include "r8a77995.dtsi"

> @@ -347,6 +366,39 @@ eeprom@50 {
>                 reg = <0x50>;
>                 pagesize = <8>;
>         };
> +
> +       ak4613: codec@10 {
> +               compatible = "asahi-kasei,ak4613";
> +               #sound-dai-cells = <0>;
> +               reg = <0x10>;
> +               clocks = <&rcar_sound 0>; /* audio_clkout */
> +
> +               asahi-kasei,in1-single-end;
> +               asahi-kasei,in2-single-end;
> +               asahi-kasei,out1-single-end;
> +               asahi-kasei,out2-single-end;
> +               asahi-kasei,out3-single-end;
> +               asahi-kasei,out4-single-end;
> +               asahi-kasei,out5-single-end;
> +               asahi-kasei,out6-single-end;
> +
> +               port {
> +                       ak4613_endpoint: endpoint {
> +                               remote-endpoint = <&rsnd_for_ak4613>;
> +                       };
> +               };

The "port" node seems to be missing from the ak4613 DT bindings:

    arch/arm64/boot/dts/renesas/r8a77995-draak.dt.yaml: codec@10:
'port' does not match any of the regexes:
'^asahi-kasei,in[1-2]-single-end$',
'^asahi-kasei,out[1-6]-single-end$', 'pinctrl-[0-9]+'
    From schema: Documentation/devicetree/bindings/sound/ak4613.yaml

> +&rcar_sound {
> +       pinctrl-0 = <&sound_pins>, <&sound_clk_pins>;
> +       pinctrl-names = "default";
> +
> +       /* Single DAI */
> +       #sound-dai-cells = <0>;

arch/arm64/boot/dts/renesas/r8a77995-draak.dt.yaml: sound@ec500000:
'dais' is a required property
        From schema: Documentation/devicetree/bindings/sound/renesas,rsnd.yaml

As this error is reported for all Renesas boards, perhaps the bindings
should be fixed instead?

> +
> +       /* audio_clkout0/1 */
> +       #clock-cells = <1>;
> +       clock-frequency = <12288000 11289600>;
> +
> +       status = "okay";
> +
> +       ports {
> +               rsnd_port0: port {
> +                       rsnd_for_ak4613: endpoint {
> +                               remote-endpoint = <&ak4613_endpoint>;
> +                               dai-format = "left_j";
> +                               bitclock-master = <&rsnd_for_ak4613>;
> +                               frame-master = <&rsnd_for_ak4613>;
> +                               playback = <&ssi3>, <&src5>, <&dvc0>;
> +                               capture  = <&ssi4>, <&src6>, <&dvc1>;
> +                       };
> +               };
> +       };

arch/arm64/boot/dts/renesas/r8a77995-draak.dt.yaml: sound@ec500000:
'ports' does not match any of the regexes: '^rcar_sound,ctu$',
'^rcar_sound,dai$', '^rcar_sound,dvc$', '^rcar_sound,mix$',
'^rcar_sound,src$', '^rcar_sound,ssi$', '^rcar_sound,ssiu$',
'pinctrl-[0-9]+'
        From schema: Documentation/devicetree/bindings/sound/renesas,rsnd.yaml

> +};
> +
> +&ssi4 {
> +       shared-pin;
> +};
> +
>  &pfc {
>         avb0_pins: avb {
>                 groups = "avb0_link", "avb0_mdio", "avb0_mii";
> @@ -449,6 +541,17 @@ sdhi2_pins_uhs: sd2_uhs {
>                 power-source = <1800>;
>         };
>
> +       sound_pins: sound {
> +               groups = "ssi34_ctrl", "ssi3_data", "ssi4_data_a";
> +               function = "ssi";
> +       };
> +
> +       sound_clk_pins: sound-clk {
> +               groups = "audio_clk_a", "audio_clk_b",
> +                        "audio_clkout", "audio_clkout1";
> +               function = "audio_clk";
> +       };
> +

Pin control looks good to me.

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