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

[PATCH 0/5] 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.

Trevor Wu (5):
  ASoC: mediatek: mt8188: separate ADDA playback dai from capture dai
  ASoC: mediatek: mt8188-mt6359: register hdmi/dp jack pins
  ASoC: mediatek: common: soundcard driver add dai_fmt support
  ASoC: mediatek: mt8188-mt6359: support new board with nau88255
  ASoC: dt-bindings: mediatek,mt8188-mt6359: update properties

 .../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   |  76 ++--
 sound/soc/mediatek/mt8188/mt8188-mt6359.c     | 326 +++++++++++++++++-
 6 files changed, 431 insertions(+), 54 deletions(-)

-- 
2.18.0

[PATCH 1/5] 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   | 76 ++++++++++---------
 sound/soc/mediatek/mt8188/mt8188-mt6359.c     | 34 +++++++--
 3 files changed, 68 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..9a6673a6f28a 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,15 @@ 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;
+	if (rate > ADDA_HIRES_THRES)
+		adda_priv->hires_required = 1;
+	else
+		adda_priv->hires_required = 0;
+
+	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 +530,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 +539,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 +559,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 1/5] ASoC: mediatek: mt8188: separate ADDA playback dai from capture dai

[AngeloGioacchino Del Regno]

Il 17/05/23 13:15, Trevor Wu ha scritto:
> 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   | 76 ++++++++++---------
>   sound/soc/mediatek/mt8188/mt8188-mt6359.c     | 34 +++++++--
>   3 files changed, 68 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..9a6673a6f28a 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,

..snip..

> @@ -503,13 +495,15 @@ 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;
> +	if (rate > ADDA_HIRES_THRES)
> +		adda_priv->hires_required = 1;
> +	else
> +		adda_priv->hires_required = 0;
> +

hires_required is a boolean, so assigning 1 or 0 should be replaced with
assigning true or false; regardless of that, what about...

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

Regards,
Angelo

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

[Trevor Wu (吳文良)]

On Wed, 2023-05-17 at 13:40 +0200, AngeloGioacchino Del Regno wrote:
> External email : Please do not click links or open attachments until
> you have verified the sender or the content.
> 
> 
> Il 17/05/23 13:15, Trevor Wu ha scritto:
> > 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   | 76 ++++++++++--
> > -------
> >   sound/soc/mediatek/mt8188/mt8188-mt6359.c     | 34 +++++++--
> >   3 files changed, 68 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..9a6673a6f28a 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,
> 
> ..snip..
> 
> > @@ -503,13 +495,15 @@ 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;
> > +     if (rate > ADDA_HIRES_THRES)
> > +             adda_priv->hires_required = 1;
> > +     else
> > +             adda_priv->hires_required = 0;
> > +
> 
> hires_required is a boolean, so assigning 1 or 0 should be replaced
> with
> assigning true or false; regardless of that, what about...
> 
>         adda_priv->hires_required = (rate > ADDA_HIRES_THRES);
> 

It's great!! I will adopt it in V2.

Thanks,
Trevor

> > +     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;
> >   }
> 
> Regards,
> Angelo

[PATCH 2/5] 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 2/5] ASoC: mediatek: mt8188-mt6359: register hdmi/dp jack pins

[AngeloGioacchino Del Regno]

Il 17/05/23 13:15, Trevor Wu ha scritto:
> 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",

"HDMI Jack" is more consistent with the snd_soc_jack_new_pins() call performed
later.

> +		.mask = SND_JACK_LINEOUT,
> +	},
> +};
> +
> +static struct snd_soc_jack_pin mt8188_dp_jack_pins[] = {
> +	{
> +		.pin = "DP",

Same here: "DP Jack"

> +		.mask = SND_JACK_LINEOUT,
> +	},
> +};
> +

Regards,
Angelo

Re: [PATCH 2/5] ASoC: mediatek: mt8188-mt6359: register hdmi/dp jack pins

[Trevor Wu (吳文良)]

On Wed, 2023-05-17 at 13:31 +0200, AngeloGioacchino Del Regno wrote:
> External email : Please do not click links or open attachments until
> you have verified the sender or the content.
> 
> 
> Il 17/05/23 13:15, Trevor Wu ha scritto:
> > 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",
> 
> "HDMI Jack" is more consistent with the snd_soc_jack_new_pins() call
> performed
> later.

