[1/3] arm64: dts: rockchip: add ES8316 codec for Rock Pi4

Message ID 20210617044955.598994-1-knaerzche@gmail.com
State New
Headers show
Series
  • [1/3] arm64: dts: rockchip: add ES8316 codec for Rock Pi4
Related show

Commit Message

Alex Bee June 17, 2021, 4:49 a.m.
Rock Pi4 boards have the codec connected to i2s0 and it is accessible
via i2c1 address 0x11.
Add an audio-graph-card it.

Signed-off-by: Alex Bee <knaerzche@gmail.com>
---
 .../boot/dts/rockchip/rk3399-rock-pi-4.dtsi   | 28 +++++++++++++++++++
 1 file changed, 28 insertions(+)

Comments

Johan Jonker June 18, 2021, 9:54 a.m. | #1
Hi Alex,

On 6/17/21 6:49 AM, Alex Bee wrote:
> Rock Pi4 boards have the codec connected to i2s0 and it is accessible

> via i2c1 address 0x11.

> Add an audio-graph-card it.

> 

> Signed-off-by: Alex Bee <knaerzche@gmail.com>

> ---

>  .../boot/dts/rockchip/rk3399-rock-pi-4.dtsi   | 28 +++++++++++++++++++

>  1 file changed, 28 insertions(+)

> 

> diff --git a/arch/arm64/boot/dts/rockchip/rk3399-rock-pi-4.dtsi b/arch/arm64/boot/dts/rockchip/rk3399-rock-pi-4.dtsi

> index 7d0a7c697703..e5c1083174ff 100644

> --- a/arch/arm64/boot/dts/rockchip/rk3399-rock-pi-4.dtsi

> +++ b/arch/arm64/boot/dts/rockchip/rk3399-rock-pi-4.dtsi

> @@ -36,6 +36,12 @@ sdio_pwrseq: sdio-pwrseq {

>  		reset-gpios = <&gpio0 RK_PB2 GPIO_ACTIVE_LOW>;

>  	};

>  

> +	sound {

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


> +		label = "rockchip,rk3399";


See previous discussion:

https://lore.kernel.org/linux-rockchip/e5ab2c62-ad00-4cdf-8b0a-24fda59c980b@gmail.com/

It seems that aplay/linux? adds "-1" to it and removes the comma and
"-", so we get:

hdmisound
rockchiprk3399
rockchiprk339_1

Shouldn't we label it with something that reflect the function/output.
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?

Proposal:

Analog
HDMI
ES8316 <---
SPDIF


Possible example solutions:

[PATCH] arm64: dts: rockchip: more user friendly name of sound nodes
https://lore.kernel.org/lkml/20210110151913.3615326-1-katsuhiro@katsuster.net/

===

Johan

> +		dais = <&i2s0_p0>;

> +	};

> +

