| lists.openwall.net | lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC | |
|
Open Source and information security mailing list archives
| ||
|
Message-ID: <5ab4375d-f08b-4d55-b056-6f7b034b1607@collabora.com>
Date: Wed, 19 Feb 2025 12:29:15 +0100
From: AngeloGioacchino Del Regno <angelogioacchino.delregno@...labora.com>
To: Nícolas F. R. A. Prado <nfraprado@...labora.com>,
Liam Girdwood <lgirdwood@...il.com>, Mark Brown <broonie@...nel.org>,
Jaroslav Kysela <perex@...ex.cz>, Takashi Iwai <tiwai@...e.com>,
Matthias Brugger <matthias.bgg@...il.com>, Rob Herring <robh@...nel.org>,
Krzysztof Kozlowski <krzk+dt@...nel.org>, Conor Dooley <conor+dt@...nel.org>
Cc: kernel@...labora.com, linux-sound@...r.kernel.org,
linux-kernel@...r.kernel.org, linux-arm-kernel@...ts.infradead.org,
linux-mediatek@...ts.infradead.org, devicetree@...r.kernel.org,
parkeryang <Parker.Yang@...iatek.com>
Subject: Re: [PATCH 3/6] ASoC: mediatek: mt8188: Add DMIC DAI driver
Il 18/02/25 21:52, Nícolas F. R. A. Prado ha scritto:
> From: parkeryang <Parker.Yang@...iatek.com>
>
> Add support for the DMIC DAIs present on the MT8188 SoC.
>
> Signed-off-by: parkeryang <Parker.Yang@...iatek.com>
> Co-developed-by: Nícolas F. R. A. Prado <nfraprado@...labora.com>
> Signed-off-by: Nícolas F. R. A. Prado <nfraprado@...labora.com>
> ---
> sound/soc/mediatek/mt8188/mt8188-afe-pcm.c | 4 -
> sound/soc/mediatek/mt8188/mt8188-dai-dmic.c | 754 ++++++++++++++++++++++++++++
> sound/soc/mediatek/mt8188/mt8188-reg.h | 18 +-
> 3 files changed, 770 insertions(+), 6 deletions(-)
>
> diff --git a/sound/soc/mediatek/mt8188/mt8188-afe-pcm.c b/sound/soc/mediatek/mt8188/mt8188-afe-pcm.c
> index 73e5c63aeec8783905d656af225c42cd95069049..d36520c6272dd8c8302bc3f59da33f82f273f366 100644
> --- a/sound/soc/mediatek/mt8188/mt8188-afe-pcm.c
> +++ b/sound/soc/mediatek/mt8188/mt8188-afe-pcm.c
> @@ -2855,10 +2855,6 @@ static bool mt8188_is_volatile_reg(struct device *dev, unsigned int reg)
> case AFE_DMIC3_SRC_DEBUG_MON0:
> case AFE_DMIC3_UL_SRC_MON0:
> case AFE_DMIC3_UL_SRC_MON1:
> - case DMIC_GAIN1_CUR:
> - case DMIC_GAIN2_CUR:
> - case DMIC_GAIN3_CUR:
> - case DMIC_GAIN4_CUR:
Why are you removing those?!
If you really have to, that goes into a different commit.
> case ETDM_IN1_MONITOR:
> case ETDM_IN2_MONITOR:
> case ETDM_OUT1_MONITOR:
> diff --git a/sound/soc/mediatek/mt8188/mt8188-dai-dmic.c b/sound/soc/mediatek/mt8188/mt8188-dai-dmic.c
> new file mode 100644
> index 0000000000000000000000000000000000000000..9df08783ff80173095809c20538d0fb073ed7fae
> --- /dev/null
> +++ b/sound/soc/mediatek/mt8188/mt8188-dai-dmic.c
> @@ -0,0 +1,754 @@
> +// SPDX-License-Identifier: GPL-2.0
> +/*
> + * MediaTek ALSA SoC Audio DAI DMIC I/F Control
> + *
> + * Copyright (c) 2020 MediaTek Inc.
> + * Author: Bicycle Tsai <bicycle.tsai@...iatek.com>
> + * Trevor Wu <trevor.wu@...iatek.com>
> + * Parker Yang <parker.yang@...iatek.com>
> + */
> +
..snip..
> +
> +static void mtk_dai_dmic_hw_gain_on(struct mtk_base_afe *afe,
> + unsigned int id, bool on)
> +{
> + const struct mtk_dai_dmic_hw_gain_ctrl_reg *reg;
> + unsigned int msk = DMIC_GAIN_CON0_GAIN_ON;
msk never changes here, just use that definition directly in the regmap calls
below.... and also, reg can be NULL, but you're not checking that, which may
result in a kernel panic.
So this function should look like....
static void mtk_dai_dmic_hw_gain_on(struct mtk_base_afe *afe,
unsigned int id, bool on)
{
const struct mtk_dai_dmic_hw_gain_ctrl_reg *reg = get_dmic_hw_gain_ctrl_reg(afe, id);
if (!reg)
return;
if (on)
regmap_set_bits(afe->regmap, reg->con0, DMIC_GAIN_CON0_GAIN_ON);
else
regmap_clear_bits( ....)
};
> +
> + reg = get_dmic_hw_gain_ctrl_reg(afe, id);
> + if (!reg)
> + return;
> +
> + if (on)
> + regmap_set_bits(afe->regmap, reg->con0, msk);
> + else
> + regmap_clear_bits(afe->regmap, reg->con0, msk);
> +}
> +
..snip..
> +
> +static int mtk_dai_dmic_configure_array(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_dmic_priv *dmic_priv = NULL;
> + unsigned int mask =
> + PWR2_TOP_CON_DMIC8_SRC_SEL_MASK |
> + PWR2_TOP_CON_DMIC7_SRC_SEL_MASK |
> + PWR2_TOP_CON_DMIC6_SRC_SEL_MASK |
> + PWR2_TOP_CON_DMIC5_SRC_SEL_MASK |
> + PWR2_TOP_CON_DMIC4_SRC_SEL_MASK |
> + PWR2_TOP_CON_DMIC3_SRC_SEL_MASK |
> + PWR2_TOP_CON_DMIC2_SRC_SEL_MASK |
> + PWR2_TOP_CON_DMIC1_SRC_SEL_MASK;
const u32 mask = ....
> + unsigned int val = 0;
const u32 val = PWR2_TOP_CON ......
> +
> + if (dai->id < 0)
> + return -EINVAL;
> +
> + dmic_priv = afe_priv->dai_priv[dai->id];
> + val = PWR2_TOP_CON_DMIC8_SRC_SEL_VAL(DMIC8_SRC_SEL) |
> + PWR2_TOP_CON_DMIC7_SRC_SEL_VAL(DMIC7_SRC_SEL) |
> + PWR2_TOP_CON_DMIC6_SRC_SEL_VAL(DMIC6_SRC_SEL) |
> + PWR2_TOP_CON_DMIC5_SRC_SEL_VAL(DMIC5_SRC_SEL) |
> + PWR2_TOP_CON_DMIC4_SRC_SEL_VAL(DMIC4_SRC_SEL) |
> + PWR2_TOP_CON_DMIC3_SRC_SEL_VAL(DMIC3_SRC_SEL) |
> + PWR2_TOP_CON_DMIC2_SRC_SEL_VAL(DMIC2_SRC_SEL) |
> + PWR2_TOP_CON_DMIC1_SRC_SEL_VAL(DMIC1_SRC_SEL);
> +
> + regmap_update_bits(afe->regmap, PWR2_TOP_CON0, mask, val);
> +
> + return 0;
> +}
> +
> +static void mtk_dai_dmic_hw_gain_enable(struct mtk_base_afe *afe,
> + unsigned int channels, bool enable)
> +{
> + struct mt8188_afe_private *afe_priv = afe->platform_priv;
> + struct mtk_dai_dmic_priv *dmic_priv;
> + unsigned int end;
> + unsigned int id;
> +
> + if (!channels)
> + return;
> +
> + dmic_priv = afe_priv->dai_priv[MT8188_AFE_IO_DMIC_IN];
> + if (channels > 3)
> + end = 3;
else
end = channels - 1;
way shorter :-)
> + else if (channels > 2)
> + end = 2;
> + else if (channels > 1)
> + end = 1;
> + else if (channels > 0)
> + end = 0;
> +
> + for (id = DMIC0; id <= end; id++) {
> + if (enable && dmic_priv->gain_on[id]) {
> + mtk_dai_dmic_hw_gain_byass(afe, id, false);
by...what? LOL! :-)
That typo is hilarious, but still fix it.
> + mtk_dai_dmic_hw_gain_on(afe, id, true);
> + } else {
> + mtk_dai_dmic_hw_gain_on(afe, id, false);
> + mtk_dai_dmic_hw_gain_byass(afe, id, true);
> + }
> + }
> +}
> +
> +static int mtk_dmic_gain_event(struct snd_soc_dapm_widget *w,
> + struct snd_kcontrol *kcontrol,
> + int event)
> +{
> + struct snd_soc_component *cmpnt = snd_soc_dapm_to_component(w->dapm);
> + struct mtk_base_afe *afe = snd_soc_component_get_drvdata(cmpnt);
> + struct mt8188_afe_private *afe_priv = afe->platform_priv;
> + struct mtk_dai_dmic_priv *dmic_priv = NULL;
> + unsigned int channels;
> +
> + dev_dbg(afe->dev, "%s(), name %s, event 0x%x\n",
> + __func__, w->name, event);
> +
> + dmic_priv = afe_priv->dai_priv[MT8188_AFE_IO_DMIC_IN];
> + channels = dmic_priv->channels;
Either you check if afe_priv is NULL before assigning, or just stack-assign these.
> +
> + switch (event) {
> + case SND_SOC_DAPM_PRE_PMU:
> + mtk_dai_dmic_hw_gain_enable(afe, channels, true);
> + break;
> + case SND_SOC_DAPM_POST_PMD:
> + mtk_dai_dmic_hw_gain_enable(afe, channels, false);
> + break;
> + default:
> + break;
> + }
> +
> + return 0;
> +}
> +
> +static int mtk_dmic_event(struct snd_soc_dapm_widget *w,
> + struct snd_kcontrol *kcontrol,
> + int event)
> +{
> + struct snd_soc_component *cmpnt = snd_soc_dapm_to_component(w->dapm);
> + struct mtk_base_afe *afe = snd_soc_component_get_drvdata(cmpnt);
> + struct mt8188_afe_private *afe_priv = afe->platform_priv;
> + struct mtk_dai_dmic_priv *dmic_priv = NULL;
> + const struct mtk_dai_dmic_ctrl_reg *reg = NULL;
> + unsigned int channels;
> + unsigned int msk;
u8 dmic_num;
> +
> + dev_dbg(afe->dev, "%s(), name %s, event 0x%x\n",
> + __func__, w->name, event);
> +
> + dmic_priv = afe_priv->dai_priv[MT8188_AFE_IO_DMIC_IN];
> + channels = dmic_priv->channels;
> +
if (channels == 0) {
dev_dbg(something);
return -EINVAL;
}
/* This function assumes that the caller checked that channels is valid */
static u8 mtk_dmic_channels_to_dmic_number_or_another_name_you_choose(unsigned int
channels)
{
switch (channels) {
case 1:
return DMIC0;
case 2:
return DMIC1;
case 3:
return DMIC2;
case 4:
default:
return DMIC3;
}
}
dmic_num = mtk_dmic_channels_to_dmic_number_or_another_name_you_choose(channels);
> + switch (event) {
> + case SND_SOC_DAPM_PRE_PMU:
> + /* request fifo soft rst */
> + msk = 0;
for (i = dmic_num; i > 0; i--)
msk |= PWR2_TOP_CON1_DMIC_FIFO_SOFT_RST_EN(i);
> + if (channels > 3)
> + msk |= PWR2_TOP_CON1_DMIC_FIFO_SOFT_RST_EN(DMIC3);
> + if (channels > 2)
> + msk |= PWR2_TOP_CON1_DMIC_FIFO_SOFT_RST_EN(DMIC2);
> + if (channels > 1)
> + msk |= PWR2_TOP_CON1_DMIC_FIFO_SOFT_RST_EN(DMIC1);
> + if (channels > 0)
> + msk |= PWR2_TOP_CON1_DMIC_FIFO_SOFT_RST_EN(DMIC0);
> +
> + regmap_set_bits(afe->regmap, PWR2_TOP_CON1, msk);
> +
> + msk = 0;
msk = AFE_DMIC_UL_SRC_CON0_UL_MODE_3P25M_CH1_CTL |
AFE_DMIC_UL ....
> + msk |= AFE_DMIC_UL_SRC_CON0_UL_MODE_3P25M_CH1_CTL;
> + msk |= AFE_DMIC_UL_SRC_CON0_UL_MODE_3P25M_CH2_CTL;
> + msk |= AFE_DMIC_UL_SRC_CON0_UL_SDM_3_LEVEL_CTL;
> + msk |= AFE_DMIC_UL_SRC_CON0_UL_IIR_ON_TMP_CTL;
> +
reg = get_dmic_ctrl_reg(dmic_num);
if (reg)
regmap_set_bits .....
> + if (channels > 3) {
> + reg = get_dmic_ctrl_reg(DMIC3);
> + if (reg)
> + regmap_set_bits(afe->regmap, reg->con0, msk);
> + }
> + if (channels > 2) {
> + reg = get_dmic_ctrl_reg(DMIC2);
> + if (reg)
> + regmap_set_bits(afe->regmap, reg->con0, msk);
> + }
> + if (channels > 1) {
> + reg = get_dmic_ctrl_reg(DMIC1);
> + if (reg)
> + regmap_set_bits(afe->regmap, reg->con0, msk);
> + }
> + if (channels > 0) {
> + reg = get_dmic_ctrl_reg(DMIC0);
> + if (reg)
> + regmap_set_bits(afe->regmap, reg->con0, msk);
> + }
> + break;
> + case SND_SOC_DAPM_POST_PMU:
> + msk = AFE_DMIC_UL_SRC_CON0_UL_SRC_ON_TMP_CTL;
same comments apply here.
> + if (channels > 3) {
> + reg = get_dmic_ctrl_reg(DMIC3);
> + if (reg)
> + regmap_set_bits(afe->regmap, reg->con0, msk);
> + }
> + if (channels > 2) {
> + reg = get_dmic_ctrl_reg(DMIC2);
> + if (reg)
> + regmap_set_bits(afe->regmap, reg->con0, msk);
> + }
> + if (channels > 1) {
> + reg = get_dmic_ctrl_reg(DMIC1);
> + if (reg)
> + regmap_set_bits(afe->regmap, reg->con0, msk);
> + }
> + if (channels > 0) {
> + reg = get_dmic_ctrl_reg(DMIC0);
> + if (reg)
> + regmap_set_bits(afe->regmap, reg->con0, msk);
> + }
> +
> + if (dmic_priv->hires_required) {
> + mt8188_afe_enable_clk(afe, afe_priv->clk[MT8188_CLK_AUD_DMIC_HIRES1]);
> + mt8188_afe_enable_clk(afe, afe_priv->clk[MT8188_CLK_AUD_DMIC_HIRES2]);
> + mt8188_afe_enable_clk(afe, afe_priv->clk[MT8188_CLK_AUD_DMIC_HIRES3]);
> + mt8188_afe_enable_clk(afe, afe_priv->clk[MT8188_CLK_AUD_DMIC_HIRES4]);
> + }
> +
> + mt8188_afe_enable_clk(afe, afe_priv->clk[MT8188_CLK_AUD_AFE_DMIC1]);
> + mt8188_afe_enable_clk(afe, afe_priv->clk[MT8188_CLK_AUD_AFE_DMIC2]);
> + mt8188_afe_enable_clk(afe, afe_priv->clk[MT8188_CLK_AUD_AFE_DMIC3]);
> + mt8188_afe_enable_clk(afe, afe_priv->clk[MT8188_CLK_AUD_AFE_DMIC4]);
> +
> + /* release fifo soft rst */
> + msk = 0;
for (i = dmic_num; i .... same as before
msk |= MACRO(i)
> + if (channels > 3)
> + msk |= PWR2_TOP_CON1_DMIC_FIFO_SOFT_RST_EN(DMIC3);
> + if (channels > 2)
> + msk |= PWR2_TOP_CON1_DMIC_FIFO_SOFT_RST_EN(DMIC2);
> + if (channels > 1)
> + msk |= PWR2_TOP_CON1_DMIC_FIFO_SOFT_RST_EN(DMIC1);
> + if (channels > 0)
> + msk |= PWR2_TOP_CON1_DMIC_FIFO_SOFT_RST_EN(DMIC0);
> +
> + regmap_clear_bits(afe->regmap, PWR2_TOP_CON1, msk);
> + break;
> + case SND_SOC_DAPM_PRE_PMD:
> + msk = AFE_DMIC_UL_SRC_CON0_UL_MODE_3P25M_CH1_CTL |
> + AFE_DMIC_UL_SRC_CON0_UL_MODE_3P25M_CH2_CTL |
> + AFE_DMIC_UL_SRC_CON0_UL_SRC_ON_TMP_CTL |
> + AFE_DMIC_UL_SRC_CON0_UL_IIR_ON_TMP_CTL |
> + AFE_DMIC_UL_SRC_CON0_UL_SDM_3_LEVEL_CTL;
> +
same fixes apply here.
> + if (channels > 3) {
> + reg = get_dmic_ctrl_reg(DMIC3);
> + if (reg)
> + regmap_clear_bits(afe->regmap, reg->con0, msk);
> + }
> + if (channels > 2) {
> + reg = get_dmic_ctrl_reg(DMIC2);
> + if (reg)
> + regmap_clear_bits(afe->regmap, reg->con0, msk);
> + }
> + if (channels > 1) {
> + reg = get_dmic_ctrl_reg(DMIC1);
> + if (reg)
> + regmap_clear_bits(afe->regmap, reg->con0, msk);
> + }
> + if (channels > 0) {
> + reg = get_dmic_ctrl_reg(DMIC0);
> + if (reg)
> + regmap_clear_bits(afe->regmap, reg->con0, msk);
> + }
> + break;
> + case SND_SOC_DAPM_POST_PMD:
> + /* should delayed 1/fs(smallest is 8k) = 125us before afe off */
> + usleep_range(125, 126);
> +
> + mt8188_afe_disable_clk(afe, afe_priv->clk[MT8188_CLK_AUD_AFE_DMIC1]);
> + mt8188_afe_disable_clk(afe, afe_priv->clk[MT8188_CLK_AUD_AFE_DMIC2]);
> + mt8188_afe_disable_clk(afe, afe_priv->clk[MT8188_CLK_AUD_AFE_DMIC3]);
> + mt8188_afe_disable_clk(afe, afe_priv->clk[MT8188_CLK_AUD_AFE_DMIC4]);
> +
> + if (dmic_priv->hires_required) {
> + mt8188_afe_disable_clk(afe, afe_priv->clk[MT8188_CLK_AUD_DMIC_HIRES1]);
> + mt8188_afe_disable_clk(afe, afe_priv->clk[MT8188_CLK_AUD_DMIC_HIRES2]);
> + mt8188_afe_disable_clk(afe, afe_priv->clk[MT8188_CLK_AUD_DMIC_HIRES3]);
> + mt8188_afe_disable_clk(afe, afe_priv->clk[MT8188_CLK_AUD_DMIC_HIRES4]);
> + }
> + break;
> + default:
> + break;
> + }
> +
> + return 0;
> +}
> +
> +static int mtk_dai_dmic_hw_params(struct snd_pcm_substream *substream,
> + struct snd_pcm_hw_params *params,
> + 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_dmic_priv *dmic_priv = NULL;
> + unsigned int rate = params_rate(params);
> + unsigned int channels = params_channels(params);
> + const struct mtk_dai_dmic_ctrl_reg *reg = NULL;
> + unsigned int val = 0;
> + unsigned int msk = 0;
> +
> + if (dai->id < 0)
> + return -EINVAL;
> +
> + dmic_priv = afe_priv->dai_priv[dai->id];
> +
> + val = AFE_DMIC_UL_SRC_CON0_UL_PHASE_SEL_CH1(CLK_PHASE_SEL_CH1) |
> + AFE_DMIC_UL_SRC_CON0_UL_PHASE_SEL_CH2(CLK_PHASE_SEL_CH2) |
> + AFE_DMIC_UL_SRC_CON0_UL_IIR_MODE_CTL(0);
> + msk = AFE_DMIC_UL_SRC_CON0_UL_TWO_WIRE_MODE_CTL_MASK |
> + AFE_DMIC_UL_SRC_CON0_UL_PHASE_SEL_MASK |
> + AFE_DMIC_UL_SRC_CON0_UL_IIR_MODE_CTL_MASK |
> + AFE_DMIC_UL_VOICE_MODE_MASK;
double assignment of both val and msk. fix it.
> +
> + mtk_dai_dmic_configure_array(dai);
> + dmic_priv->hires_required = 0;
repeat that assignment in all rate cases, don't do double assignments.
> +
> + switch (rate) {
> + case 96000:
> + val |= AFE_DMIC_UL_CON0_VOCIE_MODE_96K;
> + dmic_priv->hires_required = 1;
> + break;
> + case 48000:
> + val |= AFE_DMIC_UL_CON0_VOCIE_MODE_48K;
> + break;
> + case 32000:
> + val |= AFE_DMIC_UL_CON0_VOCIE_MODE_32K;
> + break;
> + case 16000:
> + val |= AFE_DMIC_UL_CON0_VOCIE_MODE_16K;
> + break;
> + case 8000:
> + val |= AFE_DMIC_UL_CON0_VOCIE_MODE_8K;
> + break;
> + default:
> + dev_dbg(afe->dev, "%s invalid rate %u, use 48000Hz\n", __func__, rate);
> + val |= AFE_DMIC_UL_CON0_VOCIE_MODE_48K;
> + break;
> + }
> +
> + mtk_dai_dmic_load_iir_coeff_table(afe);
> +
> + if (channels > 3) {
simplify that with the same comments as before... or actually you could even
commonize that pattern, as it seems to be duplicated over and over again.
> + reg = get_dmic_ctrl_reg(DMIC3);
> + if (reg)
> + regmap_update_bits(afe->regmap, reg->con0, msk, val);
> + }
> + if (channels > 2) {
> + reg = get_dmic_ctrl_reg(DMIC2);
> + if (reg)
> + regmap_update_bits(afe->regmap, reg->con0, msk, val);
> + }
> + if (channels > 1) {
> + reg = get_dmic_ctrl_reg(DMIC1);
> + if (reg)
> + regmap_update_bits(afe->regmap, reg->con0, msk, val);
> + }
> + if (channels > 0) {
> + reg = get_dmic_ctrl_reg(DMIC0);
> + if (reg)
> + regmap_update_bits(afe->regmap, reg->con0, msk, val);
> + }
> +
> + dmic_priv->channels = channels;
> +
> + return 0;
> +}
> +
> +static const struct snd_soc_dai_ops mtk_dai_dmic_ops = {
> + .hw_params = mtk_dai_dmic_hw_params,
> +};
> +
> +#define MTK_DMIC_RATES (SNDRV_PCM_RATE_8000 |\
> + SNDRV_PCM_RATE_16000 |\
> + SNDRV_PCM_RATE_32000 |\
> + SNDRV_PCM_RATE_48000 |\
> + SNDRV_PCM_RATE_96000)
> +
> +#define MTK_DMIC_FORMATS (SNDRV_PCM_FMTBIT_S16_LE |\
> + SNDRV_PCM_FMTBIT_S32_LE)
> +
> +static struct snd_soc_dai_driver mtk_dai_dmic_driver[] = {
> + {
> + .name = "DMIC",
> + .id = MT8188_AFE_IO_DMIC_IN,
> + .capture = {
> + .stream_name = "DMIC Capture",
> + .channels_min = 1,
> + .channels_max = 8,
> + .rates = MTK_DMIC_RATES,
> + .formats = MTK_DMIC_FORMATS,
> + },
> + .ops = &mtk_dai_dmic_ops,
> + },
> +};
> +
..snip..
> +
> +static int mtk_dai_dmic_hw_gain_ctrl_put(struct snd_kcontrol *kcontrol,
> + struct snd_ctl_elem_value *ucontrol)
> +{
> + struct snd_soc_component *component = snd_kcontrol_chip(kcontrol);
> + struct soc_enum *e = (struct soc_enum *)kcontrol->private_value;
> + struct mtk_base_afe *afe = snd_soc_component_get_drvdata(component);
> + struct mt8188_afe_private *afe_priv = afe->platform_priv;
> + struct mtk_dai_dmic_priv *dmic_priv;
> + unsigned int source = ucontrol->value.enumerated.item[0];
> + unsigned int *cached = 0;
Why are you initializing *cached?
There's no way it can be used uninitialized in this function, as if anything is
wrong you're either returning -EINVAL or assigning `source` to it later.
> +
> + if (source >= e->items)
> + return -EINVAL;
> +
> + dmic_priv = afe_priv->dai_priv[MT8188_AFE_IO_DMIC_IN];
> +
> + if (!strcmp(kcontrol->id.name, "DMIC1_HW_GAIN_EN"))
> + cached = &dmic_priv->gain_on[0];
> + else if (!strcmp(kcontrol->id.name, "DMIC2_HW_GAIN_EN"))
> + cached = &dmic_priv->gain_on[1];
> + else if (!strcmp(kcontrol->id.name, "DMIC3_HW_GAIN_EN"))
> + cached = &dmic_priv->gain_on[2];
> + else if (!strcmp(kcontrol->id.name, "DMIC4_HW_GAIN_EN"))
> + cached = &dmic_priv->gain_on[3];
> + else
> + return -EINVAL;
> +
> + if (source == *cached)
> + return 0;
> +
> + *cached = source;
> + return 1;
> +}
> +
> +static int mtk_dai_dmic_hw_gain_ctrl_get(struct snd_kcontrol *kcontrol,
> + struct snd_ctl_elem_value *ucontrol)
> +{
> + struct snd_soc_component *component =
> + snd_soc_kcontrol_component(kcontrol);
> + struct mtk_base_afe *afe = snd_soc_component_get_drvdata(component);
> + struct mt8188_afe_private *afe_priv = afe->platform_priv;
> + struct mtk_dai_dmic_priv *dmic_priv;
> + unsigned int val = 0;
double assignment again
> +
> + dmic_priv = afe_priv->dai_priv[MT8188_AFE_IO_DMIC_IN];
stack initialize dmic_priv instead.
> +
> + if (!strcmp(kcontrol->id.name, "DMIC1_HW_GAIN_EN"))
> + val = dmic_priv->gain_on[0];
> + else if (!strcmp(kcontrol->id.name, "DMIC2_HW_GAIN_EN"))
> + val = dmic_priv->gain_on[1];
> + else if (!strcmp(kcontrol->id.name, "DMIC3_HW_GAIN_EN"))
> + val = dmic_priv->gain_on[2];
> + else if (!strcmp(kcontrol->id.name, "DMIC4_HW_GAIN_EN"))
> + val = dmic_priv->gain_on[3];
else
val = 0;
> +
> + ucontrol->value.enumerated.item[0] = val;
> + return 0;
> +}
> +
..snip..
> diff --git a/sound/soc/mediatek/mt8188/mt8188-reg.h b/sound/soc/mediatek/mt8188/mt8188-reg.h
> index bdd885419ff3874bab80549ea3ff4617172b8245..4154548aca36108667036f9889476c30f9b6171a 100644
> --- a/sound/soc/mediatek/mt8188/mt8188-reg.h
> +++ b/sound/soc/mediatek/mt8188/mt8188-reg.h
> @@ -2837,9 +2837,20 @@
> #define PWR2_TOP_CON_DMIC3_SRC_SEL_MASK GENMASK(16, 14)
> #define PWR2_TOP_CON_DMIC2_SRC_SEL_MASK GENMASK(13, 11)
> #define PWR2_TOP_CON_DMIC1_SRC_SEL_MASK GENMASK(10, 8)
> +#define PWR2_TOP_CON_DMIC8_SRC_SEL_VAL(x) ((x) << 29)
> +#define PWR2_TOP_CON_DMIC7_SRC_SEL_VAL(x) ((x) << 26)
> +#define PWR2_TOP_CON_DMIC6_SRC_SEL_VAL(x) ((x) << 23)
> +#define PWR2_TOP_CON_DMIC5_SRC_SEL_VAL(x) ((x) << 20)
> +#define PWR2_TOP_CON_DMIC4_SRC_SEL_VAL(x) ((x) << 17)
> +#define PWR2_TOP_CON_DMIC3_SRC_SEL_VAL(x) ((x) << 14)
> +#define PWR2_TOP_CON_DMIC2_SRC_SEL_VAL(x) ((x) << 11)
> +#define PWR2_TOP_CON_DMIC1_SRC_SEL_VAL(x) ((x) << 8)
>
> /* PWR2_TOP_CON1 */
> -#define PWR2_TOP_CON1_DMIC_CKDIV_ON BIT(1)
> +#define PWR2_TOP_CON1_DMIC_FIFO_SOFT_RST_EN(x) BIT((5 + 6 * x))
> +#define PWR2_TOP_CON1_DMIC_CKDIV_ON BIT(1)
> +#define PWR2_TOP_CON1_DMIC_CKDIV_ON_SHIFT 1
> +
>
> /* PCM_INTF_CON1 */
> #define PCM_INTF_CON1_SYNC_OUT_INV BIT(23)
> @@ -2919,15 +2930,17 @@
> #define AFE_DMIC_UL_SRC_CON0_UL_PHASE_SEL_CH2(x) (((x) & 0x7) << 24)
> #define AFE_DMIC_UL_SRC_CON0_UL_PHASE_SEL_MASK GENMASK(29, 24)
> #define AFE_DMIC_UL_SRC_CON0_UL_TWO_WIRE_MODE_CTL BIT(23)
> +#define AFE_DMIC_UL_SRC_CON0_UL_TWO_WIRE_MODE_CTL_MASK BIT(23)
That's the same as AFE_DMIC_UL_SRC_CON0_UL_TWO_WIRE_MODE_CTL, drop it.
> #define AFE_DMIC_UL_SRC_CON0_UL_MODE_3P25M_CH2_CTL BIT(22)
> #define AFE_DMIC_UL_SRC_CON0_UL_MODE_3P25M_CH1_CTL BIT(21)
> -
> +#define AFE_DMIC_UL_VOICE_MODE(x) (((x) & 0x7) << 17)
(((x) & GENMASK(2, 0) << 17)
> #define AFE_DMIC_UL_VOICE_MODE_MASK GENMASK(19, 17)
> #define AFE_DMIC_UL_CON0_VOCIE_MODE_8K AFE_DMIC_UL_VOICE_MODE(0)
> #define AFE_DMIC_UL_CON0_VOCIE_MODE_16K AFE_DMIC_UL_VOICE_MODE(1)
> #define AFE_DMIC_UL_CON0_VOCIE_MODE_32K AFE_DMIC_UL_VOICE_MODE(2)
> #define AFE_DMIC_UL_CON0_VOCIE_MODE_48K AFE_DMIC_UL_VOICE_MODE(3)
> #define AFE_DMIC_UL_CON0_VOCIE_MODE_96K AFE_DMIC_UL_VOICE_MODE(4)
> +#define AFE_DMIC_UL_SRC_CON0_UL_IIR_MODE_CTL(x) (((x) & 0x7) << 7)
same here
> #define AFE_DMIC_UL_SRC_CON0_UL_IIR_MODE_CTL_MASK GENMASK(9, 7)
> #define AFE_DMIC_UL_SRC_CON0_UL_IIR_ON_TMP_CTL BIT(10)
> #define AFE_DMIC_UL_SRC_CON0_UL_SDM_3_LEVEL_CTL BIT(1)
> @@ -2944,6 +2957,7 @@
>
> /* DMIC_GAINx_CON0 */
> #define DMIC_GAIN_CON0_GAIN_ON BIT(0)
> +#define DMIC_GAIN_CON0_SAMPLE_PER_STEP_SHIFT 8
> #define DMIC_GAIN_CON0_SAMPLE_PER_STEP_MASK GENMASK(15, 8)
>
> /* DMIC_GAINx_CON1 */
>
Cheers,
Angelo
Powered by blists - more mailing lists