| Message ID | 20210617044955.598994-1-knaerzche@gmail.com |
|---|---|
| State | New |
| Headers | show |
| Series | [1/3] arm64: dts: rockchip: add ES8316 codec for Rock Pi4 | expand |
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 { >
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 > +}; >
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 { >>
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 > > >> +}; >> > > >
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 {
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(+)