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 <[email protected]>
---
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,
--
Mark Brown <[email protected]>
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 <[email protected]>
---
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;
--
2.30.2
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 <[email protected]>
---
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[] = {
--
2.30.2
Reporting an error on invalid values is optional but helpful to userspace
so do so.
Signed-off-by: Mark Brown <[email protected]>
---
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;
--
2.30.2
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 <[email protected]>
---
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),
--
2.30.2
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 <[email protected]>
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 <[email protected]>
Thanks,
N?colas
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 <[email protected]>
Reviewed-by: N?colas F. R. A. Prado <[email protected]>
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.
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 <[email protected]>
Reviewed-by: N?colas F. R. A. Prado <[email protected]>
Thanks,
N?colas
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 <[email protected]>
Reviewed-by: N?colas F. R. A. Prado <[email protected]>
Thanks,
N?colas
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 <[email protected]>
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 <[email protected]>
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 <[email protected]>
Reviewed-by: AngeloGioacchino Del Regno <[email protected]>
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 <[email protected]>
Reviewed-by: AngeloGioacchino Del Regno <[email protected]>
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 <[email protected]>
Reviewed-by: AngeloGioacchino Del Regno <[email protected]>
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 <[email protected]>
Reviewed-by: AngeloGioacchino Del Regno <[email protected]>
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