March 2021 - Alsa-devel - mailman.alsa-project.org

Re: [PATCH 4/4] ALSA: hda/cirrus: Add Headphone and Headset MIC Volume Control
by Takashi Iwai 04 Mar '21

04 Mar '21

On Wed, 03 Mar 2021 19:29:59 +0100, Vitaly Rodionov wrote: > > +static int cs8409_cs42l42_volume_get(struct snd_kcontrol *kcontrol, > + struct snd_ctl_elem_value *ucontrol) > +{ > + struct hda_codec *codec = snd_kcontrol_chip(kcontrol); > + hda_nid_t nid = get_amp_nid(kcontrol); > + int chs = get_amp_channels(kcontrol); > + long *valp = ucontrol->value.integer.value; > + char vol = 0; > + > + codec_dbg(codec, "%s() nid: %d\n", __func__, nid); > + snd_hda_power_up(codec); > + switch (nid) { > + case CS8409_CS42L42_HP_PIN_NID: > + mutex_lock(&cs8409_i2c_mux); > + if (chs & 1) { > + vol = -(cs8409_i2c_read(codec, CS42L42_I2C_ADDR, > + CS8409_CS42L42_REG_HS_VOLUME_CHA, 1)); Better to cache the values instead of i2c read at each time? Then the unnecessary power up/down sequence can be avoided, too. thanks, Takashi

2 1

Re: [PATCH 3/4] ALSA: hda/cirrus: Add jack detect interrupt support from CS42L42 companion codec.
by Takashi Iwai 04 Mar '21

04 Mar '21

On Wed, 03 Mar 2021 19:29:58 +0100, Vitaly Rodionov wrote: > @@ -1243,6 +1258,8 @@ static int patch_cs4213(struct hda_codec *codec) > #define CIR_I2C_QWRITE 0x005D > #define CIR_I2C_QREAD 0x005E > > +static struct mutex cs8409_i2c_mux; Any reason that this must be the global mutex? I suppose it can fit in own spec->i2c_mutex instead? > +static void cs8409_cs42l42_cap_sync_hook(struct hda_codec *codec, > + struct snd_kcontrol *kcontrol, > + struct snd_ctl_elem_value *ucontrol) > +{ > + struct cs_spec *spec = codec->spec; > + unsigned int curval, expval; > + /* CS8409 DMIC Pin only allows the setting of the Stream Parameters in > + * Power State D0. When a headset is unplugged, and the path is switched to > + * the DMIC, the Stream is restarted with the new ADC, but this is done in > + * Power State D3. Restart the Stream now DMIC is in D0. > + */ > + if (spec->gen.cur_adc == CS8409_CS42L42_DMIC_ADC_PIN_NID) { > + curval = snd_hda_codec_read(codec, spec->gen.cur_adc, > + 0, AC_VERB_GET_CONV, 0); > + expval = (spec->gen.cur_adc_stream_tag << 4) | 0; > + if (curval != expval) { > + codec_dbg(codec, "%s Restarting Stream after DMIC switch\n", __func__); > + __snd_hda_codec_cleanup_stream(codec, spec->gen.cur_adc, 1); > + snd_hda_codec_setup_stream(codec, spec->gen.cur_adc, > + spec->gen.cur_adc_stream_tag, 0, > + spec->gen.cur_adc_format); Hrm, this looks a big scary. We may need to reconsider how to handle this better later, but it's OK as long as you've tested with this code... > +static int cs8409_cs42l42_init(struct hda_codec *codec) > +{ > + int ret = 0; > + > + ret = snd_hda_gen_init(codec); > + > + if (!ret) { > + /* On Dell platforms with suspend D3 mode support we > + * have to re-initialise cs8409 bridge and companion > + * cs42l42 codec > + */ > + snd_hda_sequence_write(codec, cs8409_cs42l42_init_verbs); > + snd_hda_sequence_write(codec, cs8409_cs42l42_add_verbs); > + > + cs8409_cs42l42_hw_init(codec); Ah... the init stuff at resume appears finally here. This part should be in the second patch instead. > +static int cs8409_cs42l42_exec_verb(struct hdac_device *dev, > + unsigned int cmd, unsigned int flags, unsigned int *res) > +{ > + struct hda_codec *codec = container_of(dev, struct hda_codec, core); > + struct cs_spec *spec = codec->spec; > + > + unsigned int nid = 0; > + unsigned int verb = 0; The blank line above should be removed. > + case CS8409_CS42L42_HP_PIN_NID: > + if (verb == AC_VERB_GET_PIN_SENSE) { > + *res = (spec->cs42l42_hp_jack_in)?AC_PINSENSE_PRESENCE:0; The spaces are needed around operators. Similar coding-style issues are seen other places. Please try to run scripts/checkpatch.pl. thanks, Takashi

2 1

Re: [PATCH 2/4] ALSA: hda/cirrus: Add support for CS8409 HDA bridge and CS42L42 companion codec.
by Takashi Iwai 04 Mar '21

04 Mar '21

On Wed, 03 Mar 2021 19:29:57 +0100, Vitaly Rodionov wrote: > > +static const struct hda_verb cs8409_cs42l42_init_verbs[] = { > + { 0x01, AC_VERB_SET_POWER_STATE, 0x0000 }, /* AFG: D0 */ I guess this power state change is superfluous. The AFG node is already powered up when the codec probe or init is called. > + { 0x01, AC_VERB_SET_GPIO_DIRECTION, 0x0020 }, /* GPIO 5 out, 3,4 in */ > + { 0x01, AC_VERB_SET_GPIO_DATA, 0x0000 }, /* GPIO data 0 */ > + { 0x01, AC_VERB_SET_GPIO_MASK, 0x003f }, /* Enable GPIO */ Those are handled in spec->gpio_dir, gpio->data and gpio->mask fields. > + { 0x01, AC_VERB_SET_GPIO_WAKE_MASK, 0x0018 }, /* WAKE from GPIO 3,4 */ > + { 0x47, AC_VERB_SET_PROC_STATE, 0x0001 }, /* Enable VPW processing */ > + { 0x47, AC_VERB_SET_COEF_INDEX, 0x0002 }, /* Configure GPIO 6,7 */ > + { 0x47, AC_VERB_SET_PROC_COEF, 0x0080 }, /* I2C mode */ > + { 0x47, AC_VERB_SET_COEF_INDEX, 0x005b }, /* Set I2C bus speed */ > + { 0x47, AC_VERB_SET_PROC_COEF, 0x0200 }, /* 100kHz I2C_STO = 2 */ Those remaining verbs are good in the init verbs. But I suppose they have to be applied at the resume as well? But... > +static int cs8409_cs42l42_fixup(struct hda_codec *codec) > +{ > + int err = 0; > + struct cs_spec *spec = codec->spec; > + unsigned int pincap = 0; > + > + /* Basic initial sequence for specific hw configuration */ > + snd_hda_sequence_write(codec, cs8409_cs42l42_init_verbs); ... it seems applied only at the fixup call at parsing? Ditto about cs8409_cs42l42_hw_init(codec). thanks, Takashi

2 1

[PATCH] ALSA: hda: ignore invalid NHLT table
by Mark Pearson 04 Mar '21

04 Mar '21

On some Lenovo systems if the microphone is disabled in the BIOS only the NHLT table header is created, with no data. This means the endpoints field is not correctly set to zero - leading to an unintialised variable and hence invalid descriptors are parsed leading to page faults. The Lenovo firmware team is addressing this, but adding a check preventing invalid tables being parsed is worthwhile. Tested on a Lenovo T14. Tested-by: Philipp Leskovitz <philipp.leskovitz(a)secunet.com> Reported-by: Philipp Leskovitz <philipp.leskovitz(a)secunet.com> Signed-off-by: Mark Pearson <markpearson(a)lenovo.com> --- sound/hda/intel-nhlt.c | 5 +++++ 1 file changed, 5 insertions(+) diff --git a/sound/hda/intel-nhlt.c b/sound/hda/intel-nhlt.c index 059aaf04f..0889f2cc5 100644 --- a/sound/hda/intel-nhlt.c +++ b/sound/hda/intel-nhlt.c @@ -37,6 +37,11 @@ int intel_nhlt_get_dmic_geo(struct device *dev, struct nhlt_acpi_table *nhlt) if (!nhlt) return 0; + if (nhlt->header.length <= sizeof(struct acpi_table_header)) { + dev_warn(dev, "Invalid DMIC description table\n"); + return 0; + } + epnt = (struct nhlt_endpoint *)nhlt->desc; for (j = 0; j < nhlt->endpoint_count; j++) { -- 2.29.2

3 4

[PATCH] ALSA: usb-audio: Disable USB autosuspend properly in setup_disable_autosuspend()
by Kai-Heng Feng 04 Mar '21

04 Mar '21

Rear audio on Lenovo ThinkStation P620 stops working after commit 1965c4364bdd ("ALSA: usb-audio: Disable autosuspend for Lenovo ThinkStation P620"): [ 6.013526] usbcore: registered new interface driver snd-usb-audio [ 6.023064] usb 3-6: cannot get ctl value: req = 0x81, wValue = 0x100, wIndex = 0x0, type = 1 [ 6.023083] usb 3-6: cannot get ctl value: req = 0x81, wValue = 0x202, wIndex = 0x0, type = 4 [ 6.023090] usb 3-6: cannot get ctl value: req = 0x81, wValue = 0x100, wIndex = 0x0, type = 1 [ 6.023098] usb 3-6: cannot get ctl value: req = 0x81, wValue = 0x202, wIndex = 0x0, type = 4 [ 6.023103] usb 3-6: cannot get ctl value: req = 0x81, wValue = 0x100, wIndex = 0x0, type = 1 [ 6.023110] usb 3-6: cannot get ctl value: req = 0x81, wValue = 0x202, wIndex = 0x0, type = 4 [ 6.045846] usb 3-6: cannot get ctl value: req = 0x81, wValue = 0x100, wIndex = 0x0, type = 1 [ 6.045866] usb 3-6: cannot get ctl value: req = 0x81, wValue = 0x202, wIndex = 0x0, type = 4 [ 6.045877] usb 3-6: cannot get ctl value: req = 0x81, wValue = 0x100, wIndex = 0x0, type = 1 [ 6.045886] usb 3-6: cannot get ctl value: req = 0x81, wValue = 0x202, wIndex = 0x0, type = 4 [ 6.045894] usb 3-6: cannot get ctl value: req = 0x81, wValue = 0x100, wIndex = 0x0, type = 1 [ 6.045908] usb 3-6: cannot get ctl value: req = 0x81, wValue = 0x202, wIndex = 0x0, type = 4 I overlooked the issue because when I was working on the said commit, only the front audio is tested. Apology for that. Changing supports_autosuspend in driver is too late for disabling autosuspend, because it was already used by USB probe routine, so it can break the balance on the following code that depends on supports_autosuspend. Fix it by using usb_disable_autosuspend() helper, and balance the suspend count in disconnect callback. Fixes: 1965c4364bdd ("ALSA: usb-audio: Disable autosuspend for Lenovo ThinkStation P620") Signed-off-by: Kai-Heng Feng <kai.heng.feng(a)canonical.com> --- sound/usb/card.c | 5 +++++ sound/usb/quirks.c | 2 +- sound/usb/usbaudio.h | 1 + 3 files changed, 7 insertions(+), 1 deletion(-) diff --git a/sound/usb/card.c b/sound/usb/card.c index 85ed8507e41a..08c794883299 100644 --- a/sound/usb/card.c +++ b/sound/usb/card.c @@ -830,6 +830,8 @@ static int usb_audio_probe(struct usb_interface *intf, snd_media_device_create(chip, intf); } + chip->quirk_type = quirk->type; + usb_chip[chip->index] = chip; chip->intf[chip->num_interfaces] = intf; chip->num_interfaces++; @@ -912,6 +914,9 @@ static void usb_audio_disconnect(struct usb_interface *intf) } else { mutex_unlock(&register_mutex); } + + if (chip->quirk_type & QUIRK_SETUP_DISABLE_AUTOSUSPEND) + usb_enable_autosuspend(interface_to_usbdev(intf)); } /* lock the shutdown (disconnect) task and autoresume */ diff --git a/sound/usb/quirks.c b/sound/usb/quirks.c index 9ba4682ebc48..ef5ee899db26 100644 --- a/sound/usb/quirks.c +++ b/sound/usb/quirks.c @@ -547,7 +547,7 @@ static int setup_disable_autosuspend(struct snd_usb_audio *chip, struct usb_driver *driver, const struct snd_usb_audio_quirk *quirk) { - driver->supports_autosuspend = 0; + usb_disable_autosuspend(interface_to_usbdev(iface)); return 1; /* Continue with creating streams and mixer */ } diff --git a/sound/usb/usbaudio.h b/sound/usb/usbaudio.h index 215c1771dd57..60b9dd7df6bb 100644 --- a/sound/usb/usbaudio.h +++ b/sound/usb/usbaudio.h @@ -27,6 +27,7 @@ struct snd_usb_audio { struct snd_card *card; struct usb_interface *intf[MAX_CARD_INTERFACES]; u32 usb_id; + uint16_t quirk_type; struct mutex mutex; unsigned int system_suspend; atomic_t active; -- 2.30.0

3 3

[PATCH v1 0/5] Add missing reset controls to NVIDIA Tegra ASoC drivers
by Dmitry Osipenko 04 Mar '21

04 Mar '21

Hi, This series adds missing hardware reset controls to I2S and AC97 drivers. Currently drivers happen to work properly because reset is implicitly deasserted by tegra-clk driver, but clk driver shouldn't touch the resets and we need to fix it because this breaks other Tegra drivers. Previously we fixed the resets of the AHUB and HDMI codec drivers, but turned out that we missed the I2C and AC97 drivers. Thanks to Paul Fertser for testing the pending clk patches and finding that audio got broken on Tegra20 AC100 netbook because of the missing I2S reset. Dmitry Osipenko (5): reset: Allow devm_reset_control_array_get() to get resets in a released state reset: Add devm_reset_control_array_get_exclusive_released() ASoC: tegra20: ac97: Add reset control ASoC: tegra20: i2s: Add reset control ASoC: tegra30: i2s: Add reset control drivers/reset/core.c | 8 ++++++-- include/linux/reset.h | 20 +++++++++++++------ sound/soc/tegra/tegra20_ac97.c | 21 ++++++++++++++++++++ sound/soc/tegra/tegra20_ac97.h | 1 + sound/soc/tegra/tegra20_i2s.c | 31 +++++++++++++++++++++++++++++ sound/soc/tegra/tegra20_i2s.h | 1 + sound/soc/tegra/tegra30_ahub.c | 14 ++++++++++--- sound/soc/tegra/tegra30_i2s.c | 36 +++++++++++++++++++++++++++++++++- sound/soc/tegra/tegra30_i2s.h | 1 + 9 files changed, 121 insertions(+), 12 deletions(-) -- 2.29.2

2 8

[PATCH] ALSA: usb-audio: Apply the control quirk to Plantronics headsets
by Takashi Iwai 04 Mar '21

04 Mar '21

Other Plantronics headset models seem requiring the same workaround as C320-M to add the 20ms delay for the control messages, too. Apply the workaround generically for devices with the vendor ID 0x047f. Note that the problem didn't surface before 5.11 just with luck. Since 5.11 got a big code rewrite about the stream handling, the parameter setup procedure has changed, and this seemed triggering the problem more often. BugLink: https://bugzilla.suse.com/show_bug.cgi?id=1182552 Cc: <stable(a)vger.kernel.org> Signed-off-by: Takashi Iwai <tiwai(a)suse.de> --- sound/usb/quirks.c | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/sound/usb/quirks.c b/sound/usb/quirks.c index 0864692d8a7b..84e5dd66a56b 100644 --- a/sound/usb/quirks.c +++ b/sound/usb/quirks.c @@ -1671,10 +1671,10 @@ void snd_usb_ctl_msg_quirk(struct usb_device *dev, unsigned int pipe, msleep(20); /* - * Plantronics C320-M needs a delay to avoid random - * microhpone failures. + * Plantronics headsets (C320, C320-M, etc) need a delay to avoid + * random microhpone failures. */ - if (chip->usb_id == USB_ID(0x047f, 0xc025) && + if (USB_ID_VENDOR(chip->usb_id) == 0x047f && (requesttype & USB_TYPE_MASK) == USB_TYPE_CLASS) msleep(20); -- 2.26.2

1 0

[PATCH] ALSA: usb-audio: Fix "cannot get freq eq" errors on Dell AE515 sound bar
by Takashi Iwai 04 Mar '21

04 Mar '21

Dell AE515 sound bar (413c:a506) spews the error messages when the driver tries to read the current sample frequency, hence it needs to be on the list in snd_usb_get_sample_rate_quirk(). BugLink: https://bugzilla.kernel.org/show_bug.cgi?id=211551 Cc: <stable(a)vger.kernel.org> Signed-off-by: Takashi Iwai <tiwai(a)suse.de> --- sound/usb/quirks.c | 1 + 1 file changed, 1 insertion(+) diff --git a/sound/usb/quirks.c b/sound/usb/quirks.c index d02dac5fcd40..58f1dff79e52 100644 --- a/sound/usb/quirks.c +++ b/sound/usb/quirks.c @@ -1523,6 +1523,7 @@ bool snd_usb_get_sample_rate_quirk(struct snd_usb_audio *chip) case USB_ID(0x1901, 0x0191): /* GE B850V3 CP2114 audio interface */ case USB_ID(0x21b4, 0x0081): /* AudioQuest DragonFly */ case USB_ID(0x2912, 0x30c8): /* Audioengine D1 */ + case USB_ID(0x413c, 0xa506): /* Dell AE515 sound bar */ return true; } -- 2.26.2

1 0

[PATCH][next] ASoC: fsl: fsl_easrc: Fix uninitialized variable st2_mem_alloc
by Colin King 04 Mar '21

04 Mar '21

From: Colin Ian King <colin.king(a)canonical.com> A previous cleanup commit removed the ininitialization of st2_mem_alloc. Fix this by restoring the original behaviour by initializing it to zero. Addresses-Coverity: ("Uninitialized scalar variable") Fixes: e80382fe721f ("ASoC: fsl: fsl_easrc: remove useless assignments") Signed-off-by: Colin Ian King <colin.king(a)canonical.com> --- sound/soc/fsl/fsl_easrc.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/sound/soc/fsl/fsl_easrc.c b/sound/soc/fsl/fsl_easrc.c index 725a5d3aaa02..e823c9c13764 100644 --- a/sound/soc/fsl/fsl_easrc.c +++ b/sound/soc/fsl/fsl_easrc.c @@ -710,7 +710,7 @@ static int fsl_easrc_max_ch_for_slot(struct fsl_asrc_pair *ctx, struct fsl_easrc_slot *slot) { struct fsl_easrc_ctx_priv *ctx_priv = ctx->private; - int st1_mem_alloc = 0, st2_mem_alloc; + int st1_mem_alloc = 0, st2_mem_alloc = 0; int pf_mem_alloc = 0; int max_channels = 8 - slot->num_channel; int channels = 0; -- 2.30.0

2 1

[PATCH] SoC: codecs: fix platform_no_drv_owner.cocci warnings
by Yang Li 04 Mar '21

04 Mar '21

./sound/soc/codecs/lpass-rx-macro.c:3588:3-8: No need to set .owner here. The core will do it. Remove .owner field if calls are used which set it automatically Reported-by: Abaci Robot <abaci(a)linux.alibaba.com> Signed-off-by: Yang Li <yang.lee(a)linux.alibaba.com> --- sound/soc/codecs/lpass-rx-macro.c | 1 - 1 file changed, 1 deletion(-) diff --git a/sound/soc/codecs/lpass-rx-macro.c b/sound/soc/codecs/lpass-rx-macro.c index c9c21d22..5b9d4e9 100644 --- a/sound/soc/codecs/lpass-rx-macro.c +++ b/sound/soc/codecs/lpass-rx-macro.c @@ -3585,7 +3585,6 @@ static int rx_macro_remove(struct platform_device *pdev) static struct platform_driver rx_macro_driver = { .driver = { .name = "rx_macro", - .owner = THIS_MODULE, .of_match_table = rx_macro_dt_match, .suppress_bind_attrs = true, }, -- 1.8.3.1

2 2