>  	vcc12v_dcin: dc-12v {

>  		compatible = "regulator-fixed";

>  		regulator-name = "vcc12v_dcin";

> @@ -422,6 +428,20 @@ &i2c1 {

>  	i2c-scl-rising-time-ns = <300>;

>  	i2c-scl-falling-time-ns = <15>;

>  	status = "okay";

> +

> +	es8316: codec@11 {

> +		compatible = "everest,es8316";

> +		reg = <0x11>;

> +		clocks = <&cru SCLK_I2S_8CH_OUT>;

> +		clock-names = "mclk";

> +		#sound-dai-cells = <0>;

> +

> +		port {

> +			es8316_p0_0: endpoint {

> +				remote-endpoint = <&i2s0_p0_0>;

> +			};

> +		};

> +	};

>  };

>  

>  &i2c3 {

> @@ -441,6 +461,14 @@ &i2s0 {

>  	rockchip,capture-channels = <2>;

>  	rockchip,playback-channels = <2>;

>  	status = "okay";

> +

> +	i2s0_p0: port {

> +		i2s0_p0_0: endpoint {

> +			dai-format = "i2s";

> +			mclk-fs = <256>;

> +			remote-endpoint = <&es8316_p0_0>;

> +		};

> +	};

>  };

>  

>  &i2s1 {

>
Heiko Stuebner June 18, 2021, 1:08 p.m. | #2
Am Donnerstag, 17. Juni 2021, 06:49:54 CEST schrieb Alex Bee:
> Rock Pi 4a plus board is the successor of Rock Pi 4a board.
> 
> Differences to the original version are
> - has RK3399 OP1 SoC revision
> - has eMMC (16 or 32 GB) soldered on board (no changes required,
>   since it is enabled in rk3399-rock-pi-4.dtsi)
> - dev boards have SPI flash soldered, but as per manufacturer response,
>   this won't be the case for mass production boards
> 
> I didn't add yet another compatible, since the small set of differences
> are captured by the device tree.
> 
> Signed-off-by: Alex Bee <knaerzche@gmail.com>
> ---
>  arch/arm64/boot/dts/rockchip/Makefile              |  1 +
>  .../boot/dts/rockchip/rk3399-rock-pi-4a-plus.dts   | 14 ++++++++++++++
>  2 files changed, 15 insertions(+)
>  create mode 100644 arch/arm64/boot/dts/rockchip/rk3399-rock-pi-4a-plus.dts
> 
> diff --git a/arch/arm64/boot/dts/rockchip/Makefile b/arch/arm64/boot/dts/rockchip/Makefile
> index c3e00c0e2db7..dbd7d37950f1 100644
> --- a/arch/arm64/boot/dts/rockchip/Makefile
> +++ b/arch/arm64/boot/dts/rockchip/Makefile
> @@ -43,6 +43,7 @@ dtb-$(CONFIG_ARCH_ROCKCHIP) += rk3399-puma-haikou.dtb
>  dtb-$(CONFIG_ARCH_ROCKCHIP) += rk3399-roc-pc.dtb
>  dtb-$(CONFIG_ARCH_ROCKCHIP) += rk3399-roc-pc-mezzanine.dtb
>  dtb-$(CONFIG_ARCH_ROCKCHIP) += rk3399-rock-pi-4a.dtb
> +dtb-$(CONFIG_ARCH_ROCKCHIP) += rk3399-rock-pi-4a-plus.dtb
>  dtb-$(CONFIG_ARCH_ROCKCHIP) += rk3399-rock-pi-4b.dtb
>  dtb-$(CONFIG_ARCH_ROCKCHIP) += rk3399-rock-pi-4c.dtb
>  dtb-$(CONFIG_ARCH_ROCKCHIP) += rk3399-rock960.dtb
> diff --git a/arch/arm64/boot/dts/rockchip/rk3399-rock-pi-4a-plus.dts b/arch/arm64/boot/dts/rockchip/rk3399-rock-pi-4a-plus.dts
> new file mode 100644
> index 000000000000..2deaab7f9307
> --- /dev/null
> +++ b/arch/arm64/boot/dts/rockchip/rk3399-rock-pi-4a-plus.dts
> @@ -0,0 +1,14 @@
> +// SPDX-License-Identifier: (GPL-2.0+ OR MIT)
> +/*
> + * Copyright (c) 2019 Akash Gajjar <Akash_Gajjar@mentor.com>
> + * Copyright (c) 2019 Pragnesh Patel <Pragnesh_Patel@mentor.com>
> + */
> +
> +/dts-v1/;
> +#include "rk3399-rock-pi-4.dtsi"
> +#include "rk3399-op1-opp.dtsi"
> +
> +/ {
> +	model = "Radxa ROCK Pi 4A plus";
> +	compatible = "radxa,rockpi4a", "radxa,rockpi4", "rockchip,rk3399";

hmm, I don't really follow why you're re-using the radxa,rockpi4a
compatible. I'd assume this should be radxa,rockpi4a+ or something?

I.e. if a bootloader needs to select the matching devicetree from a list
of available devicetrees, this could end up running a regular rockpi4a
(without +) using the OP1 operating points and thus at way too high
frequencies.


Heiko


> +};
>
Alex Bee June 18, 2021, 4:14 p.m. | #3
Hi Johan,

Am 18.06.21 um 11:54 schrieb Johan Jonker:
> Hi Alex,

>

> On 6/17/21 6:49 AM, Alex Bee wrote:

>> Rock Pi4 boards have the codec connected to i2s0 and it is accessible

>> via i2c1 address 0x11.

>> Add an audio-graph-card it.

>>

>> Signed-off-by: Alex Bee <knaerzche@gmail.com>

>> ---

>>   .../boot/dts/rockchip/rk3399-rock-pi-4.dtsi   | 28 +++++++++++++++++++

>>   1 file changed, 28 insertions(+)

>>

>> diff --git a/arch/arm64/boot/dts/rockchip/rk3399-rock-pi-4.dtsi b/arch/arm64/boot/dts/rockchip/rk3399-rock-pi-4.dtsi

>> index 7d0a7c697703..e5c1083174ff 100644

>> --- a/arch/arm64/boot/dts/rockchip/rk3399-rock-pi-4.dtsi

>> +++ b/arch/arm64/boot/dts/rockchip/rk3399-rock-pi-4.dtsi

>> @@ -36,6 +36,12 @@ sdio_pwrseq: sdio-pwrseq {

>>   		reset-gpios = <&gpio0 RK_PB2 GPIO_ACTIVE_LOW>;

>>   	};

>>   

>> +	sound {

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

>> +		label = "rockchip,rk3399";

> See previous discussion:

>

> https://lore.kernel.org/linux-rockchip/e5ab2c62-ad00-4cdf-8b0a-24fda59c980b@gmail.com/

>

> It seems that aplay/linux? adds "-1" to it and removes the comma and

> "-", so we get:

>

> hdmisound

> rockchiprk3399

> rockchiprk339_1

>

> Shouldn't we label it with something that reflect the function/output.

> 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?

>

> Proposal:

>

> Analog

> HDMI

> ES8316 <---

> SPDIF


OK - thanks for that, I wasn't aware.

I'll go for "Analog", since that seems to be the accepted solution for 
RockPro64 board and I think we should keep it the same across boards (on 
distro side it can get annoying if you need a couple of alsa configs 
with the same contents, just because audio card names are different).

Alex.

>

>

> Possible example solutions:

>

> [PATCH] arm64: dts: rockchip: more user friendly name of sound nodes

> https://lore.kernel.org/lkml/20210110151913.3615326-1-katsuhiro@katsuster.net/

>

> ===

>

> Johan

>

>> +		dais = <&i2s0_p0>;

>> +	};

>> +

>>   	vcc12v_dcin: dc-12v {

>>   		compatible = "regulator-fixed";

>>   		regulator-name = "vcc12v_dcin";

>> @@ -422,6 +428,20 @@ &i2c1 {

>>   	i2c-scl-rising-time-ns = <300>;

>>   	i2c-scl-falling-time-ns = <15>;

>>   	status = "okay";

>> +

>> +	es8316: codec@11 {

>> +		compatible = "everest,es8316";

>> +		reg = <0x11>;

>> +		clocks = <&cru SCLK_I2S_8CH_OUT>;

>> +		clock-names = "mclk";

>> +		#sound-dai-cells = <0>;

>> +

>> +		port {

>> +			es8316_p0_0: endpoint {

>> +				remote-endpoint = <&i2s0_p0_0>;

>> +			};

>> +		};

>> +	};

>>   };

>>   

>>   &i2c3 {

>> @@ -441,6 +461,14 @@ &i2s0 {

>>   	rockchip,capture-channels = <2>;

>>   	rockchip,playback-channels = <2>;

>>   	status = "okay";

>> +

>> +	i2s0_p0: port {

>> +		i2s0_p0_0: endpoint {

>> +			dai-format = "i2s";

>> +			mclk-fs = <256>;

>> +			remote-endpoint = <&es8316_p0_0>;

>> +		};

>> +	};

>>   };

>>   

>>   &i2s1 {

>>
Alex Bee June 18, 2021, 4:30 p.m. | #4
Hi Heiko,

Am 18.06.21 um 15:08 schrieb Heiko Stübner:
> Am Donnerstag, 17. Juni 2021, 06:49:54 CEST schrieb Alex Bee:

>> Rock Pi 4a plus board is the successor of Rock Pi 4a board.

>>

>> Differences to the original version are

>> - has RK3399 OP1 SoC revision

>> - has eMMC (16 or 32 GB) soldered on board (no changes required,

>>    since it is enabled in rk3399-rock-pi-4.dtsi)

>> - dev boards have SPI flash soldered, but as per manufacturer response,

>>    this won't be the case for mass production boards

>>

>> I didn't add yet another compatible, since the small set of differences

>> are captured by the device tree.

>>

>> Signed-off-by: Alex Bee <knaerzche@gmail.com>

>> ---

>>   arch/arm64/boot/dts/rockchip/Makefile              |  1 +

>>   .../boot/dts/rockchip/rk3399-rock-pi-4a-plus.dts   | 14 ++++++++++++++

>>   2 files changed, 15 insertions(+)

>>   create mode 100644 arch/arm64/boot/dts/rockchip/rk3399-rock-pi-4a-plus.dts

>>

>> diff --git a/arch/arm64/boot/dts/rockchip/Makefile b/arch/arm64/boot/dts/rockchip/Makefile

>> index c3e00c0e2db7..dbd7d37950f1 100644

>> --- a/arch/arm64/boot/dts/rockchip/Makefile

>> +++ b/arch/arm64/boot/dts/rockchip/Makefile

>> @@ -43,6 +43,7 @@ dtb-$(CONFIG_ARCH_ROCKCHIP) += rk3399-puma-haikou.dtb

>>   dtb-$(CONFIG_ARCH_ROCKCHIP) += rk3399-roc-pc.dtb

>>   dtb-$(CONFIG_ARCH_ROCKCHIP) += rk3399-roc-pc-mezzanine.dtb

>>   dtb-$(CONFIG_ARCH_ROCKCHIP) += rk3399-rock-pi-4a.dtb

>> +dtb-$(CONFIG_ARCH_ROCKCHIP) += rk3399-rock-pi-4a-plus.dtb

>>   dtb-$(CONFIG_ARCH_ROCKCHIP) += rk3399-rock-pi-4b.dtb

>>   dtb-$(CONFIG_ARCH_ROCKCHIP) += rk3399-rock-pi-4c.dtb

>>   dtb-$(CONFIG_ARCH_ROCKCHIP) += rk3399-rock960.dtb

>> diff --git a/arch/arm64/boot/dts/rockchip/rk3399-rock-pi-4a-plus.dts b/arch/arm64/boot/dts/rockchip/rk3399-rock-pi-4a-plus.dts

>> new file mode 100644

>> index 000000000000..2deaab7f9307

>> --- /dev/null

>> +++ b/arch/arm64/boot/dts/rockchip/rk3399-rock-pi-4a-plus.dts

>> @@ -0,0 +1,14 @@

>> +// SPDX-License-Identifier: (GPL-2.0+ OR MIT)

>> +/*

>> + * Copyright (c) 2019 Akash Gajjar <Akash_Gajjar@mentor.com>

>> + * Copyright (c) 2019 Pragnesh Patel <Pragnesh_Patel@mentor.com>

>> + */

>> +

>> +/dts-v1/;

>> +#include "rk3399-rock-pi-4.dtsi"

>> +#include "rk3399-op1-opp.dtsi"

>> +

>> +/ {

>> +	model = "Radxa ROCK Pi 4A plus";

>> +	compatible = "radxa,rockpi4a", "radxa,rockpi4", "rockchip,rk3399";

> hmm, I don't really follow why you're re-using the radxa,rockpi4a

> compatible. I'd assume this should be radxa,rockpi4a+ or something?


Ah, yes this was part of my cover letter, which obviously got lost 
somewhere.

Anyways: Reason I thought of was: For example broadcom nvram file names 
must match the compatible string and they have to be copied/symlinked 
over and over if we add new compatibles for every minor changed revision 
of a board. I guess there are more examples for that in userland.

>

> I.e. if a bootloader needs to select the matching devicetree from a list

> of available devicetrees, this could end up running a regular rockpi4a

> (without +) using the OP1 operating points and thus at way too high

> frequencies.


Besides I wasn't aware, that "a bootloader" can do that already I 
understand your concerns and will change it.

Alex.

>

> Heiko

>

>

>> +};

>>

>

>

>

Patch

diff --git a/arch/arm64/boot/dts/rockchip/rk3399-rock-pi-4.dtsi b/arch/arm64/boot/dts/rockchip/rk3399-rock-pi-4.dtsi
index 7d0a7c697703..e5c1083174ff 100644
--- a/arch/arm64/boot/dts/rockchip/rk3399-rock-pi-4.dtsi
+++ b/arch/arm64/boot/dts/rockchip/rk3399-rock-pi-4.dtsi
@@ -36,6 +36,12 @@  sdio_pwrseq: sdio-pwrseq {
 		reset-gpios = <&gpio0 RK_PB2 GPIO_ACTIVE_LOW>;
 	};
 
+	sound {
+		compatible = "audio-graph-card";
+		label = "rockchip,rk3399";
+		dais = <&i2s0_p0>;
+	};
+
 	vcc12v_dcin: dc-12v {
 		compatible = "regulator-fixed";
 		regulator-name = "vcc12v_dcin";
@@ -422,6 +428,20 @@  &i2c1 {
 	i2c-scl-rising-time-ns = <300>;
 	i2c-scl-falling-time-ns = <15>;
 	status = "okay";
+
+	es8316: codec@11 {
+		compatible = "everest,es8316";
+		reg = <0x11>;
+		clocks = <&cru SCLK_I2S_8CH_OUT>;
+		clock-names = "mclk";
+		#sound-dai-cells = <0>;
+
+		port {
+			es8316_p0_0: endpoint {
+				remote-endpoint = <&i2s0_p0_0>;
+			};
+		};
+	};
 };
 
 &i2c3 {
@@ -441,6 +461,14 @@  &i2s0 {
 	rockchip,capture-channels = <2>;
 	rockchip,playback-channels = <2>;
 	status = "okay";
+
+	i2s0_p0: port {
+		i2s0_p0_0: endpoint {
+			dai-format = "i2s";
+			mclk-fs = <256>;
+			remote-endpoint = <&es8316_p0_0>;
+		};
+	};
 };
 
 &i2s1 {