Am Donnerstag, 25. November 2021, 20:07:21 CET schrieb Johan Jonker:
> Hi Nicolas,
>
> Some comments...
>
> On 11/25/21 11:08 AM, Nicolas Frattaroli wrote:
> > This adds the i2s0 node and an hdmi-sound sound device to the
> > rk356x device tree. On the rk356[68], the i2s0 controller is
> > connected to HDMI audio.
> >
> > Signed-off-by: Nicolas Frattaroli <frattaroli.nicolas@xxxxxxxxx>
> > ---
> > arch/arm64/boot/dts/rockchip/rk356x.dtsi | 32 ++++++++++++++++++++++++
> > 1 file changed, 32 insertions(+)
> >
> > diff --git a/arch/arm64/boot/dts/rockchip/rk356x.dtsi b/arch/arm64/boot/dts/rockchip/rk356x.dtsi
> > index 3c09cf6d4c37..ad4053402eef 100644
> > --- a/arch/arm64/boot/dts/rockchip/rk356x.dtsi
> > +++ b/arch/arm64/boot/dts/rockchip/rk356x.dtsi
> > @@ -614,6 +614,21 @@ hdmi_in_vp2: endpoint@2 {
> > };
> > };
> >
>
> > + hdmi_sound: hdmi-sound {
>
> Some DT sort rules:
>
> For nodes:
> Sort things without reg alphabetical first,
> then sort the rest by reg address.
>
> > + compatible = "simple-audio-card";
>
> simple-audio-card,name = "HDMI";
>
> > + simple-audio-card,format = "i2s";
> > + simple-audio-card,mclk-fs = <256>;
>
> > + simple-audio-card,name = "hdmi-sound";
>
> Exceptions:
> Sort simple-audio-card,name above other simple-audio-card properties.
>
> 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?
> See "aplay -l" screen print.
>
> Maybe rename to "HDMI"?
>
> > + status = "disabled";
> > +
> > + simple-audio-card,cpu {
> > + sound-dai = <&i2s0_8ch>;
> > + };
>
> Add empty line between nodes.
>
> Not sure if Heiko cares, but when alphabetical sort I get this:
> simple-audio-card,codec
> simple-audio-card,cpu
Hehe ... I do care, but would normally just (silently) re-sort these
things when applying ;-) .
Heiko
> > + simple-audio-card,codec {
> > + sound-dai = <&hdmi>;
> > + };
> > + };
> > +
> > qos_gpu: qos@fe128000 {
> > compatible = "rockchip,rk3568-qos", "syscon";
> > reg = <0x0 0xfe128000 0x0 0x20>;
> > @@ -789,6 +804,23 @@ spdif: spdif@fe460000 {
> > status = "disabled";
> > };
> >
> > + i2s0_8ch: i2s@fe400000 {
> > + compatible = "rockchip,rk3568-i2s-tdm";
> > + reg = <0x0 0xfe400000 0x0 0x1000>;
> > + interrupts = <GIC_SPI 52 IRQ_TYPE_LEVEL_HIGH>;
> > + assigned-clocks = <&cru CLK_I2S0_8CH_TX_SRC>, <&cru CLK_I2S0_8CH_RX_SRC>;
> > + assigned-clock-rates = <1188000000>, <1188000000>;
> > + clocks = <&cru MCLK_I2S0_8CH_TX>, <&cru MCLK_I2S0_8CH_RX>, <&cru HCLK_I2S0_8CH>;
> > + clock-names = "mclk_tx", "mclk_rx", "hclk";
> > + dmas = <&dmac1 0>;
> > + dma-names = "tx";
> > + resets = <&cru SRST_M_I2S0_8CH_TX>, <&cru SRST_M_I2S0_8CH_RX>;
> > + reset-names = "tx-m", "rx-m";
> > + rockchip,grf = <&grf>;
> > + #sound-dai-cells = <0>;
> > + status = "disabled";
> > + };
> > +
> > i2s1_8ch: i2s@fe410000 {
> > compatible = "rockchip,rk3568-i2s-tdm";
> > reg = <0x0 0xfe410000 0x0 0x1000>;
> >
>
