On 2023/3/30 15:43, Krzysztof Kozlowski wrote:
> On 29/03/2023 17:33, Walker Chen wrote:
>> Add the tdm controller node and sound card for the StarFive JH7110 SoC.
>>
>> Signed-off-by: Walker Chen <walker.chen@xxxxxxxxxxxxxxxx>
>> ---
>> .../jh7110-starfive-visionfive-2.dtsi | 87 +++++++++++++++++++
>> arch/riscv/boot/dts/starfive/jh7110.dtsi | 34 ++++++++
>> 2 files changed, 121 insertions(+)
>>
>> diff --git a/arch/riscv/boot/dts/starfive/jh7110-starfive-visionfive-2.dtsi b/arch/riscv/boot/dts/starfive/jh7110-starfive-visionfive-2.dtsi
>> index 1155b97b593d..35137c2edf5d 100644
>> --- a/arch/riscv/boot/dts/starfive/jh7110-starfive-visionfive-2.dtsi
>> +++ b/arch/riscv/boot/dts/starfive/jh7110-starfive-visionfive-2.dtsi
>> @@ -62,6 +62,10 @@
>> clock-frequency = <297000000>;
>> };
>>
>> +&wm8960_mclk {
>> + clock-frequency = <24576000>;
>> +};
>> +
>> &i2srx_bclk_ext {
>> clock-frequency = <12288000>;
>> };
>> @@ -102,6 +106,14 @@
>> pinctrl-names = "default";
>> pinctrl-0 = <&i2c0_pins>;
>> status = "okay";
>> +
>> + wm8960: codec@1a {
>> + compatible = "wlf,wm8960";
>> + reg = <0x1a>;
>> + #sound-dai-cells = <0>;
>> +
>> + wlf,shared-lrclk;
>> + };
>> };
>>
>> &i2c2 {
>> @@ -214,6 +226,40 @@
>> slew-rate = <0>;
>> };
>> };
>> +
>> + tdm0_pins: tdm0-pins {
>> + tdm0-pins-tx {
>> + pinmux = <GPIOMUX(44, GPOUT_SYS_TDM_TXD,
>> + GPOEN_ENABLE,
>> + GPI_NONE)>;
>> + bias-pull-up;
>> + drive-strength = <2>;
>> + input-disable;
>> + input-schmitt-disable;
>> + slew-rate = <0>;
>> + };
>> +
>> + tdm0-pins-rx {
>> + pinmux = <GPIOMUX(61, GPOUT_HIGH,
>> + GPOEN_DISABLE,
>> + GPI_SYS_TDM_RXD)>;
>> + input-enable;
>> + };
>> +
>> + tdm0-pins-sync {
>> + pinmux = <GPIOMUX(63, GPOUT_HIGH,
>> + GPOEN_DISABLE,
>> + GPI_SYS_TDM_SYNC)>;
>> + input-enable;
>> + };
>> +
>> + tdm0-pins-pcmclk {
>> + pinmux = <GPIOMUX(38, GPOUT_HIGH,
>> + GPOEN_DISABLE,
>> + GPI_SYS_TDM_CLK)>;
>> + input-enable;
>> + };
>> + };
>> };
>>
>> &uart0 {
>> @@ -221,3 +267,44 @@
>> pinctrl-0 = <&uart0_pins>;
>> status = "okay";
>> };
>> +
>> +&tdm {
>> + pinctrl-names = "default";
>> + pinctrl-0 = <&tdm0_pins>;
>> + status = "okay";
>> +};
>> +
>> +&sound0 {
>> + simple-audio-card,dai-link@0 {
>> + reg = <0>;
>> + status = "okay";
>
> Why? Drop.
Will drop it.
>
>> + format = "dsp_a";
>> + bitclock-master = <&dailink_master>;
>> + frame-master = <&dailink_master>;
>> +
>> + widgets =
>
> Drop line break.
OK, will drop it.
>
>> + "Microphone", "Mic Jack",
>> + "Line", "Line In",
>> + "Line", "Line Out",
>> + "Speaker", "Speaker",
>> + "Headphone", "Headphone Jack";
>> + routing =
>
> Drop unnecessary line break.
OK, will drop it.
>
>> + "Headphone Jack", "HP_L",
>> + "Headphone Jack", "HP_R",
>> + "Speaker", "SPK_LP",
>> + "Speaker", "SPK_LN",
>> + "LINPUT1", "Mic Jack",
>> + "LINPUT3", "Mic Jack",
>> + "RINPUT1", "Mic Jack",
>> + "RINPUT2", "Mic Jack";
>> + cpu {
>> + sound-dai = <&tdm>;
>> + };
>> +
>> + dailink_master:codec {
>
> Missing space after label:.
Will be fixed.
>
>> + sound-dai = <&wm8960>;
>> + clocks = <&wm8960_mclk>;
>> + clock-names = "mclk";
>> + };
>> + };
>> +};
>> diff --git a/arch/riscv/boot/dts/starfive/jh7110.dtsi b/arch/riscv/boot/dts/starfive/jh7110.dtsi
>> index b503b6137743..a89158d1d7a6 100644
>> --- a/arch/riscv/boot/dts/starfive/jh7110.dtsi
>> +++ b/arch/riscv/boot/dts/starfive/jh7110.dtsi
>> @@ -210,6 +210,12 @@
>> #clock-cells = <0>;
>> };
>>
>> + wm8960_mclk: wm8960_mclk {
>
> No underscores in node names. Use consistent naming - do you see here
> any nodes named "mclk"?
>
> Anyway this is some fake clock. Real clock should come out from wm8960.
Thank you for pointing out this, it will be fixed.
>
>> + compatible = "fixed-clock";
>> + clock-output-names = "wm8960_mclk";
>> + #clock-cells = <0>;
>> + };
>> +
>> i2srx_bclk_ext: i2srx-bclk-ext-clock {
>> compatible = "fixed-clock";
>> clock-output-names = "i2srx_bclk_ext";
>> @@ -375,6 +381,27 @@
>> status = "disabled";
>> };
>>
>> + tdm: tdm@10090000 {
>> + compatible = "starfive,jh7110-tdm";
>> + reg = <0x0 0x10090000 0x0 0x1000>;
>> + clocks = <&syscrg JH7110_SYSCLK_TDM_AHB>,
>> + <&syscrg JH7110_SYSCLK_TDM_APB>,
>> + <&syscrg JH7110_SYSCLK_TDM_INTERNAL>,
>> + <&syscrg JH7110_SYSCLK_TDM_TDM>,
>> + <&syscrg JH7110_SYSCLK_MCLK_INNER>,
>> + <&tdm_ext>;
>> + clock-names = "tdm_ahb", "tdm_apb",
>> + "tdm_internal", "tdm",
>> + "mclk_inner", "tdm_ext";
>> + resets = <&syscrg JH7110_SYSRST_TDM_AHB>,
>> + <&syscrg JH7110_SYSRST_TDM_APB>,
>> + <&syscrg JH7110_SYSRST_TDM_CORE>;
>> + dmas = <&dma 20>, <&dma 21>;
>> + dma-names = "rx","tx";
>> + #sound-dai-cells = <0>;
>> + status = "disabled";
>> + };
>> +
>> stgcrg: clock-controller@10230000 {
>> compatible = "starfive,jh7110-stgcrg";
>> reg = <0x0 0x10230000 0x0 0x10000>;
>> @@ -601,5 +628,12 @@
>> #reset-cells = <1>;
>> power-domains = <&pwrc JH7110_PD_VOUT>;
>> };
>> +
>> + sound0: snd-card0 {
>
> 1. Why card0?
There are several audio interfaces in JH7110 SoC, each as an independent sound card.
TDM is for snd-card0, latter i2s will be for snd-card1, spdif will be for snd-card2, etc.
> 2. Where is this node located? In MMIO bus? Run some basic checks on
> your DTS before submitting upstream.
> dtbs_check
> dtbs W=1
>
> 3. Why this is even in the DTSI? This really looks wrong.
It seems that the sound node should be located in DTS file more appropriately.
Best regards,
Walker
