* [PATCH 0/3] ALSA: hda/tas2781: fixes for 6.9-rc1
@ 2024-03-25 21:25 Gergo Koteles
2024-03-25 21:25 ` [PATCH 1/3] ALSA: hda/tas2781: remove digital gain kcontrol Gergo Koteles
` (2 more replies)
0 siblings, 3 replies; 10+ messages in thread
From: Gergo Koteles @ 2024-03-25 21:25 UTC (permalink / raw)
To: Shenghao Ding, Kevin Lu, Baojun Xu, Jaroslav Kysela, Takashi Iwai
Cc: alsa-devel, linux-sound, linux-kernel, Gergo Koteles
Hi,
This series removes the no longer needed digital gain kcontrol, which
caused problems in tas2563, because the register does not exists there.
This series also adds locking and debug statements to the other
kcontrols. They sometimes ran in parallel with tasdev_fw_ready, and
caused weird sound problems.
https://github.com/tomsom/yoga-linux/issues/58
Regards,
Gergo
Gergo Koteles (3):
ALSA: hda/tas2781: remove digital gain kcontrol
ALSA: hda/tas2781: add locks to kcontrols
ALSA: hda/tas2781: add debug statements to kcontrols
include/sound/tas2781-tlv.h | 1 -
sound/pci/hda/tas2781_hda_i2c.c | 103 +++++++++++++++++++++-----------
2 files changed, 68 insertions(+), 36 deletions(-)
base-commit: 4cece764965020c22cff7665b18a012006359095
--
2.44.0
^ permalink raw reply [flat|nested] 10+ messages in thread
* [PATCH 1/3] ALSA: hda/tas2781: remove digital gain kcontrol
2024-03-25 21:25 [PATCH 0/3] ALSA: hda/tas2781: fixes for 6.9-rc1 Gergo Koteles
@ 2024-03-25 21:25 ` Gergo Koteles
2024-03-26 12:58 ` kernel test robot
2024-03-27 4:21 ` kernel test robot
2024-03-25 21:25 ` [PATCH 2/3] ALSA: hda/tas2781: add locks to kcontrols Gergo Koteles
2024-03-25 21:25 ` [PATCH 3/3] ALSA: hda/tas2781: add debug statements " Gergo Koteles
2 siblings, 2 replies; 10+ messages in thread
From: Gergo Koteles @ 2024-03-25 21:25 UTC (permalink / raw)
To: Shenghao Ding, Kevin Lu, Baojun Xu, Jaroslav Kysela, Takashi Iwai
Cc: alsa-devel, linux-sound, linux-kernel, Gergo Koteles, stable
The "Speaker Digital Gain" kcontrol controls the TAS2781_DVC_LVL (0x1A)
register. Unfortunately the tas2563 does not have DVC_LVL, but has
INT_MASK0 in 0x1A, which has been misused so far.
Since commit c1947ce61ff4 ("ALSA: hda/realtek: tas2781: enable subwoofer
volume control") the volume of the tas2781 amplifiers can be controlled
by the master volume, so this digital gain kcontrol is not needed.
Remove it.
Fixes: 5be27f1e3ec9 ("ALSA: hda/tas2781: Add tas2781 HDA driver")
CC: stable@vger.kernel.org
Signed-off-by: Gergo Koteles <soyer@irl.hu>
---
include/sound/tas2781-tlv.h | 1 -
sound/pci/hda/tas2781_hda_i2c.c | 37 +--------------------------------
2 files changed, 1 insertion(+), 37 deletions(-)
diff --git a/include/sound/tas2781-tlv.h b/include/sound/tas2781-tlv.h
index 4038dd421150..a29bcfb5bb2b 100644
--- a/include/sound/tas2781-tlv.h
+++ b/include/sound/tas2781-tlv.h
@@ -15,7 +15,6 @@
#ifndef __TAS2781_TLV_H__
#define __TAS2781_TLV_H__
-static const DECLARE_TLV_DB_SCALE(dvc_tlv, -10000, 100, 0);
static const DECLARE_TLV_DB_SCALE(amp_vol_tlv, 1100, 50, 0);
#endif
diff --git a/sound/pci/hda/tas2781_hda_i2c.c b/sound/pci/hda/tas2781_hda_i2c.c
index 4475cea8e9f7..5acb475c10a7 100644
--- a/sound/pci/hda/tas2781_hda_i2c.c
+++ b/sound/pci/hda/tas2781_hda_i2c.c
@@ -89,7 +89,7 @@ struct tas2781_hda {
struct snd_kcontrol *dsp_prog_ctl;
struct snd_kcontrol *dsp_conf_ctl;
struct snd_kcontrol *prof_ctl;
- struct snd_kcontrol *snd_ctls[3];
+ struct snd_kcontrol *snd_ctls[2];
};
static int tas2781_get_i2c_res(struct acpi_resource *ares, void *data)
@@ -294,27 +294,6 @@ static int tasdevice_config_put(struct snd_kcontrol *kcontrol,
return ret;
}
-/*
- * tas2781_digital_getvol - get the volum control
- * @kcontrol: control pointer
- * @ucontrol: User data
- * Customer Kcontrol for tas2781 is primarily for regmap booking, paging
- * depends on internal regmap mechanism.
- * tas2781 contains book and page two-level register map, especially
- * book switching will set the register BXXP00R7F, after switching to the
- * correct book, then leverage the mechanism for paging to access the
- * register.
- */
-static int tas2781_digital_getvol(struct snd_kcontrol *kcontrol,
- struct snd_ctl_elem_value *ucontrol)
-{
- struct tasdevice_priv *tas_priv = snd_kcontrol_chip(kcontrol);
- struct soc_mixer_control *mc =
- (struct soc_mixer_control *)kcontrol->private_value;
-
- return tasdevice_digital_getvol(tas_priv, ucontrol, mc);
-}
-
static int tas2781_amp_getvol(struct snd_kcontrol *kcontrol,
struct snd_ctl_elem_value *ucontrol)
{
@@ -325,17 +304,6 @@ static int tas2781_amp_getvol(struct snd_kcontrol *kcontrol,
return tasdevice_amp_getvol(tas_priv, ucontrol, mc);
}
-static int tas2781_digital_putvol(struct snd_kcontrol *kcontrol,
- struct snd_ctl_elem_value *ucontrol)
-{
- struct tasdevice_priv *tas_priv = snd_kcontrol_chip(kcontrol);
- struct soc_mixer_control *mc =
- (struct soc_mixer_control *)kcontrol->private_value;
-
- /* The check of the given value is in tasdevice_digital_putvol. */
- return tasdevice_digital_putvol(tas_priv, ucontrol, mc);
-}
-
static int tas2781_amp_putvol(struct snd_kcontrol *kcontrol,
struct snd_ctl_elem_value *ucontrol)
{
@@ -381,9 +349,6 @@ static const struct snd_kcontrol_new tas2781_snd_controls[] = {
ACARD_SINGLE_RANGE_EXT_TLV("Speaker Analog Gain", TAS2781_AMP_LEVEL,
1, 0, 20, 0, tas2781_amp_getvol,
tas2781_amp_putvol, amp_vol_tlv),
- ACARD_SINGLE_RANGE_EXT_TLV("Speaker Digital Gain", TAS2781_DVC_LVL,
- 0, 0, 200, 1, tas2781_digital_getvol,
- tas2781_digital_putvol, dvc_tlv),
ACARD_SINGLE_BOOL_EXT("Speaker Force Firmware Load", 0,
tas2781_force_fwload_get, tas2781_force_fwload_put),
};
--
2.44.0
^ permalink raw reply related [flat|nested] 10+ messages in thread
* [PATCH 2/3] ALSA: hda/tas2781: add locks to kcontrols
2024-03-25 21:25 [PATCH 0/3] ALSA: hda/tas2781: fixes for 6.9-rc1 Gergo Koteles
2024-03-25 21:25 ` [PATCH 1/3] ALSA: hda/tas2781: remove digital gain kcontrol Gergo Koteles
@ 2024-03-25 21:25 ` Gergo Koteles
2024-03-25 21:25 ` [PATCH 3/3] ALSA: hda/tas2781: add debug statements " Gergo Koteles
2 siblings, 0 replies; 10+ messages in thread
From: Gergo Koteles @ 2024-03-25 21:25 UTC (permalink / raw)
To: Shenghao Ding, Kevin Lu, Baojun Xu, Jaroslav Kysela, Takashi Iwai
Cc: alsa-devel, linux-sound, linux-kernel, Gergo Koteles, stable
The rcabin.profile_cfg_id, cur_prog, cur_conf, force_fwload_status
variables are acccessible from multiple threads and therefore require
locking.
Fixes: 5be27f1e3ec9 ("ALSA: hda/tas2781: Add tas2781 HDA driver")
CC: stable@vger.kernel.org
Signed-off-by: Gergo Koteles <soyer@irl.hu>
---
sound/pci/hda/tas2781_hda_i2c.c | 50 +++++++++++++++++++++++++++++++--
1 file changed, 48 insertions(+), 2 deletions(-)
diff --git a/sound/pci/hda/tas2781_hda_i2c.c b/sound/pci/hda/tas2781_hda_i2c.c
index 5acb475c10a7..9a43f563bb9e 100644
--- a/sound/pci/hda/tas2781_hda_i2c.c
+++ b/sound/pci/hda/tas2781_hda_i2c.c
@@ -185,8 +185,12 @@ static int tasdevice_get_profile_id(struct snd_kcontrol *kcontrol,
{
struct tasdevice_priv *tas_priv = snd_kcontrol_chip(kcontrol);
+ mutex_lock(&tas_priv->codec_lock);
+
ucontrol->value.integer.value[0] = tas_priv->rcabin.profile_cfg_id;
+ mutex_unlock(&tas_priv->codec_lock);
+
return 0;
}
@@ -200,11 +204,15 @@ static int tasdevice_set_profile_id(struct snd_kcontrol *kcontrol,
val = clamp(nr_profile, 0, max);
+ mutex_lock(&tas_priv->codec_lock);
+
if (tas_priv->rcabin.profile_cfg_id != val) {
tas_priv->rcabin.profile_cfg_id = val;
ret = 1;
}
+ mutex_unlock(&tas_priv->codec_lock);
+
return ret;
}
@@ -241,8 +249,12 @@ static int tasdevice_program_get(struct snd_kcontrol *kcontrol,
{
struct tasdevice_priv *tas_priv = snd_kcontrol_chip(kcontrol);
+ mutex_lock(&tas_priv->codec_lock);
+
ucontrol->value.integer.value[0] = tas_priv->cur_prog;
+ mutex_unlock(&tas_priv->codec_lock);
+
return 0;
}
@@ -257,11 +269,15 @@ static int tasdevice_program_put(struct snd_kcontrol *kcontrol,
val = clamp(nr_program, 0, max);
+ mutex_lock(&tas_priv->codec_lock);
+
if (tas_priv->cur_prog != val) {
tas_priv->cur_prog = val;
ret = 1;
}
+ mutex_unlock(&tas_priv->codec_lock);
+
return ret;
}
@@ -270,8 +286,12 @@ static int tasdevice_config_get(struct snd_kcontrol *kcontrol,
{
struct tasdevice_priv *tas_priv = snd_kcontrol_chip(kcontrol);
+ mutex_lock(&tas_priv->codec_lock);
+
ucontrol->value.integer.value[0] = tas_priv->cur_conf;
+ mutex_unlock(&tas_priv->codec_lock);
+
return 0;
}
@@ -286,11 +306,15 @@ static int tasdevice_config_put(struct snd_kcontrol *kcontrol,
val = clamp(nr_config, 0, max);
+ mutex_lock(&tas_priv->codec_lock);
+
if (tas_priv->cur_conf != val) {
tas_priv->cur_conf = val;
ret = 1;
}
+ mutex_unlock(&tas_priv->codec_lock);
+
return ret;
}
@@ -300,8 +324,15 @@ static int tas2781_amp_getvol(struct snd_kcontrol *kcontrol,
struct tasdevice_priv *tas_priv = snd_kcontrol_chip(kcontrol);
struct soc_mixer_control *mc =
(struct soc_mixer_control *)kcontrol->private_value;
+ int ret;
+
+ mutex_lock(&tas_priv->codec_lock);
+
+ ret = tasdevice_amp_getvol(tas_priv, ucontrol, mc);
+
+ mutex_unlock(&tas_priv->codec_lock);
- return tasdevice_amp_getvol(tas_priv, ucontrol, mc);
+ return ret;
}
static int tas2781_amp_putvol(struct snd_kcontrol *kcontrol,
@@ -310,9 +341,16 @@ static int tas2781_amp_putvol(struct snd_kcontrol *kcontrol,
struct tasdevice_priv *tas_priv = snd_kcontrol_chip(kcontrol);
struct soc_mixer_control *mc =
(struct soc_mixer_control *)kcontrol->private_value;
+ int ret;
+
+ mutex_lock(&tas_priv->codec_lock);
/* The check of the given value is in tasdevice_amp_putvol. */
- return tasdevice_amp_putvol(tas_priv, ucontrol, mc);
+ ret = tasdevice_amp_putvol(tas_priv, ucontrol, mc);
+
+ mutex_unlock(&tas_priv->codec_lock);
+
+ return ret;
}
static int tas2781_force_fwload_get(struct snd_kcontrol *kcontrol,
@@ -320,10 +358,14 @@ static int tas2781_force_fwload_get(struct snd_kcontrol *kcontrol,
{
struct tasdevice_priv *tas_priv = snd_kcontrol_chip(kcontrol);
+ mutex_lock(&tas_priv->codec_lock);
+
ucontrol->value.integer.value[0] = (int)tas_priv->force_fwload_status;
dev_dbg(tas_priv->dev, "%s : Force FWload %s\n", __func__,
tas_priv->force_fwload_status ? "ON" : "OFF");
+ mutex_unlock(&tas_priv->codec_lock);
+
return 0;
}
@@ -333,6 +375,8 @@ static int tas2781_force_fwload_put(struct snd_kcontrol *kcontrol,
struct tasdevice_priv *tas_priv = snd_kcontrol_chip(kcontrol);
bool change, val = (bool)ucontrol->value.integer.value[0];
+ mutex_lock(&tas_priv->codec_lock);
+
if (tas_priv->force_fwload_status == val)
change = false;
else {
@@ -342,6 +386,8 @@ static int tas2781_force_fwload_put(struct snd_kcontrol *kcontrol,
dev_dbg(tas_priv->dev, "%s : Force FWload %s\n", __func__,
tas_priv->force_fwload_status ? "ON" : "OFF");
+ mutex_unlock(&tas_priv->codec_lock);
+
return change;
}
--
2.44.0
^ permalink raw reply related [flat|nested] 10+ messages in thread
* [PATCH 3/3] ALSA: hda/tas2781: add debug statements to kcontrols
2024-03-25 21:25 [PATCH 0/3] ALSA: hda/tas2781: fixes for 6.9-rc1 Gergo Koteles
2024-03-25 21:25 ` [PATCH 1/3] ALSA: hda/tas2781: remove digital gain kcontrol Gergo Koteles
2024-03-25 21:25 ` [PATCH 2/3] ALSA: hda/tas2781: add locks to kcontrols Gergo Koteles
@ 2024-03-25 21:25 ` Gergo Koteles
2024-03-25 22:01 ` Pierre-Louis Bossart
2 siblings, 1 reply; 10+ messages in thread
From: Gergo Koteles @ 2024-03-25 21:25 UTC (permalink / raw)
To: Shenghao Ding, Kevin Lu, Baojun Xu, Jaroslav Kysela, Takashi Iwai
Cc: alsa-devel, linux-sound, linux-kernel, Gergo Koteles
Sometimes it is useful to examine the timing of kcontrol events.
Add debug statements to each kcontrol.
Signed-off-by: Gergo Koteles <soyer@irl.hu>
---
sound/pci/hda/tas2781_hda_i2c.c | 22 ++++++++++++++++++++++
1 file changed, 22 insertions(+)
diff --git a/sound/pci/hda/tas2781_hda_i2c.c b/sound/pci/hda/tas2781_hda_i2c.c
index 9a43f563bb9e..b60ce4c2c090 100644
--- a/sound/pci/hda/tas2781_hda_i2c.c
+++ b/sound/pci/hda/tas2781_hda_i2c.c
@@ -189,6 +189,9 @@ static int tasdevice_get_profile_id(struct snd_kcontrol *kcontrol,
ucontrol->value.integer.value[0] = tas_priv->rcabin.profile_cfg_id;
+ dev_dbg(tas_priv->dev, "%s: %d\n", __func__,
+ tas_priv->rcabin.profile_cfg_id);
+
mutex_unlock(&tas_priv->codec_lock);
return 0;
@@ -206,6 +209,9 @@ static int tasdevice_set_profile_id(struct snd_kcontrol *kcontrol,
mutex_lock(&tas_priv->codec_lock);
+ dev_dbg(tas_priv->dev, "%s: %d -> %d\n", __func__,
+ tas_priv->rcabin.profile_cfg_id, val);
+
if (tas_priv->rcabin.profile_cfg_id != val) {
tas_priv->rcabin.profile_cfg_id = val;
ret = 1;
@@ -253,6 +259,8 @@ static int tasdevice_program_get(struct snd_kcontrol *kcontrol,
ucontrol->value.integer.value[0] = tas_priv->cur_prog;
+ dev_dbg(tas_priv->dev, "%s: %d\n", __func__, tas_priv->cur_prog);
+
mutex_unlock(&tas_priv->codec_lock);
return 0;
@@ -271,6 +279,9 @@ static int tasdevice_program_put(struct snd_kcontrol *kcontrol,
mutex_lock(&tas_priv->codec_lock);
+ dev_dbg(tas_priv->dev, "%s: %d -> %d\n", __func__,
+ tas_priv->cur_prog, val);
+
if (tas_priv->cur_prog != val) {
tas_priv->cur_prog = val;
ret = 1;
@@ -290,6 +301,8 @@ static int tasdevice_config_get(struct snd_kcontrol *kcontrol,
ucontrol->value.integer.value[0] = tas_priv->cur_conf;
+ dev_dbg(tas_priv->dev, "%s: %d\n", __func__, tas_priv->cur_conf);
+
mutex_unlock(&tas_priv->codec_lock);
return 0;
@@ -308,6 +321,9 @@ static int tasdevice_config_put(struct snd_kcontrol *kcontrol,
mutex_lock(&tas_priv->codec_lock);
+ dev_dbg(tas_priv->dev, "%s: %d -> %d\n", __func__,
+ tas_priv->cur_conf, val);
+
if (tas_priv->cur_conf != val) {
tas_priv->cur_conf = val;
ret = 1;
@@ -330,6 +346,9 @@ static int tas2781_amp_getvol(struct snd_kcontrol *kcontrol,
ret = tasdevice_amp_getvol(tas_priv, ucontrol, mc);
+ dev_dbg(tas_priv->dev, "%s: %ld\n", __func__,
+ ucontrol->value.integer.value[0]);
+
mutex_unlock(&tas_priv->codec_lock);
return ret;
@@ -345,6 +364,9 @@ static int tas2781_amp_putvol(struct snd_kcontrol *kcontrol,
mutex_lock(&tas_priv->codec_lock);
+ dev_dbg(tas_priv->dev, "%s: %ld\n", __func__,
+ ucontrol->value.integer.value[0]);
+
/* The check of the given value is in tasdevice_amp_putvol. */
ret = tasdevice_amp_putvol(tas_priv, ucontrol, mc);
--
2.44.0
^ permalink raw reply related [flat|nested] 10+ messages in thread
* Re: [PATCH 3/3] ALSA: hda/tas2781: add debug statements to kcontrols
2024-03-25 21:25 ` [PATCH 3/3] ALSA: hda/tas2781: add debug statements " Gergo Koteles
@ 2024-03-25 22:01 ` Pierre-Louis Bossart
2024-03-25 22:07 ` Andy Shevchenko
` (2 more replies)
0 siblings, 3 replies; 10+ messages in thread
From: Pierre-Louis Bossart @ 2024-03-25 22:01 UTC (permalink / raw)
To: Gergo Koteles, Shenghao Ding, Kevin Lu, Baojun Xu,
Jaroslav Kysela, Takashi Iwai
Cc: alsa-devel, linux-sound, linux-kernel
> +++ b/sound/pci/hda/tas2781_hda_i2c.c
> @@ -189,6 +189,9 @@ static int tasdevice_get_profile_id(struct snd_kcontrol *kcontrol,
>
> ucontrol->value.integer.value[0] = tas_priv->rcabin.profile_cfg_id;
>
> + dev_dbg(tas_priv->dev, "%s: %d\n", __func__,
Nit-pick: you don't need to add __func__ to dev_dbg logs, the user can
add the information with the dyndbg parameter, e.g.
options snd_intel_dspcfg dyndbg=+pmf
dev_err/warn don't have this functionality though so in those cases
there's no replacement for __func__
^ permalink raw reply [flat|nested] 10+ messages in thread
* Re: [PATCH 3/3] ALSA: hda/tas2781: add debug statements to kcontrols
2024-03-25 22:01 ` Pierre-Louis Bossart
@ 2024-03-25 22:07 ` Andy Shevchenko
2024-03-25 23:13 ` Gergo Koteles
2024-03-26 7:16 ` Takashi Iwai
2 siblings, 0 replies; 10+ messages in thread
From: Andy Shevchenko @ 2024-03-25 22:07 UTC (permalink / raw)
To: Pierre-Louis Bossart
Cc: Gergo Koteles, Shenghao Ding, Kevin Lu, Baojun Xu,
Jaroslav Kysela, Takashi Iwai, alsa-devel, linux-sound,
linux-kernel
Mon, Mar 25, 2024 at 05:01:18PM -0500, Pierre-Louis Bossart kirjoitti:
...
> > + dev_dbg(tas_priv->dev, "%s: %d\n", __func__,
>
> Nit-pick: you don't need to add __func__ to dev_dbg logs, the user can
> add the information with the dyndbg parameter, e.g.
>
> options snd_intel_dspcfg dyndbg=+pmf
>
> dev_err/warn don't have this functionality though so in those cases
> there's no replacement for __func__
You beat me up to it, I just downloaded the email thread to say the same.
Since I'm here, I think __func__ in dev_err()/dev_warn() usually says about
poorly written message itself (that it's not unique enough to distinguish
taking into account that this has device instance name as well). While pr_*()
ones indeed may benefit from having it.
--
With Best Regards,
Andy Shevchenko
^ permalink raw reply [flat|nested] 10+ messages in thread
* Re: [PATCH 3/3] ALSA: hda/tas2781: add debug statements to kcontrols
2024-03-25 22:01 ` Pierre-Louis Bossart
2024-03-25 22:07 ` Andy Shevchenko
@ 2024-03-25 23:13 ` Gergo Koteles
2024-03-26 7:16 ` Takashi Iwai
2 siblings, 0 replies; 10+ messages in thread
From: Gergo Koteles @ 2024-03-25 23:13 UTC (permalink / raw)
To: Pierre-Louis Bossart, Shenghao Ding, Kevin Lu, Baojun Xu,
Jaroslav Kysela, Takashi Iwai, Andy Shevchenko
Cc: alsa-devel, linux-sound, linux-kernel
Hi Pierre-Louis,
On Mon, 2024-03-25 at 17:01 -0500, Pierre-Louis Bossart wrote:
>
>
> > +++ b/sound/pci/hda/tas2781_hda_i2c.c
> > @@ -189,6 +189,9 @@ static int tasdevice_get_profile_id(struct snd_kcontrol *kcontrol,
> >
> > ucontrol->value.integer.value[0] = tas_priv->rcabin.profile_cfg_id;
> >
> > + dev_dbg(tas_priv->dev, "%s: %d\n", __func__,
>
> Nit-pick: you don't need to add __func__ to dev_dbg logs, the user can
> add the information with the dyndbg parameter, e.g.
>
> options snd_intel_dspcfg dyndbg=+pmf
>
> dev_err/warn don't have this functionality though so in those cases
> there's no replacement for __func__
>
Thanks. I just put a #define DEBUG into the first line and rebuilt the
module. It will be faster this way :)
I will send a v2.
Regards,
Gergo
^ permalink raw reply [flat|nested] 10+ messages in thread
* Re: [PATCH 3/3] ALSA: hda/tas2781: add debug statements to kcontrols
2024-03-25 22:01 ` Pierre-Louis Bossart
2024-03-25 22:07 ` Andy Shevchenko
2024-03-25 23:13 ` Gergo Koteles
@ 2024-03-26 7:16 ` Takashi Iwai
2 siblings, 0 replies; 10+ messages in thread
From: Takashi Iwai @ 2024-03-26 7:16 UTC (permalink / raw)
To: Pierre-Louis Bossart
Cc: Gergo Koteles, Shenghao Ding, Kevin Lu, Baojun Xu,
Jaroslav Kysela, Takashi Iwai, alsa-devel, linux-sound,
linux-kernel
On Mon, 25 Mar 2024 23:01:18 +0100,
Pierre-Louis Bossart wrote:
>
>
>
>
> > +++ b/sound/pci/hda/tas2781_hda_i2c.c
> > @@ -189,6 +189,9 @@ static int tasdevice_get_profile_id(struct snd_kcontrol *kcontrol,
> >
> > ucontrol->value.integer.value[0] = tas_priv->rcabin.profile_cfg_id;
> >
> > + dev_dbg(tas_priv->dev, "%s: %d\n", __func__,
>
> Nit-pick: you don't need to add __func__ to dev_dbg logs, the user can
> add the information with the dyndbg parameter, e.g.
>
> options snd_intel_dspcfg dyndbg=+pmf
Since this doesn't always show up, I don't mind to have the function
name shown explicitly there. OTOH, what bothers me is that all those
messages have a short format "__func__: %d" while the values are
utterly different, depending on the function. That can be confusing.
IMO, it'd be more user-friendly to indicate what values are presented,
too.
thanks,
Takashi
^ permalink raw reply [flat|nested] 10+ messages in thread
* Re: [PATCH 1/3] ALSA: hda/tas2781: remove digital gain kcontrol
2024-03-25 21:25 ` [PATCH 1/3] ALSA: hda/tas2781: remove digital gain kcontrol Gergo Koteles
@ 2024-03-26 12:58 ` kernel test robot
2024-03-27 4:21 ` kernel test robot
1 sibling, 0 replies; 10+ messages in thread
From: kernel test robot @ 2024-03-26 12:58 UTC (permalink / raw)
To: Gergo Koteles, Shenghao Ding, Kevin Lu, Baojun Xu,
Jaroslav Kysela, Takashi Iwai
Cc: oe-kbuild-all, alsa-devel, linux-sound, linux-kernel,
Gergo Koteles, stable
Hi Gergo,
kernel test robot noticed the following build errors:
[auto build test ERROR on 4cece764965020c22cff7665b18a012006359095]
url: https://github.com/intel-lab-lkp/linux/commits/Gergo-Koteles/ALSA-hda-tas2781-remove-digital-gain-kcontrol/20240326-052937
base: 4cece764965020c22cff7665b18a012006359095
patch link: https://lore.kernel.org/r/313e00499eb2caadd23a92284fdec418b650b7f4.1711401621.git.soyer%40irl.hu
patch subject: [PATCH 1/3] ALSA: hda/tas2781: remove digital gain kcontrol
config: i386-buildonly-randconfig-004-20240326 (https://download.01.org/0day-ci/archive/20240326/202403262031.SxBP17EM-lkp@intel.com/config)
compiler: gcc-12 (Debian 12.2.0-14) 12.2.0
reproduce (this is a W=1 build): (https://download.01.org/0day-ci/archive/20240326/202403262031.SxBP17EM-lkp@intel.com/reproduce)
If you fix the issue in a separate patch/commit (i.e. not just a new version of
the same patch/commit), kindly add following tags
| Reported-by: kernel test robot <lkp@intel.com>
| Closes: https://lore.kernel.org/oe-kbuild-all/202403262031.SxBP17EM-lkp@intel.com/
All errors (new ones prefixed by >>):
In file included from sound/soc/codecs/tas2781-i2c.c:29:
>> sound/soc/codecs/tas2781-i2c.c:148:41: error: 'dvc_tlv' undeclared here (not in a function)
148 | tas2781_digital_putvol, dvc_tlv),
| ^~~~~~~
include/sound/soc.h:271:19: note: in definition of macro 'SOC_SINGLE_RANGE_EXT_TLV'
271 | .tlv.p = (tlv_array), \
| ^~~~~~~~~
vim +/dvc_tlv +148 sound/soc/codecs/tas2781-i2c.c
ef3bcde75d06d6 Shenghao Ding 2023-06-18 141
ef3bcde75d06d6 Shenghao Ding 2023-06-18 142 static const struct snd_kcontrol_new tas2781_snd_controls[] = {
ef3bcde75d06d6 Shenghao Ding 2023-06-18 143 SOC_SINGLE_RANGE_EXT_TLV("Speaker Analog Gain", TAS2781_AMP_LEVEL,
ef3bcde75d06d6 Shenghao Ding 2023-06-18 144 1, 0, 20, 0, tas2781_amp_getvol,
ef3bcde75d06d6 Shenghao Ding 2023-06-18 145 tas2781_amp_putvol, amp_vol_tlv),
ef3bcde75d06d6 Shenghao Ding 2023-06-18 146 SOC_SINGLE_RANGE_EXT_TLV("Speaker Digital Gain", TAS2781_DVC_LVL,
ef3bcde75d06d6 Shenghao Ding 2023-06-18 147 0, 0, 200, 1, tas2781_digital_getvol,
ef3bcde75d06d6 Shenghao Ding 2023-06-18 @148 tas2781_digital_putvol, dvc_tlv),
ef3bcde75d06d6 Shenghao Ding 2023-06-18 149 SOC_SINGLE_BOOL_EXT("Speaker Force Firmware Load", 0,
ef3bcde75d06d6 Shenghao Ding 2023-06-18 150 tas2781_force_fwload_get, tas2781_force_fwload_put),
ef3bcde75d06d6 Shenghao Ding 2023-06-18 151 };
ef3bcde75d06d6 Shenghao Ding 2023-06-18 152
--
0-DAY CI Kernel Test Service
https://github.com/intel/lkp-tests/wiki
^ permalink raw reply [flat|nested] 10+ messages in thread
* Re: [PATCH 1/3] ALSA: hda/tas2781: remove digital gain kcontrol
2024-03-25 21:25 ` [PATCH 1/3] ALSA: hda/tas2781: remove digital gain kcontrol Gergo Koteles
2024-03-26 12:58 ` kernel test robot
@ 2024-03-27 4:21 ` kernel test robot
1 sibling, 0 replies; 10+ messages in thread
From: kernel test robot @ 2024-03-27 4:21 UTC (permalink / raw)
To: Gergo Koteles, Shenghao Ding, Kevin Lu, Baojun Xu,
Jaroslav Kysela, Takashi Iwai
Cc: llvm, oe-kbuild-all, alsa-devel, linux-sound, linux-kernel,
Gergo Koteles, stable
Hi Gergo,
kernel test robot noticed the following build errors:
[auto build test ERROR on 4cece764965020c22cff7665b18a012006359095]
url: https://github.com/intel-lab-lkp/linux/commits/Gergo-Koteles/ALSA-hda-tas2781-remove-digital-gain-kcontrol/20240326-052937
base: 4cece764965020c22cff7665b18a012006359095
patch link: https://lore.kernel.org/r/313e00499eb2caadd23a92284fdec418b650b7f4.1711401621.git.soyer%40irl.hu
patch subject: [PATCH 1/3] ALSA: hda/tas2781: remove digital gain kcontrol
config: i386-randconfig-011-20240326 (https://download.01.org/0day-ci/archive/20240327/202403271212.5Lxo4b20-lkp@intel.com/config)
compiler: clang version 17.0.6 (https://github.com/llvm/llvm-project 6009708b4367171ccdbf4b5905cb6a803753fe18)
reproduce (this is a W=1 build): (https://download.01.org/0day-ci/archive/20240327/202403271212.5Lxo4b20-lkp@intel.com/reproduce)
If you fix the issue in a separate patch/commit (i.e. not just a new version of
the same patch/commit), kindly add following tags
| Reported-by: kernel test robot <lkp@intel.com>
| Closes: https://lore.kernel.org/oe-kbuild-all/202403271212.5Lxo4b20-lkp@intel.com/
All errors (new ones prefixed by >>):
>> sound/soc/codecs/tas2781-i2c.c:148:27: error: use of undeclared identifier 'dvc_tlv'
148 | tas2781_digital_putvol, dvc_tlv),
| ^
>> sound/soc/codecs/tas2781-i2c.c:595:19: error: invalid application of 'sizeof' to an incomplete type 'const struct snd_kcontrol_new[]'
595 | .num_controls = ARRAY_SIZE(tas2781_snd_controls),
| ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
include/linux/array_size.h:11:32: note: expanded from macro 'ARRAY_SIZE'
11 | #define ARRAY_SIZE(arr) (sizeof(arr) / sizeof((arr)[0]) + __must_be_array(arr))
| ^~~~~
2 errors generated.
vim +/dvc_tlv +148 sound/soc/codecs/tas2781-i2c.c
ef3bcde75d06d6 Shenghao Ding 2023-06-18 141
ef3bcde75d06d6 Shenghao Ding 2023-06-18 142 static const struct snd_kcontrol_new tas2781_snd_controls[] = {
ef3bcde75d06d6 Shenghao Ding 2023-06-18 143 SOC_SINGLE_RANGE_EXT_TLV("Speaker Analog Gain", TAS2781_AMP_LEVEL,
ef3bcde75d06d6 Shenghao Ding 2023-06-18 144 1, 0, 20, 0, tas2781_amp_getvol,
ef3bcde75d06d6 Shenghao Ding 2023-06-18 145 tas2781_amp_putvol, amp_vol_tlv),
ef3bcde75d06d6 Shenghao Ding 2023-06-18 146 SOC_SINGLE_RANGE_EXT_TLV("Speaker Digital Gain", TAS2781_DVC_LVL,
ef3bcde75d06d6 Shenghao Ding 2023-06-18 147 0, 0, 200, 1, tas2781_digital_getvol,
ef3bcde75d06d6 Shenghao Ding 2023-06-18 @148 tas2781_digital_putvol, dvc_tlv),
ef3bcde75d06d6 Shenghao Ding 2023-06-18 149 SOC_SINGLE_BOOL_EXT("Speaker Force Firmware Load", 0,
ef3bcde75d06d6 Shenghao Ding 2023-06-18 150 tas2781_force_fwload_get, tas2781_force_fwload_put),
ef3bcde75d06d6 Shenghao Ding 2023-06-18 151 };
ef3bcde75d06d6 Shenghao Ding 2023-06-18 152
--
0-DAY CI Kernel Test Service
https://github.com/intel/lkp-tests/wiki
^ permalink raw reply [flat|nested] 10+ messages in thread
end of thread, other threads:[~2024-03-27 4:22 UTC | newest]
Thread overview: 10+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2024-03-25 21:25 [PATCH 0/3] ALSA: hda/tas2781: fixes for 6.9-rc1 Gergo Koteles
2024-03-25 21:25 ` [PATCH 1/3] ALSA: hda/tas2781: remove digital gain kcontrol Gergo Koteles
2024-03-26 12:58 ` kernel test robot
2024-03-27 4:21 ` kernel test robot
2024-03-25 21:25 ` [PATCH 2/3] ALSA: hda/tas2781: add locks to kcontrols Gergo Koteles
2024-03-25 21:25 ` [PATCH 3/3] ALSA: hda/tas2781: add debug statements " Gergo Koteles
2024-03-25 22:01 ` Pierre-Louis Bossart
2024-03-25 22:07 ` Andy Shevchenko
2024-03-25 23:13 ` Gergo Koteles
2024-03-26 7:16 ` Takashi Iwai
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox