From: "Heiko Stübner" <heiko@sntech.de>
To: Katsuhiro Suzuki <katsuhiro@katsuster.net>,
linux-rockchip@lists.infradead.org,
Johan Jonker <jbx6244@gmail.com>
Cc: linux-kernel@vger.kernel.org, linux-arm-kernel@lists.infradead.org
Subject: Re: [PATCH v3] arm64: dts: rockchip: add SPDIF node for rk3399-rockpro64
Date: Mon, 30 Nov 2020 14:36:12 +0100 [thread overview]
Message-ID: <2753094.YVpnkQH3Kb@diego> (raw)
In-Reply-To: <e5ab2c62-ad00-4cdf-8b0a-24fda59c980b@gmail.com>
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";
> >
> >
>
>
_______________________________________________
Linux-rockchip mailing list
Linux-rockchip@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-rockchip
prev parent reply other threads:[~2020-11-30 13:36 UTC|newest]
Thread overview: 4+ messages / expand[flat|nested] mbox.gz Atom feed top
2020-10-05 14:03 [PATCH v3] arm64: dts: rockchip: add SPDIF node for rk3399-rockpro64 Katsuhiro Suzuki
2020-10-06 9:51 ` Johan Jonker
2020-11-08 16:00 ` Katsuhiro Suzuki
2020-11-30 13:36 ` Heiko Stübner [this message]
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=2753094.YVpnkQH3Kb@diego \
--to=heiko@sntech.de \
--cc=jbx6244@gmail.com \
--cc=katsuhiro@katsuster.net \
--cc=linux-arm-kernel@lists.infradead.org \
--cc=linux-kernel@vger.kernel.org \
--cc=linux-rockchip@lists.infradead.org \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).