[PATCH v2 0/5] ASoC: samsung: fsd: audio support for FSD SoC

[PATCH v2 0/5] ASoC: samsung: fsd: audio support for FSD SoC

[Padmanabhan Rajanbabu]

The intention of this patch series is to enable audio support on FSD SoC.

Changes in v2:
1. New compatible added in Exynos I2S driver for FSD platform.
2. Added Fixup support for Exynos I2S CPU DAI.
3. Migration of manual PSR, OPCLK configuration to Exynos CPU DAI driver as
fixup.
4. Migrated from dedicated sound card to simple audio card.
5. Support added for tlv320aic3x-i2c codec on FSD platform.

Changes in v1:
1. Add TDM support on samsung I2S interface.
2. Allow sound card to directly configure I2S prescaler divider instead of
calculating it from frame clock.
3. The sound card support for FSD SoC which utilizes samsung I2S interface
as CPU DAI.

Padmanabhan Rajanbabu (5):
  ASoC: samsung-i2s: add FSD compatibile string
  ASoC: samsung: i2s: add support for FSD I2S
  arm64: dts: fsd: Add I2S DAI node for Tesla FSD
  arm64: dts: fsd: Add codec node for Tesla FSD
  arm64: dts: fsd: Add sound card node for Tesla FSD

 .../bindings/sound/samsung-i2s.yaml           |  4 ++
 arch/arm64/boot/dts/tesla/fsd-evb.dts         | 59 +++++++++++++++++++
 arch/arm64/boot/dts/tesla/fsd-pinctrl.dtsi    | 14 +++++
 arch/arm64/boot/dts/tesla/fsd.dtsi            | 34 +++++++++++
 sound/soc/samsung/i2s.c                       | 57 ++++++++++++++++++
 5 files changed, 168 insertions(+)

-- 
2.17.1

[PATCH v2 1/5] ASoC: dt-bindings: Add FSD I2S controller bindings

[Padmanabhan Rajanbabu]

Add bindings for FSD CPU DAI driver which supports stereo channel
audio playback and capture on FSD platform.

Signed-off-by: Padmanabhan Rajanbabu <p.rajanbabu@samsung.com>
---
 Documentation/devicetree/bindings/sound/samsung-i2s.yaml | 4 ++++
 1 file changed, 4 insertions(+)

diff --git a/Documentation/devicetree/bindings/sound/samsung-i2s.yaml b/Documentation/devicetree/bindings/sound/samsung-i2s.yaml
index 8d5dcf9cd43e..ff187e16d301 100644
--- a/Documentation/devicetree/bindings/sound/samsung-i2s.yaml
+++ b/Documentation/devicetree/bindings/sound/samsung-i2s.yaml
@@ -37,12 +37,16 @@ properties:
       samsung,exynos7-i2s1: I2S1 on previous samsung platforms supports
       stereo channels. Exynos7 I2S1 upgraded to 5.1 multichannel with
       slightly modified bit offsets.
+
+      tesla,fsd-i2s: with all the available features of Exynos7 I2S,
+      supporting only stereo channel playback and capture.
     enum:
       - samsung,s3c6410-i2s
       - samsung,s5pv210-i2s
       - samsung,exynos5420-i2s
       - samsung,exynos7-i2s
       - samsung,exynos7-i2s1
+      - tesla,fsd-i2s
 
   '#address-cells':
     const: 1
-- 
2.17.1

[PATCH v2 2/5] ASoC: samsung: i2s: add support for FSD I2S

[Padmanabhan Rajanbabu]

Add support for enabling I2S controller on FSD platform.

FSD I2S controller is based on Exynos7 I2S controller, supporting
2CH playback/capture in I2S mode and 7.1CH playback/capture in TDM
mode.

Signed-off-by: Padmanabhan Rajanbabu <p.rajanbabu@samsung.com>
---
 sound/soc/samsung/i2s-regs.h |  1 +
 sound/soc/samsung/i2s.c      | 57 ++++++++++++++++++++++++++++++++++++
 2 files changed, 58 insertions(+)

diff --git a/sound/soc/samsung/i2s-regs.h b/sound/soc/samsung/i2s-regs.h
index b4b5d6053503..4444c857d0c0 100644
--- a/sound/soc/samsung/i2s-regs.h
+++ b/sound/soc/samsung/i2s-regs.h
@@ -132,6 +132,7 @@
 #define EXYNOS7_MOD_RCLK_192FS	7
 
 #define PSR_PSREN		(1 << 15)
+#define PSR_PSVAL(x)		(((x - 1) << 8) & 0x3f00)
 
 #define FIC_TX2COUNT(x)		(((x) >>  24) & 0xf)
 #define FIC_TX1COUNT(x)		(((x) >>  16) & 0xf)
