Re: [PATCH v3] arm64: dts: rockchip: add SPDIF node for rk3399-rockpro64
From: Johan Jonker <hidden>
Date: 2020-10-06 09:52:04
Also in:
linux-rockchip, lkml
Hi Katsuhiro, Heiko, Question for the maintainer: Should we add a SPDIF node if the connector is not physical on a board, only a header? Thanks Katsuhiro for the "aplay -l" screen print. **** List of PLAYBACK Hardware Devices **** card 0: hdmisound [hdmi-sound], device 0: ff8a0000.i2s-i2s-hifi i2s-hifi-0 [ff8a0000.i2s-i2s-hifi i2s-hifi-0] Subdevices: 1/1 Subdevice #0: subdevice #0 card 1: rockchiprk3399 [rockchip,rk3399], device 0: ff890000.i2s-ES8316 HiFi ES8316 HiFi-0 [ff890000.i2s-ES8316 HiFi ES8316 HiFi-0] Subdevices: 1/1 Subdevice #0: subdevice #0 card 2: rockchiprk339_1 [rockchip,rk3399], device 0: ff870000.spdif-dit-hifi dit-hifi-0 [ff870000.spdif-dit-hifi dit-hifi-0] Subdevices: 1/1 Subdevice #0: subdevice #0 On 10/5/20 4:03 PM, Katsuhiro Suzuki wrote:
quoted hunk ↗ jump to hunk
This patch adds 'disabled' SPDIF sound node and related settings of SPDIF for rk3399-rockpro64. RockPro64 has output pins for SPDIF Tx. But RK3399 does not have enough DMA channel for enabling SPDIF tx. Current settings are: - I2S0 (Req num 0, 1): Enabled : Output to 40pins header CON40 - I2S1 (Req num 2, 3): Enabled : Output to ES8316 on board - I2S2 (Req num 4, 5): Enabled : Output to internal HDMI core - SPDIF Tx (Req num 7) : Disabled: Output to connector J10 If users want to enable ALL sound I/Os, we need 7 DMA channels for it. But unfortunately, RK3399 has only 6 DMA channels. So users have to choose from the following: - Disable one of I2S (Ex. I2S0) and enable SPDIF tx - Keep enable I2S0/1/2 and disable SPDIF tx Signed-off-by: Katsuhiro Suzuki <redacted> --- Changes in v3: - Refine commit description why adding disabled node Changes in v2: - Remove redundant status property --- .../boot/dts/rockchip/rk3399-rockpro64.dtsi | 27 +++++++++++++++++++ 1 file changed, 27 insertions(+)diff --git a/arch/arm64/boot/dts/rockchip/rk3399-rockpro64.dtsi b/arch/arm64/boot/dts/rockchip/rk3399-rockpro64.dtsi index 6e553ff47534..58097245994a 100644 --- a/arch/arm64/boot/dts/rockchip/rk3399-rockpro64.dtsi +++ b/arch/arm64/boot/dts/rockchip/rk3399-rockpro64.dtsi@@ -76,6 +76,23 @@ sound { dais = <&i2s1_p0>; };
hdmi_sound: hdmi-sound {
compatible = "simple-audio-card";
simple-audio-card,name = "hdmi-sound";
Maybe rename to "HDMI"?
[..]
};
sound {
compatible = "audio-graph-card";
label = "rockchip,rk3399";
Maybe change this to "ES8316" to prevent confusion?
dais = <&i2s1_p0>;
};
+ sound-dit {
+ compatible = "audio-graph-card"
+ label = "rockchip,rk3399";This would be the second sound card with the same label. It seems that aplay/linux? adds "-1" to it and removes the comma, 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: HDMI ES8316 SPDIF
+ dais = <&spdif_p0>;
Maybe disable too? The "sound-dit" node is standard enabled and will start some process with a dia in a node that is disabled.
+ };
+
+ spdif-dit {
+ compatible = "linux,spdif-dit";
+ #sound-dai-cells = <0>;Maybe disable too?
+
+ port {
+ dit_p0_0: endpoint {+ remote-endpoint = <&spdif_p0_0>;
This also points to something that's disabled.
quoted hunk ↗ jump to hunk
+ }; + }; + }; + vcc12v_dcin: vcc12v-dcin { compatible = "regulator-fixed"; regulator-name = "vcc12v_dcin";@@ -698,6 +715,16 @@ &sdhci { status = "okay"; }; +&spdif { + pinctrl-0 = <&spdif_bus_1>;
This node is disabled.
+
+ spdif_p0: port {
+ spdif_p0_0: endpoint {
+ remote-endpoint = <&dit_p0_0>;
+ };
+ };
+};
+
&spi1 {
status = "okay";
_______________________________________________ linux-arm-kernel mailing list linux-arm-kernel@lists.infradead.org http://lists.infradead.org/mailman/listinfo/linux-arm-kernel