LKML Archive on lore.kernel.org
help / color / mirror / Atom feed
* [PATCH 0/2] ASoC: mediatek: Add support for MT8195 sound card with rt1011 and rt5682
@ 2021-09-10 10:44 Trevor Wu
  2021-09-10 10:44 ` [PATCH 1/2] ASoC: mediatek: mt8195: add machine driver with mt6359, " Trevor Wu
                   ` (2 more replies)
  0 siblings, 3 replies; 9+ messages in thread
From: Trevor Wu @ 2021-09-10 10:44 UTC (permalink / raw)
  To: broonie, tiwai, robh+dt, matthias.bgg
  Cc: trevor.wu, alsa-devel, linux-mediatek, linux-arm-kernel,
	linux-kernel, devicetree, aaronyu

This series of patches adds support for mt8195 board with mt6359, rt1011
and rt5682.
Patches are based on broonie tree "for-next" branch.

Trevor Wu (2):
  ASoC: mediatek: mt8195: add machine driver with mt6359, rt1011 and
    rt5682
  dt-bindings: mediatek: mt8195: add mt8195-mt6359-rt1011-rt5682
    document

 .../sound/mt8195-mt6359-rt1011-rt5682.yaml    |   47 +
 sound/soc/mediatek/Kconfig                    |   15 +
 sound/soc/mediatek/mt8195/Makefile            |    1 +
 .../mt8195/mt8195-mt6359-rt1011-rt5682.c      | 1140 +++++++++++++++++
 4 files changed, 1203 insertions(+)
 create mode 100644 Documentation/devicetree/bindings/sound/mt8195-mt6359-rt1011-rt5682.yaml
 create mode 100644 sound/soc/mediatek/mt8195/mt8195-mt6359-rt1011-rt5682.c

-- 
2.18.0


^ permalink raw reply	[flat|nested] 9+ messages in thread

* [PATCH 1/2] ASoC: mediatek: mt8195: add machine driver with mt6359, rt1011 and rt5682
  2021-09-10 10:44 [PATCH 0/2] ASoC: mediatek: Add support for MT8195 sound card with rt1011 and rt5682 Trevor Wu
@ 2021-09-10 10:44 ` Trevor Wu
  2021-09-10 16:47   ` Pierre-Louis Bossart
  2021-09-10 10:44 ` [PATCH 2/2] dt-bindings: mediatek: mt8195: add mt8195-mt6359-rt1011-rt5682 document Trevor Wu
  2021-10-29 20:54 ` [PATCH 0/2] ASoC: mediatek: Add support for MT8195 sound card with rt1011 and rt5682 Mark Brown
  2 siblings, 1 reply; 9+ messages in thread
From: Trevor Wu @ 2021-09-10 10:44 UTC (permalink / raw)
  To: broonie, tiwai, robh+dt, matthias.bgg
  Cc: trevor.wu, alsa-devel, linux-mediatek, linux-arm-kernel,
	linux-kernel, devicetree, aaronyu

This patch adds support for mt8195 board with mt6359, rt1011 and rt5682.

Signed-off-by: Trevor Wu <trevor.wu@mediatek.com>
---
 sound/soc/mediatek/Kconfig                    |   15 +
 sound/soc/mediatek/mt8195/Makefile            |    1 +
 .../mt8195/mt8195-mt6359-rt1011-rt5682.c      | 1140 +++++++++++++++++
 3 files changed, 1156 insertions(+)
 create mode 100644 sound/soc/mediatek/mt8195/mt8195-mt6359-rt1011-rt5682.c

diff --git a/sound/soc/mediatek/Kconfig b/sound/soc/mediatek/Kconfig
index 81ad2dcee9eb..8c798ff20aab 100644
--- a/sound/soc/mediatek/Kconfig
+++ b/sound/soc/mediatek/Kconfig
@@ -212,3 +212,18 @@ config SND_SOC_MT8195_MT6359_RT1019_RT5682
 	  with the MT6359 RT1019 RT5682 audio codec.
 	  Select Y if you have such device.
 	  If unsure select "N".
+
+config SND_SOC_MT8195_MT6359_RT1011_RT5682
+	tristate "ASoC Audio driver for MT8195 with MT6359 RT1011 RT5682 codec"
+	depends on I2C
+	depends on SND_SOC_MT8195 && MTK_PMIC_WRAP
+	select SND_SOC_MT6359
+	select SND_SOC_RT1011
+	select SND_SOC_RT5682_I2C
+	select SND_SOC_DMIC
+	select SND_SOC_HDMI_CODEC
+	help
+	  This adds ASoC driver for Mediatek MT8195 boards
+	  with the MT6359 RT1011 RT5682 audio codec.
+	  Select Y if you have such device.
+	  If unsure select "N".
diff --git a/sound/soc/mediatek/mt8195/Makefile b/sound/soc/mediatek/mt8195/Makefile
index 44775f400b40..e5f0df5010b6 100644
--- a/sound/soc/mediatek/mt8195/Makefile
+++ b/sound/soc/mediatek/mt8195/Makefile
@@ -13,3 +13,4 @@ obj-$(CONFIG_SND_SOC_MT8195) += snd-soc-mt8195-afe.o
 
 # machine driver
 obj-$(CONFIG_SND_SOC_MT8195_MT6359_RT1019_RT5682) += mt8195-mt6359-rt1019-rt5682.o
