Am Dienstag, 6. Oktober 2020, 11:51:58 CET schrieb Johan Jonker: > 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?
I think so ... the connector always is just like n-pins on a board, only "sometimes" someone solders a fancy jack to it ;-) Heiko > > 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: > > 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 <[email protected]> > > > > --- > > > > 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. > > > + }; > > + }; > > + }; > > + > > 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"; > > > > > >
