All of lore.kernel.org
 help / color / mirror / Atom feed
From: Pierre-Louis Bossart <pierre-louis.bossart@linux.intel.com>
To: Trevor Wu <trevor.wu@mediatek.com>,
	broonie@kernel.org, tiwai@suse.com, robh+dt@kernel.org,
	matthias.bgg@gmail.com
Cc: devicetree@vger.kernel.org, alsa-devel@alsa-project.org,
	linux-kernel@vger.kernel.org, linux-mediatek@lists.infradead.org,
	aaronyu@google.com, linux-arm-kernel@lists.infradead.org
Subject: Re: [PATCH 1/2] ASoC: mediatek: mt8195: add machine driver with mt6359, rt1011 and rt5682
Date: Fri, 10 Sep 2021 11:47:51 -0500	[thread overview]
Message-ID: <10fc49fa-9791-0225-365d-e3074680596c@linux.intel.com> (raw)
In-Reply-To: <20210910104405.11420-2-trevor.wu@mediatek.com>


> +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");
> 

WARNING: multiple messages have this Message-ID (diff)
From: Pierre-Louis Bossart <pierre-louis.bossart@linux.intel.com>
To: Trevor Wu <trevor.wu@mediatek.com>,
	broonie@kernel.org, tiwai@suse.com, robh+dt@kernel.org,
	matthias.bgg@gmail.com
Cc: devicetree@vger.kernel.org, alsa-devel@alsa-project.org,
	linux-kernel@vger.kernel.org, linux-mediatek@lists.infradead.org,
	aaronyu@google.com, linux-arm-kernel@lists.infradead.org
Subject: Re: [PATCH 1/2] ASoC: mediatek: mt8195: add machine driver with mt6359, rt1011 and rt5682
Date: Fri, 10 Sep 2021 11:47:51 -0500	[thread overview]
Message-ID: <10fc49fa-9791-0225-365d-e3074680596c@linux.intel.com> (raw)
In-Reply-To: <20210910104405.11420-2-trevor.wu@mediatek.com>


> +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");
> 

_______________________________________________
Linux-mediatek mailing list
Linux-mediatek@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-mediatek

WARNING: multiple messages have this Message-ID (diff)
From: Pierre-Louis Bossart <pierre-louis.bossart@linux.intel.com>
To: Trevor Wu <trevor.wu@mediatek.com>,
	broonie@kernel.org, tiwai@suse.com, robh+dt@kernel.org,
	matthias.bgg@gmail.com
Cc: devicetree@vger.kernel.org, alsa-devel@alsa-project.org,
	linux-kernel@vger.kernel.org, linux-mediatek@lists.infradead.org,
	aaronyu@google.com, linux-arm-kernel@lists.infradead.org
Subject: Re: [PATCH 1/2] ASoC: mediatek: mt8195: add machine driver with mt6359, rt1011 and rt5682
Date: Fri, 10 Sep 2021 11:47:51 -0500	[thread overview]
Message-ID: <10fc49fa-9791-0225-365d-e3074680596c@linux.intel.com> (raw)
In-Reply-To: <20210910104405.11420-2-trevor.wu@mediatek.com>


> +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");
> 

_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

  reply	other threads:[~2021-09-10 17:05 UTC|newest]

Thread overview: 34+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
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 10:44 ` Trevor Wu
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
2021-09-10 10:44   ` Trevor Wu
2021-09-10 10:44   ` Trevor Wu
2021-09-10 10:44   ` Trevor Wu
2021-09-10 16:47   ` Pierre-Louis Bossart [this message]
2021-09-10 16:47     ` Pierre-Louis Bossart
2021-09-10 16:47     ` Pierre-Louis Bossart
2021-09-13 10:24     ` Trevor Wu
2021-09-13 10:24       ` Trevor Wu
2021-09-13 10:24       ` Trevor Wu
2021-09-13 10:24       ` Trevor Wu
2021-09-24  3:54       ` Trevor Wu
2021-09-24  3:54         ` Trevor Wu
2021-09-24  3:54         ` Trevor Wu
2021-09-24  3:54         ` Trevor Wu
2021-09-24 14:46         ` Pierre-Louis Bossart
2021-09-24 14:46           ` Pierre-Louis Bossart
2021-09-24 14:46           ` Pierre-Louis Bossart
2021-09-27 10:33           ` Trevor Wu
2021-09-27 10:33             ` Trevor Wu
2021-09-27 10:33             ` Trevor Wu
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-09-10 10:44   ` Trevor Wu
2021-09-10 10:44   ` 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
2021-10-29 20:54   ` Mark Brown
2021-10-29 20:54   ` Mark Brown
2021-10-29 20:54   ` Mark Brown

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=10fc49fa-9791-0225-365d-e3074680596c@linux.intel.com \
    --to=pierre-louis.bossart@linux.intel.com \
    --cc=aaronyu@google.com \
    --cc=alsa-devel@alsa-project.org \
    --cc=broonie@kernel.org \
    --cc=devicetree@vger.kernel.org \
    --cc=linux-arm-kernel@lists.infradead.org \
    --cc=linux-kernel@vger.kernel.org \
    --cc=linux-mediatek@lists.infradead.org \
    --cc=matthias.bgg@gmail.com \
    --cc=robh+dt@kernel.org \
    --cc=tiwai@suse.com \
    --cc=trevor.wu@mediatek.com \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.