+obj-$(CONFIG_SND_SOC_MT8195_MT6359_RT1011_RT5682) += mt8195-mt6359-rt1011-rt5682.o
diff --git a/sound/soc/mediatek/mt8195/mt8195-mt6359-rt1011-rt5682.c b/sound/soc/mediatek/mt8195/mt8195-mt6359-rt1011-rt5682.c
new file mode 100644
index 000000000000..a84ba77d6432
--- /dev/null
+++ b/sound/soc/mediatek/mt8195/mt8195-mt6359-rt1011-rt5682.c
@@ -0,0 +1,1140 @@
+// SPDX-License-Identifier: GPL-2.0
+//
+// mt8195-mt6359-rt1011-rt5682.c  --
+//	MT8195-MT6359-RT1011-RT6358 ALSA SoC machine driver
+//
+// Copyright (c) 2021 MediaTek Inc.
+// Author: Trevor Wu <trevor.wu@mediatek.com>
+//
+
+#include <linux/input.h>
+#include <linux/module.h>
+#include <linux/pm_runtime.h>
+#include <sound/jack.h>
+#include <sound/pcm_params.h>
+#include <sound/rt5682.h>
+#include <sound/soc.h>
+#include "../../codecs/mt6359.h"
+#include "../../codecs/rt1011.h"
+#include "../../codecs/rt5682.h"
+#include "../common/mtk-afe-platform-driver.h"
+#include "mt8195-afe-common.h"
+
+#define RT1011_CODEC_DAI	"rt1011-aif"
+#define RT1011_DEV0_NAME	"rt1011.2-0038"
+#define RT1011_DEV1_NAME	"rt1011.2-0039"
+
+#define RT5682_CODEC_DAI	"rt5682-aif1"
+#define RT5682_DEV0_NAME	"rt5682.2-001a"
+
+struct mt8195_mt6359_rt1011_rt5682_priv {
+	struct snd_soc_jack headset_jack;
+	struct snd_soc_jack dp_jack;
+	struct snd_soc_jack hdmi_jack;
+};
+
+static const struct snd_soc_dapm_widget
+mt8195_mt6359_rt1011_rt5682_widgets[] = {
+	SND_SOC_DAPM_SPK("Left Speaker", NULL),
+	SND_SOC_DAPM_SPK("Right Speaker", NULL),
+	SND_SOC_DAPM_HP("Headphone Jack", NULL),
+	SND_SOC_DAPM_MIC("Headset Mic", NULL),
+};
+
+static const struct snd_soc_dapm_route mt8195_mt6359_rt1011_rt5682_routes[] = {
+	/* speaker */
+	{ "Left Speaker", NULL, "Left SPO" },
+	{ "Right Speaker", NULL, "Right SPO" },
+	/* headset */
+	{ "Headphone Jack", NULL, "HPOL" },
+	{ "Headphone Jack", NULL, "HPOR" },
+	{ "IN1P", NULL, "Headset Mic" },
+};
+
+static const struct snd_kcontrol_new mt8195_mt6359_rt1011_rt5682_controls[] = {
+	SOC_DAPM_PIN_SWITCH("Left Speaker"),
+	SOC_DAPM_PIN_SWITCH("Right Speaker"),
+	SOC_DAPM_PIN_SWITCH("Headphone Jack"),
+	SOC_DAPM_PIN_SWITCH("Headset Mic"),
+};
+
+static int mt8195_rt5682_etdm_hw_params(struct snd_pcm_substream *substream,
+					struct snd_pcm_hw_params *params)
+{
+	struct snd_soc_pcm_runtime *rtd = substream->private_data;
+	struct snd_soc_card *card = rtd->card;
+	struct snd_soc_dai *cpu_dai = asoc_rtd_to_cpu(rtd, 0);
+	struct snd_soc_dai *codec_dai = asoc_rtd_to_codec(rtd, 0);
+	unsigned int rate = params_rate(params);
+	unsigned int mclk_fs_ratio = 128;
+	unsigned int mclk_fs = rate * mclk_fs_ratio;
+	int bitwidth;
+	int ret;
+
+	bitwidth = snd_pcm_format_width(params_format(params));
+	if (bitwidth < 0) {
+		dev_err(card->dev, "invalid bit width: %d\n", bitwidth);
+		return bitwidth;
+	}
+
+	ret = snd_soc_dai_set_tdm_slot(codec_dai, 0x00, 0x0, 0x2, bitwidth);
+	if (ret) {
+		dev_err(card->dev, "failed to set tdm slot\n");
+		return ret;
+	}
+
+	ret = snd_soc_dai_set_pll(codec_dai, RT5682_PLL1,
+				  RT5682_PLL1_S_BCLK1,
+				  params_rate(params) * 64,
+				  params_rate(params) * 512);
+	if (ret) {
+		dev_err(card->dev, "failed to set pll\n");
+		return ret;
+	}
+
+	ret = snd_soc_dai_set_sysclk(codec_dai,
+				     RT5682_SCLK_S_PLL1,
+				     params_rate(params) * 512,
+				     SND_SOC_CLOCK_IN);
+	if (ret) {
+		dev_err(card->dev, "failed to set sysclk\n");
+		return ret;
+	}
+
+	return snd_soc_dai_set_sysclk(cpu_dai, 0, mclk_fs, SND_SOC_CLOCK_OUT);
+}
+
+static const struct snd_soc_ops mt8195_rt5682_etdm_ops = {
+	.hw_params = mt8195_rt5682_etdm_hw_params,
+};
+
+static int mt8195_rt1011_etdm_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;
+	struct snd_soc_card *card = rtd->card;
+	int srate, i, ret = 0;
+
+	srate = params_rate(params);
+
+	for_each_rtd_codec_dais(rtd, i, codec_dai) {
+		ret = snd_soc_dai_set_pll(codec_dai, 0, RT1011_PLL1_S_BCLK,
+					  64 * srate, 256 * srate);
+		if (ret < 0) {
+			dev_err(card->dev, "codec_dai clock not set\n");
+			return ret;
+		}
+
+		ret = snd_soc_dai_set_sysclk(codec_dai,
+					     RT1011_FS_SYS_PRE_S_PLL1,
+					     256 * srate, SND_SOC_CLOCK_IN);
+		if (ret < 0) {
+			dev_err(card->dev, "codec_dai clock not set\n");
+			return ret;
+		}
+	}
+	return ret;
+}
+
+static const struct snd_soc_ops mt8195_rt1011_etdm_ops = {
+	.hw_params = mt8195_rt1011_etdm_hw_params,
+};
+
+#define CKSYS_AUD_TOP_CFG 0x032c
+#define CKSYS_AUD_TOP_MON 0x0330
+
+static int mt8195_mt6359_mtkaif_calibration(struct snd_soc_pcm_runtime *rtd)
+{
+	struct snd_soc_component *cmpnt_afe =
+		snd_soc_rtdcom_lookup(rtd, AFE_PCM_NAME);
+	struct snd_soc_component *cmpnt_codec =
+		asoc_rtd_to_codec(rtd, 0)->component;
+	struct mtk_base_afe *afe = snd_soc_component_get_drvdata(cmpnt_afe);
+	struct mt8195_afe_private *afe_priv = afe->platform_priv;
+	struct mtkaif_param *param = &afe_priv->mtkaif_params;
+	int phase;
+	unsigned int monitor;
+	int mtkaif_calibration_num_phase;
+	int test_done_1, test_done_2, test_done_3;
+	int cycle_1, cycle_2, cycle_3;
+	int prev_cycle_1, prev_cycle_2, prev_cycle_3;
+	int chosen_phase_1, chosen_phase_2, chosen_phase_3;
+	int counter;
+	bool mtkaif_calibration_ok;
+	int mtkaif_chosen_phase[MT8195_MTKAIF_MISO_NUM];
+	int mtkaif_phase_cycle[MT8195_MTKAIF_MISO_NUM];
+	int i;
+
+	dev_info(afe->dev, "%s(), start\n", __func__);
+
+	param->mtkaif_calibration_ok = false;
+	for (i = 0; i < MT8195_MTKAIF_MISO_NUM; i++) {
+		param->mtkaif_chosen_phase[i] = -1;
+		param->mtkaif_phase_cycle[i] = 0;
+		mtkaif_chosen_phase[i] = -1;
+		mtkaif_phase_cycle[i] = 0;
+	}
+
+	if (IS_ERR(afe_priv->topckgen)) {
+		dev_info(afe->dev, "%s() Cannot find topckgen controller\n",
+			 __func__);
+		return 0;
+	}
+
+	pm_runtime_get_sync(afe->dev);
+	mt6359_mtkaif_calibration_enable(cmpnt_codec);
+
+	/* set test type to synchronizer pulse */
+	regmap_update_bits(afe_priv->topckgen,
+			   CKSYS_AUD_TOP_CFG, 0xffff, 0x4);
+	mtkaif_calibration_num_phase = 42;	/* mt6359: 0 ~ 42 */
+	mtkaif_calibration_ok = true;
+
+	for (phase = 0;
+	     phase <= mtkaif_calibration_num_phase && mtkaif_calibration_ok;
+	     phase++) {
+		mt6359_set_mtkaif_calibration_phase(cmpnt_codec,
+						    phase, phase, phase);
+
+		regmap_update_bits(afe_priv->topckgen,
+				   CKSYS_AUD_TOP_CFG, 0x1, 0x1);
+
+		test_done_1 = 0;
+		test_done_2 = 0;
+		test_done_3 = 0;
+		cycle_1 = -1;
+		cycle_2 = -1;
+		cycle_3 = -1;
+		counter = 0;
+		while (!(test_done_1 & test_done_2 & test_done_3)) {
+			regmap_read(afe_priv->topckgen,
+				    CKSYS_AUD_TOP_MON, &monitor);
+			test_done_1 = (monitor >> 28) & 0x1;
+			test_done_2 = (monitor >> 29) & 0x1;
+			test_done_3 = (monitor >> 30) & 0x1;
+			if (test_done_1 == 1)
+				cycle_1 = monitor & 0xf;
+
+			if (test_done_2 == 1)
+				cycle_2 = (monitor >> 4) & 0xf;
+
+			if (test_done_3 == 1)
+				cycle_3 = (monitor >> 8) & 0xf;
+
+			/* handle if never test done */
+			if (++counter > 10000) {
+				dev_info(afe->dev, "%s(), test fail, cycle_1 %d, cycle_2 %d, cycle_3 %d, monitor 0x%x\n",
+					 __func__,
+					 cycle_1, cycle_2, cycle_3, monitor);
+				mtkaif_calibration_ok = false;
+				break;
+			}
+		}
+
+		if (phase == 0) {
+			prev_cycle_1 = cycle_1;
+			prev_cycle_2 = cycle_2;
+			prev_cycle_3 = cycle_3;
+		}
+
+		if (cycle_1 != prev_cycle_1 &&
+		    mtkaif_chosen_phase[MT8195_MTKAIF_MISO_0] < 0) {
+			mtkaif_chosen_phase[MT8195_MTKAIF_MISO_0] = phase - 1;
+			mtkaif_phase_cycle[MT8195_MTKAIF_MISO_0] = prev_cycle_1;
+		}
+
+		if (cycle_2 != prev_cycle_2 &&
+		    mtkaif_chosen_phase[MT8195_MTKAIF_MISO_1] < 0) {
+			mtkaif_chosen_phase[MT8195_MTKAIF_MISO_1] = phase - 1;
+			mtkaif_phase_cycle[MT8195_MTKAIF_MISO_1] = prev_cycle_2;
+		}
+
+		if (cycle_3 != prev_cycle_3 &&
+		    mtkaif_chosen_phase[MT8195_MTKAIF_MISO_2] < 0) {
+			mtkaif_chosen_phase[MT8195_MTKAIF_MISO_2] = phase - 1;
+			mtkaif_phase_cycle[MT8195_MTKAIF_MISO_2] = prev_cycle_3;
+		}
+
+		regmap_update_bits(afe_priv->topckgen,
+				   CKSYS_AUD_TOP_CFG, 0x1, 0x0);
+
+		if (mtkaif_chosen_phase[MT8195_MTKAIF_MISO_0] >= 0 &&
+		    mtkaif_chosen_phase[MT8195_MTKAIF_MISO_1] >= 0 &&
+		    mtkaif_chosen_phase[MT8195_MTKAIF_MISO_2] >= 0)
+			break;
+	}
+
+	if (mtkaif_chosen_phase[MT8195_MTKAIF_MISO_0] < 0) {
+		mtkaif_calibration_ok = false;
+		chosen_phase_1 = 0;
+	} else {
+		chosen_phase_1 = mtkaif_chosen_phase[MT8195_MTKAIF_MISO_0];
+	}
+
+	if (mtkaif_chosen_phase[MT8195_MTKAIF_MISO_1] < 0) {
+		mtkaif_calibration_ok = false;
+		chosen_phase_2 = 0;
+	} else {
+		chosen_phase_2 = mtkaif_chosen_phase[MT8195_MTKAIF_MISO_1];
+	}
+
+	if (mtkaif_chosen_phase[MT8195_MTKAIF_MISO_2] < 0) {
+		mtkaif_calibration_ok = false;
+		chosen_phase_3 = 0;
+	} else {
+		chosen_phase_3 = mtkaif_chosen_phase[MT8195_MTKAIF_MISO_2];
+	}
+
+	mt6359_set_mtkaif_calibration_phase(cmpnt_codec,
+					    chosen_phase_1,
+					    chosen_phase_2,
+					    chosen_phase_3);
+
+	mt6359_mtkaif_calibration_disable(cmpnt_codec);
+	pm_runtime_put(afe->dev);
+
+	param->mtkaif_calibration_ok = mtkaif_calibration_ok;
+	param->mtkaif_chosen_phase[MT8195_MTKAIF_MISO_0] = chosen_phase_1;
+	param->mtkaif_chosen_phase[MT8195_MTKAIF_MISO_1] = chosen_phase_2;
+	param->mtkaif_chosen_phase[MT8195_MTKAIF_MISO_2] = chosen_phase_3;
+	for (i = 0; i < MT8195_MTKAIF_MISO_NUM; i++)
+		param->mtkaif_phase_cycle[i] = mtkaif_phase_cycle[i];
+
+	dev_info(afe->dev, "%s(), end, calibration ok %d\n",
+		 __func__, param->mtkaif_calibration_ok);
+
+	return 0;
+}
+
+static int mt8195_mt6359_init(struct snd_soc_pcm_runtime *rtd)
+{
+	struct snd_soc_component *cmpnt_codec =
+		asoc_rtd_to_codec(rtd, 0)->component;
+
+	/* set mtkaif protocol */
+	mt6359_set_mtkaif_protocol(cmpnt_codec,
+				   MT6359_MTKAIF_PROTOCOL_2_CLK_P2);
+
+	/* mtkaif calibration */
+	mt8195_mt6359_mtkaif_calibration(rtd);
+
+	return 0;
+}
+
+static int mt8195_rt5682_init(struct snd_soc_pcm_runtime *rtd)
+{
+	struct snd_soc_component *cmpnt_codec =
+		asoc_rtd_to_codec(rtd, 0)->component;
+	struct mt8195_mt6359_rt1011_rt5682_priv *priv =
+		snd_soc_card_get_drvdata(rtd->card);
+	struct snd_soc_jack *jack = &priv->headset_jack;
+	int ret;
+
+	ret = snd_soc_card_jack_new(rtd->card, "Headset Jack",
+				    SND_JACK_HEADSET | SND_JACK_BTN_0 |
+				    SND_JACK_BTN_1 | SND_JACK_BTN_2 |
+				    SND_JACK_BTN_3,
+				    jack, NULL, 0);
+	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(cmpnt_codec, jack, NULL);
+	if (ret) {
+		dev_err(rtd->dev, "Headset Jack set failed: %d\n", ret);
+		return ret;
+	}
+
+	return 0;
+};
+
+static int mt8195_etdm_hw_params_fixup(struct snd_soc_pcm_runtime *rtd,
+				       struct snd_pcm_hw_params *params)
+{
+	/* fix BE i2s format to 32bit, clean param mask first */
+	snd_mask_reset_range(hw_param_mask(params, SNDRV_PCM_HW_PARAM_FORMAT),
+			     0, (__force unsigned int)SNDRV_PCM_FORMAT_LAST);
+
+	params_set_format(params, SNDRV_PCM_FORMAT_S24_LE);
+
+	return 0;
+}
+
+static int mt8195_hdmitx_dptx_startup(struct snd_pcm_substream *substream)
+{
+	static const unsigned int rates[] = {
+		48000
+	};
+	static const unsigned int channels[] = {
+		2, 4, 6, 8
+	};
+	static const struct snd_pcm_hw_constraint_list constraints_rates = {
+		.count = ARRAY_SIZE(rates),
+		.list  = rates,
+		.mask = 0,
+	};
+	static const struct snd_pcm_hw_constraint_list constraints_channels = {
+		.count = ARRAY_SIZE(channels),
+		.list  = channels,
+		.mask = 0,
+	};
+
+	struct snd_soc_pcm_runtime *rtd = asoc_substream_to_rtd(substream);
+	struct snd_pcm_runtime *runtime = substream->runtime;
+	int ret;
+
+	ret = snd_pcm_hw_constraint_list(runtime, 0,
+					 SNDRV_PCM_HW_PARAM_RATE,
+					 &constraints_rates);
+	if (ret < 0) {
+		dev_err(rtd->dev, "hw_constraint_list rate failed\n");
+		return ret;
+	}
+
+	ret = snd_pcm_hw_constraint_list(runtime, 0,
+					 SNDRV_PCM_HW_PARAM_CHANNELS,
+					 &constraints_channels);
+	if (ret < 0) {
+		dev_err(rtd->dev, "hw_constraint_list channel failed\n");
+		return ret;
+	}
+
+	return 0;
+}
+
+static const struct snd_soc_ops mt8195_hdmitx_dptx_playback_ops = {
+	.startup = mt8195_hdmitx_dptx_startup,
+};
+
+static int mt8195_dptx_hw_params(struct snd_pcm_substream *substream,
+				 struct snd_pcm_hw_params *params)
+{
+	struct snd_soc_pcm_runtime *rtd = substream->private_data;
+	struct snd_soc_dai *cpu_dai = asoc_rtd_to_cpu(rtd, 0);
+	unsigned int rate = params_rate(params);
+	unsigned int mclk_fs_ratio = 256;
+	unsigned int mclk_fs = rate * mclk_fs_ratio;
+
+	return snd_soc_dai_set_sysclk(cpu_dai, 0, mclk_fs,
+				      SND_SOC_CLOCK_OUT);
+}
+
+static struct snd_soc_ops mt8195_dptx_ops = {
+	.hw_params = mt8195_dptx_hw_params,
+};
+
+static int mt8195_dptx_codec_init(struct snd_soc_pcm_runtime *rtd)
+{
+	struct mt8195_mt6359_rt1011_rt5682_priv *priv =
+		snd_soc_card_get_drvdata(rtd->card);
+	struct snd_soc_component *cmpnt_codec =
+		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, NULL, 0);
+	if (ret)
+		return ret;
+
+	return snd_soc_component_set_jack(cmpnt_codec, &priv->dp_jack, NULL);
+}
+
+static int mt8195_hdmi_codec_init(struct snd_soc_pcm_runtime *rtd)
+{
+	struct mt8195_mt6359_rt1011_rt5682_priv *priv =
+		snd_soc_card_get_drvdata(rtd->card);
+	struct snd_soc_component *cmpnt_codec =
+		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, NULL, 0);
+	if (ret)
+		return ret;
+
+	return snd_soc_component_set_jack(cmpnt_codec, &priv->hdmi_jack, NULL);
+}
+
+static int mt8195_hdmitx_dptx_hw_params_fixup(struct snd_soc_pcm_runtime *rtd,
+					      struct snd_pcm_hw_params *params)
+
+{
+	/* fix BE i2s format to 32bit, clean param mask first */
+	snd_mask_reset_range(hw_param_mask(params, SNDRV_PCM_HW_PARAM_FORMAT),
+			     0, (__force unsigned int)SNDRV_PCM_FORMAT_LAST);
+
+	params_set_format(params, SNDRV_PCM_FORMAT_S24_LE);
+
+	return 0;
+}
+
+static int mt8195_playback_startup(struct snd_pcm_substream *substream)
+{
+	static const unsigned int rates[] = {
+		48000
+	};
+	static const unsigned int channels[] = {
+		2
+	};
+	static const struct snd_pcm_hw_constraint_list constraints_rates = {
+		.count = ARRAY_SIZE(rates),
+		.list  = rates,
+		.mask = 0,
+	};
+	static const struct snd_pcm_hw_constraint_list constraints_channels = {
+		.count = ARRAY_SIZE(channels),
+		.list  = channels,
+		.mask = 0,
+	};
+
+	struct snd_soc_pcm_runtime *rtd = asoc_substream_to_rtd(substream);
+	struct snd_pcm_runtime *runtime = substream->runtime;
+	int ret;
+
+	ret = snd_pcm_hw_constraint_list(runtime, 0,
+					 SNDRV_PCM_HW_PARAM_RATE,
+					 &constraints_rates);
+	if (ret < 0) {
+		dev_err(rtd->dev, "hw_constraint_list rate failed\n");
+		return ret;
+	}
+
+	ret = snd_pcm_hw_constraint_list(runtime, 0,
+					 SNDRV_PCM_HW_PARAM_CHANNELS,
+					 &constraints_channels);
+	if (ret < 0) {
+		dev_err(rtd->dev, "hw_constraint_list channel failed\n");
+		return ret;
+	}
+
+	return 0;
+}
+
+static const struct snd_soc_ops mt8195_playback_ops = {
+	.startup = mt8195_playback_startup,
+};
+
+static int mt8195_capture_startup(struct snd_pcm_substream *substream)
+{
+	static const unsigned int rates[] = {
+		48000
+	};
+	static const unsigned int channels[] = {
+		1, 2
+	};
+	static const struct snd_pcm_hw_constraint_list constraints_rates = {
+		.count = ARRAY_SIZE(rates),
+		.list  = rates,
+		.mask = 0,
+	};
+	static const struct snd_pcm_hw_constraint_list constraints_channels = {
+		.count = ARRAY_SIZE(channels),
+		.list  = channels,
+		.mask = 0,
+	};
+
+	struct snd_soc_pcm_runtime *rtd = asoc_substream_to_rtd(substream);
+	struct snd_pcm_runtime *runtime = substream->runtime;
+	int ret;
+
+	ret = snd_pcm_hw_constraint_list(runtime, 0,
+					 SNDRV_PCM_HW_PARAM_RATE,
+					 &constraints_rates);
+	if (ret < 0) {
+		dev_err(rtd->dev, "hw_constraint_list rate failed\n");
+		return ret;
+	}
+
+	ret = snd_pcm_hw_constraint_list(runtime, 0,
+					 SNDRV_PCM_HW_PARAM_CHANNELS,
+					 &constraints_channels);
+	if (ret < 0) {
+		dev_err(rtd->dev, "hw_constraint_list channel failed\n");
+		return ret;
+	}
+
+	return 0;
+}
+
+static const struct snd_soc_ops mt8195_capture_ops = {
+	.startup = mt8195_capture_startup,
+};
+
+enum {
+	DAI_LINK_DL2_FE,
+	DAI_LINK_DL3_FE,
+	DAI_LINK_DL6_FE,
+	DAI_LINK_DL7_FE,
+	DAI_LINK_DL8_FE,
+	DAI_LINK_DL10_FE,
+	DAI_LINK_DL11_FE,
+	DAI_LINK_UL1_FE,
+	DAI_LINK_UL2_FE,
+	DAI_LINK_UL3_FE,
+	DAI_LINK_UL4_FE,
+	DAI_LINK_UL5_FE,
+	DAI_LINK_UL6_FE,
+	DAI_LINK_UL8_FE,
+	DAI_LINK_UL9_FE,
+	DAI_LINK_UL10_FE,
+	DAI_LINK_DL_SRC_BE,
+	DAI_LINK_DPTX_BE,
+	DAI_LINK_ETDM1_IN_BE,
+	DAI_LINK_ETDM2_IN_BE,
+	DAI_LINK_ETDM1_OUT_BE,
+	DAI_LINK_ETDM2_OUT_BE,
+	DAI_LINK_ETDM3_OUT_BE,
+	DAI_LINK_PCM1_BE,
+	DAI_LINK_UL_SRC1_BE,
+	DAI_LINK_UL_SRC2_BE,
+};
+
+/* FE */
+SND_SOC_DAILINK_DEFS(DL2_FE,
+		     DAILINK_COMP_ARRAY(COMP_CPU("DL2")),
+		     DAILINK_COMP_ARRAY(COMP_DUMMY()),
+		     DAILINK_COMP_ARRAY(COMP_EMPTY()));
+
+SND_SOC_DAILINK_DEFS(DL3_FE,
+		     DAILINK_COMP_ARRAY(COMP_CPU("DL3")),
+		     DAILINK_COMP_ARRAY(COMP_DUMMY()),
+		     DAILINK_COMP_ARRAY(COMP_EMPTY()));
+
+SND_SOC_DAILINK_DEFS(DL6_FE,
+		     DAILINK_COMP_ARRAY(COMP_CPU("DL6")),
+		     DAILINK_COMP_ARRAY(COMP_DUMMY()),
+		     DAILINK_COMP_ARRAY(COMP_EMPTY()));
+
+SND_SOC_DAILINK_DEFS(DL7_FE,
+		     DAILINK_COMP_ARRAY(COMP_CPU("DL7")),
+		     DAILINK_COMP_ARRAY(COMP_DUMMY()),
+		     DAILINK_COMP_ARRAY(COMP_EMPTY()));
+
+SND_SOC_DAILINK_DEFS(DL8_FE,
+		     DAILINK_COMP_ARRAY(COMP_CPU("DL8")),
+		     DAILINK_COMP_ARRAY(COMP_DUMMY()),
+		     DAILINK_COMP_ARRAY(COMP_EMPTY()));
+
+SND_SOC_DAILINK_DEFS(DL10_FE,
+		     DAILINK_COMP_ARRAY(COMP_CPU("DL10")),
+		     DAILINK_COMP_ARRAY(COMP_DUMMY()),
+		     DAILINK_COMP_ARRAY(COMP_EMPTY()));
+
+SND_SOC_DAILINK_DEFS(DL11_FE,
+		     DAILINK_COMP_ARRAY(COMP_CPU("DL11")),
+		     DAILINK_COMP_ARRAY(COMP_DUMMY()),
+		     DAILINK_COMP_ARRAY(COMP_EMPTY()));
+
+SND_SOC_DAILINK_DEFS(UL1_FE,
+		     DAILINK_COMP_ARRAY(COMP_CPU("UL1")),
+		     DAILINK_COMP_ARRAY(COMP_DUMMY()),
+		     DAILINK_COMP_ARRAY(COMP_EMPTY()));
+
+SND_SOC_DAILINK_DEFS(UL2_FE,
+		     DAILINK_COMP_ARRAY(COMP_CPU("UL2")),
+		     DAILINK_COMP_ARRAY(COMP_DUMMY()),
+		     DAILINK_COMP_ARRAY(COMP_EMPTY()));
+
+SND_SOC_DAILINK_DEFS(UL3_FE,
+		     DAILINK_COMP_ARRAY(COMP_CPU("UL3")),
+		     DAILINK_COMP_ARRAY(COMP_DUMMY()),
+		     DAILINK_COMP_ARRAY(COMP_EMPTY()));
+
+SND_SOC_DAILINK_DEFS(UL4_FE,
+		     DAILINK_COMP_ARRAY(COMP_CPU("UL4")),
+		     DAILINK_COMP_ARRAY(COMP_DUMMY()),
+		     DAILINK_COMP_ARRAY(COMP_EMPTY()));
+
+SND_SOC_DAILINK_DEFS(UL5_FE,
+		     DAILINK_COMP_ARRAY(COMP_CPU("UL5")),
+		     DAILINK_COMP_ARRAY(COMP_DUMMY()),
+		     DAILINK_COMP_ARRAY(COMP_EMPTY()));
+
+SND_SOC_DAILINK_DEFS(UL6_FE,
+		     DAILINK_COMP_ARRAY(COMP_CPU("UL6")),
+		     DAILINK_COMP_ARRAY(COMP_DUMMY()),
+		     DAILINK_COMP_ARRAY(COMP_EMPTY()));
+
+SND_SOC_DAILINK_DEFS(UL8_FE,
+		     DAILINK_COMP_ARRAY(COMP_CPU("UL8")),
+		     DAILINK_COMP_ARRAY(COMP_DUMMY()),
+		     DAILINK_COMP_ARRAY(COMP_EMPTY()));
+
+SND_SOC_DAILINK_DEFS(UL9_FE,
+		     DAILINK_COMP_ARRAY(COMP_CPU("UL9")),
+		     DAILINK_COMP_ARRAY(COMP_DUMMY()),
+		     DAILINK_COMP_ARRAY(COMP_EMPTY()));
+
+SND_SOC_DAILINK_DEFS(UL10_FE,
+		     DAILINK_COMP_ARRAY(COMP_CPU("UL10")),
+		     DAILINK_COMP_ARRAY(COMP_DUMMY()),
+		     DAILINK_COMP_ARRAY(COMP_EMPTY()));
+
+/* BE */
+SND_SOC_DAILINK_DEFS(DL_SRC_BE,
+		     DAILINK_COMP_ARRAY(COMP_CPU("DL_SRC")),
+		     DAILINK_COMP_ARRAY(COMP_CODEC("mt6359-sound",
+						   "mt6359-snd-codec-aif1")),
+		     DAILINK_COMP_ARRAY(COMP_EMPTY()));
+
+SND_SOC_DAILINK_DEFS(DPTX_BE,
+		     DAILINK_COMP_ARRAY(COMP_CPU("DPTX")),
+		     DAILINK_COMP_ARRAY(COMP_DUMMY()),
+		     DAILINK_COMP_ARRAY(COMP_EMPTY()));
+
+SND_SOC_DAILINK_DEFS(ETDM1_IN_BE,
+		     DAILINK_COMP_ARRAY(COMP_CPU("ETDM1_IN")),
+		     DAILINK_COMP_ARRAY(COMP_DUMMY()),
+		     DAILINK_COMP_ARRAY(COMP_EMPTY()));
+
+SND_SOC_DAILINK_DEFS(ETDM2_IN_BE,
+		     DAILINK_COMP_ARRAY(COMP_CPU("ETDM2_IN")),
+		     DAILINK_COMP_ARRAY(COMP_CODEC(RT5682_DEV0_NAME,
+						   RT5682_CODEC_DAI)),
+		     DAILINK_COMP_ARRAY(COMP_EMPTY()));
+
+SND_SOC_DAILINK_DEFS(ETDM1_OUT_BE,
+		     DAILINK_COMP_ARRAY(COMP_CPU("ETDM1_OUT")),
+		     DAILINK_COMP_ARRAY(COMP_CODEC(RT5682_DEV0_NAME,
+						   RT5682_CODEC_DAI)),
+		     DAILINK_COMP_ARRAY(COMP_EMPTY()));
+
+SND_SOC_DAILINK_DEFS(ETDM2_OUT_BE,
+		     DAILINK_COMP_ARRAY(COMP_CPU("ETDM2_OUT")),
+		     DAILINK_COMP_ARRAY(COMP_CODEC(RT1011_DEV0_NAME,
+						   RT1011_CODEC_DAI),
+					COMP_CODEC(RT1011_DEV1_NAME,
+						   RT1011_CODEC_DAI)),
+		     DAILINK_COMP_ARRAY(COMP_EMPTY()));
+
+SND_SOC_DAILINK_DEFS(ETDM3_OUT_BE,
+		     DAILINK_COMP_ARRAY(COMP_CPU("ETDM3_OUT")),
+		     DAILINK_COMP_ARRAY(COMP_DUMMY()),
+		     DAILINK_COMP_ARRAY(COMP_EMPTY()));
+
+SND_SOC_DAILINK_DEFS(PCM1_BE,
+		     DAILINK_COMP_ARRAY(COMP_CPU("PCM1")),
+		     DAILINK_COMP_ARRAY(COMP_DUMMY()),
+		     DAILINK_COMP_ARRAY(COMP_EMPTY()));
+
+SND_SOC_DAILINK_DEFS(UL_SRC1_BE,
+		     DAILINK_COMP_ARRAY(COMP_CPU("UL_SRC1")),
+		     DAILINK_COMP_ARRAY(COMP_CODEC("mt6359-sound",
+						   "mt6359-snd-codec-aif1"),
+					COMP_CODEC("dmic-codec",
+						   "dmic-hifi")),
+		     DAILINK_COMP_ARRAY(COMP_EMPTY()));
+
+SND_SOC_DAILINK_DEFS(UL_SRC2_BE,
+		     DAILINK_COMP_ARRAY(COMP_CPU("UL_SRC2")),
+		     DAILINK_COMP_ARRAY(COMP_CODEC("mt6359-sound",
+						   "mt6359-snd-codec-aif2")),
+		     DAILINK_COMP_ARRAY(COMP_EMPTY()));
+
+static struct snd_soc_dai_link mt8195_mt6359_rt1011_rt5682_dai_links[] = {
+	/* FE */
+	[DAI_LINK_DL2_FE] = {
+		.name = "DL2_FE",
+		.stream_name = "DL2 Playback",
+		.trigger = {
+			SND_SOC_DPCM_TRIGGER_POST,
+			SND_SOC_DPCM_TRIGGER_POST,
+		},
+		.dynamic = 1,
+		.dpcm_playback = 1,
+		.ops = &mt8195_playback_ops,
+		SND_SOC_DAILINK_REG(DL2_FE),
+	},
+	[DAI_LINK_DL3_FE] = {
+		.name = "DL3_FE",
+		.stream_name = "DL3 Playback",
+		.trigger = {
+			SND_SOC_DPCM_TRIGGER_POST,
+			SND_SOC_DPCM_TRIGGER_POST,
+		},
+		.dynamic = 1,
+		.dpcm_playback = 1,
+		.ops = &mt8195_playback_ops,
+		SND_SOC_DAILINK_REG(DL3_FE),
+	},
+	[DAI_LINK_DL6_FE] = {
+		.name = "DL6_FE",
+		.stream_name = "DL6 Playback",
+		.trigger = {
+			SND_SOC_DPCM_TRIGGER_POST,
+			SND_SOC_DPCM_TRIGGER_POST,
+		},
+		.dynamic = 1,
+		.dpcm_playback = 1,
+		.ops = &mt8195_playback_ops,
+		SND_SOC_DAILINK_REG(DL6_FE),
+	},
+	[DAI_LINK_DL7_FE] = {
+		.name = "DL7_FE",
+		.stream_name = "DL7 Playback",
+		.trigger = {
+			SND_SOC_DPCM_TRIGGER_PRE,
+			SND_SOC_DPCM_TRIGGER_PRE,
+		},
+		.dynamic = 1,
+		.dpcm_playback = 1,
+		SND_SOC_DAILINK_REG(DL7_FE),
+	},
+	[DAI_LINK_DL8_FE] = {
+		.name = "DL8_FE",
+		.stream_name = "DL8 Playback",
+		.trigger = {
+			SND_SOC_DPCM_TRIGGER_POST,
+			SND_SOC_DPCM_TRIGGER_POST,
+		},
+		.dynamic = 1,
+		.dpcm_playback = 1,
+		.ops = &mt8195_playback_ops,
+		SND_SOC_DAILINK_REG(DL8_FE),
+	},
+	[DAI_LINK_DL10_FE] = {
+		.name = "DL10_FE",
+		.stream_name = "DL10 Playback",
+		.trigger = {
+			SND_SOC_DPCM_TRIGGER_POST,
+			SND_SOC_DPCM_TRIGGER_POST,
+		},
+		.dynamic = 1,
+		.dpcm_playback = 1,
+		.ops = &mt8195_hdmitx_dptx_playback_ops,
+		SND_SOC_DAILINK_REG(DL10_FE),
+	},
+	[DAI_LINK_DL11_FE] = {
+		.name = "DL11_FE",
+		.stream_name = "DL11 Playback",
+		.trigger = {
+			SND_SOC_DPCM_TRIGGER_POST,
+			SND_SOC_DPCM_TRIGGER_POST,
+		},
+		.dynamic = 1,
+		.dpcm_playback = 1,
+		.ops = &mt8195_playback_ops,
+		SND_SOC_DAILINK_REG(DL11_FE),
+	},
+	[DAI_LINK_UL1_FE] = {
+		.name = "UL1_FE",
+		.stream_name = "UL1 Capture",
+		.trigger = {
+			SND_SOC_DPCM_TRIGGER_PRE,
+			SND_SOC_DPCM_TRIGGER_PRE,
+		},
+		.dynamic = 1,
+		.dpcm_capture = 1,
+		SND_SOC_DAILINK_REG(UL1_FE),
+	},
+	[DAI_LINK_UL2_FE] = {
+		.name = "UL2_FE",
+		.stream_name = "UL2 Capture",
+		.trigger = {
+			SND_SOC_DPCM_TRIGGER_POST,
+			SND_SOC_DPCM_TRIGGER_POST,
+		},
+		.dynamic = 1,
+		.dpcm_capture = 1,
+		.ops = &mt8195_capture_ops,
+		SND_SOC_DAILINK_REG(UL2_FE),
+	},
+	[DAI_LINK_UL3_FE] = {
+		.name = "UL3_FE",
+		.stream_name = "UL3 Capture",
+		.trigger = {
+			SND_SOC_DPCM_TRIGGER_POST,
+			SND_SOC_DPCM_TRIGGER_POST,
+		},
+		.dynamic = 1,
+		.dpcm_capture = 1,
+		.ops = &mt8195_capture_ops,
+		SND_SOC_DAILINK_REG(UL3_FE),
+	},
+	[DAI_LINK_UL4_FE] = {
+		.name = "UL4_FE",
+		.stream_name = "UL4 Capture",
+		.trigger = {
+			SND_SOC_DPCM_TRIGGER_POST,
+			SND_SOC_DPCM_TRIGGER_POST,
+		},
+		.dynamic = 1,
+		.dpcm_capture = 1,
+		.ops = &mt8195_capture_ops,
+		SND_SOC_DAILINK_REG(UL4_FE),
+	},
+	[DAI_LINK_UL5_FE] = {
+		.name = "UL5_FE",
+		.stream_name = "UL5 Capture",
+		.trigger = {
+			SND_SOC_DPCM_TRIGGER_POST,
+			SND_SOC_DPCM_TRIGGER_POST,
+		},
+		.dynamic = 1,
+		.dpcm_capture = 1,
+		.ops = &mt8195_capture_ops,
+		SND_SOC_DAILINK_REG(UL5_FE),
+	},
+	[DAI_LINK_UL6_FE] = {
+		.name = "UL6_FE",
+		.stream_name = "UL6 Capture",
+		.trigger = {
+			SND_SOC_DPCM_TRIGGER_PRE,
+			SND_SOC_DPCM_TRIGGER_PRE,
+		},
+		.dynamic = 1,
+		.dpcm_capture = 1,
+		SND_SOC_DAILINK_REG(UL6_FE),
+	},
+	[DAI_LINK_UL8_FE] = {
+		.name = "UL8_FE",
+		.stream_name = "UL8 Capture",
+		.trigger = {
+			SND_SOC_DPCM_TRIGGER_POST,
+			SND_SOC_DPCM_TRIGGER_POST,
+		},
+		.dynamic = 1,
+		.dpcm_capture = 1,
+		.ops = &mt8195_capture_ops,
+		SND_SOC_DAILINK_REG(UL8_FE),
+	},
+	[DAI_LINK_UL9_FE] = {
+		.name = "UL9_FE",
+		.stream_name = "UL9 Capture",
+		.trigger = {
+			SND_SOC_DPCM_TRIGGER_POST,
+			SND_SOC_DPCM_TRIGGER_POST,
+		},
+		.dynamic = 1,
+		.dpcm_capture = 1,
+		.ops = &mt8195_capture_ops,
+		SND_SOC_DAILINK_REG(UL9_FE),
+	},
+	[DAI_LINK_UL10_FE] = {
+		.name = "UL10_FE",
+		.stream_name = "UL10 Capture",
+		.trigger = {
+			SND_SOC_DPCM_TRIGGER_POST,
+			SND_SOC_DPCM_TRIGGER_POST,
+		},
+		.dynamic = 1,
+		.dpcm_capture = 1,
+		.ops = &mt8195_capture_ops,
+		SND_SOC_DAILINK_REG(UL10_FE),
+	},
+	/* BE */
+	[DAI_LINK_DL_SRC_BE] = {
+		.name = "DL_SRC_BE",
+		.init = mt8195_mt6359_init,
+		.no_pcm = 1,
+		.dpcm_playback = 1,
+		SND_SOC_DAILINK_REG(DL_SRC_BE),
+	},
+	[DAI_LINK_DPTX_BE] = {
+		.name = "DPTX_BE",
+		.no_pcm = 1,
+		.dpcm_playback = 1,
+		.ops = &mt8195_dptx_ops,
+		.be_hw_params_fixup = mt8195_hdmitx_dptx_hw_params_fixup,
+		SND_SOC_DAILINK_REG(DPTX_BE),
+	},
+	[DAI_LINK_ETDM1_IN_BE] = {
+		.name = "ETDM1_IN_BE",
+		.no_pcm = 1,
+		.dai_fmt = SND_SOC_DAIFMT_I2S |
+			SND_SOC_DAIFMT_NB_NF |
+			SND_SOC_DAIFMT_CBS_CFS,
+		.dpcm_capture = 1,
+		SND_SOC_DAILINK_REG(ETDM1_IN_BE),
+	},
+	[DAI_LINK_ETDM2_IN_BE] = {
+		.name = "ETDM2_IN_BE",
+		.no_pcm = 1,
+		.dai_fmt = SND_SOC_DAIFMT_I2S |
+			SND_SOC_DAIFMT_NB_NF |
+			SND_SOC_DAIFMT_CBS_CFS,
+		.dpcm_capture = 1,
+		.init = mt8195_rt5682_init,
+		.ops = &mt8195_rt5682_etdm_ops,
+		.be_hw_params_fixup = mt8195_etdm_hw_params_fixup,
+		SND_SOC_DAILINK_REG(ETDM2_IN_BE),
+	},
+	[DAI_LINK_ETDM1_OUT_BE] = {
+		.name = "ETDM1_OUT_BE",
+		.no_pcm = 1,
+		.dai_fmt = SND_SOC_DAIFMT_I2S |
+			SND_SOC_DAIFMT_NB_NF |
+			SND_SOC_DAIFMT_CBS_CFS,
+		.dpcm_playback = 1,
+		.ops = &mt8195_rt5682_etdm_ops,
+		.be_hw_params_fixup = mt8195_etdm_hw_params_fixup,
+		SND_SOC_DAILINK_REG(ETDM1_OUT_BE),
+	},
+	[DAI_LINK_ETDM2_OUT_BE] = {
+		.name = "ETDM2_OUT_BE",
+		.no_pcm = 1,
+		.dai_fmt = SND_SOC_DAIFMT_I2S |
+			SND_SOC_DAIFMT_NB_NF |
+			SND_SOC_DAIFMT_CBS_CFS,
+		.dpcm_playback = 1,
+		.ops = &mt8195_rt1011_etdm_ops,
+		.be_hw_params_fixup = mt8195_etdm_hw_params_fixup,
+		SND_SOC_DAILINK_REG(ETDM2_OUT_BE),
+	},
+	[DAI_LINK_ETDM3_OUT_BE] = {
+		.name = "ETDM3_OUT_BE",
+		.no_pcm = 1,
+		.dai_fmt = SND_SOC_DAIFMT_I2S |
+			SND_SOC_DAIFMT_NB_NF |
+			SND_SOC_DAIFMT_CBS_CFS,
+		.dpcm_playback = 1,
+		.be_hw_params_fixup = mt8195_hdmitx_dptx_hw_params_fixup,
+		SND_SOC_DAILINK_REG(ETDM3_OUT_BE),
+	},
+	[DAI_LINK_PCM1_BE] = {
+		.name = "PCM1_BE",
+		.no_pcm = 1,
+		.dai_fmt = SND_SOC_DAIFMT_I2S |
+			SND_SOC_DAIFMT_NB_NF |
+			SND_SOC_DAIFMT_CBS_CFS,
+		.dpcm_capture = 1,
+		SND_SOC_DAILINK_REG(PCM1_BE),
+	},
+	[DAI_LINK_UL_SRC1_BE] = {
+		.name = "UL_SRC1_BE",
+		.no_pcm = 1,
+		.dpcm_capture = 1,
+		SND_SOC_DAILINK_REG(UL_SRC1_BE),
+	},
+	[DAI_LINK_UL_SRC2_BE] = {
+		.name = "UL_SRC2_BE",
+		.no_pcm = 1,
+		.dpcm_capture = 1,
+		SND_SOC_DAILINK_REG(UL_SRC2_BE),
+	},
+};
+
+static struct snd_soc_codec_conf rt1011_amp_conf[] = {
+	{
+		.dlc = COMP_CODEC_CONF(RT1011_DEV0_NAME),
+		.name_prefix = "Left",
+	},
+	{
+		.dlc = COMP_CODEC_CONF(RT1011_DEV1_NAME),
+		.name_prefix = "Right",
+	},
+};
+
+static struct snd_soc_card mt8195_mt6359_rt1011_rt5682_soc_card = {
+	.name = "mt8195_r1011_5682",
+	.owner = THIS_MODULE,
+	.dai_link = mt8195_mt6359_rt1011_rt5682_dai_links,
+	.num_links = ARRAY_SIZE(mt8195_mt6359_rt1011_rt5682_dai_links),
+	.controls = mt8195_mt6359_rt1011_rt5682_controls,
+	.num_controls = ARRAY_SIZE(mt8195_mt6359_rt1011_rt5682_controls),
+	.dapm_widgets = mt8195_mt6359_rt1011_rt5682_widgets,
+	.num_dapm_widgets = ARRAY_SIZE(mt8195_mt6359_rt1011_rt5682_widgets),
+	.dapm_routes = mt8195_mt6359_rt1011_rt5682_routes,
+	.num_dapm_routes = ARRAY_SIZE(mt8195_mt6359_rt1011_rt5682_routes),
+	.codec_conf = rt1011_amp_conf,
+	.num_configs = ARRAY_SIZE(rt1011_amp_conf),
+};
+
+static int mt8195_mt6359_rt1011_rt5682_dev_probe(struct platform_device *pdev)
+{
+	struct snd_soc_card *card = &mt8195_mt6359_rt1011_rt5682_soc_card;
+	struct device_node *platform_node;
+	struct snd_soc_dai_link *dai_link;
+	struct mt8195_mt6359_rt1011_rt5682_priv *priv = NULL;
+	int ret, i;
+
+	card->dev = &pdev->dev;
+
+	platform_node = of_parse_phandle(pdev->dev.of_node,
+					 "mediatek,platform", 0);
+	if (!platform_node) {
+		dev_dbg(&pdev->dev, "Property 'platform' missing or invalid\n");
+		return -EINVAL;
+	}
+
+	for_each_card_prelinks(card, i, dai_link) {
+		if (!dai_link->platforms->name)
+			dai_link->platforms->of_node = platform_node;
+
+		if (strcmp(dai_link->name, "DPTX_BE") == 0) {
+			dai_link->codecs->of_node =
+				of_parse_phandle(pdev->dev.of_node,
+						 "mediatek,dptx-codec", 0);
+			if (!dai_link->codecs->of_node) {
+				dev_dbg(&pdev->dev, "No property 'dptx-codec'\n");
+			} else {
+				dai_link->codecs->name = NULL;
+				dai_link->codecs->dai_name = "i2s-hifi";
+				dai_link->init = mt8195_dptx_codec_init;
+			}
+		}
+
+		if (strcmp(dai_link->name, "ETDM3_OUT_BE") == 0) {
+			dai_link->codecs->of_node =
+				of_parse_phandle(pdev->dev.of_node,
+						 "mediatek,hdmi-codec", 0);
+			if (!dai_link->codecs->of_node) {
+				dev_dbg(&pdev->dev, "No property 'hdmi-codec'\n");
+			} else {
+				dai_link->codecs->name = NULL;
+				dai_link->codecs->dai_name = "i2s-hifi";
+				dai_link->init = mt8195_hdmi_codec_init;
+			}
+		}
+	}
+
+	priv = devm_kzalloc(&pdev->dev, sizeof(*priv), GFP_KERNEL);
+	if (!priv)
+		return -ENOMEM;
+
+	snd_soc_card_set_drvdata(card, priv);
+
+	ret = devm_snd_soc_register_card(&pdev->dev, card);
+	if (ret)
+		dev_err(&pdev->dev, "%s snd_soc_register_card fail %d\n",
+			__func__, ret);
+	return ret;
+}
+
+#ifdef CONFIG_OF
+static const struct of_device_id mt8195_mt6359_rt1011_rt5682_dt_match[] = {
+	{.compatible = "mediatek,mt8195_mt6359_rt1011_rt5682",},
+	{}
+};
+#endif
+
+static const struct dev_pm_ops mt8195_mt6359_rt1011_rt5682_pm_ops = {
+	.poweroff = snd_soc_poweroff,
+	.restore = snd_soc_resume,
+};
+
+static struct platform_driver mt8195_mt6359_rt1011_rt5682_driver = {
+	.driver = {
+		.name = "mt8195_mt6359_rt1011_rt5682",
+#ifdef CONFIG_OF
+		.of_match_table = mt8195_mt6359_rt1011_rt5682_dt_match,
+#endif
+		.pm = &mt8195_mt6359_rt1011_rt5682_pm_ops,
+	},
+	.probe = mt8195_mt6359_rt1011_rt5682_dev_probe,
+};
+
+module_platform_driver(mt8195_mt6359_rt1011_rt5682_driver);
+
+/* Module information */
+MODULE_DESCRIPTION("MT8195-MT6359-RT1011-RT5682 ALSA SoC machine driver");
+MODULE_AUTHOR("Trevor Wu <trevor.wu@mediatek.com>");
+MODULE_LICENSE("GPL v2");
+MODULE_ALIAS("mt8195_mt6359_rt1011_rt5682 soc card");
-- 
2.18.0


