Re: [PATCH 11/18] ASoC: mediatek: mt8365: Add platform driver

Krzysztof Kozlowski
2024-02-27T08:51:17Z

On 26/02/2024 15:01, Alexandre Mergnat wrote:
> Add mt8365 platform driver.
>
> Signed-off-by: Alexandre Mergnat <amergnat@baylibre.com>

...

> + memif->substream = substream;
> +
> + snd_pcm_hw_constraint_step(substream->runtime, 0,
> + SNDRV_PCM_HW_PARAM_BUFFER_BYTES, 16);
> +
> + snd_soc_set_runtime_hwparams(substream, mtk_afe_hardware);
> +
> + ret = snd_pcm_hw_constraint_integer(runtime,
> + SNDRV_PCM_HW_PARAM_PERIODS);
> + if (ret < 0)
> + dev_err(afe->dev, "snd_pcm_hw_constraint_integer failed\n");
> +
> + mt8365_afe_enable_main_clk(afe);
> + return ret;
> +}
> +
> +static void mt8365_afe_fe_shutdown(struct snd_pcm_substream *substream,
> + struct snd_soc_dai *dai)
> +{
> + struct snd_soc_pcm_runtime *rtd = snd_soc_substream_to_rtd(substream);
> + struct mtk_base_afe *afe = snd_soc_dai_get_drvdata(dai);
> + int memif_num = snd_soc_rtd_to_cpu(rtd, 0)->id;
> + struct mtk_base_afe_memif *memif = &afe->memif[memif_num];
> +
> + dev_dbg(afe->dev, "%s %s\n", __func__, memif->data->name);

Drop trace-like prints. Tracing is for this. Before upstreaming driver
from vendor repository, such stuff should be cleaned.

> +
> + memif->substream = NULL;
> +
> + mt8365_afe_disable_main_clk(afe);
> +}
> +
> +static int mt8365_afe_fe_hw_params(struct snd_pcm_substream *substream,
> + struct snd_pcm_hw_params *params, struct snd_soc_dai *dai)
> +{
> + struct snd_soc_pcm_runtime *rtd = snd_soc_substream_to_rtd(substream);
> + struct mtk_base_afe *afe = snd_soc_dai_get_drvdata(dai);
> + struct mt8365_afe_private *afe_priv = afe->platform_priv;
> + int dai_id = snd_soc_rtd_to_cpu(rtd, 0)->id;
> + struct mt8365_control_data *ctrl_data = &afe_priv->ctrl_data;
> + struct mtk_base_afe_memif *memif = &afe->memif[dai_id];
> + struct mt8365_fe_dai_data *fe_data = &afe_priv->fe_data[dai_id];
> + const size_t request_size = params_buffer_bytes(params);
> + unsigned int channels = params_channels(params);
> + unsigned int rate = params_rate(params);
> + int ret, fs = 0;
> + unsigned int base_end_offset = 8;
> +
> + dev_info(afe->dev,
> + "%s %s period = %d rate = %d channels = %d\n",
> + __func__, memif->data->name, params_period_size(params),
> + rate, channels);
> +
> + if (dai_id == MT8365_AFE_MEMIF_VUL2) {
> + if (!ctrl_data->bypass_cm1)
> + /* configure cm1 */
> + mt8365_afe_configure_cm(afe,
> + MT8365_CM1, channels, rate);
> + else
> + regmap_update_bits(afe->regmap, AFE_CM1_CON0,
> + CM_AFE_CM1_VUL_SEL, CM_AFE_CM1_VUL_SEL);
> + } else if (dai_id == MT8365_AFE_MEMIF_TDM_IN) {
> + if (!ctrl_data->bypass_cm2)
> + /* configure cm2 */
> + mt8365_afe_configure_cm(afe,
> + MT8365_CM2, channels, rate);
> + else
> + regmap_update_bits(afe->regmap, AFE_CM2_CON0,
> + CM_AFE_CM2_TDM_SEL, ~CM_AFE_CM2_TDM_SEL);
> +
> + base_end_offset = 4;
> + }
> +
> + if (request_size > fe_data->sram_size) {
> + ret = snd_pcm_lib_malloc_pages(substream, request_size);
> + if (ret < 0) {
> + dev_err(afe->dev,
> + "%s %s malloc pages %zu bytes failed %d\n",
> + __func__, memif->data->name, request_size, ret);
> + return ret;
> + }
> +
> + fe_data->use_sram = false;
> +
> + mt8365_afe_emi_clk_on(afe);
> + } else {
> + struct snd_dma_buffer *dma_buf = &substream->dma_buffer;
> +
> + dma_buf->dev.type = SNDRV_DMA_TYPE_DEV;
> + dma_buf->dev.dev = substream->pcm->card->dev;
> + dma_buf->area = (unsigned char *)fe_data->sram_vir_addr;
> + dma_buf->addr = fe_data->sram_phy_addr;
> + dma_buf->bytes = request_size;
> + snd_pcm_set_runtime_buffer(substream, dma_buf);
> +
> + fe_data->use_sram = true;
> + }
> +
> + memif->phys_buf_addr = lower_32_bits(substream->runtime->dma_addr);
> + memif->buffer_size = substream->runtime->dma_bytes;
> +
> + /* start */
> + regmap_write(afe->regmap, memif->data->reg_ofs_base,
> + memif->phys_buf_addr);
> + /* end */
> + regmap_write(afe->regmap,
> + memif->data->reg_ofs_base + base_end_offset,
> + memif->phys_buf_addr + memif->buffer_size - 1);
> +
> + /* set channel */
> + if (memif->data->mono_shift >= 0) {
> + unsigned int mono = (params_channels(params) == 1) ? 1 : 0;
> +
> + if (memif->data->mono_reg < 0)
> + dev_info(afe->dev, "%s mono_reg is NULL\n", __func__);
> + else
> + regmap_update_bits(afe->regmap, memif->data->mono_reg,
> + 1 << memif->data->mono_shift,
> + mono << memif->data->mono_shift);
> + }
> +
> + /* set rate */
> + if (memif->data->fs_shift < 0)
> + return 0;
> +
> + fs = afe->memif_fs(substream, params_rate(params));
> +
> + if (fs < 0)
> + return -EINVAL;
> +
> + if (memif->data->fs_reg < 0)
> + dev_info(afe->dev, "%s fs_reg is NULL\n", __func__);
> + else
> + regmap_update_bits(afe->regmap, memif->data->fs_reg,
> + memif->data->fs_maskbit << memif->data->fs_shift,
> + fs << memif->data->fs_shift);
> +
> + return 0;
> +}
> +
> +static int mt8365_afe_fe_hw_free(struct snd_pcm_substream *substream,
> + struct snd_soc_dai *dai)
> +{
> + struct snd_soc_pcm_runtime *rtd = snd_soc_substream_to_rtd(substream);
> + struct mtk_base_afe *afe = snd_soc_dai_get_drvdata(dai);
> + struct mt8365_afe_private *afe_priv = afe->platform_priv;
> + int dai_id = snd_soc_rtd_to_cpu(rtd, 0)->id;
> + struct mtk_base_afe_memif *memif = &afe->memif[dai_id];
> + struct mt8365_fe_dai_data *fe_data = &afe_priv->fe_data[dai_id];
> + int ret = 0;
> +
> + dev_dbg(afe->dev, "%s %s\n", __func__, memif->data->name);

Drop

> +
> + if (fe_data->use_sram) {
> + snd_pcm_set_runtime_buffer(substream, NULL);
> + } else {
> + ret = snd_pcm_lib_free_pages(substream);
> +
> + mt8365_afe_emi_clk_off(afe);
> + }
> +
> + return ret;
> +}
> +
> +static int mt8365_afe_fe_prepare(struct snd_pcm_substream *substream,
> + struct snd_soc_dai *dai)
> +{
> + struct snd_soc_pcm_runtime *rtd = snd_soc_substream_to_rtd(substream);
> + struct mtk_base_afe *afe = snd_soc_dai_get_drvdata(dai);
> + int dai_id = snd_soc_rtd_to_cpu(rtd, 0)->id;
> + struct mtk_base_afe_memif *memif = &afe->memif[dai_id];
> +
> + /* set format */
> + if (memif->data->hd_reg >= 0) {
> + switch (substream->runtime->format) {
> + case SNDRV_PCM_FORMAT_S16_LE:
> + regmap_update_bits(afe->regmap, memif->data->hd_reg,
> + 3 << memif->data->hd_shift,
> + 0 << memif->data->hd_shift);
> + break;
> + case SNDRV_PCM_FORMAT_S32_LE:
> + regmap_update_bits(afe->regmap, memif->data->hd_reg,
> + 3 << memif->data->hd_shift,
> + 3 << memif->data->hd_shift);
> +
> + if (dai_id == MT8365_AFE_MEMIF_TDM_IN) {
> + regmap_update_bits(afe->regmap,
> + memif->data->hd_reg,
> + 3 << memif->data->hd_shift,
> + 1 << memif->data->hd_shift);
> + regmap_update_bits(afe->regmap,
> + memif->data->hd_reg,
> + 1 << memif->data->hd_align_mshift,
> + 1 << memif->data->hd_align_mshift);
> + }
> + break;
> + case SNDRV_PCM_FORMAT_S24_LE:
> + regmap_update_bits(afe->regmap, memif->data->hd_reg,
> + 3 << memif->data->hd_shift,
> + 1 << memif->data->hd_shift);
> + break;
> + default:
> + return -EINVAL;
> + }
> + }
> +
> + mt8365_afe_irq_direction_enable(afe,
> + memif->irq_usage,
> + MT8365_AFE_IRQ_DIR_MCU);
> +
> + return 0;
> +}
> +
> +int mt8365_afe_fe_trigger(struct snd_pcm_substream *substream, int cmd,
> + struct snd_soc_dai *dai)
> +{
> + struct snd_soc_pcm_runtime *rtd = snd_soc_substream_to_rtd(substream);
> + struct mtk_base_afe *afe = snd_soc_dai_get_drvdata(dai);
> + struct mt8365_afe_private *afe_priv = afe->platform_priv;
> + int dai_id = snd_soc_rtd_to_cpu(rtd, 0)->id;
> + struct mt8365_control_data *ctrl_data = &afe_priv->ctrl_data;
> +
> + switch (cmd) {
> + case SNDRV_PCM_TRIGGER_START:
> + case SNDRV_PCM_TRIGGER_RESUME:
> + /* enable channel merge */
> + if (dai_id == MT8365_AFE_MEMIF_VUL2 &&
> + !ctrl_data->bypass_cm1) {
> + regmap_update_bits(afe->regmap,
> + AFE_CM1_CON0,
> + CM_AFE_CM_ON, CM_AFE_CM_ON);
> + } else if (dai_id == MT8365_AFE_MEMIF_TDM_IN &&
> + !ctrl_data->bypass_cm2) {
> + regmap_update_bits(afe->regmap,
> + AFE_CM2_CON0,
> + CM_AFE_CM_ON, CM_AFE_CM_ON);
> + }
> + break;
> + case SNDRV_PCM_TRIGGER_STOP:
> + case SNDRV_PCM_TRIGGER_SUSPEND:
> + /* disable channel merge */
> + if (dai_id == MT8365_AFE_MEMIF_VUL2 &&
> + !ctrl_data->bypass_cm1) {
> + regmap_update_bits(afe->regmap,
> + AFE_CM1_CON0,
> + CM_AFE_CM_ON, ~CM_AFE_CM_ON);
> + } else if (dai_id == MT8365_AFE_MEMIF_TDM_IN &&
> + !ctrl_data->bypass_cm2) {
> + regmap_update_bits(afe->regmap,
> + AFE_CM2_CON0,
> + CM_AFE_CM_ON, ~CM_AFE_CM_ON);
> + }
> + break;
> + default:
> + break;
> + }
> +
> + return mtk_afe_fe_trigger(substream, cmd, dai);
> +}
> +
> +static int mt8365_afe_hw_gain1_startup(struct snd_pcm_substream *substream,
> + struct snd_soc_dai *dai)
> +{
> + struct mtk_base_afe *afe = snd_soc_dai_get_drvdata(dai);
> +
> + mt8365_afe_enable_main_clk(afe);
> + return 0;
> +}
> +
> +static void mt8365_afe_hw_gain1_shutdown(struct snd_pcm_substream *substream,
> + struct snd_soc_dai *dai)
> +{
> + struct mtk_base_afe *afe = snd_soc_dai_get_drvdata(dai);
> + struct mt8365_afe_private *afe_priv = afe->platform_priv;
> + struct mt8365_be_dai_data *be =
> + &afe_priv->be_data[dai->id - MT8365_AFE_BACKEND_BASE];
> +
> + const unsigned int stream = substream->stream;

Why do you const local scalars? What is the point of this variable anyway?

> +
> + if (be->prepared[stream]) {
> + regmap_update_bits(afe->regmap, AFE_GAIN1_CON0,
> + AFE_GAIN1_CON0_EN_MASK, 0);
> + be->prepared[stream] = false;
> + }
> + mt8365_afe_disable_main_clk(afe);
> +}
> +
> +static int mt8365_afe_hw_gain1_prepare(struct snd_pcm_substream *substream,
> + struct snd_soc_dai *dai)
> +{
> + struct snd_pcm_runtime * const runtime = substream->runtime;
> + struct mtk_base_afe *afe = snd_soc_dai_get_drvdata(dai);
> + struct mt8365_afe_private *afe_priv = afe->platform_priv;
> + struct mt8365_be_dai_data *be =
> + &afe_priv->be_data[dai->id - MT8365_AFE_BACKEND_BASE];
> +
> + const unsigned int rate = runtime->rate;
> + const unsigned int stream = substream->stream;

So this const is everywhere? How does it change it from code safety? No
one expects to overwrite such variables anyway, so there is no increase
in readability, either.

> + int fs;
> + unsigned int val1 = 0, val2 = 0;
> +
> + if (be->prepared[stream]) {
> + dev_info(afe->dev, "%s prepared already\n", __func__);
> + return 0;
> + }
> +
> + fs = mt8365_afe_fs_timing(rate);
> + regmap_update_bits(afe->regmap, AFE_GAIN1_CON0,
> + AFE_GAIN1_CON0_MODE_MASK, (unsigned int)fs<<4);
> +
> + regmap_read(afe->regmap, AFE_GAIN1_CON1, &val1);
> + regmap_read(afe->regmap, AFE_GAIN1_CUR, &val2);
> + if ((val1 & AFE_GAIN1_CON1_MASK) != (val2 & AFE_GAIN1_CUR_MASK))
> + regmap_update_bits(afe->regmap, AFE_GAIN1_CUR,
> + AFE_GAIN1_CUR_MASK, val1);
> +
> + regmap_update_bits(afe->regmap, AFE_GAIN1_CON0,
> + AFE_GAIN1_CON0_EN_MASK, 1);
> + be->prepared[stream] = true;
> +
> + return 0;
> +}
> +
> +static const struct snd_pcm_hardware mt8365_hostless_hardware = {
> + .info = (SNDRV_PCM_INFO_MMAP | SNDRV_PCM_INFO_INTERLEAVED |
> + SNDRV_PCM_INFO_MMAP_VALID),
> + .period_bytes_min = 256,
> + .period_bytes_max = 4 * 48 * 1024,
> + .periods_min = 2,
> + .periods_max = 256,
> + .buffer_bytes_max = 8 * 48 * 1024,
> + .fifo_size = 0,
> +};
> +
> +/* dai ops */
> +static int mtk_dai_hostless_startup(struct snd_pcm_substream *substream,
> + struct snd_soc_dai *dai)
> +{
> + struct mtk_base_afe *afe = snd_soc_dai_get_drvdata(dai);
> + struct snd_pcm_runtime *runtime = substream->runtime;
> + int ret;
> +
> + snd_soc_set_runtime_hwparams(substream, &mt8365_hostless_hardware);
> +
> + ret = snd_pcm_hw_constraint_integer(runtime,
> + SNDRV_PCM_HW_PARAM_PERIODS);
> + if (ret < 0)
> + dev_info(afe->dev, "snd_pcm_hw_constraint_integer failed\n");
> + return ret;
> +}
> +
> +/* FE DAIs */
> +static const struct snd_soc_dai_ops mt8365_afe_fe_dai_ops = {
> + .startup = mt8365_afe_fe_startup,
> + .shutdown = mt8365_afe_fe_shutdown,
> + .hw_params = mt8365_afe_fe_hw_params,
> + .hw_free = mt8365_afe_fe_hw_free,
> + .prepare = mt8365_afe_fe_prepare,
> + .trigger = mt8365_afe_fe_trigger,
> +};
> +
> +static const struct snd_soc_dai_ops mt8365_dai_hostless_ops = {
> + .startup = mtk_dai_hostless_startup,
> +};
> +

