Received-SPF: pass (google.com: domain of linux-kernel+bounces-152704-linux.lists.archive=gmail.com@vger.kernel.org designates 147.75.199.223 as permitted sender) client-ip=147.75.199.223;
Date: Mon, 22 Apr 2024 11:42:20 +0800
From: Shawn Guo <shawnguo2@yeah.net>
To: Frank Li <Frank.Li@nxp.com>
Cc: Rob Herring <robh@kernel.org>,
	Krzysztof Kozlowski <krzysztof.kozlowski+dt@linaro.org>,
	Conor Dooley <conor+dt@kernel.org>, Shawn Guo <shawnguo@kernel.org>,
	Sascha Hauer <s.hauer@pengutronix.de>,
	Pengutronix Kernel Team <kernel@pengutronix.de>,
	Fabio Estevam <festevam@gmail.com>, devicetree@vger.kernel.org,
	imx@lists.linux.dev, linux-arm-kernel@lists.infradead.org,
	linux-kernel@vger.kernel.org
Subject: Re: [PATCH 3/3] arm64: dts: imx8dxl-evk: add audio nodes
Message-ID: <ZiXcnMtk5f0RSrHc@dragon>
References: <20240402-b4-dts_dxl_audio-v1-0-d26d25b84a08@nxp.com>
 <20240402-b4-dts_dxl_audio-v1-3-d26d25b84a08@nxp.com>
Precedence: bulk
MIME-Version: 1.0
Content-Type: text/plain; charset=us-ascii
Content-Disposition: inline
In-Reply-To: <20240402-b4-dts_dxl_audio-v1-3-d26d25b84a08@nxp.com>