^ permalink raw reply	[flat|nested] 9+ messages in thread

* [PATCH 2/2] dt-bindings: mediatek: mt8195: add mt8195-mt6359-rt1011-rt5682 document
  2021-09-10 10:44 [PATCH 0/2] ASoC: mediatek: Add support for MT8195 sound card with rt1011 and rt5682 Trevor Wu
  2021-09-10 10:44 ` [PATCH 1/2] ASoC: mediatek: mt8195: add machine driver with mt6359, " Trevor Wu
@ 2021-09-10 10:44 ` Trevor Wu
  2021-10-29 20:54 ` [PATCH 0/2] ASoC: mediatek: Add support for MT8195 sound card with rt1011 and rt5682 Mark Brown
  2 siblings, 0 replies; 9+ messages in thread
From: Trevor Wu @ 2021-09-10 10:44 UTC (permalink / raw)
  To: broonie, tiwai, robh+dt, matthias.bgg
  Cc: trevor.wu, alsa-devel, linux-mediatek, linux-arm-kernel,
	linux-kernel, devicetree, aaronyu

This patch adds document for mt8195 board with mt6359, rt1011 and rt5682

Signed-off-by: Trevor Wu <trevor.wu@mediatek.com>
---
 .../sound/mt8195-mt6359-rt1011-rt5682.yaml    | 47 +++++++++++++++++++
 1 file changed, 47 insertions(+)
 create mode 100644 Documentation/devicetree/bindings/sound/mt8195-mt6359-rt1011-rt5682.yaml

diff --git a/Documentation/devicetree/bindings/sound/mt8195-mt6359-rt1011-rt5682.yaml b/Documentation/devicetree/bindings/sound/mt8195-mt6359-rt1011-rt5682.yaml
new file mode 100644
index 000000000000..d354c30d3377
--- /dev/null
+++ b/Documentation/devicetree/bindings/sound/mt8195-mt6359-rt1011-rt5682.yaml
@@ -0,0 +1,47 @@
+# SPDX-License-Identifier: (GPL-2.0-only OR BSD-2-Clause)
+%YAML 1.2
+---
+$id: http://devicetree.org/schemas/sound/mt8195-mt6359-rt1011-rt5682.yaml#
+$schema: http://devicetree.org/meta-schemas/core.yaml#
+
+title: Mediatek MT8195 with MT6359, RT1011 and RT5682 ASoC sound card driver
+
+maintainers:
+  - Trevor Wu <trevor.wu@mediatek.com>
+
+description:
+  This binding describes the MT8195 sound card with RT1011 and RT5682.
+
+properties:
+  compatible:
+    const: mediatek,mt8195_mt6359_rt1011_rt5682
+
+  mediatek,platform:
+    $ref: "/schemas/types.yaml#/definitions/phandle"
+    description: The phandle of MT8195 ASoC platform.
+
+  mediatek,dptx-codec:
+    $ref: "/schemas/types.yaml#/definitions/phandle"
+    description: The phandle of MT8195 Display Port Tx codec node.
+
+  mediatek,hdmi-codec:
+    $ref: "/schemas/types.yaml#/definitions/phandle"
+    description: The phandle of MT8195 HDMI codec node.
+
+additionalProperties: false
+
+required:
+  - compatible
+  - mediatek,platform
+
+examples:
+  - |
+
+    sound: mt8195-sound {
+        compatible = "mediatek,mt8195_mt6359_rt1011_rt5682";
+        mediatek,platform = <&afe>;
+        pinctrl-names = "default";
+        pinctrl-0 = <&aud_pins_default>;
+    };
+
+...
-- 
2.18.0


^ permalink raw reply	[flat|nested] 9+ messages in thread

* Re: [PATCH 1/2] ASoC: mediatek: mt8195: add machine driver with mt6359, rt1011 and rt5682
  2021-09-10 10:44 ` [PATCH 1/2] ASoC: mediatek: mt8195: add machine driver with mt6359, " Trevor Wu
