lkml.org 
[lkml]   [2020]   [Nov]   [30]   [last100]   RSS Feed
Views: [wrap][no wrap]   [headers]  [forward] 
 
Messages in this thread
/
From
SubjectRe: [PATCH v3] arm64: dts: rockchip: add SPDIF node for rk3399-rockpro64
Date
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 <katsuhiro@katsuster.net>
> >
> > ---
> >
> > 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";
> >
> >
>
>




\
 
 \ /
  Last update: 2020-11-30 14:38    [W:1.149 / U:0.000 seconds]
©2003-2020 Jasper Spaans|hosted at Digital Ocean and TransIP|Read the blog|Advertise on this site