diff mbox series

[1/2] arm64: dts: rockchip: rk356x: Add HDMI audio nodes

Message ID 20211125100836.423808-2-frattaroli.nicolas@gmail.com (mailing list archive)
State New, archived
Headers show
Series HDMI Audio on RK356x/Quartz64 Model A | expand

Commit Message

Nicolas Frattaroli Nov. 25, 2021, 10:08 a.m. UTC
This adds the i2s0 node and an hdmi-sound sound device to the
rk356x device tree. On the rk356[68], the i2s0 controller is
connected to HDMI audio.

Signed-off-by: Nicolas Frattaroli <frattaroli.nicolas@gmail.com>
---
 arch/arm64/boot/dts/rockchip/rk356x.dtsi | 32 ++++++++++++++++++++++++
 1 file changed, 32 insertions(+)

Comments

Michael Riesch Nov. 25, 2021, 6:47 p.m. UTC | #1
Hello Nicolas,

On 11/25/21 11:08 AM, Nicolas Frattaroli wrote:
> This adds the i2s0 node and an hdmi-sound sound device to the
> rk356x device tree. On the rk356[68], the i2s0 controller is
> connected to HDMI audio.
> 
> Signed-off-by: Nicolas Frattaroli <frattaroli.nicolas@gmail.com>
> ---
>   arch/arm64/boot/dts/rockchip/rk356x.dtsi | 32 ++++++++++++++++++++++++
>   1 file changed, 32 insertions(+)
> 
> diff --git a/arch/arm64/boot/dts/rockchip/rk356x.dtsi b/arch/arm64/boot/dts/rockchip/rk356x.dtsi
> index 3c09cf6d4c37..ad4053402eef 100644
> --- a/arch/arm64/boot/dts/rockchip/rk356x.dtsi
> +++ b/arch/arm64/boot/dts/rockchip/rk356x.dtsi
> @@ -614,6 +614,21 @@ hdmi_in_vp2: endpoint@2 {
>   		};
>   	};
>   
> +	hdmi_sound: hdmi-sound {
> +		compatible = "simple-audio-card";
> +		simple-audio-card,format = "i2s";
> +		simple-audio-card,mclk-fs = <256>;
> +		simple-audio-card,name = "hdmi-sound";
> +		status = "disabled";
> +
> +		simple-audio-card,cpu {
> +			sound-dai = <&i2s0_8ch>;
> +		};
> +		simple-audio-card,codec {
> +			sound-dai = <&hdmi>;
> +		};
> +	};
> +
>   	qos_gpu: qos@fe128000 {
>   		compatible = "rockchip,rk3568-qos", "syscon";
>   		reg = <0x0 0xfe128000 0x0 0x20>;
> @@ -789,6 +804,23 @@ spdif: spdif@fe460000 {
>   		status = "disabled";
>   	};
>   
> +	i2s0_8ch: i2s@fe400000 {
> +		compatible = "rockchip,rk3568-i2s-tdm";
> +		reg = <0x0 0xfe400000 0x0 0x1000>;
> +		interrupts = <GIC_SPI 52 IRQ_TYPE_LEVEL_HIGH>;
> +		assigned-clocks = <&cru CLK_I2S0_8CH_TX_SRC>, <&cru CLK_I2S0_8CH_RX_SRC>;
> +		assigned-clock-rates = <1188000000>, <1188000000>;
> +		clocks = <&cru MCLK_I2S0_8CH_TX>, <&cru MCLK_I2S0_8CH_RX>, <&cru HCLK_I2S0_8CH>;
> +		clock-names = "mclk_tx", "mclk_rx", "hclk";
> +		dmas = <&dmac1 0>;
> +		dma-names = "tx";
> +		resets = <&cru SRST_M_I2S0_8CH_TX>, <&cru SRST_M_I2S0_8CH_RX>;
> +		reset-names = "tx-m", "rx-m";
> +		rockchip,grf = <&grf>;
> +		#sound-dai-cells = <0>;
> +		status = "disabled";
> +	};
> +
>   	i2s1_8ch: i2s@fe410000 {
>   		compatible = "rockchip,rk3568-i2s-tdm";
>   		reg = <0x0 0xfe410000 0x0 0x1000>;
> 

on a RK3568 EVB1 with corresponding dts changes:

Tested-by: Michael Riesch <michael.riesch@wolfvision.net>

Thanks and best regards,
Michael
Johan Jonker Nov. 25, 2021, 7:07 p.m. UTC | #2
Hi Nicolas,

Some comments...

On 11/25/21 11:08 AM, Nicolas Frattaroli wrote:
> This adds the i2s0 node and an hdmi-sound sound device to the
> rk356x device tree. On the rk356[68], the i2s0 controller is
> connected to HDMI audio.
> 
> Signed-off-by: Nicolas Frattaroli <frattaroli.nicolas@gmail.com>
> ---
>  arch/arm64/boot/dts/rockchip/rk356x.dtsi | 32 ++++++++++++++++++++++++
>  1 file changed, 32 insertions(+)
> 
> diff --git a/arch/arm64/boot/dts/rockchip/rk356x.dtsi b/arch/arm64/boot/dts/rockchip/rk356x.dtsi
> index 3c09cf6d4c37..ad4053402eef 100644
> --- a/arch/arm64/boot/dts/rockchip/rk356x.dtsi
> +++ b/arch/arm64/boot/dts/rockchip/rk356x.dtsi
> @@ -614,6 +614,21 @@ hdmi_in_vp2: endpoint@2 {
>  		};
>  	};
>  

> +	hdmi_sound: hdmi-sound {

Some DT sort rules:

For nodes:
Sort things without reg alphabetical first,
then sort the rest by reg address.

> +		compatible = "simple-audio-card";

simple-audio-card,name = "HDMI";

> +		simple-audio-card,format = "i2s";
> +		simple-audio-card,mclk-fs = <256>;

> +		simple-audio-card,name = "hdmi-sound";

Exceptions:
Sort simple-audio-card,name above other simple-audio-card properties.

Shouldn't we standardize to SPDIF, HDMI and Analog similar to rk3318/rk3328?
Make a shorter label without spaces or special chars, so that chars
don't get removed?
See "aplay -l" screen print.

Maybe rename to "HDMI"?

> +		status = "disabled";
> +
> +		simple-audio-card,cpu {
> +			sound-dai = <&i2s0_8ch>;
> +		};

Add empty line between nodes.

Not sure if Heiko cares, but when alphabetical sort I get this:
simple-audio-card,codec
simple-audio-card,cpu

> +		simple-audio-card,codec {
> +			sound-dai = <&hdmi>;
> +		};
> +	};
> +
>  	qos_gpu: qos@fe128000 {
>  		compatible = "rockchip,rk3568-qos", "syscon";
>  		reg = <0x0 0xfe128000 0x0 0x20>;
> @@ -789,6 +804,23 @@ spdif: spdif@fe460000 {
>  		status = "disabled";
>  	};
>  
> +	i2s0_8ch: i2s@fe400000 {
> +		compatible = "rockchip,rk3568-i2s-tdm";
> +		reg = <0x0 0xfe400000 0x0 0x1000>;
> +		interrupts = <GIC_SPI 52 IRQ_TYPE_LEVEL_HIGH>;
> +		assigned-clocks = <&cru CLK_I2S0_8CH_TX_SRC>, <&cru CLK_I2S0_8CH_RX_SRC>;
> +		assigned-clock-rates = <1188000000>, <1188000000>;
> +		clocks = <&cru MCLK_I2S0_8CH_TX>, <&cru MCLK_I2S0_8CH_RX>, <&cru HCLK_I2S0_8CH>;
> +		clock-names = "mclk_tx", "mclk_rx", "hclk";
> +		dmas = <&dmac1 0>;
> +		dma-names = "tx";
> +		resets = <&cru SRST_M_I2S0_8CH_TX>, <&cru SRST_M_I2S0_8CH_RX>;
> +		reset-names = "tx-m", "rx-m";
> +		rockchip,grf = <&grf>;
> +		#sound-dai-cells = <0>;
> +		status = "disabled";
> +	};
> +
>  	i2s1_8ch: i2s@fe410000 {
>  		compatible = "rockchip,rk3568-i2s-tdm";
>  		reg = <0x0 0xfe410000 0x0 0x1000>;
>
Heiko Stübner Nov. 25, 2021, 7:30 p.m. UTC | #3
Am Donnerstag, 25. November 2021, 20:07:21 CET schrieb Johan Jonker:
> Hi Nicolas,
> 
> Some comments...
> 
> On 11/25/21 11:08 AM, Nicolas Frattaroli wrote:
> > This adds the i2s0 node and an hdmi-sound sound device to the
> > rk356x device tree. On the rk356[68], the i2s0 controller is
> > connected to HDMI audio.
> > 
> > Signed-off-by: Nicolas Frattaroli <frattaroli.nicolas@gmail.com>
> > ---
> >  arch/arm64/boot/dts/rockchip/rk356x.dtsi | 32 ++++++++++++++++++++++++
> >  1 file changed, 32 insertions(+)
> > 
> > diff --git a/arch/arm64/boot/dts/rockchip/rk356x.dtsi b/arch/arm64/boot/dts/rockchip/rk356x.dtsi
> > index 3c09cf6d4c37..ad4053402eef 100644
> > --- a/arch/arm64/boot/dts/rockchip/rk356x.dtsi
> > +++ b/arch/arm64/boot/dts/rockchip/rk356x.dtsi
> > @@ -614,6 +614,21 @@ hdmi_in_vp2: endpoint@2 {
> >  		};
> >  	};
> >  
> 
> > +	hdmi_sound: hdmi-sound {
> 
> Some DT sort rules:
> 
> For nodes:
> Sort things without reg alphabetical first,
> then sort the rest by reg address.
> 
> > +		compatible = "simple-audio-card";
> 
> simple-audio-card,name = "HDMI";
> 
> > +		simple-audio-card,format = "i2s";
> > +		simple-audio-card,mclk-fs = <256>;
> 
> > +		simple-audio-card,name = "hdmi-sound";
> 
> Exceptions:
> Sort simple-audio-card,name above other simple-audio-card properties.
> 
> Shouldn't we standardize to SPDIF, HDMI and Analog similar to rk3318/rk3328?
> Make a shorter label without spaces or special chars, so that chars
> don't get removed?
> See "aplay -l" screen print.
> 
> Maybe rename to "HDMI"?
> 
> > +		status = "disabled";
> > +
> > +		simple-audio-card,cpu {
> > +			sound-dai = <&i2s0_8ch>;
> > +		};
> 
> Add empty line between nodes.
> 
> Not sure if Heiko cares, but when alphabetical sort I get this:
> simple-audio-card,codec
> simple-audio-card,cpu

Hehe ... I do care, but would normally just (silently) re-sort these
things when applying ;-) .


Heiko


> > +		simple-audio-card,codec {
> > +			sound-dai = <&hdmi>;
> > +		};
> > +	};
> > +
> >  	qos_gpu: qos@fe128000 {
> >  		compatible = "rockchip,rk3568-qos", "syscon";
> >  		reg = <0x0 0xfe128000 0x0 0x20>;
> > @@ -789,6 +804,23 @@ spdif: spdif@fe460000 {
> >  		status = "disabled";
> >  	};
> >  
> > +	i2s0_8ch: i2s@fe400000 {
> > +		compatible = "rockchip,rk3568-i2s-tdm";
> > +		reg = <0x0 0xfe400000 0x0 0x1000>;
> > +		interrupts = <GIC_SPI 52 IRQ_TYPE_LEVEL_HIGH>;
> > +		assigned-clocks = <&cru CLK_I2S0_8CH_TX_SRC>, <&cru CLK_I2S0_8CH_RX_SRC>;
> > +		assigned-clock-rates = <1188000000>, <1188000000>;
> > +		clocks = <&cru MCLK_I2S0_8CH_TX>, <&cru MCLK_I2S0_8CH_RX>, <&cru HCLK_I2S0_8CH>;
> > +		clock-names = "mclk_tx", "mclk_rx", "hclk";
> > +		dmas = <&dmac1 0>;
> > +		dma-names = "tx";
> > +		resets = <&cru SRST_M_I2S0_8CH_TX>, <&cru SRST_M_I2S0_8CH_RX>;
> > +		reset-names = "tx-m", "rx-m";
> > +		rockchip,grf = <&grf>;
> > +		#sound-dai-cells = <0>;
> > +		status = "disabled";
> > +	};
> > +
> >  	i2s1_8ch: i2s@fe410000 {
> >  		compatible = "rockchip,rk3568-i2s-tdm";
> >  		reg = <0x0 0xfe410000 0x0 0x1000>;
> > 
>
diff mbox series

Patch

diff --git a/arch/arm64/boot/dts/rockchip/rk356x.dtsi b/arch/arm64/boot/dts/rockchip/rk356x.dtsi
index 3c09cf6d4c37..ad4053402eef 100644
--- a/arch/arm64/boot/dts/rockchip/rk356x.dtsi
+++ b/arch/arm64/boot/dts/rockchip/rk356x.dtsi
@@ -614,6 +614,21 @@  hdmi_in_vp2: endpoint@2 {
 		};
 	};
 
+	hdmi_sound: hdmi-sound {
+		compatible = "simple-audio-card";
+		simple-audio-card,format = "i2s";
+		simple-audio-card,mclk-fs = <256>;
+		simple-audio-card,name = "hdmi-sound";
+		status = "disabled";
+
+		simple-audio-card,cpu {
+			sound-dai = <&i2s0_8ch>;
+		};
+		simple-audio-card,codec {
+			sound-dai = <&hdmi>;
+		};
+	};
+
 	qos_gpu: qos@fe128000 {
 		compatible = "rockchip,rk3568-qos", "syscon";
 		reg = <0x0 0xfe128000 0x0 0x20>;
@@ -789,6 +804,23 @@  spdif: spdif@fe460000 {
 		status = "disabled";
 	};
 
+	i2s0_8ch: i2s@fe400000 {
+		compatible = "rockchip,rk3568-i2s-tdm";
+		reg = <0x0 0xfe400000 0x0 0x1000>;
+		interrupts = <GIC_SPI 52 IRQ_TYPE_LEVEL_HIGH>;
+		assigned-clocks = <&cru CLK_I2S0_8CH_TX_SRC>, <&cru CLK_I2S0_8CH_RX_SRC>;
+		assigned-clock-rates = <1188000000>, <1188000000>;
+		clocks = <&cru MCLK_I2S0_8CH_TX>, <&cru MCLK_I2S0_8CH_RX>, <&cru HCLK_I2S0_8CH>;
+		clock-names = "mclk_tx", "mclk_rx", "hclk";
+		dmas = <&dmac1 0>;
+		dma-names = "tx";
+		resets = <&cru SRST_M_I2S0_8CH_TX>, <&cru SRST_M_I2S0_8CH_RX>;
+		reset-names = "tx-m", "rx-m";
+		rockchip,grf = <&grf>;
+		#sound-dai-cells = <0>;
+		status = "disabled";
+	};
+
 	i2s1_8ch: i2s@fe410000 {
 		compatible = "rockchip,rk3568-i2s-tdm";
 		reg = <0x0 0xfe410000 0x0 0x1000>;