[PATCH v2 0/7] ASoC: mt8188: add new board support

[PATCH v2 0/7] ASoC: mt8188: add new board support

[Trevor Wu]

In the series, we extend the capability of mt8188-mt6359 driver.

The following changes are included.
1. Divide ADDA BE dai into two dais for SOF.
2. Register hdmi/dp jack pins.
3. dai_fmt can be configured from device tree.
4. Add some I2S codecs support.

In addition, new compatible string "mediatek,mt8188-nau8825" is
included for a new board support.

Changes since v1:
  - add "Headphone Jack" for Nau8825
  - separate binding patch 2 from patch 6, and rename the subject
  - clean up debugfs for freed widget

Trevor Wu (7):
  ASoC: mediatek: mt8188: separate ADDA playback dai from capture dai
  ASoC: dt-bindings: mediatek,mt8188-mt6359: remove ADDA_BE from
    link-name
  ASoC: mediatek: mt8188-mt6359: register hdmi/dp jack pins
  ASoC: mediatek: common: soundcard driver add dai_fmt support
  ASoC: soc-dapm.c: clean up debugfs for freed widget
  ASoC: mediatek: mt8188-mt6359: support new board with nau88255
  ASoC: dt-bindings: mediatek,mt8188-mt6359: add NAU8825 support

 .../sound/mediatek,mt8188-mt6359.yaml         |  27 +-
 sound/soc/mediatek/Kconfig                    |   4 +
 .../mediatek/common/mtk-soundcard-driver.c    |  49 ++-
 sound/soc/mediatek/mt8188/mt8188-afe-common.h |   3 +-
 sound/soc/mediatek/mt8188/mt8188-dai-adda.c   |  73 ++--
 sound/soc/mediatek/mt8188/mt8188-mt6359.c     | 389 +++++++++++++++++-
 sound/soc/soc-dapm.c                          |  16 +
 7 files changed, 507 insertions(+), 54 deletions(-)

-- 
2.18.0

[PATCH v2 1/7] ASoC: mediatek: mt8188: separate ADDA playback dai from capture dai

[Trevor Wu]

MT8188 will support SOF. In SOF, be_hw_params_fixup callback are used to
configure BE hardware parameters. However, playback and capture stream
share the same callback function in which it can't know the stream type.

It's possible to require different paremters for playback and capture
stream, so separate them into two dais for SOF usage.

Signed-off-by: Trevor Wu <trevor.wu@mediatek.com>
---
 sound/soc/mediatek/mt8188/mt8188-afe-common.h |  3 +-
 sound/soc/mediatek/mt8188/mt8188-dai-adda.c   | 73 ++++++++++---------
 sound/soc/mediatek/mt8188/mt8188-mt6359.c     | 34 +++++++--
 3 files changed, 65 insertions(+), 45 deletions(-)