Hi Angelo,

I see jack_kctl_name_gen() will append "Jack" to the name if I don't
name the pin "HDMI Jack". Do you mean that I could directly use "HDMI
Jack" because ALSA uses the name finally?

Thanks,
Trevor

> 
> > +             .mask = SND_JACK_LINEOUT,
> > +     },
> > +};
> > +
> > +static struct snd_soc_jack_pin mt8188_dp_jack_pins[] = {
> > +     {
> > +             .pin = "DP",
> 
> Same here: "DP Jack"
> 
> > +             .mask = SND_JACK_LINEOUT,
> > +     },
> > +};
> > +
> 
> Regards,
> Angelo
> 

Re: [PATCH 2/5] ASoC: mediatek: mt8188-mt6359: register hdmi/dp jack pins

[AngeloGioacchino Del Regno]

Il 17/05/23 14:10, Trevor Wu (吳文良) ha scritto:
> On Wed, 2023-05-17 at 13:31 +0200, AngeloGioacchino Del Regno wrote:
>> External email : Please do not click links or open attachments until
>> you have verified the sender or the content.
>>
>>
>> Il 17/05/23 13:15, Trevor Wu ha scritto:
>>> 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",
>>
>> "HDMI Jack" is more consistent with the snd_soc_jack_new_pins() call
>> performed
>> later.
> 
> Hi Angelo,
> 
> I see jack_kctl_name_gen() will append "Jack" to the name if I don't
> name the pin "HDMI Jack". Do you mean that I could directly use "HDMI
> Jack" because ALSA uses the name finally?
> 

You're right and I just checked; the comment even says
'remove redundant " Jack" from src_name'

So yes, the current names are fine. Sorry about that.

Regards,
Angelo

Re: [PATCH 2/5] ASoC: mediatek: mt8188-mt6359: register hdmi/dp jack pins

[Trevor Wu (吳文良)]

On Wed, 2023-05-17 at 17:10 +0200, AngeloGioacchino Del Regno wrote:
> External email : Please do not click links or open attachments until
> you have verified the sender or the content.
> 
> 
> Il 17/05/23 14:10, Trevor Wu (吳文良) ha scritto:
> > On Wed, 2023-05-17 at 13:31 +0200, AngeloGioacchino Del Regno
> > wrote:
> > > External email : Please do not click links or open attachments
> > > until
> > > you have verified the sender or the content.
> > > 
> > > 
> > > Il 17/05/23 13:15, Trevor Wu ha scritto:
> > > > 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",
> > > 
> > > "HDMI Jack" is more consistent with the snd_soc_jack_new_pins()
> > > call
> > > performed
> > > later.
> > 
> > Hi Angelo,
> > 
> > I see jack_kctl_name_gen() will append "Jack" to the name if I
> > don't
> > name the pin "HDMI Jack". Do you mean that I could directly use
> > "HDMI
> > Jack" because ALSA uses the name finally?
> > 
> 
> You're right and I just checked; the comment even says
> 'remove redundant " Jack" from src_name'
> 
> So yes, the current names are fine. Sorry about that.
> 
> Regards,
> Angelo

I checked the original commit[1] for the function. The purpose of
jack_kctl_name_gen() is to ensure only one "Jack" shown in kctrl_name.
If "Jack" is already found in src_name, don't append "Jack" again.

I'm not sure which is better for pin name, but it's certain they get
the same kctrl_name finally.

[1] 
https://lore.kernel.org/all/1430140862-17207-3-git-send-email-yang.jie@intel.com


Thanks,
Trevor

[PATCH 3/5] 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 3/5] ASoC: mediatek: common: soundcard driver add dai_fmt support

[AngeloGioacchino Del Regno]

Il 17/05/23 13:15, Trevor Wu ha scritto:
> 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 },
> +	};
> +

This is an optional property and this function should not do anything if that
property is not found, so....

	/*
	 * Check "mediatek,clk-provider" to select the clock provider
	 * in SND_SOC_DAIFMT_CLOCK_PROVIDER_MASK area, if specified.
	 */
	if (!of_property_read_string(sub_node, "mediatek,clk-provider", &str))
		return 0;

