[PATCH 0/4] ASoC: mt8192: Fixes from initial glance at kselftest run
This is a collection of fixes I came up after glancing through an initial test run with the Spherion Chromebook on KernelCI. There are more issues flagged, this is just what I fixed thus far - the volume controls on the MT6359 have issues for example, and a lot of controls aren't marked as Switches like they should be.
Signed-off-by: Mark Brown broonie@kernel.org --- Mark Brown (4): ASoC: mt8192: Remove spammy log messages ASoC: mt8192: Fix event generation for controls ASoC: mt8192: Report an error if when an invalid sidetone gain is written ASoC: mt8192: Fix range for sidetone positive gain
sound/soc/mediatek/mt8192/mt8192-dai-adda.c | 58 ++++++++++------------------- 1 file changed, 19 insertions(+), 39 deletions(-) --- base-commit: b361d5d2464a88184f6e17a6462719ba79180b1a change-id: 20230223-asoc-mt8192-quick-fixes-8f3e0a187226
Best regards,
There are a lot of info level log messages in the mt8192 ADDA driver which are trivially triggerable from userspace, many in normal operation. Remove these to avoid spamming the console.
Signed-off-by: Mark Brown broonie@kernel.org --- sound/soc/mediatek/mt8192/mt8192-dai-adda.c | 33 ----------------------------- 1 file changed, 33 deletions(-)
diff --git a/sound/soc/mediatek/mt8192/mt8192-dai-adda.c b/sound/soc/mediatek/mt8192/mt8192-dai-adda.c index f8c73e8624df..bc8753f1001c 100644 --- a/sound/soc/mediatek/mt8192/mt8192-dai-adda.c +++ b/sound/soc/mediatek/mt8192/mt8192-dai-adda.c @@ -303,9 +303,6 @@ static int mtk_adda_ul_event(struct snd_soc_dapm_widget *w, struct mt8192_afe_private *afe_priv = afe->platform_priv; int mtkaif_dmic = afe_priv->mtkaif_dmic;
- dev_info(afe->dev, "%s(), name %s, event 0x%x, mtkaif_dmic %d\n", - __func__, w->name, event, mtkaif_dmic); - switch (event) { case SND_SOC_DAPM_PRE_PMU: mt8192_afe_gpio_request(afe->dev, true, MT8192_DAI_ADDA, 1); @@ -345,10 +342,6 @@ static int mtk_adda_ch34_ul_event(struct snd_soc_dapm_widget *w, int mtkaif_dmic = afe_priv->mtkaif_dmic_ch34; int mtkaif_adda6_only = afe_priv->mtkaif_adda6_only;
- dev_info(afe->dev, - "%s(), name %s, event 0x%x, mtkaif_dmic %d, mtkaif_adda6_only %d\n", - __func__, w->name, event, mtkaif_dmic, mtkaif_adda6_only); - switch (event) { case SND_SOC_DAPM_PRE_PMU: mt8192_afe_gpio_request(afe->dev, true, MT8192_DAI_ADDA_CH34, @@ -538,9 +531,6 @@ static int mtk_adda_dl_event(struct snd_soc_dapm_widget *w, struct snd_soc_component *cmpnt = snd_soc_dapm_to_component(w->dapm); struct mtk_base_afe *afe = snd_soc_component_get_drvdata(cmpnt);
- dev_info(afe->dev, "%s(), name %s, event 0x%x\n", - __func__, w->name, event); - switch (event) { case SND_SOC_DAPM_PRE_PMU: mt8192_afe_gpio_request(afe->dev, true, MT8192_DAI_ADDA, 0); @@ -564,9 +554,6 @@ static int mtk_adda_ch34_dl_event(struct snd_soc_dapm_widget *w, struct snd_soc_component *cmpnt = snd_soc_dapm_to_component(w->dapm); struct mtk_base_afe *afe = snd_soc_component_get_drvdata(cmpnt);
- dev_info(afe->dev, "%s(), name %s, event 0x%x\n", - __func__, w->name, event); - switch (event) { case SND_SOC_DAPM_PRE_PMU: mt8192_afe_gpio_request(afe->dev, true, MT8192_DAI_ADDA_CH34, @@ -612,9 +599,6 @@ static int stf_positive_gain_set(struct snd_kcontrol *kcontrol, AFE_SIDETONE_GAIN, POSITIVE_GAIN_MASK_SFT, (gain_db / 6) << POSITIVE_GAIN_SFT); - } else { - dev_warn(afe->dev, "%s(), gain_db %d invalid\n", - __func__, gain_db); } return 0; } @@ -640,9 +624,6 @@ static int mt8192_adda_dmic_set(struct snd_kcontrol *kcontrol,
dmic_on = ucontrol->value.integer.value[0];
- dev_info(afe->dev, "%s(), kcontrol name %s, dmic_on %d\n", - __func__, kcontrol->id.name, dmic_on); - afe_priv->mtkaif_dmic = dmic_on; afe_priv->mtkaif_dmic_ch34 = dmic_on; return 0; @@ -669,9 +650,6 @@ static int mt8192_adda6_only_set(struct snd_kcontrol *kcontrol,
mtkaif_adda6_only = ucontrol->value.integer.value[0];
- dev_info(afe->dev, "%s(), kcontrol name %s, mtkaif_adda6_only %d\n", - __func__, kcontrol->id.name, mtkaif_adda6_only); - afe_priv->mtkaif_adda6_only = mtkaif_adda6_only; return 0; } @@ -750,9 +728,6 @@ static int mtk_stf_event(struct snd_soc_dapm_widget *w,
regmap_read(afe->regmap, AFE_SIDETONE_CON1, ®_value);
- dev_info(afe->dev, "%s(), name %s, event 0x%x, ul_rate 0x%x, AFE_SIDETONE_CON1 0x%x\n", - __func__, w->name, event, ul_rate, reg_value); - switch (event) { case SND_SOC_DAPM_PRE_PMU: /* set side tone gain = 0 */ @@ -1163,12 +1138,6 @@ static int mtk_dai_adda_hw_params(struct snd_pcm_substream *substream, unsigned int rate = params_rate(params); int id = dai->id;
- dev_info(afe->dev, "%s(), id %d, stream %d, rate %d\n", - __func__, - id, - substream->stream, - rate); - if (substream->stream == SNDRV_PCM_STREAM_PLAYBACK) { unsigned int dl_src2_con0 = 0; unsigned int dl_src2_con1 = 0; @@ -1441,8 +1410,6 @@ int mt8192_dai_adda_register(struct mtk_base_afe *afe) struct mtk_base_afe_dai *dai; struct mt8192_afe_private *afe_priv = afe->platform_priv;
- dev_info(afe->dev, "%s()\n", __func__); - dai = devm_kzalloc(afe->dev, sizeof(*dai), GFP_KERNEL); if (!dai) return -ENOMEM;
On Fri, Feb 24, 2023 at 02:03:55PM +0000, Mark Brown wrote:
There are a lot of info level log messages in the mt8192 ADDA driver which are trivially triggerable from userspace, many in normal operation. Remove these to avoid spamming the console.
Signed-off-by: Mark Brown broonie@kernel.org
Probably the spammiest messages are
[ 33.881459] mt8192-audio 11210000.syscon:mt8192-afe-pcm: mt8192_afe_runtime_resume() [ 33.889320] mt8192-audio 11210000.syscon:mt8192-afe-pcm: mt8192_afe_enable_clock() [ 34.029456] mt8192-audio 11210000.syscon:mt8192-afe-pcm: mt8192_afe_runtime_suspend() [ 34.041718] mt8192-audio 11210000.syscon:mt8192-afe-pcm: mt8192_afe_disable_clock()
from mt8192-afe-pcm.c, so I think it would make sense to get rid of those in this commit as well.
Way less spammy, but there are also
[ 176.209790] mt8192-audio 11210000.syscon:mt8192-afe-pcm: mtk_dai_tdm_hw_params(), id 43, rate 48000, channels 2, format 6, mclk_rate 24576000, bck_rate 3072000 [ 176.224149] mt8192-audio 11210000.syscon:mt8192-afe-pcm: mtk_dai_tdm_hw_params(), out_channels_per_sdata = 2 [ 180.272153] mt8192-audio 11210000.syscon:mt8192-afe-pcm: mtk_tdm_en_event(), name TDM_EN, event 0x8 [ 180.281462] mt8192-audio 11210000.syscon:mt8192-afe-pcm: mtk_tdm_bck_en_event(), name TDM_BCK, event 0x8, dai_id 43
from mt8192-dai-tdm.c.
Anyway, if you prefer to keep only changes in the ADDA driver for this commit that's fine too, and I can send another patch removing these other ones later.
Either way,
Reviewed-by: Nícolas F. R. A. Prado nfraprado@collabora.com
Thanks, Nícolas
On Fri, Feb 24, 2023 at 02:06:57PM -0500, Nícolas F. R. A. Prado wrote:
Probably the spammiest messages are
[ 33.881459] mt8192-audio 11210000.syscon:mt8192-afe-pcm: mt8192_afe_runtime_resume() [ 33.889320] mt8192-audio 11210000.syscon:mt8192-afe-pcm: mt8192_afe_enable_clock() [ 34.029456] mt8192-audio 11210000.syscon:mt8192-afe-pcm: mt8192_afe_runtime_suspend() [ 34.041718] mt8192-audio 11210000.syscon:mt8192-afe-pcm: mt8192_afe_disable_clock()
from mt8192-afe-pcm.c, so I think it would make sense to get rid of those in this commit as well.
They should definitely go as well, I don't know that there's a specific need for it to be this commit though - it was mostly just what I noticed while looking at the controls that were failing tests.
Way less spammy, but there are also
[ 176.209790] mt8192-audio 11210000.syscon:mt8192-afe-pcm: mtk_dai_tdm_hw_params(), id 43, rate 48000, channels 2, format 6, mclk_rate 24576000, bck_rate 3072000 [ 176.224149] mt8192-audio 11210000.syscon:mt8192-afe-pcm: mtk_dai_tdm_hw_params(), out_channels_per_sdata = 2 [ 180.272153] mt8192-audio 11210000.syscon:mt8192-afe-pcm: mtk_tdm_en_event(), name TDM_EN, event 0x8 [ 180.281462] mt8192-audio 11210000.syscon:mt8192-afe-pcm: mtk_tdm_bck_en_event(), name TDM_BCK, event 0x8, dai_id 43
from mt8192-dai-tdm.c.
Yup.
Anyway, if you prefer to keep only changes in the ADDA driver for this commit that's fine too, and I can send another patch removing these other ones later.
That'd be great, I will probably take more passes at this stuff at some point but it's very much a background thing.
Il 24/02/23 15:03, Mark Brown ha scritto:
There are a lot of info level log messages in the mt8192 ADDA driver which are trivially triggerable from userspace, many in normal operation. Remove these to avoid spamming the console.
Signed-off-by: Mark Brown broonie@kernel.org
Reviewed-by: AngeloGioacchino Del Regno angelogioacchino.delregno@collabora.com
ALSA controls put() operations should return 1 if the value changed and 0 if it remains the same, fix the mt8192 driver to do so.
Signed-off-by: Mark Brown broonie@kernel.org --- sound/soc/mediatek/mt8192/mt8192-dai-adda.c | 25 ++++++++++++++++++------- 1 file changed, 18 insertions(+), 7 deletions(-)
diff --git a/sound/soc/mediatek/mt8192/mt8192-dai-adda.c b/sound/soc/mediatek/mt8192/mt8192-dai-adda.c index bc8753f1001c..a33d1ce33349 100644 --- a/sound/soc/mediatek/mt8192/mt8192-dai-adda.c +++ b/sound/soc/mediatek/mt8192/mt8192-dai-adda.c @@ -591,16 +591,19 @@ static int stf_positive_gain_set(struct snd_kcontrol *kcontrol, struct mtk_base_afe *afe = snd_soc_component_get_drvdata(cmpnt); struct mt8192_afe_private *afe_priv = afe->platform_priv; int gain_db = ucontrol->value.integer.value[0]; + bool change = false;
afe_priv->stf_positive_gain_db = gain_db;
if (gain_db >= 0 && gain_db <= 24) { - regmap_update_bits(afe->regmap, - AFE_SIDETONE_GAIN, - POSITIVE_GAIN_MASK_SFT, - (gain_db / 6) << POSITIVE_GAIN_SFT); + regmap_update_bits_check(afe->regmap, + AFE_SIDETONE_GAIN, + POSITIVE_GAIN_MASK_SFT, + (gain_db / 6) << POSITIVE_GAIN_SFT, + &change); } - return 0; + + return change; }
static int mt8192_adda_dmic_get(struct snd_kcontrol *kcontrol, @@ -621,12 +624,17 @@ static int mt8192_adda_dmic_set(struct snd_kcontrol *kcontrol, struct mtk_base_afe *afe = snd_soc_component_get_drvdata(cmpnt); struct mt8192_afe_private *afe_priv = afe->platform_priv; int dmic_on; + bool change;
dmic_on = ucontrol->value.integer.value[0];
+ change = (afe_priv->mtkaif_dmic != dmic_on) || + (afe_priv->mtkaif_dmic_ch34 != dmic_on); + afe_priv->mtkaif_dmic = dmic_on; afe_priv->mtkaif_dmic_ch34 = dmic_on; - return 0; + + return change; }
static int mt8192_adda6_only_get(struct snd_kcontrol *kcontrol, @@ -647,11 +655,14 @@ static int mt8192_adda6_only_set(struct snd_kcontrol *kcontrol, struct mtk_base_afe *afe = snd_soc_component_get_drvdata(cmpnt); struct mt8192_afe_private *afe_priv = afe->platform_priv; int mtkaif_adda6_only; + bool change;
mtkaif_adda6_only = ucontrol->value.integer.value[0];
+ change = afe_priv->mtkaif_adda6_only != mtkaif_adda6_only; afe_priv->mtkaif_adda6_only = mtkaif_adda6_only; - return 0; + + return change; }
static const struct snd_kcontrol_new mtk_adda_controls[] = {
On Fri, Feb 24, 2023 at 02:03:56PM +0000, Mark Brown wrote:
ALSA controls put() operations should return 1 if the value changed and 0 if it remains the same, fix the mt8192 driver to do so.
Signed-off-by: Mark Brown broonie@kernel.org
Reviewed-by: Nícolas F. R. A. Prado nfraprado@collabora.com
Thanks, Nícolas
Il 24/02/23 15:03, Mark Brown ha scritto:
ALSA controls put() operations should return 1 if the value changed and 0 if it remains the same, fix the mt8192 driver to do so.
Signed-off-by: Mark Brown broonie@kernel.org
Reviewed-by: AngeloGioacchino Del Regno angelogioacchino.delregno@collabora.com
Reporting an error on invalid values is optional but helpful to userspace so do so.
Signed-off-by: Mark Brown broonie@kernel.org --- sound/soc/mediatek/mt8192/mt8192-dai-adda.c | 2 ++ 1 file changed, 2 insertions(+)
diff --git a/sound/soc/mediatek/mt8192/mt8192-dai-adda.c b/sound/soc/mediatek/mt8192/mt8192-dai-adda.c index a33d1ce33349..a02a297c0450 100644 --- a/sound/soc/mediatek/mt8192/mt8192-dai-adda.c +++ b/sound/soc/mediatek/mt8192/mt8192-dai-adda.c @@ -601,6 +601,8 @@ static int stf_positive_gain_set(struct snd_kcontrol *kcontrol, POSITIVE_GAIN_MASK_SFT, (gain_db / 6) << POSITIVE_GAIN_SFT, &change); + } else { + return -EINVAL; }
return change;
On Fri, Feb 24, 2023 at 02:03:57PM +0000, Mark Brown wrote:
Reporting an error on invalid values is optional but helpful to userspace so do so.
Signed-off-by: Mark Brown broonie@kernel.org
Reviewed-by: Nícolas F. R. A. Prado nfraprado@collabora.com
Thanks, Nícolas
Il 24/02/23 15:03, Mark Brown ha scritto:
Reporting an error on invalid values is optional but helpful to userspace so do so.
Signed-off-by: Mark Brown broonie@kernel.org
Reviewed-by: AngeloGioacchino Del Regno angelogioacchino.delregno@collabora.com
The Sidetone_Positive_Gain_dB control reports a range of 0..100 as valid but the put() function rejects anything larger than 24. Fix this.
There are numerous other problems with this control, the name is very non idiomatic and it should be a TLV, but it's ABI so probably we should leave those alone.
Signed-off-by: Mark Brown broonie@kernel.org --- sound/soc/mediatek/mt8192/mt8192-dai-adda.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/sound/soc/mediatek/mt8192/mt8192-dai-adda.c b/sound/soc/mediatek/mt8192/mt8192-dai-adda.c index a02a297c0450..4919535e2759 100644 --- a/sound/soc/mediatek/mt8192/mt8192-dai-adda.c +++ b/sound/soc/mediatek/mt8192/mt8192-dai-adda.c @@ -670,7 +670,7 @@ static int mt8192_adda6_only_set(struct snd_kcontrol *kcontrol, static const struct snd_kcontrol_new mtk_adda_controls[] = { SOC_SINGLE("Sidetone_Gain", AFE_SIDETONE_GAIN, SIDE_TONE_GAIN_SFT, SIDE_TONE_GAIN_MASK, 0), - SOC_SINGLE_EXT("Sidetone_Positive_Gain_dB", SND_SOC_NOPM, 0, 100, 0, + SOC_SINGLE_EXT("Sidetone_Positive_Gain_dB", SND_SOC_NOPM, 0, 24, 0, stf_positive_gain_get, stf_positive_gain_set), SOC_SINGLE("ADDA_DL_GAIN", AFE_ADDA_DL_SRC2_CON1, DL_2_GAIN_CTL_PRE_SFT, DL_2_GAIN_CTL_PRE_MASK, 0),
On Fri, Feb 24, 2023 at 02:03:58PM +0000, Mark Brown wrote:
The Sidetone_Positive_Gain_dB control reports a range of 0..100 as valid but the put() function rejects anything larger than 24. Fix this.
There are numerous other problems with this control, the name is very non idiomatic and it should be a TLV, but it's ABI so probably we should leave those alone.
Signed-off-by: Mark Brown broonie@kernel.org
Reviewed-by: Nícolas F. R. A. Prado nfraprado@collabora.com
Thanks, Nícolas
Il 24/02/23 15:03, Mark Brown ha scritto:
The Sidetone_Positive_Gain_dB control reports a range of 0..100 as valid but the put() function rejects anything larger than 24. Fix this.
There are numerous other problems with this control, the name is very non idiomatic and it should be a TLV, but it's ABI so probably we should leave those alone.
Signed-off-by: Mark Brown broonie@kernel.org
Reviewed-by: AngeloGioacchino Del Regno angelogioacchino.delregno@collabora.com
On Fri, Feb 24, 2023 at 02:03:54PM +0000, Mark Brown wrote:
This is a collection of fixes I came up after glancing through an initial test run with the Spherion Chromebook on KernelCI. There are more issues flagged, this is just what I fixed thus far - the volume controls on the MT6359 have issues for example, and a lot of controls aren't marked as Switches like they should be.
Signed-off-by: Mark Brown broonie@kernel.org
Thanks for the fixes.
Tested on a spherion chromebook to make sure audio still works as expected, and the following tests from mixer-test in the alsa kselftest are fixed:
# No event generated for MTKAIF_ADDA6_ONLY Switch not ok 1910 event_missing.0.13
# No event generated for MTKAIF_DMIC Switch not ok 1917 event_missing.0.12
# No event generated for Sidetone_Positive_Gain_dB # Sidetone_Positive_Gain_dB.0 value -1 less than minimum 0 # Sidetone_Positive_Gain_dB.0 value 101 more than maximum 100 not ok 1930 write_invalid.0.10
(no change in pcm-test results)
So,
Tested-by: Nícolas F. R. A. Prado nfraprado@collabora.com
Thanks, Nícolas
On Fri, 24 Feb 2023 14:03:54 +0000, Mark Brown wrote:
This is a collection of fixes I came up after glancing through an initial test run with the Spherion Chromebook on KernelCI. There are more issues flagged, this is just what I fixed thus far - the volume controls on the MT6359 have issues for example, and a lot of controls aren't marked as Switches like they should be.
[...]
Applied to
https://git.kernel.org/pub/scm/linux/kernel/git/broonie/sound.git for-next
Thanks!
[1/4] ASoC: mt8192: Remove spammy log messages commit: 5df1a5d28449f2b98ff84f69dea74b06f9b8e362 [2/4] ASoC: mt8192: Fix event generation for controls commit: b373076f609993d333dbbc3283b65320c7a41834 [3/4] ASoC: mt8192: Report an error if when an invalid sidetone gain is written commit: 05437a91173b8780692ac35313f98cac68be7c42 [4/4] ASoC: mt8192: Fix range for sidetone positive gain commit: ce40d93b062c0bdcd29218c12ab1dba544382dd8
All being well this means that it will be integrated into the linux-next tree (usually sometime in the next 24 hours) and sent to Linus during the next merge window (or sooner if it is a bug fix), however if problems are discovered then the patch may be dropped or reverted.
You may get further e-mails resulting from automated or manual testing and review of the tree, please engage with people reporting problems and send followup patches addressing any issues that are reported if needed.
If any updates are required or you are submitting further changes they should be sent as incremental updates against current git, existing patches will not be replaced.
Please add any relevant lists and maintainers to the CCs when replying to this mail.
Thanks, Mark
participants (3)
-
AngeloGioacchino Del Regno -
Mark Brown -
Nícolas F. R. A. Prado