Re: [PATCH 01/14] ASoC: mediatek: common: modify mtk afe common driver for mt8196
From: AngeloGioacchino Del Regno <angelogioacchino.delregno@collabora.com>
Date: 2025-03-10 15:23:18
Also in:
linux-arm-kernel, linux-gpio, linux-mediatek, linux-sound, lkml
Il 07/03/25 13:47, Darren.Ye ha scritto:
quoted hunk ↗ jump to hunk
From: Darren Ye <redacted> Export register read and write interface, add sample reate interface, and update the mtk_memif_set_channel interface for the mt8196 platform. Signed-off-by: Darren Ye <redacted> --- sound/soc/mediatek/common/mtk-afe-fe-dai.c | 30 ++++++++++++++-------- sound/soc/mediatek/common/mtk-afe-fe-dai.h | 6 +++++ sound/soc/mediatek/common/mtk-base-afe.h | 13 ++++++++++ 3 files changed, 38 insertions(+), 11 deletions(-)diff --git a/sound/soc/mediatek/common/mtk-afe-fe-dai.c b/sound/soc/mediatek/common/mtk-afe-fe-dai.c index 3809068f5620..c36dae520f04 100644 --- a/sound/soc/mediatek/common/mtk-afe-fe-dai.c +++ b/sound/soc/mediatek/common/mtk-afe-fe-dai.c@@ -18,7 +18,7 @@ #define AFE_BASE_END_OFFSET 8 -static int mtk_regmap_update_bits(struct regmap *map, int reg, +int mtk_regmap_update_bits(struct regmap *map, int reg, unsigned int mask, unsigned int val, int shift) {@@ -26,13 +26,16 @@ static int mtk_regmap_update_bits(struct regmap *map, int reg, return 0; return regmap_update_bits(map, reg, mask << shift, val << shift); } +EXPORT_SYMBOL(mtk_regmap_update_bits);
Please don't export this function: it's not the greatest, `reg` should be unsigned and so it should the shift - it's kinda wrong in principle, and you can simply use regmap_update_bits() directly in your drivers. Please, do so.
+
+int mtk_regmap_write(struct regmap *map, int reg, unsigned int val)
-static int mtk_regmap_write(struct regmap *map, int reg, unsigned int val)
{
if (reg < 0)
return 0;
return regmap_write(map, reg, val);
}
+EXPORT_SYMBOL(mtk_regmap_write);
Same here
quoted hunk ↗ jump to hunk
int mtk_afe_fe_startup(struct snd_pcm_substream *substream, struct snd_soc_dai *dai)@@ -459,8 +462,12 @@ int mtk_memif_set_channel(struct mtk_base_afe *afe, struct mtk_base_afe_memif *memif = &afe->memif[id]; unsigned int mono; - if (memif->data->mono_shift < 0) - return 0; + dev_info(afe->dev, "%s(), id: %d, channel: %d\n", __func__, id, channel);
If you really really really need this print, it's for debugging, so this should be dev_dbg() instead... but otherwise just delete it.
quoted hunk ↗ jump to hunk
+ mono = memif->data->mono_invert ^ (channel == 1); + + if (memif->data->mono_shift > 0) + mtk_regmap_update_bits(afe->regmap, memif->data->mono_reg, + 0x1, mono, memif->data->mono_shift); if (memif->data->quad_ch_mask) { unsigned int quad_ch = (channel == 4) ? 1 : 0;@@ -470,11 +477,6 @@ int mtk_memif_set_channel(struct mtk_base_afe *afe, quad_ch, memif->data->quad_ch_shift); } - if (memif->data->mono_invert) - mono = (channel == 1) ? 0 : 1; - else - mono = (channel == 1) ? 1 : 0; - /* for specific configuration of memif mono mode */ if (memif->data->int_odd_flag_reg) mtk_regmap_update_bits(afe->regmap,@@ -482,8 +484,14 @@ int mtk_memif_set_channel(struct mtk_base_afe *afe, 1, mono, memif->data->int_odd_flag_shift); - return mtk_regmap_update_bits(afe->regmap, memif->data->mono_reg, - 1, mono, memif->data->mono_shift);
Don't break older platforms. You're removing functionality here.
+ if (memif->data->ch_num_maskbit) {
+ dev_info(afe->dev, "%s(), set ch num id: %d, channel: %d\n", __func__, id, channel);Same comment applies (plus, this is a double print, even worse).
quoted hunk ↗ jump to hunk
+ mtk_regmap_update_bits(afe->regmap, memif->data->ch_num_reg, + memif->data->ch_num_maskbit, + channel, memif->data->ch_num_shift); + } + + return 0; } EXPORT_SYMBOL_GPL(mtk_memif_set_channel);diff --git a/sound/soc/mediatek/common/mtk-afe-fe-dai.h b/sound/soc/mediatek/common/mtk-afe-fe-dai.h index b6d0f2b27e86..64b10ccba291 100644 --- a/sound/soc/mediatek/common/mtk-afe-fe-dai.h +++ b/sound/soc/mediatek/common/mtk-afe-fe-dai.h@@ -12,7 +12,13 @@ struct snd_soc_dai_ops; struct mtk_base_afe; struct mtk_base_afe_memif; +struct mtk_base_irq_data; +int mtk_regmap_update_bits(struct regmap *map, int reg, + unsigned int mask, unsigned int val, + int shift); +int mtk_regmap_write(struct regmap *map, int reg, + unsigned int val);
No, don't export these. Use regmap directly. Regards, Angelo