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

[PATCH] ALSA: echoaudio: remove redundant assignment to variable bytes
by Colin Ian King 04 Mar '22

04 Mar '22

The variable bytes is being assigned a value that is never read, it is being re-assigned inside a following if block. The assignment is redundant and can be removed. Cleans up clang scan build warning: sound/pci/echoaudio/midi.c:211:9: warning: Although the value stored to 'bytes' is used in the enclosing expression, the value is never actually read from 'bytes' [deadcode.DeadStores] Signed-off-by: Colin Ian King <colin.i.king(a)gmail.com> --- sound/pci/echoaudio/midi.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/sound/pci/echoaudio/midi.c b/sound/pci/echoaudio/midi.c index cb72d27e809e..7be5c3327b16 100644 --- a/sound/pci/echoaudio/midi.c +++ b/sound/pci/echoaudio/midi.c @@ -208,7 +208,7 @@ static void snd_echo_midi_output_write(struct timer_list *t) /* No interrupts are involved: we have to check at regular intervals if the card's output buffer has room for new data. */ - sent = bytes = 0; + sent = 0; spin_lock_irqsave(&chip->lock, flags); chip->midi_full = 0; if (!snd_rawmidi_transmit_empty(chip->midi_out)) { -- 2.34.1

2 1

Re: Issue in pcm_dsnoop.c in alsa-lib
by S.J. Wang 04 Mar '22

04 Mar '22

> > > > Hi Takashi Iwai, Jaroslav Kysela > > > > We encountered an issue in the pcm_dsnoop use case, could you > > please help to have a look? > > > > *Issue description:* > > With two instances for dsnoop type device running in parallel, > > after suspend/resume, one of the instances will be hung in memcpy > > because the very large copy size is obtained. > > > > #3 0x0000ffffa78d5098 in snd_pcm_dsnoop_sync_ptr > (pcm=0xaaab06563da0) > > at pcm_dsnoop.c:158 dsnoop = 0xaaab06563c20 slave_hw_ptr = 64 > > old_slave_hw_ptr = 533120 avail = *187651522444320* > > > > * Reason analysis: * > > The root cause that I analysis is that after suspend/resume, one > > instance will get the SND_PCM_STATE_SUSPENDED state from slave pcm > device, > > then it will do snd_pcm_prepare() and snd_pcm_start(), which will > > reset the dsnoop->slave_hw_ptr and the hw_ptr of slave pcm device, > > then the state of this instance is correct. But another instance may > > not get the SND_PCM_STATE_SUSPENDED state from slave pcm device > > because slave device may have been recovered by first instance, so > > the dsnoop->slave_hw_ptr is not reset. but because hw_ptr of slave > > pcm device has been reset, so there will be a very large "avail" size. > > > > *Solution:* > > I didn't come up with a fix for this issue, seems there is no easy > > way to let another instance know this case and reset the > > dsnoop->slave_hw_ptr, could you please help? > > Could you try topic/pcm-direct-resume branch on > > https://eur01.safelinks.protection.outlook.com/?url=https%3A%2F%2Fgithub > .com%2Ftiwai%2Falsa- > lib&data=04%7C01%7Cshengjiu.wang%40nxp.com%7C95f97de3f2c840d > 9853508d9fd2e79ea%7C686ea1d3bc2b4c6fa92cd99c5c301635%7C0%7C0%7C > 637819198319430045%7CUnknown%7CTWFpbGZsb3d8eyJWIjoiMC4wLjAwM > DAiLCJQIjoiV2luMzIiLCJBTiI6Ik1haWwiLCJXVCI6Mn0%3D%7C3000&sdat > a=WWX1ZlcQhJF3pHJdHPIH%2B0xG9o%2FjQnHG5fHDbKXwQwE%3D&r > eserved=0 > Thanks, I push my test result in https://github.com/alsa-project/alsa-lib/issues/213 Could you please review? Best regards Wang shengjiu

2 1

Re: [PATCH 2/6] treewide: remove using list iterator after loop body as a ptr
by Linus Torvalds 04 Mar '22

04 Mar '22

On Mon, Feb 28, 2022 at 11:56 AM Linus Torvalds <torvalds(a)linux-foundation.org> wrote: > > I do wish we could actually poison the 'pos' value after the loop > somehow - but clearly the "might be uninitialized" I was hoping for > isn't the way to do it. Side note: we do need *some* way to do it. Because if we make that change, and only set it to another pointer when not the head, then we very much change the semantics of "list_for_each_head()". The "list was empty" case would now exit with 'pos' not set at all (or set to NULL if we add that). Or it would be set to the last entry. And regardless, that means that all the if (pos == head) kinds of checks after the loop would be fundamentally broken. Darn. I really hoped for (and naively expected) that we could actually have the compiler warn about the use-after-loop case. That whole "compiler will now complain about bad use" was integral to my clever plan to use the C99 feature of declaring the iterator inside the loop. But my "clever plan" was apparently some ACME-level Wile E. Coyote sh*t. Darn. Linus

11 34

[PATCH] ASoC: rt5665: Don't block workqueue if card is unbound
by Zhen Ni 04 Mar '22

04 Mar '22

The current rt5665_jack_detect_handler() assumes the component and card will always show up and implements an infinite usleep loop waiting for them to show up. This does not hold true if a codec interrupt (or other event) occurs when the card is unbound. The codec driver's remove or shutdown functions cannot cancel the workqueue due to the wait loop. As a result, code can either end up blocking the workqueue, or hit a kernel oops when the card is freed. Fix the issue by rescheduling the jack detect handler in case the card is not ready. In case card never shows up, the shutdown/remove/suspend calls can now cancel the detect task. Signed-off-by: Zhen Ni <nizhen(a)uniontech.com> --- sound/soc/codecs/rt5665.c | 20 +++++++------------- 1 file changed, 7 insertions(+), 13 deletions(-) diff --git a/sound/soc/codecs/rt5665.c b/sound/soc/codecs/rt5665.c index 33e889802ff8..cdfd7085df2a 100644 --- a/sound/soc/codecs/rt5665.c +++ b/sound/soc/codecs/rt5665.c @@ -1293,19 +1293,13 @@ static void rt5665_jack_detect_handler(struct work_struct *work) container_of(work, struct rt5665_priv, jack_detect_work.work); int val, btn_type; - while (!rt5665->component) { - pr_debug("%s codec = null\n", __func__); - usleep_range(10000, 15000); - } - - while (!rt5665->component->card->instantiated) { - pr_debug("%s\n", __func__); - usleep_range(10000, 15000); - } - - while (!rt5665->calibration_done) { - pr_debug("%s calibration not ready\n", __func__); - usleep_range(10000, 15000); + if (!rt5665->component || !rt5665->component->card->instantiated || + !rt5665->calibration_done) { + pr_debug("%s card not yet ready\n", __func__); + /* try later */ + mod_delayed_work(system_power_efficient_wq, + &rt5665->jack_detect_work, msecs_to_jiffies(15)); + return; } mutex_lock(&rt5665->calibrate_mutex); -- 2.20.1

1 0

Re: Re: [PATCH] ASoC: wm8350: Handle error for wm8350_register_irq
by Jiasheng Jiang 04 Mar '22

04 Mar '22

On Thu, Mar 03, 2022 at 09:38:34PM +0800, Charles Keepax wrote: >> if (ret != 0) >> - return ret; >> + goto err; > > I would probably just leave this as a return, nothing is gained > changing it to a goto. > >> >> + ret = wm8350_register_irq(wm8350, WM8350_IRQ_CODEC_JCK_DET_R, >> wm8350_hpr_jack_handler, 0, "Right jack detect", >> priv); >> - wm8350_register_irq(wm8350, WM8350_IRQ_CODEC_MICSCD, >> + if (ret != 0) >> + goto free_JCK_DET_L; > > Probably better to use non-caps here, having caps in a label is a > little unusual. Thansk, I have submitted v2 to solve the problems. Jiang

1 0

Re: mmotm 2022-02-23-21-20 uploaded (Kconfig warning: SND_SOC_WCD938X)
by Randy Dunlap 03 Mar '22

03 Mar '22

On 2/23/22 21:21, Andrew Morton wrote: > The mm-of-the-moment snapshot 2022-02-23-21-20 has been uploaded to > > https://www.ozlabs.org/~akpm/mmotm/ > > mmotm-readme.txt says > > README for mm-of-the-moment: > > https://www.ozlabs.org/~akpm/mmotm/ > > This is a snapshot of my -mm patch queue. Uploaded at random hopefully > more than once a week. on x86_64: WARNING: unmet direct dependencies detected for SND_SOC_WCD938X Depends on [n]: SOUND [=y] && !UML && SND [=y] && SND_SOC [=m] && SND_SOC_WCD938X_SDW [=n] && (SOUNDWIRE [=n] || !SOUNDWIRE [=n]) Selected by [m]: - SND_SOC_SC7280 [=m] && SOUND [=y] && !UML && SND [=y] && SND_SOC [=m] && SND_SOC_QCOM [=m] && (I2C [=y] && SOUNDWIRE [=n] || COMPILE_TEST [=y]) Full randconfig file is attached. -- ~Randy

1 1

[PATCH] soundwire: qcom: fix build failures if CONFIG_PM=n
by Srinivas Kandagatla 03 Mar '22

03 Mar '22

Fix below build failure drivers/soundwire/qcom.c:1460:12: error: 'swrm_runtime_resume' defined but not used [-Werror=unused-function] by marking the resume swrm_runtime_resume() with __maybe_unused attribute. Reported-by: Geert Uytterhoeven <geert(a)linux-m68k.org> Reported-by: Stephen Rothwell <sfr(a)canb.auug.org.au> Signed-off-by: Srinivas Kandagatla <srinivas.kandagatla(a)linaro.org> --- drivers/soundwire/qcom.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/drivers/soundwire/qcom.c b/drivers/soundwire/qcom.c index e893aee1b057..6575695cd400 100644 --- a/drivers/soundwire/qcom.c +++ b/drivers/soundwire/qcom.c @@ -1459,7 +1459,7 @@ static bool swrm_wait_for_frame_gen_enabled(struct qcom_swrm_ctrl *swrm) return false; } -static int swrm_runtime_resume(struct device *dev) +static int __maybe_unused swrm_runtime_resume(struct device *dev) { struct qcom_swrm_ctrl *ctrl = dev_get_drvdata(dev); int ret; -- 2.21.0

2 2

Issue in pcm_dsnoop.c in alsa-lib
by Shengjiu Wang 03 Mar '22

03 Mar '22

Hi Takashi Iwai, Jaroslav Kysela We encountered an issue in the pcm_dsnoop use case, could you please help to have a look? *Issue description:* With two instances for dsnoop type device running in parallel, after suspend/resume, one of the instances will be hung in memcpy because the very large copy size is obtained. #3 0x0000ffffa78d5098 in snd_pcm_dsnoop_sync_ptr (pcm=0xaaab06563da0) at pcm_dsnoop.c:158 dsnoop = 0xaaab06563c20 slave_hw_ptr = 64 old_slave_hw_ptr = 533120 avail = *187651522444320* * Reason analysis: * The root cause that I analysis is that after suspend/resume, one instance will get the SND_PCM_STATE_SUSPENDED state from slave pcm device, then it will do snd_pcm_prepare() and snd_pcm_start(), which will reset the dsnoop->slave_hw_ptr and the hw_ptr of slave pcm device, then the state of this instance is correct. But another instance may not get the SND_PCM_STATE_SUSPENDED state from slave pcm device because slave device may have been recovered by first instance, so the dsnoop->slave_hw_ptr is not reset. but because hw_ptr of slave pcm device has been reset, so there will be a very large "avail" size. *Solution:* I didn't come up with a fix for this issue, seems there is no easy way to let another instance know this case and reset the dsnoop->slave_hw_ptr, could you please help? Best regards Wang shengjiu

2 1

[PATCH] ASoC: wm8350: Handle error for wm8350_register_irq
by Jiasheng Jiang 03 Mar '22

03 Mar '22

As the potential failure of the wm8350_register_irq(), it should be better to check it and return error if fails. Also, use 'free_' in order to avoid the same code. Fixes: a6ba2b2dabb5 ("ASoC: Implement WM8350 headphone jack detection") Signed-off-by: Jiasheng Jiang <jiasheng(a)iscas.ac.cn> --- sound/soc/codecs/wm8350.c | 30 +++++++++++++++++++++++++----- 1 file changed, 25 insertions(+), 5 deletions(-) diff --git a/sound/soc/codecs/wm8350.c b/sound/soc/codecs/wm8350.c index 15d42ce3b21d..0c70bbfbedb5 100644 --- a/sound/soc/codecs/wm8350.c +++ b/sound/soc/codecs/wm8350.c @@ -1483,7 +1483,7 @@ static int wm8350_component_probe(struct snd_soc_component *component) ret = devm_regulator_bulk_get(wm8350->dev, ARRAY_SIZE(priv->supplies), priv->supplies); if (ret != 0) - return ret; + goto err; /* Put the codec into reset if it wasn't already */ wm8350_clear_bits(wm8350, WM8350_POWER_MGMT_5, WM8350_CODEC_ENA); @@ -1537,18 +1537,38 @@ static int wm8350_component_probe(struct snd_soc_component *component) wm8350_clear_bits(wm8350, WM8350_JACK_DETECT, WM8350_JDL_ENA | WM8350_JDR_ENA); - wm8350_register_irq(wm8350, WM8350_IRQ_CODEC_JCK_DET_L, + ret = wm8350_register_irq(wm8350, WM8350_IRQ_CODEC_JCK_DET_L, wm8350_hpl_jack_handler, 0, "Left jack detect", priv); - wm8350_register_irq(wm8350, WM8350_IRQ_CODEC_JCK_DET_R, + if (ret != 0) + goto err; + + ret = wm8350_register_irq(wm8350, WM8350_IRQ_CODEC_JCK_DET_R, wm8350_hpr_jack_handler, 0, "Right jack detect", priv); - wm8350_register_irq(wm8350, WM8350_IRQ_CODEC_MICSCD, + if (ret != 0) + goto free_JCK_DET_L; + + ret = wm8350_register_irq(wm8350, WM8350_IRQ_CODEC_MICSCD, wm8350_mic_handler, 0, "Microphone short", priv); - wm8350_register_irq(wm8350, WM8350_IRQ_CODEC_MICD, + if (ret != 0) + goto free_JCK_DET_R; + + ret = wm8350_register_irq(wm8350, WM8350_IRQ_CODEC_MICD, wm8350_mic_handler, 0, "Microphone detect", priv); + if (ret != 0) + goto free_MICSCD; return 0; + +free_MICSCD: + wm8350_free_irq(wm8350, WM8350_IRQ_CODEC_MICSCD, priv); +free_JCK_DET_R: + wm8350_free_irq(wm8350, WM8350_IRQ_CODEC_JCK_DET_R, priv); +free_JCK_DET_L: + wm8350_free_irq(wm8350, WM8350_IRQ_CODEC_JCK_DET_L, priv); +err: + return ret; } static void wm8350_component_remove(struct snd_soc_component *component) -- 2.25.1

2 1

[PATCH] ASoC: hdac_hda: Avoid unexpected match when pcm_name is "Analog"
by Meng Tang 03 Mar '22

03 Mar '22

pcm name can be "Analog" and "Alt Analog", cpcm->name can be "Analog Codec DAI" and "Alt Analog Codec DAI". When pcm_name is "Analog", "Analog Codec DAI" and "Alt Analog Codec DAI" are both satisfy the 'if (strstr(cpcm->name, pcm_name))' condition, which may cause the returned cpcm to be "Alt Analog Codec DAI". Even if we get the pcm name by id, and "Analog Codec DAI" goes into the loop before "Alt Analog Codec DAI", but I still think we'd better have multiple insurances against unexpected return values. After, we can correctly return the expected result even if other relevant places are changed. Signed-off-by: Meng Tang <tangmeng(a)uniontech.com> --- sound/soc/codecs/hdac_hda.c | 7 ++++++- 1 file changed, 6 insertions(+), 1 deletion(-) diff --git a/sound/soc/codecs/hdac_hda.c b/sound/soc/codecs/hdac_hda.c index 667f3df239c7..a9f61c7e44ee 100644 --- a/sound/soc/codecs/hdac_hda.c +++ b/sound/soc/codecs/hdac_hda.c @@ -363,8 +363,13 @@ static struct hda_pcm *snd_soc_find_pcm_from_dai(struct hdac_hda_priv *hda_pvt, } list_for_each_entry(cpcm, &hcodec->pcm_list_head, list) { - if (strstr(cpcm->name, pcm_name)) + if (strstr(cpcm->name, pcm_name)) { + if (strcmp(pcm_name, "Analog") == 0) { + if (strstr(cpcm->name, "Alt Analog")) + continue; + } return cpcm; + } } dev_err(&hcodec->core.dev, "didn't find PCM for DAI %s\n", dai->name); -- 2.20.1

2 1