@ 2021-09-10 16:47   ` Pierre-Louis Bossart
  2021-09-13 10:24     ` Trevor Wu
  0 siblings, 1 reply; 9+ messages in thread
From: Pierre-Louis Bossart @ 2021-09-10 16:47 UTC (permalink / raw)
  To: Trevor Wu, broonie, tiwai, robh+dt, matthias.bgg
  Cc: devicetree, alsa-devel, linux-kernel, linux-mediatek, aaronyu,
	linux-arm-kernel


> +static int mt8195_rt5682_etdm_hw_params(struct snd_pcm_substream *substream,
> +					struct snd_pcm_hw_params *params)
> +{
> +	struct snd_soc_pcm_runtime *rtd = substream->private_data;
> +	struct snd_soc_card *card = rtd->card;
> +	struct snd_soc_dai *cpu_dai = asoc_rtd_to_cpu(rtd, 0);
> +	struct snd_soc_dai *codec_dai = asoc_rtd_to_codec(rtd, 0);
> +	unsigned int rate = params_rate(params);
> +	unsigned int mclk_fs_ratio = 128;
> +	unsigned int mclk_fs = rate * mclk_fs_ratio;
> +	int bitwidth;
> +	int ret;
> +
> +	bitwidth = snd_pcm_format_width(params_format(params));
> +	if (bitwidth < 0) {
> +		dev_err(card->dev, "invalid bit width: %d\n", bitwidth);
> +		return bitwidth;
> +	}
> +
> +	ret = snd_soc_dai_set_tdm_slot(codec_dai, 0x00, 0x0, 0x2, bitwidth);
> +	if (ret) {
> +		dev_err(card->dev, "failed to set tdm slot\n");
> +		return ret;
> +	}
> +
> +	ret = snd_soc_dai_set_pll(codec_dai, RT5682_PLL1,
> +				  RT5682_PLL1_S_BCLK1,
> +				  params_rate(params) * 64,
> +				  params_rate(params) * 512);
> +	if (ret) {
> +		dev_err(card->dev, "failed to set pll\n");
> +		return ret;
> +	}
> +
> +	ret = snd_soc_dai_set_sysclk(codec_dai,
> +				     RT5682_SCLK_S_PLL1,
> +				     params_rate(params) * 512,
> +				     SND_SOC_CLOCK_IN);
> +	if (ret) {
> +		dev_err(card->dev, "failed to set sysclk\n");
> +		return ret;
> +	}
> +
> +	return snd_soc_dai_set_sysclk(cpu_dai, 0, mclk_fs, SND_SOC_CLOCK_OUT);

If you are using params_rate(params) x factor, then it'd be more
consistent to use:

return snd_soc_dai_set_sysclk(cpu_dai, 0, params_rate(params) * 128,
SND_SOC_CLOCK_OUT);


> +static int mt8195_mt6359_mtkaif_calibration(struct snd_soc_pcm_runtime *rtd)
> +{
> +	struct snd_soc_component *cmpnt_afe =
> +		snd_soc_rtdcom_lookup(rtd, AFE_PCM_NAME);
> +	struct snd_soc_component *cmpnt_codec =
> +		asoc_rtd_to_codec(rtd, 0)->component;
> +	struct mtk_base_afe *afe = snd_soc_component_get_drvdata(cmpnt_afe);
> +	struct mt8195_afe_private *afe_priv = afe->platform_priv;
> +	struct mtkaif_param *param = &afe_priv->mtkaif_params;
> +	int phase;
> +	unsigned int monitor;
> +	int mtkaif_calibration_num_phase;
> +	int test_done_1, test_done_2, test_done_3;
> +	int cycle_1, cycle_2, cycle_3;
> +	int prev_cycle_1, prev_cycle_2, prev_cycle_3;
> +	int chosen_phase_1, chosen_phase_2, chosen_phase_3;
> +	int counter;
> +	bool mtkaif_calibration_ok;
> +	int mtkaif_chosen_phase[MT8195_MTKAIF_MISO_NUM];
> +	int mtkaif_phase_cycle[MT8195_MTKAIF_MISO_NUM];
> +	int i;

reverse x-mas style with longer declarations first?

> +
> +	dev_info(afe->dev, "%s(), start\n", __func__);

dev_dbg

> +
> +	param->mtkaif_calibration_ok = false;
> +	for (i = 0; i < MT8195_MTKAIF_MISO_NUM; i++) {
> +		param->mtkaif_chosen_phase[i] = -1;
> +		param->mtkaif_phase_cycle[i] = 0;
> +		mtkaif_chosen_phase[i] = -1;
> +		mtkaif_phase_cycle[i] = 0;
> +	}
> +
> +	if (IS_ERR(afe_priv->topckgen)) {
> +		dev_info(afe->dev, "%s() Cannot find topckgen controller\n",
> +			 __func__);
> +		return 0;

is this not an error? Why not dev_err() and return -EINVAL or something?

> +	}
> +
> +	pm_runtime_get_sync(afe->dev);

test if this worked?

> +	mt6359_mtkaif_calibration_enable(cmpnt_codec);
> +
> +	/* set test type to synchronizer pulse */
> +	regmap_update_bits(afe_priv->topckgen,
> +			   CKSYS_AUD_TOP_CFG, 0xffff, 0x4);
> +	mtkaif_calibration_num_phase = 42;	/* mt6359: 0 ~ 42 */
> +	mtkaif_calibration_ok = true;
> +
> +	for (phase = 0;
> +	     phase <= mtkaif_calibration_num_phase && mtkaif_calibration_ok;
> +	     phase++) {
> +		mt6359_set_mtkaif_calibration_phase(cmpnt_codec,
> +						    phase, phase, phase);
> +
> +		regmap_update_bits(afe_priv->topckgen,
> +				   CKSYS_AUD_TOP_CFG, 0x1, 0x1);
> +
> +		test_done_1 = 0;
> +		test_done_2 = 0;
> +		test_done_3 = 0;
> +		cycle_1 = -1;
> +		cycle_2 = -1;
> +		cycle_3 = -1;
> +		counter = 0;
> +		while (!(test_done_1 & test_done_2 & test_done_3)) {
> +			regmap_read(afe_priv->topckgen,
> +				    CKSYS_AUD_TOP_MON, &monitor);
> +			test_done_1 = (monitor >> 28) & 0x1;
> +			test_done_2 = (monitor >> 29) & 0x1;
> +			test_done_3 = (monitor >> 30) & 0x1;
> +			if (test_done_1 == 1)
> +				cycle_1 = monitor & 0xf;
> +
> +			if (test_done_2 == 1)
> +				cycle_2 = (monitor >> 4) & 0xf;
> +
> +			if (test_done_3 == 1)
> +				cycle_3 = (monitor >> 8) & 0xf;
> +
> +			/* handle if never test done */
> +			if (++counter > 10000) {
> +				dev_info(afe->dev, "%s(), test fail, cycle_1 %d, cycle_2 %d, cycle_3 %d, monitor 0x%x\n",
> +					 __func__,
> +					 cycle_1, cycle_2, cycle_3, monitor);
> +				mtkaif_calibration_ok = false;
> +				break;
> +			}
> +		}
> +
> +		if (phase == 0) {
> +			prev_cycle_1 = cycle_1;
> +			prev_cycle_2 = cycle_2;
> +			prev_cycle_3 = cycle_3;
> +		}
> +
> +		if (cycle_1 != prev_cycle_1 &&
> +		    mtkaif_chosen_phase[MT8195_MTKAIF_MISO_0] < 0) {
> +			mtkaif_chosen_phase[MT8195_MTKAIF_MISO_0] = phase - 1;
> +			mtkaif_phase_cycle[MT8195_MTKAIF_MISO_0] = prev_cycle_1;
> +		}
> +
> +		if (cycle_2 != prev_cycle_2 &&
> +		    mtkaif_chosen_phase[MT8195_MTKAIF_MISO_1] < 0) {
> +			mtkaif_chosen_phase[MT8195_MTKAIF_MISO_1] = phase - 1;
> +			mtkaif_phase_cycle[MT8195_MTKAIF_MISO_1] = prev_cycle_2;
> +		}
> +
> +		if (cycle_3 != prev_cycle_3 &&
> +		    mtkaif_chosen_phase[MT8195_MTKAIF_MISO_2] < 0) {
> +			mtkaif_chosen_phase[MT8195_MTKAIF_MISO_2] = phase - 1;
> +			mtkaif_phase_cycle[MT8195_MTKAIF_MISO_2] = prev_cycle_3;
> +		}
> +
> +		regmap_update_bits(afe_priv->topckgen,
> +				   CKSYS_AUD_TOP_CFG, 0x1, 0x0);
> +
> +		if (mtkaif_chosen_phase[MT8195_MTKAIF_MISO_0] >= 0 &&
> +		    mtkaif_chosen_phase[MT8195_MTKAIF_MISO_1] >= 0 &&
> +		    mtkaif_chosen_phase[MT8195_MTKAIF_MISO_2] >= 0)
> +			break;
> +	}
> +
> +	if (mtkaif_chosen_phase[MT8195_MTKAIF_MISO_0] < 0) {
> +		mtkaif_calibration_ok = false;
> +		chosen_phase_1 = 0;
> +	} else {
> +		chosen_phase_1 = mtkaif_chosen_phase[MT8195_MTKAIF_MISO_0];
> +	}
> +
> +	if (mtkaif_chosen_phase[MT8195_MTKAIF_MISO_1] < 0) {
> +		mtkaif_calibration_ok = false;
> +		chosen_phase_2 = 0;
> +	} else {
> +		chosen_phase_2 = mtkaif_chosen_phase[MT8195_MTKAIF_MISO_1];
> +	}
> +
> +	if (mtkaif_chosen_phase[MT8195_MTKAIF_MISO_2] < 0) {
> +		mtkaif_calibration_ok = false;
> +		chosen_phase_3 = 0;
> +	} else {
> +		chosen_phase_3 = mtkaif_chosen_phase[MT8195_MTKAIF_MISO_2];
> +	}
> +
> +	mt6359_set_mtkaif_calibration_phase(cmpnt_codec,
> +					    chosen_phase_1,
> +					    chosen_phase_2,
> +					    chosen_phase_3);
> +
> +	mt6359_mtkaif_calibration_disable(cmpnt_codec);
> +	pm_runtime_put(afe->dev);
> +
> +	param->mtkaif_calibration_ok = mtkaif_calibration_ok;
> +	param->mtkaif_chosen_phase[MT8195_MTKAIF_MISO_0] = chosen_phase_1;
> +	param->mtkaif_chosen_phase[MT8195_MTKAIF_MISO_1] = chosen_phase_2;
> +	param->mtkaif_chosen_phase[MT8195_MTKAIF_MISO_2] = chosen_phase_3;
> +	for (i = 0; i < MT8195_MTKAIF_MISO_NUM; i++)
> +		param->mtkaif_phase_cycle[i] = mtkaif_phase_cycle[i];
> +
> +	dev_info(afe->dev, "%s(), end, calibration ok %d\n",
> +		 __func__, param->mtkaif_calibration_ok);

dev_dbg?

> +
> +	return 0;
> +}
> +

> +static int mt8195_hdmitx_dptx_startup(struct snd_pcm_substream *substream)
> +{
> +	static const unsigned int rates[] = {
> +		48000
> +	};
> +	static const unsigned int channels[] = {
> +		2, 4, 6, 8
> +	};
> +	static const struct snd_pcm_hw_constraint_list constraints_rates = {
> +		.count = ARRAY_SIZE(rates),
> +		.list  = rates,
> +		.mask = 0,
> +	};
> +	static const struct snd_pcm_hw_constraint_list constraints_channels = {
> +		.count = ARRAY_SIZE(channels),
> +		.list  = channels,
> +		.mask = 0,
> +	};

you use the same const tables several times, move to a higher scope and
reuse?

> +	struct snd_soc_pcm_runtime *rtd = asoc_substream_to_rtd(substream);
> +	struct snd_pcm_runtime *runtime = substream->runtime;
> +	int ret;
> +
> +	ret = snd_pcm_hw_constraint_list(runtime, 0,
> +					 SNDRV_PCM_HW_PARAM_RATE,
> +					 &constraints_rates);
> +	if (ret < 0) {
> +		dev_err(rtd->dev, "hw_constraint_list rate failed\n");
> +		return ret;
> +	}
> +
> +	ret = snd_pcm_hw_constraint_list(runtime, 0,
> +					 SNDRV_PCM_HW_PARAM_CHANNELS,
> +					 &constraints_channels);
> +	if (ret < 0) {
> +		dev_err(rtd->dev, "hw_constraint_list channel failed\n");
> +		return ret;
> +	}
> +
> +	return 0;
> +}
> +
> +static const struct snd_soc_ops mt8195_hdmitx_dptx_playback_ops = {
> +	.startup = mt8195_hdmitx_dptx_startup,
> +};
> +
> +static int mt8195_dptx_hw_params(struct snd_pcm_substream *substream,
> +				 struct snd_pcm_hw_params *params)
> +{
> +	struct snd_soc_pcm_runtime *rtd = substream->private_data;
> +	struct snd_soc_dai *cpu_dai = asoc_rtd_to_cpu(rtd, 0);
> +	unsigned int rate = params_rate(params);
> +	unsigned int mclk_fs_ratio = 256;
> +	unsigned int mclk_fs = rate * mclk_fs_ratio;
> +
> +	return snd_soc_dai_set_sysclk(cpu_dai, 0, mclk_fs,
> +				      SND_SOC_CLOCK_OUT);

return snd_soc_dai_set_sysclk(cpu_dai, 0, params_rate(params) * 256,
SND_SOC_CLOCK_OUT);
?


> +static int mt8195_dptx_codec_init(struct snd_soc_pcm_runtime *rtd)
> +{
> +	struct mt8195_mt6359_rt1011_rt5682_priv *priv =
> +		snd_soc_card_get_drvdata(rtd->card);
> +	struct snd_soc_component *cmpnt_codec =
> +		asoc_rtd_to_codec(rtd, 0)->component;
> +	int ret = 0;

unnecessary init

> +	ret = snd_soc_card_jack_new(rtd->card, "DP Jack", SND_JACK_LINEOUT,
> +				    &priv->dp_jack, NULL, 0);
> +	if (ret)
> +		return ret;
> +
> +	return snd_soc_component_set_jack(cmpnt_codec, &priv->dp_jack, NULL);
> +}
> +
> +static int mt8195_hdmi_codec_init(struct snd_soc_pcm_runtime *rtd)
> +{
> +	struct mt8195_mt6359_rt1011_rt5682_priv *priv =
> +		snd_soc_card_get_drvdata(rtd->card);
> +	struct snd_soc_component *cmpnt_codec =
> +		asoc_rtd_to_codec(rtd, 0)->component;
> +	int ret = 0;

unnecessary init

> +	ret = snd_soc_card_jack_new(rtd->card, "HDMI Jack", SND_JACK_LINEOUT,
> +				    &priv->hdmi_jack, NULL, 0);
> +	if (ret)
> +		return ret;
> +
> +	return snd_soc_component_set_jack(cmpnt_codec, &priv->hdmi_jack, NULL);
> +}
> +
> +static int mt8195_hdmitx_dptx_hw_params_fixup(struct snd_soc_pcm_runtime *rtd,
> +					      struct snd_pcm_hw_params *params)
> +

spurious line?

> +{
> +	/* fix BE i2s format to 32bit, clean param mask first */
> +	snd_mask_reset_range(hw_param_mask(params, SNDRV_PCM_HW_PARAM_FORMAT),
> +			     0, (__force unsigned int)SNDRV_PCM_FORMAT_LAST);
> +
> +	params_set_format(params, SNDRV_PCM_FORMAT_S24_LE);
> +
> +	return 0;
> +}
> +
> +static int mt8195_playback_startup(struct snd_pcm_substream *substream)
> +{
> +	static const unsigned int rates[] = {
> +		48000
> +	};
> +	static const unsigned int channels[] = {
> +		2
> +	};
> +	static const struct snd_pcm_hw_constraint_list constraints_rates = {
> +		.count = ARRAY_SIZE(rates),
> +		.list  = rates,
> +		.mask = 0,
> +	};
> +	static const struct snd_pcm_hw_constraint_list constraints_channels = {
> +		.count = ARRAY_SIZE(channels),
> +		.list  = channels,
> +		.mask = 0,
> +	};

actually now I realize it's only the number of channels that differs...

> +
> +	struct snd_soc_pcm_runtime *rtd = asoc_substream_to_rtd(substream);
> +	struct snd_pcm_runtime *runtime = substream->runtime;
> +	int ret;
> +
> +	ret = snd_pcm_hw_constraint_list(runtime, 0,
> +					 SNDRV_PCM_HW_PARAM_RATE,
> +					 &constraints_rates);
> +	if (ret < 0) {
> +		dev_err(rtd->dev, "hw_constraint_list rate failed\n");
> +		return ret;
> +	}
> +
> +	ret = snd_pcm_hw_constraint_list(runtime, 0,
> +					 SNDRV_PCM_HW_PARAM_CHANNELS,
> +					 &constraints_channels);
> +	if (ret < 0) {
> +		dev_err(rtd->dev, "hw_constraint_list channel failed\n");
> +		return ret;
> +	}
> +
> +	return 0;
> +}

> +static struct snd_soc_dai_link mt8195_mt6359_rt1011_rt5682_dai_links[] = {
> +	/* FE */
> +	[DAI_LINK_DL2_FE] = {
> +		.name = "DL2_FE",
> +		.stream_name = "DL2 Playback",
> +		.trigger = {
> +			SND_SOC_DPCM_TRIGGER_POST,
> +			SND_SOC_DPCM_TRIGGER_POST,
> +		},
> +		.dynamic = 1,
> +		.dpcm_playback = 1,
> +		.ops = &mt8195_playback_ops,
> +		SND_SOC_DAILINK_REG(DL2_FE),
> +	},
> +	[DAI_LINK_DL3_FE] = {
> +		.name = "DL3_FE",
> +		.stream_name = "DL3 Playback",
> +		.trigger = {
> +			SND_SOC_DPCM_TRIGGER_POST,
> +			SND_SOC_DPCM_TRIGGER_POST,
> +		},
> +		.dynamic = 1,
> +		.dpcm_playback = 1,
> +		.ops = &mt8195_playback_ops,
> +		SND_SOC_DAILINK_REG(DL3_FE),
> +	},
> +	[DAI_LINK_DL6_FE] = {
> +		.name = "DL6_FE",
> +		.stream_name = "DL6 Playback",
> +		.trigger = {
> +			SND_SOC_DPCM_TRIGGER_POST,
> +			SND_SOC_DPCM_TRIGGER_POST,
> +		},
> +		.dynamic = 1,
> +		.dpcm_playback = 1,
> +		.ops = &mt8195_playback_ops,
> +		SND_SOC_DAILINK_REG(DL6_FE),
> +	},
> +	[DAI_LINK_DL7_FE] = {
> +		.name = "DL7_FE",
> +		.stream_name = "DL7 Playback",
> +		.trigger = {
> +			SND_SOC_DPCM_TRIGGER_PRE,
> +			SND_SOC_DPCM_TRIGGER_PRE,
> +		},

this is interesting, is it intentional that the trigger order is
different from all FEs?

> +		.dynamic = 1,
> +		.dpcm_playback = 1,

also no .ops?

> +		SND_SOC_DAILINK_REG(DL7_FE),
> +	},
> +	[DAI_LINK_DL8_FE] = {
> +		.name = "DL8_FE",
> +		.stream_name = "DL8 Playback",
> +		.trigger = {
> +			SND_SOC_DPCM_TRIGGER_POST,
> +			SND_SOC_DPCM_TRIGGER_POST,
> +		},
> +		.dynamic = 1,
> +		.dpcm_playback = 1,
> +		.ops = &mt8195_playback_ops,
> +		SND_SOC_DAILINK_REG(DL8_FE),
> +	},
> +	[DAI_LINK_DL10_FE] = {
> +		.name = "DL10_FE",
> +		.stream_name = "DL10 Playback",
> +		.trigger = {
> +			SND_SOC_DPCM_TRIGGER_POST,
> +			SND_SOC_DPCM_TRIGGER_POST,
> +		},
> +		.dynamic = 1,
> +		.dpcm_playback = 1,
> +		.ops = &mt8195_hdmitx_dptx_playback_ops,
> +		SND_SOC_DAILINK_REG(DL10_FE),
> +	},
> +	[DAI_LINK_DL11_FE] = {
> +		.name = "DL11_FE",
> +		.stream_name = "DL11 Playback",
> +		.trigger = {
> +			SND_SOC_DPCM_TRIGGER_POST,
> +			SND_SOC_DPCM_TRIGGER_POST,
> +		},
> +		.dynamic = 1,
> +		.dpcm_playback = 1,
> +		.ops = &mt8195_playback_ops,
> +		SND_SOC_DAILINK_REG(DL11_FE),
> +	},
> +	[DAI_LINK_UL1_FE] = {
> +		.name = "UL1_FE",
> +		.stream_name = "UL1 Capture",
> +		.trigger = {
> +			SND_SOC_DPCM_TRIGGER_PRE,
> +			SND_SOC_DPCM_TRIGGER_PRE,

and again here, why PRE and no ops?

> +static int mt8195_mt6359_rt1011_rt5682_dev_probe(struct platform_device *pdev)
> +{
> +	struct snd_soc_card *card = &mt8195_mt6359_rt1011_rt5682_soc_card;
> +	struct device_node *platform_node;
> +	struct snd_soc_dai_link *dai_link;
> +	struct mt8195_mt6359_rt1011_rt5682_priv *priv = NULL;

initialization is not necessary

> +	int ret, i;
> +
> +	card->dev = &pdev->dev;
> +
> +	platform_node = of_parse_phandle(pdev->dev.of_node,
> +					 "mediatek,platform", 0);
> +	if (!platform_node) {
> +		dev_dbg(&pdev->dev, "Property 'platform' missing or invalid\n");
> +		return -EINVAL;
> +	}
> +
> +	for_each_card_prelinks(card, i, dai_link) {
> +		if (!dai_link->platforms->name)
> +			dai_link->platforms->of_node = platform_node;
> +
> +		if (strcmp(dai_link->name, "DPTX_BE") == 0) {
> +			dai_link->codecs->of_node =
> +				of_parse_phandle(pdev->dev.of_node,
> +						 "mediatek,dptx-codec", 0);
> +			if (!dai_link->codecs->of_node) {
> +				dev_dbg(&pdev->dev, "No property 'dptx-codec'\n");
> +			} else {
> +				dai_link->codecs->name = NULL;
> +				dai_link->codecs->dai_name = "i2s-hifi";
> +				dai_link->init = mt8195_dptx_codec_init;
> +			}
> +		}
> +
> +		if (strcmp(dai_link->name, "ETDM3_OUT_BE") == 0) {
> +			dai_link->codecs->of_node =
> +				of_parse_phandle(pdev->dev.of_node,
> +						 "mediatek,hdmi-codec", 0);
> +			if (!dai_link->codecs->of_node) {
> +				dev_dbg(&pdev->dev, "No property 'hdmi-codec'\n");
> +			} else {
> +				dai_link->codecs->name = NULL;
> +				dai_link->codecs->dai_name = "i2s-hifi";
> +				dai_link->init = mt8195_hdmi_codec_init;
> +			}
> +		}
> +	}
> +
> +	priv = devm_kzalloc(&pdev->dev, sizeof(*priv), GFP_KERNEL);
> +	if (!priv)
> +		return -ENOMEM;
> +
> +	snd_soc_card_set_drvdata(card, priv);
> +
> +	ret = devm_snd_soc_register_card(&pdev->dev, card);
> +	if (ret)
> +		dev_err(&pdev->dev, "%s snd_soc_register_card fail %d\n",
> +			__func__, ret);
> +	return ret;
> +}
> +
> +#ifdef CONFIG_OF
> +static const struct of_device_id mt8195_mt6359_rt1011_rt5682_dt_match[] = {
> +	{.compatible = "mediatek,mt8195_mt6359_rt1011_rt5682",},
> +	{}
> +};
> +#endif
> +
> +static const struct dev_pm_ops mt8195_mt6359_rt1011_rt5682_pm_ops = {
> +	.poweroff = snd_soc_poweroff,
> +	.restore = snd_soc_resume,
> +};
> +
> +static struct platform_driver mt8195_mt6359_rt1011_rt5682_driver = {
> +	.driver = {
> +		.name = "mt8195_mt6359_rt1011_rt5682",
> +#ifdef CONFIG_OF
> +		.of_match_table = mt8195_mt6359_rt1011_rt5682_dt_match,
> +#endif
> +		.pm = &mt8195_mt6359_rt1011_rt5682_pm_ops,
> +	},
> +	.probe = mt8195_mt6359_rt1011_rt5682_dev_probe,
> +};
> +
> +module_platform_driver(mt8195_mt6359_rt1011_rt5682_driver);
> +
> +/* Module information */
> +MODULE_DESCRIPTION("MT8195-MT6359-RT1011-RT5682 ALSA SoC machine driver");
> +MODULE_AUTHOR("Trevor Wu <trevor.wu@mediatek.com>");
> +MODULE_LICENSE("GPL v2");

"GPL" is enough

> +MODULE_ALIAS("mt8195_mt6359_rt1011_rt5682 soc card");
> 

^ permalink raw reply	[flat|nested] 9+ messages in thread

* Re: [PATCH 1/2] ASoC: mediatek: mt8195: add machine driver with mt6359, rt1011 and rt5682
  2021-09-10 16:47   ` Pierre-Louis Bossart
@ 2021-09-13 10:24     ` Trevor Wu
  2021-09-24  3:54       ` Trevor Wu
  0 siblings, 1 reply; 9+ messages in thread
From: Trevor Wu @ 2021-09-13 10:24 UTC (permalink / raw)
  To: Pierre-Louis Bossart, broonie, tiwai, robh+dt, matthias.bgg
  Cc: devicetree, alsa-devel, linux-kernel, linux-mediatek, aaronyu,
	linux-arm-kernel

On Fri, 2021-09-10 at 11:47 -0500, Pierre-Louis Bossart wrote:
> > +static int mt8195_rt5682_etdm_hw_params(struct snd_pcm_substream
> > *substream,
> > +					struct snd_pcm_hw_params
> > *params)
> > +{
> > +	struct snd_soc_pcm_runtime *rtd = substream->private_data;
> > +	struct snd_soc_card *card = rtd->card;
> > +	struct snd_soc_dai *cpu_dai = asoc_rtd_to_cpu(rtd, 0);
> > +	struct snd_soc_dai *codec_dai = asoc_rtd_to_codec(rtd, 0);
> > +	unsigned int rate = params_rate(params);
> > +	unsigned int mclk_fs_ratio = 128;
> > +	unsigned int mclk_fs = rate * mclk_fs_ratio;
> > +	int bitwidth;
> > +	int ret;
> > +
> > +	bitwidth = snd_pcm_format_width(params_format(params));
> > +	if (bitwidth < 0) {
> > +		dev_err(card->dev, "invalid bit width: %d\n",
> > bitwidth);
> > +		return bitwidth;
> > +	}
> > +
> > +	ret = snd_soc_dai_set_tdm_slot(codec_dai, 0x00, 0x0, 0x2,
> > bitwidth);
> > +	if (ret) {
> > +		dev_err(card->dev, "failed to set tdm slot\n");
> > +		return ret;
> > +	}
> > +
> > +	ret = snd_soc_dai_set_pll(codec_dai, RT5682_PLL1,
> > +				  RT5682_PLL1_S_BCLK1,
> > +				  params_rate(params) * 64,
> > +				  params_rate(params) * 512);
> > +	if (ret) {
> > +		dev_err(card->dev, "failed to set pll\n");
> > +		return ret;
> > +	}
> > +
> > +	ret = snd_soc_dai_set_sysclk(codec_dai,
> > +				     RT5682_SCLK_S_PLL1,
> > +				     params_rate(params) * 512,
> > +				     SND_SOC_CLOCK_IN);
> > +	if (ret) {
> > +		dev_err(card->dev, "failed to set sysclk\n");
> > +		return ret;
> > +	}
> > +
> > +	return snd_soc_dai_set_sysclk(cpu_dai, 0, mclk_fs,
> > SND_SOC_CLOCK_OUT);
> 
> If you are using params_rate(params) x factor, then it'd be more
> consistent to use:
> 
> return snd_soc_dai_set_sysclk(cpu_dai, 0, params_rate(params) * 128,
> SND_SOC_CLOCK_OUT);
> 
> 

OK, I will modify it in v2.

> > +static int mt8195_mt6359_mtkaif_calibration(struct
> > snd_soc_pcm_runtime *rtd)
> > +{
> > +	struct snd_soc_component *cmpnt_afe =
> > +		snd_soc_rtdcom_lookup(rtd, AFE_PCM_NAME);
> > +	struct snd_soc_component *cmpnt_codec =
> > +		asoc_rtd_to_codec(rtd, 0)->component;
> > +	struct mtk_base_afe *afe =
> > snd_soc_component_get_drvdata(cmpnt_afe);
> > +	struct mt8195_afe_private *afe_priv = afe->platform_priv;
> > +	struct mtkaif_param *param = &afe_priv->mtkaif_params;
> > +	int phase;
> > +	unsigned int monitor;
> > +	int mtkaif_calibration_num_phase;
> > +	int test_done_1, test_done_2, test_done_3;
> > +	int cycle_1, cycle_2, cycle_3;
> > +	int prev_cycle_1, prev_cycle_2, prev_cycle_3;
> > +	int chosen_phase_1, chosen_phase_2, chosen_phase_3;
> > +	int counter;
> > +	bool mtkaif_calibration_ok;
> > +	int mtkaif_chosen_phase[MT8195_MTKAIF_MISO_NUM];
> > +	int mtkaif_phase_cycle[MT8195_MTKAIF_MISO_NUM];
> > +	int i;
> 
> reverse x-mas style with longer declarations first?
> 

OK. I will reorder the variables and move the lognger declaration to
the top.

> > +
> > +	dev_info(afe->dev, "%s(), start\n", __func__);
> 
> dev_dbg
> 
OK.

> > +
> > +	param->mtkaif_calibration_ok = false;
> > +	for (i = 0; i < MT8195_MTKAIF_MISO_NUM; i++) {
> > +		param->mtkaif_chosen_phase[i] = -1;
> > +		param->mtkaif_phase_cycle[i] = 0;
> > +		mtkaif_chosen_phase[i] = -1;
> > +		mtkaif_phase_cycle[i] = 0;
> > +	}
> > +
> > +	if (IS_ERR(afe_priv->topckgen)) {
> > +		dev_info(afe->dev, "%s() Cannot find topckgen
> > controller\n",
> > +			 __func__);
> > +		return 0;
> 
> is this not an error? Why not dev_err() and return -EINVAL or
> something?
> 

Should I still return an error, even if the caller didn't check it?

Based on my understanding, the calibration function is used to make the
signal more stable. 
Most of the time, mtkaif still works, even though the calibration
fails.
I guess that's why the caller(I refered to the implementation of
mt8192.) didn't check the return value of calibration function.


> > +	}
> > +
> > +	pm_runtime_get_sync(afe->dev);
> 
> test if this worked?
> 

Yes, if I didn't add pm_runtime_get_sync here, the calibration failed.

> > +	mt6359_mtkaif_calibration_enable(cmpnt_codec);
> > +
> > +	/* set test type to synchronizer pulse */
> > +	regmap_update_bits(afe_priv->topckgen,
> > +			   CKSYS_AUD_TOP_CFG, 0xffff, 0x4);
> > +	mtkaif_calibration_num_phase = 42;	/* mt6359: 0 ~ 42 */
> > +	mtkaif_calibration_ok = true;
> > +
> > +	for (phase = 0;
> > +	     phase <= mtkaif_calibration_num_phase &&
> > mtkaif_calibration_ok;
> > +	     phase++) {
> > +		mt6359_set_mtkaif_calibration_phase(cmpnt_codec,
> > +						    phase, phase,
> > phase);
> > +
> > +		regmap_update_bits(afe_priv->topckgen,
> > +				   CKSYS_AUD_TOP_CFG, 0x1, 0x1);
> > +
> > +		test_done_1 = 0;
> > +		test_done_2 = 0;
> > +		test_done_3 = 0;
> > +		cycle_1 = -1;
> > +		cycle_2 = -1;
> > +		cycle_3 = -1;
> > +		counter = 0;
> > +		while (!(test_done_1 & test_done_2 & test_done_3)) {
> > +			regmap_read(afe_priv->topckgen,
> > +				    CKSYS_AUD_TOP_MON, &monitor);
> > +			test_done_1 = (monitor >> 28) & 0x1;
> > +			test_done_2 = (monitor >> 29) & 0x1;
> > +			test_done_3 = (monitor >> 30) & 0x1;
> > +			if (test_done_1 == 1)
> > +				cycle_1 = monitor & 0xf;
> > +
> > +			if (test_done_2 == 1)
> > +				cycle_2 = (monitor >> 4) & 0xf;
> > +
> > +			if (test_done_3 == 1)
> > +				cycle_3 = (monitor >> 8) & 0xf;
> > +
> > +			/* handle if never test done */
> > +			if (++counter > 10000) {
> > +				dev_info(afe->dev, "%s(), test fail,
> > cycle_1 %d, cycle_2 %d, cycle_3 %d, monitor 0x%x\n",
> > +					 __func__,
> > +					 cycle_1, cycle_2, cycle_3,
> > monitor);
> > +				mtkaif_calibration_ok = false;
> > +				break;
> > +			}
> > +		}
> > +
> > +		if (phase == 0) {
> > +			prev_cycle_1 = cycle_1;
> > +			prev_cycle_2 = cycle_2;
> > +			prev_cycle_3 = cycle_3;
> > +		}
> > +
> > +		if (cycle_1 != prev_cycle_1 &&
> > +		    mtkaif_chosen_phase[MT8195_MTKAIF_MISO_0] < 0) {
> > +			mtkaif_chosen_phase[MT8195_MTKAIF_MISO_0] =
> > phase - 1;
> > +			mtkaif_phase_cycle[MT8195_MTKAIF_MISO_0] =
> > prev_cycle_1;
> > +		}
> > +
> > +		if (cycle_2 != prev_cycle_2 &&
> > +		    mtkaif_chosen_phase[MT8195_MTKAIF_MISO_1] < 0) {
> > +			mtkaif_chosen_phase[MT8195_MTKAIF_MISO_1] =
> > phase - 1;
> > +			mtkaif_phase_cycle[MT8195_MTKAIF_MISO_1] =
> > prev_cycle_2;
> > +		}
> > +
> > +		if (cycle_3 != prev_cycle_3 &&
> > +		    mtkaif_chosen_phase[MT8195_MTKAIF_MISO_2] < 0) {
> > +			mtkaif_chosen_phase[MT8195_MTKAIF_MISO_2] =
> > phase - 1;
> > +			mtkaif_phase_cycle[MT8195_MTKAIF_MISO_2] =
> > prev_cycle_3;
> > +		}
> > +
> > +		regmap_update_bits(afe_priv->topckgen,
> > +				   CKSYS_AUD_TOP_CFG, 0x1, 0x0);
> > +
> > +		if (mtkaif_chosen_phase[MT8195_MTKAIF_MISO_0] >= 0 &&
> > +		    mtkaif_chosen_phase[MT8195_MTKAIF_MISO_1] >= 0 &&
> > +		    mtkaif_chosen_phase[MT8195_MTKAIF_MISO_2] >= 0)
> > +			break;
> > +	}
> > +
> > +	if (mtkaif_chosen_phase[MT8195_MTKAIF_MISO_0] < 0) {
> > +		mtkaif_calibration_ok = false;
> > +		chosen_phase_1 = 0;
> > +	} else {
> > +		chosen_phase_1 =
> > mtkaif_chosen_phase[MT8195_MTKAIF_MISO_0];
> > +	}
> > +
> > +	if (mtkaif_chosen_phase[MT8195_MTKAIF_MISO_1] < 0) {
> > +		mtkaif_calibration_ok = false;
> > +		chosen_phase_2 = 0;
> > +	} else {
> > +		chosen_phase_2 =
> > mtkaif_chosen_phase[MT8195_MTKAIF_MISO_1];
> > +	}
> > +
> > +	if (mtkaif_chosen_phase[MT8195_MTKAIF_MISO_2] < 0) {
> > +		mtkaif_calibration_ok = false;
> > +		chosen_phase_3 = 0;
> > +	} else {
> > +		chosen_phase_3 =
> > mtkaif_chosen_phase[MT8195_MTKAIF_MISO_2];
> > +	}
> > +
> > +	mt6359_set_mtkaif_calibration_phase(cmpnt_codec,
> > +					    chosen_phase_1,
> > +					    chosen_phase_2,
> > +					    chosen_phase_3);
> > +
> > +	mt6359_mtkaif_calibration_disable(cmpnt_codec);
> > +	pm_runtime_put(afe->dev);
> > +
> > +	param->mtkaif_calibration_ok = mtkaif_calibration_ok;
> > +	param->mtkaif_chosen_phase[MT8195_MTKAIF_MISO_0] =
> > chosen_phase_1;
> > +	param->mtkaif_chosen_phase[MT8195_MTKAIF_MISO_1] =
> > chosen_phase_2;
> > +	param->mtkaif_chosen_phase[MT8195_MTKAIF_MISO_2] =
> > chosen_phase_3;
> > +	for (i = 0; i < MT8195_MTKAIF_MISO_NUM; i++)
> > +		param->mtkaif_phase_cycle[i] = mtkaif_phase_cycle[i];
> > +
> > +	dev_info(afe->dev, "%s(), end, calibration ok %d\n",
> > +		 __func__, param->mtkaif_calibration_ok);
> 
> dev_dbg?
> 

Because we don't regard calibration failure as an error, it is a hint
to show the calibration result.
I prefer to keep dev_info here.
Is it OK?

> > +
> > +	return 0;
> > +}
> > +
> > +static int mt8195_hdmitx_dptx_startup(struct snd_pcm_substream
> > *substream)
> > +{
> > +	static const unsigned int rates[] = {
> > +		48000
> > +	};
> > +	static const unsigned int channels[] = {
> > +		2, 4, 6, 8
> > +	};
> > +	static const struct snd_pcm_hw_constraint_list
> > constraints_rates = {
> > +		.count = ARRAY_SIZE(rates),
> > +		.list  = rates,
> > +		.mask = 0,
> > +	};
> > +	static const struct snd_pcm_hw_constraint_list
> > constraints_channels = {
> > +		.count = ARRAY_SIZE(channels),
> > +		.list  = channels,
> > +		.mask = 0,
> > +	};
> 
> you use the same const tables several times, move to a higher scope
> and
> reuse?
> 

There is little difference in channels between these startup ops.

> > +	struct snd_soc_pcm_runtime *rtd =
> > asoc_substream_to_rtd(substream);
> > +	struct snd_pcm_runtime *runtime = substream->runtime;
> > +	int ret;
> > +
> > +	ret = snd_pcm_hw_constraint_list(runtime, 0,
> > +					 SNDRV_PCM_HW_PARAM_RATE,
> > +					 &constraints_rates);
> > +	if (ret < 0) {
> > +		dev_err(rtd->dev, "hw_constraint_list rate failed\n");
> > +		return ret;
> > +	}
> > +
> > +	ret = snd_pcm_hw_constraint_list(runtime, 0,
> > +					 SNDRV_PCM_HW_PARAM_CHANNELS,
> > +					 &constraints_channels);
> > +	if (ret < 0) {
> > +		dev_err(rtd->dev, "hw_constraint_list channel
> > failed\n");
> > +		return ret;
> > +	}
> > +
> > +	return 0;
> > +}
> > +
> > +static const struct snd_soc_ops mt8195_hdmitx_dptx_playback_ops =
> > {
> > +	.startup = mt8195_hdmitx_dptx_startup,
> > +};
> > +
> > +static int mt8195_dptx_hw_params(struct snd_pcm_substream
> > *substream,
> > +				 struct snd_pcm_hw_params *params)
> > +{
> > +	struct snd_soc_pcm_runtime *rtd = substream->private_data;
> > +	struct snd_soc_dai *cpu_dai = asoc_rtd_to_cpu(rtd, 0);
> > +	unsigned int rate = params_rate(params);
> > +	unsigned int mclk_fs_ratio = 256;
> > +	unsigned int mclk_fs = rate * mclk_fs_ratio;
> > +
> > +	return snd_soc_dai_set_sysclk(cpu_dai, 0, mclk_fs,
> > +				      SND_SOC_CLOCK_OUT);
> 
> return snd_soc_dai_set_sysclk(cpu_dai, 0, params_rate(params) * 256,
> SND_SOC_CLOCK_OUT);
> ?
> 
> 
OK, I will modify it in v2.

> > +static int mt8195_dptx_codec_init(struct snd_soc_pcm_runtime *rtd)
> > +{
> > +	struct mt8195_mt6359_rt1011_rt5682_priv *priv =
> > +		snd_soc_card_get_drvdata(rtd->card);
> > +	struct snd_soc_component *cmpnt_codec =
> > +		asoc_rtd_to_codec(rtd, 0)->component;
> > +	int ret = 0;
> 
> unnecessary init

OK. 
> 
> > +	ret = snd_soc_card_jack_new(rtd->card, "DP Jack",
> > SND_JACK_LINEOUT,
> > +				    &priv->dp_jack, NULL, 0);
> > +	if (ret)
> > +		return ret;
> > +
> > +	return snd_soc_component_set_jack(cmpnt_codec, &priv->dp_jack,
> > NULL);
> > +}
> > +
> > +static int mt8195_hdmi_codec_init(struct snd_soc_pcm_runtime *rtd)
> > +{
> > +	struct mt8195_mt6359_rt1011_rt5682_priv *priv =
> > +		snd_soc_card_get_drvdata(rtd->card);
> > +	struct snd_soc_component *cmpnt_codec =
> > +		asoc_rtd_to_codec(rtd, 0)->component;
> > +	int ret = 0;
> 
> unnecessary init
> 
OK.

> > +	ret = snd_soc_card_jack_new(rtd->card, "HDMI Jack",
> > SND_JACK_LINEOUT,
> > +				    &priv->hdmi_jack, NULL, 0);
> > +	if (ret)
> > +		return ret;
> > +
> > +	return snd_soc_component_set_jack(cmpnt_codec, &priv-
> > >hdmi_jack, NULL);
> > +}
> > +
> > +static int mt8195_hdmitx_dptx_hw_params_fixup(struct
> > snd_soc_pcm_runtime *rtd,
> > +					      struct snd_pcm_hw_params
> > *params)
> > +
> 
> spurious line?
> 

Thanks, I didn't notice the line.
> > +{
> > +	/* fix BE i2s format to 32bit, clean param mask first */
> > +	snd_mask_reset_range(hw_param_mask(params,
> > SNDRV_PCM_HW_PARAM_FORMAT),
> > +			     0, (__force unsigned
> > int)SNDRV_PCM_FORMAT_LAST);
> > +
> > +	params_set_format(params, SNDRV_PCM_FORMAT_S24_LE);
> > +
> > +	return 0;
> > +}
> > +
> > +static int mt8195_playback_startup(struct snd_pcm_substream
> > *substream)
> > +{
> > +	static const unsigned int rates[] = {
> > +		48000
> > +	};
> > +	static const unsigned int channels[] = {
> > +		2
> > +	};
> > +	static const struct snd_pcm_hw_constraint_list
> > constraints_rates = {
> > +		.count = ARRAY_SIZE(rates),
> > +		.list  = rates,
> > +		.mask = 0,
> > +	};
> > +	static const struct snd_pcm_hw_constraint_list
> > constraints_channels = {
> > +		.count = ARRAY_SIZE(channels),
> > +		.list  = channels,
> > +		.mask = 0,
> > +	};
> 
> actually now I realize it's only the number of channels that
> differs...
> 
> > +
> > +	struct snd_soc_pcm_runtime *rtd =
> > asoc_substream_to_rtd(substream);
> > +	struct snd_pcm_runtime *runtime = substream->runtime;
> > +	int ret;
> > +
> > +	ret = snd_pcm_hw_constraint_list(runtime, 0,
> > +					 SNDRV_PCM_HW_PARAM_RATE,
> > +					 &constraints_rates);
> > +	if (ret < 0) {
> > +		dev_err(rtd->dev, "hw_constraint_list rate failed\n");
> > +		return ret;
> > +	}
> > +
> > +	ret = snd_pcm_hw_constraint_list(runtime, 0,
> > +					 SNDRV_PCM_HW_PARAM_CHANNELS,
> > +					 &constraints_channels);
> > +	if (ret < 0) {
> > +		dev_err(rtd->dev, "hw_constraint_list channel
> > failed\n");
> > +		return ret;
> > +	}
> > +
> > +	return 0;
> > +}
> > +static struct snd_soc_dai_link
> > mt8195_mt6359_rt1011_rt5682_dai_links[] = {
> > +	/* FE */
> > +	[DAI_LINK_DL2_FE] = {
> > +		.name = "DL2_FE",
> > +		.stream_name = "DL2 Playback",
> > +		.trigger = {
> > +			SND_SOC_DPCM_TRIGGER_POST,
> > +			SND_SOC_DPCM_TRIGGER_POST,
> > +		},
> > +		.dynamic = 1,
> > +		.dpcm_playback = 1,
> > +		.ops = &mt8195_playback_ops,
> > +		SND_SOC_DAILINK_REG(DL2_FE),
> > +	},
> > +	[DAI_LINK_DL3_FE] = {
> > +		.name = "DL3_FE",
> > +		.stream_name = "DL3 Playback",
> > +		.trigger = {
> > +			SND_SOC_DPCM_TRIGGER_POST,
> > +			SND_SOC_DPCM_TRIGGER_POST,
> > +		},
> > +		.dynamic = 1,
> > +		.dpcm_playback = 1,
> > +		.ops = &mt8195_playback_ops,
> > +		SND_SOC_DAILINK_REG(DL3_FE),
> > +	},
> > +	[DAI_LINK_DL6_FE] = {
> > +		.name = "DL6_FE",
> > +		.stream_name = "DL6 Playback",
> > +		.trigger = {
> > +			SND_SOC_DPCM_TRIGGER_POST,
> > +			SND_SOC_DPCM_TRIGGER_POST,
> > +		},
> > +		.dynamic = 1,
> > +		.dpcm_playback = 1,
> > +		.ops = &mt8195_playback_ops,
> > +		SND_SOC_DAILINK_REG(DL6_FE),
> > +	},
> > +	[DAI_LINK_DL7_FE] = {
> > +		.name = "DL7_FE",
> > +		.stream_name = "DL7 Playback",
> > +		.trigger = {
> > +			SND_SOC_DPCM_TRIGGER_PRE,
> > +			SND_SOC_DPCM_TRIGGER_PRE,
> > +		},
> 
> this is interesting, is it intentional that the trigger order is
> different from all FEs?

DL7, UL1 and UL6 connect to the specific HWs separately.
In such path, FE is required to enabled before BE because of the HW
design.
> 
> > +		.dynamic = 1,
> > +		.dpcm_playback = 1,
> 
> also no .ops?
> 

.ops is used to add constraints for the capability, but now the
interface is not used. That's why no .ops is assigned here.

> > +		SND_SOC_DAILINK_REG(DL7_FE),
> > +	},
> > +	[DAI_LINK_DL8_FE] = {
> > +		.name = "DL8_FE",
> > +		.stream_name = "DL8 Playback",
> > +		.trigger = {
> > +			SND_SOC_DPCM_TRIGGER_POST,
> > +			SND_SOC_DPCM_TRIGGER_POST,
> > +		},
> > +		.dynamic = 1,
> > +		.dpcm_playback = 1,
> > +		.ops = &mt8195_playback_ops,
> > +		SND_SOC_DAILINK_REG(DL8_FE),
> > +	},
> > +	[DAI_LINK_DL10_FE] = {
> > +		.name = "DL10_FE",
> > +		.stream_name = "DL10 Playback",
> > +		.trigger = {
> > +			SND_SOC_DPCM_TRIGGER_POST,
> > +			SND_SOC_DPCM_TRIGGER_POST,
> > +		},
> > +		.dynamic = 1,
> > +		.dpcm_playback = 1,
> > +		.ops = &mt8195_hdmitx_dptx_playback_ops,
> > +		SND_SOC_DAILINK_REG(DL10_FE),
> > +	},
> > +	[DAI_LINK_DL11_FE] = {
> > +		.name = "DL11_FE",
> > +		.stream_name = "DL11 Playback",
> > +		.trigger = {
> > +			SND_SOC_DPCM_TRIGGER_POST,
> > +			SND_SOC_DPCM_TRIGGER_POST,
> > +		},
> > +		.dynamic = 1,
> > +		.dpcm_playback = 1,
> > +		.ops = &mt8195_playback_ops,
> > +		SND_SOC_DAILINK_REG(DL11_FE),
> > +	},
> > +	[DAI_LINK_UL1_FE] = {
> > +		.name = "UL1_FE",
> > +		.stream_name = "UL1 Capture",
> > +		.trigger = {
> > +			SND_SOC_DPCM_TRIGGER_PRE,
> > +			SND_SOC_DPCM_TRIGGER_PRE,
> 
> and again here, why PRE and no ops?
> 
> > +static int mt8195_mt6359_rt1011_rt5682_dev_probe(struct
> > platform_device *pdev)
> > +{
> > +	struct snd_soc_card *card =
> > &mt8195_mt6359_rt1011_rt5682_soc_card;
> > +	struct device_node *platform_node;
> > +	struct snd_soc_dai_link *dai_link;
> > +	struct mt8195_mt6359_rt1011_rt5682_priv *priv = NULL;
> 
> initialization is not necessary
> 
OK.

> > +	int ret, i;
> > +
> > +	card->dev = &pdev->dev;
> > +
> > +	platform_node = of_parse_phandle(pdev->dev.of_node,
> > +					 "mediatek,platform", 0);
> > +	if (!platform_node) {
> > +		dev_dbg(&pdev->dev, "Property 'platform' missing or
> > invalid\n");
> > +		return -EINVAL;
> > +	}
> > +
> > +	for_each_card_prelinks(card, i, dai_link) {
> > +		if (!dai_link->platforms->name)
> > +			dai_link->platforms->of_node = platform_node;
> > +
> > +		if (strcmp(dai_link->name, "DPTX_BE") == 0) {
> > +			dai_link->codecs->of_node =
> > +				of_parse_phandle(pdev->dev.of_node,
> > +						 "mediatek,dptx-codec", 
> > 0);
> > +			if (!dai_link->codecs->of_node) {
> > +				dev_dbg(&pdev->dev, "No property 'dptx-
> > codec'\n");
> > +			} else {
> > +				dai_link->codecs->name = NULL;
> > +				dai_link->codecs->dai_name = "i2s-
> > hifi";
> > +				dai_link->init =
> > mt8195_dptx_codec_init;
> > +			}
> > +		}
> > +
> > +		if (strcmp(dai_link->name, "ETDM3_OUT_BE") == 0) {
> > +			dai_link->codecs->of_node =
> > +				of_parse_phandle(pdev->dev.of_node,
> > +						 "mediatek,hdmi-codec", 
> > 0);
> > +			if (!dai_link->codecs->of_node) {
> > +				dev_dbg(&pdev->dev, "No property 'hdmi-
> > codec'\n");
> > +			} else {
> > +				dai_link->codecs->name = NULL;
> > +				dai_link->codecs->dai_name = "i2s-
> > hifi";
> > +				dai_link->init =
> > mt8195_hdmi_codec_init;
> > +			}
> > +		}
> > +	}
> > +
> > +	priv = devm_kzalloc(&pdev->dev, sizeof(*priv), GFP_KERNEL);
> > +	if (!priv)
> > +		return -ENOMEM;
> > +
> > +	snd_soc_card_set_drvdata(card, priv);
> > +
> > +	ret = devm_snd_soc_register_card(&pdev->dev, card);
> > +	if (ret)
> > +		dev_err(&pdev->dev, "%s snd_soc_register_card fail
> > %d\n",
> > +			__func__, ret);
> > +	return ret;
> > +}
> > +
> > +#ifdef CONFIG_OF
> > +static const struct of_device_id
> > mt8195_mt6359_rt1011_rt5682_dt_match[] = {
> > +	{.compatible = "mediatek,mt8195_mt6359_rt1011_rt5682",},
> > +	{}
> > +};
> > +#endif
> > +
> > +static const struct dev_pm_ops mt8195_mt6359_rt1011_rt5682_pm_ops
> > = {
> > +	.poweroff = snd_soc_poweroff,
> > +	.restore = snd_soc_resume,
> > +};
> > +
> > +static struct platform_driver mt8195_mt6359_rt1011_rt5682_driver =
> > {
> > +	.driver = {
> > +		.name = "mt8195_mt6359_rt1011_rt5682",
> > +#ifdef CONFIG_OF
> > +		.of_match_table = mt8195_mt6359_rt1011_rt5682_dt_match,
> > +#endif
> > +		.pm = &mt8195_mt6359_rt1011_rt5682_pm_ops,
> > +	},
> > +	.probe = mt8195_mt6359_rt1011_rt5682_dev_probe,
> > +};
> > +
> > +module_platform_driver(mt8195_mt6359_rt1011_rt5682_driver);
> > +
> > +/* Module information */
> > +MODULE_DESCRIPTION("MT8195-MT6359-RT1011-RT5682 ALSA SoC machine
> > driver");
> > +MODULE_AUTHOR("Trevor Wu <trevor.wu@mediatek.com>");
> > +MODULE_LICENSE("GPL v2");
> 
> "GPL" is enough
> 

I see many projects use GPL v2 here, and all mediatek projects use GPL
v2, too.
I'm not sure which one is better.
Do I need to modify this?


Thanks,
Trevor

> > +MODULE_ALIAS("mt8195_mt6359_rt1011_rt5682 soc card");
> > 


^ permalink raw reply	[flat|nested] 9+ messages in thread

* Re: [PATCH 1/2] ASoC: mediatek: mt8195: add machine driver with mt6359, rt1011 and rt5682
  2021-09-13 10:24     ` Trevor Wu
@ 2021-09-24  3:54       ` Trevor Wu
  2021-09-24 14:46         ` Pierre-Louis Bossart
  0 siblings, 1 reply; 9+ messages in thread
From: Trevor Wu @ 2021-09-24  3:54 UTC (permalink / raw)
  To: Pierre-Louis Bossart, broonie, tiwai, robh+dt, matthias.bgg
  Cc: devicetree, alsa-devel, linux-kernel, linux-mediatek, aaronyu,
	linux-arm-kernel, trevor.wu

Hi Pierre-Louis,

On Mon, 2021-09-13 at 18:24 +0800, Trevor Wu wrote:
> On Fri, 2021-09-10 at 11:47 -0500, Pierre-Louis Bossart wrote:
> > > 
> 
> > > +
> > > +	param->mtkaif_calibration_ok = false;
> > > +	for (i = 0; i < MT8195_MTKAIF_MISO_NUM; i++) {
> > > +		param->mtkaif_chosen_phase[i] = -1;
> > > +		param->mtkaif_phase_cycle[i] = 0;
> > > +		mtkaif_chosen_phase[i] = -1;
> > > +		mtkaif_phase_cycle[i] = 0;
> > > +	}
> > > +
> > > +	if (IS_ERR(afe_priv->topckgen)) {
> > > +		dev_info(afe->dev, "%s() Cannot find topckgen
> > > controller\n",
> > > +			 __func__);
> > > +		return 0;
> > 
> > is this not an error? Why not dev_err() and return -EINVAL or
> > something?
> > 
> 
> Should I still return an error, even if the caller didn't check it?
> 
> Based on my understanding, the calibration function is used to make
> the
> signal more stable. 
> Most of the time, mtkaif still works, even though the calibration
> fails.
> I guess that's why the caller(I refered to the implementation of
> mt8192.) didn't check the return value of calibration function.
> 
> 
> > > +	}
> > > +
> > > +	pm_runtime_get_sync(afe->dev);
> > 
> > test if this worked?
> > 
> 
> Yes, if I didn't add pm_runtime_get_sync here, the calibration
> failed.
> 
> > > +	mt6359_mtkaif_calibration_enable(cmpnt_codec);
> > > +
> > > 
[...]
> > > +	mt6359_set_mtkaif_calibration_phase(cmpnt_codec,
> > > +					    chosen_phase_1,
> > > +					    chosen_phase_2,
> > > +					    chosen_phase_3);
> > > +
> > > +	mt6359_mtkaif_calibration_disable(cmpnt_codec);
> > > +	pm_runtime_put(afe->dev);
> > > +
> > > +	param->mtkaif_calibration_ok = mtkaif_calibration_ok;
> > > +	param->mtkaif_chosen_phase[MT8195_MTKAIF_MISO_0] =
> > > chosen_phase_1;
> > > +	param->mtkaif_chosen_phase[MT8195_MTKAIF_MISO_1] =
> > > chosen_phase_2;
> > > +	param->mtkaif_chosen_phase[MT8195_MTKAIF_MISO_2] =
> > > chosen_phase_3;
> > > +	for (i = 0; i < MT8195_MTKAIF_MISO_NUM; i++)
> > > +		param->mtkaif_phase_cycle[i] = mtkaif_phase_cycle[i];
> > > +
> > > +	dev_info(afe->dev, "%s(), end, calibration ok %d\n",
> > > +		 __func__, param->mtkaif_calibration_ok);
> > 
> > dev_dbg?
> > 
> 
> Because we don't regard calibration failure as an error, it is a hint
> to show the calibration result.
> I prefer to keep dev_info here.
> Is it OK?
> 
> > > +
> > > +	return 0;
> > > +}
> > > +
> > > +static int mt8195_hdmitx_dptx_startup(struct snd_pcm_substream
> > > *substream)
> > > +{
> > > +	static const unsigned int rates[] = {
> > > +		48000
> > > +	};
> > > +	static const unsigned int channels[] = {
> > > +		2, 4, 6, 8
> > > +	};
> > > +	static const struct snd_pcm_hw_constraint_list
> > > constraints_rates = {
> > > +		.count = ARRAY_SIZE(rates),
> > > +		.list  = rates,
> > > +		.mask = 0,
> > > +	};
> > > +	static const struct snd_pcm_hw_constraint_list
> > > constraints_channels = {
> > > +		.count = ARRAY_SIZE(channels),
> > > +		.list  = channels,
> > > +		.mask = 0,
> > > +	};
> > 
> > you use the same const tables several times, move to a higher scope
> > and
> > reuse?
> > 
> 
> There is little difference in channels between these startup ops.
> 
> > > +	struct snd_soc_pcm_runtime *rtd =
> > > asoc_substream_to_rtd(substream);
> > > +	struct snd_pcm_runtime *runtime = substream->runtime;
> > > +	int ret;
> > > +
> > > +	ret = snd_pcm_hw_constraint_list(runtime, 0,
> > > +					 SNDRV_PCM_HW_PARAM_RATE,
> > > +					 &constraints_rates);
> > > +	if (ret < 0) {
> > > +		dev_err(rtd->dev, "hw_constraint_list rate failed\n");
> > > +		return ret;
> > > +	}
> > > +
> > > +	ret = snd_pcm_hw_constraint_list(runtime, 0,
> > > +					 SNDRV_PCM_HW_PARAM_CHANNELS,
> > > +					 &constraints_channels);
> > > +	if (ret < 0) {
> > > +		dev_err(rtd->dev, "hw_constraint_list channel
> > > failed\n");
> > > +		return ret;
> > > +	}
> > > +
> > > +	return 0;
> > > +}
> > > +
> > > 
> > > +
> > > +static struct platform_driver mt8195_mt6359_rt1011_rt5682_driver
> > > =
> > > {
> > > +	.driver = {
> > > +		.name = "mt8195_mt6359_rt1011_rt5682",
> > > +#ifdef CONFIG_OF
> > > +		.of_match_table = mt8195_mt6359_rt1011_rt5682_dt_match,
> > > +#endif
> > > +		.pm = &mt8195_mt6359_rt1011_rt5682_pm_ops,
> > > +	},
> > > +	.probe = mt8195_mt6359_rt1011_rt5682_dev_probe,
> > > +};
> > > +
> > > +module_platform_driver(mt8195_mt6359_rt1011_rt5682_driver);
> > > +
> > > +/* Module information */
> > > +MODULE_DESCRIPTION("MT8195-MT6359-RT1011-RT5682 ALSA SoC machine
> > > driver");
> > > +MODULE_AUTHOR("Trevor Wu <trevor.wu@mediatek.com>");
> > > +MODULE_LICENSE("GPL v2");
> > 
> > "GPL" is enough
> > 
> 
> I see many projects use GPL v2 here, and all mediatek projects use
> GPL
> v2, too.
> I'm not sure which one is better.
> Do I need to modify this?
> 

> 
> > > +MODULE_ALIAS("mt8195_mt6359_rt1011_rt5682 soc card");
> > > 

Gentle ping.

Thanks,
Trevor


^ permalink raw reply	[flat|nested] 9+ messages in thread

* Re: [PATCH 1/2] ASoC: mediatek: mt8195: add machine driver with mt6359, rt1011 and rt5682
  2021-09-24  3:54       ` Trevor Wu
@ 2021-09-24 14:46         ` Pierre-Louis Bossart
  2021-09-27 10:33           ` Trevor Wu
  0 siblings, 1 reply; 9+ messages in thread
From: Pierre-Louis Bossart @ 2021-09-24 14:46 UTC (permalink / raw)
  To: Trevor Wu, broonie, tiwai, robh+dt, matthias.bgg
  Cc: devicetree, alsa-devel, linux-kernel, linux-mediatek, aaronyu,
	linux-arm-kernel


>>>> +/* Module information */
>>>> +MODULE_DESCRIPTION("MT8195-MT6359-RT1011-RT5682 ALSA SoC machine
>>>> driver");
>>>> +MODULE_AUTHOR("Trevor Wu <trevor.wu@mediatek.com>");
>>>> +MODULE_LICENSE("GPL v2");
>>>
>>> "GPL" is enough
>>>
>>
>> I see many projects use GPL v2 here, and all mediatek projects use
>> GPL
>> v2, too.
>> I'm not sure which one is better.
>> Do I need to modify this?

See
https://www.kernel.org/doc/html/latest/process/license-rules.html?highlight=module_license#id1

Loadable kernel modules also require a MODULE_LICENSE() tag. This tag is
neither a replacement for proper source code license information
(SPDX-License-Identifier) nor in any way relevant for expressing or
determining the exact license under which the source code of the module
is provided.

“GPL”

Module is licensed under GPL version 2. This does not express any
distinction between GPL-2.0-only or GPL-2.0-or-later. The exact license
information can only be determined via the license information in the
corresponding source files.

“GPL v2”

Same as “GPL”. It exists for historic reasons.

So "GPL v2" is not incorrect but for new contributions you might as well
use the recommended tag.

^ permalink raw reply	[flat|nested] 9+ messages in thread

* Re: [PATCH 1/2] ASoC: mediatek: mt8195: add machine driver with mt6359, rt1011 and rt5682
  2021-09-24 14:46         ` Pierre-Louis Bossart
@ 2021-09-27 10:33           ` Trevor Wu
  0 siblings, 0 replies; 9+ messages in thread
From: Trevor Wu @ 2021-09-27 10:33 UTC (permalink / raw)
  To: Pierre-Louis Bossart, broonie, tiwai, robh+dt, matthias.bgg
  Cc: devicetree, alsa-devel, linux-kernel, linux-mediatek, aaronyu,
	linux-arm-kernel

On Fri, 2021-09-24 at 09:46 -0500, Pierre-Louis Bossart wrote:
> > > > > +/* Module information */
> > > > > +MODULE_DESCRIPTION("MT8195-MT6359-RT1011-RT5682 ALSA SoC
> > > > > machine
> > > > > driver");
> > > > > +MODULE_AUTHOR("Trevor Wu <trevor.wu@mediatek.com>");
> > > > > +MODULE_LICENSE("GPL v2");
> > > > 
> > > > "GPL" is enough
> > > > 
> > > 
> > > I see many projects use GPL v2 here, and all mediatek projects
> > > use
> > > GPL
> > > v2, too.
> > > I'm not sure which one is better.
> > > Do I need to modify this?
> 
> See
> 
https://urldefense.com/v3/__https://www.kernel.org/doc/html/latest/process/license-rules.html?highlight=module_license*id1__;Iw!!CTRNKA9wMg0ARbw!0xwqsodizM7jFI4lwpT7_h2bk6xHtdNb32YDo2lneZ9u-cs5hAqqdqTci89qK8FwLg$
>  
> 
> Loadable kernel modules also require a MODULE_LICENSE() tag. This tag
> is
> neither a replacement for proper source code license information
> (SPDX-License-Identifier) nor in any way relevant for expressing or
> determining the exact license under which the source code of the
> module
> is provided.
> 
> “GPL”
> 
> Module is licensed under GPL version 2. This does not express any
> distinction between GPL-2.0-only or GPL-2.0-or-later. The exact
> license
> information can only be determined via the license information in the
> corresponding source files.
> 
> “GPL v2”
> 
> Same as “GPL”. It exists for historic reasons.
> 
> So "GPL v2" is not incorrect but for new contributions you might as
> well
> use the recommended tag.


Got it.
Thanks for your detailed explanation.
I will correct it in V2.

Trevor


^ permalink raw reply	[flat|nested] 9+ messages in thread

* Re: [PATCH 0/2] ASoC: mediatek: Add support for MT8195 sound card with rt1011 and rt5682
  2021-09-10 10:44 [PATCH 0/2] ASoC: mediatek: Add support for MT8195 sound card with rt1011 and rt5682 Trevor Wu
  2021-09-10 10:44 ` [PATCH 1/2] ASoC: mediatek: mt8195: add machine driver with mt6359, " Trevor Wu
  2021-09-10 10:44 ` [PATCH 2/2] dt-bindings: mediatek: mt8195: add mt8195-mt6359-rt1011-rt5682 document Trevor Wu
@ 2021-10-29 20:54 ` Mark Brown
  2 siblings, 0 replies; 9+ messages in thread
From: Mark Brown @ 2021-10-29 20:54 UTC (permalink / raw)
  To: Trevor Wu, matthias.bgg, robh+dt, tiwai
  Cc: alsa-devel, devicetree, linux-kernel, aaronyu, linux-arm-kernel,
	linux-mediatek

On Fri, 10 Sep 2021 18:44:03 +0800, Trevor Wu wrote:
> This series of patches adds support for mt8195 board with mt6359, rt1011
> and rt5682.
> Patches are based on broonie tree "for-next" branch.
> 
> Trevor Wu (2):
>   ASoC: mediatek: mt8195: add machine driver with mt6359, rt1011 and
>     rt5682
>   dt-bindings: mediatek: mt8195: add mt8195-mt6359-rt1011-rt5682
>     document
> 
> [...]

Applied to

   https://git.kernel.org/pub/scm/linux/kernel/git/broonie/sound.git for-next

Thanks!

[1/2] ASoC: mediatek: mt8195: add machine driver with mt6359, rt1011 and rt5682
      commit: 0261e36477cfa2608468c1300e30cb667c5e1269
[2/2] dt-bindings: mediatek: mt8195: add mt8195-mt6359-rt1011-rt5682 document
      (no commit info)

All being well this means that it will be integrated into the linux-next
tree (usually sometime in the next 24 hours) and sent to Linus during
the next merge window (or sooner if it is a bug fix), however if
problems are discovered then the patch may be dropped or reverted.

You may get further e-mails resulting from automated or manual testing
and review of the tree, please engage with people reporting problems and
send followup patches addressing any issues that are reported if needed.

If any updates are required or you are submitting further changes they
should be sent as incremental updates against current git, existing
patches will not be replaced.

Please add any relevant lists and maintainers to the CCs when replying
to this mail.

Thanks,
Mark

^ permalink raw reply	[flat|nested] 9+ messages in thread

end of thread, other threads:[~2021-10-29 20:55 UTC | newest]

Thread overview: 9+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2021-09-10 10:44 [PATCH 0/2] ASoC: mediatek: Add support for MT8195 sound card with rt1011 and rt5682 Trevor Wu
2021-09-10 10:44 ` [PATCH 1/2] ASoC: mediatek: mt8195: add machine driver with mt6359, " Trevor Wu
2021-09-10 16:47   ` Pierre-Louis Bossart
2021-09-13 10:24     ` Trevor Wu
2021-09-24  3:54       ` Trevor Wu
2021-09-24 14:46         ` Pierre-Louis Bossart
2021-09-27 10:33           ` Trevor Wu
2021-09-10 10:44 ` [PATCH 2/2] dt-bindings: mediatek: mt8195: add mt8195-mt6359-rt1011-rt5682 document Trevor Wu
2021-10-29 20:54 ` [PATCH 0/2] ASoC: mediatek: Add support for MT8195 sound card with rt1011 and rt5682 Mark Brown

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).