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
> + 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>;
>
