Re: [PATCH v8 08/10] ASoC: mediatek: mt8196: add platform driver

Cyril Chao (钞悦)
2026-04-16T05:53:46Z

Thank you for your assistance in reviewing. Could you please also
review the modifications in the diff? If everything is okay, I will
include them in v9 in the next update.

diff --git a/sound/soc/mediatek/mt8196/mt8196-afe-pcm.c
b/sound/soc/mediatek/mt8196/mt8196-afe-pcm.c
index 3d3174cd8efb..ff7aa89e4779 100644
--- a/sound/soc/mediatek/mt8196/mt8196-afe-pcm.c
+++ b/sound/soc/mediatek/mt8196/mt8196-afe-pcm.c
@@ -90,9 +90,20 @@ static int mt8196_set_cm(struct mtk_base_afe *afe,
int id,
struct mt8196_afe_private *afe_priv = afe->platform_priv;
unsigned int rate = afe_priv->cm_rate[id];
unsigned int rate_val = mt8196_rate_transform(afe->dev, rate);
- unsigned int update_val = update ? ((((26000000 / rate) - 10) /
(ch / 2)) - 1) : 0x64;
+ unsigned int ch_pair = ch / 2;
+ unsigned int update_val;
int reg = AFE_CM0_CON0 + 0x10 * id;

+ if (update) {
+ if (ch_pair == 0) {
+ dev_err(afe->dev, "CM%d: invalid channel count
%u\n", id, ch);
+ return -EINVAL;
+ }
+ update_val = (26000000 / rate - 10) / ch_pair - 1;
+ } else {
+ update_val = 0x64;
+ }
+
dev_dbg(afe->dev, "CM%d, rate %d, update %d, swap %d, ch %d\n",
id, rate, update, swap, ch);

@@ -471,6 +482,7 @@ static int ul_cm0_event(struct snd_soc_dapm_widget
*w,
struct mtk_base_afe *afe =
snd_soc_component_get_drvdata(cmpnt);
struct mt8196_afe_private *afe_priv = afe->platform_priv;
unsigned int channels = afe_priv->cm_channels;
+ int ret;

dev_dbg(afe->dev, "event 0x%x, name %s, channels %u\n",
event, w->name, channels);
@@ -478,7 +490,9 @@ static int ul_cm0_event(struct snd_soc_dapm_widget
*w,
switch (event) {
case SND_SOC_DAPM_PRE_PMU:
mt8196_enable_cm_bypass(afe, CM0, false);
- mt8196_set_cm(afe, CM0, true, false, channels);
+ ret = mt8196_set_cm(afe, CM0, true, false, channels);
+ if (ret)
+ return ret;
regmap_update_bits(afe->regmap, AUDIO_TOP_CON0,
PDN_CM0_MASK_SFT, 0 << PDN_CM0_SFT);
break;
@@ -502,6 +516,7 @@ static int ul_cm1_event(struct snd_soc_dapm_widget
*w,
struct mtk_base_afe *afe =
snd_soc_component_get_drvdata(cmpnt);
struct mt8196_afe_private *afe_priv = afe->platform_priv;
unsigned int channels = afe_priv->cm_channels;
+ int ret;

dev_dbg(afe->dev, "event 0x%x, name %s, channels %u\n",
event, w->name, channels);
@@ -509,7 +524,9 @@ static int ul_cm1_event(struct snd_soc_dapm_widget
*w,
switch (event) {
case SND_SOC_DAPM_PRE_PMU:
mt8196_enable_cm_bypass(afe, CM1, false);
- mt8196_set_cm(afe, CM1, true, false, channels);
+ ret = mt8196_set_cm(afe, CM1, true, false, channels);
+ if (ret)
+ return ret;
regmap_update_bits(afe->regmap, AUDIO_TOP_CON0,
PDN_CM1_MASK_SFT, 0 << PDN_CM1_SFT);
break;
@@ -533,6 +550,7 @@ static int ul_cm2_event(struct snd_soc_dapm_widget
*w,
struct mtk_base_afe *afe =
snd_soc_component_get_drvdata(cmpnt);
struct mt8196_afe_private *afe_priv = afe->platform_priv;
unsigned int channels = afe_priv->cm_channels;
+ int ret;

dev_dbg(afe->dev, "event 0x%x, name %s, channels %u\n",
event, w->name, channels);
@@ -540,7 +558,9 @@ static int ul_cm2_event(struct snd_soc_dapm_widget
*w,
switch (event) {
case SND_SOC_DAPM_PRE_PMU:
mt8196_enable_cm_bypass(afe, CM2, false);
- mt8196_set_cm(afe, CM2, true, false, channels);
+ ret = mt8196_set_cm(afe, CM2, true, false, channels);
+ if (ret)
+ return ret;
regmap_update_bits(afe->regmap, AUDIO_TOP_CON0,
PDN_CM2_MASK_SFT, 0 << PDN_CM2_SFT);
break;

Best Regards
Cyril Chao

On Fri, 2026-04-03 at 15:07 +0100, Mark Brown wrote:
> On Tue, Mar 24, 2026 at 09:56:49AM +0800, Cyril Chao wrote:
>
> > +static int mt8196_set_cm(struct mtk_base_afe *afe, int id,
> > + bool update, bool swap, unsigned int ch)
> > +{
> > + struct mt8196_afe_private *afe_priv = afe->platform_priv;
> > + unsigned int rate = afe_priv->cm_rate[id];
> > + unsigned int rate_val = mt8196_rate_transform(afe->dev, rate);
> > + unsigned int update_val = update ? ((((26000000 / rate) - 10) /
> > (ch / 2)) - 1) : 0x64;
> > + int reg = AFE_CM0_CON0 + 0x10 * id;
>
> The driver looks like it supports mono so won't this trigger divide
> by
> zero?
>
> Also please write normal conditional statements, it's much more
> leigible.