diff --git a/sound/soc/samsung/i2s.c b/sound/soc/samsung/i2s.c
index 9505200f3d11..dcb5c438cb2f 100644
--- a/sound/soc/samsung/i2s.c
+++ b/sound/soc/samsung/i2s.c
@@ -50,6 +50,10 @@ struct samsung_i2s_dai_data {
 	u32 quirks;
 	unsigned int pcm_rates;
 	const struct samsung_i2s_variant_regs *i2s_variant_regs;
+	void (*fixup_early)(struct snd_pcm_substream *substream,
+					struct snd_soc_dai *dai);
+	void (*fixup_late)(struct snd_pcm_substream *substream,
+					struct snd_soc_dai *dai);
 };
 
 struct i2s_dai {
@@ -111,6 +115,10 @@ struct samsung_i2s_priv {
 	u32 suspend_i2spsr;
 
 	const struct samsung_i2s_variant_regs *variant_regs;
+	void (*fixup_early)(struct snd_pcm_substream *substream,
+						struct snd_soc_dai *dai);
+	void (*fixup_late)(struct snd_pcm_substream *substream,
+						struct snd_soc_dai *dai);
 	u32 quirks;
 
 	/* The clock provider's data */
@@ -940,6 +948,10 @@ static int i2s_trigger(struct snd_pcm_substream *substream,
 	case SNDRV_PCM_TRIGGER_RESUME:
 	case SNDRV_PCM_TRIGGER_PAUSE_RELEASE:
 		pm_runtime_get_sync(dai->dev);
+
+		if (priv->fixup_early)
+			priv->fixup_early(substream, dai);
+
 		spin_lock_irqsave(&priv->lock, flags);
 
 		if (config_setup(i2s)) {
@@ -947,6 +959,13 @@ static int i2s_trigger(struct snd_pcm_substream *substream,
 			return -EINVAL;
 		}
 
+		spin_unlock_irqrestore(&priv->lock, flags);
+
+		if (priv->fixup_late)
+			priv->fixup_late(substream, dai);
+
+		spin_lock_irqsave(&priv->lock, flags);
+
 		if (capture)
 			i2s_rxctrl(i2s, 1);
 		else
@@ -1410,6 +1429,8 @@ static int samsung_i2s_probe(struct platform_device *pdev)
 
 	if (np) {
 		priv->quirks = i2s_dai_data->quirks;
+		priv->fixup_early = i2s_dai_data->fixup_early;
+		priv->fixup_late = i2s_dai_data->fixup_late;
 	} else {
 		if (!i2s_pdata) {
 			dev_err(&pdev->dev, "Missing platform data\n");
@@ -1563,6 +1584,31 @@ static int samsung_i2s_remove(struct platform_device *pdev)
 	return 0;
 }
 
+void fsd_i2s_fixup_early(struct snd_pcm_substream *substream,
+		struct snd_soc_dai *dai)
+{
+	struct snd_soc_pcm_runtime *rtd = asoc_substream_to_rtd(substream);
+	struct i2s_dai *i2s = to_info(asoc_rtd_to_cpu(rtd, 0));
+	struct i2s_dai *other = get_other_dai(i2s);
+
+	if (!is_opened(other)) {
+		i2s_set_sysclk(dai, SAMSUNG_I2S_CDCLK, 192, SND_SOC_CLOCK_OUT);
+		i2s_set_sysclk(dai, SAMSUNG_I2S_OPCLK, 0, MOD_OPCLK_PCLK);
+	}
+}
+
+void fsd_i2s_fixup_late(struct snd_pcm_substream *substream,
+		struct snd_soc_dai *dai)
+{
+	struct snd_soc_pcm_runtime *rtd = asoc_substream_to_rtd(substream);
+	struct samsung_i2s_priv *priv = snd_soc_dai_get_drvdata(dai);
+	struct i2s_dai *i2s = to_info(asoc_rtd_to_cpu(rtd, 0));
+	struct i2s_dai *other = get_other_dai(i2s);
+
+	if (!is_opened(other))
+		writel(PSR_PSVAL(2) | PSR_PSREN, priv->addr + I2SPSR);
+}
+
 static const struct samsung_i2s_variant_regs i2sv3_regs = {
 	.bfs_off = 1,
 	.rfs_off = 3,
@@ -1652,6 +1698,14 @@ static const struct samsung_i2s_dai_data i2sv5_dai_type_i2s1 __maybe_unused = {
 	.i2s_variant_regs = &i2sv5_i2s1_regs,
 };
 
+static const struct samsung_i2s_dai_data fsd_dai_type __maybe_unused = {
+	.quirks = QUIRK_SEC_DAI | QUIRK_NEED_RSTCLR | QUIRK_SUPPORTS_TDM,
+	.pcm_rates = SNDRV_PCM_RATE_8000_192000,
+	.i2s_variant_regs = &i2sv7_regs,
+	.fixup_early = fsd_i2s_fixup_early,
+	.fixup_late = fsd_i2s_fixup_late,
+};
+
 static const struct platform_device_id samsung_i2s_driver_ids[] = {
 	{
 		.name           = "samsung-i2s",
@@ -1678,6 +1732,9 @@ static const struct of_device_id exynos_i2s_match[] = {
 	}, {
 		.compatible = "samsung,exynos7-i2s1",
 		.data = &i2sv5_dai_type_i2s1,
+	}, {
+		.compatible = "tesla,fsd-i2s",
+		.data = &fsd_dai_type,
 	},
 	{},
 };
-- 
2.17.1

[PATCH v2 3/5] arm64: dts: fsd: Add I2S DAI node for Tesla FSD

[Padmanabhan Rajanbabu]

Add device tree node for I2S0 and I2S1 CPU DAI instances for Tesla
FSD platform.

FSD SoC has 2 I2S instances driving stereo channel audio playback
and capture with external DMA support.

Signed-off-by: Padmanabhan Rajanbabu <p.rajanbabu@samsung.com>
---
 arch/arm64/boot/dts/tesla/fsd-evb.dts      |  8 +++++
 arch/arm64/boot/dts/tesla/fsd-pinctrl.dtsi | 14 +++++++++
 arch/arm64/boot/dts/tesla/fsd.dtsi         | 34 ++++++++++++++++++++++
 3 files changed, 56 insertions(+)

diff --git a/arch/arm64/boot/dts/tesla/fsd-evb.dts b/arch/arm64/boot/dts/tesla/fsd-evb.dts
index 1db6ddf03f01..cf5f2ce4d2a7 100644
--- a/arch/arm64/boot/dts/tesla/fsd-evb.dts
+++ b/arch/arm64/boot/dts/tesla/fsd-evb.dts
@@ -38,6 +38,14 @@
 	status = "okay";
 };
 
+&i2s_0 {
+	status = "okay";
+};
+
+&i2s_1 {
+	status = "okay";
+};
+
 &ufs {
 	status = "okay";
 };
diff --git a/arch/arm64/boot/dts/tesla/fsd-pinctrl.dtsi b/arch/arm64/boot/dts/tesla/fsd-pinctrl.dtsi
index e3852c946352..6b51e31afcff 100644
--- a/arch/arm64/boot/dts/tesla/fsd-pinctrl.dtsi
+++ b/arch/arm64/boot/dts/tesla/fsd-pinctrl.dtsi
@@ -339,6 +339,20 @@
 		samsung,pin-pud = <FSD_PIN_PULL_UP>;
 		samsung,pin-drv = <FSD_PIN_DRV_LV4>;
 	};
+
+	i2s0_bus: i2s0-bus-pins {
+		samsung,pins = "gpd1-0", "gpd1-1", "gpd1-2", "gpd1-3", "gpd1-4";
+		samsung,pin-function = <FSD_PIN_FUNC_2>;
+		samsung,pin-pud = <FSD_PIN_PULL_DOWN>;
+		samsung,pin-drv = <FSD_PIN_DRV_LV4>;
+	};
+
+	i2s1_bus: i2s1-bus-pins {
+		samsung,pins = "gpd2-0", "gpd2-1", "gpd2-2", "gpd2-3", "gpd2-4";
+		samsung,pin-function = <FSD_PIN_FUNC_2>;
+		samsung,pin-pud = <FSD_PIN_PULL_DOWN>;
+		samsung,pin-drv = <FSD_PIN_DRV_LV4>;
+	};
 };
 
 &pinctrl_pmu {
diff --git a/arch/arm64/boot/dts/tesla/fsd.dtsi b/arch/arm64/boot/dts/tesla/fsd.dtsi
index f35bc5a288c2..3e7265a41e37 100644
--- a/arch/arm64/boot/dts/tesla/fsd.dtsi
+++ b/arch/arm64/boot/dts/tesla/fsd.dtsi
@@ -738,6 +738,40 @@
 			status = "disabled";
 		};
 
+		i2s_0: i2s@140e0000 {
+			compatible = "tesla,fsd-i2s";
+			reg = <0x0 0x140e0000 0x0 0x100>;
+			interrupts = <GIC_SPI 206 IRQ_TYPE_LEVEL_HIGH>;
+			dmas = <&pdma1 14>, <&pdma1 13>, <&pdma1 12>;
+			dma-names = "tx", "rx", "tx-sec";
+			#clock-cells = <1>;
+			clocks = <&clock_peric PERIC_PCLK_TDM0>,
+				 <&clock_peric PERIC_HCLK_TDM0>,
+				 <&clock_peric PERIC_HCLK_TDM0>;
+			clock-names = "iis", "i2s_opclk0", "i2s_opclk1";
+			pinctrl-names = "default";
+			pinctrl-0 = <&i2s0_bus>;
+			#sound-dai-cells = <1>;
+			status = "disabled";
+		};
+
+		i2s_1: i2s@140f0000 {
+			compatible = "tesla,fsd-i2s";
+			reg = <0x0 0x140f0000 0x0 0x100>;
+			interrupts = <GIC_SPI 207 IRQ_TYPE_LEVEL_HIGH>;
+			dmas = <&pdma1 17>, <&pdma1 16>, <&pdma1 15>;
+			dma-names = "tx", "rx", "tx-sec";
+			#clock-cells = <1>;
+			clocks = <&clock_peric PERIC_PCLK_TDM1>,
+				 <&clock_peric PERIC_HCLK_TDM1>,
+				 <&clock_peric PERIC_HCLK_TDM1>;
+			clock-names = "iis", "i2s_opclk0", "i2s_opclk1";
+			pinctrl-names = "default";
+			pinctrl-0 = <&i2s1_bus>;
+			#sound-dai-cells = <1>;
+			status = "disabled";
+		};
+
 		pinctrl_pmu: pinctrl@114f0000 {
 			compatible = "tesla,fsd-pinctrl";
 			reg = <0x0 0x114f0000 0x0 0x1000>;
-- 
2.17.1

[PATCH v2 4/5] arm64: dts: fsd: Add codec node for Tesla FSD

[Padmanabhan Rajanbabu]

Add device tree node support for codec on Tesla FSD platform.

Signed-off-by: Padmanabhan Rajanbabu <p.rajanbabu@samsung.com>
---
 arch/arm64/boot/dts/tesla/fsd-evb.dts | 13 +++++++++++++
 1 file changed, 13 insertions(+)

diff --git a/arch/arm64/boot/dts/tesla/fsd-evb.dts b/arch/arm64/boot/dts/tesla/fsd-evb.dts
index cf5f2ce4d2a7..e2fd49774f15 100644
--- a/arch/arm64/boot/dts/tesla/fsd-evb.dts
+++ b/arch/arm64/boot/dts/tesla/fsd-evb.dts
@@ -10,6 +10,7 @@
 
 /dts-v1/;
 #include "fsd.dtsi"
+#include <dt-bindings/gpio/gpio.h>
 
 / {
 	model = "Tesla Full Self-Driving (FSD) Evaluation board";
@@ -34,6 +35,18 @@
 	clock-frequency = <24000000>;
 };
 
+&hsi2c_5 {
+	status = "okay";
+
+	tlv320aic3x: codec@18 {
+		#sound-dai-cells = <0>;
+		compatible = "ti,tlv320aic3104";
+		reg = <0x18>;
+		reset-gpios = <&gpg1 6 GPIO_ACTIVE_LOW>;
+		status = "okay";
+	};
+};
+
 &serial_0 {
 	status = "okay";
 };
-- 
2.17.1

[PATCH v2 5/5] arm64: dts: fsd: Add sound card node for Tesla FSD

[Padmanabhan Rajanbabu]

Add device tree node support for sound card on Tesla FSD board

Signed-off-by: Padmanabhan Rajanbabu <p.rajanbabu@samsung.com>
---
 arch/arm64/boot/dts/tesla/fsd-evb.dts | 37 +++++++++++++++++++++++++++
 1 file changed, 37 insertions(+)

diff --git a/arch/arm64/boot/dts/tesla/fsd-evb.dts b/arch/arm64/boot/dts/tesla/fsd-evb.dts
index e2fd49774f15..ce726bddfb50 100644
--- a/arch/arm64/boot/dts/tesla/fsd-evb.dts
+++ b/arch/arm64/boot/dts/tesla/fsd-evb.dts
@@ -29,6 +29,43 @@
 		device_type = "memory";
 		reg = <0x0 0x80000000 0x2 0x00000000>;
 	};
+
+	sound {
+		compatible = "simple-audio-card";
+
+		#address-cells = <1>;
+		#size-cells = <0>;
+
+		simple-audio-card,name = "FSD Audio Card";
+		simple-audio-card,widgets =
+			"Line", "Line Out",
+			"Line", "Line In";
+		simple-audio-card,routing =
+			"Line Out", "LLOUT",
+			"Line Out", "RLOUT",
+			"MIC2L", "Line In",
+			"MIC2R", "Line In";
+
+		status = "okay";
+
+		simple-audio-card,dai-link@0 {
+			reg = <0>;
+			format = "i2s";
+			bitclock-master = <&tlv320aic3x>;
+			frame-master = <&tlv320aic3x>;
+
+			cpu0 {
+				sound-dai = <&i2s_0 0>;
+			};
+			cpu1 {
+				sound-dai = <&i2s_0 1>;
+			};
+			codec {
+				sound-dai = <&tlv320aic3x>;
+				system-clock-frequency = <33000000>;
+			};
+		};
+	};
 };
 
 &fin_pll {
-- 
2.17.1

Re: [PATCH v2 2/5] ASoC: samsung: i2s: add support for FSD I2S

[Krzysztof Kozlowski]

On 03/01/2023 05:56, Padmanabhan Rajanbabu wrote:
> Add support for enabling I2S controller on FSD platform.
> 
> FSD I2S controller is based on Exynos7 I2S controller, supporting
> 2CH playback/capture in I2S mode and 7.1CH playback/capture in TDM
> mode.
> 
> Signed-off-by: Padmanabhan Rajanbabu <p.rajanbabu@samsung.com>
> ---
>  sound/soc/samsung/i2s-regs.h |  1 +
>  sound/soc/samsung/i2s.c      | 57 ++++++++++++++++++++++++++++++++++++
>  2 files changed, 58 insertions(+)
> 
> diff --git a/sound/soc/samsung/i2s-regs.h b/sound/soc/samsung/i2s-regs.h
> index b4b5d6053503..4444c857d0c0 100644
> --- a/sound/soc/samsung/i2s-regs.h
> +++ b/sound/soc/samsung/i2s-regs.h
> @@ -132,6 +132,7 @@
>  #define EXYNOS7_MOD_RCLK_192FS	7
>  
>  #define PSR_PSREN		(1 << 15)
> +#define PSR_PSVAL(x)		(((x - 1) << 8) & 0x3f00)
>  
>  #define FIC_TX2COUNT(x)		(((x) >>  24) & 0xf)
>  #define FIC_TX1COUNT(x)		(((x) >>  16) & 0xf)
> diff --git a/sound/soc/samsung/i2s.c b/sound/soc/samsung/i2s.c
> index 9505200f3d11..dcb5c438cb2f 100644
> --- a/sound/soc/samsung/i2s.c
> +++ b/sound/soc/samsung/i2s.c
> @@ -50,6 +50,10 @@ struct samsung_i2s_dai_data {
>  	u32 quirks;
>  	unsigned int pcm_rates;
>  	const struct samsung_i2s_variant_regs *i2s_variant_regs;
> +	void (*fixup_early)(struct snd_pcm_substream *substream,
> +					struct snd_soc_dai *dai);
> +	void (*fixup_late)(struct snd_pcm_substream *substream,
> +					struct snd_soc_dai *dai);
>  };
>  
>  struct i2s_dai {
> @@ -111,6 +115,10 @@ struct samsung_i2s_priv {
>  	u32 suspend_i2spsr;
>  
>  	const struct samsung_i2s_variant_regs *variant_regs;
> +	void (*fixup_early)(struct snd_pcm_substream *substream,
> +						struct snd_soc_dai *dai);
> +	void (*fixup_late)(struct snd_pcm_substream *substream,
> +						struct snd_soc_dai *dai);
>  	u32 quirks;
>  
>  	/* The clock provider's data */
> @@ -940,6 +948,10 @@ static int i2s_trigger(struct snd_pcm_substream *substream,
>  	case SNDRV_PCM_TRIGGER_RESUME:
>  	case SNDRV_PCM_TRIGGER_PAUSE_RELEASE:
>  		pm_runtime_get_sync(dai->dev);
> +
> +		if (priv->fixup_early)
> +			priv->fixup_early(substream, dai);
> +
>  		spin_lock_irqsave(&priv->lock, flags);
>  
>  		if (config_setup(i2s)) {
> @@ -947,6 +959,13 @@ static int i2s_trigger(struct snd_pcm_substream *substream,
>  			return -EINVAL;
>  		}
>  

Except several warnings this patch generates, this is a bit surprising:

> +		spin_unlock_irqrestore(&priv->lock, flags);

You have critical section which you now break into two. You cannot do
this usually. How the synchronization is now kept?

> +
> +		if (priv->fixup_late)
> +			priv->fixup_late(substream, dai);
> +
> +		spin_lock_irqsave(&priv->lock, flags);
> +
>  		if (capture)
>  			i2s_rxctrl(i2s, 1);
>  		else

Best regards,
Krzysztof

Re: [PATCH v2 4/5] arm64: dts: fsd: Add codec node for Tesla FSD

[Krzysztof Kozlowski]

On 03/01/2023 05:56, Padmanabhan Rajanbabu wrote:
> Add device tree node support for codec on Tesla FSD platform.
> 
> Signed-off-by: Padmanabhan Rajanbabu <p.rajanbabu@samsung.com>
> ---
>  arch/arm64/boot/dts/tesla/fsd-evb.dts | 13 +++++++++++++
>  1 file changed, 13 insertions(+)
> 
> diff --git a/arch/arm64/boot/dts/tesla/fsd-evb.dts b/arch/arm64/boot/dts/tesla/fsd-evb.dts
> index cf5f2ce4d2a7..e2fd49774f15 100644
> --- a/arch/arm64/boot/dts/tesla/fsd-evb.dts
> +++ b/arch/arm64/boot/dts/tesla/fsd-evb.dts
> @@ -10,6 +10,7 @@
>  
>  /dts-v1/;
>  #include "fsd.dtsi"
> +#include <dt-bindings/gpio/gpio.h>
>  
>  / {
>  	model = "Tesla Full Self-Driving (FSD) Evaluation board";
> @@ -34,6 +35,18 @@
>  	clock-frequency = <24000000>;
>  };
>  
> +&hsi2c_5 {
> +	status = "okay";
> +
> +	tlv320aic3x: codec@18 {
> +		#sound-dai-cells = <0>;
> +		compatible = "ti,tlv320aic3104";
> +		reg = <0x18>;

First compatible, then reg, then the rest.

> +		reset-gpios = <&gpg1 6 GPIO_ACTIVE_LOW>;
> +		status = "okay";

It's by default. Why do you need it?

Best regards,
Krzysztof

Re: [PATCH v2 5/5] arm64: dts: fsd: Add sound card node for Tesla FSD

[Krzysztof Kozlowski]

On 03/01/2023 05:56, Padmanabhan Rajanbabu wrote:
> Add device tree node support for sound card on Tesla FSD board
> 
> Signed-off-by: Padmanabhan Rajanbabu <p.rajanbabu@samsung.com>
> ---
>  arch/arm64/boot/dts/tesla/fsd-evb.dts | 37 +++++++++++++++++++++++++++
>  1 file changed, 37 insertions(+)
> 
> diff --git a/arch/arm64/boot/dts/tesla/fsd-evb.dts b/arch/arm64/boot/dts/tesla/fsd-evb.dts
> index e2fd49774f15..ce726bddfb50 100644
> --- a/arch/arm64/boot/dts/tesla/fsd-evb.dts
> +++ b/arch/arm64/boot/dts/tesla/fsd-evb.dts
> @@ -29,6 +29,43 @@
>  		device_type = "memory";
>  		reg = <0x0 0x80000000 0x2 0x00000000>;
>  	};
> +
> +	sound {
> +		compatible = "simple-audio-card";
> +
> +		#address-cells = <1>;
> +		#size-cells = <0>;
> +
> +		simple-audio-card,name = "FSD Audio Card";
> +		simple-audio-card,widgets =
> +			"Line", "Line Out",

I don't think you need to break the line after '='.

> +			"Line", "Line In";
> +		simple-audio-card,routing =
> +			"Line Out", "LLOUT",
> +			"Line Out", "RLOUT",
> +			"MIC2L", "Line In",
> +			"MIC2R", "Line In";
> +
> +		status = "okay";

Why?

> +
> +		simple-audio-card,dai-link@0 {
> +			reg = <0>;
> +			format = "i2s";
> +			bitclock-master = <&tlv320aic3x>;
> +			frame-master = <&tlv320aic3x>;
> +
> +			cpu0 {

Does not look like you tested the DTS against bindings. Please run `make
dtbs_check` (see Documentation/devicetree/bindings/writing-schema.rst
for instructions).

> +				sound-dai = <&i2s_0 0>;
> +			};
> +			cpu1 {
> +				sound-dai = <&i2s_0 1>;
> +			};
> +			codec {
> +				sound-dai = <&tlv320aic3x>;
> +				system-clock-frequency = <33000000>;
> +			};

Best regards,
Krzysztof

Re: [PATCH v2 1/5] ASoC: dt-bindings: Add FSD I2S controller bindings

[Mark Brown]

[-- Attachment #1: Type: text/plain, Size: 257 bytes --]

On Tue, Jan 03, 2023 at 10:26:09AM +0530, Padmanabhan Rajanbabu wrote:

> +
> +      tesla,fsd-i2s: with all the available features of Exynos7 I2S,
> +      supporting only stereo channel playback and capture.

The driver claims that 7.1 is also supported.

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 488 bytes --]

Re: [PATCH v2 2/5] ASoC: samsung: i2s: add support for FSD I2S

[Mark Brown]

[-- Attachment #1: Type: text/plain, Size: 580 bytes --]

On Tue, Jan 03, 2023 at 10:26:10AM +0530, Padmanabhan Rajanbabu wrote:

> +void fsd_i2s_fixup_early(struct snd_pcm_substream *substream,
> +		struct snd_soc_dai *dai)
> +{
> +	struct snd_soc_pcm_runtime *rtd = asoc_substream_to_rtd(substream);
> +	struct i2s_dai *i2s = to_info(asoc_rtd_to_cpu(rtd, 0));
> +	struct i2s_dai *other = get_other_dai(i2s);
> +
> +	if (!is_opened(other)) {
> +		i2s_set_sysclk(dai, SAMSUNG_I2S_CDCLK, 192, SND_SOC_CLOCK_OUT);
> +		i2s_set_sysclk(dai, SAMSUNG_I2S_OPCLK, 0, MOD_OPCLK_PCLK);
> +	}
> +}

This looks like we're just hard coding to 192kHz?

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 488 bytes --]

RE: [PATCH v2 2/5] ASoC: samsung: i2s: add support for FSD I2S

[Padmanabhan Rajanbabu]

> -----Original Message-----
> From: Krzysztof Kozlowski [mailto:krzysztof.kozlowski@linaro.org]
> Sent: 03 January 2023 04:39 PM
> To: Padmanabhan Rajanbabu <p.rajanbabu@samsung.com>;
> lgirdwood@gmail.com; broonie@kernel.org; robh+dt@kernel.org;
> krzysztof.kozlowski+dt@linaro.org; s.nawrocki@samsung.com;
> perex@perex.cz; tiwai@suse.com; pankaj.dubey@samsung.com;
> alim.akhtar@samsung.com; rcsekar@samsung.com;
> aswani.reddy@samsung.com
> Cc: alsa-devel@alsa-project.org; devicetree@vger.kernel.org; linux-
> kernel@vger.kernel.org; linux-samsung-soc@vger.kernel.org
> Subject: Re: [PATCH v2 2/5] ASoC: samsung: i2s: add support for FSD I2S
> 
> On 03/01/2023 05:56, Padmanabhan Rajanbabu wrote:
> > Add support for enabling I2S controller on FSD platform.
> >
> > FSD I2S controller is based on Exynos7 I2S controller, supporting 2CH
> > playback/capture in I2S mode and 7.1CH playback/capture in TDM mode.
> >
> > Signed-off-by: Padmanabhan Rajanbabu <p.rajanbabu@samsung.com>
> > ---
> >  sound/soc/samsung/i2s-regs.h |  1 +
> >  sound/soc/samsung/i2s.c      | 57
> ++++++++++++++++++++++++++++++++++++
> >  2 files changed, 58 insertions(+)
> >
> > diff --git a/sound/soc/samsung/i2s-regs.h
> > b/sound/soc/samsung/i2s-regs.h index b4b5d6053503..4444c857d0c0
> 100644
> > --- a/sound/soc/samsung/i2s-regs.h
> > +++ b/sound/soc/samsung/i2s-regs.h
> > @@ -132,6 +132,7 @@
> >  #define EXYNOS7_MOD_RCLK_192FS	7
> >
> >  #define PSR_PSREN		(1 << 15)
> > +#define PSR_PSVAL(x)		(((x - 1) << 8) & 0x3f00)
> >
> >  #define FIC_TX2COUNT(x)		(((x) >>  24) & 0xf)
> >  #define FIC_TX1COUNT(x)		(((x) >>  16) & 0xf)
> > diff --git a/sound/soc/samsung/i2s.c b/sound/soc/samsung/i2s.c index
> > 9505200f3d11..dcb5c438cb2f 100644
> > --- a/sound/soc/samsung/i2s.c
> > +++ b/sound/soc/samsung/i2s.c
> > @@ -50,6 +50,10 @@ struct samsung_i2s_dai_data {
> >  	u32 quirks;
> >  	unsigned int pcm_rates;
> >  	const struct samsung_i2s_variant_regs *i2s_variant_regs;
> > +	void (*fixup_early)(struct snd_pcm_substream *substream,
> > +					struct snd_soc_dai *dai);
> > +	void (*fixup_late)(struct snd_pcm_substream *substream,
> > +					struct snd_soc_dai *dai);
> >  };
> >
> >  struct i2s_dai {
> > @@ -111,6 +115,10 @@ struct samsung_i2s_priv {
> >  	u32 suspend_i2spsr;
> >
> >  	const struct samsung_i2s_variant_regs *variant_regs;
> > +	void (*fixup_early)(struct snd_pcm_substream *substream,
> > +						struct snd_soc_dai *dai);
> > +	void (*fixup_late)(struct snd_pcm_substream *substream,
> > +						struct snd_soc_dai *dai);
> >  	u32 quirks;
> >
> >  	/* The clock provider's data */
> > @@ -940,6 +948,10 @@ static int i2s_trigger(struct snd_pcm_substream
> *substream,
> >  	case SNDRV_PCM_TRIGGER_RESUME:
> >  	case SNDRV_PCM_TRIGGER_PAUSE_RELEASE:
> >  		pm_runtime_get_sync(dai->dev);
> > +
> > +		if (priv->fixup_early)
> > +			priv->fixup_early(substream, dai);
> > +
> >  		spin_lock_irqsave(&priv->lock, flags);
> >
> >  		if (config_setup(i2s)) {
> > @@ -947,6 +959,13 @@ static int i2s_trigger(struct snd_pcm_substream
> *substream,
> >  			return -EINVAL;
> >  		}
> >
> 
> Except several warnings this patch generates, this is a bit surprising:
> 
> > +		spin_unlock_irqrestore(&priv->lock, flags);
> 
> You have critical section which you now break into two. You cannot do this
> usually. How the synchronization is now kept?
> 

The actual reason behind breaking the critical section is to allow the use
of already existing functions related to configuration of CDCLK and OPCLK
source.

Based on the review comments from previous patch-set related to the actual
need of custom sound card driver, we did migrate to simple-card, where
Exynos specific configurations like RCLKSRC selection, OPCLK and CDCLK
configuration is not being handled by the simple-card. To overcome this
scenario, fixups has been added during i2s_trigger to let Exynos users to
configure the Exynos I2S specific dividers and mux.

Rather than re-implementing the routines (or) configurations already
Available in the driver, the fixup functions can call the i2s_set_sysclk
and similar functions directly (which also takes the spin lock).

But we noticed that fixup_late implemented for FSD may not require
releasing of spinlock as it involves PSR configuration and will not cause
any harm if it is still kept inside the existing critical section only. I'll
update the patch to keep the fixup_late inside critical section and
will post the same in the next patch set.

> > +
> > +		if (priv->fixup_late)
> > +			priv->fixup_late(substream, dai);
> > +
> > +		spin_lock_irqsave(&priv->lock, flags);
> > +
> >  		if (capture)
> >  			i2s_rxctrl(i2s, 1);
> >  		else
> 
> Best regards,
> Krzysztof

Thanks,
Padmanabhan R.

RE: [PATCH v2 4/5] arm64: dts: fsd: Add codec node for Tesla FSD

[Padmanabhan Rajanbabu]

> -----Original Message-----
> From: Krzysztof Kozlowski [mailto:krzysztof.kozlowski@linaro.org]
> Sent: 03 January 2023 04:41 PM
> To: Padmanabhan Rajanbabu <p.rajanbabu@samsung.com>;
> lgirdwood@gmail.com; broonie@kernel.org; robh+dt@kernel.org;
> krzysztof.kozlowski+dt@linaro.org; s.nawrocki@samsung.com;
> perex@perex.cz; tiwai@suse.com; pankaj.dubey@samsung.com;
> alim.akhtar@samsung.com; rcsekar@samsung.com;
> aswani.reddy@samsung.com
> Cc: alsa-devel@alsa-project.org; devicetree@vger.kernel.org; linux-
> kernel@vger.kernel.org; linux-samsung-soc@vger.kernel.org
> Subject: Re: [PATCH v2 4/5] arm64: dts: fsd: Add codec node for Tesla FSD
> 
> On 03/01/2023 05:56, Padmanabhan Rajanbabu wrote:
> > Add device tree node support for codec on Tesla FSD platform.
> >
> > Signed-off-by: Padmanabhan Rajanbabu <p.rajanbabu@samsung.com>
> > ---
> >  arch/arm64/boot/dts/tesla/fsd-evb.dts | 13 +++++++++++++
> >  1 file changed, 13 insertions(+)
> >
> > diff --git a/arch/arm64/boot/dts/tesla/fsd-evb.dts
> > b/arch/arm64/boot/dts/tesla/fsd-evb.dts
> > index cf5f2ce4d2a7..e2fd49774f15 100644
> > --- a/arch/arm64/boot/dts/tesla/fsd-evb.dts
> > +++ b/arch/arm64/boot/dts/tesla/fsd-evb.dts
> > @@ -10,6 +10,7 @@
> >
> >  /dts-v1/;
> >  #include "fsd.dtsi"
> > +#include <dt-bindings/gpio/gpio.h>
> >
> >  / {
> >  	model = "Tesla Full Self-Driving (FSD) Evaluation board"; @@ -34,6
> > +35,18 @@
> >  	clock-frequency = <24000000>;
> >  };
> >
> > +&hsi2c_5 {
> > +	status = "okay";
> > +
> > +	tlv320aic3x: codec@18 {
> > +		#sound-dai-cells = <0>;
> > +		compatible = "ti,tlv320aic3104";
> > +		reg = <0x18>;
> 
> First compatible, then reg, then the rest.

Okay, I'll update the same in the next patch set.

> 
> > +		reset-gpios = <&gpg1 6 GPIO_ACTIVE_LOW>;
> > +		status = "okay";
> 
> It's by default. Why do you need it?

Okay, I understood that if there is no status entry, it is
treated as "okay". I'll update the same in the next
patch set.

> 
> Best regards,
> Krzysztof

Thanks,
Padmanabhan R.

RE: [PATCH v2 5/5] arm64: dts: fsd: Add sound card node for Tesla FSD

[Padmanabhan Rajanbabu]

> -----Original Message-----
> From: Krzysztof Kozlowski [mailto:krzysztof.kozlowski@linaro.org]
> Sent: 03 January 2023 04:43 PM
> To: Padmanabhan Rajanbabu <p.rajanbabu@samsung.com>;
> lgirdwood@gmail.com; broonie@kernel.org; robh+dt@kernel.org;
> krzysztof.kozlowski+dt@linaro.org; s.nawrocki@samsung.com;
> perex@perex.cz; tiwai@suse.com; pankaj.dubey@samsung.com;
> alim.akhtar@samsung.com; rcsekar@samsung.com;
> aswani.reddy@samsung.com
> Cc: alsa-devel@alsa-project.org; devicetree@vger.kernel.org; linux-
> kernel@vger.kernel.org; linux-samsung-soc@vger.kernel.org
> Subject: Re: [PATCH v2 5/5] arm64: dts: fsd: Add sound card node for Tesla
> FSD
> 
> On 03/01/2023 05:56, Padmanabhan Rajanbabu wrote:
> > Add device tree node support for sound card on Tesla FSD board
> >
> > Signed-off-by: Padmanabhan Rajanbabu <p.rajanbabu@samsung.com>
> > ---
> >  arch/arm64/boot/dts/tesla/fsd-evb.dts | 37
> > +++++++++++++++++++++++++++
> >  1 file changed, 37 insertions(+)
> >
> > diff --git a/arch/arm64/boot/dts/tesla/fsd-evb.dts
> > b/arch/arm64/boot/dts/tesla/fsd-evb.dts
> > index e2fd49774f15..ce726bddfb50 100644
> > --- a/arch/arm64/boot/dts/tesla/fsd-evb.dts
> > +++ b/arch/arm64/boot/dts/tesla/fsd-evb.dts
> > @@ -29,6 +29,43 @@
> >  		device_type = "memory";
> >  		reg = <0x0 0x80000000 0x2 0x00000000>;
> >  	};
> > +
> > +	sound {
> > +		compatible = "simple-audio-card";
> > +
> > +		#address-cells = <1>;
> > +		#size-cells = <0>;
> > +
> > +		simple-audio-card,name = "FSD Audio Card";
> > +		simple-audio-card,widgets =
> > +			"Line", "Line Out",
> 
> I don't think you need to break the line after '='.

Okay, will change the same in the next patch set.

> 
> > +			"Line", "Line In";
> > +		simple-audio-card,routing =
> > +			"Line Out", "LLOUT",
> > +			"Line Out", "RLOUT",
> > +			"MIC2L", "Line In",
> > +			"MIC2R", "Line In";
> > +
> > +		status = "okay";
> 
> Why?

Okay, I understood that if there is no status entry, it is
treated as "okay". I'll update the same in the next
patch set.

> 
> > +
> > +		simple-audio-card,dai-link@0 {
> > +			reg = <0>;
> > +			format = "i2s";
> > +			bitclock-master = <&tlv320aic3x>;
> > +			frame-master = <&tlv320aic3x>;
> > +
> > +			cpu0 {
> 
> Does not look like you tested the DTS against bindings. Please run `make
> dtbs_check` (see Documentation/devicetree/bindings/writing-schema.rst
> for instructions).

I did not encounter any error while running dtbs_check for fsd-evb.dts

This is the command I executed
make dtbs_check > output.txt 2>&1

dt-mk-schema --version: 2022.11

Please let me know if I'm missing anything here.

> 
> > +				sound-dai = <&i2s_0 0>;
> > +			};
> > +			cpu1 {
> > +				sound-dai = <&i2s_0 1>;
> > +			};
> > +			codec {
> > +				sound-dai = <&tlv320aic3x>;
> > +				system-clock-frequency = <33000000>;
> > +			};
> 
> Best regards,
> Krzysztof

Thanks,
Padmanabhan R.

RE: [PATCH v2 1/5] ASoC: dt-bindings: Add FSD I2S controller bindings

[Padmanabhan Rajanbabu]

> -----Original Message-----
> From: Mark Brown [mailto:broonie@kernel.org]
> Sent: 03 January 2023 11:35 PM
> To: Padmanabhan Rajanbabu <p.rajanbabu@samsung.com>
> Cc: lgirdwood@gmail.com; robh+dt@kernel.org;
> krzysztof.kozlowski+dt@linaro.org; s.nawrocki@samsung.com;
> perex@perex.cz; tiwai@suse.com; pankaj.dubey@samsung.com;
> alim.akhtar@samsung.com; rcsekar@samsung.com;
> aswani.reddy@samsung.com; alsa-devel@alsa-project.org;
> devicetree@vger.kernel.org; linux-kernel@vger.kernel.org; linux-samsung-
> soc@vger.kernel.org
> Subject: Re: [PATCH v2 1/5] ASoC: dt-bindings: Add FSD I2S controller
> bindings
> 
> On Tue, Jan 03, 2023 at 10:26:09AM +0530, Padmanabhan Rajanbabu wrote:
> 
> > +
> > +      tesla,fsd-i2s: with all the available features of Exynos7 I2S,
> > +      supporting only stereo channel playback and capture.
> 
> The driver claims that 7.1 is also supported.

For I2S mode, data lines for channel 3 to 6 are not routed out of the SoC.
i.e FSD SoC has only SDO0 line routed out. SDO1 and 2 are floating pins. 

So Exynos7-I2S in FSD SoC supports only stereo playback and capture in
I2S mode. I'll reword the description in the next patch set to reflect the
above information.

Thanks,
Padmanabhan R.

RE: [PATCH v2 2/5] ASoC: samsung: i2s: add support for FSD I2S

[Padmanabhan Rajanbabu]

> -----Original Message-----
> From: Mark Brown [mailto:broonie@kernel.org]
> Sent: 03 January 2023 11:39 PM
> To: Padmanabhan Rajanbabu <p.rajanbabu@samsung.com>
> Cc: lgirdwood@gmail.com; robh+dt@kernel.org;
> krzysztof.kozlowski+dt@linaro.org; s.nawrocki@samsung.com;
> perex@perex.cz; tiwai@suse.com; pankaj.dubey@samsung.com;
> alim.akhtar@samsung.com; rcsekar@samsung.com;
> aswani.reddy@samsung.com; alsa-devel@alsa-project.org;
> devicetree@vger.kernel.org; linux-kernel@vger.kernel.org; linux-samsung-
> soc@vger.kernel.org
> Subject: Re: [PATCH v2 2/5] ASoC: samsung: i2s: add support for FSD I2S
> 
> On Tue, Jan 03, 2023 at 10:26:10AM +0530, Padmanabhan Rajanbabu wrote:
> 
> > +void fsd_i2s_fixup_early(struct snd_pcm_substream *substream,
> > +		struct snd_soc_dai *dai)
> > +{
> > +	struct snd_soc_pcm_runtime *rtd =
> asoc_substream_to_rtd(substream);
> > +	struct i2s_dai *i2s = to_info(asoc_rtd_to_cpu(rtd, 0));
> > +	struct i2s_dai *other = get_other_dai(i2s);
> > +
> > +	if (!is_opened(other)) {
> > +		i2s_set_sysclk(dai, SAMSUNG_I2S_CDCLK, 192,
> SND_SOC_CLOCK_OUT);
> > +		i2s_set_sysclk(dai, SAMSUNG_I2S_OPCLK, 0,
> MOD_OPCLK_PCLK);
> > +	}
> > +}
> 
> This looks like we're just hard coding to 192kHz?

Not actually. The value 192 being passed is for the RFS divider
based on which the Root clock source is divided to generate bit-clock
and frame-clock in master mode.

But, FSD SoC is utilizing the Exynos7-I2S controller in slave
mode, where bit-clock and frame-clock is sourced by the codec.
Therefore the sampling of data happens with codec clock source and
not based on the clock source from RCLK. However, we still need RFS and
BFS configured to default value for the proper operation of the controller.

The current operation being performed above is to change the Codec
clock direction to "out", so that codec will use this clock source to
generate bit clock and frame clock from its own PLL.

I'll make the changes in the next patch set to pass 0 instead of 192 here,
so that RFS and BFS will be configured to default value in config_setup
function.

Thanks,
Padmanabhan R.

Re: [PATCH v2 5/5] arm64: dts: fsd: Add sound card node for Tesla FSD

[Krzysztof Kozlowski]

On 09/01/2023 05:05, Padmanabhan Rajanbabu wrote:
> 
> 
>> -----Original Message-----
>> From: Krzysztof Kozlowski [mailto:krzysztof.kozlowski@linaro.org]
>> Sent: 03 January 2023 04:43 PM
>> To: Padmanabhan Rajanbabu <p.rajanbabu@samsung.com>;
>> lgirdwood@gmail.com; broonie@kernel.org; robh+dt@kernel.org;
>> krzysztof.kozlowski+dt@linaro.org; s.nawrocki@samsung.com;
>> perex@perex.cz; tiwai@suse.com; pankaj.dubey@samsung.com;
>> alim.akhtar@samsung.com; rcsekar@samsung.com;
>> aswani.reddy@samsung.com
>> Cc: alsa-devel@alsa-project.org; devicetree@vger.kernel.org; linux-
>> kernel@vger.kernel.org; linux-samsung-soc@vger.kernel.org
>> Subject: Re: [PATCH v2 5/5] arm64: dts: fsd: Add sound card node for Tesla
>> FSD
>>
>> On 03/01/2023 05:56, Padmanabhan Rajanbabu wrote:
>>> Add device tree node support for sound card on Tesla FSD board
>>>
>>> Signed-off-by: Padmanabhan Rajanbabu <p.rajanbabu@samsung.com>
>>> ---
>>>  arch/arm64/boot/dts/tesla/fsd-evb.dts | 37
>>> +++++++++++++++++++++++++++
>>>  1 file changed, 37 insertions(+)
>>>
>>> diff --git a/arch/arm64/boot/dts/tesla/fsd-evb.dts
>>> b/arch/arm64/boot/dts/tesla/fsd-evb.dts
>>> index e2fd49774f15..ce726bddfb50 100644
>>> --- a/arch/arm64/boot/dts/tesla/fsd-evb.dts
>>> +++ b/arch/arm64/boot/dts/tesla/fsd-evb.dts
>>> @@ -29,6 +29,43 @@
>>>  		device_type = "memory";
>>>  		reg = <0x0 0x80000000 0x2 0x00000000>;
>>>  	};
>>> +
>>> +	sound {
>>> +		compatible = "simple-audio-card";
>>> +
>>> +		#address-cells = <1>;
>>> +		#size-cells = <0>;
>>> +
>>> +		simple-audio-card,name = "FSD Audio Card";
>>> +		simple-audio-card,widgets =
>>> +			"Line", "Line Out",
>>
>> I don't think you need to break the line after '='.
> 
> Okay, will change the same in the next patch set.
> 
>>
>>> +			"Line", "Line In";
>>> +		simple-audio-card,routing =
>>> +			"Line Out", "LLOUT",
>>> +			"Line Out", "RLOUT",
>>> +			"MIC2L", "Line In",
>>> +			"MIC2R", "Line In";
>>> +
>>> +		status = "okay";
>>
>> Why?
> 
> Okay, I understood that if there is no status entry, it is
> treated as "okay". I'll update the same in the next
> patch set.
> 
>>
>>> +
>>> +		simple-audio-card,dai-link@0 {
>>> +			reg = <0>;
>>> +			format = "i2s";
>>> +			bitclock-master = <&tlv320aic3x>;
>>> +			frame-master = <&tlv320aic3x>;
>>> +
>>> +			cpu0 {
>>
>> Does not look like you tested the DTS against bindings. Please run `make
>> dtbs_check` (see Documentation/devicetree/bindings/writing-schema.rst
>> for instructions).
> 
> I did not encounter any error while running dtbs_check for fsd-evb.dts
> 
> This is the command I executed
> make dtbs_check > output.txt 2>&1
> 
> dt-mk-schema --version: 2022.11
> 
> Please let me know if I'm missing anything here.

You are right. I misread the pattern in simple-card.yaml. I'll fix the
file, so let's make these nodes: cpu-0 and cpu-1.

Best regards,
Krzysztof

RE: [PATCH v2 5/5] arm64: dts: fsd: Add sound card node for Tesla FSD

[Padmanabhan Rajanbabu]

> -----Original Message-----
> From: Krzysztof Kozlowski [mailto:krzysztof.kozlowski@linaro.org]
> Sent: 09 January 2023 01:58 PM
> To: Padmanabhan Rajanbabu <p.rajanbabu@samsung.com>;
> lgirdwood@gmail.com; broonie@kernel.org; robh+dt@kernel.org;
> krzysztof.kozlowski+dt@linaro.org; s.nawrocki@samsung.com;
> perex@perex.cz; tiwai@suse.com; pankaj.dubey@samsung.com;
> alim.akhtar@samsung.com; rcsekar@samsung.com;
> aswani.reddy@samsung.com
> Cc: alsa-devel@alsa-project.org; devicetree@vger.kernel.org; linux-
> kernel@vger.kernel.org; linux-samsung-soc@vger.kernel.org
> Subject: Re: [PATCH v2 5/5] arm64: dts: fsd: Add sound card node for Tesla
> FSD
> 
> On 09/01/2023 05:05, Padmanabhan Rajanbabu wrote:
> >
> >
> >> -----Original Message-----
> >> From: Krzysztof Kozlowski [mailto:krzysztof.kozlowski@linaro.org]
> >> Sent: 03 January 2023 04:43 PM
> >> To: Padmanabhan Rajanbabu <p.rajanbabu@samsung.com>;
> >> lgirdwood@gmail.com; broonie@kernel.org; robh+dt@kernel.org;
> >> krzysztof.kozlowski+dt@linaro.org; s.nawrocki@samsung.com;
> >> perex@perex.cz; tiwai@suse.com; pankaj.dubey@samsung.com;
> >> alim.akhtar@samsung.com; rcsekar@samsung.com;
> >> aswani.reddy@samsung.com
> >> Cc: alsa-devel@alsa-project.org; devicetree@vger.kernel.org; linux-
> >> kernel@vger.kernel.org; linux-samsung-soc@vger.kernel.org
> >> Subject: Re: [PATCH v2 5/5] arm64: dts: fsd: Add sound card node for
> >> Tesla FSD
> >>
> >> On 03/01/2023 05:56, Padmanabhan Rajanbabu wrote:
> >>> Add device tree node support for sound card on Tesla FSD board
> >>>
> >>> Signed-off-by: Padmanabhan Rajanbabu <p.rajanbabu@samsung.com>
> >>> ---
> >>>  arch/arm64/boot/dts/tesla/fsd-evb.dts | 37
> >>> +++++++++++++++++++++++++++
> >>>  1 file changed, 37 insertions(+)
> >>>
> >>> diff --git a/arch/arm64/boot/dts/tesla/fsd-evb.dts
> >>> b/arch/arm64/boot/dts/tesla/fsd-evb.dts
> >>> index e2fd49774f15..ce726bddfb50 100644
> >>> --- a/arch/arm64/boot/dts/tesla/fsd-evb.dts
> >>> +++ b/arch/arm64/boot/dts/tesla/fsd-evb.dts
> >>> @@ -29,6 +29,43 @@
> >>>  		device_type = "memory";
> >>>  		reg = <0x0 0x80000000 0x2 0x00000000>;
> >>>  	};
> >>> +
> >>> +	sound {
> >>> +		compatible = "simple-audio-card";
> >>> +
> >>> +		#address-cells = <1>;
> >>> +		#size-cells = <0>;
> >>> +
> >>> +		simple-audio-card,name = "FSD Audio Card";
> >>> +		simple-audio-card,widgets =
> >>> +			"Line", "Line Out",
> >>
> >> I don't think you need to break the line after '='.
> >
> > Okay, will change the same in the next patch set.
> >
> >>
> >>> +			"Line", "Line In";
> >>> +		simple-audio-card,routing =
> >>> +			"Line Out", "LLOUT",
> >>> +			"Line Out", "RLOUT",
> >>> +			"MIC2L", "Line In",
> >>> +			"MIC2R", "Line In";
> >>> +
> >>> +		status = "okay";
> >>
> >> Why?
> >
> > Okay, I understood that if there is no status entry, it is treated as
> > "okay". I'll update the same in the next patch set.
> >
> >>
> >>> +
> >>> +		simple-audio-card,dai-link@0 {
> >>> +			reg = <0>;
> >>> +			format = "i2s";
> >>> +			bitclock-master = <&tlv320aic3x>;
> >>> +			frame-master = <&tlv320aic3x>;
> >>> +
> >>> +			cpu0 {
> >>
> >> Does not look like you tested the DTS against bindings. Please run
> >> `make dtbs_check` (see
> >> Documentation/devicetree/bindings/writing-schema.rst
> >> for instructions).
> >
> > I did not encounter any error while running dtbs_check for fsd-evb.dts
> >
> > This is the command I executed
> > make dtbs_check > output.txt 2>&1
> >
> > dt-mk-schema --version: 2022.11
> >
> > Please let me know if I'm missing anything here.
> 
> You are right. I misread the pattern in simple-card.yaml. I'll fix the file, so let's
> make these nodes: cpu-0 and cpu-1.

Okay, Thank you for confirming, I'll change the node names to cpu-0 and cpu-1

> 
> Best regards,
> Krzysztof