Re: [PATCH v4 05/13] ASoC: mediatek: mt8188: support etdm in platform driver
From: AngeloGioacchino Del Regno
Date: Mon Jan 02 2023 - 09:04:32 EST
Il 30/12/22 06:54, Trevor Wu ha scritto:
Add mt8188 etdm dai driver support.
Signed-off-by: Trevor Wu <trevor.wu@xxxxxxxxxxxx>
---
sound/soc/mediatek/mt8188/mt8188-dai-etdm.c | 2591 +++++++++++++++++++
1 file changed, 2591 insertions(+)
create mode 100644 sound/soc/mediatek/mt8188/mt8188-dai-etdm.c
diff --git a/sound/soc/mediatek/mt8188/mt8188-dai-etdm.c b/sound/soc/mediatek/mt8188/mt8188-dai-etdm.c
new file mode 100644
index 000000000000..f8e69d655706
--- /dev/null
+++ b/sound/soc/mediatek/mt8188/mt8188-dai-etdm.c
@@ -0,0 +1,2591 @@
+// SPDX-License-Identifier: GPL-2.0
+/*
+ * MediaTek ALSA SoC Audio DAI eTDM Control
+ *
+ * Copyright (c) 2022 MediaTek Inc.
+ * Author: Bicycle Tsai <bicycle.tsai@xxxxxxxxxxxx>
+ * Trevor Wu <trevor.wu@xxxxxxxxxxxx>
+ * Chun-Chia Chiu <chun-chia.chiu@xxxxxxxxxxxx>
+ */
+
..snip..
+
+static int mtk_dai_etdm_get_clkdiv_id_by_dai_id(int dai_id)
+{
+ int clk_id = -1;
+
+ switch (dai_id) {
+ case MT8188_AFE_IO_DPTX:
+ clk_id = MT8188_CLK_TOP_APLL12_DIV9;
return MT8188_CLK_TOP_APLL12_DIV_9;
+ break;
+ case MT8188_AFE_IO_ETDM1_IN:
+ clk_id = MT8188_CLK_TOP_APLL12_DIV0;
return MT8188_CLK_TOP_APLL12_DIV0;
+ break;
+ case MT8188_AFE_IO_ETDM2_IN:
+ clk_id = MT8188_CLK_TOP_APLL12_DIV1;
return .....
+ break;
+ case MT8188_AFE_IO_ETDM1_OUT:
+ clk_id = MT8188_CLK_TOP_APLL12_DIV2;
+ break;
+ case MT8188_AFE_IO_ETDM2_OUT:
+ clk_id = MT8188_CLK_TOP_APLL12_DIV3;
+ break;
+ case MT8188_AFE_IO_ETDM3_OUT:
+ default:
+ break;
+ }
+
+ return clk_id;
return -EINVAL;
...so you don't need clk_id at all :-)
+}
+
..snip..
+
+static int mtk_dai_etdm_mclk_configure(struct mtk_base_afe *afe, int dai_id)
+{
+ struct mt8188_afe_private *afe_priv = afe->platform_priv;
+ struct mtk_dai_etdm_priv *etdm_data;
+ int clk_id = mtk_dai_etdm_get_clk_id_by_dai_id(dai_id);
+ int clkdiv_id = mtk_dai_etdm_get_clkdiv_id_by_dai_id(dai_id);
+ int apll;
+ int apll_clk_id;
+ struct etdm_con_reg etdm_reg;
+ int ret = 0;
+
+ if (clk_id < 0 || clkdiv_id < 0)
+ return 0;
+
+ if (!is_valid_etdm_dai(dai_id))
+ return -EINVAL;
+ etdm_data = afe_priv->dai_priv[dai_id];
+
+ ret = get_etdm_reg(dai_id, &etdm_reg);
+ if (ret < 0)
+ return ret;
+
+ if (etdm_data->mclk_dir == SND_SOC_CLOCK_OUT)
+ regmap_set_bits(afe->regmap, etdm_reg.con1,
+ ETDM_CON1_MCLK_OUTPUT);
+ else
+ regmap_clear_bits(afe->regmap, etdm_reg.con1,
+ ETDM_CON1_MCLK_OUTPUT);
+
+ if (etdm_data->mclk_freq) {
+ apll = etdm_data->mclk_apll;
+ apll_clk_id = mt8188_afe_get_mclk_source_clk_id(apll);
+ if (apll_clk_id < 0)
+ return apll_clk_id;
+
+ /* select apll */
+ ret = mt8188_afe_set_clk_parent(afe, afe_priv->clk[clk_id],
+ afe_priv->clk[apll_clk_id]);
+ if (ret)
+ return ret;
+
+ /* set rate */
+ ret = mt8188_afe_set_clk_rate(afe, afe_priv->clk[clkdiv_id],
+ etdm_data->mclk_freq);
Check ret here...
+ } else {
+ if (etdm_data->mclk_dir == SND_SOC_CLOCK_OUT)
+ dev_dbg(afe->dev, "%s mclk freq = 0\n", __func__);
+ }
+
...and return 0 here.
+ return ret;
+}
+
..snip..
+
+static int mtk_dai_etdm_hw_params(struct snd_pcm_substream *substream,
+ struct snd_pcm_hw_params *params,
+ struct snd_soc_dai *dai)
+{
+ int ret = 0;
+ unsigned int rate = params_rate(params);
+ unsigned int bit_width = params_width(params);
+ unsigned int channels = params_channels(params);
+ struct mtk_base_afe *afe = snd_soc_dai_get_drvdata(dai);
+ struct mt8188_afe_private *afe_priv = afe->platform_priv;
+ struct mtk_dai_etdm_priv *mst_etdm_data;
+ int mst_dai_id;
+ int slv_dai_id;
+ int i;
+
+ dev_dbg(afe->dev, "%s '%s' period %u-%u\n",
+ __func__, snd_pcm_stream_str(substream),
+ params_period_size(params), params_periods(params));
+
+ if (is_cowork_mode(dai)) {
+ mst_dai_id = get_etdm_cowork_master_id(dai);
+ if (!is_valid_etdm_dai(mst_dai_id))
+ return -EINVAL;
+
+ ret = mtk_dai_etdm_mclk_configure(afe, mst_dai_id);
+ if (ret)
+ return ret;
+
+ ret = mtk_dai_etdm_configure(afe, rate, channels,
+ bit_width, mst_dai_id);
+ if (ret)
+ return ret;
+
+ mst_etdm_data = afe_priv->dai_priv[mst_dai_id];
+ for (i = 0; i < mst_etdm_data->cowork_slv_count; i++) {
+ slv_dai_id = mst_etdm_data->cowork_slv_id[i];
+ ret = mtk_dai_etdm_configure(afe, rate, channels,
+ bit_width, slv_dai_id);
+ if (ret)
+ return ret;
+
+ ret = mt8188_etdm_sync_mode_configure(afe, slv_dai_id);
+ if (ret)
+ return ret;
+ }
+ } else {
+ ret = mtk_dai_etdm_mclk_configure(afe, dai->id);
+ if (ret)
+ return ret;
+
+ ret = mtk_dai_etdm_configure(afe, rate, channels,
+ bit_width, dai->id);
Check return value here...
+ }
+
...and return 0 here.
+ return ret;
+}
+
+static int mtk_dai_etdm_prepare(struct snd_pcm_substream *substream,
+ struct snd_soc_dai *dai)
+{
+ struct mtk_base_afe *afe = snd_soc_dai_get_drvdata(dai);
+ struct mt8188_afe_private *afe_priv = afe->platform_priv;
+ struct mtk_dai_etdm_priv *mst_etdm_data;
+ int mst_dai_id;
+ int slv_dai_id;
+ int i;
+ int ret = 0;
+
+ if (!is_valid_etdm_dai(dai->id))
+ return -EINVAL;
+ mst_etdm_data = afe_priv->dai_priv[dai->id];
+
+ dev_dbg(afe->dev, "%s(), dai id %d, prepared %d\n", __func__, dai->id,
+ mst_etdm_data->is_prepared);
+
+ if (mst_etdm_data->is_prepared)
+ return 0;
+
+ mst_etdm_data->is_prepared = true;
+
+ if (is_cowork_mode(dai)) {
+ mst_dai_id = get_etdm_cowork_master_id(dai);
+ if (!is_valid_etdm_dai(mst_dai_id))
+ return -EINVAL;
+ mst_etdm_data = afe_priv->dai_priv[mst_dai_id];
+
+ for (i = 0; i < mst_etdm_data->cowork_slv_count; i++) {
+ slv_dai_id = mst_etdm_data->cowork_slv_id[i];
+ ret |= mt8188_afe_enable_etdm(afe, slv_dai_id);
+ }
+
+ ret |= mt8188_afe_enable_etdm(afe, mst_dai_id);
OR'ing return values is not an acceptable practice: you should check for error
right after the call and immediately take action if anything wrong happened.
+ } else {
+ ret = mt8188_afe_enable_etdm(afe, dai->id);
Same comment as the previous function: check ret here.
+ }
+
+ return ret;
+}
+
..snip..
+
+static int mtk_dai_hdmitx_dptx_prepare(struct snd_pcm_substream *substream,
+ struct snd_soc_dai *dai)
+{
+ struct mtk_base_afe *afe = snd_soc_dai_get_drvdata(dai);
+ struct mt8188_afe_private *afe_priv = afe->platform_priv;
+ struct mtk_dai_etdm_priv *etdm_data;
+ int ret;
+
+ if (!is_valid_etdm_dai(dai->id))
+ return -EINVAL;
+ etdm_data = afe_priv->dai_priv[dai->id];
+
+ dev_dbg(afe->dev, "%s(), dai id %d, prepared %d\n", __func__, dai->id,
+ etdm_data->is_prepared);
+
+ if (etdm_data->is_prepared)
+ return 0;
+
+ etdm_data->is_prepared = true;
+
+ /* enable dptx interface */
+ if (dai->id == MT8188_AFE_IO_DPTX)
+ regmap_set_bits(afe->regmap, AFE_DPTX_CON, AFE_DPTX_CON_ON);
+
+ /* enable etdm_out3 */
+ ret = mt8188_afe_enable_etdm(afe, dai->id);
You don't need the `ret` variable here...
return mt8188_afe_enable_etdm(afe, dai->id);
+
+ return ret;
+}
+
Apart from this, looks good.
Regards,
Angelo