On Tue, Apr 02, 2024 at 05:02:30PM -0400, Frank Li wrote:
> Add audio nodes for imx8dxl-evk boards.
> 
> Signed-off-by: Frank Li <Frank.Li@nxp.com>
> ---
>  arch/arm64/boot/dts/freescale/imx8dxl-evk.dts | 234 ++++++++++++++++++++++++++
>  1 file changed, 234 insertions(+)
> 
> diff --git a/arch/arm64/boot/dts/freescale/imx8dxl-evk.dts b/arch/arm64/boot/dts/freescale/imx8dxl-evk.dts
> index 2123d431e0613..ba4cdc3534362 100644
> --- a/arch/arm64/boot/dts/freescale/imx8dxl-evk.dts
> +++ b/arch/arm64/boot/dts/freescale/imx8dxl-evk.dts
> @@ -125,6 +125,81 @@ mii_select: regulator-4 {
>  		enable-active-high;
>  		regulator-always-on;
>  	};
> +
> +	bt_sco_codec: bt_sco_codec {

Hyphen is recommended for node name.

> +		#sound-dai-cells = <1>;
> +		compatible = "linux,bt-sco";

We usually have 'compatible' go first.

> +	};
> +
> +	sound-bt-sco {
> +		compatible = "simple-audio-card";
> +		simple-audio-card,name = "bt-sco-audio";
> +		simple-audio-card,format = "dsp_a";
> +		simple-audio-card,bitclock-inversion;
> +		simple-audio-card,frame-master = <&btcpu>;
> +		simple-audio-card,bitclock-master = <&btcpu>;
> +
> +		btcpu: simple-audio-card,cpu {
> +			sound-dai = <&sai0>;
> +			dai-tdm-slot-num = <2>;
> +			dai-tdm-slot-width = <16>;
> +		};
> +
> +		simple-audio-card,codec {
> +			sound-dai = <&bt_sco_codec 1>;
> +		};
> +	};
> +
> +	sound-wm8960-1 {
> +		compatible = "fsl,imx7d-evk-wm8960", "fsl,imx-audio-wm8960";
> +		model = "wm8960-audio";
> +		audio-cpu = <&sai1>;
> +		audio-codec = <&wm8960_1>;
> +		audio-asrc = <&asrc0>;
> +		audio-routing =
> +			"Headphone Jack", "HP_L",
> +			"Headphone Jack", "HP_R",
> +			"Ext Spk", "SPK_LP",
> +			"Ext Spk", "SPK_LN",
> +			"Ext Spk", "SPK_RP",
> +			"Ext Spk", "SPK_RN",
> +			"LINPUT1", "Mic Jack",
> +			"Mic Jack", "MICB";

Nit: can we do this?

		audio-routing = "Headphone Jack", "HP_L",
				"Headphone Jack", "HP_R",
			        ...

> +	};
> +
> +	sound-wm8960-2 {
> +		compatible = "fsl,imx7d-evk-wm8960", "fsl,imx-audio-wm8960";
> +		model = "wm8960-audio-2";
> +		audio-cpu = <&sai2>;
> +		audio-codec = <&wm8960_2>;
> +		capture-only;
> +		audio-routing =
> +			"Headphone Jack", "HP_L",
> +			"Headphone Jack", "HP_R",
> +			"Ext Spk", "SPK_LP",
> +			"Ext Spk", "SPK_LN",
> +			"Ext Spk", "SPK_RP",
> +			"Ext Spk", "SPK_RN",
> +			"LINPUT1", "Mic Jack",
> +			"Mic Jack", "MICB";
> +	};
> +
> +	sound-wm8960-3 {
> +		compatible = "fsl,imx7d-evk-wm8960", "fsl,imx-audio-wm8960";
> +		model = "wm8960-audio-3";
> +		audio-cpu = <&sai3>;
> +		audio-codec = <&wm8960_3>;
> +		capture-only;
> +		audio-routing =
> +			"Headphone Jack", "HP_L",
> +			"Headphone Jack", "HP_R",
> +			"Ext Spk", "SPK_LP",
> +			"Ext Spk", "SPK_LN",
> +			"Ext Spk", "SPK_RP",
> +			"Ext Spk", "SPK_RN",
> +			"LINPUT1", "Mic Jack",
> +			"Mic Jack", "MICB";
> +	};
>  };
>  
>  &adc0 {
> @@ -132,6 +207,11 @@ &adc0 {
>  	status = "okay";
>  };
>  
> +&asrc0 {
> +	fsl,asrc-rate  = <48000>;

One space around =

> +	status = "okay";
> +};
> +
>  &eqos {
>  	pinctrl-names = "default";
>  	pinctrl-0 = <&pinctrl_eqos>;
> @@ -259,6 +339,78 @@ max7322: gpio@68 {
>  			};
>  		};
>  
> +		i2c@1 {
> +			#address-cells = <1>;
> +			#size-cells = <0>;
> +			reg = <0x1>;
> +
> +			wm8960_1: wm8960@1a {
> +				compatible = "wlf,wm8960";
> +				reg = <0x1a>;
> +				clocks = <&mclkout1_lpcg IMX_LPCG_CLK_0>;
> +				clock-names = "mclk";
> +				wlf,shared-lrclk;
> +				wlf,hp-cfg = <2 2 3>;
> +				wlf,gpio-cfg = <1 3>;
> +				assigned-clocks = <&clk IMX_SC_R_AUDIO_PLL_0 IMX_SC_PM_CLK_PLL>,
> +						  <&clk IMX_SC_R_AUDIO_PLL_0 IMX_SC_PM_CLK_SLV_BUS>,
> +						  <&clk IMX_SC_R_AUDIO_PLL_0 IMX_SC_PM_CLK_MST_BUS>,
> +						  <&mclkout1_lpcg IMX_LPCG_CLK_0>;
> +				assigned-clock-rates = <786432000>,
> +						       <49152000>,
> +						       <12288000>,
> +						       <12288000>;
> +			};
> +		};
> +
> +		i2c@2 {
> +			#address-cells = <1>;
> +			#size-cells = <0>;
> +			reg = <0x2>;
> +
> +			wm8960_2: wm8960@1a {
> +				compatible = "wlf,wm8960";
> +				reg = <0x1a>;
> +				clocks = <&mclkout1_lpcg IMX_LPCG_CLK_0>;
> +				clock-names = "mclk";
> +				wlf,shared-lrclk;
> +				wlf,hp-cfg = <2 2 3>;
> +				wlf,gpio-cfg = <1 3>;
> +				assigned-clocks = <&clk IMX_SC_R_AUDIO_PLL_0 IMX_SC_PM_CLK_PLL>,
> +						  <&clk IMX_SC_R_AUDIO_PLL_0 IMX_SC_PM_CLK_SLV_BUS>,
> +						  <&clk IMX_SC_R_AUDIO_PLL_0 IMX_SC_PM_CLK_MST_BUS>,
> +						  <&mclkout1_lpcg IMX_LPCG_CLK_0>;
> +				assigned-clock-rates = <786432000>,
> +						       <49152000>,
> +						       <12288000>,
> +						       <12288000>;
> +			};
> +		};
> +
> +		i2c@3 {
> +			#address-cells = <1>;
> +			#size-cells = <0>;
> +			reg = <0x3>;
> +
> +			wm8960_3: wm8960@1a {

audio-codec for node name.

> +				compatible = "wlf,wm8960";
> +				reg = <0x1a>;
> +				clocks = <&mclkout1_lpcg IMX_LPCG_CLK_0>;
> +				clock-names = "mclk";
> +				wlf,shared-lrclk;
> +				wlf,hp-cfg = <2 2 3>;
> +				wlf,gpio-cfg = <1 3>;
> +				assigned-clocks = <&clk IMX_SC_R_AUDIO_PLL_0 IMX_SC_PM_CLK_PLL>,
> +						  <&clk IMX_SC_R_AUDIO_PLL_0 IMX_SC_PM_CLK_SLV_BUS>,
> +						  <&clk IMX_SC_R_AUDIO_PLL_0 IMX_SC_PM_CLK_MST_BUS>,
> +						  <&mclkout1_lpcg IMX_LPCG_CLK_0>;
> +				assigned-clock-rates = <786432000>,
> +						       <49152000>,
> +						       <12288000>,
> +						       <12288000>;
> +			};
> +		};
> +
>  		i2c@4 {
>  			#address-cells = <1>;
>  			#size-cells = <0>;
> @@ -362,6 +514,53 @@ &lsio_gpio5 {
>  	status = "okay";
>  };
>  
> +&sai0 {
> +	pinctrl-names = "default";
> +	pinctrl-0 = <&pinctrl_sai0>;
> +	#sound-dai-cells = <0>;
> +	assigned-clocks = <&clk IMX_SC_R_AUDIO_PLL_0 IMX_SC_PM_CLK_PLL>,
> +			<&clk IMX_SC_R_AUDIO_PLL_0 IMX_SC_PM_CLK_SLV_BUS>,
> +			<&clk IMX_SC_R_AUDIO_PLL_0 IMX_SC_PM_CLK_MST_BUS>,
> +			<&sai0_lpcg IMX_LPCG_CLK_0>;

	assigned-clocks = <&clk IMX_SC_R_AUDIO_PLL_0 IMX_SC_PM_CLK_PLL>,
			  <&clk IMX_SC_R_AUDIO_PLL_0 IMX_SC_PM_CLK_SLV_BUS>,
			  ...

> +	assigned-clock-rates = <786432000>, <49152000>, <12288000>, <49152000>;
> +	status = "okay";
> +};
> +
> +&sai1 {
> +	assigned-clocks = <&clk IMX_SC_R_AUDIO_PLL_0 IMX_SC_PM_CLK_PLL>,
> +			<&clk IMX_SC_R_AUDIO_PLL_0 IMX_SC_PM_CLK_SLV_BUS>,
> +			<&clk IMX_SC_R_AUDIO_PLL_0 IMX_SC_PM_CLK_MST_BUS>,
> +			<&sai1_lpcg IMX_LPCG_CLK_0>;
> +	assigned-clock-rates = <786432000>, <49152000>, <12288000>, <49152000>;
> +	pinctrl-names = "default";
> +	pinctrl-0 = <&pinctrl_sai1>;
> +	status = "okay";
> +};
> +
> +&sai2 {
> +	assigned-clocks = <&clk IMX_SC_R_AUDIO_PLL_0 IMX_SC_PM_CLK_PLL>,
> +			<&clk IMX_SC_R_AUDIO_PLL_0 IMX_SC_PM_CLK_SLV_BUS>,
> +			<&clk IMX_SC_R_AUDIO_PLL_0 IMX_SC_PM_CLK_MST_BUS>,
> +			<&sai2_lpcg IMX_LPCG_CLK_0>;
> +	assigned-clock-rates = <786432000>, <49152000>, <12288000>, <49152000>;
> +	pinctrl-names = "default";
> +	pinctrl-0 = <&pinctrl_sai2>;
> +	fsl,sai-asynchronous;
> +	status = "okay";
> +};
> +
> +&sai3 {
> +	assigned-clocks = <&clk IMX_SC_R_AUDIO_PLL_0 IMX_SC_PM_CLK_PLL>,
> +			<&clk IMX_SC_R_AUDIO_PLL_0 IMX_SC_PM_CLK_SLV_BUS>,
> +			<&clk IMX_SC_R_AUDIO_PLL_0 IMX_SC_PM_CLK_MST_BUS>,
> +			<&sai3_lpcg IMX_LPCG_CLK_0>;
> +	assigned-clock-rates = <786432000>, <49152000>, <12288000>, <49152000>;
> +	pinctrl-names = "default";
> +	pinctrl-0 = <&pinctrl_sai3>;
> +	fsl,sai-asynchronous;
> +	status = "okay";
> +};
> +
>  &thermal_zones {
>  	pmic-thermal {
>  		polling-delay-passive = <250>;
> @@ -595,6 +794,41 @@ IMX8DXL_UART0_TX_ADMA_UART0_TX		0x06000020
>  		>;
>  	};
>  
> +	pinctrl_sai0: sai0grp {
> +		fsl,pins = <
> +			IMX8DXL_SPI0_CS0_ADMA_SAI0_RXD		0x06000060
> +			IMX8DXL_SPI0_CS1_ADMA_SAI0_RXC		0x06000040
> +			IMX8DXL_SPI0_SCK_ADMA_SAI0_TXC		0x06000060
> +			IMX8DXL_SPI0_SDI_ADMA_SAI0_TXD		0x06000060
> +			IMX8DXL_SPI0_SDO_ADMA_SAI0_TXFS		0x06000040
> +		>;
> +	};
> +
> +	pinctrl_sai1: sai1grp {
> +		fsl,pins = <
> +			IMX8DXL_FLEXCAN0_RX_ADMA_SAI1_TXC     0x06000040

Can the indent between pin constant and value be tabs instead of spaces?

Shawn

> +			IMX8DXL_FLEXCAN0_TX_ADMA_SAI1_TXFS    0x06000040
> +			IMX8DXL_FLEXCAN1_RX_ADMA_SAI1_TXD     0x06000060
> +			IMX8DXL_FLEXCAN1_TX_ADMA_SAI1_RXD     0x06000060
> +		>;
> +	};
> +
> +	pinctrl_sai2: sai2grp {
> +		fsl,pins = <
> +			IMX8DXL_SNVS_TAMPER_OUT3_ADMA_SAI2_RXC   0x06000040
> +			IMX8DXL_SNVS_TAMPER_IN0_ADMA_SAI2_RXFS   0x06000040
> +			IMX8DXL_SNVS_TAMPER_OUT4_ADMA_SAI2_RXD   0x06000060
> +		>;
> +	};
> +
> +	pinctrl_sai3: sai3grp {
> +		fsl,pins = <
> +			IMX8DXL_SNVS_TAMPER_IN1_ADMA_SAI3_RXC    0x06000040
> +			IMX8DXL_SNVS_TAMPER_IN3_ADMA_SAI3_RXFS   0x06000040
> +			IMX8DXL_SNVS_TAMPER_IN2_ADMA_SAI3_RXD    0x06000060
> +		>;
> +	};
> +
>  	pinctrl_usdhc1: usdhc1grp {
>  		fsl,pins = <
>  			IMX8DXL_EMMC0_CLK_CONN_EMMC0_CLK	0x06000041
> 
> -- 
> 2.34.1
>