...this allows you to reduce indentation as well.

> +	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) {

if (ret) {
  ...
}

> +			of_node_put(sub_node);
> +			return ret;
> +		}

Regards,
Angelo

Re: [PATCH 3/5] ASoC: mediatek: common: soundcard driver add dai_fmt support

[Trevor Wu (吳文良)]

On Wed, 2023-05-17 at 13:48 +0200, AngeloGioacchino Del Regno wrote:
> External email : Please do not click links or open attachments until
> you have verified the sender or the content.
> 
> 
> Il 17/05/23 13:15, Trevor Wu ha scritto:
> > 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 },
> > +     };
> > +
> 
> This is an optional property and this function should not do anything
> if that
> property is not found, so....
> 
>         /*
>          * Check "mediatek,clk-provider" to select the clock provider
>          * in SND_SOC_DAIFMT_CLOCK_PROVIDER_MASK area, if specified.
>          */
>         if (!of_property_read_string(sub_node, "mediatek,clk-
> provider", &str))
>                 return 0;
> 
> ...this allows you to reduce indentation as well.

Hi Angelo,

Actually, there are two properties handled by the function.
"dai-format" is parsed at snd_soc_daifmt_parse_format(), and
"mediatek,clk-provider" is parsed by of_property_read_string().
I have to go through both properties in the function, so it can't
return directly if "mediatek,clk-provider" is not found.

> 
> > +     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) {
> 
> if (ret) {
>   ...
> }
> 

Currently, this function only returns 0, but I'm not sure if it will
return a positive value for some other usage in the future.
Because a negative value is always returned for an error and the
preceding rule is also "ret < 0", I prefer "ret < 0" here. 

Thanks,
Trevor

> > +                     of_node_put(sub_node);
> > +                     return ret;
> > +             }
> 
> Regards,
> Angelo
> 

[PATCH 4/5] 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 | 267 +++++++++++++++++++++-
 2 files changed, 270 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..15fce2741f01 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,25 @@
 #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"
 
+/*
+ * 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 +159,15 @@ 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;
 };
 
 static struct snd_soc_jack_pin mt8188_hdmi_jack_pins[] = {
@@ -165,11 +184,50 @@ static struct snd_soc_jack_pin mt8188_dp_jack_pins[] = {
 	},
 };
 
+static struct snd_soc_jack_pin nau8825_jack_pins[] = {
+	{
+		.pin    = "Headphone",
+		.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_kcontrols[] = {
+	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_kcontrols[] = {
+	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_kcontrols[] = {
+	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),
@@ -451,6 +509,174 @@ 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_kcontrols,
+					ARRAY_SIZE(mt8188_dumb_spk_kcontrols));
+	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_kcontrols,
+					ARRAY_SIZE(mt8188_dual_spk_kcontrols));
+	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_kcontrols,
+					ARRAY_SIZE(mt8188_rear_spk_kcontrols));
+	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 mt8188_mt6359_priv *priv = snd_soc_card_get_drvdata(rtd->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_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] = {
@@ -726,6 +952,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,6 +1005,34 @@ 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;
+				}
+			}
 		}
 	}
 
@@ -795,11 +1052,19 @@ static struct mt8188_card_data mt8188_evb_card = {
 	.name = "mt8188_mt6359",
 };
 
+static struct mt8188_card_data mt8188_nau8825_card = {
+	.name = "mt8188_nau8825",
+};
+
 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 4/5] ASoC: mediatek: mt8188-mt6359: support new board with nau88255

[AngeloGioacchino Del Regno]

Il 17/05/23 13:15, Trevor Wu ha scritto:
> 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 | 267 +++++++++++++++++++++-
>   2 files changed, 270 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..15fce2741f01 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,25 @@
>   #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"
>   
> +/*
> + * 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 +159,15 @@ 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;
>   };
>   
>   static struct snd_soc_jack_pin mt8188_hdmi_jack_pins[] = {
> @@ -165,11 +184,50 @@ static struct snd_soc_jack_pin mt8188_dp_jack_pins[] = {
>   	},
>   };
>   
> +static struct snd_soc_jack_pin nau8825_jack_pins[] = {
> +	{
> +		.pin    = "Headphone",

One Intel driver using NAU8825 declares this as

		.pin	= "Headphone Jack",

can we please use the same name, so that we're able to eventually share the
same configuration in the userspace?

For reference, please check intel/avs/boards/nau8825.c

Thanks,
Angelo

Re: [PATCH 4/5] ASoC: mediatek: mt8188-mt6359: support new board with nau88255

[Trevor Wu (吳文良)]

On Wed, 2023-05-17 at 13:54 +0200, AngeloGioacchino Del Regno wrote:
> External email : Please do not click links or open attachments until
> you have verified the sender or the content.
> 
> 
> Il 17/05/23 13:15, Trevor Wu ha scritto:
> > 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 | 267
> > +++++++++++++++++++++-
> >   2 files changed, 270 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..15fce2741f01 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,25 @@
> >   #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"
> > 
> > +/*
> > + * 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 +159,15 @@ 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;
> >   };
> > 
> >   static struct snd_soc_jack_pin mt8188_hdmi_jack_pins[] = {
> > @@ -165,11 +184,50 @@ static struct snd_soc_jack_pin
> > mt8188_dp_jack_pins[] = {
> >       },
> >   };
> > 
> > +static struct snd_soc_jack_pin nau8825_jack_pins[] = {
> > +     {
> > +             .pin    = "Headphone",
> 
> One Intel driver using NAU8825 declares this as
> 
>                 .pin    = "Headphone Jack",
> 
> can we please use the same name, so that we're able to eventually
> share the
> same configuration in the userspace?
> 
> For reference, please check intel/avs/boards/nau8825.c
> 
> Thanks,
> Angelo

OK. I will replace "Headphone" with "Headphone Jack" in V2.

Thanks,
Trevor

[PATCH 5/5] ASoC: dt-bindings: mediatek,mt8188-mt6359: update properties

[Trevor Wu]

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

ADDA_BE is used to connect to mt6359. For the machine, it must be
fixed to the same codec and configured on the machine driver. Remove
ADDA_BE from items of link-name.

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 format, but only one dai-link builds link with codec.

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

diff --git a/Documentation/devicetree/bindings/sound/mediatek,mt8188-mt6359.yaml b/Documentation/devicetree/bindings/sound/mediatek,mt8188-mt6359.yaml
index 6640272b3f4f..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
@@ -42,7 +44,6 @@ patternProperties:
           we are going to update parameters in this node.
         items:
           enum:
-            - ADDA_BE
             - DPTX_BE
             - ETDM1_IN_BE
             - ETDM2_IN_BE
@@ -62,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
 
@@ -87,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

Re: [PATCH 5/5] ASoC: dt-bindings: mediatek,mt8188-mt6359: update properties

[Krzysztof Kozlowski]

On 17/05/2023 13:15, Trevor Wu wrote:
> Add compatible string "mediatek,mt8188-nau8825" to support new board
> with nau8825 codec.

Subject: everything can be update of properties. Rephrase to focus on
actual change, e.g. on adding NAU8825.

> 
> ADDA_BE is used to connect to mt6359. For the machine, it must be
> fixed to the same codec and configured on the machine driver. Remove
> ADDA_BE from items of link-name.

I don't understand the justification for this. If this is not a correct
link name for MT6359 setup, then it should be removed in its own patch.
If this is removed because of adding NAU8825, then why does it affect
MT6359?

> 
> 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 format, but only one dai-link builds link with codec.
> 
> Signed-off-by: Trevor Wu <trevor.wu@mediatek.com>
> ---
>  .../sound/mediatek,mt8188-mt6359.yaml         | 27 ++++++++++++++++---
>  1 file changed, 23 insertions(+), 4 deletions(-)
> 
> diff --git a/Documentation/devicetree/bindings/sound/mediatek,mt8188-mt6359.yaml b/Documentation/devicetree/bindings/sound/mediatek,mt8188-mt6359.yaml
> index 6640272b3f4f..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
> @@ -42,7 +44,6 @@ patternProperties:
>            we are going to update parameters in this node.
>          items:
>            enum:
> -            - ADDA_BE


Best regards,
Krzysztof

Re: [PATCH 5/5] ASoC: dt-bindings: mediatek,mt8188-mt6359: update properties

[Trevor Wu (吳文良)]

On Wed, 2023-05-17 at 16:43 +0200, Krzysztof Kozlowski wrote:
> External email : Please do not click links or open attachments until
> you have verified the sender or the content.
> 
> 
> On 17/05/2023 13:15, Trevor Wu wrote:
> > Add compatible string "mediatek,mt8188-nau8825" to support new
> > board
> > with nau8825 codec.
> 
> Subject: everything can be update of properties. Rephrase to focus on
> actual change, e.g. on adding NAU8825.
> 
OK. I will rename the topic in v2.

> > 
> > ADDA_BE is used to connect to mt6359. For the machine, it must be
> > fixed to the same codec and configured on the machine driver.
> > Remove
> > ADDA_BE from items of link-name.
> 
> I don't understand the justification for this. If this is not a
> correct
> link name for MT6359 setup, then it should be removed in its own
> patch.
> If this is removed because of adding NAU8825, then why does it affect
> MT6359?
> 

In the patch series, [PATCH 1/5] divides ADDA_BE into UL_SRC_BE and
DL_SRC_BE. I was going to replace the link name, but I found it
shouldn't be configured for the machine, because mt6359 is the only
codec for ADDA_BE and it's hardcoded in the machine driver. As a
result, I decided to remove it.

I will separate it from the patch, and put the new patch after [PATCH
1/5].

Thanks,
Trevor

> > 
> > 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 format, but only one dai-link builds link with codec.
> > 
> > Signed-off-by: Trevor Wu <trevor.wu@mediatek.com>
> > ---
> >  .../sound/mediatek,mt8188-mt6359.yaml         | 27
> > ++++++++++++++++---
> >  1 file changed, 23 insertions(+), 4 deletions(-)
> > 
> > diff --git
> > a/Documentation/devicetree/bindings/sound/mediatek,mt8188-
> > mt6359.yaml
> > b/Documentation/devicetree/bindings/sound/mediatek,mt8188-
> > mt6359.yaml
> > index 6640272b3f4f..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
> > @@ -42,7 +44,6 @@ patternProperties:
> >            we are going to update parameters in this node.
> >          items:
> >            enum:
> > -            - ADDA_BE
> 
> 
> Best regards,
> Krzysztof
> 

Re: [PATCH 5/5] ASoC: dt-bindings: mediatek,mt8188-mt6359: update properties

[Krzysztof Kozlowski]

On 18/05/2023 04:37, Trevor Wu (吳文良) wrote:
> On Wed, 2023-05-17 at 16:43 +0200, Krzysztof Kozlowski wrote:
>> External email : Please do not click links or open attachments until
>> you have verified the sender or the content.
>>
>>
>> On 17/05/2023 13:15, Trevor Wu wrote:
>>> Add compatible string "mediatek,mt8188-nau8825" to support new
>>> board
>>> with nau8825 codec.
>>
>> Subject: everything can be update of properties. Rephrase to focus on
>> actual change, e.g. on adding NAU8825.
>>
> OK. I will rename the topic in v2.
> 
>>>
>>> ADDA_BE is used to connect to mt6359. For the machine, it must be
>>> fixed to the same codec and configured on the machine driver.
>>> Remove
>>> ADDA_BE from items of link-name.
>>
>> I don't understand the justification for this. If this is not a
>> correct
>> link name for MT6359 setup, then it should be removed in its own
>> patch.
>> If this is removed because of adding NAU8825, then why does it affect
>> MT6359?
>>
> 
> In the patch series, [PATCH 1/5] divides ADDA_BE into UL_SRC_BE and
> DL_SRC_BE. I was going to replace the link name, but I found it
> shouldn't be configured for the machine, because mt6359 is the only
> codec for ADDA_BE and it's hardcoded in the machine driver. As a
> result, I decided to remove it.
> 
> I will separate it from the patch, and put the new patch after [PATCH
> 1/5].

Yes, please.

Best regards,
Krzysztof