...

> +static int mt8365_afe_cm2_io_input_mux_get(struct snd_kcontrol *kcontrol,
> + struct snd_ctl_elem_value *ucontrol)
> +{
> + ucontrol->value.integer.value[0] = mCM2Input;
> +
> + return 0;
> +}
> +
> +static int mt8365_afe_cm2_io_input_mux_put(struct snd_kcontrol *kcontrol,
> + struct snd_ctl_elem_value *ucontrol)

Run checkpatch --strict

> +{
> + struct snd_soc_dapm_context *dapm =
> + snd_soc_dapm_kcontrol_dapm(kcontrol);
> + struct snd_soc_component *comp = snd_soc_dapm_to_component(dapm);
> + struct mtk_base_afe *afe = snd_soc_component_get_drvdata(comp);
> + struct mt8365_afe_private *afe_priv = afe->platform_priv;
> + int ret;
> +
> + mCM2Input = ucontrol->value.enumerated.item[0];
> +
> + afe_priv->cm2_mux_input = mCM2Input;
> + ret = snd_soc_dapm_put_enum_double(kcontrol, ucontrol);
> +
> + return ret;
> +}
> +

...

> +
> +static const struct regmap_config mt8365_afe_regmap_config = {
> + .reg_bits = 32,
> + .reg_stride = 4,
> + .val_bits = 32,
> + .max_register = MAX_REGISTER,
> + .cache_type = REGCACHE_NONE,
> +};
> +
> +static irqreturn_t mt8365_afe_irq_handler(int irq, void *dev_id)
> +{
> + struct mtk_base_afe *afe = dev_id;
> + unsigned int reg_value;
> + unsigned int mcu_irq_mask;
> + int i, ret;
> +
> + ret = regmap_read(afe->regmap, AFE_IRQ_MCU_STATUS, &reg_value);
> + if (ret) {
> + dev_err(afe->dev, "%s irq status err\n", __func__);

I can imagine happy user seeing flooded dmesg on each IRQ with these
messages...

> + reg_value = AFE_IRQ_STATUS_BITS;
> + goto err_irq;
> + }
> +
> + ret = regmap_read(afe->regmap, AFE_IRQ_MCU_EN, &mcu_irq_mask);
> + if (ret) {
> + dev_err(afe->dev, "%s irq mcu_en err\n", __func__);
> + reg_value = AFE_IRQ_STATUS_BITS;
> + goto err_irq;
> + }
> +
> + /* only clr cpu irq */
> + reg_value &= mcu_irq_mask;
> +
> + for (i = 0; i < MT8365_AFE_MEMIF_NUM; i++) {
> + struct mtk_base_afe_memif *memif = &afe->memif[i];
> + struct mtk_base_afe_irq *mcu_irq;
> +
> + if (memif->irq_usage < 0)
> + continue;
> +
> + mcu_irq = &afe->irqs[memif->irq_usage];
> +
> + if (!(reg_value & (1 << mcu_irq->irq_data->irq_clr_shift)))
> + continue;
> +
> + snd_pcm_period_elapsed(memif->substream);
> + }
> +
> +err_irq:
> + /* clear irq */
> + regmap_write(afe->regmap, AFE_IRQ_MCU_CLR,
> + reg_value & AFE_IRQ_STATUS_BITS);
> +
> + return IRQ_HANDLED;
> +}
> +
> +static int __maybe_unused mt8365_afe_runtime_suspend(struct device *dev)
> +{
> + return 0;
> +}
> +
> +static int mt8365_afe_runtime_resume(struct device *dev)
> +{
> + return 0;
> +}
> +
> +static int __maybe_unused mt8365_afe_suspend(struct device *dev)
> +{
> + struct mtk_base_afe *afe = dev_get_drvdata(dev);
> + struct regmap *regmap = afe->regmap;
> + int i;
> +
> + mt8365_afe_enable_main_clk(afe);
> +
> + if (!afe->reg_back_up)
> + afe->reg_back_up =
> + devm_kcalloc(dev, afe->reg_back_up_list_num,
> + sizeof(unsigned int), GFP_KERNEL);
> +
> + for (i = 0; i < afe->reg_back_up_list_num; i++)
> + regmap_read(regmap, afe->reg_back_up_list[i],
> + &afe->reg_back_up[i]);
> +
> + mt8365_afe_disable_main_clk(afe);
> +
> + return 0;
> +}
> +
> +static int __maybe_unused mt8365_afe_resume(struct device *dev)
> +{
> + struct mtk_base_afe *afe = dev_get_drvdata(dev);
> + struct regmap *regmap = afe->regmap;
> + int i = 0;
> +
> + if (!afe->reg_back_up) {
> + dev_dbg(dev, "%s no reg_backup\n", __func__);
> + return 0;
> + }
> +
> + mt8365_afe_enable_main_clk(afe);
> +
> + for (i = 0; i < afe->reg_back_up_list_num; i++)
> + regmap_write(regmap, afe->reg_back_up_list[i],
> + afe->reg_back_up[i]);
> +
> + mt8365_afe_disable_main_clk(afe);
> +
> + return 0;
> +}
> +
> +static int __maybe_unused mt8365_afe_dev_runtime_suspend(struct device *dev)
> +{
> + struct mtk_base_afe *afe = dev_get_drvdata(dev);
> +
> + dev_dbg(afe->dev, "%s suspend %d %d >>\n", __func__,
> + pm_runtime_status_suspended(dev), afe->suspended);

Drop silly trace-like prints. Tracing is for this.

> +
> + if (pm_runtime_status_suspended(dev) || afe->suspended)
> + return 0;
> +
> + mt8365_afe_suspend(dev);
> +
> + afe->suspended = true;
> +
> + dev_dbg(afe->dev, "%s <<\n", __func__);

Drop silly trace-like prints. Tracing is for this.

> +
> + return 0;
> +}
> +
> +static int __maybe_unused mt8365_afe_dev_runtime_resume(struct device *dev)
> +{
> + struct mtk_base_afe *afe = dev_get_drvdata(dev);
> +
> + dev_dbg(afe->dev, "%s suspend %d %d >>\n", __func__,
> + pm_runtime_status_suspended(dev), afe->suspended);

Drop silly trace-like prints. Tracing is for this.

> +
> + if (pm_runtime_status_suspended(dev) || !afe->suspended)
> + return 0;
> +
> + mt8365_afe_resume(dev);
> +
> + afe->suspended = false;
> +
> + dev_dbg(afe->dev, "%s <<\n", __func__);

Drop silly trace-like prints. Tracing is for this.

> +
> + return 0;
> +}
> +
> +static int mt8365_afe_init_registers(struct mtk_base_afe *afe)
> +{
> + size_t i;
> +
> + static struct {
> + unsigned int reg;
> + unsigned int mask;
> + unsigned int val;
> + } init_regs[] = {
> + { AFE_CONN_24BIT, GENMASK(31, 0), GENMASK(31, 0) },
> + { AFE_CONN_24BIT_1, GENMASK(21, 0), GENMASK(21, 0) },
> + };
> +
> + mt8365_afe_enable_main_clk(afe);
> +
> + for (i = 0; i < ARRAY_SIZE(init_regs); i++)
> + regmap_update_bits(afe->regmap, init_regs[i].reg,
> + init_regs[i].mask, init_regs[i].val);
> +
> + mt8365_afe_disable_main_clk(afe);
> +
> + return 0;
> +}
> +
> +static int mt8365_afe_component_probe(struct snd_soc_component *component)
> +{
> + return mtk_afe_add_sub_dai_control(component);
> +}
> +
> +static const struct snd_soc_component_driver mt8365_afe_component = {
> + .name = AFE_PCM_NAME,
> + .probe = mt8365_afe_component_probe,
> + .pointer = mtk_afe_pcm_pointer,
> + .pcm_construct = mtk_afe_pcm_new,
> +};
> +
> +static const struct snd_soc_component_driver mt8365_afe_pcm_component = {
> + .name = "mt8365-afe-pcm-dai",
> +};
> +
> +static int mt8365_dai_memif_register(struct mtk_base_afe *afe)
> +{
> + struct mtk_base_afe_dai *dai;
> +
> + dai = devm_kzalloc(afe->dev, sizeof(*dai), GFP_KERNEL);
> + if (!dai)
> + return -ENOMEM;
> +
> + list_add(&dai->list, &afe->sub_dais);
> +
> + dai->dai_drivers = mt8365_memif_dai_driver;
> + dai->num_dai_drivers = ARRAY_SIZE(mt8365_memif_dai_driver);
> +
> + dai->dapm_widgets = mt8365_memif_widgets;
> + dai->num_dapm_widgets = ARRAY_SIZE(mt8365_memif_widgets);
> + dai->dapm_routes = mt8365_memif_routes;
> + dai->num_dapm_routes = ARRAY_SIZE(mt8365_memif_routes);
> + return 0;
> +}
> +
> +typedef int (*dai_register_cb)(struct mtk_base_afe *);
> +static const dai_register_cb dai_register_cbs[] = {
> + mt8365_dai_pcm_register,
> + mt8365_dai_i2s_register,
> + mt8365_dai_adda_register,
> + mt8365_dai_dmic_register,
> + mt8365_dai_memif_register,
> +};
> +
> +static int mt8365_afe_pcm_dev_probe(struct platform_device *pdev)
> +{
> + struct mtk_base_afe *afe;
> + struct mt8365_afe_private *afe_priv;
> + struct device *dev;
> + int ret, i, sel_irq;
> + unsigned int irq_id;
> + struct resource *res;
> +
> + afe = devm_kzalloc(&pdev->dev, sizeof(*afe), GFP_KERNEL);
> + if (!afe)
> + return -ENOMEM;
> + platform_set_drvdata(pdev, afe);
> +
> + afe->platform_priv = devm_kzalloc(&pdev->dev, sizeof(*afe_priv),
> + GFP_KERNEL);
> + if (!afe->platform_priv)
> + return -ENOMEM;
> +
> + afe_priv = afe->platform_priv;
> + afe->dev = &pdev->dev;
> + dev = afe->dev;
> +
> + spin_lock_init(&afe_priv->afe_ctrl_lock);
> + mutex_init(&afe_priv->afe_clk_mutex);
> +
> + res = platform_get_resource(pdev, IORESOURCE_MEM, 0);
> + afe->base_addr = devm_ioremap_resource(&pdev->dev, res);

Why not using helper combining these two?

> + if (IS_ERR(afe->base_addr))
> + return PTR_ERR(afe->base_addr);
> +
> + res = platform_get_resource(pdev, IORESOURCE_MEM, 1);
> + if (res) {
> + afe_priv->afe_sram_vir_addr =
> + devm_ioremap_resource(&pdev->dev, res);
> + if (!IS_ERR(afe_priv->afe_sram_vir_addr)) {
> + afe_priv->afe_sram_phy_addr = res->start;
> + afe_priv->afe_sram_size = resource_size(res);
> + }
> + }
> +
> + /* initial audio related clock */
> + ret = mt8365_afe_init_audio_clk(afe);
> + if (ret) {
> + dev_err(afe->dev, "mt8365_afe_init_audio_clk fail\n");

return dev_err_probe()

> + return ret;
> + }
> +
> + afe->regmap = devm_regmap_init_mmio_clk(&pdev->dev, "top_audio_sel",
> + afe->base_addr, &mt8365_afe_regmap_config);
> + if (IS_ERR(afe->regmap))
> + return PTR_ERR(afe->regmap);
> +
> + /* memif % irq initialize*/
> + afe->memif_size = MT8365_AFE_MEMIF_NUM;
> + afe->memif = devm_kcalloc(afe->dev, afe->memif_size,
> + sizeof(*afe->memif), GFP_KERNEL);
> + if (!afe->memif)
> + return -ENOMEM;
> +
> + afe->irqs_size = MT8365_AFE_IRQ_NUM;
> + afe->irqs = devm_kcalloc(afe->dev, afe->irqs_size,
> + sizeof(*afe->irqs), GFP_KERNEL);
> + if (!afe->irqs)
> + return -ENOMEM;
> +
> + for (i = 0; i < afe->irqs_size; i++)
> + afe->irqs[i].irq_data = &irq_data[i];
> +
> + irq_id = platform_get_irq(pdev, 0);
> + if (!irq_id) {

That does not look correct. 0 is valid. Look at help of this function.

> + dev_err(afe->dev, "np %s no irq\n", afe->dev->of_node->name);

return dev_err_probe()

> + return -ENXIO;
> + }
> + ret = devm_request_irq(afe->dev, irq_id, mt8365_afe_irq_handler,
> + 0, "Afe_ISR_Handle", (void *)afe);
> + if (ret) {
> + dev_err(afe->dev, "could not request_irq\n");

return dev_err_probe()

> + return ret;
> + }
> +
> + /* init sub_dais */
> + INIT_LIST_HEAD(&afe->sub_dais);
> +
> + for (i = 0; i < ARRAY_SIZE(dai_register_cbs); i++) {
> + ret = dai_register_cbs[i](afe);
> + if (ret) {
> + dev_warn(afe->dev, "dai register i %d fail, ret %d\n",
> + i, ret);
> + return ret;
> + }
> + }
> +
> + /* init dai_driver and component_driver */
> + ret = mtk_afe_combine_sub_dai(afe);
> + if (ret) {
> + dev_warn(afe->dev, "mtk_afe_combine_sub_dai fail, ret %d\n",
> + ret);
> + return ret;
> + }
> +
> + for (i = 0; i < afe->memif_size; i++) {
> + afe->memif[i].data = &memif_data[i];
> + sel_irq = memif_specified_irqs[i];
> + if (sel_irq >= 0) {
> + afe->memif[i].irq_usage = sel_irq;
> + afe->memif[i].const_irq = 1;
> + afe->irqs[sel_irq].irq_occupyed = true;
> + } else {
> + afe->memif[i].irq_usage = -1;
> + }
> + }
> +
> + afe->mtk_afe_hardware = &mt8365_afe_hardware;
> + afe->memif_fs = mt8365_memif_fs;
> + afe->irq_fs = mt8365_irq_fs;
> +
> + pm_runtime_enable(&pdev->dev);
> +
> + pm_runtime_get_sync(&pdev->dev);
> + afe->reg_back_up_list = mt8365_afe_backup_list;
> + afe->reg_back_up_list_num = ARRAY_SIZE(mt8365_afe_backup_list);
> + afe->runtime_resume = mt8365_afe_runtime_resume;
> + afe->runtime_suspend = mt8365_afe_runtime_suspend;
> +
> + /* open afe pdn for dapm read/write audio register */
> + mt8365_afe_enable_top_cg(afe, MT8365_TOP_CG_AFE);
> +
> + /* Set 26m parent clk */
> + mt8365_afe_set_clk_parent(afe,
> + afe_priv->clocks[MT8365_CLK_TOP_AUD_SEL],
> + afe_priv->clocks[MT8365_CLK_CLK26M]);
> +
> + ret = devm_snd_soc_register_component(&pdev->dev,
> + &mt8365_afe_component, NULL, 0);
> + if (ret) {
> + dev_warn(dev, "err_platform\n");
> + return ret;
> + }
> +
> + ret = devm_snd_soc_register_component(&pdev->dev,
> + &mt8365_afe_pcm_component,
> + afe->dai_drivers,
> + afe->num_dai_drivers);
> + if (ret) {
> + dev_warn(dev, "err_dai_component\n");
> + return ret;
> + }
> +
> + mt8365_afe_init_registers(afe);
> +
> + dev_info(&pdev->dev, "MT8365 AFE driver initialized.\n");

Drop simple probe success messages everywhere. They are not needed and
they don't bring useful information.

Best regards,
Krzysztof