diff --git a/sound/soc/mediatek/mt8188/mt8188-afe-common.h b/sound/soc/mediatek/mt8188/mt8188-afe-common.h
index eb7e57c239bd..1304d685a306 100644
--- a/sound/soc/mediatek/mt8188/mt8188-afe-common.h
+++ b/sound/soc/mediatek/mt8188/mt8188-afe-common.h
@@ -39,7 +39,7 @@ enum {
 	MT8188_AFE_MEMIF_END,
 	MT8188_AFE_MEMIF_NUM = (MT8188_AFE_MEMIF_END - MT8188_AFE_MEMIF_START),
 	MT8188_AFE_IO_START = MT8188_AFE_MEMIF_END,
-	MT8188_AFE_IO_ADDA = MT8188_AFE_IO_START,
+	MT8188_AFE_IO_DL_SRC = MT8188_AFE_IO_START,
 	MT8188_AFE_IO_DMIC_IN,
 	MT8188_AFE_IO_DPTX,
 	MT8188_AFE_IO_ETDM_START,
@@ -52,6 +52,7 @@ enum {
 	MT8188_AFE_IO_ETDM_NUM =
 		(MT8188_AFE_IO_ETDM_END - MT8188_AFE_IO_ETDM_START),
 	MT8188_AFE_IO_PCM = MT8188_AFE_IO_ETDM_END,
+	MT8188_AFE_IO_UL_SRC,
 	MT8188_AFE_IO_END,
 	MT8188_AFE_IO_NUM = (MT8188_AFE_IO_END - MT8188_AFE_IO_START),
 	MT8188_DAI_END = MT8188_AFE_IO_END,
diff --git a/sound/soc/mediatek/mt8188/mt8188-dai-adda.c b/sound/soc/mediatek/mt8188/mt8188-dai-adda.c
index fed9f927e623..7dc029f2b428 100644
--- a/sound/soc/mediatek/mt8188/mt8188-dai-adda.c
+++ b/sound/soc/mediatek/mt8188/mt8188-dai-adda.c
@@ -53,8 +53,7 @@ enum {
 };
 
 struct mtk_dai_adda_priv {
-	unsigned int dl_rate;
-	unsigned int ul_rate;
+	bool hires_required;
 };
 
 static unsigned int afe_adda_dl_rate_transform(struct mtk_base_afe *afe,
@@ -241,42 +240,35 @@ static int mtk_adda_ul_event(struct snd_soc_dapm_widget *w,
 	return 0;
 }
 
-static int mtk_afe_adc_hires_connect(struct snd_soc_dapm_widget *source,
-				     struct snd_soc_dapm_widget *sink)
+static struct mtk_dai_adda_priv *get_adda_priv_by_name(struct mtk_base_afe *afe,
+						       const char *name)
 {
-	struct snd_soc_dapm_widget *w = source;
-	struct snd_soc_component *cmpnt = snd_soc_dapm_to_component(w->dapm);
-	struct mtk_base_afe *afe = snd_soc_component_get_drvdata(cmpnt);
 	struct mt8188_afe_private *afe_priv = afe->platform_priv;
-	struct mtk_dai_adda_priv *adda_priv;
-
-	adda_priv = afe_priv->dai_priv[MT8188_AFE_IO_ADDA];
 
-	if (!adda_priv) {
-		dev_err(afe->dev, "%s adda_priv == NULL", __func__);
-		return 0;
-	}
-
-	return !!(adda_priv->ul_rate > ADDA_HIRES_THRES);
+	if (strstr(name, "aud_adc_hires"))
+		return afe_priv->dai_priv[MT8188_AFE_IO_UL_SRC];
+	else if (strstr(name, "aud_dac_hires"))
+		return afe_priv->dai_priv[MT8188_AFE_IO_DL_SRC];
+	else
+		return NULL;
 }
 
-static int mtk_afe_dac_hires_connect(struct snd_soc_dapm_widget *source,
-				     struct snd_soc_dapm_widget *sink)
+static int mtk_afe_adda_hires_connect(struct snd_soc_dapm_widget *source,
+				      struct snd_soc_dapm_widget *sink)
 {
 	struct snd_soc_dapm_widget *w = source;
 	struct snd_soc_component *cmpnt = snd_soc_dapm_to_component(w->dapm);
 	struct mtk_base_afe *afe = snd_soc_component_get_drvdata(cmpnt);
-	struct mt8188_afe_private *afe_priv = afe->platform_priv;
 	struct mtk_dai_adda_priv *adda_priv;
 
-	adda_priv = afe_priv->dai_priv[MT8188_AFE_IO_ADDA];
+	adda_priv = get_adda_priv_by_name(afe, w->name);
 
 	if (!adda_priv) {
-		dev_err(afe->dev, "%s adda_priv == NULL", __func__);
+		dev_dbg(afe->dev, "adda_priv == NULL");
 		return 0;
 	}
 
-	return !!(adda_priv->dl_rate > ADDA_HIRES_THRES);
+	return (adda_priv->hires_required) ? 1 : 0;
 }
 
 static const struct snd_kcontrol_new mtk_dai_adda_o176_mix[] = {
@@ -361,7 +353,7 @@ static const struct snd_soc_dapm_route mtk_dai_adda_routes[] = {
 	{"ADDA Capture", NULL, "ADDA Capture Enable"},
 	{"ADDA Capture", NULL, "ADDA_MTKAIF_CFG"},
 	{"ADDA Capture", NULL, "aud_adc"},
-	{"ADDA Capture", NULL, "aud_adc_hires", mtk_afe_adc_hires_connect},
+	{"ADDA Capture", NULL, "aud_adc_hires", mtk_afe_adda_hires_connect},
 
 	{"I168", NULL, "ADDA Capture"},
 	{"I169", NULL, "ADDA Capture"},
@@ -369,7 +361,7 @@ static const struct snd_soc_dapm_route mtk_dai_adda_routes[] = {
 	{"ADDA Playback", NULL, "ADDA Enable"},
 	{"ADDA Playback", NULL, "ADDA Playback Enable"},
 	{"ADDA Playback", NULL, "aud_dac"},
-	{"ADDA Playback", NULL, "aud_dac_hires", mtk_afe_dac_hires_connect},
+	{"ADDA Playback", NULL, "aud_dac_hires", mtk_afe_adda_hires_connect},
 
 	{"DL_GAIN", NULL, "O176"},
 	{"DL_GAIN", NULL, "O177"},
@@ -503,13 +495,12 @@ static int mtk_dai_adda_hw_params(struct snd_pcm_substream *substream,
 	dev_dbg(afe->dev, "%s(), id %d, stream %d, rate %u\n",
 		__func__, id, substream->stream, rate);
 
-	if (substream->stream == SNDRV_PCM_STREAM_PLAYBACK) {
-		adda_priv->dl_rate = rate;
+	adda_priv->hires_required = (rate > ADDA_HIRES_THRES);
+
+	if (substream->stream == SNDRV_PCM_STREAM_PLAYBACK)
 		ret = mtk_dai_da_configure(afe, rate, id);
-	} else {
-		adda_priv->ul_rate = rate;
+	else
 		ret = mtk_dai_ad_configure(afe, rate, id);
-	}
 
 	return ret;
 }
@@ -536,8 +527,8 @@ static const struct snd_soc_dai_ops mtk_dai_adda_ops = {
 
 static struct snd_soc_dai_driver mtk_dai_adda_driver[] = {
 	{
-		.name = "ADDA",
-		.id = MT8188_AFE_IO_ADDA,
+		.name = "DL_SRC",
+		.id = MT8188_AFE_IO_DL_SRC,
 		.playback = {
 			.stream_name = "ADDA Playback",
 			.channels_min = 1,
@@ -545,6 +536,11 @@ static struct snd_soc_dai_driver mtk_dai_adda_driver[] = {
 			.rates = MTK_ADDA_PLAYBACK_RATES,
 			.formats = MTK_ADDA_FORMATS,
 		},
+		.ops = &mtk_dai_adda_ops,
+	},
+	{
+		.name = "UL_SRC",
+		.id = MT8188_AFE_IO_UL_SRC,
 		.capture = {
 			.stream_name = "ADDA Capture",
 			.channels_min = 1,
@@ -560,13 +556,18 @@ static int init_adda_priv_data(struct mtk_base_afe *afe)
 {
 	struct mt8188_afe_private *afe_priv = afe->platform_priv;
 	struct mtk_dai_adda_priv *adda_priv;
+	int adda_dai_list[] = {MT8188_AFE_IO_DL_SRC, MT8188_AFE_IO_UL_SRC};
+	int i;
 
-	adda_priv = devm_kzalloc(afe->dev, sizeof(struct mtk_dai_adda_priv),
-				 GFP_KERNEL);
-	if (!adda_priv)
-		return -ENOMEM;
+	for (i = 0; i < ARRAY_SIZE(adda_dai_list); i++) {
+		adda_priv = devm_kzalloc(afe->dev,
+					 sizeof(struct mtk_dai_adda_priv),
+					 GFP_KERNEL);
+		if (!adda_priv)
+			return -ENOMEM;
 
-	afe_priv->dai_priv[MT8188_AFE_IO_ADDA] = adda_priv;
+		afe_priv->dai_priv[adda_dai_list[i]] = adda_priv;
+	}
 
 	return 0;
 }
diff --git a/sound/soc/mediatek/mt8188/mt8188-mt6359.c b/sound/soc/mediatek/mt8188/mt8188-mt6359.c
index 919d74ea1934..833bc362dad2 100644
--- a/sound/soc/mediatek/mt8188/mt8188-mt6359.c
+++ b/sound/soc/mediatek/mt8188/mt8188-mt6359.c
@@ -99,8 +99,8 @@ SND_SOC_DAILINK_DEFS(capture10,
 		     DAILINK_COMP_ARRAY(COMP_EMPTY()));
 
 /* BE */
-SND_SOC_DAILINK_DEFS(adda,
-		     DAILINK_COMP_ARRAY(COMP_CPU("ADDA")),
+SND_SOC_DAILINK_DEFS(dl_src,
+		     DAILINK_COMP_ARRAY(COMP_CPU("DL_SRC")),
 		     DAILINK_COMP_ARRAY(COMP_CODEC("mt6359-sound",
 						   "mt6359-snd-codec-aif1")),
 		     DAILINK_COMP_ARRAY(COMP_EMPTY()));
@@ -140,6 +140,12 @@ SND_SOC_DAILINK_DEFS(pcm1,
 		     DAILINK_COMP_ARRAY(COMP_DUMMY()),
 		     DAILINK_COMP_ARRAY(COMP_EMPTY()));
 
+SND_SOC_DAILINK_DEFS(ul_src,
+		     DAILINK_COMP_ARRAY(COMP_CPU("UL_SRC")),
+		     DAILINK_COMP_ARRAY(COMP_CODEC("mt6359-sound",
+						   "mt6359-snd-codec-aif1")),
+		     DAILINK_COMP_ARRAY(COMP_EMPTY()));
+
 struct mt8188_mt6359_priv {
 	struct snd_soc_jack dp_jack;
 	struct snd_soc_jack hdmi_jack;
@@ -345,7 +351,7 @@ enum {
 	DAI_LINK_UL8_FE,
 	DAI_LINK_UL9_FE,
 	DAI_LINK_UL10_FE,
-	DAI_LINK_ADDA_BE,
+	DAI_LINK_DL_SRC_BE,
 	DAI_LINK_DPTX_BE,
 	DAI_LINK_ETDM1_IN_BE,
 	DAI_LINK_ETDM2_IN_BE,
@@ -353,6 +359,7 @@ enum {
 	DAI_LINK_ETDM2_OUT_BE,
 	DAI_LINK_ETDM3_OUT_BE,
 	DAI_LINK_PCM1_BE,
+	DAI_LINK_UL_SRC_BE,
 };
 
 static int mt8188_dptx_hw_params(struct snd_pcm_substream *substream,
@@ -604,13 +611,11 @@ static struct snd_soc_dai_link mt8188_mt6359_dai_links[] = {
 		SND_SOC_DAILINK_REG(capture10),
 	},
 	/* BE */
-	[DAI_LINK_ADDA_BE] = {
-		.name = "ADDA_BE",
+	[DAI_LINK_DL_SRC_BE] = {
+		.name = "DL_SRC_BE",
 		.no_pcm = 1,
 		.dpcm_playback = 1,
-		.dpcm_capture = 1,
-		.init = mt8188_mt6359_init,
-		SND_SOC_DAILINK_REG(adda),
+		SND_SOC_DAILINK_REG(dl_src),
 	},
 	[DAI_LINK_DPTX_BE] = {
 		.name = "DPTX_BE",
@@ -676,6 +681,12 @@ static struct snd_soc_dai_link mt8188_mt6359_dai_links[] = {
 		.dpcm_capture = 1,
 		SND_SOC_DAILINK_REG(pcm1),
 	},
+	[DAI_LINK_UL_SRC_BE] = {
+		.name = "UL_SRC_BE",
+		.no_pcm = 1,
+		.dpcm_capture = 1,
+		SND_SOC_DAILINK_REG(ul_src),
+	},
 };
 
 static struct snd_soc_card mt8188_mt6359_soc_card = {
@@ -695,6 +706,7 @@ static int mt8188_mt6359_dev_probe(struct platform_device *pdev)
 	struct mt8188_mt6359_priv *priv;
 	struct mt8188_card_data *card_data;
 	struct snd_soc_dai_link *dai_link;
+	bool init_mt6359 = false;
 	int ret, i;
 
 	card_data = (struct mt8188_card_data *)of_device_get_match_data(&pdev->dev);
@@ -739,6 +751,12 @@ static int mt8188_mt6359_dev_probe(struct platform_device *pdev)
 		} else if (strcmp(dai_link->name, "ETDM3_OUT_BE") == 0) {
 			if (strcmp(dai_link->codecs->dai_name, "snd-soc-dummy-dai"))
 				dai_link->init = mt8188_hdmi_codec_init;
+		} else if (strcmp(dai_link->name, "DL_SRC_BE") == 0 ||
+			   strcmp(dai_link->name, "UL_SRC_BE") == 0) {
+			if (!init_mt6359) {
+				dai_link->init = mt8188_mt6359_init;
+				init_mt6359 = true;
+			}
 		}
 	}
 
-- 
2.18.0

Re: [PATCH v2 1/7] ASoC: mediatek: mt8188: separate ADDA playback dai from capture dai

[Alexandre Mergnat]

On 23/05/2023 04:19, Trevor Wu wrote:
> MT8188 will support SOF. In SOF, be_hw_params_fixup callback are used to

Can you write what is SOF please ? (Sound Open Firmware I guess ?)

> configure BE hardware parameters. However, playback and capture stream
> share the same callback function in which it can't know the stream type.
> 
> It's possible to require different paremters for playback and capture

paremters => parameters

Code looks good, after the commit messages fixes:

Reviewed-by: Alexandre Mergnat <amergnat@baylibre.com>

-- 
Regards,
Alexandre

Re: [PATCH v2 1/7] ASoC: mediatek: mt8188: separate ADDA playback dai from capture dai

[Trevor Wu (吳文良)]

On Tue, 2023-05-23 at 16:22 +0200, Alexandre Mergnat wrote:
> External email : Please do not click links or open attachments until
> you have verified the sender or the content.
> 
> 
> On 23/05/2023 04:19, Trevor Wu wrote:
> > MT8188 will support SOF. In SOF, be_hw_params_fixup callback are
> > used to
> 
> Can you write what is SOF please ? (Sound Open Firmware I guess ?)
> 
> > configure BE hardware parameters. However, playback and capture
> > stream
> > share the same callback function in which it can't know the stream
> > type.
> > 
> > It's possible to require different paremters for playback and
> > capture
> 
> paremters => parameters
> 
> Code looks good, after the commit messages fixes:
> 
> Reviewed-by: Alexandre Mergnat <amergnat@baylibre.com>
> 
> --
> Regards,
> Alexandre
> 
Hi Alexandre,

Thank you for pointing out my mistake.
I will correct it in v3.

Thanks,
Trevor

[PATCH v2 2/7] ASoC: dt-bindings: mediatek,mt8188-mt6359: remove ADDA_BE from link-name

[Trevor Wu]

ADDA_BE is used to connect to mt6359. For machine mt8188-mt6359, codec
for ADDA_BE must be mt6359 which are configured on the machine driver.
Besides, ADDA_BE is divided into two dais, UL_SRC_BE and DL_SRC_BE.
As a result, remove ADDA_BE from items of link-name.

Signed-off-by: Trevor Wu <trevor.wu@mediatek.com>
---
 .../devicetree/bindings/sound/mediatek,mt8188-mt6359.yaml        | 1 -
 1 file changed, 1 deletion(-)

diff --git a/Documentation/devicetree/bindings/sound/mediatek,mt8188-mt6359.yaml b/Documentation/devicetree/bindings/sound/mediatek,mt8188-mt6359.yaml
index 6640272b3f4f..3d2c01b693be 100644
--- a/Documentation/devicetree/bindings/sound/mediatek,mt8188-mt6359.yaml
+++ b/Documentation/devicetree/bindings/sound/mediatek,mt8188-mt6359.yaml
@@ -42,7 +42,6 @@ patternProperties:
           we are going to update parameters in this node.
         items:
           enum:
-            - ADDA_BE
             - DPTX_BE
             - ETDM1_IN_BE
             - ETDM2_IN_BE
-- 
2.18.0

Re: [PATCH v2 2/7] ASoC: dt-bindings: mediatek,mt8188-mt6359: remove ADDA_BE from link-name

[Alexandre Mergnat]

On 23/05/2023 04:19, Trevor Wu wrote:
> ADDA_BE is used to connect to mt6359. For machine mt8188-mt6359, codec
> for ADDA_BE must be mt6359 which are configured on the machine driver.
> Besides, ADDA_BE is divided into two dais, UL_SRC_BE and DL_SRC_BE.
> As a result, remove ADDA_BE from items of link-name.
> 
> Signed-off-by: Trevor Wu<trevor.wu@mediatek.com>

I don't understand how "DL_SRC_BE" and "UL_SRC_BE" links are done.
Why these dais don't replace "ADDA_BE" in this binding ?

Regards,
Alexandre

Re: [PATCH v2 2/7] ASoC: dt-bindings: mediatek,mt8188-mt6359: remove ADDA_BE from link-name

[Trevor Wu (吳文良)]

On Tue, 2023-05-23 at 18:26 +0200, Alexandre Mergnat wrote:
> On 23/05/2023 04:19, Trevor Wu wrote:
> > ADDA_BE is used to connect to mt6359. For machine mt8188-mt6359,
> > codec
> > for ADDA_BE must be mt6359 which are configured on the machine
> > driver.
> > Besides, ADDA_BE is divided into two dais, UL_SRC_BE and DL_SRC_BE.
> > As a result, remove ADDA_BE from items of link-name.
> > 
> > Signed-off-by: Trevor Wu<trevor.wu@mediatek.com>
> 
> I don't understand how "DL_SRC_BE" and "UL_SRC_BE" links are done.
> Why these dais don't replace "ADDA_BE" in this binding ?
> 
> Regards,
> Alexandre
> 

Hi Alexandre,

Because the sound card is mt8188-mt6359, the codec for these two links
must be mt6359. Thus, I specifiy the codec in machine driver directly.
If the codec is changed, there will be a new sound card and binding
file. In conclusion, the codec won't be updated via dts, and that's why
I don't just replace ADDA_BE in this binding.

Do you suggest me add some information in the commit message?

Thanks,
Trevor

Re: [PATCH v2 2/7] ASoC: dt-bindings: mediatek,mt8188-mt6359: remove ADDA_BE from link-name

[Alexandre Mergnat]

On 24/05/2023 04:25, Trevor Wu (吳文良) wrote:
> On Tue, 2023-05-23 at 18:26 +0200, Alexandre Mergnat wrote:
>> On 23/05/2023 04:19, Trevor Wu wrote:
>>> ADDA_BE is used to connect to mt6359. For machine mt8188-mt6359,
>>> codec
>>> for ADDA_BE must be mt6359 which are configured on the machine
>>> driver.
>>> Besides, ADDA_BE is divided into two dais, UL_SRC_BE and DL_SRC_BE.
>>> As a result, remove ADDA_BE from items of link-name.
>>>
>>> Signed-off-by: Trevor Wu<trevor.wu@mediatek.com>
>>
>> I don't understand how "DL_SRC_BE" and "UL_SRC_BE" links are done.
>> Why these dais don't replace "ADDA_BE" in this binding ?
>>
>> Regards,
>> Alexandre
>>
> 
> Hi Alexandre,
> 
> Because the sound card is mt8188-mt6359, the codec for these two links
> must be mt6359. Thus, I specifiy the codec in machine driver directly.
> If the codec is changed, there will be a new sound card and binding
> file. In conclusion, the codec won't be updated via dts, and that's why
> I don't just replace ADDA_BE in this binding.
> 
> Do you suggest me add some information in the commit message?

No it's fine, I'm just trying to understand.

When you say "I specifiy the codec in machine driver directly", you
are talking about this change ?

+		} else if (strcmp(dai_link->name, "DL_SRC_BE") == 0 ||
+			   strcmp(dai_link->name, "UL_SRC_BE") == 0) {
+			if (!init_mt6359) {
+				dai_link->init = mt8188_mt6359_init;

I'm asking because the equivalent was done here:

-	[DAI_LINK_ADDA_BE] = {
-		.name = "ADDA_BE",
+	[DAI_LINK_DL_SRC_BE] = {
+		.name = "DL_SRC_BE",
  		.no_pcm = 1,
  		.dpcm_playback = 1,
-		.dpcm_capture = 1,
-		.init = mt8188_mt6359_init,
-		SND_SOC_DAILINK_REG(adda),
+		SND_SOC_DAILINK_REG(dl_src),

So I'm wondering why "ADDA_BE" & "DPTX_BE" & "ETDM3_OUT_BE" are in the 
enum list of the binding since the codec is already specified in
machine driver too. I probably miss something but I don't know what.

-- 
Regards,
Alexandre

Re: [PATCH v2 2/7] ASoC: dt-bindings: mediatek,mt8188-mt6359: remove ADDA_BE from link-name

[Trevor Wu (吳文良)]

On Wed, 2023-05-24 at 15:28 +0200, Alexandre Mergnat wrote:
> External email : Please do not click links or open attachments until
> you have verified the sender or the content.
> 
> 
> On 24/05/2023 04:25, Trevor Wu (吳文良) wrote:
> > On Tue, 2023-05-23 at 18:26 +0200, Alexandre Mergnat wrote:
> > > On 23/05/2023 04:19, Trevor Wu wrote:
> > > > ADDA_BE is used to connect to mt6359. For machine mt8188-
> > > > mt6359,
> > > > codec
> > > > for ADDA_BE must be mt6359 which are configured on the machine
> > > > driver.
> > > > Besides, ADDA_BE is divided into two dais, UL_SRC_BE and
> > > > DL_SRC_BE.
> > > > As a result, remove ADDA_BE from items of link-name.
> > > > 
> > > > Signed-off-by: Trevor Wu<trevor.wu@mediatek.com>
> > > 
> > > I don't understand how "DL_SRC_BE" and "UL_SRC_BE" links are
> > > done.
> > > Why these dais don't replace "ADDA_BE" in this binding ?
> > > 
> > > Regards,
> > > Alexandre
> > > 
> > 
> > Hi Alexandre,
> > 
> > Because the sound card is mt8188-mt6359, the codec for these two
> > links
> > must be mt6359. Thus, I specifiy the codec in machine driver
> > directly.
> > If the codec is changed, there will be a new sound card and binding
> > file. In conclusion, the codec won't be updated via dts, and that's
> > why
> > I don't just replace ADDA_BE in this binding.
> > 
> > Do you suggest me add some information in the commit message?
> 
> No it's fine, I'm just trying to understand.
> 
> When you say "I specifiy the codec in machine driver directly", you
> are talking about this change ?
> 
> +               } else if (strcmp(dai_link->name, "DL_SRC_BE") == 0
> ||
> +                          strcmp(dai_link->name, "UL_SRC_BE") == 0)
> {
> +                       if (!init_mt6359) {
> +                               dai_link->init = mt8188_mt6359_init;
> 
> I'm asking because the equivalent was done here:
> 
> -       [DAI_LINK_ADDA_BE] = {
> -               .name = "ADDA_BE",
> +       [DAI_LINK_DL_SRC_BE] = {
> +               .name = "DL_SRC_BE",
>                 .no_pcm = 1,
>                 .dpcm_playback = 1,
> -               .dpcm_capture = 1,
> -               .init = mt8188_mt6359_init,
> -               SND_SOC_DAILINK_REG(adda),
> +               SND_SOC_DAILINK_REG(dl_src),
> 
> So I'm wondering why "ADDA_BE" & "DPTX_BE" & "ETDM3_OUT_BE" are in
> the
> enum list of the binding since the codec is already specified in
> machine driver too. I probably miss something but I don't know what.
> 
> 


The following code snippet is cut from [PATCH v2 1/7].

 /* BE */
-SND_SOC_DAILINK_DEFS(adda,
-                    DAILINK_COMP_ARRAY(COMP_CPU("ADDA")),
+SND_SOC_DAILINK_DEFS(dl_src,
+                    DAILINK_COMP_ARRAY(COMP_CPU("DL_SRC")),
                     DAILINK_COMP_ARRAY(COMP_CODEC("mt6359-sound",
                                                   "mt6359-snd-codec-
aif1")),
                     DAILINK_COMP_ARRAY(COMP_EMPTY()));
@@ -140,6 +140,12 @@ SND_SOC_DAILINK_DEFS(pcm1,
                     DAILINK_COMP_ARRAY(COMP_DUMMY()),
                     DAILINK_COMP_ARRAY(COMP_EMPTY()));
 
+SND_SOC_DAILINK_DEFS(ul_src,
+                    DAILINK_COMP_ARRAY(COMP_CPU("UL_SRC")),
+                    DAILINK_COMP_ARRAY(COMP_CODEC("mt6359-sound",
+                                                  "mt6359-snd-codec-
aif1")),
+                    DAILINK_COMP_ARRAY(COMP_EMPTY()));


This is why I talk about specifying the codec it connects in the
machine driver.
If you check other dai-links, you would see COMP_DUMMY() in the
COMP_CODEC() field.

Thanks,
Trevor
> 

Re: [PATCH v2 2/7] ASoC: dt-bindings: mediatek,mt8188-mt6359: remove ADDA_BE from link-name

[Alexandre Mergnat]

On 24/05/2023 15:45, Trevor Wu (吳文良) wrote:
> On Wed, 2023-05-24 at 15:28 +0200, Alexandre Mergnat wrote:
>> External email : Please do not click links or open attachments until
>> you have verified the sender or the content.
>>
>>
>> On 24/05/2023 04:25, Trevor Wu (吳文良) wrote:
>>> On Tue, 2023-05-23 at 18:26 +0200, Alexandre Mergnat wrote:
>>>> On 23/05/2023 04:19, Trevor Wu wrote:
>>>>> ADDA_BE is used to connect to mt6359. For machine mt8188-
>>>>> mt6359,
>>>>> codec
>>>>> for ADDA_BE must be mt6359 which are configured on the machine
>>>>> driver.
>>>>> Besides, ADDA_BE is divided into two dais, UL_SRC_BE and
>>>>> DL_SRC_BE.
>>>>> As a result, remove ADDA_BE from items of link-name.
>>>>>
>>>>> Signed-off-by: Trevor Wu<trevor.wu@mediatek.com>
>>>>
>>>> I don't understand how "DL_SRC_BE" and "UL_SRC_BE" links are
>>>> done.
>>>> Why these dais don't replace "ADDA_BE" in this binding ?
>>>>
>>>> Regards,
>>>> Alexandre
>>>>
>>>
>>> Hi Alexandre,
>>>
>>> Because the sound card is mt8188-mt6359, the codec for these two
>>> links
>>> must be mt6359. Thus, I specifiy the codec in machine driver
>>> directly.
>>> If the codec is changed, there will be a new sound card and binding
>>> file. In conclusion, the codec won't be updated via dts, and that's
>>> why
>>> I don't just replace ADDA_BE in this binding.
>>>
>>> Do you suggest me add some information in the commit message?
>>
>> No it's fine, I'm just trying to understand.
>>
>> When you say "I specifiy the codec in machine driver directly", you
>> are talking about this change ?
>>
>> +               } else if (strcmp(dai_link->name, "DL_SRC_BE") == 0
>> ||
>> +                          strcmp(dai_link->name, "UL_SRC_BE") == 0)
>> {
>> +                       if (!init_mt6359) {
>> +                               dai_link->init = mt8188_mt6359_init;
>>
>> I'm asking because the equivalent was done here:
>>
>> -       [DAI_LINK_ADDA_BE] = {
>> -               .name = "ADDA_BE",
>> +       [DAI_LINK_DL_SRC_BE] = {
>> +               .name = "DL_SRC_BE",
>>                  .no_pcm = 1,
>>                  .dpcm_playback = 1,
>> -               .dpcm_capture = 1,
>> -               .init = mt8188_mt6359_init,
>> -               SND_SOC_DAILINK_REG(adda),
>> +               SND_SOC_DAILINK_REG(dl_src),
>>
>> So I'm wondering why "ADDA_BE" & "DPTX_BE" & "ETDM3_OUT_BE" are in
>> the
>> enum list of the binding since the codec is already specified in
>> machine driver too. I probably miss something but I don't know what.
>>
>>
> 
> 
> The following code snippet is cut from [PATCH v2 1/7].
> 
>   /* BE */
> -SND_SOC_DAILINK_DEFS(adda,
> -                    DAILINK_COMP_ARRAY(COMP_CPU("ADDA")),
> +SND_SOC_DAILINK_DEFS(dl_src,
> +                    DAILINK_COMP_ARRAY(COMP_CPU("DL_SRC")),
>                       DAILINK_COMP_ARRAY(COMP_CODEC("mt6359-sound",
>                                                     "mt6359-snd-codec-
> aif1")),
>                       DAILINK_COMP_ARRAY(COMP_EMPTY()));
> @@ -140,6 +140,12 @@ SND_SOC_DAILINK_DEFS(pcm1,
>                       DAILINK_COMP_ARRAY(COMP_DUMMY()),
>                       DAILINK_COMP_ARRAY(COMP_EMPTY()));
>   
> +SND_SOC_DAILINK_DEFS(ul_src,
> +                    DAILINK_COMP_ARRAY(COMP_CPU("UL_SRC")),
> +                    DAILINK_COMP_ARRAY(COMP_CODEC("mt6359-sound",
> +                                                  "mt6359-snd-codec-
> aif1")),
> +                    DAILINK_COMP_ARRAY(COMP_EMPTY()));
> 
> 
> This is why I talk about specifying the codec it connects in the
> machine driver.
> If you check other dai-links, you would see COMP_DUMMY() in the
> COMP_CODEC() field.

Ok thanks for the explanation. If I understand well, ADDA_BE could have 
been removed from the enum list before your serie because the codec was 
already specified for ADDA_BE.

Reviewed-by: Alexandre Mergnat <amergnat@baylibre.com>

-- 
Regards,
Alexandre

Re: [PATCH v2 2/7] ASoC: dt-bindings: mediatek,mt8188-mt6359: remove ADDA_BE from link-name

[Trevor Wu (吳文良)]

On Wed, 2023-05-24 at 16:46 +0200, Alexandre Mergnat wrote:
> On 24/05/2023 15:45, Trevor Wu (吳文良) wrote:
> > On Wed, 2023-05-24 at 15:28 +0200, Alexandre Mergnat wrote:
> > > External email : Please do not click links or open attachments
> > > until
> > > you have verified the sender or the content.
> > > 
> > > 
> > > On 24/05/2023 04:25, Trevor Wu (吳文良) wrote:
> > > > On Tue, 2023-05-23 at 18:26 +0200, Alexandre Mergnat wrote:
> > > > > On 23/05/2023 04:19, Trevor Wu wrote:
> > > > > > ADDA_BE is used to connect to mt6359. For machine mt8188-
> > > > > > mt6359,
> > > > > > codec
> > > > > > for ADDA_BE must be mt6359 which are configured on the
> > > > > > machine
> > > > > > driver.
> > > > > > Besides, ADDA_BE is divided into two dais, UL_SRC_BE and
> > > > > > DL_SRC_BE.
> > > > > > As a result, remove ADDA_BE from items of link-name.
> > > > > > 
> > > > > > Signed-off-by: Trevor Wu<trevor.wu@mediatek.com>
> > > > > 
> > > > > I don't understand how "DL_SRC_BE" and "UL_SRC_BE" links are
> > > > > done.
> > > > > Why these dais don't replace "ADDA_BE" in this binding ?
> > > > > 
> > > > > Regards,
> > > > > Alexandre
> > > > > 
> > > > 
> > > > Hi Alexandre,
> > > > 
> > > > Because the sound card is mt8188-mt6359, the codec for these
> > > > two
> > > > links
> > > > must be mt6359. Thus, I specifiy the codec in machine driver
> > > > directly.
> > > > If the codec is changed, there will be a new sound card and
> > > > binding
> > > > file. In conclusion, the codec won't be updated via dts, and
> > > > that's
> > > > why
> > > > I don't just replace ADDA_BE in this binding.
> > > > 
> > > > Do you suggest me add some information in the commit message?
> > > 
> > > No it's fine, I'm just trying to understand.
> > > 
> > > When you say "I specifiy the codec in machine driver directly",
> > > you
> > > are talking about this change ?
> > > 
> > > +               } else if (strcmp(dai_link->name, "DL_SRC_BE") ==
> > > 0
> > > > > 
> > > 
> > > +                          strcmp(dai_link->name, "UL_SRC_BE") ==
> > > 0)
> > > {
> > > +                       if (!init_mt6359) {
> > > +                               dai_link->init =
> > > mt8188_mt6359_init;
> > > 
> > > I'm asking because the equivalent was done here:
> > > 
> > > -       [DAI_LINK_ADDA_BE] = {
> > > -               .name = "ADDA_BE",
> > > +       [DAI_LINK_DL_SRC_BE] = {
> > > +               .name = "DL_SRC_BE",
> > >                  .no_pcm = 1,
> > >                  .dpcm_playback = 1,
> > > -               .dpcm_capture = 1,
> > > -               .init = mt8188_mt6359_init,
> > > -               SND_SOC_DAILINK_REG(adda),
> > > +               SND_SOC_DAILINK_REG(dl_src),
> > > 
> > > So I'm wondering why "ADDA_BE" & "DPTX_BE" & "ETDM3_OUT_BE" are
> > > in
> > > the
> > > enum list of the binding since the codec is already specified in
> > > machine driver too. I probably miss something but I don't know
> > > what.
> > > 
> > > 
> > 
> > 
> > The following code snippet is cut from [PATCH v2 1/7].
> > 
> >   /* BE */
> > -SND_SOC_DAILINK_DEFS(adda,
> > -                    DAILINK_COMP_ARRAY(COMP_CPU("ADDA")),
> > +SND_SOC_DAILINK_DEFS(dl_src,
> > +                    DAILINK_COMP_ARRAY(COMP_CPU("DL_SRC")),
> >                       DAILINK_COMP_ARRAY(COMP_CODEC("mt6359-sound",
> >                                                     "mt6359-snd-
> > codec-
> > aif1")),
> >                       DAILINK_COMP_ARRAY(COMP_EMPTY()));
> > @@ -140,6 +140,12 @@ SND_SOC_DAILINK_DEFS(pcm1,
> >                       DAILINK_COMP_ARRAY(COMP_DUMMY()),
> >                       DAILINK_COMP_ARRAY(COMP_EMPTY()));
> > 
> > +SND_SOC_DAILINK_DEFS(ul_src,
> > +                    DAILINK_COMP_ARRAY(COMP_CPU("UL_SRC")),
> > +                    DAILINK_COMP_ARRAY(COMP_CODEC("mt6359-sound",
> > +                                                  "mt6359-snd-
> > codec-
> > aif1")),
> > +                    DAILINK_COMP_ARRAY(COMP_EMPTY()));
> > 
> > 
> > This is why I talk about specifying the codec it connects in the
> > machine driver.
> > If you check other dai-links, you would see COMP_DUMMY() in the
> > COMP_CODEC() field.
> 
> Ok thanks for the explanation. If I understand well, ADDA_BE could
> have
> been removed from the enum list before your serie because the codec
> was
> already specified for ADDA_BE.
> 
> Reviewed-by: Alexandre Mergnat <amergnat@baylibre.com>
> 


Actually, it's allowed to be overwritten by DTS configuration. That's
why I listed ADDA_BE in the enum list before. However, I found the
binding and machine driver was implemented for mt6359, so I think it's
more reasonable to remove it from the list.

Thanks,
Trevor

[PATCH v2 3/7] ASoC: mediatek: mt8188-mt6359: register hdmi/dp jack pins

[Trevor Wu]

Some userspace applications need jack control events, so register hdmi
and dp jack pins to activate jack control events.

Signed-off-by: Trevor Wu <trevor.wu@mediatek.com>
---
 sound/soc/mediatek/mt8188/mt8188-mt6359.c | 27 +++++++++++++++++++----
 1 file changed, 23 insertions(+), 4 deletions(-)

diff --git a/sound/soc/mediatek/mt8188/mt8188-mt6359.c b/sound/soc/mediatek/mt8188/mt8188-mt6359.c
index 833bc362dad2..6c3f36e2fffd 100644
--- a/sound/soc/mediatek/mt8188/mt8188-mt6359.c
+++ b/sound/soc/mediatek/mt8188/mt8188-mt6359.c
@@ -151,6 +151,20 @@ struct mt8188_mt6359_priv {
 	struct snd_soc_jack hdmi_jack;
 };
 
+static struct snd_soc_jack_pin mt8188_hdmi_jack_pins[] = {
+	{
+		.pin = "HDMI",
+		.mask = SND_JACK_LINEOUT,
+	},
+};
+
+static struct snd_soc_jack_pin mt8188_dp_jack_pins[] = {
+	{
+		.pin = "DP",
+		.mask = SND_JACK_LINEOUT,
+	},
+};
+
 struct mt8188_card_data {
 	const char *name;
 	unsigned long quirk;
@@ -159,6 +173,8 @@ struct mt8188_card_data {
 static const struct snd_soc_dapm_widget mt8188_mt6359_widgets[] = {
 	SND_SOC_DAPM_HP("Headphone", NULL),
 	SND_SOC_DAPM_MIC("Headset Mic", NULL),
+	SND_SOC_DAPM_SINK("HDMI"),
+	SND_SOC_DAPM_SINK("DP"),
 };
 
 static const struct snd_kcontrol_new mt8188_mt6359_controls[] = {
@@ -396,8 +412,10 @@ static int mt8188_hdmi_codec_init(struct snd_soc_pcm_runtime *rtd)
 	struct snd_soc_component *component = asoc_rtd_to_codec(rtd, 0)->component;
 	int ret = 0;
 
-	ret = snd_soc_card_jack_new(rtd->card, "HDMI Jack", SND_JACK_LINEOUT,
-				    &priv->hdmi_jack);
+	ret = snd_soc_card_jack_new_pins(rtd->card, "HDMI Jack",
+					 SND_JACK_LINEOUT, &priv->hdmi_jack,
+					 mt8188_hdmi_jack_pins,
+					 ARRAY_SIZE(mt8188_hdmi_jack_pins));
 	if (ret) {
 		dev_info(rtd->dev, "%s, new jack failed: %d\n", __func__, ret);
 		return ret;
@@ -417,8 +435,9 @@ static int mt8188_dptx_codec_init(struct snd_soc_pcm_runtime *rtd)
 	struct snd_soc_component *component = asoc_rtd_to_codec(rtd, 0)->component;
 	int ret = 0;
 
-	ret = snd_soc_card_jack_new(rtd->card, "DP Jack", SND_JACK_LINEOUT,
-				    &priv->dp_jack);
+	ret = snd_soc_card_jack_new_pins(rtd->card, "DP Jack", SND_JACK_LINEOUT,
+					 &priv->dp_jack, mt8188_dp_jack_pins,
+					 ARRAY_SIZE(mt8188_dp_jack_pins));
 	if (ret) {
 		dev_info(rtd->dev, "%s, new jack failed: %d\n", __func__, ret);
 		return ret;
-- 
2.18.0

Re: [PATCH v2 3/7] ASoC: mediatek: mt8188-mt6359: register hdmi/dp jack pins

[Alexandre Mergnat]

On 23/05/2023 04:19, Trevor Wu wrote:
> Some userspace applications need jack control events, so register hdmi
> and dp jack pins to activate jack control events.
> 
> Signed-off-by: Trevor Wu<trevor.wu@mediatek.com>

Reviewed-by: Alexandre Mergnat <amergnat@baylibre.com>

-- 
Regards,
Alexandre

[PATCH v2 4/7] ASoC: mediatek: common: soundcard driver add dai_fmt support

[Trevor Wu]

There are two changes included in the patch.

First, add set_dailink_daifmt() function, so dai_fmt can be updated by
the configuration in dai-link sub node.

Second, remove codec phandle from required property in dai-link sub node.
For example, user possibly needs to update dai-format for all etdm
co-clock dai-links, but codec doesn't need to be specified in capture
dai-link for a speaker amp.

Signed-off-by: Trevor Wu <trevor.wu@mediatek.com>
---
 .../mediatek/common/mtk-soundcard-driver.c    | 49 ++++++++++++++++++-
 1 file changed, 48 insertions(+), 1 deletion(-)

diff --git a/sound/soc/mediatek/common/mtk-soundcard-driver.c b/sound/soc/mediatek/common/mtk-soundcard-driver.c
index 738093451ccb..5e291092046b 100644
--- a/sound/soc/mediatek/common/mtk-soundcard-driver.c
+++ b/sound/soc/mediatek/common/mtk-soundcard-driver.c
@@ -22,7 +22,7 @@ static int set_card_codec_info(struct snd_soc_card *card,
 
 	codec_node = of_get_child_by_name(sub_node, "codec");
 	if (!codec_node)
-		return -EINVAL;
+		return 0;
 
 	/* set card codec info */
 	ret = snd_soc_of_get_dai_link_codecs(dev, codec_node, dai_link);
@@ -36,6 +36,47 @@ static int set_card_codec_info(struct snd_soc_card *card,
 	return 0;
 }
 
+static int set_dailink_daifmt(struct snd_soc_card *card,
+			      struct device_node *sub_node,
+			      struct snd_soc_dai_link *dai_link)
+{
+	unsigned int daifmt;
+	const char *str;
+	int ret;
+	struct {
+		char *name;
+		unsigned int val;
+	} of_clk_table[] = {
+		{ "cpu",	SND_SOC_DAIFMT_CBC_CFC },
+		{ "codec",	SND_SOC_DAIFMT_CBP_CFP },
+	};
+
+	daifmt = snd_soc_daifmt_parse_format(sub_node, NULL);
+	if (daifmt) {
+		dai_link->dai_fmt &= SND_SOC_DAIFMT_CLOCK_PROVIDER_MASK;
+		dai_link->dai_fmt |= daifmt;
+	}
+
+	/*
+	 * check "mediatek,clk-provider = xxx"
+	 * SND_SOC_DAIFMT_CLOCK_PROVIDER_MASK area
+	 */
+	ret = of_property_read_string(sub_node, "mediatek,clk-provider", &str);
+	if (ret == 0) {
+		int i;
+
+		for (i = 0; i < ARRAY_SIZE(of_clk_table); i++) {
+			if (strcmp(str, of_clk_table[i].name) == 0) {
+				dai_link->dai_fmt &= ~SND_SOC_DAIFMT_CLOCK_PROVIDER_MASK;
+				dai_link->dai_fmt |= of_clk_table[i].val;
+				break;
+			}
+		}
+	}
+
+	return 0;
+}
+
 int parse_dai_link_info(struct snd_soc_card *card)
 {
 	struct device *dev = card->dev;
@@ -67,6 +108,12 @@ int parse_dai_link_info(struct snd_soc_card *card)
 			of_node_put(sub_node);
 			return ret;
 		}
+
+		ret = set_dailink_daifmt(card, sub_node, dai_link);
+		if (ret < 0) {
+			of_node_put(sub_node);
+			return ret;
+		}
 	}
 
 	return 0;
-- 
2.18.0

Re: [PATCH v2 4/7] ASoC: mediatek: common: soundcard driver add dai_fmt support

[Alexandre Mergnat]

On 23/05/2023 04:19, Trevor Wu wrote:
> There are two changes included in the patch.
> 
> First, add set_dailink_daifmt() function, so dai_fmt can be updated by
> the configuration in dai-link sub node.
> 
> Second, remove codec phandle from required property in dai-link sub node.
> For example, user possibly needs to update dai-format for all etdm
> co-clock dai-links, but codec doesn't need to be specified in capture
> dai-link for a speaker amp.
> 
> Signed-off-by: Trevor Wu <trevor.wu@mediatek.com>
> ---
>   .../mediatek/common/mtk-soundcard-driver.c    | 49 ++++++++++++++++++-
>   1 file changed, 48 insertions(+), 1 deletion(-)
> 
> diff --git a/sound/soc/mediatek/common/mtk-soundcard-driver.c b/sound/soc/mediatek/common/mtk-soundcard-driver.c
> index 738093451ccb..5e291092046b 100644
> --- a/sound/soc/mediatek/common/mtk-soundcard-driver.c
> +++ b/sound/soc/mediatek/common/mtk-soundcard-driver.c
> @@ -22,7 +22,7 @@ static int set_card_codec_info(struct snd_soc_card *card,
>   
>   	codec_node = of_get_child_by_name(sub_node, "codec");
>   	if (!codec_node)
> -		return -EINVAL;

IMO, the debug or info print should be welcome here because the behavior
change: dai link info parse will not stop if the codec node is NULL.
That could help to understand what is happening during init.



Reviewed-by: Alexandre Mergnat <amergnat@baylibre.com>

-- 
Regards,
Alexandre

Re: [PATCH v2 4/7] ASoC: mediatek: common: soundcard driver add dai_fmt support

[Trevor Wu (吳文良)]

On Wed, 2023-05-24 at 16:12 +0200, Alexandre Mergnat wrote:
> On 23/05/2023 04:19, Trevor Wu wrote:
> > There are two changes included in the patch.
> > 
> > First, add set_dailink_daifmt() function, so dai_fmt can be updated
> > by
> > the configuration in dai-link sub node.
> > 
> > Second, remove codec phandle from required property in dai-link sub
> > node.
> > For example, user possibly needs to update dai-format for all etdm
> > co-clock dai-links, but codec doesn't need to be specified in
> > capture
> > dai-link for a speaker amp.
> > 
> > Signed-off-by: Trevor Wu <trevor.wu@mediatek.com>
> > ---
> >   .../mediatek/common/mtk-soundcard-driver.c    | 49
> > ++++++++++++++++++-
> >   1 file changed, 48 insertions(+), 1 deletion(-)
> > 
> > diff --git a/sound/soc/mediatek/common/mtk-soundcard-driver.c
> > b/sound/soc/mediatek/common/mtk-soundcard-driver.c
> > index 738093451ccb..5e291092046b 100644
> > --- a/sound/soc/mediatek/common/mtk-soundcard-driver.c
> > +++ b/sound/soc/mediatek/common/mtk-soundcard-driver.c
> > @@ -22,7 +22,7 @@ static int set_card_codec_info(struct
> > snd_soc_card *card,
> > 
> >       codec_node = of_get_child_by_name(sub_node, "codec");
> >       if (!codec_node)
> > -             return -EINVAL;
> 
> IMO, the debug or info print should be welcome here because the
> behavior
> change: dai link info parse will not stop if the codec node is NULL.
> That could help to understand what is happening during init.
> 
> 
> 
> Reviewed-by: Alexandre Mergnat <amergnat@baylibre.com>
> 

Thanks for your suggestion. I will add a debug message here.

Thanks,
Trevor

[PATCH v2 5/7] ASoC: soc-dapm.c: clean up debugfs for freed widget

[Trevor Wu]

When a widget is added to dapm via snd_soc_dapm_new_widgets,
dapm_debugfs_add_widget is also called to create a corresponding debugfs
file. However, when a widget is freed by snd_soc_dapm_free_widget, the
corresponding debugfs is not cleared. As a result, the freed widget is
still seen in the dapm directory.

This patch adds dapm_debugfs_free_widget to free the debugfs of a
specified widget, and it's called at snd_soc_dapm_free_widget to clean
up the debugfs for freed widget.

Signed-off-by: Trevor Wu <trevor.wu@mediatek.com>
---
 sound/soc/soc-dapm.c | 16 ++++++++++++++++
 1 file changed, 16 insertions(+)

diff --git a/sound/soc/soc-dapm.c b/sound/soc/soc-dapm.c
index f2f04ce693a1..c65cc374bb3f 100644
--- a/sound/soc/soc-dapm.c
+++ b/sound/soc/soc-dapm.c
@@ -2216,6 +2216,16 @@ static void dapm_debugfs_add_widget(struct snd_soc_dapm_widget *w)
 			    &dapm_widget_power_fops);
 }
 
+static void dapm_debugfs_free_widget(struct snd_soc_dapm_widget *w)
+{
+	struct snd_soc_dapm_context *dapm = w->dapm;
+
+	if (!dapm->debugfs_dapm || !w->name)
+		return;
+
+	debugfs_lookup_and_remove(w->name, dapm->debugfs_dapm);
+}
+
 static void dapm_debugfs_cleanup(struct snd_soc_dapm_context *dapm)
 {
 	debugfs_remove_recursive(dapm->debugfs_dapm);
@@ -2232,6 +2242,10 @@ static inline void dapm_debugfs_add_widget(struct snd_soc_dapm_widget *w)
 {
 }
 
+static inline void dapm_debugfs_free_widget(struct snd_soc_dapm_widget *w)
+{
+}
+
 static inline void dapm_debugfs_cleanup(struct snd_soc_dapm_context *dapm)
 {
 }
@@ -2495,6 +2509,8 @@ void snd_soc_dapm_free_widget(struct snd_soc_dapm_widget *w)
 			dapm_free_path(p);
 	}
 
+	dapm_debugfs_free_widget(w);
+
 	kfree(w->kcontrols);
 	kfree_const(w->name);
 	kfree_const(w->sname);
-- 
2.18.0

Re: [PATCH v2 5/7] ASoC: soc-dapm.c: clean up debugfs for freed widget

[Alexandre Mergnat]

On 23/05/2023 04:19, Trevor Wu wrote:
> When a widget is added to dapm via snd_soc_dapm_new_widgets,
> dapm_debugfs_add_widget is also called to create a corresponding debugfs
> file. However, when a widget is freed by snd_soc_dapm_free_widget, the
> corresponding debugfs is not cleared. As a result, the freed widget is
> still seen in the dapm directory.
> 
> This patch adds dapm_debugfs_free_widget to free the debugfs of a
> specified widget, and it's called at snd_soc_dapm_free_widget to clean
> up the debugfs for freed widget.
> 
> Signed-off-by: Trevor Wu<trevor.wu@mediatek.com>

Reviewed-by: Alexandre Mergnat <amergnat@baylibre.com>

-- 
Regards,
Alexandre

[PATCH v2 6/7] ASoC: mediatek: mt8188-mt6359: support new board with nau88255

[Trevor Wu]

This patch adds multiple i2s codecs support including NAU88L25,
MAX98390, and the dumb amp like NAU8318 usage. In addition, dmic-codec
is also added to skip the beginning pop noise.

Signed-off-by: Trevor Wu <trevor.wu@mediatek.com>
---
 sound/soc/mediatek/Kconfig                |   4 +
 sound/soc/mediatek/mt8188/mt8188-mt6359.c | 330 +++++++++++++++++++++-
 2 files changed, 333 insertions(+), 1 deletion(-)

diff --git a/sound/soc/mediatek/Kconfig b/sound/soc/mediatek/Kconfig
index 4baac72677d9..4ea012342b52 100644
--- a/sound/soc/mediatek/Kconfig
+++ b/sound/soc/mediatek/Kconfig
@@ -225,6 +225,10 @@ config SND_SOC_MT8188_MT6359
 	depends on SND_SOC_MT8188 && MTK_PMIC_WRAP
 	select SND_SOC_MT6359
 	select SND_SOC_HDMI_CODEC
+	select SND_SOC_DMIC
+	select SND_SOC_MAX98390
+	select SND_SOC_NAU8315
+	select SND_SOC_NAU8825
 	help
 	  This adds support for ASoC machine driver for MediaTek MT8188
 	  boards with the MT6359 and other I2S audio codecs.
diff --git a/sound/soc/mediatek/mt8188/mt8188-mt6359.c b/sound/soc/mediatek/mt8188/mt8188-mt6359.c
index 6c3f36e2fffd..1d3673873f05 100644
--- a/sound/soc/mediatek/mt8188/mt8188-mt6359.c
+++ b/sound/soc/mediatek/mt8188/mt8188-mt6359.c
@@ -6,6 +6,7 @@
  * Author: Trevor Wu <trevor.wu@mediatek.com>
  */
 
+#include <linux/input.h>
 #include <linux/module.h>
 #include <linux/of_device.h>
 #include <linux/pm_runtime.h>
@@ -13,10 +14,27 @@
 #include <sound/pcm_params.h>
 #include <sound/soc.h>
 #include "mt8188-afe-common.h"
+#include "../../codecs/nau8825.h"
 #include "../../codecs/mt6359.h"
 #include "../common/mtk-afe-platform-driver.h"
 #include "../common/mtk-soundcard-driver.h"
 
+#define NAU8825_HS_PRESENT	BIT(0)
+
+/*
+ * Maxim MAX98390
+ */
+#define MAX98390_CODEC_DAI     "max98390-aif1"
+#define MAX98390_DEV0_NAME     "max98390.0-0038" /* rear right */
+#define MAX98390_DEV1_NAME     "max98390.0-0039" /* rear left */
+#define MAX98390_DEV2_NAME     "max98390.0-003a" /* front right */
+#define MAX98390_DEV3_NAME     "max98390.0-003b" /* front left */
+
+/*
+ * Nau88l25
+ */
+#define NAU8825_CODEC_DAI  "nau8825-hifi"
+
 /* FE */
 SND_SOC_DAILINK_DEFS(playback2,
 		     DAILINK_COMP_ARRAY(COMP_CPU("DL2")),
@@ -143,12 +161,16 @@ SND_SOC_DAILINK_DEFS(pcm1,
 SND_SOC_DAILINK_DEFS(ul_src,
 		     DAILINK_COMP_ARRAY(COMP_CPU("UL_SRC")),
 		     DAILINK_COMP_ARRAY(COMP_CODEC("mt6359-sound",
-						   "mt6359-snd-codec-aif1")),
+						   "mt6359-snd-codec-aif1"),
+					COMP_CODEC("dmic-codec",
+						   "dmic-hifi")),
 		     DAILINK_COMP_ARRAY(COMP_EMPTY()));
 
 struct mt8188_mt6359_priv {
 	struct snd_soc_jack dp_jack;
 	struct snd_soc_jack hdmi_jack;
+	struct snd_soc_jack headset_jack;
+	void *private_data;
 };
 
 static struct snd_soc_jack_pin mt8188_hdmi_jack_pins[] = {
@@ -165,11 +187,50 @@ static struct snd_soc_jack_pin mt8188_dp_jack_pins[] = {
 	},
 };
 
+static struct snd_soc_jack_pin nau8825_jack_pins[] = {
+	{
+		.pin    = "Headphone Jack",
+		.mask   = SND_JACK_HEADPHONE,
+	},
+	{
+		.pin    = "Headset Mic",
+		.mask   = SND_JACK_MICROPHONE,
+	},
+};
+
 struct mt8188_card_data {
 	const char *name;
 	unsigned long quirk;
 };
 
+static const struct snd_kcontrol_new mt8188_dumb_spk_controls[] = {
+	SOC_DAPM_PIN_SWITCH("Ext Spk"),
+};
+
+static const struct snd_soc_dapm_widget mt8188_dumb_spk_widgets[] = {
+	SND_SOC_DAPM_SPK("Ext Spk", NULL),
+};
+
+static const struct snd_kcontrol_new mt8188_dual_spk_controls[] = {
+	SOC_DAPM_PIN_SWITCH("Left Spk"),
+	SOC_DAPM_PIN_SWITCH("Right Spk"),
+};
+
+static const struct snd_soc_dapm_widget mt8188_dual_spk_widgets[] = {
+	SND_SOC_DAPM_SPK("Left Spk", NULL),
+	SND_SOC_DAPM_SPK("Right Spk", NULL),
+};
+
+static const struct snd_kcontrol_new mt8188_rear_spk_controls[] = {
+	SOC_DAPM_PIN_SWITCH("Rear Left Spk"),
+	SOC_DAPM_PIN_SWITCH("Rear Right Spk"),
+};
+
+static const struct snd_soc_dapm_widget mt8188_rear_spk_widgets[] = {
+	SND_SOC_DAPM_SPK("Rear Left Spk", NULL),
+	SND_SOC_DAPM_SPK("Rear Right Spk", NULL),
+};
+
 static const struct snd_soc_dapm_widget mt8188_mt6359_widgets[] = {
 	SND_SOC_DAPM_HP("Headphone", NULL),
 	SND_SOC_DAPM_MIC("Headset Mic", NULL),
@@ -182,6 +243,14 @@ static const struct snd_kcontrol_new mt8188_mt6359_controls[] = {
 	SOC_DAPM_PIN_SWITCH("Headset Mic"),
 };
 
+static const struct snd_soc_dapm_widget mt8188_nau8825_widgets[] = {
+	SND_SOC_DAPM_HP("Headphone Jack", NULL),
+};
+
+static const struct snd_kcontrol_new mt8188_nau8825_controls[] = {
+	SOC_DAPM_PIN_SWITCH("Headphone Jack"),
+};
+
 #define CKSYS_AUD_TOP_CFG 0x032c
 #define CKSYS_AUD_TOP_MON 0x0330
 
@@ -451,6 +520,189 @@ static int mt8188_dptx_codec_init(struct snd_soc_pcm_runtime *rtd)
 	return ret;
 }
 
+static int mt8188_dumb_amp_init(struct snd_soc_pcm_runtime *rtd)
+{
+	struct snd_soc_card *card = rtd->card;
+	int ret = 0;
+
+	ret = snd_soc_dapm_new_controls(&card->dapm, mt8188_dumb_spk_widgets,
+					ARRAY_SIZE(mt8188_dumb_spk_widgets));
+	if (ret) {
+		dev_err(rtd->dev, "unable to add Dumb Speaker dapm, ret %d\n", ret);
+		return ret;
+	}
+
+	ret = snd_soc_add_card_controls(card, mt8188_dumb_spk_controls,
+					ARRAY_SIZE(mt8188_dumb_spk_controls));
+	if (ret) {
+		dev_err(rtd->dev, "unable to add Dumb card controls, ret %d\n", ret);
+		return ret;
+	}
+
+	return ret;
+}
+
+static int mt8188_max98390_hw_params(struct snd_pcm_substream *substream,
+				     struct snd_pcm_hw_params *params)
+{
+	struct snd_soc_pcm_runtime *rtd = substream->private_data;
+	unsigned int bit_width = params_width(params);
+	struct snd_soc_dai *cpu_dai = asoc_rtd_to_cpu(rtd, 0);
+	struct snd_soc_dai *codec_dai;
+	int i;
+
+	snd_soc_dai_set_tdm_slot(cpu_dai, 0xf, 0xf, 4, bit_width);
+
+	for_each_rtd_codec_dais(rtd, i, codec_dai) {
+		if (!strcmp(codec_dai->component->name, MAX98390_DEV0_NAME))
+			snd_soc_dai_set_tdm_slot(codec_dai, 0x8, 0x3, 4, bit_width);
+
+		if (!strcmp(codec_dai->component->name, MAX98390_DEV1_NAME))
+			snd_soc_dai_set_tdm_slot(codec_dai, 0x4, 0x3, 4, bit_width);
+
+		if (!strcmp(codec_dai->component->name, MAX98390_DEV2_NAME))
+			snd_soc_dai_set_tdm_slot(codec_dai, 0x2, 0x3, 4, bit_width);
+
+		if (!strcmp(codec_dai->component->name, MAX98390_DEV3_NAME))
+			snd_soc_dai_set_tdm_slot(codec_dai, 0x1, 0x3, 4, bit_width);
+	}
+	return 0;
+}
+
+static const struct snd_soc_ops mt8188_max98390_ops = {
+	.hw_params = mt8188_max98390_hw_params,
+};
+
+static int mt8188_max98390_codec_init(struct snd_soc_pcm_runtime *rtd)
+{
+	struct snd_soc_card *card = rtd->card;
+	int ret;
+
+	/* add regular speakers dapm route */
+	ret = snd_soc_dapm_new_controls(&card->dapm, mt8188_dual_spk_widgets,
+					ARRAY_SIZE(mt8188_dual_spk_widgets));
+	if (ret) {
+		dev_err(rtd->dev, "unable to add Left/Right Speaker widget, ret %d\n", ret);
+		return ret;
+	}
+
+	ret = snd_soc_add_card_controls(card, mt8188_dual_spk_controls,
+					ARRAY_SIZE(mt8188_dual_spk_controls));
+	if (ret) {
+		dev_err(rtd->dev, "unable to add Left/Right card controls, ret %d\n", ret);
+		return ret;
+	}
+
+	if (rtd->dai_link->num_codecs <= 2)
+		return ret;
+
+	/* add widgets/controls/dapm for rear speakers */
+	ret = snd_soc_dapm_new_controls(&card->dapm, mt8188_rear_spk_widgets,
+					ARRAY_SIZE(mt8188_rear_spk_widgets));
+	if (ret) {
+		dev_err(rtd->dev, "unable to add Rear Speaker widget, ret %d\n", ret);
+		/* Don't need to add routes if widget addition failed */
+		return ret;
+	}
+
+	ret = snd_soc_add_card_controls(card, mt8188_rear_spk_controls,
+					ARRAY_SIZE(mt8188_rear_spk_controls));
+	if (ret) {
+		dev_err(rtd->dev, "unable to add Rear card controls, ret %d\n", ret);
+		return ret;
+	}
+
+	return ret;
+}
+
+static int mt8188_nau8825_codec_init(struct snd_soc_pcm_runtime *rtd)
+{
+	struct snd_soc_card *card = rtd->card;
+	struct mt8188_mt6359_priv *priv = snd_soc_card_get_drvdata(card);
+	struct snd_soc_component *component = asoc_rtd_to_codec(rtd, 0)->component;
+	struct snd_soc_jack *jack = &priv->headset_jack;
+	int ret;
+
+	ret = snd_soc_dapm_new_controls(&card->dapm, mt8188_nau8825_widgets,
+					ARRAY_SIZE(mt8188_nau8825_widgets));
+	if (ret) {
+		dev_err(rtd->dev, "unable to add nau8825 card widget, ret %d\n", ret);
+		return ret;
+	}
+
+	ret = snd_soc_add_card_controls(card, mt8188_nau8825_controls,
+					ARRAY_SIZE(mt8188_nau8825_controls));
+	if (ret) {
+		dev_err(rtd->dev, "unable to add nau8825 card controls, ret %d\n", ret);
+		return ret;
+	}
+
+	ret = snd_soc_card_jack_new_pins(rtd->card, "Headset Jack",
+					 SND_JACK_HEADSET | SND_JACK_BTN_0 |
+					 SND_JACK_BTN_1 | SND_JACK_BTN_2 |
+					 SND_JACK_BTN_3,
+					 jack,
+					 nau8825_jack_pins,
+					 ARRAY_SIZE(nau8825_jack_pins));
+	if (ret) {
+		dev_err(rtd->dev, "Headset Jack creation failed: %d\n", ret);
+		return ret;
+	}
+
+	snd_jack_set_key(jack->jack, SND_JACK_BTN_0, KEY_PLAYPAUSE);
+	snd_jack_set_key(jack->jack, SND_JACK_BTN_1, KEY_VOICECOMMAND);
+	snd_jack_set_key(jack->jack, SND_JACK_BTN_2, KEY_VOLUMEUP);
+	snd_jack_set_key(jack->jack, SND_JACK_BTN_3, KEY_VOLUMEDOWN);
+	ret = snd_soc_component_set_jack(component, jack, NULL);
+
+	if (ret) {
+		dev_err(rtd->dev, "Headset Jack call-back failed: %d\n", ret);
+		return ret;
+	}
+
+	return ret;
+};
+
+static void mt8188_nau8825_codec_exit(struct snd_soc_pcm_runtime *rtd)
+{
+	struct snd_soc_component *component = asoc_rtd_to_codec(rtd, 0)->component;
+
+	snd_soc_component_set_jack(component, NULL, NULL);
+}
+
+static int mt8188_nau8825_hw_params(struct snd_pcm_substream *substream,
+				    struct snd_pcm_hw_params *params)
+{
+	struct snd_soc_pcm_runtime *rtd = asoc_substream_to_rtd(substream);
+	struct snd_soc_dai *codec_dai = asoc_rtd_to_codec(rtd, 0);
+	unsigned int rate = params_rate(params);
+	unsigned int bit_width = params_width(params);
+	int clk_freq, ret;
+
+	clk_freq = rate * 2 * bit_width;
+
+	/* Configure clock for codec */
+	ret = snd_soc_dai_set_sysclk(codec_dai, NAU8825_CLK_FLL_BLK, 0,
+				     SND_SOC_CLOCK_IN);
+	if (ret < 0) {
+		dev_err(codec_dai->dev, "can't set BCLK clock %d\n", ret);
+		return ret;
+	}
+
+	/* Configure pll for codec */
+	ret = snd_soc_dai_set_pll(codec_dai, 0, 0, clk_freq,
+				  params_rate(params) * 256);
+	if (ret < 0) {
+		dev_err(codec_dai->dev, "can't set BCLK: %d\n", ret);
+		return ret;
+	}
+
+	return ret;
+}
+
+static const struct snd_soc_ops mt8188_nau8825_ops = {
+	.hw_params = mt8188_nau8825_hw_params,
+};
 static struct snd_soc_dai_link mt8188_mt6359_dai_links[] = {
 	/* FE */
 	[DAI_LINK_DL2_FE] = {
@@ -708,6 +960,40 @@ static struct snd_soc_dai_link mt8188_mt6359_dai_links[] = {
 	},
 };
 
+static struct snd_kcontrol *ctl_find(struct snd_card *card, const char *name)
+{
+	struct snd_ctl_elem_id sid;
+
+	memset(&sid, 0, sizeof(sid));
+	strcpy(sid.name, name);
+	sid.iface = SNDRV_CTL_ELEM_IFACE_MIXER;
+	return snd_ctl_find_id(card, &sid);
+}
+
+static void mt8188_fixup_controls(struct snd_soc_card *card)
+{
+	struct mt8188_mt6359_priv *priv = snd_soc_card_get_drvdata(card);
+	struct mt8188_card_data *card_data = (struct mt8188_card_data *)priv->private_data;
+	struct snd_kcontrol *kctl;
+
+	if (card_data->quirk && NAU8825_HS_PRESENT) {
+		struct snd_soc_dapm_widget *w, *next_w;
+
+		for_each_card_widgets_safe(card, w, next_w) {
+			if (strcmp(w->name, "Headphone"))
+				continue;
+
+			snd_soc_dapm_free_widget(w);
+		}
+
+		kctl = ctl_find(card->snd_card, "Headphone Switch");
+		if (kctl)
+			snd_ctl_remove(card->snd_card, kctl);
+		else
+			dev_warn(card->dev, "Cannot find ctl : Headphone Switch\n");
+	}
+}
+
 static struct snd_soc_card mt8188_mt6359_soc_card = {
 	.owner = THIS_MODULE,
 	.dai_link = mt8188_mt6359_dai_links,
@@ -716,6 +1002,7 @@ static struct snd_soc_card mt8188_mt6359_soc_card = {
 	.num_dapm_widgets = ARRAY_SIZE(mt8188_mt6359_widgets),
 	.controls = mt8188_mt6359_controls,
 	.num_controls = ARRAY_SIZE(mt8188_mt6359_controls),
+	.fixup_controls = mt8188_fixup_controls,
 };
 
 static int mt8188_mt6359_dev_probe(struct platform_device *pdev)
@@ -726,6 +1013,9 @@ static int mt8188_mt6359_dev_probe(struct platform_device *pdev)
 	struct mt8188_card_data *card_data;
 	struct snd_soc_dai_link *dai_link;
 	bool init_mt6359 = false;
+	bool init_nau8825 = false;
+	bool init_max98390 = false;
+	bool init_dumb = false;
 	int ret, i;
 
 	card_data = (struct mt8188_card_data *)of_device_get_match_data(&pdev->dev);
@@ -776,9 +1066,38 @@ static int mt8188_mt6359_dev_probe(struct platform_device *pdev)
 				dai_link->init = mt8188_mt6359_init;
 				init_mt6359 = true;
 			}
+		} else if (strcmp(dai_link->name, "ETDM1_OUT_BE") == 0 ||
+			   strcmp(dai_link->name, "ETDM2_OUT_BE") == 0 ||
+			   strcmp(dai_link->name, "ETDM1_IN_BE") == 0 ||
+			   strcmp(dai_link->name, "ETDM2_IN_BE") == 0) {
+			if (!strcmp(dai_link->codecs->dai_name, MAX98390_CODEC_DAI)) {
+				dai_link->ops = &mt8188_max98390_ops;
+				if (init_max98390)
+					continue;
+
+				dai_link->init = mt8188_max98390_codec_init;
+				init_max98390 = true;
+			} else if (!strcmp(dai_link->codecs->dai_name, NAU8825_CODEC_DAI)) {
+				dai_link->ops = &mt8188_nau8825_ops;
+				if (init_nau8825)
+					continue;
+
+				dai_link->init = mt8188_nau8825_codec_init;
+				dai_link->exit = mt8188_nau8825_codec_exit;
+				init_nau8825 = true;
+			} else {
+				if (strcmp(dai_link->codecs->dai_name, "snd-soc-dummy-dai")) {
+					if (init_dumb)
+						continue;
+
+					dai_link->init = mt8188_dumb_amp_init;
+					init_dumb = true;
+				}
+			}
 		}
 	}
 
+	priv->private_data = card_data;
 	snd_soc_card_set_drvdata(card, priv);
 
 	ret = devm_snd_soc_register_card(&pdev->dev, card);
@@ -795,11 +1114,20 @@ static struct mt8188_card_data mt8188_evb_card = {
 	.name = "mt8188_mt6359",
 };
 
+static struct mt8188_card_data mt8188_nau8825_card = {
+	.name = "mt8188_nau8825",
+	.quirk = NAU8825_HS_PRESENT,
+};
+
 static const struct of_device_id mt8188_mt6359_dt_match[] = {
 	{
 		.compatible = "mediatek,mt8188-mt6359-evb",
 		.data = &mt8188_evb_card,
 	},
+	{
+		.compatible = "mediatek,mt8188-nau8825",
+		.data = &mt8188_nau8825_card,
+	},
 	{},
 };
 MODULE_DEVICE_TABLE(of, mt8188_mt6359_dt_match);
-- 
2.18.0

Re: [PATCH v2 6/7] ASoC: mediatek: mt8188-mt6359: support new board with nau88255

[Alexandre Mergnat]

On 23/05/2023 04:19, Trevor Wu wrote:
>   	card_data = (struct mt8188_card_data *)of_device_get_match_data(&pdev->dev);
> @@ -776,9 +1066,38 @@ static int mt8188_mt6359_dev_probe(struct platform_device *pdev)
>   				dai_link->init = mt8188_mt6359_init;
>   				init_mt6359 = true;
>   			}
> +		} else if (strcmp(dai_link->name, "ETDM1_OUT_BE") == 0 ||
> +			   strcmp(dai_link->name, "ETDM2_OUT_BE") == 0 ||
> +			   strcmp(dai_link->name, "ETDM1_IN_BE") == 0 ||
> +			   strcmp(dai_link->name, "ETDM2_IN_BE") == 0) {
> +			if (!strcmp(dai_link->codecs->dai_name, MAX98390_CODEC_DAI)) {
> +				dai_link->ops = &mt8188_max98390_ops;
> +				if (init_max98390)
> +					continue;

I prefer that you do like your patch [PATCH v2 1/7] to be consistent
and easy to read.

+			if (!init_mt6359) {
+				dai_link->init = mt8188_mt6359_init;
+				init_mt6359 = true;
+			}

> +
> +				dai_link->init = mt8188_max98390_codec_init;
> +				init_max98390 = true;
> +			} else if (!strcmp(dai_link->codecs->dai_name, NAU8825_CODEC_DAI)) {
> +				dai_link->ops = &mt8188_nau8825_ops;
> +				if (init_nau8825)
> +					continue;

ditto

> +
> +				dai_link->init = mt8188_nau8825_codec_init;
> +				dai_link->exit = mt8188_nau8825_codec_exit;
> +				init_nau8825 = true;
> +			} else {
> +				if (strcmp(dai_link->codecs->dai_name, "snd-soc-dummy-dai")) {

Shouldn't be with a NOT operator ("!") ?
+ if (!strcmp(dai_link->codecs->dai_name, "snd-soc-dummy-dai")) {


> +					if (init_dumb)
> +						continue;

ditto

> +
> +					dai_link->init = mt8188_dumb_amp_init;
> +					init_dumb = true;
> +				}
> +			}
>   		}
>   	}
>   
> +	priv->private_data = card_data;
>   	snd_soc_card_set_drvdata(card, priv);
>   
>   	ret = devm_snd_soc_register_card(&pdev->dev, card);
> @@ -795,11 +1114,20 @@ static struct mt8188_card_data mt8188_evb_card = {
>   	.name = "mt8188_mt6359",
>   };
>   
> +static struct mt8188_card_data mt8188_nau8825_card = {
> +	.name = "mt8188_nau8825",
> +	.quirk = NAU8825_HS_PRESENT,
> +};
> +
>   static const struct of_device_id mt8188_mt6359_dt_match[] = {
>   	{
>   		.compatible = "mediatek,mt8188-mt6359-evb",
>   		.data = &mt8188_evb_card,
>   	},
> +	{
> +		.compatible = "mediatek,mt8188-nau8825",
> +		.data = &mt8188_nau8825_card,
> +	},
>   	{},
>   };
>   MODULE_DEVICE_TABLE(of, mt8188_mt6359_dt_match);

-- 
Regards,
Alexandre

Re: [PATCH v2 6/7] ASoC: mediatek: mt8188-mt6359: support new board with nau88255

[Trevor Wu (吳文良)]

On Wed, 2023-05-24 at 17:51 +0200, Alexandre Mergnat wrote:
> External email : Please do not click links or open attachments until
> you have verified the sender or the content.
> 
> 
> On 23/05/2023 04:19, Trevor Wu wrote:
> >       card_data = (struct mt8188_card_data
> > *)of_device_get_match_data(&pdev->dev);
> > @@ -776,9 +1066,38 @@ static int mt8188_mt6359_dev_probe(struct
> > platform_device *pdev)
> >                               dai_link->init = mt8188_mt6359_init;
> >                               init_mt6359 = true;
> >                       }
> > +             } else if (strcmp(dai_link->name, "ETDM1_OUT_BE") ==
> > 0 ||
> > +                        strcmp(dai_link->name, "ETDM2_OUT_BE") ==
> > 0 ||
> > +                        strcmp(dai_link->name, "ETDM1_IN_BE") == 0
> > ||
> > +                        strcmp(dai_link->name, "ETDM2_IN_BE") ==
> > 0) {
> > +                     if (!strcmp(dai_link->codecs->dai_name,
> > MAX98390_CODEC_DAI)) {
> > +                             dai_link->ops = &mt8188_max98390_ops;
> > +                             if (init_max98390)
> > +                                     continue;
> 
> I prefer that you do like your patch [PATCH v2 1/7] to be consistent
> and easy to read.

OK, I will unify the style in v3.

> 
> +                       if (!init_mt6359) {
> +                               dai_link->init = mt8188_mt6359_init;
> +                               init_mt6359 = true;
> +                       }
> 
> > +
> > +                             dai_link->init =
> > mt8188_max98390_codec_init;
> > +                             init_max98390 = true;
> > +                     } else if (!strcmp(dai_link->codecs-
> > >dai_name, NAU8825_CODEC_DAI)) {
> > +                             dai_link->ops = &mt8188_nau8825_ops;
> > +                             if (init_nau8825)
> > +                                     continue;
> 
> ditto
> 
> > +
> > +                             dai_link->init =
> > mt8188_nau8825_codec_init;
> > +                             dai_link->exit =
> > mt8188_nau8825_codec_exit;
> > +                             init_nau8825 = true;
> > +                     } else {
> > +                             if (strcmp(dai_link->codecs-
> > >dai_name, "snd-soc-dummy-dai")) {
> 
> Shouldn't be with a NOT operator ("!") ?
> + if (!strcmp(dai_link->codecs->dai_name, "snd-soc-dummy-dai")) {
> 

strcmp returns 0 when the contents of both strings are equal. The
default codec name is "snd-soc-dummy-dai". When the codec is specified
from dts, the return value of strcmp will be non-zero and this is the
case I want to assign init function. Therefore, it doesn't require a
NOT operator here.

Thanks,
Trevor

> 

Re: [PATCH v2 6/7] ASoC: mediatek: mt8188-mt6359: support new board with nau88255

[Alexandre Mergnat]

On 25/05/2023 05:44, Trevor Wu (吳文良) wrote:
> On Wed, 2023-05-24 at 17:51 +0200, Alexandre Mergnat wrote:
>> External email : Please do not click links or open attachments until
>> you have verified the sender or the content.
>>
>>
>> On 23/05/2023 04:19, Trevor Wu wrote:
>>>        card_data = (struct mt8188_card_data
>>> *)of_device_get_match_data(&pdev->dev);
>>> @@ -776,9 +1066,38 @@ static int mt8188_mt6359_dev_probe(struct
>>> platform_device *pdev)
>>>                                dai_link->init = mt8188_mt6359_init;
>>>                                init_mt6359 = true;
>>>                        }
>>> +             } else if (strcmp(dai_link->name, "ETDM1_OUT_BE") ==
>>> 0 ||
>>> +                        strcmp(dai_link->name, "ETDM2_OUT_BE") ==
>>> 0 ||
>>> +                        strcmp(dai_link->name, "ETDM1_IN_BE") == 0
>>> ||
>>> +                        strcmp(dai_link->name, "ETDM2_IN_BE") ==
>>> 0) {
>>> +                     if (!strcmp(dai_link->codecs->dai_name,
>>> MAX98390_CODEC_DAI)) {
>>> +                             dai_link->ops = &mt8188_max98390_ops;
>>> +                             if (init_max98390)
>>> +                                     continue;
>>
>> I prefer that you do like your patch [PATCH v2 1/7] to be consistent
>> and easy to read.
> 
> OK, I will unify the style in v3.
> 
>>
>> +                       if (!init_mt6359) {
>> +                               dai_link->init = mt8188_mt6359_init;
>> +                               init_mt6359 = true;
>> +                       }
>>
>>> +
>>> +                             dai_link->init =
>>> mt8188_max98390_codec_init;
>>> +                             init_max98390 = true;
>>> +                     } else if (!strcmp(dai_link->codecs-
>>>> dai_name, NAU8825_CODEC_DAI)) {
>>> +                             dai_link->ops = &mt8188_nau8825_ops;
>>> +                             if (init_nau8825)
>>> +                                     continue;
>>
>> ditto
>>
>>> +
>>> +                             dai_link->init =
>>> mt8188_nau8825_codec_init;
>>> +                             dai_link->exit =
>>> mt8188_nau8825_codec_exit;
>>> +                             init_nau8825 = true;
>>> +                     } else {
>>> +                             if (strcmp(dai_link->codecs-
>>>> dai_name, "snd-soc-dummy-dai")) {
>>
>> Shouldn't be with a NOT operator ("!") ?
>> + if (!strcmp(dai_link->codecs->dai_name, "snd-soc-dummy-dai")) {
>>
> 
> strcmp returns 0 when the contents of both strings are equal. The
> default codec name is "snd-soc-dummy-dai". When the codec is specified
> from dts, the return value of strcmp will be non-zero and this is the
> case I want to assign init function. Therefore, it doesn't require a
> NOT operator here.

Ok, I thought "mt8188_dumb_amp_init" was for "snd-soc-dummy-dai".
It's fine for me.

-- 
Regards,
Alexandre

[PATCH v2 7/7] ASoC: dt-bindings: mediatek,mt8188-mt6359: add NAU8825 support

[Trevor Wu]

Add compatible string "mediatek,mt8188-nau8825" to support new board
with nau8825 codec.

Introduce two properties "dai-format" and "mediatek,clk-provider" under
dai-link subnode to configure dai-link parameters via dts.

"codec" property is removed from required property of dai-link subnode.
For co-clock case, it's possible two dai-links should be configured to
the same dai format, but only one cpu dai is bound with codec.

Signed-off-by: Trevor Wu <trevor.wu@mediatek.com>
---
 .../sound/mediatek,mt8188-mt6359.yaml         | 26 ++++++++++++++++---
 1 file changed, 23 insertions(+), 3 deletions(-)

diff --git a/Documentation/devicetree/bindings/sound/mediatek,mt8188-mt6359.yaml b/Documentation/devicetree/bindings/sound/mediatek,mt8188-mt6359.yaml
index 3d2c01b693be..05e532b5d50a 100644
--- a/Documentation/devicetree/bindings/sound/mediatek,mt8188-mt6359.yaml
+++ b/Documentation/devicetree/bindings/sound/mediatek,mt8188-mt6359.yaml
@@ -11,7 +11,9 @@ maintainers:
 
 properties:
   compatible:
-    const: mediatek,mt8188-mt6359-evb
+    enum:
+      - mediatek,mt8188-mt6359-evb
+      - mediatek,mt8188-nau8825
 
   model:
     $ref: /schemas/types.yaml#/definitions/string
@@ -61,11 +63,28 @@ patternProperties:
         required:
           - sound-dai
 
+      dai-format:
+        description: audio format.
+        items:
+          enum:
+            - i2s
+            - right_j
+            - left_j
+            - dsp_a
+            - dsp_b
+
+      mediatek,clk-provider:
+        $ref: /schemas/types.yaml#/definitions/string
+        description: Indicates dai-link clock master.
+        items:
+          enum:
+            - cpu
+            - codec
+
     additionalProperties: false
 
     required:
       - link-name
-      - codec
 
 additionalProperties: false
 
@@ -86,7 +105,8 @@ examples:
             "AIN1", "Headset Mic";
         dai-link-0 {
             link-name = "ETDM3_OUT_BE";
-
+            dai-format = "i2s";
+            mediatek,clk-provider = "cpu";
             codec {
                 sound-dai = <&hdmi0>;
             };
-- 
2.18.0