* [PATCH v2 1/3] ASoC: codecs: lpass-wsa-macro: Switch to PM clock framework for runtime PM
From: Ajay Kumar Nandam @ 2026-04-20 12:28 UTC (permalink / raw)
To: Srinivas Kandagatla, Liam Girdwood, Mark Brown, Jaroslav Kysela,
Takashi Iwai
Cc: linux-sound, linux-arm-msm, linux-kernel, mohammad.rafi.shaik,
ajay.nandam
In-Reply-To: <20260420122843.327171-1-ajay.nandam@oss.qualcomm.com>
Convert the LPASS WSA macro codec driver to use the PM clock framework
for runtime power management.
The driver now relies on pm_clk helpers and runtime PM instead of
manually enabling and disabling macro, dcodec, mclk, npl, and fsgen
clocks. Runtime suspend and resume handling is delegated to the PM
core via pm_clk_suspend() and pm_clk_resume(), while existing runtime
PM callbacks continue to manage regcache state.
This ensures clocks are enabled only when the WSA macro is active,
improves power efficiency on LPASS platforms supporting LPI/island
modes, and aligns the driver with common ASoC runtime PM patterns used
across Qualcomm LPASS codec drivers.
Signed-off-by: Ajay Kumar Nandam <ajay.nandam@oss.qualcomm.com>
---
sound/soc/codecs/lpass-wsa-macro.c | 113 ++++++++++-------------------
1 file changed, 38 insertions(+), 75 deletions(-)
diff --git a/sound/soc/codecs/lpass-wsa-macro.c b/sound/soc/codecs/lpass-wsa-macro.c
index b695c77c18ac..ded1cd8db831 100644
--- a/sound/soc/codecs/lpass-wsa-macro.c
+++ b/sound/soc/codecs/lpass-wsa-macro.c
@@ -14,6 +14,7 @@
#include <sound/soc-dapm.h>
#include <linux/pm_runtime.h>
#include <linux/of_platform.h>
+#include <linux/pm_clock.h>
#include <sound/tlv.h>
#include "lpass-macro-common.h"
@@ -2529,15 +2530,15 @@ static const struct snd_soc_dapm_route wsa_audio_map[] = {
static int wsa_swrm_clock(struct wsa_macro *wsa, bool enable)
{
struct regmap *regmap = wsa->regmap;
+ int ret;
- if (enable) {
- int ret;
+ ret = pm_runtime_get_sync(wsa->dev);
+ if (ret < 0) {
+ pm_runtime_put_noidle(wsa->dev);
+ return ret;
+ }
- ret = clk_prepare_enable(wsa->mclk);
- if (ret) {
- dev_err(wsa->dev, "failed to enable mclk\n");
- return ret;
- }
+ if (enable) {
wsa_macro_mclk_enable(wsa, true);
regmap_update_bits(regmap, CDC_WSA_CLK_RST_CTRL_SWR_CONTROL,
@@ -2548,9 +2549,9 @@ static int wsa_swrm_clock(struct wsa_macro *wsa, bool enable)
regmap_update_bits(regmap, CDC_WSA_CLK_RST_CTRL_SWR_CONTROL,
CDC_WSA_SWR_CLK_EN_MASK, 0);
wsa_macro_mclk_enable(wsa, false);
- clk_disable_unprepare(wsa->mclk);
}
+ pm_runtime_put_autosuspend(wsa->dev);
return 0;
}
@@ -2776,25 +2777,23 @@ static int wsa_macro_probe(struct platform_device *pdev)
clk_set_rate(wsa->mclk, WSA_MACRO_MCLK_FREQ);
clk_set_rate(wsa->npl, WSA_MACRO_MCLK_FREQ);
- ret = clk_prepare_enable(wsa->macro);
+ ret = devm_pm_clk_create(dev);
if (ret)
- goto err;
+ return ret;
- ret = clk_prepare_enable(wsa->dcodec);
- if (ret)
- goto err_dcodec;
+ ret = of_pm_clk_add_clks(dev);
+ if (ret < 0)
+ return ret;
- ret = clk_prepare_enable(wsa->mclk);
- if (ret)
- goto err_mclk;
+ pm_runtime_set_autosuspend_delay(dev, 100);
+ pm_runtime_use_autosuspend(dev);
+ pm_runtime_enable(dev);
- ret = clk_prepare_enable(wsa->npl);
- if (ret)
- goto err_npl;
- ret = clk_prepare_enable(wsa->fsgen);
- if (ret)
- goto err_fsgen;
+ ret = pm_runtime_resume_and_get(dev);
+ if (ret < 0) {
+ goto err_rpm_disable;
+ }
/* reset swr ip */
regmap_update_bits(wsa->regmap, CDC_WSA_CLK_RST_CTRL_SWR_CONTROL,
@@ -2811,44 +2810,25 @@ static int wsa_macro_probe(struct platform_device *pdev)
wsa_macro_dai,
ARRAY_SIZE(wsa_macro_dai));
if (ret)
- goto err_clkout;
-
- pm_runtime_set_autosuspend_delay(dev, 3000);
- pm_runtime_use_autosuspend(dev);
- pm_runtime_mark_last_busy(dev);
- pm_runtime_set_active(dev);
- pm_runtime_enable(dev);
+ goto err_rpm_put;
ret = wsa_macro_register_mclk_output(wsa);
if (ret)
- goto err_clkout;
+ goto err_rpm_put;
- return 0;
+ pm_runtime_put_autosuspend(dev);
-err_clkout:
- clk_disable_unprepare(wsa->fsgen);
-err_fsgen:
- clk_disable_unprepare(wsa->npl);
-err_npl:
- clk_disable_unprepare(wsa->mclk);
-err_mclk:
- clk_disable_unprepare(wsa->dcodec);
-err_dcodec:
- clk_disable_unprepare(wsa->macro);
-err:
+ return 0;
+err_rpm_put:
+ pm_runtime_put_noidle(dev);
+err_rpm_disable:
+ pm_runtime_disable(dev);
return ret;
-
}
static void wsa_macro_remove(struct platform_device *pdev)
{
- struct wsa_macro *wsa = dev_get_drvdata(&pdev->dev);
-
- clk_disable_unprepare(wsa->macro);
- clk_disable_unprepare(wsa->dcodec);
- clk_disable_unprepare(wsa->mclk);
- clk_disable_unprepare(wsa->npl);
- clk_disable_unprepare(wsa->fsgen);
+ pm_runtime_disable(&pdev->dev);
}
static int wsa_macro_runtime_suspend(struct device *dev)
@@ -2858,11 +2838,7 @@ static int wsa_macro_runtime_suspend(struct device *dev)
regcache_cache_only(wsa->regmap, true);
regcache_mark_dirty(wsa->regmap);
- clk_disable_unprepare(wsa->fsgen);
- clk_disable_unprepare(wsa->npl);
- clk_disable_unprepare(wsa->mclk);
-
- return 0;
+ return pm_clk_suspend(dev);
}
static int wsa_macro_runtime_resume(struct device *dev)
@@ -2870,34 +2846,21 @@ static int wsa_macro_runtime_resume(struct device *dev)
struct wsa_macro *wsa = dev_get_drvdata(dev);
int ret;
- ret = clk_prepare_enable(wsa->mclk);
+ ret = pm_clk_resume(dev);
if (ret) {
- dev_err(dev, "unable to prepare mclk\n");
+ regcache_cache_only(wsa->regmap, true);
return ret;
}
+ regcache_cache_only(wsa->regmap, false);
- ret = clk_prepare_enable(wsa->npl);
- if (ret) {
- dev_err(dev, "unable to prepare mclkx2\n");
- goto err_npl;
- }
-
- ret = clk_prepare_enable(wsa->fsgen);
+ ret = regcache_sync(wsa->regmap);
if (ret) {
- dev_err(dev, "unable to prepare fsgen\n");
- goto err_fsgen;
+ regcache_cache_only(wsa->regmap, true);
+ pm_clk_suspend(dev);
+ return ret;
}
- regcache_cache_only(wsa->regmap, false);
- regcache_sync(wsa->regmap);
-
return 0;
-err_fsgen:
- clk_disable_unprepare(wsa->npl);
-err_npl:
- clk_disable_unprepare(wsa->mclk);
-
- return ret;
}
static const struct dev_pm_ops wsa_macro_pm_ops = {
--
2.34.1
^ permalink raw reply related
* [PATCH v2 2/3] ASoC: codecs: lpass-va-macro: Switch to PM clock framework for runtime PM
From: Ajay Kumar Nandam @ 2026-04-20 12:28 UTC (permalink / raw)
To: Srinivas Kandagatla, Liam Girdwood, Mark Brown, Jaroslav Kysela,
Takashi Iwai
Cc: linux-sound, linux-arm-msm, linux-kernel, mohammad.rafi.shaik,
ajay.nandam
In-Reply-To: <20260420122843.327171-1-ajay.nandam@oss.qualcomm.com>
Convert the LPASS VA macro codec driver to use the PM clock framework
for runtime power management.
The driver now relies on pm_clk helpers and runtime PM instead of
manually enabling and disabling macro, dcodec, mclk, and npl clocks.
All clock control during runtime suspend and resume is delegated to
the PM core via pm_clk_suspend() and pm_clk_resume().
This change ensures clocks are only enabled when the VA macro is
active, improves power efficiency on LPASS platforms supporting
LPI/island modes, and aligns the driver with common ASoC runtime PM
patterns used across Qualcomm LPASS codec drivers.
Signed-off-by: Ajay Kumar Nandam <ajay.nandam@oss.qualcomm.com>
---
sound/soc/codecs/lpass-va-macro.c | 123 ++++++++++++++----------------
1 file changed, 58 insertions(+), 65 deletions(-)
diff --git a/sound/soc/codecs/lpass-va-macro.c b/sound/soc/codecs/lpass-va-macro.c
index 528d5b167ecf..949275f1dfac 100644
--- a/sound/soc/codecs/lpass-va-macro.c
+++ b/sound/soc/codecs/lpass-va-macro.c
@@ -11,6 +11,7 @@
#include <linux/of_platform.h>
#include <linux/platform_device.h>
#include <linux/pm_runtime.h>
+#include <linux/pm_clock.h>
#include <linux/regmap.h>
#include <linux/regulator/consumer.h>
#include <sound/soc.h>
@@ -1348,18 +1349,22 @@ static int fsgen_gate_enable(struct clk_hw *hw)
struct regmap *regmap = va->regmap;
int ret;
- if (va->has_swr_master) {
- ret = clk_prepare_enable(va->mclk);
- if (ret)
- return ret;
+ ret = pm_runtime_resume_and_get(va->dev);
+ if (ret < 0) {
+ pm_runtime_put_noidle(va->dev);
+ return ret;
}
ret = va_macro_mclk_enable(va, true);
+ if (ret) {
+ pm_runtime_put_noidle(va->dev);
+ return ret;
+ }
if (va->has_swr_master)
regmap_update_bits(regmap, CDC_VA_CLK_RST_CTRL_SWR_CONTROL,
CDC_VA_SWR_CLK_EN_MASK, CDC_VA_SWR_CLK_ENABLE);
- return ret;
+ return 0;
}
static void fsgen_gate_disable(struct clk_hw *hw)
@@ -1372,8 +1377,23 @@ static void fsgen_gate_disable(struct clk_hw *hw)
CDC_VA_SWR_CLK_EN_MASK, 0x0);
va_macro_mclk_enable(va, false);
- if (va->has_swr_master)
- clk_disable_unprepare(va->mclk);
+
+ pm_runtime_put_autosuspend(va->dev);
+}
+
+static int va_macro_setup_pm_clocks(struct device *dev, struct va_macro *va)
+{
+ int ret;
+
+ ret = devm_pm_clk_create(dev);
+ if (ret)
+ return ret;
+
+ ret = of_pm_clk_add_clks(dev);
+ if (ret < 0)
+ return ret;
+
+ return 0;
}
static int fsgen_gate_is_enabled(struct clk_hw *hw)
@@ -1534,6 +1554,7 @@ static int va_macro_probe(struct platform_device *pdev)
void __iomem *base;
u32 sample_rate = 0;
int ret;
+ int rpm_ret;
va = devm_kzalloc(dev, sizeof(*va), GFP_KERNEL);
if (!va)
@@ -1601,22 +1622,18 @@ static int va_macro_probe(struct platform_device *pdev)
clk_set_rate(va->npl, 2 * VA_MACRO_MCLK_FREQ);
}
- ret = clk_prepare_enable(va->macro);
- if (ret)
- goto err;
-
- ret = clk_prepare_enable(va->dcodec);
+ ret = va_macro_setup_pm_clocks(dev, va);
if (ret)
- goto err_dcodec;
+ goto err_rpm_disable;
- ret = clk_prepare_enable(va->mclk);
- if (ret)
- goto err_mclk;
+ pm_runtime_set_autosuspend_delay(dev, 100);
+ pm_runtime_use_autosuspend(dev);
+ pm_runtime_enable(dev);
- if (va->has_npl_clk) {
- ret = clk_prepare_enable(va->npl);
- if (ret)
- goto err_npl;
+ rpm_ret = pm_runtime_resume_and_get(dev);
+ if (rpm_ret < 0) {
+ ret = rpm_ret;
+ goto err_rpm_disable;
}
/**
@@ -1629,7 +1646,7 @@ static int va_macro_probe(struct platform_device *pdev)
/* read version from register */
ret = va_macro_set_lpass_codec_version(va);
if (ret)
- goto err_clkout;
+ goto err_rpm_put;
}
if (va->has_swr_master) {
@@ -1659,35 +1676,26 @@ static int va_macro_probe(struct platform_device *pdev)
va_macro_dais,
ARRAY_SIZE(va_macro_dais));
if (ret)
- goto err_clkout;
-
- pm_runtime_set_autosuspend_delay(dev, 3000);
- pm_runtime_use_autosuspend(dev);
- pm_runtime_mark_last_busy(dev);
- pm_runtime_set_active(dev);
- pm_runtime_enable(dev);
+ goto err_rpm_put;
ret = va_macro_register_fsgen_output(va);
if (ret)
- goto err_clkout;
+ goto err_rpm_put;
va->fsgen = devm_clk_hw_get_clk(dev, &va->hw, "fsgen");
if (IS_ERR(va->fsgen)) {
ret = PTR_ERR(va->fsgen);
- goto err_clkout;
+ goto err_rpm_put;
}
+ pm_runtime_put_autosuspend(dev);
+
return 0;
-err_clkout:
- if (va->has_npl_clk)
- clk_disable_unprepare(va->npl);
-err_npl:
- clk_disable_unprepare(va->mclk);
-err_mclk:
- clk_disable_unprepare(va->dcodec);
-err_dcodec:
- clk_disable_unprepare(va->macro);
+err_rpm_put:
+ pm_runtime_put_noidle(dev);
+err_rpm_disable:
+ pm_runtime_disable(dev);
err:
lpass_macro_pds_exit(va->pds);
@@ -1698,12 +1706,7 @@ static void va_macro_remove(struct platform_device *pdev)
{
struct va_macro *va = dev_get_drvdata(&pdev->dev);
- if (va->has_npl_clk)
- clk_disable_unprepare(va->npl);
-
- clk_disable_unprepare(va->mclk);
- clk_disable_unprepare(va->dcodec);
- clk_disable_unprepare(va->macro);
+ pm_runtime_disable(&pdev->dev);
lpass_macro_pds_exit(va->pds);
}
@@ -1715,12 +1718,7 @@ static int va_macro_runtime_suspend(struct device *dev)
regcache_cache_only(va->regmap, true);
regcache_mark_dirty(va->regmap);
- if (va->has_npl_clk)
- clk_disable_unprepare(va->npl);
-
- clk_disable_unprepare(va->mclk);
-
- return 0;
+ return pm_clk_suspend(dev);
}
static int va_macro_runtime_resume(struct device *dev)
@@ -1728,23 +1726,18 @@ static int va_macro_runtime_resume(struct device *dev)
struct va_macro *va = dev_get_drvdata(dev);
int ret;
- ret = clk_prepare_enable(va->mclk);
- if (ret) {
- dev_err(va->dev, "unable to prepare mclk\n");
+ ret = pm_clk_resume(dev);
+ if (ret)
return ret;
- }
-
- if (va->has_npl_clk) {
- ret = clk_prepare_enable(va->npl);
- if (ret) {
- clk_disable_unprepare(va->mclk);
- dev_err(va->dev, "unable to prepare npl\n");
- return ret;
- }
- }
regcache_cache_only(va->regmap, false);
- regcache_sync(va->regmap);
+
+ ret = regcache_sync(va->regmap);
+ if (ret) {
+ regcache_cache_only(va->regmap, true);
+ pm_clk_suspend(dev);
+ return ret;
+ }
return 0;
}
--
2.34.1
^ permalink raw reply related
* [PATCH v2 3/3] ASoC: codecs: lpass-wsa-macro: Use devm_clk_hw_register() for MCLK output
From: Ajay Kumar Nandam @ 2026-04-20 12:28 UTC (permalink / raw)
To: Srinivas Kandagatla, Liam Girdwood, Mark Brown, Jaroslav Kysela,
Takashi Iwai
Cc: linux-sound, linux-arm-msm, linux-kernel, mohammad.rafi.shaik,
ajay.nandam
In-Reply-To: <20260420122843.327171-1-ajay.nandam@oss.qualcomm.com>
The WSA macro driver registers the MCLK output clock using
clk_hw_register(), but does not explicitly unregister it in the remove
path or on probe failure.
Switch to devm_clk_hw_register() to make the registration resource-managed
so the clk_hw is automatically unregistered when the device is unbound or
probe fails. This avoids lifetime and cleanup issues and simplifies error
handling.
No functional change intended.
Signed-off-by: Ajay Kumar Nandam <ajay.nandam@oss.qualcomm.com>
---
sound/soc/codecs/lpass-wsa-macro.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/sound/soc/codecs/lpass-wsa-macro.c b/sound/soc/codecs/lpass-wsa-macro.c
index ded1cd8db831..29c8edfa739d 100644
--- a/sound/soc/codecs/lpass-wsa-macro.c
+++ b/sound/soc/codecs/lpass-wsa-macro.c
@@ -2657,7 +2657,7 @@ static int wsa_macro_register_mclk_output(struct wsa_macro *wsa)
init.num_parents = 1;
wsa->hw.init = &init;
hw = &wsa->hw;
- ret = clk_hw_register(wsa->dev, hw);
+ ret = devm_clk_hw_register(wsa->dev, hw);
if (ret)
return ret;
--
2.34.1
^ permalink raw reply related
* sashiko.dev review (Re: [PATCH v4 01/17] dmaengine: sh: rz-dmac: Move interrupt request after everything is set up)
From: Claudiu Beznea @ 2026-04-20 12:33 UTC (permalink / raw)
To: vkoul, Frank.Li, lgirdwood, broonie, perex, tiwai, biju.das.jz,
prabhakar.mahadev-lad.rj, p.zabel, geert+renesas,
fabrizio.castro.jz, long.luu.ur
Cc: dmaengine, linux-kernel, linux-sound, linux-renesas-soc,
Claudiu Beznea, stable
In-Reply-To: <20260411114303.2814115-2-claudiu.beznea.uj@bp.renesas.com>
> commit b3dfdd5fbd2463db407b1396ee1ae79801cb96bf
> Author: Claudiu Beznea <claudiu.beznea.uj@bp.renesas.com>
> dmaengine: sh: rz-dmac: Move interrupt request after everything is set up
> This patch moves the channel interrupt request to the end of the channel
> probe function to prevent the IRQ handler from accessing uninitialized
> data. It also replaces dma_alloc_coherent() with dmam_alloc_coherent()
> to simplify the error path.
> > diff --git a/drivers/dma/sh/rz-dmac.c b/drivers/dma/sh/rz-dmac.c
> > index 625ff29024de0..9f206a33dcc6b 100644
> > --- a/drivers/dma/sh/rz-dmac.c
> > +++ b/drivers/dma/sh/rz-dmac.c
> [ ... ]
> > @@ -1210,16 +1207,6 @@ static int rz_dmac_probe(struct platform_device *pdev)
> > dma_register_err:
> > of_dma_controller_free(pdev->dev.of_node);
> > err:
> > - channel_num = i ? i - 1 : 0;
> > - for (i = 0; i < channel_num; i++) {
> > - struct rz_dmac_chan *channel = &dmac->channels[i];
> > -
> > - dma_free_coherent(&pdev->dev,
> > - sizeof(struct rz_lmdesc) * DMAC_NR_LMDESC,
> > - channel->lmdesc.base,
> > - channel->lmdesc.base_dma);
> > - }
> > -
> > reset_control_assert(dmac->rstc);
> > err_pm_runtime_put:
> > pm_runtime_put(&pdev->dev);
> > @@ -1232,18 +1219,9 @@ static int rz_dmac_probe(struct platform_device *pdev)
> > static void rz_dmac_remove(struct platform_device *pdev)
> > {
> > struct rz_dmac *dmac = platform_get_drvdata(pdev);
> > - unsigned int i;
> >
> > dma_async_device_unregister(&dmac->engine);
> > of_dma_controller_free(pdev->dev.of_node);
> > - for (i = 0; i < dmac->n_channels; i++) {
> > - struct rz_dmac_chan *channel = &dmac->channels[i];
> > -
> > - dma_free_coherent(&pdev->dev,
> > - sizeof(struct rz_lmdesc) * DMAC_NR_LMDESC,
> > - channel->lmdesc.base,
> > - channel->lmdesc.base_dma);
> > - }
> > reset_control_assert(dmac->rstc);
> > pm_runtime_put(&pdev->dev);
> > pm_runtime_disable(&pdev->dev);
> > }
> While this patch fixes the initialization races, does it leave a similar
> vulnerability exposed during teardown?
> Since devm_request_threaded_irq() is used to allocate the interrupts, they
> will remain active until after rz_dmac_remove() and the rz_dmac_probe() error
> paths complete.
> If an interrupt fires during or just after rz_dmac_remove(), could the handler
> attempt to access hardware registers while the device is in reset or powered
> down by pm_runtime_put()?
> Would it be safer to explicitly free or disable the IRQs before asserting the
> hardware reset and disabling runtime PM, or perhaps manage the reset and PM
> states via devm actions to guarantee correct teardown ordering?
On either failure or remove path the device is with runtime PM put (clocks being
disabled) and in reset state. The controller cannot generate interrupts from
this state.
However, to keep this series simple, I'll add this in a cleanup patch after the
current series will be merged.
^ permalink raw reply
* sashiko.dev review (Re: [PATCH v4 04/17] dmaengine: sh: rz-dmac: Use rz_dmac_disable_hw())
From: Claudiu Beznea @ 2026-04-20 12:34 UTC (permalink / raw)
To: vkoul, Frank.Li, lgirdwood, broonie, perex, tiwai, biju.das.jz,
prabhakar.mahadev-lad.rj, p.zabel, geert+renesas,
fabrizio.castro.jz, long.luu.ur
Cc: dmaengine, linux-kernel, linux-sound, linux-renesas-soc,
Claudiu Beznea
In-Reply-To: <20260411114303.2814115-5-claudiu.beznea.uj@bp.renesas.com>
> commit f7b3ac8bceb85f09d10e2c6b3d49ae768415c77c
> Author: Claudiu Beznea <claudiu.beznea.uj@bp.renesas.com>
> dmaengine: sh: rz-dmac: Use rz_dmac_disable_hw()
> This patch unifies the hardware disable code by replacing an open-coded
> register write with rz_dmac_disable_hw(), moving it after vchan_init()
> so the channel device is properly initialized.
> > diff --git a/drivers/dma/sh/rz-dmac.c b/drivers/dma/sh/rz-dmac.c
> > index 1717b407ab9e9..40ddf534c0948 100644
> > --- a/drivers/dma/sh/rz-dmac.c
> > +++ b/drivers/dma/sh/rz-dmac.c
> [ ... ]
> > @@ -1000,15 +1000,15 @@ static int rz_dmac_chan_probe(struct rz_dmac *dmac,
> > }
> > rz_lmdesc_setup(channel, lmdesc);
> >
> > - /* Initialize register for each channel */
> > - rz_dmac_ch_writel(channel, CHCTRL_DEFAULT, CHCTRL, 1);
> > -
> > channel->vc.desc_free = rz_dmac_virt_desc_free;
> > vchan_init(&channel->vc, &dmac->engine);
> This is not a bug introduced by this patch, but seeing the vchan
> initialization here made me wonder about the driver teardown path.
> When vchan_init() is called, it sets up a background tasklet for the
> channel. During driver removal in rz_dmac_remove(), the DMA device is
> unregistered, but it does not appear that tasklet_kill() is ever called
> for this tasklet.
> Because the channel structures are allocated with devm_kzalloc, their
> memory is automatically freed immediately after rz_dmac_remove() returns.
> If a DMA transfer is active when the driver is forcefully unbound, could
> the IRQ thread schedule the tasklet via vchan_cookie_complete() right
> before the IRQ is freed by devres?
> Since free_irq() waits for the IRQ thread to finish but does not flush
> tasklets, could this scheduled tasklet execute after the device memory
> has been freed, resulting in a use-after-free?
> Should the driver explicitly kill the tasklet before returning from
> rz_dmac_remove()?
> > INIT_LIST_HEAD(&channel->ld_queue);
> > INIT_LIST_HEAD(&channel->ld_free);
> > INIT_LIST_HEAD(&channel->ld_active);
tasklet_setup() only sets the members of a struct tasklet object. The tasklet
itself is scheduled at runtime when the DMA clients requests transfers. On
remove the DMA clients would have already been called the
rz_dmac_terminate_all() and/or rz_dmac_device_synchronize() which should kill
the previously scheduled tasklet.
^ permalink raw reply
* sashiko.dev review (Re: [PATCH v4 07/17] dmaengine: sh: rz-dmac: Save the start LM descriptor)
From: Claudiu Beznea @ 2026-04-20 12:37 UTC (permalink / raw)
To: vkoul, Frank.Li, lgirdwood, broonie, perex, tiwai, biju.das.jz,
prabhakar.mahadev-lad.rj, p.zabel, geert+renesas,
fabrizio.castro.jz, long.luu.ur
Cc: dmaengine, linux-kernel, linux-sound, linux-renesas-soc,
Claudiu Beznea
In-Reply-To: <20260411114303.2814115-8-claudiu.beznea.uj@bp.renesas.com>
> commit 3d17698f275248947e3d2df42293a89e89cafaa6
> Author: Claudiu Beznea <claudiu.beznea.uj@bp.renesas.com>
> dmaengine: sh: rz-dmac: Save the start LM descriptor
> Save the start LM descriptor to avoid looping through the entire
> channel's LM descriptor list when computing the residue. This avoids
> unnecessary iterations.
> > diff --git a/drivers/dma/sh/rz-dmac.c b/drivers/dma/sh/rz-dmac.c
> > index 6bea7c8c70537..0f871c0a28bd9 100644
> > --- a/drivers/dma/sh/rz-dmac.c
> > +++ b/drivers/dma/sh/rz-dmac.c
> [ ... ]
> > @@ -377,6 +380,7 @@ static void rz_dmac_prepare_descs_for_slave_sg(struct
rz_dmac_chan *channel)
> > }
> >
> > lmdesc = channel->lmdesc.tail;
> > + d->start_lmdesc = lmdesc;
> >
> > for (i = 0, sg = sgl; i < sg_len; i++, sg = sg_next(sg)) {
> > if (d->direction == DMA_DEV_TO_MEM) {
> This isn't a bug introduced by this patch, but is there a potential buffer
> overflow in the link descriptor ring due to a lack of bounds checking?
That can happen but this is not the scope of this series. Series is already long
enough. I will address this in a separate series after the current one will be
merged.
> The driver allocates a fixed-size ring buffer of 64 link descriptors
> (DMAC_NR_LMDESC). However, rz_dmac_prepare_descs_for_slave_sg() iterates over
> the provided scatterlist and increments the lmdesc pointer without ever
> verifying if the number of scatterlist elements exceeds 64.
> If a client submits a sufficiently large scatterlist, will it silently
> overflow the ring buffer, overwriting the descriptors it just wrote, and
> cause the DMA controller to read a corrupted list of operations?
> [ ... ]
> > @@ -693,9 +697,10 @@ rz_dmac_get_next_lmdesc(struct rz_lmdesc *base, struct
rz_lmdesc *lmdesc)
> > return next;
> > }
> >
> > -static u32 rz_dmac_calculate_residue_bytes_in_vd(struct rz_dmac_chan
*channel, u32 crla)
> > +static u32 rz_dmac_calculate_residue_bytes_in_vd(struct rz_dmac_chan *channel,
> > + struct rz_dmac_desc *desc, u32
crla)
> > {
> > - struct rz_lmdesc *lmdesc = channel->lmdesc.head;
> > + struct rz_lmdesc *lmdesc = desc->start_lmdesc;
> > struct dma_chan *chan = &channel->vc.chan;
> > struct rz_dmac *dmac = to_rz_dmac(chan->device);
> > u32 residue = 0, i = 0;
> Can this lead to a NULL pointer dereference?
> In rz_dmac_prep_slave_sg(), a new descriptor is immediately added to
> channel->ld_queue. If a previous transfer completes before the client calls
> dma_async_issue_pending(), rz_dmac_irq_handler_thread() attempts to start the
> next transfer from ld_queue.
> However, vchan_next_desc() returns NULL since the transfer hasn't been
> issued, causing rz_dmac_xfer_desc() to return 0 without executing the hardware
> preparation function. The IRQ handler sees the 0 return value and erroneously
> moves the unprepared descriptor to channel->ld_active.
> If dmaengine_tx_status() is subsequently called, rz_dmac_chan_get_residue()
> fetches this active descriptor. Because it was never prepared, the newly added
> desc->start_lmdesc pointer remains NULL.
> The loop while (lmdesc->nxla != crla) then immediately dereferences NULL.
> Note that this issue was fixed later in the series by commit 'dmaengine: sh:
> rz-dmac: Use virt-dma APIs for channel descriptor processing'.
All the above could happen and the scope of patch "dmaengine: sh:
rz-dmac: Use virt-dma APIs for channel descriptor processing" was to fix all
these subtle issues. The patch here is only a preparation patch to keep things
simple in "dmaengine: sh: rz-dmac: Use virt-dma APIs for channel descriptor
processing"
> Additionally, this isn't a bug introduced by this patch, but does the
> mathematical logic in the residue calculation produce incorrect results?
> The loop while (lmdesc->nxla != crla) in
> rz_dmac_calculate_residue_bytes_in_vd() locates the link descriptor preceding
> the currently executing one. The subsequent loop while (lmdesc->chcfg &
> CHCFG_DEM) then iterates from this preceding descriptor, adding lmdesc->tb to
> the residue.
> If the hardware is executing the second descriptor of a transfer, it adds the
> total size of the first (already completed) descriptor, the total size of the
> second (currently executing) descriptor, and crtb (the remaining bytes of the
> second descriptor). Does this mathematically double-count bytes and include
> already transferred data?
> Conversely, if executing the first descriptor, the first loop wraps around the
> entire ring buffer, encounters the final descriptor of the previous transfer,
> and since its CHCFG_DEM bit is cleared, the second loop immediately
> terminates. Does this completely omit all subsequent descriptors from the
> residue calculation?
I think this is a valid point that I also noticed and thought about it. However,
I would like to keep it aside from this series, as it is already too big.
> [ ... ]
> This isn't a bug introduced by this patch, but is there a pre-existing race
> condition with dma_async_issue_pending()?
> rz_dmac_issue_pending() unconditionally calls rz_dmac_xfer_desc() if
> vchan_issue_pending() returns true, without checking if a transfer is already
> actively executing in hardware (e.g., if channel->desc is already set).
> For RZ_DMAC_DESC_MEMCPY transfers, the driver does not advance
> channel->lmdesc.tail, meaning the newly issued transfer will silently
> overwrite the exact link descriptor currently being executed by the active
> hardware. Does this lead to memory corruption and undefined hardware behavior?
> Note that this is fixed later in the series by commit 'dmaengine: sh: rz-dmac:
> Use virt-dma APIs for channel descriptor processing' which properly checks
> !channel->desc.
The role of the pointed descriptors was to fix all these subtle issues.
^ permalink raw reply
* sashiko.dev review (Re: [PATCH v4 10/17] dmaengine: sh: rz-dmac: Use virt-dma APIs for channel descriptor processing)
From: Claudiu Beznea @ 2026-04-20 12:41 UTC (permalink / raw)
To: vkoul, Frank.Li, lgirdwood, broonie, perex, tiwai, biju.das.jz,
prabhakar.mahadev-lad.rj, p.zabel, geert+renesas,
fabrizio.castro.jz, long.luu.ur
Cc: dmaengine, linux-kernel, linux-sound, linux-renesas-soc,
Claudiu Beznea
In-Reply-To: <20260411114303.2814115-11-claudiu.beznea.uj@bp.renesas.com>
> commit c2a00d64377d19d3ae2ce858af0fab6e9fae6f1a
> Author: Claudiu Beznea <claudiu.beznea.uj@bp.renesas.com>
> Subject: dmaengine: sh: rz-dmac: Use virt-dma APIs for channel descriptor
processing
> This commit converts the rz-dmac driver to fully use virt-dma APIs for
> descriptor processing. It removes custom queue management (ld_queue,
> ld_active), keeping only ld_free to track pre-allocated descriptors. It also
> simplifies residue calculation and synchronization mechanisms.
> > diff --git a/drivers/dma/sh/rz-dmac.c b/drivers/dma/sh/rz-dmac.c
> > index f35ff57397189..04eb1a7f1e627 100644
> > --- a/drivers/dma/sh/rz-dmac.c
> > +++ b/drivers/dma/sh/rz-dmac.c
> [ ... ]
> > @@ -494,8 +485,6 @@ static void rz_dmac_free_chan_resources(struct dma_chan
*chan)
> > rz_lmdesc_setup(channel, channel->lmdesc.base);
> >
> > rz_dmac_disable_hw(channel);
> > - list_splice_tail_init(&channel->ld_active, &channel->ld_free);
> > - list_splice_tail_init(&channel->ld_queue, &channel->ld_free);
> >
> > if (channel->mid_rid >= 0) {
> > clear_bit(channel->mid_rid, dmac->modules);
> > @@ -504,13 +493,19 @@ static void rz_dmac_free_chan_resources(struct
dma_chan *chan)
> >
> > spin_unlock_irqrestore(&channel->vc.lock, flags);
> >
> > + vchan_free_chan_resources(&channel->vc);
> > +
> > + spin_lock_irqsave(&channel->vc.lock, flags);
> Can this result in a memory leak and use-after-free regression?
> The patch removes the old list splicing but does not clear channel->desc.
> If rz_dmac_free_chan_resources() is called without a prior terminate,
> channel->desc is not returned to ld_free or freed by
> vchan_free_chan_resources(), leading to a memory leak of the descriptor.
AFAIK, it is expected hat device_terminate_all() is called before before
device_free_chan_resources().
Vinod, Frank,
Could you please let me know if my assumption is wrong?
Thank you!
> Additionally, the hardware is disabled and the lock is temporarily dropped
> before calling vchan_free_chan_resources(). During this unlocked window, the
> rz_dmac_irq_handler_thread could execute since channel->desc is not NULL.
> This would complete the descriptor, call rz_dmac_xfer_desc(), fetch the next
> descriptor, and restart the hardware while the channel is being destroyed.
If device_terminate_all() is called before before device_free_chan_resources()
this shouldn't happen. channel->desc is set to null in device_terminate_all().
> Should vchan_terminate_vdesc() be called to clear channel->desc under the
> lock before dropping it?
This is already done in device_terminate_all().
> [ ... ]
> > @@ -558,22 +553,22 @@ rz_dmac_prep_slave_sg(struct dma_chan *chan, struct
scatterlist *sgl,
> > if (!desc)
> > return NULL;
> >
> > - for_each_sg(sgl, sg, sg_len, i)
> > - dma_length += sg_dma_len(sg);
> > + list_del(&desc->node);
> > + }
> >
> > - desc->type = RZ_DMAC_DESC_SLAVE_SG;
> > - desc->sg = sgl;
> > - desc->sgcount = sg_len;
> This issue is not introduced by this commit, but could this assignment cause a
> buffer overflow?
> The variable sg_len is assigned to desc->sgcount without validating that it is
> less than or equal to the hardware limit (DMAC_NR_LMDESC, which is 64). Later
> in rz_dmac_prepare_descs_for_slave_sg(), the code iterates sgcount times to
> populate the hardware descriptor ring. If an SG list with more than 64 entries
> is provided, the loop will wrap around and overwrite the beginning of the DMA
> ring, corrupting the DMA descriptors.
As mentioned in a previous review comment, this can happen but, as it is not
introduced by this series, I'll prepare a cleanup patch later, after this series
will be integrated, as it is already big enough.
> [ ... ]
> > @@ -692,6 +681,8 @@ static void rz_dmac_device_synchronize(struct dma_chan
*chan)
> > u32 chstat;
> > int ret;
> >
> > + vchan_synchronize(&channel->vc);
> > +
> > ret = read_poll_timeout(rz_dmac_ch_readl, chstat, !(chstat & CHSTAT_EN),
> > 100, 100000, false, channel, CHSTAT, 1);
> Could calling vchan_synchronize() before polling the hardware lead to a
> use-after-free?
> If the hardware is actively processing a transfer, vchan_synchronize() will
> return immediately because no completion tasklet has been scheduled yet.
> The hardware is then polled until it finishes, at which point it asserts an
> IRQ and schedules the virt-dma tasklet.
> However, the caller of rz_dmac_device_synchronize() would have already
> resumed assuming all activity is finished, potentially freeing the transfer
> context before the delayed tasklet runs.
> Should vchan_synchronize() be called after read_poll_timeout() ensures the
> hardware has actually stopped?
AFAIK, it is expected at this point that the user alredy called
device_terminate_all(). That frees the active transfer and sets channel->desc =
NULL which don't allow scheduling any tasklet.
Vinod, Frank,
Could you please let me know if I'm wrong here?
> [ ... ]
> > @@ -739,58 +730,22 @@ static u32
rz_dmac_calculate_residue_bytes_in_vd(struct rz_dmac_chan *channel,
> > static u32 rz_dmac_chan_get_residue(struct rz_dmac_chan *channel,
> > dma_cookie_t cookie)
> > {
> > - struct rz_dmac_desc *current_desc, *desc;
> > - enum dma_status status;
> > + struct rz_dmac_desc *desc = NULL;
> > + struct virt_dma_desc *vd;
> > u32 crla, crtb, i;
> >
> > - /* Get current processing virtual descriptor */
> > - current_desc = list_first_entry_or_null(&channel->ld_active,
> > - struct rz_dmac_desc, node);
> > - if (!current_desc)
> > - return 0;
> [ ... ]
> > + vd = vchan_find_desc(&channel->vc, cookie);
> > + if (vd) {
> > + /* Descriptor has been issued but not yet processed. */
> > + desc = to_rz_dmac_desc(vd);
> > + return desc->len;
> > + } else if (channel->desc && channel->desc->vd.tx.cookie == cookie) {
> > + /* Descriptor is currently processed. */
> > + desc = channel->desc;
> > + }
> >
> > - /*
> > - * No descriptor found for the cookie, there's thus no residue.
> > - * This shouldn't happen if the calling driver passes a correct
> > - * cookie value.
> > - */
> > - WARN(1, "No descriptor for cookie!");
> > + if (!desc) {
> > + /* Descriptor was not found. May be already completed by now. */
> > return 0;
> > }
> Does this logic correctly handle descriptors that have been submitted but not
> yet issued?
> The vchan_find_desc() function only searches the desc_issued list. If a client
> has submitted a transfer using vchan_tx_submit() but has not yet called
> dma_async_issue_pending(), the descriptor will be in the desc_submitted list.
> As a result, vd will be NULL, and the function will fall through to return
> 0 bytes instead of the descriptor's true length. This changes the behavior
> from the previous implementation and may break clients that rely on
> dmaengine_tx_status() to check the progress of submitted transfers.
Investigating the already existing drivers using the pattern proposed in this
set found that this is the way the checked drivers are implemeinting the
tx_status API. Based on that, I've adjusted it like this.
Vinod, Frank,
Could you please let me know what is expected here?
Thank you!
> [ ... ]
> > @@ -914,28 +865,18 @@ static irqreturn_t rz_dmac_irq_handler(int irq, void
*dev_id)
> > static irqreturn_t rz_dmac_irq_handler_thread(int irq, void *dev_id)
> > {
> > struct rz_dmac_chan *channel = dev_id;
> > - struct rz_dmac_desc *desc = NULL;
> > - unsigned long flags;
> > + struct rz_dmac_desc *desc;
> >
> > - spin_lock_irqsave(&channel->vc.lock, flags);
> > + guard(spinlock_irqsave)(&channel->vc.lock);
> >
> > - if (list_empty(&channel->ld_active)) {
> > - /* Someone might have called terminate all */
> > - goto out;
> > - }
> > + desc = channel->desc;
> > + if (!desc)
> > + return IRQ_HANDLED;
> >
> > - desc = list_first_entry(&channel->ld_active, struct rz_dmac_desc, node);
> > vchan_cookie_complete(&desc->vd);
> > - list_move_tail(channel->ld_active.next, &channel->ld_free);
> This issue is not introduced by this commit, but can a race condition here
> lead to premature completion of new transfers?
> If a transfer completes and asserts the hardirq, the threaded handler is
> scheduled. Before the thread runs, the client could call
> dmaengine_terminate_all() and immediately issue a new transfer. The
> rz_dmac_issue_pending() function would start the new transfer and assign it
> to channel->desc.
> When the delayed IRQ thread finally runs, it takes the lock and sees
> channel->desc pointing to the new transfer. It then unconditionally completes
> it via vchan_cookie_complete(). This could result in a transfer being
> prematurely reported as complete while it is still running.
I think this can happen in theory, but, as it is a behaviour not introduced by
this commit I will keep the series as is.
^ permalink raw reply
* sashiko.dev review (Re: [PATCH v4 11/17] dmaengine: sh: rz-dmac: Refactor pause/resume code)
From: Claudiu Beznea @ 2026-04-20 12:42 UTC (permalink / raw)
To: vkoul, Frank.Li, lgirdwood, broonie, perex, tiwai, biju.das.jz,
prabhakar.mahadev-lad.rj, p.zabel, geert+renesas,
fabrizio.castro.jz, long.luu.ur
Cc: dmaengine, linux-kernel, linux-sound, linux-renesas-soc,
Claudiu Beznea
In-Reply-To: <20260411114303.2814115-12-claudiu.beznea.uj@bp.renesas.com>
> commit c08d50f3400ecf3dbc032455be7a589baf899258
> Author: Claudiu Beznea <claudiu.beznea.uj@bp.renesas.com>
> dmaengine: sh: rz-dmac: Refactor pause/resume code
> This commit refactors the pause/resume support to use a bitmask for the
> channel status. This prepares the driver for upcoming system suspend and
> cyclic DMA support where multiple independent pause states might be active.
> > diff --git a/drivers/dma/sh/rz-dmac.c b/drivers/dma/sh/rz-dmac.c
> > index 04eb1a7f1e627..d009b7607d447 100644
> > --- a/drivers/dma/sh/rz-dmac.c
> > +++ b/drivers/dma/sh/rz-dmac.c
> [ ... ]
> > +static int rz_dmac_device_resume_set(struct rz_dmac_chan *channel,
> > + unsigned long clear_bitmask)
> > +{
> > + int ret = 0;
> > + u32 val;
> > +
> > + lockdep_assert_held(&channel->vc.lock);
> > +
> > + /* Do not check CHSTAT_SUS but rely on HW capabilities. */
> > +
> > + rz_dmac_ch_writel(channel, CHCTRL_CLRSUS, CHCTRL, 1);
> > + ret = read_poll_timeout_atomic(rz_dmac_ch_readl, val,
> > + !(val & CHSTAT_SUS), 1, 1024, false,
> > + channel, CHSTAT, 1);
> > +
> > + channel->status &= ~clear_bitmask;
> > +
> > + return ret;
> > +}
> If multiple pause bits are set in channel->status, will this unconditionally
> resume the hardware?
> If a channel is paused explicitly by a dmaengine client using
> RZ_DMAC_CHAN_STATUS_PAUSED, and is subsequently paused by the upcoming system
> suspend functionality,
The upcomming system suspend functionality don't call rz_dmac_device_pause_set()
if RZ_DMAC_CHAN_STATUS_PAUSED is set or the channel is not enabled(), it just
return. The following could be checked:
static int rz_dmac_device_pause_internal(struct rz_dmac_chan *channel)
{
lockdep_assert_held(&channel->vc.lock);
/* Skip channels explicitly paused by consummers or disabled. */
if (channel->status & BIT(RZ_DMAC_CHAN_STATUS_PAUSED) ||
!rz_dmac_chan_is_enabled(channel))
return 0;
return rz_dmac_device_pause_set(channel, BIT(RZ_DMAC_CHAN_STATUS_PAUSED_INTERNAL));
}
> it appears this code will write CHCTRL_CLRSUS to the
> hardware as soon as the first of those conditions is cleared.
> This would silently override the other pause request, causing the hardware to
> resume prematurely while another client expects it to be paused.
> Should this update channel->status first, and only write CHCTRL_CLRSUS
> to the hardware if no other pause bits remain active (channel->status == 0)?
^ permalink raw reply
* Re: [PATCH] ASoC: fsl_micfil: Add DC output remover control
From: Mark Brown @ 2026-04-20 12:50 UTC (permalink / raw)
To: Shengjiu Wang
Cc: shengjiu.wang, Xiubo.Lee, festevam, nicoleotsuka, lgirdwood,
perex, tiwai, linux-sound, linuxppc-dev, linux-kernel
In-Reply-To: <20260420085344.2081805-1-shengjiu.wang@nxp.com>
[-- Attachment #1: Type: text/plain, Size: 733 bytes --]
On Mon, Apr 20, 2026 at 04:53:44PM +0800, Shengjiu Wang wrote:
> +static int micfil_put_dc_out_remover_state(struct snd_kcontrol *kcontrol,
> + struct snd_ctl_elem_value *ucontrol)
> +{
> + if (val < 0 || val > 3)
> + return -EINVAL;
> +
> + ret = pm_runtime_resume_and_get(comp->dev);
> + if (ret)
> + return ret;
> +
> + micfil->dc_out_remover = val;
...
> + /* Update DC Remover mode for all channels */
> + ret = snd_soc_component_update_bits(comp, REG_MICFIL_DC_OUT_CTRL,
> + MICFIL_DC_CTRL_CONFIG, reg_val);
> +
> + pm_runtime_put_autosuspend(comp->dev);
> +
> + return ret;
This will return 0 not 1 when the value changes, meaning event
generation is missed. The mixer-test selftest should report this.
[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 488 bytes --]
^ permalink raw reply
* sashiko.dev review (Re: [PATCH v4 13/17] dmaengine: sh: rz-dmac: Add cyclic DMA support)
From: Claudiu Beznea @ 2026-04-20 12:51 UTC (permalink / raw)
To: vkoul, Frank.Li, lgirdwood, broonie, perex, tiwai, biju.das.jz,
prabhakar.mahadev-lad.rj, p.zabel, geert+renesas,
fabrizio.castro.jz, long.luu.ur
Cc: dmaengine, linux-kernel, linux-sound, linux-renesas-soc,
Claudiu Beznea
In-Reply-To: <20260411114303.2814115-14-claudiu.beznea.uj@bp.renesas.com>
> commit cb66d54ec04fe2598560135c66410142d7295633
> Author: Claudiu Beznea <claudiu.beznea.uj@bp.renesas.com>
> dmaengine: sh: rz-dmac: Add cyclic DMA support
> This commit introduces cyclic DMA support to the RZ DMAC driver. It adds a
> per-channel status bit to mark cyclic channels and sets it during the DMA
> prepare callback. The IRQ handler then uses this bit to determine whether
> to invoke the cyclic callback function.
> > diff --git a/drivers/dma/sh/rz-dmac.c b/drivers/dma/sh/rz-dmac.c
> > index 958ee45abc700..9a10430109e59 100644
> > --- a/drivers/dma/sh/rz-dmac.c
> > +++ b/drivers/dma/sh/rz-dmac.c
> [ ... ]
> > @@ -586,6 +645,55 @@ rz_dmac_prep_slave_sg(struct dma_chan *chan, struct
scatterlist *sgl,
> [ ... ]
> > + scoped_guard(spinlock_irqsave, &channel->vc.lock) {
> > + if (channel->status & BIT(RZ_DMAC_CHAN_STATUS_CYCLIC))
> > + return NULL;
> > +
> > + desc = list_first_entry_or_null(&channel->ld_free, struct
rz_dmac_desc, node);
> > + if (!desc)
> > + return NULL;
> > +
> > + list_del(&desc->node);
> > +
> > + channel->status |= BIT(RZ_DMAC_CHAN_STATUS_CYCLIC);
> > + }
> > +
> > + desc->type = RZ_DMAC_DESC_CYCLIC;
> > + desc->sgcount = period_len;
> Can tracking cyclic state via the global channel->status flag lead to
> cross-descriptor state corruption?
> If a prepared cyclic descriptor is freed via dmaengine_desc_free(), the
> global flag might not be cleared during virt-dma desc release, leaving
> the channel poisoned.
dmaengine_desc_free() works only for reusable descriptors. This functionality is
not implemented by the rz-dmac driver.
> Also, because the lock is dropped when exiting the scoped_guard before
> vchan_tx_prep() is called, a concurrent terminate_all() could potentially
> clear the flag before the descriptor is queued.
> Could the driver rely on the desc->type field instead of a global state
> flag to identify cyclic transfers?
I'm going to explore that but I think this will complicates the code further.
Vinod, Frank,
Could you please let me know if this is a valid expected usecase?
> [ ... ]
> > @@ -733,9 +841,18 @@ static u32
rz_dmac_calculate_residue_bytes_in_vd(struct rz_dmac_chan *channel,
> > }
> >
> > /* Calculate residue from next lmdesc to end of virtual desc */
> > - while (lmdesc->chcfg & CHCFG_DEM) {
> > - residue += lmdesc->tb;
> > - lmdesc = rz_dmac_get_next_lmdesc(channel->lmdesc.base, lmdesc);
> > + if (channel->status & BIT(RZ_DMAC_CHAN_STATUS_CYCLIC)) {
> > + u32 start_lmdesc_addr = rz_dmac_lmdesc_addr(channel,
desc->start_lmdesc);
> > +
> > + while (lmdesc->nxla != start_lmdesc_addr) {
> > + residue += lmdesc->tb;
> > + lmdesc = rz_dmac_get_next_lmdesc(channel->lmdesc.base,
lmdesc);
> > + }
> Will this loop calculate a monotonically decreasing residue for cyclic
> transfers?
Yes, according to my testing.
> ALSA requires the residue to be the exact, monotonically decreasing number
> of bytes remaining in the buffer. Iterating until the end of the descriptor
> ring and adding the lengths of subsequent periods might cause the returned
> residue to jump wildly depending on which period is currently executing.
> For example, in a 3-period buffer (A, B, C), executing period B might
> return a much larger residue than executing period A, since it would add
> the remaining bytes in the ring differently.
> Could this non-monotonic behavior trigger 'PCM position is invalid'
> errors in the ALSA framework, breaking audio playback?
Never see this in my testing.
> [ ... ]
> > @@ -924,10 +1041,14 @@ static irqreturn_t rz_dmac_irq_handler_thread(int
irq, void *dev_id)
> > if (!desc)
> > return IRQ_HANDLED;
> >
> > - vchan_cookie_complete(&desc->vd);
> > - channel->desc = NULL;
> > + if (channel->status & BIT(RZ_DMAC_CHAN_STATUS_CYCLIC)) {
> > + vchan_cyclic_callback(&desc->vd);
> > + } else {
> > + vchan_cookie_complete(&desc->vd);
> > + channel->desc = NULL;
> If a non-cyclic transfer completes while the global channel->status flag
> is set, could the IRQ handler erroneously call vchan_cyclic_callback()
> instead of vchan_cookie_complete(), permanently hanging the channel?
> Checking desc->type == RZ_DMAC_DESC_CYCLIC here instead might prevent
> misidentifying non-cyclic transfers as cyclic.
Once the channel is configured as cyclic it cannot be used otherwise. The use
has to call terminate_all(), synchornize() + re-get + re-prepare the transfers.
So, this exact scenario here is invalid according to the current use case.
If the status is set it means the channel->desc = NULL, which means the IRQ
handler is not executed up to this point.
^ permalink raw reply
* sashiko.dev review (Re: [PATCH v4 14/17] dmaengine: sh: rz-dmac: Add suspend to RAM support)
From: Claudiu Beznea @ 2026-04-20 12:52 UTC (permalink / raw)
To: vkoul, Frank.Li, lgirdwood, broonie, perex, tiwai, biju.das.jz,
prabhakar.mahadev-lad.rj, p.zabel, geert+renesas,
fabrizio.castro.jz, long.luu.ur
Cc: dmaengine, linux-kernel, linux-sound, linux-renesas-soc,
Claudiu Beznea
In-Reply-To: <20260411114303.2814115-15-claudiu.beznea.uj@bp.renesas.com>
> commit e7af08b0f2dc569cc63b17e07dd4e506e50fee28
> Author: Claudiu Beznea <claudiu.beznea.uj@bp.renesas.com>
> dmaengine: sh: rz-dmac: Add suspend to RAM support
> The Renesas RZ/G3S SoC supports a power saving mode in which power to most
> of the SoC components is turned off, including the DMA IP. Add suspend to
> RAM support to save and restore the DMA IP registers.
> > diff --git a/drivers/dma/sh/rz-dmac.c b/drivers/dma/sh/rz-dmac.c
> > --- a/drivers/dma/sh/rz-dmac.c
> > +++ b/drivers/dma/sh/rz-dmac.c
> [ ... ]
> > @@ -1354,6 +1407,130 @@ static void rz_dmac_remove(struct platform_device
*pdev)
> > pm_runtime_disable(&pdev->dev);
> > }
> >
> > +static int rz_dmac_suspend_prepare(struct device *dev)
> > +{
> > + struct rz_dmac *dmac = dev_get_drvdata(dev);
> > +
> > + for (unsigned int i = 0; i < dmac->n_channels; i++) {
> > + struct rz_dmac_chan *channel = &dmac->channels[i];
> > +
> > + guard(spinlock_irqsave)(&channel->vc.lock);
> > +
> > + /* Wait for transfer completion, except in cyclic case. */
> > + if (channel->status & BIT(RZ_DMAC_CHAN_STATUS_CYCLIC))
> > + continue;
> > +
> > + if (rz_dmac_chan_is_enabled(channel))
> > + return -EAGAIN;
> Is it intended that this aborts the system suspend instead of waiting?
Returning -EAGAIN here makes the core suspend code to keep trying calling the
prepare API of this driver. That's why it has mentioned waiting.
> The commit message states the .prepare callback "waits for all the ongoing
> transfers to complete before allowing suspend-to-RAM to proceed."
> However, returning -EAGAIN from a system sleep .prepare callback immediately
> aborts the system suspend process instead of waiting.
> Additionally, since the PM core executes the .prepare phase for all devices
> before executing any .suspend callbacks, consumer devices haven't yet run
> their own .suspend callbacks to stop their DMA transfers.
> If any normal background DMA activity is ongoing, won't this immediately
> abort the entire system suspend transition?
If DMA activity is ongoing the expectation is that it will finish and the
suspend_prepare() will go forward.
> > + }
> > +
> > + return 0;
> > +}
> [ ... ]
> > +static int rz_dmac_suspend(struct device *dev)
> > +{
> > + struct rz_dmac *dmac = dev_get_drvdata(dev);
> > + int ret;
> Can ret be used uninitialized here?
Yes, this is a typo. I'll properly initialize this here.
> If the DMA controller has no cyclic channels configured, the loop below will
> skip every iteration due to the continue statement, leaving ret
> uninitialized.
> > +
> > + for (unsigned int i = 0; i < dmac->n_channels; i++) {
> > + struct rz_dmac_chan *channel = &dmac->channels[i];
> > +
> > + guard(spinlock_irqsave)(&channel->vc.lock);
> > +
> > + if (!(channel->status & BIT(RZ_DMAC_CHAN_STATUS_CYCLIC)))
> > + continue;
> > +
> > + ret = rz_dmac_device_pause_internal(channel);
> > + if (ret) {
> > + dev_err(dev, "Failed to suspend channel %s\n",
> > + dma_chan_name(&channel->vc.chan));
> > + break;
> > + }
> > +
> > + channel->pm_state.nxla = rz_dmac_ch_readl(channel, NXLA, 1);
> > + }
> > +
> > + if (ret) {
> If ret contains uninitialized stack memory that evaluates to true, could this
> trigger the error recovery path and silently abort the system suspend
> transition?
> > + rz_dmac_suspend_recover(dmac);
> > + return ret;
> > + }
> > +
> > + pm_runtime_put_sync(dmac->dev);
> > +
> > + ret = reset_control_assert(dmac->rstc);
> Is the ordering of runtime PM and reset control inverted here compared to
> rz_dmac_remove()?
Yes, I'll adjust it.
> In rz_dmac_remove(), reset_control_assert() is called before pm_runtime_put().
> Operating on the reset control after the device's power domain is suspended
> might cause the operations to be silently ignored by the hardware or lead to
> system bus lockups.
> > + if (ret) {
> > + pm_runtime_resume_and_get(dmac->dev);
> > + rz_dmac_suspend_recover(dmac);
> > + }
> > +
> > + return ret;
> > +}
> > +
> > +static int rz_dmac_resume(struct device *dev)
> > +{
> > + struct rz_dmac *dmac = dev_get_drvdata(dev);
> > + int errors = 0, ret;
> > +
> > + ret = reset_control_deassert(dmac->rstc);
> Similarly, is the ordering of reset control and runtime PM inverted here
> compared to rz_dmac_probe()?
Yes, I'll adjust it.
> In rz_dmac_probe(), pm_runtime_resume_and_get() is called before
> reset_control_deassert(). Doing this in the reverse order here means the
> reset is deasserted before the hardware power domain is enabled.
> > + if (ret)
> > + return ret;
> > +
> > + ret = pm_runtime_resume_and_get(dmac->dev);
> > + if (ret) {
> > + reset_control_assert(dmac->rstc);
> > + return ret;
> > + }
> [ ... ]
> > @@ -1380,6 +1557,7 @@ static struct platform_driver rz_dmac_driver = {
> > .driver = {
> > .name = "rz-dmac",
> > .of_match_table = of_rz_dmac_match,
> > + .pm = pm_sleep_ptr(&rz_dmac_pm_ops),
> Should this use pm_ptr() instead of pm_sleep_ptr()?
> If CONFIG_PM_SLEEP is disabled, pm_sleep_ptr() evaluates to NULL, but the
> struct rz_dmac_pm_ops itself and its unconditional callbacks like
> rz_dmac_suspend_prepare will remain fully compiled into the binary as unused
> dead code, potentially leading to compiler warnings.
> Using pm_ptr() for the dev_pm_ops structure pointer and pm_sleep_ptr()
> around the sleep callbacks inside the structure might resolve this.
> > },
> > .probe = rz_dmac_probe,
> > .remove = rz_dmac_remove,
I think pm_sleep_ptr() fits better here as CONFIG_PM_SLEEP depends on
CONFIG_SUSPEND which is the flag under which the system suspend/resume code
resides. Also, CONFIG_PM_SLEEP selects CONFIG_PM, so CONFIG_PM_SLEEP cannot be
enabled w/o CONFIG_PM.
^ permalink raw reply
* [PATCH AUTOSEL 6.18] ALSA: hda/realtek: add quirk for Framework F111:000F
From: Sasha Levin @ 2026-04-20 13:08 UTC (permalink / raw)
To: patches, stable
Cc: Dustin L. Howett, Takashi Iwai, Sasha Levin, perex, tiwai,
linux-sound, linux-kernel
In-Reply-To: <20260420131539.986432-1-sashal@kernel.org>
From: "Dustin L. Howett" <dustin@howett.net>
[ Upstream commit bac1e57adf08c9ee33e95fb09cd032f330294e70 ]
Similar to commit 7b509910b3ad ("ALSA hda/realtek: Add quirk for
Framework F111:000C") and previous quirks for Framework systems with
Realtek codecs.
000F is another new platform with an ALC285 which needs the same quirk.
Signed-off-by: Dustin L. Howett <dustin@howett.net>
Link: https://patch.msgid.link/20260327-framework-alsa-000f-v1-1-74013aba1c00@howett.net
Signed-off-by: Takashi Iwai <tiwai@suse.de>
Signed-off-by: Sasha Levin <sashal@kernel.org>
---
LLM Generated explanations, may be completely bogus:
Error: Failed to generate final synthesis
sound/hda/codecs/realtek/alc269.c | 1 +
1 file changed, 1 insertion(+)
diff --git a/sound/hda/codecs/realtek/alc269.c b/sound/hda/codecs/realtek/alc269.c
index 0c975005793e7..e7f7b148b40e5 100644
--- a/sound/hda/codecs/realtek/alc269.c
+++ b/sound/hda/codecs/realtek/alc269.c
@@ -7555,6 +7555,7 @@ static const struct hda_quirk alc269_fixup_tbl[] = {
SND_PCI_QUIRK(0xf111, 0x0009, "Framework Laptop", ALC295_FIXUP_FRAMEWORK_LAPTOP_MIC_NO_PRESENCE),
SND_PCI_QUIRK(0xf111, 0x000b, "Framework Laptop", ALC295_FIXUP_FRAMEWORK_LAPTOP_MIC_NO_PRESENCE),
SND_PCI_QUIRK(0xf111, 0x000c, "Framework Laptop", ALC295_FIXUP_FRAMEWORK_LAPTOP_MIC_NO_PRESENCE),
+ SND_PCI_QUIRK(0xf111, 0x000f, "Framework Laptop", ALC295_FIXUP_FRAMEWORK_LAPTOP_MIC_NO_PRESENCE),
#if 0
/* Below is a quirk table taken from the old code.
--
2.53.0
^ permalink raw reply related
* [PATCH AUTOSEL 7.0-5.15] ALSA: pcm: Serialize snd_pcm_suspend_all() with open_mutex
From: Sasha Levin @ 2026-04-20 13:08 UTC (permalink / raw)
To: patches, stable
Cc: Cássio Gabriel, Takashi Iwai, Takashi Iwai, Sasha Levin,
perex, linux-sound, linux-kernel
In-Reply-To: <20260420131539.986432-1-sashal@kernel.org>
From: Cássio Gabriel <cassiogabrielcontato@gmail.com>
[ Upstream commit 1a56641b7ae4f19216774a59d68024be3e6197d0 ]
snd_pcm_suspend_all() walks all PCM substreams and uses a lockless
runtime check to skip closed streams. It then calls snd_pcm_suspend()
for each remaining substream and finally runs snd_pcm_sync_stop() in a
second pass.
The runtime lifetime is still controlled by pcm->open_mutex in the
open/release path. That means a concurrent close can clear or free
substream->runtime after the initial check in snd_pcm_suspend_all(),
leaving the later suspend or sync-stop path to dereference a stale or
NULL runtime pointer.
Serialize snd_pcm_suspend_all() with pcm->open_mutex so the runtime
pointer stays stable across both loops. This matches the existing PCM
runtime lifetime rule already used by other core paths that access
substream->runtime outside the stream lock.
Suggested-by: Takashi Iwai <tiwai@suse.com>
Signed-off-by: Cássio Gabriel <cassiogabrielcontato@gmail.com>
Link: https://patch.msgid.link/20260327-alsa-pcm-suspend-open-close-lock-v2-1-cc4baca4dcd6@gmail.com
Signed-off-by: Takashi Iwai <tiwai@suse.de>
Signed-off-by: Sasha Levin <sashal@kernel.org>
---
LLM Generated explanations, may be completely bogus:
Here is my complete analysis:
---
## PHASE 1: COMMIT MESSAGE FORENSICS
### Step 1.1: Subject Line
- **Subsystem**: `ALSA: pcm:` (ALSA PCM core)
- **Action verb**: "Serialize" — implies fixing a synchronization issue
(race condition)
- **Summary**: Serialize `snd_pcm_suspend_all()` with `open_mutex` to
prevent concurrent access
Record: [ALSA PCM core] [Serialize] [Add mutex to prevent race between
suspend and open/close]
### Step 1.2: Tags
- **Suggested-by: Takashi Iwai `<tiwai@suse.com>`** — Takashi Iwai is
the ALSA subsystem maintainer. He suggested this fix, which carries
enormous weight.
- **Signed-off-by: Takashi Iwai `<tiwai@suse.de>`** — Maintainer also
signed off (committed it).
- **Signed-off-by: Cássio Gabriel** — Author of the patch.
- **Link:** to patch.msgid.link (v2 of the patch).
- No Fixes: tag, no Reported-by, no Cc: stable — expected for autosel
candidates.
Record: Fix suggested and committed by the subsystem maintainer (Takashi
Iwai). This is the highest trust signal possible.
### Step 1.3: Commit Body Analysis
The commit describes:
- **Bug**: `snd_pcm_suspend_all()` walks PCM substreams with a lockless
runtime check, then calls `snd_pcm_suspend()` and
`snd_pcm_sync_stop()`. A concurrent close can clear/free
`substream->runtime` via the open/close path (which uses
`pcm->open_mutex`).
- **Failure mode**: Stale or NULL runtime pointer dereference —
crash/UAF.
- **Fix approach**: Take `pcm->open_mutex` in `snd_pcm_suspend_all()`,
matching the existing PCM runtime lifetime rule.
Record: UAF/NULL deref race during system suspend. Author explains clear
mechanism.
### Step 1.4: Hidden Bug Fix Detection
This is NOT hidden — it's an explicit race condition fix. The code
itself had a `/* FIXME: the open/close code should lock this as well */`
comment acknowledging the bug.
---
## PHASE 2: DIFF ANALYSIS
### Step 2.1: Inventory
- **Files changed**: 1 (`sound/core/pcm_native.c`)
- **Lines**: +5 (including comment update), -1 (removing FIXME comment).
Net functional change: +1 line (`guard(mutex)(&pcm->open_mutex)`)
- **Functions modified**: `snd_pcm_suspend_all()`
- **Scope**: Single-file, single-function, surgical fix.
### Step 2.2: Code Flow Change
**Before**: `snd_pcm_suspend_all()` iterates substreams without any
lock, checking `substream->runtime` and calling `snd_pcm_suspend()` /
`snd_pcm_sync_stop()` unsynchronized with the open/close path.
**After**: `guard(mutex)(&pcm->open_mutex)` is taken at function entry,
serializing the entire function with the release path
(`snd_pcm_release()` at line 2941 holds `open_mutex` around
`snd_pcm_release_substream()`).
### Step 2.3: Bug Mechanism
**Category**: Race condition / Use-After-Free / NULL pointer dereference
The race:
1. Thread A (`snd_pcm_suspend_all`): checks `!substream->runtime` → sees
non-NULL
2. Thread B (`snd_pcm_release`): under `open_mutex`, calls
`snd_pcm_detach_substream()` → sets `substream->runtime = NULL` →
`kfree(runtime)` (confirmed in `sound/core/pcm.c` lines 980-1003)
3. Thread A: calls `snd_pcm_suspend(substream)` → dereferences
freed/NULL runtime → **CRASH**
Second race window: the second loop calls `snd_pcm_sync_stop()` on ALL
substreams, which accesses `substream->runtime` (line 641).
### Step 2.4: Fix Quality
- Obviously correct: takes the same mutex already used by open/close
paths.
- Minimal: one functional line added.
- Regression risk: Very low. The `open_mutex` is not taken in interrupt
context; `snd_pcm_suspend()` internally uses
`guard(pcm_stream_lock_irqsave)` which is a different lock, so no
deadlock risk. The mutex merely serializes with open/close.
---
## PHASE 3: GIT HISTORY INVESTIGATION
### Step 3.1: Blame
The FIXME comment (`/* FIXME: the open/close code should lock this as
well */`) was introduced in commit `8d19b4e0b377e2` (2021-02-06, "ALSA:
pcm: Use for_each_pcm_substream() macro") by Takashi Iwai himself. The
core `snd_pcm_suspend_all` function dates back to `1da177e4c3f41` (Linus
Torvalds, 2005-04-16, the initial Linux tree).
Record: The race has existed since v2.6.12 (original tree). It was made
worse in v5.12 when `snd_pcm_sync_stop` loop was added (commit
`2c87c1a49c9d`). The FIXME was planted by the maintainer acknowledging
the bug.
### Step 3.2: Fixes tag
No Fixes: tag present. The race effectively has existed since the
beginning.
### Step 3.3: File History
Recent changes to `pcm_native.c` include `guard()`-based locking
conversions and race fixes (e.g., `93a81ca065775` "Fix race of buffer
access at PCM OSS layer"). This shows the subsystem is actively being
hardened for concurrency.
### Step 3.4: Author
Cássio Gabriel has one other commit in the sound subsystem. However, the
fix was **suggested by Takashi Iwai** (ALSA maintainer) and **committed
by Takashi Iwai**, giving it the highest credibility.
### Step 3.5: Dependencies
The fix uses `guard(mutex)` which requires the cleanup.h infrastructure
(available since v6.5) and the guard conversions in pcm_native.c
(`dd0da75b9a276`, available since v6.12). For older stable trees, a
trivial adaptation to `mutex_lock`/`mutex_unlock` would be needed.
---
## PHASE 4: MAILING LIST RESEARCH
### Step 4.1-4.2: Patch Discussion
b4 dig did not find the patch by commit hash (likely because this is on
a v7.0 tree). The Link: tag points to `patch.msgid.link`, which
redirected to lore.kernel.org but was blocked by anti-bot protection.
However, the commit subject says "v2", indicating the patch went through
at least two revisions. The maintainer (Takashi Iwai) suggested and
committed the fix — the highest form of endorsement.
### Step 4.3-4.5: Bug Report and Stable History
No explicit bug report (no Reported-by:). This is a proactive fix based
on code analysis (the FIXME comment). No prior stable discussion found.
---
## PHASE 5: CODE SEMANTIC ANALYSIS
### Step 5.1-5.2: Callers of `snd_pcm_suspend_all`
- `sound/core/pcm.c:do_pcm_suspend()` — PM callback, called during
**system suspend**
- `sound/soc/soc-core.c` — ASoC suspend
- `sound/hda/common/codec.c` — HDA codec suspend
- `sound/usb/usx2y/us144mkii.c` — USB audio suspend
This is called on **every system suspend** for every PCM device. This
means every laptop, desktop, or embedded system with audio is affected.
### Step 5.3-5.4: Call Chain
System suspend → `do_pcm_suspend()` → `snd_pcm_suspend_all()` → race
with user-space closing audio fd → `snd_pcm_release()` → UAF/NULL deref.
Trigger: Closing a laptop lid while an audio application is running.
This is an extremely common scenario.
---
## PHASE 6: STABLE TREE ANALYSIS
### Step 6.1: Buggy Code in Stable
The buggy code (unsynchronized runtime access in `snd_pcm_suspend_all`)
has existed since v2.6.12. The `snd_pcm_sync_stop` second loop was added
around v5.12. All active stable trees contain the buggy code.
### Step 6.2: Backport Complications
For v7.0 and v6.12+: should apply cleanly (same `guard()`
infrastructure).
For v6.6 and older: would need trivial adaptation from `guard(mutex)` to
`mutex_lock`/`mutex_unlock`.
---
## PHASE 7: SUBSYSTEM CONTEXT
### Step 7.1: Subsystem Criticality
- **Subsystem**: ALSA PCM core (`sound/core/`)
- **Criticality**: IMPORTANT — affects every system with audio hardware
(virtually all Linux systems)
### Step 7.2: Activity
Very actively maintained by Takashi Iwai. Recent commits show ongoing
concurrency hardening.
---
## PHASE 8: IMPACT AND RISK ASSESSMENT
### Step 8.1: Who Is Affected
Every Linux system with audio hardware (virtually universal).
### Step 8.2: Trigger Conditions
System suspend while an audio stream is being closed. This is a common
scenario on laptops (lid close while music playing). The race window is
between checking `substream->runtime` and using it — a narrow but real
window.
### Step 8.3: Failure Mode Severity
NULL pointer dereference or use-after-free → **kernel oops/crash during
suspend**. Severity: **CRITICAL**.
### Step 8.4: Risk-Benefit
- **Benefit**: HIGH — prevents kernel crash during suspend for all
audio-capable systems
- **Risk**: VERY LOW — one functional line, uses existing mutex already
employed by the open/close paths, no deadlock risk, suggested and
committed by the subsystem maintainer
- **Ratio**: Strongly favorable
---
## PHASE 9: FINAL SYNTHESIS
### Step 9.1: Evidence Summary
**FOR backporting:**
- Fixes a real race condition (UAF / NULL deref) acknowledged by a FIXME
comment from the subsystem maintainer
- Fix was suggested and committed by Takashi Iwai (ALSA maintainer)
- Minimal change: 1 functional line added
- Affects every system with audio during system suspend
- Failure mode is kernel crash (CRITICAL severity)
- Uses existing locking infrastructure already in use by the same code
paths
- Second revision (v2), showing review iteration
**AGAINST backporting:**
- No user-reported crash (proactive fix based on code analysis)
- May need minor adaptation for older stable trees (guard →
mutex_lock/unlock)
### Step 9.2: Stable Rules Checklist
1. Obviously correct and tested? **YES** — suggested by maintainer, uses
existing mutex pattern
2. Fixes a real bug? **YES** — race between suspend and release, FIXME
acknowledged the issue
3. Important issue? **YES** — UAF/NULL deref → kernel crash during
suspend
4. Small and contained? **YES** — 1 functional line in 1 function in 1
file
5. No new features or APIs? **YES** — pure synchronization fix
6. Can apply to stable? **YES** — clean for 6.12+, trivial adaptation
for older
### Step 9.3: Exception Categories
Not an exception — this is a standard race condition fix, a core stable
candidate.
### Step 9.4: Decision
This is a textbook stable fix: a single-line mutex acquisition that
prevents a UAF/NULL deref race during system suspend, suggested and
committed by the subsystem maintainer, fixing a longstanding FIXME.
---
## Verification
- [Phase 1] Parsed tags: Suggested-by and Signed-off-by from Takashi
Iwai (ALSA maintainer)
- [Phase 2] Diff analysis: +1 functional line
(`guard(mutex)(&pcm->open_mutex)`), removes 1-line FIXME comment
- [Phase 2] Verified `snd_pcm_sync_stop()` accesses `substream->runtime`
at line 641 without lock
- [Phase 2] Verified `snd_pcm_detach_substream()` sets
`substream->runtime = NULL` and `kfree(runtime)` at lines 997-1003 of
`sound/core/pcm.c`
- [Phase 2] Verified `snd_pcm_release()` holds `open_mutex` around
`snd_pcm_release_substream()` at line 2941
- [Phase 3] git blame: FIXME comment introduced by Takashi Iwai in
`8d19b4e0b377e2` (2021-02-06), core function from `1da177e4c3f41`
(2005)
- [Phase 3] `snd_pcm_sync_stop` loop added in `2c87c1a49c9d11` ("Assure
sync with pending stop operation at suspend", 2021)
- [Phase 3] `guard()` infrastructure for pcm_native.c in `dd0da75b9a276`
(v6.12+)
- [Phase 4] b4 dig: did not find match (this is a v7.0 tree). Link: in
commit points to v2 of the patch.
- [Phase 5] Callers: `do_pcm_suspend()` (PM callback), soc-core, HDA
codec, USB audio — called on every system suspend
- [Phase 6] Buggy code present in all active stable trees (since
v2.6.12)
- [Phase 7] ALSA PCM core — affects virtually all Linux systems with
audio
- [Phase 8] Failure mode: NULL deref or UAF during system suspend →
kernel crash (CRITICAL)
**YES**
sound/core/pcm_native.c | 6 +++++-
1 file changed, 5 insertions(+), 1 deletion(-)
diff --git a/sound/core/pcm_native.c b/sound/core/pcm_native.c
index 5a64453da7283..1ccc482a8dfd3 100644
--- a/sound/core/pcm_native.c
+++ b/sound/core/pcm_native.c
@@ -1761,6 +1761,9 @@ static int snd_pcm_suspend(struct snd_pcm_substream *substream)
* snd_pcm_suspend_all - trigger SUSPEND to all substreams in the given pcm
* @pcm: the PCM instance
*
+ * Takes and releases pcm->open_mutex to serialize against
+ * concurrent open/close while walking the substreams.
+ *
* After this call, all streams are changed to SUSPENDED state.
*
* Return: Zero if successful (or @pcm is %NULL), or a negative error code.
@@ -1773,8 +1776,9 @@ int snd_pcm_suspend_all(struct snd_pcm *pcm)
if (! pcm)
return 0;
+ guard(mutex)(&pcm->open_mutex);
+
for_each_pcm_substream(pcm, stream, substream) {
- /* FIXME: the open/close code should lock this as well */
if (!substream->runtime)
continue;
--
2.53.0
^ permalink raw reply related
* [PATCH AUTOSEL 7.0-6.18] ALSA: hda/realtek: Add quirk for CSL Unity BF24B
From: Sasha Levin @ 2026-04-20 13:08 UTC (permalink / raw)
To: patches, stable
Cc: Zhang Heng, Takashi Iwai, Sasha Levin, perex, tiwai, linux-sound,
linux-kernel
In-Reply-To: <20260420131539.986432-1-sashal@kernel.org>
From: Zhang Heng <zhangheng@kylinos.cn>
[ Upstream commit de65275fc94e2e0acc79bd016d60889bf251ccd9 ]
The CSL Unity BF24B all-in-one PC uses a Realtek ALC662 rev3 audio
codec and requires the correct GPIO configuration to enable sound
output from both the speakers and the headphone.
Link: https://bugzilla.kernel.org/show_bug.cgi?id=221258
Signed-off-by: Zhang Heng <zhangheng@kylinos.cn>
Link: https://patch.msgid.link/20260409024028.1297587-1-zhangheng@kylinos.cn
Signed-off-by: Takashi Iwai <tiwai@suse.de>
Signed-off-by: Sasha Levin <sashal@kernel.org>
---
LLM Generated explanations, may be completely bogus:
Now I have all the information needed for a comprehensive analysis. Let
me compile the findings.
---
## PHASE 1: COMMIT MESSAGE FORENSICS
**Step 1.1: Subject Line**
- Subsystem: `ALSA: hda/realtek`
- Action verb: "Add quirk" — this is adding a hardware-specific
workaround
- Summary: Adding GPIO quirk for CSL Unity BF24B all-in-one PC to enable
audio output
- Record: [ALSA/HDA/Realtek] [Add quirk] [GPIO config to enable
speakers/headphone on CSL Unity BF24B]
**Step 1.2: Tags**
- Link: `https://bugzilla.kernel.org/show_bug.cgi?id=221258` — user-
filed bug report
- Signed-off-by: Zhang Heng (author, regular Realtek quirk contributor)
- Link: patch.msgid.link (for tracking the patch)
- Signed-off-by: Takashi Iwai (ALSA subsystem maintainer — applied the
patch)
- No Fixes: tag (expected — this is a quirk addition, not fixing a code
defect)
- No Cc: stable (expected — this is why we're reviewing it)
- Record: Bugzilla report from real user. Signed off by the ALSA
maintainer.
**Step 1.3: Commit Body**
- Bug: CSL Unity BF24B all-in-one PC has no sound output from speakers
or headphones
- Codec: Realtek ALC662 rev3
- Root cause: GPIO configuration needed to enable the amplifier
- Record: Complete audio failure on specific hardware without GPIO
toggle workaround.
**Step 1.4: Hidden Bug Fix Detection**
- This is not a hidden bug fix — it's an explicit hardware quirk
addition. The device literally has no audio without it.
## PHASE 2: DIFF ANALYSIS
**Step 2.1: Inventory**
- Files changed: 1 (`sound/hda/codecs/realtek/alc662.c`)
- Lines added: ~25 (new function + enum + fixup entry + quirk entry)
- Functions modified: none existing; one new function added
(`alc662_fixup_csl_amp`)
- Scope: single-file, surgical, self-contained
**Step 2.2: Code Flow Change**
1. New function `alc662_fixup_csl_amp`:
- `HDA_FIXUP_ACT_PRE_PROBE`: Sets GPIO mask and direction for bits
0+1
- `HDA_FIXUP_ACT_INIT`: Toggles GPIO 0+1 high, waits 100ms, then low
— to enable the amplifier
2. New enum `ALC662_FIXUP_CSL_GPIO` added at end of existing enum
3. New fixup table entry linking enum to function
4. New quirk table entry: `SND_PCI_QUIRK(0x1022, 0xc950, "CSL Unity
BF24B", ALC662_FIXUP_CSL_GPIO)`
**Step 2.3: Bug Mechanism**
- Category: Hardware workaround (h)
- The CSL Unity BF24B's amplifier requires a GPIO toggle pulse to
enable. Without it, the amp stays off and no sound comes out.
**Step 2.4: Fix Quality**
- The pattern is nearly identical to `alc245_fixup_hp_x360_amp` in
alc269.c (lines 1448-1465), which toggles GPIO0 similarly. This new
one toggles GPIO0+GPIO1 (mask 0x03).
- Obviously correct — follows well-established patterns used dozens of
times in this driver.
- Minimal regression risk — only affects devices with PCI SSID
0x1022:0xc950.
- Takashi Iwai noted a minor inefficiency (extra GPIO write from
`alc_auto_init_amp`) but accepted the patch as-is, planning cleanup
later.
## PHASE 3: GIT HISTORY INVESTIGATION
**Step 3.1: Blame**
- The ALC662 quirk infrastructure has been in the kernel for many years.
The file was recently split from `sound/pci/hda/patch_realtek.c` in
commit `aeeb85f26c3bb` (July 2025, kernel 7.0-rc cycle), which was
itself moved from `sound/pci/hda/` to `sound/hda/codecs/` in
`6014e9021b28e`.
- The ALC662 fixup enum and table existed in `patch_realtek.c` long
before the split.
**Step 3.2: No Fixes: tag** — expected for a quirk addition.
**Step 3.3: File History**
- Recent changes are all quirk additions — this is a very common pattern
for this file.
- The author (Zhang Heng) has contributed many similar quirks: Acer
Swift, HP Laptop, ASUS ROG, Lenovo Yoga, etc.
**Step 3.4: Author**
- Zhang Heng is a prolific Realtek quirk contributor with 10+ similar
commits in the tree.
**Step 3.5: Dependencies**
- This is fully standalone. Uses only existing APIs
(`alc_update_gpio_data`, `alc_spec`).
- For stable trees (6.x), the patch would need to target
`sound/pci/hda/patch_realtek.c` instead. The functions and structures
are identical there (verified: `alc_update_gpio_data` appears 14 times
in pre-split `patch_realtek.c`).
## PHASE 4: MAILING LIST AND EXTERNAL RESEARCH
**Step 4.1: Patch Discussion**
- Found full discussion on patchew.org. Takashi Iwai reviewed the patch
and asked about the necessity of setting `gpio_mask`/`gpio_dir` in
`PRE_PROBE`. He proposed a cleaner GPIO helper approach.
- Despite reservations about minor inefficiency, Iwai explicitly stated:
*"I'm going to take your patch for now, but we might need to
reconsider the implementation"*.
- The patch was accepted as v1, no subsequent versions.
**Step 4.2: Reviewers**
- Takashi Iwai (ALSA subsystem maintainer) directly reviewed and applied
it.
**Step 4.3: Bug Report**
- Bugzilla #221258 — user tested with `hda-verb` commands setting GPIO
mask, direction, and data to confirm the workaround works. The user's
manual commands were:
- SET_GPIO_MASK 0x03, SET_GPIO_DIRECTION 0x03, SET_GPIO_DATA 0x03,
then toggle
**Step 4.4: Series Context**
- Single standalone patch, not part of a series.
**Step 4.5: Stable Discussion**
- No explicit stable nomination found, but audio quirks are routinely
backported.
## PHASE 5: CODE SEMANTIC ANALYSIS
**Step 5.1: Functions**
- New: `alc662_fixup_csl_amp` — only called via the fixup framework for
matching PCI SSID
- Uses: `alc_update_gpio_data` (well-tested existing function, used 14+
times across the driver)
**Step 5.2: Callers**
- Called only by the HDA fixup framework when a device with PCI SSID
0x1022:0xc950 is detected.
**Step 5.3-5.5**: The function follows an identical pattern to
`alc245_fixup_hp_x360_amp`, which is well-established and proven.
## PHASE 6: CROSS-REFERENCING AND STABLE TREE ANALYSIS
**Step 6.1: Buggy Code in Stable**
- The ALC662 codec support exists in all stable trees. The
`alc662_fixups[]` table and `alc662_fixup_tbl[]` quirk table have been
present since early kernel versions.
- The functions used (`alc_update_gpio_data`, `msleep`, etc.) exist in
all active stable trees.
**Step 6.2: Backport Complications**
- **Key issue**: In 7.0, the driver was split into
`sound/hda/codecs/realtek/alc662.c`. In stable 6.x trees, the
equivalent code lives in `sound/pci/hda/patch_realtek.c`. The patch
will need path adjustment but the code structures are identical.
- Record: Minor mechanical adjustment needed for file path; code applies
cleanly to older structure.
**Step 6.3: No related fixes already in stable.**
## PHASE 7: SUBSYSTEM AND MAINTAINER CONTEXT
**Step 7.1**: Subsystem is ALSA/HDA (audio). Criticality: IMPORTANT —
audio is essential for desktop/laptop users.
**Step 7.2**: Very actively developed — constant quirk additions for new
hardware.
## PHASE 8: IMPACT AND RISK ASSESSMENT
**Step 8.1: Who is Affected**
- Users of CSL Unity BF24B all-in-one PCs. Without this quirk, they have
**zero audio output**.
**Step 8.2: Trigger Conditions**
- Every boot, every resume from suspend. Audio is completely non-
functional without the quirk.
**Step 8.3: Failure Mode Severity**
- Complete loss of audio functionality on the device. Severity: HIGH for
affected users.
**Step 8.4: Risk-Benefit Ratio**
- BENEFIT: High — enables audio on a specific PC model that has zero
sound without it
- RISK: Very low — 25 lines, only affects one PCI SSID, follows proven
patterns, reviewed by subsystem maintainer
- Ratio: Strongly favorable
## PHASE 9: FINAL SYNTHESIS
**Step 9.1: Evidence Summary**
- FOR: Hardware quirk for broken audio; user-filed bugzilla; subsystem
maintainer reviewed and applied; follows identical pattern to existing
quirks; small and surgical; standalone
- AGAINST: File path needs adjustment for stable (mechanical); Takashi
noted minor inefficiency (but accepted anyway)
- UNRESOLVED: Could not access bugzilla directly (anti-bot protection),
but bug context confirmed through mailing list discussion
**Step 9.2: Stable Rules Checklist**
1. Obviously correct and tested? **YES** — follows proven pattern,
reviewed by maintainer, tested by user with hda-verb
2. Fixes a real bug? **YES** — zero audio output on specific hardware
3. Important issue? **YES** — complete loss of audio functionality
4. Small and contained? **YES** — 25 lines in one file
5. No new features or APIs? **YES** — only a device-specific workaround
6. Can apply to stable? **YES** — needs file path adjustment but code is
compatible
**Step 9.3: Exception Category**
- This is a textbook **HARDWARE QUIRK/WORKAROUND** — explicitly listed
as a YES exception for stable.
## Verification
- [Phase 1] Parsed tags: Link to bugzilla.kernel.org #221258, signed off
by Takashi Iwai (ALSA maintainer)
- [Phase 2] Diff analysis: 25 lines added — new fixup function, enum
entry, fixup table entry, quirk table entry
- [Phase 2] Pattern match: `alc662_fixup_csl_amp` is nearly identical to
`alc245_fixup_hp_x360_amp` at alc269.c:1448-1465
- [Phase 3] git log: file split from `patch_realtek.c` in aeeb85f26c3bb
(Jul 2025), previously moved in 6014e9021b28e
- [Phase 3] git log --author: Zhang Heng has 10+ similar quirk commits
in this tree
- [Phase 3] Verified `alc_update_gpio_data` exists 14 times in pre-split
`patch_realtek.c`
- [Phase 4] Patchew.org: full discussion retrieved; Takashi reviewed,
questioned PRE_PROBE gpio_mask but accepted patch
- [Phase 4] Takashi Iwai explicitly stated: "I'm going to take your
patch for now"
- [Phase 4] User tested with hda-verb commands confirming GPIO toggle
enables audio
- [Phase 5] `alc_update_gpio_data` is a well-tested exported function
used across the driver
- [Phase 6] ALC662 support and fixup infrastructure exist in all active
stable trees
- [Phase 6] Backport needs path adjustment:
`sound/hda/codecs/realtek/alc662.c` → `sound/pci/hda/patch_realtek.c`
- [Phase 8] Failure mode: complete audio loss on affected hardware,
severity HIGH
- UNVERIFIED: Could not directly access bugzilla #221258 (anti-bot
protection), but context confirmed via mailing list
This is a textbook hardware quirk addition — small, self-contained,
fixes complete audio failure on a specific device, follows proven
patterns, reviewed and signed off by the subsystem maintainer. It falls
directly into the "quirks and workarounds" exception category that is
automatically appropriate for stable.
**YES**
sound/hda/codecs/realtek/alc662.c | 25 +++++++++++++++++++++++++
1 file changed, 25 insertions(+)
diff --git a/sound/hda/codecs/realtek/alc662.c b/sound/hda/codecs/realtek/alc662.c
index 5073165d1f3cf..3abe41c7315c4 100644
--- a/sound/hda/codecs/realtek/alc662.c
+++ b/sound/hda/codecs/realtek/alc662.c
@@ -255,6 +255,25 @@ static void alc_fixup_headset_mode_alc668(struct hda_codec *codec,
alc_fixup_headset_mode(codec, fix, action);
}
+static void alc662_fixup_csl_amp(struct hda_codec *codec,
+ const struct hda_fixup *fix, int action)
+{
+ struct alc_spec *spec = codec->spec;
+
+ switch (action) {
+ case HDA_FIXUP_ACT_PRE_PROBE:
+ spec->gpio_mask |= 0x03;
+ spec->gpio_dir |= 0x03;
+ break;
+ case HDA_FIXUP_ACT_INIT:
+ /* need to toggle GPIO to enable the amp */
+ alc_update_gpio_data(codec, 0x03, true);
+ msleep(100);
+ alc_update_gpio_data(codec, 0x03, false);
+ break;
+ }
+}
+
enum {
ALC662_FIXUP_ASPIRE,
ALC662_FIXUP_LED_GPIO1,
@@ -313,6 +332,7 @@ enum {
ALC897_FIXUP_HEADSET_MIC_PIN2,
ALC897_FIXUP_UNIS_H3C_X500S,
ALC897_FIXUP_HEADSET_MIC_PIN3,
+ ALC662_FIXUP_CSL_GPIO,
};
static const struct hda_fixup alc662_fixups[] = {
@@ -766,11 +786,16 @@ static const struct hda_fixup alc662_fixups[] = {
{ }
},
},
+ [ALC662_FIXUP_CSL_GPIO] = {
+ .type = HDA_FIXUP_FUNC,
+ .v.func = alc662_fixup_csl_amp,
+ },
};
static const struct hda_quirk alc662_fixup_tbl[] = {
SND_PCI_QUIRK(0x1019, 0x9087, "ECS", ALC662_FIXUP_ASUS_MODE2),
SND_PCI_QUIRK(0x1019, 0x9859, "JP-IK LEAP W502", ALC897_FIXUP_HEADSET_MIC_PIN3),
+ SND_PCI_QUIRK(0x1022, 0xc950, "CSL Unity BF24B", ALC662_FIXUP_CSL_GPIO),
SND_PCI_QUIRK(0x1025, 0x022f, "Acer Aspire One", ALC662_FIXUP_INV_DMIC),
SND_PCI_QUIRK(0x1025, 0x0241, "Packard Bell DOTS", ALC662_FIXUP_INV_DMIC),
SND_PCI_QUIRK(0x1025, 0x0308, "Acer Aspire 8942G", ALC662_FIXUP_ASPIRE),
--
2.53.0
^ permalink raw reply related
* [PATCH AUTOSEL 7.0-5.10] ALSA: aoa/onyx: Fix OF node leak on probe failure
From: Sasha Levin @ 2026-04-20 13:08 UTC (permalink / raw)
To: patches, stable
Cc: wangdicheng, Takashi Iwai, Sasha Levin, johannes, perex, tiwai,
linuxppc-dev, linux-sound, linux-kernel
In-Reply-To: <20260420131539.986432-1-sashal@kernel.org>
From: wangdicheng <wangdicheng@kylinos.cn>
[ Upstream commit e5d5aef802a5f41283084f7d443ef4fd4b65d86d ]
Add missing of_node_put() in the error path.
Signed-off-by: wangdicheng <wangdicheng@kylinos.cn>
Link: https://patch.msgid.link/20260403014736.33014-1-wangdich9700@163.com
Signed-off-by: Takashi Iwai <tiwai@suse.de>
Signed-off-by: Sasha Levin <sashal@kernel.org>
---
LLM Generated explanations, may be completely bogus:
Now I have all the information needed. Let me compile the full analysis.
---
## PHASE 1: COMMIT MESSAGE FORENSICS
### Step 1.1: Parse the Subject Line
- **Subsystem**: ALSA: aoa/onyx (Apple Onboard Audio, onyx codec)
- **Action verb**: "Fix" — direct bug fix
- **Summary**: Fixes an OF (Open Firmware) device tree node reference
leak when the probe function fails
### Step 1.2: Parse All Commit Message Tags
- **Signed-off-by**: wangdicheng <wangdicheng@kylinos.cn> (author)
- **Link**:
https://patch.msgid.link/20260403014736.33014-1-wangdich9700@163.com
(patch discussion)
- **Signed-off-by**: Takashi Iwai <tiwai@suse.de> (ALSA subsystem
maintainer; this means it was reviewed/applied by the maintainer)
- No Fixes: tag (expected — that's why we're reviewing)
- No Cc: stable (expected — that's why we're reviewing)
- No Reported-by (static analysis / code review finding)
### Step 1.3: Analyze the Commit Body Text
The commit body is concise: "Add missing of_node_put() in the error
path." This describes a classic OF node reference leak fix. When
`of_node_get()` acquires a reference but the error path doesn't call
`of_node_put()`, the node's reference count is leaked.
### Step 1.4: Detect Hidden Bug Fixes
This is an explicit bug fix, not disguised. The subject says "Fix" and
the body confirms a missing cleanup call.
---
## PHASE 2: DIFF ANALYSIS - LINE BY LINE
### Step 2.1: Inventory the Changes
- **File**: `sound/aoa/codecs/onyx.c`
- **Lines**: +3, -1 (net +2 lines)
- **Function**: `onyx_i2c_probe()`
- **Scope**: Single-file surgical fix to one error path
### Step 2.2: Understand the Code Flow Change
The key flow in `onyx_i2c_probe()`:
```978:990:sound/aoa/codecs/onyx.c
onyx->codec.init = onyx_init_codec;
onyx->codec.exit = onyx_exit_codec;
onyx->codec.node = of_node_get(node); // acquires OF node
reference
if (aoa_codec_register(&onyx->codec)) {
goto fail; // BEFORE: leaks OF node reference
}
...
fail:
kfree(onyx); // frees memory but doesn't release OF node ref
return -ENODEV;
```
**After the fix**: `goto fail` becomes `goto fail_put`, which first
calls `of_node_put(onyx->codec.node)` then falls through to `fail`.
### Step 2.3: Identify the Bug Mechanism
Category: **Reference counting fix / resource leak**. `of_node_get()`
increments the device tree node refcount. If `aoa_codec_register()`
fails, the refcount is never decremented, leaking the OF node.
### Step 2.4: Assess the Fix Quality
- **Obviously correct**: Yes — `of_node_get()` at line 980 must be
balanced by `of_node_put()` on error. The remove path at line 997
already correctly calls `of_node_put()`.
- **Minimal/surgical**: Yes — only 3 lines added, 1 changed.
- **Regression risk**: Essentially zero. The new label only executes on
error paths and merely balances a reference count.
---
## PHASE 3: GIT HISTORY INVESTIGATION
### Step 3.1: Blame the Changed Lines
The buggy code was introduced by commit `f3d9478b2ce468` ("[ALSA] snd-
aoa: add snd-aoa") by Johannes Berg, dated **2006-06-21**. This is the
original addition of the entire AOA subsystem. The bug has been present
for ~20 years, meaning it exists in **every supported stable tree**.
### Step 3.2: Follow the Fixes Tag
No Fixes: tag present, but the implicit fix target is `f3d9478b2ce468`
(2006). The same commit was referenced by the related fix
`222bce5eb88d1` ("ALSA: snd-aoa: add of_node_put() in error path") which
fixed a *similar* OF node leak in `sound/aoa/core/gpio-feature.c` but
did NOT touch `onyx.c`.
### Step 3.3: Related Changes
- `222bce5eb88d1` — Fixed same bug class in `gpio-feature.c` (2018),
never touched onyx.c
- The sibling codec `tas.c` has the **exact same bug** at lines 864-875
(acquires `of_node_get(node)` but `goto fail` doesn't release it)
### Step 3.4: Author's Other Commits
wangdicheng appears to be contributing fixes across multiple subsystems
(KylinOS developer). Takashi Iwai, who applied the patch, is the ALSA
subsystem maintainer — adding significant confidence.
### Step 3.5: Dependent/Prerequisite Commits
None. The fix is fully standalone. The code structure (`fail` label,
`kfree(onyx)`) has been unchanged since 2006.
---
## PHASE 4: MAILING LIST AND EXTERNAL RESEARCH
### Step 4.1-4.5: Patch Discussion
Lore.kernel.org is behind a proof-of-work challenge, and b4 dig couldn't
match this commit by patch-id. However, the patch was applied by ALSA
maintainer Takashi Iwai directly, which confirms it passed maintainer
review. The Link tag provides the message-id for the submission.
---
## PHASE 5: CODE SEMANTIC ANALYSIS
### Step 5.1: Key Functions
- `onyx_i2c_probe()` — the I2C driver probe function, called when the
device is matched
### Step 5.2: Callers
`onyx_i2c_probe()` is the `.probe` callback in `onyx_driver` (i2c_driver
struct at line 1012). It is called by the I2C core during device
enumeration/binding. This is a standard device probe path.
### Step 5.3-5.4: Reachability
The probe function is called whenever the kernel attempts to bind the
onyx I2C codec to its driver. On Apple Mac hardware with this audio
codec, this happens during boot. If `aoa_codec_register()` fails (which
can happen — e.g. if the fabric isn't registered yet), this leak
triggers.
### Step 5.5: Similar Patterns
As verified, `sound/aoa/codecs/tas.c` has the **identical bug** (lines
864-875): `of_node_get(node)` followed by `goto fail` that doesn't call
`of_node_put()`.
---
## PHASE 6: CROSS-REFERENCING AND STABLE TREE ANALYSIS
### Step 6.1: Buggy Code Existence in Stable Trees
The buggy code exists in **all** stable trees. It was introduced in 2006
(`f3d9478b2ce468`) and has never been fixed.
### Step 6.2: Backport Complications
The file has had minor changes (guard() conversions, alloc_obj macros)
but the probe function structure around the error path is essentially
unchanged since 2006. The patch should apply **cleanly** to all stable
trees, possibly with trivial context adjustments (e.g. `kzalloc_obj` vs
`kzalloc`).
### Step 6.3: Related Fixes Already in Stable
No. The related `222bce5eb88d1` fix was for `gpio-feature.c`, not
`onyx.c`.
---
## PHASE 7: SUBSYSTEM AND MAINTAINER CONTEXT
### Step 7.1: Subsystem Criticality
- **Subsystem**: ALSA / AOA (Apple Onboard Audio) — driver-specific
- **Criticality**: PERIPHERAL — affects Apple PowerPC/Mac hardware with
onyx codecs
- **Maintainer review**: Applied by Takashi Iwai (ALSA maintainer)
directly
### Step 7.2: Subsystem Activity
Low activity (last substantive change was treewide refactoring). This is
a mature, stable driver.
---
## PHASE 8: IMPACT AND RISK ASSESSMENT
### Step 8.1: Who Is Affected
Users of Apple Mac hardware with the pcm3052 (onyx) audio codec. This is
primarily older PowerPC-based Macs.
### Step 8.2: Trigger Conditions
The leak triggers when `aoa_codec_register()` fails during probe. This
can happen if the AOA fabric isn't registered, or if
`attach_codec_to_fabric()` returns an error. While not extremely common,
repeated probe failures (e.g. during deferred probing or error
injection) would accumulate leaked OF nodes.
### Step 8.3: Failure Mode Severity
- **Failure mode**: OF node reference count leak (resource leak)
- **Severity**: LOW-MEDIUM — repeated leaks consume memory, and the
leaked OF node can never be freed, but this is a one-time probe path,
not a hot path
### Step 8.4: Risk-Benefit Ratio
- **Benefit**: Fixes a real resource leak that has existed for 20 years.
Trivial to understand and verify.
- **Risk**: Essentially zero — 3 lines added to an error path, one
`goto` target changed
- **Ratio**: Very favorable — minimal risk for a correct bug fix
---
## PHASE 9: FINAL SYNTHESIS
### Step 9.1: Evidence Compiled
**FOR backporting:**
- Fixes a real bug: missing `of_node_put()` causing OF node reference
leak
- Extremely small and surgical fix (3 lines added, 1 changed)
- Obviously correct — verified by reading the code and comparing with
the remove path
- Applied by ALSA subsystem maintainer (Takashi Iwai)
- Bug exists in all stable trees (introduced in 2006)
- Zero regression risk
- Consistent with similar fixes applied to the same subsystem
(222bce5eb88d1)
**AGAINST backporting:**
- Low-severity bug (resource leak, not crash/security/corruption)
- Affects niche hardware (Apple PowerPC Macs)
- Only triggers on probe failure (not common under normal conditions)
### Step 9.2: Stable Rules Checklist
1. Obviously correct and tested? **YES** — trivial fix, applied by
maintainer
2. Fixes a real bug? **YES** — OF node reference leak
3. Important issue? **MEDIUM** — resource leak, not critical
4. Small and contained? **YES** — 3 lines, single file, single function
5. No new features or APIs? **YES** — pure fix
6. Can apply to stable trees? **YES** — code unchanged since 2006
### Step 9.3: Exception Categories
None applicable, but this is a standard reference counting bug fix — a
very common category for stable backports.
### Step 9.4: Decision
This is a small, obvious, correct fix for a real reference counting bug.
While the severity is low (resource leak on error path in niche
hardware), the fix carries essentially zero regression risk and meets
all stable kernel criteria.
---
## Verification
- [Phase 1] Parsed tags: Signed-off-by author + Takashi Iwai
(maintainer), Link tag present
- [Phase 2] Diff analysis: 3 lines added to error path in
`onyx_i2c_probe()`, adds missing `of_node_put()` after `of_node_get()`
at line 980
- [Phase 3] git blame: buggy code introduced in `f3d9478b2ce468`
(2006-06-21), present in all stable trees
- [Phase 3] Related fix `222bce5eb88d1`: same bug class fixed in `gpio-
feature.c` but NOT in `onyx.c`
- [Phase 3] Confirmed `tas.c` has identical unfixed bug at lines 864-875
- [Phase 4] Lore blocked by anti-scraping; b4 dig failed to match. Patch
applied by ALSA maintainer confirms review
- [Phase 5] `onyx_i2c_probe()` is standard I2C probe callback, called
during device binding
- [Phase 5] `aoa_codec_register()` at `sound/aoa/core/core.c:57`
confirmed: can return error from `attach_codec_to_fabric()`
- [Phase 6] Code structure unchanged since 2006; patch should apply
cleanly to all stable trees
- [Phase 6] No related fix already in stable for this specific file
- [Phase 8] Failure mode: OF node reference leak, severity LOW-MEDIUM;
risk of fix: essentially zero
**YES**
sound/aoa/codecs/onyx.c | 4 +++-
1 file changed, 3 insertions(+), 1 deletion(-)
diff --git a/sound/aoa/codecs/onyx.c b/sound/aoa/codecs/onyx.c
index 04961c456d2c5..da0eebf5dfbc2 100644
--- a/sound/aoa/codecs/onyx.c
+++ b/sound/aoa/codecs/onyx.c
@@ -980,10 +980,12 @@ static int onyx_i2c_probe(struct i2c_client *client)
onyx->codec.node = of_node_get(node);
if (aoa_codec_register(&onyx->codec)) {
- goto fail;
+ goto fail_put;
}
printk(KERN_DEBUG PFX "created and attached onyx instance\n");
return 0;
+ fail_put:
+ of_node_put(onyx->codec.node);
fail:
kfree(onyx);
return -ENODEV;
--
2.53.0
^ permalink raw reply related
* [PATCH AUTOSEL 7.0-5.10] ASoC: tas2552: Allow audio enable GPIO to sleep
From: Sasha Levin @ 2026-04-20 13:16 UTC (permalink / raw)
To: patches, stable
Cc: Marek Vasut, Mark Brown, Sasha Levin, shenghao-ding, kevin-lu,
baojun.xu, lgirdwood, perex, tiwai, linux-sound, linux-kernel
From: Marek Vasut <marex@nabladev.com>
[ Upstream commit 5ebc20921b7fff9feb44de465448e17a382c9965 ]
The audio enable GPIO is not toggled in any critical section where it
could not sleep, allow the audio enable GPIO to sleep. This allows the
driver to operate the audio enable GPIO connected to I2C GPIO expander.
Signed-off-by: Marek Vasut <marex@nabladev.com>
Link: https://patch.msgid.link/20260220202332.241035-1-marex@nabladev.com
Signed-off-by: Mark Brown <broonie@kernel.org>
Signed-off-by: Sasha Levin <sashal@kernel.org>
---
LLM Generated explanations, may be completely bogus:
Now I have all the information needed for the full analysis.
---
## PHASE 1: COMMIT MESSAGE FORENSICS
**Step 1.1: Subject Line**
- Subsystem: `ASoC: tas2552` (sound, ASoC codec driver)
- Action verb: "Allow" - implies enabling something previously not
supported
- Summary: Allow the enable GPIO to sleep, enabling use with I2C GPIO
expanders
**Step 1.2: Tags**
- Signed-off-by: Marek Vasut <marex@nabladev.com> (author)
- Link:
https://patch.msgid.link/20260220202332.241035-1-marex@nabladev.com
- Signed-off-by: Mark Brown <broonie@kernel.org> (ASoC subsystem
maintainer)
- No Fixes: tag, no Reported-by, no Cc: stable (expected for autosel
candidates)
**Step 1.3: Commit Body**
The commit describes that the enable GPIO is never toggled from atomic
context, so it's safe to use the sleeping variant. This allows the
driver to work when the enable GPIO is connected to an I2C GPIO expander
(which requires sleeping for bus access).
**Step 1.4: Hidden Bug Fix Detection**
YES - this is a bug fix. Using `gpiod_set_value()` with a sleeping GPIO
triggers `WARN_ON(desc->gdev->can_sleep)` in gpiolib.c:3899. This is
incorrect API usage that produces kernel warnings.
## PHASE 2: DIFF ANALYSIS
**Step 2.1: Inventory**
- 1 file changed: `sound/soc/codecs/tas2552.c`
- 5 lines changed (identical substitution at each site)
- Functions modified: `tas2552_runtime_suspend`,
`tas2552_runtime_resume`, `tas2552_component_probe` (x2),
`tas2552_component_remove`
**Step 2.2: Code Flow Change**
Each hunk is identical: `gpiod_set_value(tas2552->enable_gpio, X)` ->
`gpiod_set_value_cansleep(tas2552->enable_gpio, X)`. No logic change —
both APIs call the same `gpiod_set_value_nocheck()` internally.
**Step 2.3: Bug Mechanism**
Verified in `drivers/gpio/gpiolib.c`:
```3895:3901:drivers/gpio/gpiolib.c
int gpiod_set_value(struct gpio_desc *desc, int value)
{
VALIDATE_DESC(desc);
/* Should be using gpiod_set_value_cansleep() */
WARN_ON(desc->gdev->can_sleep);
return gpiod_set_value_nocheck(desc, value);
}
```
vs:
```4359:4364:drivers/gpio/gpiolib.c
int gpiod_set_value_cansleep(struct gpio_desc *desc, int value)
{
might_sleep();
VALIDATE_DESC(desc);
return gpiod_set_value_nocheck(desc, value);
}
```
The bug: When the enable GPIO is on an I2C GPIO expander (`can_sleep =
true`), `gpiod_set_value()` fires `WARN_ON` producing a kernel warning
with stack trace on every suspend/resume cycle and on probe/remove.
**Step 2.4: Fix Quality**
- Obviously correct: the only change is which wrapper is used; both call
the same underlying function
- Minimal: 5 identical one-line substitutions
- Zero regression risk: `gpiod_set_value_cansleep()` is strictly more
permissive (works with both sleeping and non-sleeping GPIOs)
- All call sites are process context (PM callbacks, probe, remove) where
sleeping is allowed
## PHASE 3: GIT HISTORY
**Step 3.1: Blame**
The `gpiod_set_value()` calls were introduced by commit `82cf77a1bd61d9`
(Axel Lin, 2015) which simplified NULL checks. The original code existed
since `5df7f71d5cdfbc` (Dan Murphy, 2014). The buggy code has been
present since v4.3-rc1.
**Step 3.2: Fixes tag**
No Fixes: tag present (expected for autosel).
**Step 3.3: File History**
Recent changes to the file are trivial: RUNTIME_PM_OPS conversion,
removing redundant `pm_runtime_mark_last_busy()`, dropping unused GPIO
includes. No conflicts.
**Step 3.4: Author**
Marek Vasut is a prolific kernel contributor with extensive work across
DRM, DT bindings, and sound subsystems. Not the TAS2552 maintainer but a
well-known contributor.
**Step 3.5: Prerequisites**
None. The change is standalone and independent of the RUNTIME_PM_OPS
conversion. It touches only the `gpiod_set_value()` calls which exist in
all stable trees.
## PHASE 4: MAILING LIST RESEARCH
**Step 4.1: Original Patch**
Found via `b4 am`. The patch was submitted as a single standalone patch
on 2026-02-20. CC'd appropriate maintainers (Mark Brown, Takashi Iwai,
TI engineers, linux-sound, linux-kernel). Applied directly by Mark Brown
(ASoC maintainer). No v2/v3 revisions — accepted as-is.
**Step 4.2: Reviewers**
The patch was CC'd to all relevant TI and ASoC maintainers. Mark Brown
(ASoC subsystem maintainer) applied it directly.
**Step 4.3: Bug Report**
No external bug report. Marek Vasut likely encountered this on a board
with an I2C GPIO expander.
**Step 4.4: Related Patches**
This is a well-established pattern. Multiple identical fixes have been
applied to other ASoC codecs:
- `5f83ee4b1f0c0` ASoC: tas5086: use sleeping variants of gpiod API
- `897d8e86bac76` ASoC: tlv320aic31xx: switch to
gpiod_set_value_cansleep
- `5d7e0b1516dfc` ASoC: dmic: Allow GPIO operations to sleep
- `ea2a2ad17ca1e` ASoC: dio2125: use gpiod_set_value_cansleep (had
Fixes: tag)
**Step 4.5: Stable Discussion**
No stable-specific discussion found. The dio2125 variant (ea2a2ad17ca1e)
had a Fixes: tag and was likely auto-selected for stable.
## PHASE 5: CODE SEMANTIC ANALYSIS
**Step 5.1: Functions Modified**
- `tas2552_runtime_suspend()` — PM runtime callback
- `tas2552_runtime_resume()` — PM runtime callback
- `tas2552_component_probe()` — ASoC component probe (x2 sites)
- `tas2552_component_remove()` — ASoC component remove
**Step 5.2: Callers**
All five call sites are invoked from process context:
- Runtime PM callbacks are invoked by the PM subsystem in process
context
- Component probe/remove are called from the ASoC registration path,
always sleepable
**Step 5.3-5.4: No atomic context concerns**
All callers can sleep. The `gpiod_set_value_cansleep()` API with its
`might_sleep()` is the correct choice.
**Step 5.5: Similar Patterns**
There are 5 remaining `gpiod_set_value()` calls in this file — this
patch converts all of them. Other ASoC drivers have undergone identical
transformations.
## PHASE 6: STABLE TREE ANALYSIS
**Step 6.1: Buggy Code in Stable?**
YES. The `gpiod_set_value()` calls date back to commit `82cf77a1bd61d9`
(v4.3-rc1, 2015). All active stable trees contain this code.
**Step 6.2: Backport Complications**
Minimal. The `gpiod_set_value()` lines are identical across all stable
versions. The only difference is that pre-6.12 trees have `#ifdef
CONFIG_PM` guards and `SET_RUNTIME_PM_OPS` instead of `RUNTIME_PM_OPS`,
but this doesn't affect the changed lines. The patch should apply
cleanly or with trivial context offset.
**Step 6.3: No Existing Fix**
No related fix for this specific issue in any stable tree.
## PHASE 7: SUBSYSTEM CONTEXT
**Step 7.1:** Sound/ASoC codec driver — PERIPHERAL criticality (specific
codec driver), but TAS2552 is used in embedded systems.
**Step 7.2:** The driver is mature/stable with minimal recent activity
(only cleanup commits).
## PHASE 8: IMPACT AND RISK ASSESSMENT
**Step 8.1: Affected Users**
Users of the TAS2552 audio amplifier who have the enable GPIO connected
to an I2C GPIO expander. This is a valid embedded hardware
configuration.
**Step 8.2: Trigger Conditions**
The WARN_ON triggers on EVERY suspend/resume cycle and on driver
probe/remove — not rare at all for affected hardware.
**Step 8.3: Failure Mode**
- WARN_ON with full stack trace in kernel log (MEDIUM severity normally)
- With `panic_on_warn=1`: kernel panic (CRITICAL severity for those
configurations)
- Without panic_on_warn: console spam, log pollution, incorrect API
usage flagged
**Step 8.4: Risk-Benefit**
- BENEFIT: Eliminates WARN_ON spam, enables I2C GPIO expander hardware
correctly, fixes incorrect API usage
- RISK: Virtually zero — both APIs call the same underlying function,
the only difference is the debug assertion
## PHASE 9: FINAL SYNTHESIS
**Step 9.1: Evidence Summary**
FOR backporting:
- Fixes incorrect GPIO API usage that triggers WARN_ON
- Can cause kernel panic with `panic_on_warn=1`
- Trivially small (5 identical line substitutions), trivially correct
- Zero regression risk (both APIs have identical functional behavior)
- Well-established pattern applied to many other drivers
- Applied by ASoC subsystem maintainer Mark Brown
- Buggy code present since v4.3 — affects all stable trees
- No dependencies on other patches
AGAINST backporting:
- No Reported-by (no evidence of real-world user reports)
- WARN_ON is non-fatal in default configurations
- Affects specific hardware configuration (I2C GPIO expander)
- Could be seen as "enablement" rather than pure "fix"
**Step 9.2: Stable Rules Checklist**
1. Obviously correct and tested: YES — trivially provable
2. Fixes a real bug: YES — WARN_ON from incorrect API usage
3. Important issue: MEDIUM — WARN_ON, potential panic with panic_on_warn
4. Small and contained: YES — 5 lines in 1 file
5. No new features: YES — just API correction
6. Can apply to stable: YES — clean apply expected
**Step 9.3: Exception Categories**
This is closest to a hardware quirk/workaround — it makes the driver
work correctly with I2C GPIO expanders.
## Verification
- [Phase 1] Parsed tags: Signed-off-by from author and ASoC maintainer,
Link to patch.msgid
- [Phase 2] Diff analysis: 5 identical `gpiod_set_value` ->
`gpiod_set_value_cansleep` substitutions
- [Phase 2] Verified `gpiod_set_value()` has
`WARN_ON(desc->gdev->can_sleep)` at gpiolib.c:3899
- [Phase 2] Verified `gpiod_set_value_cansleep()` calls same
`gpiod_set_value_nocheck()` at gpiolib.c:4363
- [Phase 3] git blame: `gpiod_set_value` calls introduced by commit
82cf77a1bd61d9 (v4.3-rc1, 2015)
- [Phase 3] git log: no conflicting changes in recent history
- [Phase 3] RUNTIME_PM_OPS conversion (1570c33f2f38b) does not affect
the changed lines
- [Phase 4] b4 am: found original patch, single standalone submission
- [Phase 4] Multiple identical fixes applied to other ASoC codecs
(tas5086, tlv320aic31xx, dmic, dio2125)
- [Phase 5] All call sites (runtime_suspend/resume,
component_probe/remove) are process context — can sleep
- [Phase 6] Buggy code present since v4.3, exists in all active stable
trees
- [Phase 6] Patch applies cleanly — no conflicting changes to the
affected lines
- [Phase 8] WARN_ON triggers on every suspend/resume for affected
hardware, not a rare event
The fix is trivially correct, carries zero regression risk, and fixes
incorrect GPIO API usage that produces kernel warnings (and potential
panics) for valid hardware configurations. While it's not fixing a crash
in the default configuration, the WARN_ON is a genuine bug indicator,
and this well-established pattern has been applied to many other
drivers.
**YES**
sound/soc/codecs/tas2552.c | 10 +++++-----
1 file changed, 5 insertions(+), 5 deletions(-)
diff --git a/sound/soc/codecs/tas2552.c b/sound/soc/codecs/tas2552.c
index 43449d7c25843..80206c2e09462 100644
--- a/sound/soc/codecs/tas2552.c
+++ b/sound/soc/codecs/tas2552.c
@@ -487,7 +487,7 @@ static int tas2552_runtime_suspend(struct device *dev)
regcache_cache_only(tas2552->regmap, true);
regcache_mark_dirty(tas2552->regmap);
- gpiod_set_value(tas2552->enable_gpio, 0);
+ gpiod_set_value_cansleep(tas2552->enable_gpio, 0);
return 0;
}
@@ -496,7 +496,7 @@ static int tas2552_runtime_resume(struct device *dev)
{
struct tas2552_data *tas2552 = dev_get_drvdata(dev);
- gpiod_set_value(tas2552->enable_gpio, 1);
+ gpiod_set_value_cansleep(tas2552->enable_gpio, 1);
tas2552_sw_shutdown(tas2552, 0);
@@ -583,7 +583,7 @@ static int tas2552_component_probe(struct snd_soc_component *component)
return ret;
}
- gpiod_set_value(tas2552->enable_gpio, 1);
+ gpiod_set_value_cansleep(tas2552->enable_gpio, 1);
ret = pm_runtime_resume_and_get(component->dev);
if (ret < 0) {
@@ -608,7 +608,7 @@ static int tas2552_component_probe(struct snd_soc_component *component)
probe_fail:
pm_runtime_put_noidle(component->dev);
- gpiod_set_value(tas2552->enable_gpio, 0);
+ gpiod_set_value_cansleep(tas2552->enable_gpio, 0);
regulator_bulk_disable(ARRAY_SIZE(tas2552->supplies),
tas2552->supplies);
@@ -621,7 +621,7 @@ static void tas2552_component_remove(struct snd_soc_component *component)
pm_runtime_put(component->dev);
- gpiod_set_value(tas2552->enable_gpio, 0);
+ gpiod_set_value_cansleep(tas2552->enable_gpio, 0);
};
#ifdef CONFIG_PM
--
2.53.0
^ permalink raw reply related
* [PATCH AUTOSEL 7.0-5.10] ASoC: Intel: bytcr_rt5651: Fix MCLK leak on platform_clock_control error
From: Sasha Levin @ 2026-04-20 13:16 UTC (permalink / raw)
To: patches, stable
Cc: Aravind Anilraj, Cezary Rojewski, Mark Brown, Sasha Levin,
liam.r.girdwood, peter.ujfalusi, yung-chuan.liao,
ranjani.sridharan, kai.vehmanen, perex, tiwai, linux-sound,
linux-kernel
In-Reply-To: <20260420132314.1023554-1-sashal@kernel.org>
From: Aravind Anilraj <aravindanilraj0702@gmail.com>
[ Upstream commit b022e5c142efe4c5497e6cfda1f143618b4b9254 ]
If byt_rt5651_prepare_and_enable_pll1() fails, the function returns
without calling clk_disable_unprepare() on priv->mclk, which was
already enabled earlier in the same code path. Add the missing
cleanup call to prevent the clock from leaking.
Signed-off-by: Aravind Anilraj <aravindanilraj0702@gmail.com>
Reviewed-by: Cezary Rojewski <cezary.rojewski@intel.com>
Link: https://patch.msgid.link/20260401220507.23557-3-aravindanilraj0702@gmail.com
Signed-off-by: Mark Brown <broonie@kernel.org>
Signed-off-by: Sasha Levin <sashal@kernel.org>
---
LLM Generated explanations, may be completely bogus:
Now I have all the information needed. Let me compile the full analysis.
---
## PHASE 1: COMMIT MESSAGE FORENSICS
### Step 1.1: Subject Line
- **Subsystem**: ASoC: Intel: bytcr_rt5651
- **Action verb**: "Fix" — explicitly a bug fix
- **Summary**: Fix clock (MCLK) resource leak on error path in
`platform_clock_control`
### Step 1.2: Tags
- **Signed-off-by**: Aravind Anilraj (author, community contributor — no
prior commits in this subsystem)
- **Reviewed-by**: Cezary Rojewski (Intel audio maintainer)
- **Link**: patch.msgid.link/20260401220507.23557-3 — patch 3 in a
series
- **Signed-off-by**: Mark Brown (ASoC subsystem maintainer — applied the
patch)
- No Fixes: tag, no Cc: stable, no Reported-by (all expected for autosel
review)
### Step 1.3: Commit Body
The message clearly describes: if `byt_rt5651_prepare_and_enable_pll1()`
fails, the function returns without calling `clk_disable_unprepare()` on
`priv->mclk`, which was already enabled by `clk_prepare_enable()`. This
is a straightforward clock resource leak on an error path.
### Step 1.4: Hidden Bug Fix?
No — this is explicitly labeled as a bug fix and is genuinely one. The
commit message directly describes the resource leak mechanism.
## PHASE 2: DIFF ANALYSIS
### Step 2.1: Inventory
- **Files changed**: 1 (`sound/soc/intel/boards/bytcr_rt5651.c`)
- **Lines added**: 2 (`+if (ret < 0)` and
`+clk_disable_unprepare(priv->mclk);`)
- **Lines removed**: 0
- **Function modified**: `platform_clock_control()`
- **Scope**: Single-file, single-function, 2-line surgical fix
### Step 2.2: Code Flow Change
**Before**: In the `SND_SOC_DAPM_EVENT_ON` branch:
1. `clk_prepare_enable(priv->mclk)` — enables the clock
2. `byt_rt5651_prepare_and_enable_pll1()` — configures PLL
3. If step 2 fails, `ret < 0` falls through to the error path at line
225, which logs the error and returns — **without disabling the
clock**
**After**: If `byt_rt5651_prepare_and_enable_pll1()` fails,
`clk_disable_unprepare(priv->mclk)` is called immediately, properly
balancing the earlier `clk_prepare_enable()`.
### Step 2.3: Bug Mechanism
**Category**: Resource leak (clock) on error path.
- `clk_prepare_enable()` increments the clock's reference count
- On PLL1 failure, the corresponding `clk_disable_unprepare()` was never
called
- The clock remains permanently enabled, leaking the resource
### Step 2.4: Fix Quality
- **Obviously correct**: Yes — directly mirrors the existing cleanup in
the `else` branch (line 221-222)
- **Minimal**: Yes — 2 lines, no unnecessary changes
- **Regression risk**: Essentially zero — only executes on an existing
error path
## PHASE 3: GIT HISTORY INVESTIGATION
### Step 3.1: Blame
- Line 206 (`clk_prepare_enable`): Refactored by `a8627df5491e00` (Andy
Shevchenko, 2021-10-07) — but the original logic dates to
`02c0a3b3047f8f` (Pierre-Louis Bossart, 2017-10-12)
- Line 211 (`byt_rt5651_prepare_and_enable_pll1`): Introduced by
`aeec6cc0821573` (Hans de Goede, 2018-03-04) — **this is when the bug
was introduced**. The PLL1 call was added between the clock enable and
the end of the branch, without error handling for the clock.
### Step 3.2: Fixes Target
No explicit Fixes: tag. The implicit fix target is `aeec6cc0821573`
("ASoC: Intel: bytcr_rt5651: Configure PLL1 before using it",
v4.17-rc1). This commit is present in **all active stable trees** (it
dates to 2018).
### Step 3.3: Related Changes
The file has had several unrelated changes since the bug was introduced,
but none touch the specific error path being fixed. The fix applies
cleanly.
### Step 3.4: Author
Aravind Anilraj appears to be a community contributor (no other commits
in this subsystem found). However, the patch was **Reviewed-by** Cezary
Rojewski (Intel audio maintainer) and **merged by** Mark Brown (ASoC
maintainer).
### Step 3.5: Dependencies
None. The fix is completely standalone — it references only `priv->mclk`
and `clk_disable_unprepare()`, both of which have existed since the
original code. No prerequisites needed.
## PHASE 4: MAILING LIST AND EXTERNAL RESEARCH
### Step 4.1-4.2: Patch Discussion
The Link tag indicates this is patch 3 in a series (message-id contains
"23557-3"). Lore.kernel.org was blocked by anti-bot protection,
preventing direct discussion retrieval. However:
- The patch was reviewed by Intel's audio maintainer (Cezary Rojewski)
- Merged by the ASoC subsystem maintainer (Mark Brown)
- Both are strong trust indicators
### Step 4.3-4.5
No explicit bug report or syzbot link — this appears to be found by code
inspection. No previous stable discussion found.
## PHASE 5: CODE SEMANTIC ANALYSIS
### Step 5.1-5.2: Function Context
`platform_clock_control()` is registered as a DAPM supply callback:
```253:255:sound/soc/intel/boards/bytcr_rt5651.c
SND_SOC_DAPM_SUPPLY("Platform Clock", SND_SOC_NOPM, 0, 0,
platform_clock_control, SND_SOC_DAPM_PRE_PMU
|
SND_SOC_DAPM_POST_PMD),
```
This is called every time the audio path is powered up (PRE_PMU) or down
(POST_PMD). It is a **common path** for any user of this audio hardware.
### Step 5.3: Callees
- `clk_prepare_enable()` / `clk_disable_unprepare()`: standard Linux
clock framework
- `byt_rt5651_prepare_and_enable_pll1()`: configures PLL via
`snd_soc_dai_set_pll()` and `snd_soc_dai_set_sysclk()` — can fail if
the codec rejects the configuration
### Step 5.4: Sibling Pattern Confirmation
The sibling driver `bytcr_rt5640.c` has the **identical bug** at lines
285-291:
```285:291:sound/soc/intel/boards/bytcr_rt5640.c
if (SND_SOC_DAPM_EVENT_ON(event)) {
ret = clk_prepare_enable(priv->mclk);
if (ret < 0) {
dev_err(card->dev, "could not configure MCLK
state\n");
return ret;
}
ret = byt_rt5640_prepare_and_enable_pll1(codec_dai,
48000);
```
No `clk_disable_unprepare()` on PLL1 failure there either. This confirms
the bug pattern is real and systematic.
## PHASE 6: STABLE TREE ANALYSIS
### Step 6.1: Code in Stable
The buggy code was introduced in `aeec6cc0821573` (v4.17-rc1, March
2018). This code exists in **all active stable trees** (5.4.y, 5.10.y,
5.15.y, 6.1.y, 6.6.y, 6.12.y).
### Step 6.2: Backport Complications
The fix is 2 lines with minimal context sensitivity. The surrounding
code has been stable since the a8627df5491e00 refactoring in 2021. It
should apply cleanly to all trees from 5.15+ at minimum; older trees may
need trivial context adjustment for the `BYT_RT5651_MCLK_EN` quirk check
that was removed by `a8627df5491e00`.
### Step 6.3: No related fixes for this bug already in stable.
## PHASE 7: SUBSYSTEM AND MAINTAINER CONTEXT
- **Subsystem**: ASoC / Intel audio machine driver
- **Criticality**: PERIPHERAL — affects users of Bay Trail / Cherry
Trail devices with RT5651 codec (budget tablets, laptops)
- **Activity**: Moderate — file sees occasional updates
## PHASE 8: IMPACT AND RISK ASSESSMENT
### Step 8.1: Affected Users
Users with Intel Bay Trail or Cherry Trail devices using the RT5651
audio codec. These are common budget tablets and laptops.
### Step 8.2: Trigger Conditions
Triggered when: (a) audio starts playing (DAPM PRE_PMU event), AND (b)
PLL1 configuration fails. While PLL failure is itself an error
condition, repeated failures with clock leaks can compound the problem
and prevent power management from working correctly.
### Step 8.3: Failure Mode
- **Severity: MEDIUM** — The leaked clock prevents the MCLK from being
disabled, which:
- Wastes power (clock stays enabled)
- May prevent subsequent clock operations from working correctly
- Could contribute to audio subsystem malfunction after error recovery
### Step 8.4: Risk-Benefit
- **Benefit**: Fixes a real resource leak, prevents clock reference
count imbalance
- **Risk**: Essentially zero — 2-line addition only on error path,
obviously correct
- **Ratio**: Very favorable for backport
## PHASE 9: FINAL SYNTHESIS
### Step 9.1: Evidence Summary
**FOR backporting:**
- Fixes a real clock resource leak on error path
- 2-line fix, minimal and surgical
- Obviously correct (mirrors existing cleanup pattern in the else
branch)
- Reviewed by Intel audio maintainer, merged by ASoC subsystem
maintainer
- Bug exists since v4.17, present in all active stable trees
- No dependencies, completely standalone
- Sibling driver (bytcr_rt5640) has the identical bug, confirming the
pattern
**AGAINST backporting:**
- The trigger requires PLL1 configuration failure (uncommon in normal
operation)
- No user reports or syzbot bugs — found by code inspection
- Author is a community contributor with no other commits in this
subsystem
### Step 9.2: Stable Rules Checklist
1. Obviously correct and tested? **YES** — trivially correct, reviewed
by maintainer
2. Fixes a real bug? **YES** — clock resource leak on error path
3. Important issue? **MEDIUM** — resource leak, not a crash
4. Small and contained? **YES** — 2 lines in 1 file
5. No new features/APIs? **YES** — no new functionality
6. Applies to stable trees? **YES** — code unchanged, clean apply
expected
### Step 9.3: Exception Categories
Not applicable — this is a standard bug fix, not an exception category.
---
## Verification
- [Phase 1] Parsed tags: Reviewed-by Cezary Rojewski (Intel), Signed-
off-by Mark Brown (ASoC maintainer)
- [Phase 2] Diff analysis: 2 lines added to error path in
`platform_clock_control()`, adds missing `clk_disable_unprepare()`
- [Phase 3] git blame: buggy code path introduced in `aeec6cc0821573`
(v4.17-rc1, 2018-03-04), present in all stable trees
- [Phase 3] git describe: confirmed `aeec6cc0821573` first appeared in
v4.17-rc1
- [Phase 3] git log: no prior fix for this issue exists
- [Phase 4] lore.kernel.org: blocked by anti-bot protection, could not
read discussion
- [Phase 4] b4 dig: confirmed PLL1 commit is patch v3 15/22 from Hans de
Goede series
- [Phase 5] Verified `platform_clock_control()` is a DAPM supply
callback (line 253-255), called on every audio path enable/disable
- [Phase 5] Verified sibling `bytcr_rt5640.c` has identical bug pattern
at lines 285-291
- [Phase 6] Code exists in all active stable trees (bug from v4.17,
2018)
- [Phase 8] Failure mode: clock resource leak preventing proper power
management, severity MEDIUM
- UNVERIFIED: Could not read mailing list discussion due to anti-bot
protection; relied on tags in commit message for review assessment
The fix is small, surgical, obviously correct, and meets all stable
kernel criteria. It fixes a genuine resource leak with zero regression
risk.
**YES**
sound/soc/intel/boards/bytcr_rt5651.c | 2 ++
1 file changed, 2 insertions(+)
diff --git a/sound/soc/intel/boards/bytcr_rt5651.c b/sound/soc/intel/boards/bytcr_rt5651.c
index 68cf463f1d507..8932fc5d6f4f2 100644
--- a/sound/soc/intel/boards/bytcr_rt5651.c
+++ b/sound/soc/intel/boards/bytcr_rt5651.c
@@ -209,6 +209,8 @@ static int platform_clock_control(struct snd_soc_dapm_widget *w,
return ret;
}
ret = byt_rt5651_prepare_and_enable_pll1(codec_dai, 48000, 50);
+ if (ret < 0)
+ clk_disable_unprepare(priv->mclk);
} else {
/*
* Set codec clock source to internal clock before
--
2.53.0
^ permalink raw reply related
* [PATCH AUTOSEL 7.0-6.18] ALSA: usb-audio: Add iface reset and delay quirk for HUAWEI USB-C HEADSET
From: Sasha Levin @ 2026-04-20 13:17 UTC (permalink / raw)
To: patches, stable
Cc: Lianqin Hu, Takashi Iwai, Sasha Levin, perex, tiwai, linux-sound,
linux-kernel
In-Reply-To: <20260420132314.1023554-1-sashal@kernel.org>
From: Lianqin Hu <hulianqin@vivo.com>
[ Upstream commit 9575766a682f50ec4bcb85ecd438685bdc09f9cc ]
Setting up the interface when suspended/resumeing fail on this card.
Adding a reset and delay quirk will eliminate this problem.
usb 1-1: new full-speed USB device number 2 using xhci-hcd
usb 1-1: New USB device found, idVendor=12d1, idProduct=3a07
usb 1-1: New USB device strings: Mfr=1, Product=2, SerialNumber=3
usb 1-1: Product: HUAWEI USB-C HEADSET
usb 1-1: Manufacturer: bestechnic
usb 1-1: SerialNumber: 0296C100000000000000000000000
Signed-off-by: Lianqin Hu <hulianqin@vivo.com>
Link: https://patch.msgid.link/TYUPR06MB62176A18EA7A9DD0AC2826BCD2582@TYUPR06MB6217.apcprd06.prod.outlook.com
Signed-off-by: Takashi Iwai <tiwai@suse.de>
Signed-off-by: Sasha Levin <sashal@kernel.org>
---
LLM Generated explanations, may be completely bogus:
Now I have sufficient information for a thorough analysis. Let me
compile everything.
---
## PHASE 1: COMMIT MESSAGE FORENSICS
### Step 1.1: Parse the Subject Line
- **Subsystem**: `ALSA: usb-audio`
- **Action verb**: "Add" (adding quirk flags)
- **Summary**: Adds interface reset and delay quirk for HUAWEI USB-C
HEADSET (0x12d1:0x3a07)
Record: [ALSA: usb-audio] [Add] [Hardware quirk flags for suspend/resume
fix on HUAWEI USB-C HEADSET]
### Step 1.2: Parse All Commit Message Tags
- **Signed-off-by**: Lianqin Hu <hulianqin@vivo.com> (author)
- **Signed-off-by**: Takashi Iwai <tiwai@suse.de> (ALSA subsystem
maintainer)
- **Link**: https://patch.msgid.link/TYUPR06MB62176A18EA7A9DD0AC2826BCD2
582@TYUPR06MB6217.apcprd06.prod.outlook.com
- No Fixes: tag (expected for quirk addition)
- No Reported-by: tag (the author discovered the issue themselves)
- No Cc: stable tag
Record: Accepted by Takashi Iwai (ALSA subsystem maintainer). Author
works at vivo (mobile phone manufacturer - plausible USB-C headset
user).
### Step 1.3: Analyze the Commit Body Text
The commit says: "Setting up the interface when suspended/resumeing fail
on this card." This describes a concrete bug: the USB audio interface
setup fails during suspend/resume cycles. The fix is adding
`QUIRK_FLAG_FORCE_IFACE_RESET` and `QUIRK_FLAG_IFACE_DELAY` flags. The
USB device info (VID/PID, manufacturer "bestechnic", product "HUAWEI
USB-C HEADSET") is included for identification.
Record: Bug = interface setup failure during suspend/resume. Symptom =
audio device non-functional after suspend/resume. Root cause = device
requires an interface reset and a 50ms delay during interface setup.
### Step 1.4: Detect Hidden Bug Fixes
This is not hidden - it's an explicit hardware workaround for a device
that fails during suspend/resume. This is a textbook USB audio quirk.
Record: Not a hidden bug fix; it's an explicit hardware
quirk/workaround.
---
## PHASE 2: DIFF ANALYSIS
### Step 2.1: Inventory the Changes
- **Files changed**: 1 (`sound/usb/quirks.c`)
- **Lines**: -2 / +3 (net +1 line)
- **Functions modified**: None - only a data table entry is changed
- **Scope**: Single-line modification to an existing quirk table entry
Record: Extremely minimal change. Only the existing DEVICE_FLG entry for
0x12d1, 0x3a07 is modified.
### Step 2.2: Understand the Code Flow Change
- **Before**: `QUIRK_FLAG_MIXER_PLAYBACK_MIN_MUTE |
QUIRK_FLAG_MIXER_CAPTURE_MIN_MUTE`
- **After**: `QUIRK_FLAG_MIXER_PLAYBACK_MIN_MUTE |
QUIRK_FLAG_MIXER_CAPTURE_MIN_MUTE | QUIRK_FLAG_FORCE_IFACE_RESET |
QUIRK_FLAG_IFACE_DELAY`
- Also updated the comment from "Huawei Technologies Co., Ltd." to
"HUAWEI USB-C HEADSET" (cosmetic)
The two new flags are consumed in existing code paths:
- `QUIRK_FLAG_IFACE_DELAY` causes a 50ms sleep in
`snd_usb_endpoint_set_interface()` (endpoint.c:942-943) and in
`snd_usb_init_sample_rate()` (clock.c:649-650)
- `QUIRK_FLAG_FORCE_IFACE_RESET` forces `need_prepare = true` and
`need_setup = true` when stopping a stream (endpoint.c:1695-1700)
Record: Data-only change adding well-established flags to an existing
device entry. No logic changes.
### Step 2.3: Identify the Bug Mechanism
Category: **(h) Hardware workarounds**
- This is a device-specific quirk table entry modification
- The HUAWEI USB-C HEADSET (Bestechnic chipset) requires both an
interface reset and a delay for proper operation during suspend/resume
Record: Hardware quirk. The device's USB audio firmware doesn't handle
interface re-setup correctly without a forced reset and delay.
### Step 2.4: Assess the Fix Quality
- Obviously correct: adds flags to a static data table, no logic change
- Minimal/surgical: 3 lines modified in a single entry
- Regression risk: Essentially zero. These flags are already used by 10+
other devices. The only effect is a 50ms delay and a forced interface
reset for this specific device.
Record: Fix quality = excellent. Regression risk = negligible.
---
## PHASE 3: GIT HISTORY INVESTIGATION
### Step 3.1: Blame the Changed Lines
From git blame:
- `2cbe4ac193ed71` (qaqland, 2025-08-29): Added initial entry for
0x12d1, 0x3a07 with `QUIRK_FLAG_MIXER_PLAYBACK_MIN_MUTE`
- `806a38293fc0df` (Cryolitia PukNgae, 2025-09-03): Added
`QUIRK_FLAG_MIXER_CAPTURE_MIN_MUTE`
The device entry was present since kernel ~6.17.
Record: Device entry exists in tree since ~v6.17. The flags being added
(FORCE_IFACE_RESET since 2022, IFACE_DELAY since 2021) are both long-
established.
### Step 3.2: Follow the Fixes: Tag
No Fixes: tag present. This is expected for a quirk addition.
### Step 3.3: Check File History
Recent changes to `sound/usb/quirks.c` show 5 nearly identical commits
by the same author adding `QUIRK_FLAG_FORCE_IFACE_RESET |
QUIRK_FLAG_IFACE_DELAY` for other devices:
- AB13X USB Audio (2 variants)
- AB17X USB Audio
- SPACETOUCH USB Audio
- GHW-123P
Record: The author has submitted numerous identical-pattern quirk
patches, all accepted by Takashi Iwai. This is a well-established
pattern.
### Step 3.4: Check the Author's Other Commits
Lianqin Hu has 10+ commits in the USB audio area, almost all adding
delay/reset quirks for specific devices. They are clearly a regular
contributor for USB audio quirks, likely working at vivo on mobile
device compatibility.
Record: Regular USB audio quirk contributor with a track record of
accepted patches.
### Step 3.5: Check for Dependent/Prerequisite Commits
The prerequisite is that the device entry (0x12d1, 0x3a07) exists in the
target tree. This was added in ~v6.17 (commit 2cbe4ac193ed71). Both
quirk flags have existed since v5.x era.
Record: Requires the base entry (v6.17+). Both flags exist since v5.x.
---
## PHASE 4: MAILING LIST AND EXTERNAL RESEARCH
### Step 4.1-4.2: Find the Original Patch Discussion
The Link: in the commit points to the patch submission. The patch was
accepted directly by Takashi Iwai (ALSA subsystem maintainer). Web
search confirms the pattern: identical patches for AB13X were submitted
with the same message format and accepted.
Record: Accepted by subsystem maintainer. Standard quirk addition
pattern.
### Step 4.3-4.5: Bug Report and Stable Context
No separate bug report link. The author discovered the suspend/resume
failure with this device directly. Similar quirk additions have been
routinely backported to stable in the past.
Record: Self-reported by hardware tester at vivo.
---
## PHASE 5: CODE SEMANTIC ANALYSIS
### Step 5.1: Key Functions in the Diff
No functions modified - only a data table entry.
### Step 5.2-5.4: Trace Callers
The flags are consumed by:
1. `snd_usb_endpoint_set_interface()` in endpoint.c (IFACE_DELAY: adds
50ms sleep after usb_set_interface)
2. Stream stop path in endpoint.c (FORCE_IFACE_RESET: marks interface as
needing re-setup)
3. `snd_usb_init_sample_rate()` in clock.c (IFACE_DELAY: 50ms sleep
after rate change)
These are core USB audio paths that run during stream start/stop and
suspend/resume.
Record: The flags affect well-tested code paths in the USB audio stack.
---
## PHASE 6: CROSS-REFERENCING AND STABLE TREE ANALYSIS
### Step 6.1: Does the Buggy Code Exist in Stable Trees?
The device entry exists in 7.0. For older stable trees (6.6.y, 6.1.y,
etc.), the device entry does NOT exist (it was added in v6.17). So this
quirk would only be relevant for stable trees >= 6.17 (or wherever the
entry was backported to).
Record: Relevant for stable trees that contain the device entry
(v6.17+).
### Step 6.2: Backport Complications
The patch should apply cleanly to any tree that has the existing
DEVICE_FLG(0x12d1, 0x3a07, ...) entry. The diff is minimal.
Record: Clean apply expected for any tree with the base device entry.
---
## PHASE 7: SUBSYSTEM AND MAINTAINER CONTEXT
### Step 7.1: Subsystem Criticality
- **Subsystem**: sound/usb (USB audio driver)
- **Criticality**: IMPORTANT - USB-C headsets are very common on laptops
and phones
Record: USB audio = IMPORTANT subsystem. USB-C headsets are widely used.
### Step 7.2: Subsystem Activity
54 changes to `sound/usb/quirks.c` between v6.12 and v7.0. Extremely
active - this file gets frequent quirk additions.
Record: Highly active, many quirk additions routinely accepted.
---
## PHASE 8: IMPACT AND RISK ASSESSMENT
### Step 8.1: Who Is Affected
Users of HUAWEI USB-C HEADSET (VID 0x12d1, PID 0x3a07, manufactured by
Bestechnic). This is a branded headset likely sold with Huawei phones
but also usable on any USB-C Linux device.
Record: Device-specific. Affects all users of this specific HUAWEI USB-C
headset.
### Step 8.2: Trigger Conditions
Triggered on every suspend/resume cycle when the headset is connected.
Very common for laptop users.
Record: Common trigger - any suspend/resume with device connected.
### Step 8.3: Failure Mode Severity
Without the quirk, the interface setup fails during resume. The headset
stops working after suspend/resume, requiring re-plugging. This is a
functional failure.
Record: Severity = HIGH for affected users (device non-functional after
suspend/resume).
### Step 8.4: Risk-Benefit Ratio
- **Benefit**: Fixes suspend/resume for HUAWEI USB-C headset users
- **Risk**: Extremely low. Data table change only. The flags are used by
10+ other devices already. No logic changes.
Record: Very high benefit/risk ratio.
---
## PHASE 9: FINAL SYNTHESIS
### Step 9.1: Evidence Summary
**FOR backporting:**
- Classic hardware quirk addition to existing device entry
- Fixes real suspend/resume failure
- Tiny, data-only change (3 lines in a static table)
- Both flags are well-established (2021/2022 vintage)
- Same flags used by 10+ other devices successfully
- Accepted by ALSA subsystem maintainer (Takashi Iwai)
- Author has track record of identical, accepted quirk patches
- Zero regression risk (device-specific, data-only)
**AGAINST backporting:**
- No Fixes: tag (expected for quirks)
- Limited to one specific device
- Base device entry only exists in v6.17+
### Step 9.2: Stable Rules Checklist
1. Obviously correct and tested? **YES** - data table change, accepted
by maintainer
2. Fixes a real bug? **YES** - suspend/resume failure
3. Important issue? **YES** - device non-functional after suspend/resume
4. Small and contained? **YES** - 3 lines in one file
5. No new features or APIs? **YES** - just adds flags to existing entry
6. Can apply to stable trees? **YES** - clean apply for trees with the
base entry
### Step 9.3: Exception Categories
**YES** - This is a hardware quirk/workaround, one of the explicitly
listed exception categories that are automatically YES for stable.
---
## Verification
- [Phase 1] Parsed tags: Signed-off-by Takashi Iwai (ALSA maintainer),
Link to patch submission
- [Phase 2] Diff analysis: +1 net line, adds
QUIRK_FLAG_FORCE_IFACE_RESET | QUIRK_FLAG_IFACE_DELAY to existing
entry for 0x12d1:0x3a07
- [Phase 3] git blame: device entry introduced by 2cbe4ac193ed71 (v6.17
era), flags exist since 2021/2022
- [Phase 3] Author commits: 10+ similar quirk patches all accepted by
Takashi Iwai
- [Phase 4] Web search: confirmed identical pattern patches (AB13X,
AB17X, etc.) accepted
- [Phase 5] Flag usage: IFACE_DELAY triggers 50ms sleep in
endpoint.c:942, FORCE_IFACE_RESET forces re-setup in
endpoint.c:1695-1700
- [Phase 5] 10+ other devices already use these same flags (verified via
grep)
- [Phase 6] Base entry exists in 7.0 tree (confirmed via git blame)
- [Phase 7] sound/usb/quirks.c: 54 changes between v6.12-v7.0, highly
active
- [Phase 8] Failure mode: device non-functional after suspend/resume,
severity HIGH for affected users
- UNVERIFIED: Could not access lore.kernel.org directly due to anti-bot
protection; relied on web search confirmation
This is a textbook USB audio hardware quirk addition. It adds two well-
established flags to an existing device entry, fixing suspend/resume
failure for a specific HUAWEI USB-C headset. The change is data-only,
obviously correct, zero regression risk, and accepted by the ALSA
subsystem maintainer.
**YES**
sound/usb/quirks.c | 5 +++--
1 file changed, 3 insertions(+), 2 deletions(-)
diff --git a/sound/usb/quirks.c b/sound/usb/quirks.c
index 4cfa24c06fcdf..d3a69995c1ad5 100644
--- a/sound/usb/quirks.c
+++ b/sound/usb/quirks.c
@@ -2291,8 +2291,9 @@ static const struct usb_audio_quirk_flags_table quirk_flags_table[] = {
QUIRK_FLAG_MIXER_PLAYBACK_MIN_MUTE),
DEVICE_FLG(0x1101, 0x0003, /* Audioengine D1 */
QUIRK_FLAG_GET_SAMPLE_RATE),
- DEVICE_FLG(0x12d1, 0x3a07, /* Huawei Technologies Co., Ltd. */
- QUIRK_FLAG_MIXER_PLAYBACK_MIN_MUTE | QUIRK_FLAG_MIXER_CAPTURE_MIN_MUTE),
+ DEVICE_FLG(0x12d1, 0x3a07, /* HUAWEI USB-C HEADSET */
+ QUIRK_FLAG_MIXER_PLAYBACK_MIN_MUTE | QUIRK_FLAG_MIXER_CAPTURE_MIN_MUTE |
+ QUIRK_FLAG_FORCE_IFACE_RESET | QUIRK_FLAG_IFACE_DELAY),
DEVICE_FLG(0x1224, 0x2a25, /* Jieli Technology USB PHY 2.0 */
QUIRK_FLAG_GET_SAMPLE_RATE | QUIRK_FLAG_MIC_RES_16),
DEVICE_FLG(0x1395, 0x740a, /* Sennheiser DECT */
--
2.53.0
^ permalink raw reply related
* [PATCH AUTOSEL 6.18] ALSA: hda/realtek: Add quirk for Lenovo Yoga Pro 7 14IMH9
From: Sasha Levin @ 2026-04-20 13:17 UTC (permalink / raw)
To: patches, stable
Cc: Alexander Savenko, Takashi Iwai, Sasha Levin, perex, tiwai,
linux-sound, linux-kernel
In-Reply-To: <20260420132314.1023554-1-sashal@kernel.org>
From: Alexander Savenko <alex.sav4387@gmail.com>
[ Upstream commit 217d5bc9f96272316ac5a3215c7cc32a5127bbf3 ]
The Lenovo Yoga Pro 7 14IMH9 (DMI: 83E2) shares PCI SSID 17aa:3847
with the Legion 7 16ACHG6, but has a different codec subsystem ID
(17aa:38cf). The existing SND_PCI_QUIRK for 17aa:3847 applies
ALC287_FIXUP_LEGION_16ACHG6, which attempts to initialize an external
I2C amplifier (CLSA0100) that is not present on the Yoga Pro 7 14IMH9.
As a result, pin 0x17 (bass speakers) is connected to DAC 0x06 which
has no volume control, making hardware volume adjustment completely
non-functional. Audio is either silent or at maximum volume regardless
of the slider position.
Add a HDA_CODEC_QUIRK entry using the codec subsystem ID (17aa:38cf)
to correctly identify the Yoga Pro 7 14IMH9 and apply
ALC287_FIXUP_YOGA9_14IMH9_BASS_SPK_PIN, which redirects pin 0x17 to
DAC 0x02 and restores proper volume control. The existing Legion entry
is preserved unchanged.
This follows the same pattern used for 17aa:386e, where Legion Y9000X
and Yoga Pro 7 14ARP8 share a PCI SSID but are distinguished via
HDA_CODEC_QUIRK.
Link: https://github.com/nomad4tech/lenovo-yoga-pro-7-linux
Tested-by: Alexander Savenko <alex.sav4387@gmail.com>
Signed-off-by: Alexander Savenko <alex.sav4387@gmail.com>
Link: https://patch.msgid.link/20260331082929.44890-1-alex.sav4387@gmail.com
Signed-off-by: Takashi Iwai <tiwai@suse.de>
Signed-off-by: Sasha Levin <sashal@kernel.org>
---
LLM Generated explanations, may be completely bogus:
Error: Failed to generate final synthesis
sound/hda/codecs/realtek/alc269.c | 4 ++++
1 file changed, 4 insertions(+)
diff --git a/sound/hda/codecs/realtek/alc269.c b/sound/hda/codecs/realtek/alc269.c
index 1c8ee8263ab3a..2e89528e5cec1 100644
--- a/sound/hda/codecs/realtek/alc269.c
+++ b/sound/hda/codecs/realtek/alc269.c
@@ -7402,6 +7402,10 @@ static const struct hda_quirk alc269_fixup_tbl[] = {
SND_PCI_QUIRK(0x17aa, 0x3834, "Lenovo IdeaPad Slim 9i 14ITL5", ALC287_FIXUP_YOGA7_14ITL_SPEAKERS),
SND_PCI_QUIRK(0x17aa, 0x383d, "Legion Y9000X 2019", ALC285_FIXUP_LEGION_Y9000X_SPEAKERS),
SND_PCI_QUIRK(0x17aa, 0x3843, "Lenovo Yoga 9i / Yoga Book 9i", ALC287_FIXUP_LENOVO_YOGA_BOOK_9I),
+ /* Yoga Pro 7 14IMH9 shares PCI SSID 17aa:3847 with Legion 7 16ACHG6;
+ * use codec SSID to distinguish them
+ */
+ HDA_CODEC_QUIRK(0x17aa, 0x38cf, "Lenovo Yoga Pro 7 14IMH9", ALC287_FIXUP_YOGA9_14IMH9_BASS_SPK_PIN),
SND_PCI_QUIRK(0x17aa, 0x3847, "Legion 7 16ACHG6", ALC287_FIXUP_LEGION_16ACHG6),
SND_PCI_QUIRK(0x17aa, 0x384a, "Lenovo Yoga 7 15ITL5", ALC287_FIXUP_YOGA7_14ITL_SPEAKERS),
SND_PCI_QUIRK(0x17aa, 0x3852, "Lenovo Yoga 7 14ITL5", ALC287_FIXUP_YOGA7_14ITL_SPEAKERS),
--
2.53.0
^ permalink raw reply related
* [PATCH AUTOSEL 7.0-5.10] ALSA: aoa/tas: Fix OF node leak on probe failure
From: Sasha Levin @ 2026-04-20 13:17 UTC (permalink / raw)
To: patches, stable
Cc: wangdicheng, Takashi Iwai, Sasha Levin, johannes, perex, tiwai,
linuxppc-dev, linux-sound, linux-kernel
In-Reply-To: <20260420132314.1023554-1-sashal@kernel.org>
From: wangdicheng <wangdicheng@kylinos.cn>
[ Upstream commit 1558905669e4da922fbaa7cf6507eb14779bffbd ]
Add missing of_node_put() in the error path.
Signed-off-by: wangdicheng <wangdicheng@kylinos.cn>
Link: https://patch.msgid.link/20260402023604.54682-1-wangdich9700@163.com
Signed-off-by: Takashi Iwai <tiwai@suse.de>
Signed-off-by: Sasha Levin <sashal@kernel.org>
---
LLM Generated explanations, may be completely bogus:
Now I have all the information needed for a complete analysis.
---
## PHASE 1: COMMIT MESSAGE FORENSICS
### Step 1.1: Subject Line
- **Subsystem**: `ALSA: aoa/tas` (ALSA Apple Onboard Audio, TAS codec)
- **Action verb**: "Fix"
- **Summary**: Fix OF (Open Firmware) node reference leak on probe
failure path
### Step 1.2: Tags
- **Signed-off-by**: wangdicheng <wangdicheng@kylinos.cn> (author)
- **Link**:
`https://patch.msgid.link/20260402023604.54682-1-wangdich9700@163.com`
- **Signed-off-by**: Takashi Iwai <tiwai@suse.de> (ALSA subsystem
maintainer)
- No Fixes: tag, no Cc: stable, no Reported-by — all expected for
AUTOSEL candidates
- Takashi Iwai as committer is a strong signal: he is the ALSA
maintainer
### Step 1.3: Commit Body
The message is very brief: "Add missing of_node_put() in the error
path." This concisely describes a reference counting bug (missing put on
error path).
### Step 1.4: Hidden Bug Fix
This is an explicit bug fix — no disguise. The commit directly states it
fixes a missing `of_node_put()`.
Record: [Reference counting bug fix — missing of_node_put on error path
in probe function]
---
## PHASE 2: DIFF ANALYSIS
### Step 2.1: Inventory
- **Files changed**: 1 (`sound/aoa/codecs/tas.c`)
- **Lines added**: 1
- **Lines removed**: 0
- **Function modified**: `tas_i2c_probe()` — the `fail:` error path
- **Scope**: Single-file, single-line, surgical fix
### Step 2.2: Code Flow Change
Before the fix, the `fail:` path in `tas_i2c_probe()`:
```873:876:sound/aoa/codecs/tas.c
fail:
mutex_destroy(&tas->mtx);
kfree(tas);
return -EINVAL;
```
After the fix, `of_node_put(tas->codec.node)` is added between
`mutex_destroy` and `kfree`. The reference taken at line 864
(`tas->codec.node = of_node_get(node)`) is now properly released.
### Step 2.3: Bug Mechanism
**Category**: Reference counting bug (OF node reference leak)
- At line 864, `of_node_get(node)` increments the OF node's refcount and
stores the result in `tas->codec.node`
- If `aoa_codec_register()` fails at line 866, execution jumps to
`fail:`
- Without the fix, the `fail:` path calls `kfree(tas)` which frees the
struct holding the only pointer to the refcounted node — the refcount
is never decremented
- The `tas_i2c_remove()` function at line 885 correctly calls
`of_node_put(tas->codec.node)`, confirming the expected pattern
### Step 2.4: Fix Quality
- **Obviously correct**: Yes — mirrors the cleanup pattern already in
`tas_i2c_remove()` (line 885)
- **Minimal**: Yes — 1 line added
- **Regression risk**: Essentially zero — only adds cleanup on an error
path
- **Placement**: Correct — `of_node_put(tas->codec.node)` is placed
before `kfree(tas)` so the pointer is still valid
Record: [1 file, +1 line, reference counting fix on error path,
obviously correct, zero regression risk]
---
## PHASE 3: GIT HISTORY INVESTIGATION
### Step 3.1: Blame
The buggy code (`of_node_get(node)` without matching put on error path)
was introduced in commit `f3d9478b2ce468` by Johannes Berg on 2006-06-21
("[ALSA] snd-aoa: add snd-aoa"). This is the initial commit for the
entire snd-aoa subsystem, from the v2.6.18 era.
Record: [Bug has been present since 2006 (v2.6.18). Present in ALL
stable trees.]
### Step 3.2: No Fixes: Tag
No Fixes: tag present — expected for AUTOSEL candidates. However, the
implicit fix target is `f3d9478b2ce468`.
### Step 3.3: File History
The file has had only minor maintenance changes (strscpy, guard()
conversions, kzalloc_obj treewide changes). No related of_node_put fixes
for this specific path.
The related commit `222bce5eb88d1` ("ALSA: snd-aoa: add of_node_put() in
error path") fixed a similar bug in `sound/aoa/core/gpio-feature.c` —
different file, same subsystem, same bug class.
### Step 3.4: Author
The author (wangdicheng) has contributed several ALSA fixes. The patch
was accepted by Takashi Iwai, the ALSA maintainer, giving it strong
credibility.
### Step 3.5: Dependencies
None. The fix is a single `of_node_put()` call — it is completely
standalone and applies cleanly.
Record: [No dependencies. Standalone fix. Accepted by subsystem
maintainer.]
---
## PHASE 4: MAILING LIST
### Step 4.1: Original Submission
b4 dig could not find the original submission (the commit hash is from
the autosel pipeline, not mainline). The Link: in the commit message
points to
`patch.msgid.link/20260402023604.54682-1-wangdich9700@163.com`. Lore was
not accessible due to bot protection.
### Step 4.2-4.5
Could not access lore.kernel.org due to Anubis anti-scraping protection.
However, the commit was accepted by the ALSA maintainer (Takashi Iwai),
which means it passed his review.
Record: [Lore inaccessible. Patch accepted by ALSA maintainer Takashi
Iwai.]
---
## PHASE 5: CODE SEMANTIC ANALYSIS
### Step 5.1: Functions Modified
Only `tas_i2c_probe()` — specifically its `fail:` error path.
### Step 5.2: Callers
`tas_i2c_probe` is the I2C probe callback registered in the `tas_driver`
struct (line 904). It is called by the I2C subsystem when the device is
enumerated. This is a standard device probe path.
### Step 5.3-5.4: Call Chain
The error path is reached when `aoa_codec_register()` fails. Looking at
the function body (`sound/aoa/core/core.c` lines 57-69), it fails when
`attach_codec_to_fabric()` returns an error. This is a plausible failure
scenario during boot or module loading.
### Step 5.5: Similar Patterns
The sibling driver `onyx.c` has the **exact same bug** at lines 980-988:
- `onyx->codec.node = of_node_get(node)` at line 980
- The `fail:` label at line 987-989 calls `kfree(onyx)` without
`of_node_put(onyx->codec.node)`
Record: [Same pattern bug exists in onyx.c. Probe function called by I2C
subsystem during device enumeration.]
---
## PHASE 6: STABLE TREE ANALYSIS
### Step 6.1: Code Existence
The buggy code was introduced in 2006 (`f3d9478b2ce468`). It exists in
**every** stable tree (5.4.y, 5.10.y, 5.15.y, 6.1.y, 6.6.y, 6.12.y,
etc.).
### Step 6.2: Backport Complications
The only concern is the `kzalloc_obj` conversion on line 848 (from Feb
2026), which exists only in mainline 7.0. In older stable trees, this
will be `kzalloc(sizeof(*tas), GFP_KERNEL)`. However, the fix (adding
one line in the `fail:` path) is completely independent of the
allocation call. The `fail:` label context (mutex_destroy + kfree) has
been stable since 2006. The fix should apply cleanly or with trivial
context adjustment.
### Step 6.3: No related fixes in stable
No previous fix for this specific bug exists in stable trees.
Record: [Bug exists in all stable trees. Fix should apply cleanly with
minor context fuzz.]
---
## PHASE 7: SUBSYSTEM CONTEXT
### Step 7.1: Subsystem
- **Subsystem**: ALSA (sound), Apple Onboard Audio — codec driver for
TAS3004
- **Criticality**: PERIPHERAL — only affects Apple PowerPC-based
machines with TAS3004 codec (PowerBooks, PowerMacs)
### Step 7.2: Activity
The file gets very infrequent changes (mostly treewide cleanups). This
is a mature, stable subsystem with minimal churn.
Record: [PERIPHERAL subsystem (Apple PowerPC audio). Mature code with
low churn.]
---
## PHASE 8: IMPACT AND RISK ASSESSMENT
### Step 8.1: Affected Population
Users with Apple PowerPC hardware using the TAS3004 audio codec. This is
a niche population, but real users exist (retrocomputing, legacy
systems).
### Step 8.2: Trigger Conditions
The bug triggers only when `aoa_codec_register()` fails during probe.
This is an error path, so it's not common, but it represents a real leak
each time it occurs (e.g., resource contention, misconfiguration).
### Step 8.3: Failure Mode Severity
- **OF node reference leak**: The node refcount is never decremented, so
the OF node structure is never freed.
- Severity: **LOW-MEDIUM** — resource leak on error path, not a crash or
security issue, but a real correctness bug.
### Step 8.4: Risk-Benefit Ratio
- **Benefit**: Fixes a real reference counting bug that leaks memory on
probe failure.
- **Risk**: Essentially zero — 1-line, obviously correct, mirrors
existing cleanup in the remove function.
- **Ratio**: Very favorable — minimal risk, real correctness
improvement.
---
## PHASE 9: FINAL SYNTHESIS
### Step 9.1: Evidence Summary
**FOR backporting**:
- Fixes a real reference counting bug (missing `of_node_put()` on error
path)
- 1-line fix, obviously correct, mirrors cleanup in `tas_i2c_remove()`
- Bug has existed since 2006, present in all stable trees
- Accepted by ALSA maintainer Takashi Iwai
- Zero regression risk
- Classic reference counting fix — standard stable material
**AGAINST backporting**:
- Only affects Apple PowerPC hardware with TAS3004 codec (niche)
- Only triggers on probe failure (error path, not common)
- Severity is LOW-MEDIUM (resource leak, not crash/security)
### Step 9.2: Stable Rules Checklist
1. Obviously correct and tested? **YES** — mirrors existing cleanup in
remove()
2. Fixes a real bug? **YES** — OF node reference leak
3. Important issue? **MODERATE** — resource leak, not critical
4. Small and contained? **YES** — 1 line, 1 file
5. No new features/APIs? **YES** — pure fix
6. Can apply to stable? **YES** — standalone, no dependencies
### Step 9.3: Exception Categories
Not an exception category. Standard bug fix.
### Step 9.4: Decision
This is a small, obviously correct reference counting fix. While the
impact is limited (niche hardware, error path only), the fix has
essentially zero risk and addresses a real correctness bug. OF node
reference leak fixes are routinely accepted in stable, and this follows
a well-established pattern.
---
## Verification
- [Phase 1] Parsed subject: ALSA: aoa/tas subsystem, "Fix" verb, OF node
leak on probe failure
- [Phase 1] Parsed tags: Signed-off-by from author and Takashi Iwai
(ALSA maintainer), Link to lore
- [Phase 2] Diff analysis: 1 line added in `tas_i2c_probe()` fail path:
`of_node_put(tas->codec.node)` before `kfree(tas)`
- [Phase 2] Verified `of_node_get(node)` at line 864 takes a reference
that is not released on error path
- [Phase 2] Verified `tas_i2c_remove()` at line 885 calls
`of_node_put(tas->codec.node)` — confirming correct pattern
- [Phase 3] git blame: buggy code introduced in f3d9478b2ce468
(2006-06-21, "[ALSA] snd-aoa: add snd-aoa")
- [Phase 3] git log: only treewide cleanups on this file, no prior fix
for this specific bug
- [Phase 3] Related commit 222bce5eb88d1 fixed same bug class in gpio-
feature.c (different file, same subsystem)
- [Phase 4] b4 dig: could not match rebased commit hash; lore.kernel.org
blocked by Anubis
- [Phase 5] Verified `onyx.c` at lines 980-988 has the same unfixed bug
pattern (systematic issue)
- [Phase 5] Verified `aoa_codec_register()` in core.c can fail when
`attach_codec_to_fabric()` errors
- [Phase 6] Bug exists since 2006 — present in all active stable trees
- [Phase 6] Minor context difference in stable (kzalloc_obj vs kzalloc)
won't affect the fix line
- [Phase 8] Failure mode: OF node reference leak on probe error path,
severity LOW-MEDIUM
- UNVERIFIED: Could not access lore.kernel.org to check full review
discussion or stable nominations
**YES**
sound/aoa/codecs/tas.c | 1 +
1 file changed, 1 insertion(+)
diff --git a/sound/aoa/codecs/tas.c b/sound/aoa/codecs/tas.c
index 13da2b159ad0d..25214d3da65d1 100644
--- a/sound/aoa/codecs/tas.c
+++ b/sound/aoa/codecs/tas.c
@@ -872,6 +872,7 @@ static int tas_i2c_probe(struct i2c_client *client)
return 0;
fail:
mutex_destroy(&tas->mtx);
+ of_node_put(tas->codec.node);
kfree(tas);
return -EINVAL;
}
--
2.53.0
^ permalink raw reply related
* [PATCH AUTOSEL 7.0-5.10] ASoC: ti: davinci-mcasp: Add system suspend/resume support
From: Sasha Levin @ 2026-04-20 13:17 UTC (permalink / raw)
To: patches, stable
Cc: Sen Wang, Mark Brown, Sasha Levin, peter.ujfalusi, lgirdwood,
perex, tiwai, linux-sound, linux-kernel
In-Reply-To: <20260420132314.1023554-1-sashal@kernel.org>
From: Sen Wang <sen@ti.com>
[ Upstream commit 5879521cb558871472b97c4744dbe634a4286f0e ]
The McASP driver supports runtime PM callbacks for register save/restore
during device idle, but doesn't provide system suspend/resume callbacks.
This causes audio to fail to resume after system suspend.
Since the driver already handles runtime suspend & resume, we can reuse
existing runtime PM logics.
Signed-off-by: Sen Wang <sen@ti.com>
Link: https://patch.msgid.link/20260211221001.155843-1-sen@ti.com
Signed-off-by: Mark Brown <broonie@kernel.org>
Signed-off-by: Sasha Levin <sashal@kernel.org>
---
LLM Generated explanations, may be completely bogus:
Now I have all the information needed for a thorough analysis.
---
## PHASE 1: COMMIT MESSAGE FORENSICS
### Step 1.1: Subject Line
- **Subsystem**: ASoC: ti: davinci-mcasp
- **Action verb**: "Add" (system suspend/resume support)
- **Summary**: Adds system suspend/resume PM ops to bridge to existing
runtime PM callbacks
Record: [ASoC: ti: davinci-mcasp] [Add] [system suspend/resume support
by reusing runtime PM]
### Step 1.2: Tags
- **Signed-off-by**: Sen Wang <sen@ti.com> (author, TI employee - the
SoC vendor)
- **Link**: https://patch.msgid.link/20260211221001.155843-1-sen@ti.com
- **Signed-off-by**: Mark Brown <broonie@kernel.org> (ASoC subsystem
maintainer, applied the patch)
- No Fixes: tag (expected for commits under review)
- No Reported-by: tag
- No Cc: stable
Record: Author is TI employee (hardware vendor). Applied by ASoC
maintainer Mark Brown.
### Step 1.3: Commit Body
The message states: "The McASP driver supports runtime PM callbacks for
register save/restore during device idle, but doesn't provide system
suspend/resume callbacks. **This causes audio to fail to resume after
system suspend.**"
This describes a clear user-visible failure: audio breaks after system
suspend.
Record: Bug = audio fails to resume after S2RAM/suspend. Root cause =
missing system sleep PM ops when runtime PM callbacks handle context
save/restore. No stack traces or error messages described.
### Step 1.4: Hidden Bug Fix Detection
Despite the subject saying "Add", this is actually **restoring**
functionality that was removed by commit 6175471755075d (Jan 2019). That
commit moved context save/restore from DAI-level suspend/resume to
runtime PM callbacks but failed to bridge system sleep to runtime PM.
This is a regression fix disguised as a feature addition.
Record: YES, this is a hidden bug fix - it restores system suspend
functionality that was inadvertently broken in commit 6175471755075d.
---
## PHASE 2: DIFF ANALYSIS
### Step 2.1: Inventory
- **Files changed**: 1 (`sound/soc/ti/davinci-mcasp.c`)
- **Lines added**: 2
- **Lines removed**: 0
- **Functions modified**: None (only the `davinci_mcasp_pm_ops` struct
is changed)
- **Scope**: Single-file, ultra-surgical, 2-line addition
### Step 2.2: Code Flow Change
**Before**: `davinci_mcasp_pm_ops` only had `SET_RUNTIME_PM_OPS()`.
System suspend/resume had no callbacks → device context lost on S2RAM.
**After**: `SET_SYSTEM_SLEEP_PM_OPS(pm_runtime_force_suspend,
pm_runtime_force_resume)` is added, which tells the PM core to force
runtime suspend/resume during system sleep. This triggers
`davinci_mcasp_runtime_suspend()` and `davinci_mcasp_runtime_resume()`
during S2RAM, saving and restoring all McASP registers.
### Step 2.3: Bug Mechanism
Category: **Logic/correctness fix** + **hardware workaround**
The McASP hardware loses its register context during system suspend.
Without system sleep callbacks, the registers are never saved, so after
resume the hardware is in an undefined state and audio playback/capture
fails.
`pm_runtime_force_suspend`/`pm_runtime_force_resume` are standard kernel
PM helpers used by 35+ other sound drivers in the exact same way.
### Step 2.4: Fix Quality
- **Obviously correct**: YES - this is the standard, well-documented
pattern
- **Minimal/surgical**: YES - 2 lines, no unrelated changes
- **Regression risk**: Extremely low - uses standard PM infrastructure,
used identically by dozens of drivers
- **Red flags**: None
---
## PHASE 3: GIT HISTORY
### Step 3.1: Blame
The `davinci_mcasp_pm_ops` struct was introduced by commit
6175471755075d (Peter Ujfalusi, 2019-01-03) titled "ASoC: ti: davinci-
mcasp: Move context save/restore to runtime_pm callbacks". This commit:
1. Removed the old DAI-level
`davinci_mcasp_suspend()`/`davinci_mcasp_resume()` which DID handle
system suspend
2. Added
`davinci_mcasp_runtime_suspend()`/`davinci_mcasp_runtime_resume()`
for runtime PM
3. Created `davinci_mcasp_pm_ops` with only `SET_RUNTIME_PM_OPS` —
**missing the system sleep bridge**
Record: Bug introduced by 6175471755075d (Jan 2019), which first
appeared around v5.2-rc1. Present in all active stable trees.
### Step 3.2: Fixes Tag
No Fixes: tag present, but the implicit target is 6175471755075d.
Confirmed this commit exists in stable trees (verified it's tagged in
p-5.10, p-5.15 and later).
### Step 3.3: File History
20+ commits to this file since v5.15, but none touch the PM ops area.
The `davinci_mcasp_pm_ops` struct has been unchanged since
6175471755075d.
### Step 3.4: Author
Sen Wang (<sen@ti.com>) is a TI employee with recent contributions to
this driver (3 commits to davinci-mcasp, plus DT bindings). TI is the
manufacturer of the McASP hardware.
### Step 3.5: Dependencies
None. The fix adds 2 lines to an existing struct using APIs that have
been available since v4.x. No prerequisite commits needed.
---
## PHASE 4: MAILING LIST RESEARCH
b4 dig could not find the commit by message-id. Lore.kernel.org returned
anti-bot pages. However, the Link: tag in the commit message confirms it
was submitted and reviewed on the ALSA/ASoC mailing list. Mark Brown
(ASoC maintainer) applied it directly.
### Step 4.2: Reviewers
Applied by Mark Brown (broonie@kernel.org), the ASoC subsystem
maintainer. He is the authoritative reviewer for this subsystem.
---
## PHASE 5: CODE SEMANTIC ANALYSIS
### Step 5.1: Key Functions
The diff doesn't modify any functions - it adds
`pm_runtime_force_suspend`/`pm_runtime_force_resume` (existing kernel
APIs) to the PM ops struct.
### Step 5.2: Callers
The PM core calls these functions during system suspend/resume based on
the `dev_pm_ops` structure. This affects every system that uses McASP
hardware and performs system suspend (S2RAM, hibernate).
### Step 5.3-5.5: Similar Patterns
Verified that 35+ sound drivers use this exact same
`SYSTEM_SLEEP_PM_OPS(pm_runtime_force_suspend, pm_runtime_force_resume)`
pattern. This is the standard approach for drivers that handle context
save/restore via runtime PM.
---
## PHASE 6: STABLE TREE ANALYSIS
### Step 6.1: Code Existence
The buggy code (`davinci_mcasp_pm_ops` with only `SET_RUNTIME_PM_OPS`)
exists in ALL active stable trees since v5.2. Confirmed commit
6175471755075d is in p-5.10 and p-5.15.
### Step 6.2: Backport Complications
The PM ops area has been completely unchanged since 2019. The patch
should apply cleanly to all stable trees (6.6.y, 6.1.y, 5.15.y, 5.10.y).
### Step 6.3: No related fixes already in stable for this issue.
---
## PHASE 7: SUBSYSTEM AND MAINTAINER CONTEXT
### Step 7.1: Subsystem
`sound/soc/ti/` - TI Audio SoC driver. Criticality: PERIPHERAL (specific
embedded hardware - TI AM335x/AM437x/AM65x/J7 platforms, including
BeagleBone). However, this is a widely-used embedded platform.
### Step 7.2: Activity
Moderately active - 10+ commits since v6.6.
---
## PHASE 8: IMPACT AND RISK ASSESSMENT
### Step 8.1: Affected Users
All users of TI DaVinci McASP audio hardware who perform system
suspend/resume. This includes BeagleBone users, TI AM335x/AM437x/AM65x
industrial platforms, and any TI-based embedded system using audio.
### Step 8.2: Trigger
System suspend/resume (S2RAM). Very common operation on laptops and many
embedded systems. Triggered by user action (closing lid, `systemctl
suspend`).
### Step 8.3: Failure Mode
Audio completely fails after resume - the hardware registers are in
undefined state. Severity: **HIGH** - functionality loss requiring
reboot to recover.
### Step 8.4: Risk-Benefit
- **BENEFIT**: HIGH - fixes audio breakage after suspend for all McASP
users
- **RISK**: Very low - 2 lines using standard, well-tested kernel PM
APIs used by 35+ other drivers
- **Ratio**: Strongly favorable
---
## PHASE 9: FINAL SYNTHESIS
### Step 9.1: Evidence Summary
**FOR backporting:**
- Fixes a real user-visible bug (audio fails after system suspend)
- This is a regression fix: commit 6175471755075d (2019) removed system
suspend callbacks without adding the runtime PM bridge
- Ultra-minimal: 2 lines added
- Uses standard kernel PM APIs
(`pm_runtime_force_suspend`/`pm_runtime_force_resume`)
- Pattern used by 35+ other sound drivers
- No dependencies or prerequisites
- Applies cleanly to all stable trees
- Authored by TI employee (hardware vendor), applied by ASoC maintainer
- Bug affects all stable trees since v5.2
**AGAINST backporting:**
- Subject line says "Add" which sounds like new feature (but substance
is a regression fix)
- No Fixes: tag (expected for candidates under review)
- No explicit Reported-by (but the TI author would be testing this
hardware)
- The bug has existed since 2019 without being reported/fixed (low
urgency, but real)
### Step 9.2: Stable Rules Checklist
1. Obviously correct and tested? **YES** - standard pattern, accepted by
maintainer
2. Fixes a real bug? **YES** - audio fails after system suspend
3. Important issue? **YES** - complete audio loss requiring reboot
4. Small and contained? **YES** - 2 lines in one file
5. No new features or APIs? **YES** - restores accidentally removed
functionality using existing APIs
6. Can apply to stable? **YES** - clean apply expected
### Step 9.3: Exception Category
This is a suspend/resume fix, which is a common category of stable-
appropriate fixes (fixes prevent data loss / functionality loss on real
hardware).
### Step 9.4: Decision
This is a regression fix for commit 6175471755075d which accidentally
removed system suspend support in 2019. The fix is 2 lines, uses
standard, well-proven kernel PM infrastructure, has zero regression
risk, and fixes complete audio loss after system suspend for all TI
McASP users. It should be backported.
---
## Verification
- [Phase 1] Parsed tags: Author is TI employee, applied by ASoC
maintainer Mark Brown
- [Phase 2] Diff analysis: 2 lines added to `davinci_mcasp_pm_ops`
struct, adding `SET_SYSTEM_SLEEP_PM_OPS(pm_runtime_force_suspend,
pm_runtime_force_resume)`
- [Phase 3] git blame: PM ops struct introduced by commit 6175471755075d
(2019-01-03), which removed old system suspend callbacks without
adding runtime PM bridge
- [Phase 3] git show 6175471755075d: confirmed it removed
`davinci_mcasp_suspend()`/`davinci_mcasp_resume()` DAI callbacks that
previously handled system suspend
- [Phase 3] git tag --contains 6175471755075d: confirmed present in
p-5.10 and p-5.15 (all active stable trees)
- [Phase 3] git log v6.1..HEAD / v5.15..HEAD: PM ops area unchanged in
stable trees, patch will apply cleanly
- [Phase 4] b4 dig: could not match by message-id; lore blocked by anti-
bot measures
- [Phase 5] Grep for `pm_runtime_force_suspend` in sound/: found 35+
drivers using identical pattern
- [Phase 6] Code exists unchanged in all active stable trees since v5.2
- [Phase 8] Failure mode: complete audio loss after system suspend,
severity HIGH
- UNVERIFIED: Could not read lore.kernel.org discussion due to anti-bot
protection. However, the maintainer's Signed-off-by confirms review
and acceptance.
**YES**
sound/soc/ti/davinci-mcasp.c | 2 ++
1 file changed, 2 insertions(+)
diff --git a/sound/soc/ti/davinci-mcasp.c b/sound/soc/ti/davinci-mcasp.c
index 2d260fbc9b835..14267be4a288a 100644
--- a/sound/soc/ti/davinci-mcasp.c
+++ b/sound/soc/ti/davinci-mcasp.c
@@ -2823,6 +2823,8 @@ static int davinci_mcasp_runtime_resume(struct device *dev)
#endif
static const struct dev_pm_ops davinci_mcasp_pm_ops = {
+ SET_SYSTEM_SLEEP_PM_OPS(pm_runtime_force_suspend,
+ pm_runtime_force_resume)
SET_RUNTIME_PM_OPS(davinci_mcasp_runtime_suspend,
davinci_mcasp_runtime_resume,
NULL)
--
2.53.0
^ permalink raw reply related
* [PATCH AUTOSEL 6.18] ALSA: asihpi: avoid write overflow check warning
From: Sasha Levin @ 2026-04-20 13:17 UTC (permalink / raw)
To: patches, stable
Cc: Arnd Bergmann, Takashi Iwai, Sasha Levin, perex, tiwai,
linux-sound, linux-kernel
In-Reply-To: <20260420132314.1023554-1-sashal@kernel.org>
From: Arnd Bergmann <arnd@arndb.de>
[ Upstream commit 591721223be9e28f83489a59289579493b8e3d83 ]
clang-22 rightfully warns that the memcpy() in adapter_prepare() copies
between different structures, crossing the boundary of nested
structures inside it:
In file included from sound/pci/asihpi/hpimsgx.c:13:
In file included from include/linux/string.h:386:
include/linux/fortify-string.h:569:4: error: call to '__write_overflow_field' declared with 'warning' attribute: detected write beyond size of field (1st parameter); maybe use struct_group()? [-Werror,-Wattribute-warning]
569 | __write_overflow_field(p_size_field, size);
The two structures seem to refer to the same layout, despite the
separate definitions, so the code is in fact correct.
Avoid the warning by copying the two inner structures separately.
I see the same pattern happens in other functions in the same file,
so there is a chance that this may come back in the future, but
this instance is the only one that I saw in practice, hitting it
multiple times per day in randconfig build.
Signed-off-by: Arnd Bergmann <arnd@arndb.de>
Link: https://patch.msgid.link/20260318124016.3488566-1-arnd@kernel.org
Signed-off-by: Takashi Iwai <tiwai@suse.de>
Signed-off-by: Sasha Levin <sashal@kernel.org>
---
LLM Generated explanations, may be completely bogus:
Error: Failed to generate final synthesis
sound/pci/asihpi/hpimsgx.c | 6 ++++--
1 file changed, 4 insertions(+), 2 deletions(-)
diff --git a/sound/pci/asihpi/hpimsgx.c b/sound/pci/asihpi/hpimsgx.c
index b68e6bfbbfbab..ed1c7b7744361 100644
--- a/sound/pci/asihpi/hpimsgx.c
+++ b/sound/pci/asihpi/hpimsgx.c
@@ -581,8 +581,10 @@ static u16 adapter_prepare(u16 adapter)
HPI_ADAPTER_OPEN);
hm.adapter_index = adapter;
hw_entry_point(&hm, &hr);
- memcpy(&rESP_HPI_ADAPTER_OPEN[adapter], &hr,
- sizeof(rESP_HPI_ADAPTER_OPEN[0]));
+ memcpy(&rESP_HPI_ADAPTER_OPEN[adapter].h, &hr,
+ sizeof(rESP_HPI_ADAPTER_OPEN[adapter].h));
+ memcpy(&rESP_HPI_ADAPTER_OPEN[adapter].a, &hr.u.ax.info,
+ sizeof(rESP_HPI_ADAPTER_OPEN[adapter].a));
if (hr.error)
return hr.error;
--
2.53.0
^ permalink raw reply related
* [PATCH AUTOSEL 7.0-5.10] ASoC: rt5640: Handle 0Hz sysclk during stream shutdown
From: Sasha Levin @ 2026-04-20 13:17 UTC (permalink / raw)
To: patches, stable
Cc: Sheetal, Mark Brown, Sasha Levin, oder_chiou, lgirdwood, perex,
tiwai, linux-sound, linux-kernel
In-Reply-To: <20260420132314.1023554-1-sashal@kernel.org>
From: Sheetal <sheetal@nvidia.com>
[ Upstream commit 247d1c13992d2c501e2e020e84d9d2920e11bf78 ]
Commit 2458adb8f92a ("SoC: simple-card-utils: set 0Hz to sysclk when
shutdown") sends a 0Hz sysclk request during stream shutdown to clear
codec rate constraints. The rt5640 codec forwards this 0Hz to
clk_set_rate(), which can cause clock controller firmware faults on
platforms where MCLK is SoC-driven (e.g. Tegra) and 0Hz falls below
the hardware minimum rate.
Handle the 0Hz case by clearing the internal sysclk state and
returning early, avoiding the invalid clk_set_rate() call.
Signed-off-by: Sheetal <sheetal@nvidia.com>
Link: https://patch.msgid.link/20260406090547.988966-1-sheetal@nvidia.com
Signed-off-by: Mark Brown <broonie@kernel.org>
Signed-off-by: Sasha Levin <sashal@kernel.org>
---
LLM Generated explanations, may be completely bogus:
The code is identical across v6.6, v6.12, and v7.0. The fix will apply
cleanly. Let me now compile my complete analysis.
---
## PHASE 1: COMMIT MESSAGE FORENSICS
### Step 1.1: Subject Line
- **Subsystem**: ASoC: rt5640 (audio codec driver)
- **Action verb**: "Handle" - indicates fixing incorrect handling of a
specific case
- **Summary**: Handles the 0Hz sysclk case during stream shutdown to
prevent clock controller faults
### Step 1.2: Tags
- **Link**:
`https://patch.msgid.link/20260406090547.988966-1-sheetal@nvidia.com`
- original patch submission
- **Signed-off-by**: Sheetal (NVIDIA) - author, and Mark Brown
(subsystem maintainer) - applied it
- No Fixes: tag (expected for autosel candidates)
- No Reported-by tag
- No Cc: stable tag
### Step 1.3: Commit Body Analysis
The commit explains:
- **Cause**: Commit 2458adb8f92a added a 0Hz sysclk request during
stream shutdown
- **Bug**: rt5640 forwards this 0Hz to `clk_set_rate()`, which causes
clock controller firmware faults on Tegra platforms where MCLK is SoC-
driven and 0Hz falls below hardware minimum
- **Symptom**: Clock controller firmware fault during audio stream
shutdown
- **Fix approach**: Check for 0Hz, clear internal sysclk state, return
early
### Step 1.4: Hidden Bug Fix Detection
This is explicitly a bug fix for an interaction between two commits. The
word "Handle" means "fix the missing handling of this case."
Record: This IS a bug fix - it prevents firmware faults during normal
audio stream shutdown.
## PHASE 2: DIFF ANALYSIS
### Step 2.1: Inventory
- **Files**: `sound/soc/codecs/rt5640.c` only (single file)
- **Lines**: +4 added, 0 removed
- **Function modified**: `rt5640_set_dai_sysclk()`
- **Scope**: Single-file surgical fix
### Step 2.2: Code Flow Change
- **Before**: `rt5640_set_dai_sysclk()` enters the switch statement
unconditionally. With `clk_id=0` (RT5640_SCLK_S_MCLK),
`clk_set_rate(rt5640->mclk, 0)` is called, hitting the clock
controller with an invalid 0Hz rate.
- **After**: When `freq==0`, the function sets `rt5640->sysclk = 0` and
returns early, avoiding the `clk_set_rate(mclk, 0)` call entirely.
### Step 2.3: Bug Mechanism
Category: **Logic/correctness fix** (missing case handling)
- The 0Hz value is a convention from `simple-card-utils` to signal
"clear constraints"
- rt5640 didn't handle this convention, passing the invalid rate
directly to the clock framework
- This causes firmware faults on platforms like Tegra
### Step 2.4: Fix Quality
- **Obviously correct**: Yes - the exact same pattern exists in
rockchip, ep93xx, wm8904, and stm32 sai drivers
- **Minimal/surgical**: Yes - 4 lines, single early-return guard
- **Regression risk**: Extremely low - only affects the freq==0 case,
which was already broken
- **No red flags**: Doesn't touch locking, APIs, or other subsystems
## PHASE 3: GIT HISTORY INVESTIGATION
### Step 3.1: Blame
- `rt5640_set_dai_sysclk()` original structure by Bard Liao (2013,
v3.11-era)
- `clk_set_rate()` call added by Sameer Pujar (9f138bb2eaf661, v6.3) -
an NVIDIA engineer
- The 0Hz shutdown behavior (2458adb8f92ad) was added in v5.10
### Step 3.2: Fixes tag analysis
No explicit Fixes: tag, but the commit references 2458adb8f92a. The bug
is an interaction between:
1. 2458adb8f92a (v5.10) - sends 0Hz sysclk on shutdown
2. 9f138bb2eaf661 (v6.3) - added `clk_set_rate(mclk, freq)` to rt5640
Both commits are in v6.6 and later stable trees.
### Step 3.3: Related changes
Multiple identical fixes exist for the same 0Hz issue in other drivers:
- f1879d7b98dc9 - rockchip (v5.10)
- 9b7a7f921689d - stm32 sai (v5.10)
- 66dc3b9b9a6f4 - ep93xx (v6.3) - includes a concrete crash stack trace
- 2a0bda276c642 - wm8904
### Step 3.4: Author context
Sheetal is an NVIDIA Tegra audio engineer. Sameer Pujar (who added
clk_set_rate) is also NVIDIA. This is the team that owns this platform
and hit this bug directly.
### Step 3.5: Dependencies
None. The fix is completely standalone - it adds a guard before the
existing switch statement.
## PHASE 4: MAILING LIST RESEARCH
### Step 4.1-4.2: Patch discussion
- b4 dig could not find the commit by message-id (too new)
- Lore is behind Anubis anti-scraping protection
- Mark Brown (ASoC maintainer) applied it directly, confirming
acceptance
- The pattern is well-established: identical fixes for ep93xx, rockchip,
stm32, wm8904
### Step 4.3: Bug report
The ep93xx fix (66dc3b9b9a6f4) includes a full stack trace showing
`__div0` crash from `clk_set_rate(0)` during shutdown. The rt5640 commit
describes "clock controller firmware faults" on Tegra - same class of
issue.
### Step 4.4-4.5: Series context
This is a standalone single-patch fix, not part of a series.
## PHASE 5: CODE SEMANTIC ANALYSIS
### Step 5.1-5.2: Function context
`rt5640_set_dai_sysclk()` is registered as `.set_sysclk` in the DAI ops.
It is called by `snd_soc_dai_set_sysclk()` from the ASoC core, triggered
by `asoc_simple_shutdown()` during every stream close.
### Step 5.3-5.4: Call chain
```
PCM stream close -> soc_pcm_close -> soc_pcm_clean ->
snd_soc_link_shutdown
-> asoc_simple_shutdown -> snd_soc_dai_set_sysclk(codec_dai, 0, 0, ...)
-> rt5640_set_dai_sysclk(freq=0) -> clk_set_rate(mclk, 0) -> FIRMWARE
FAULT
```
This is a normal-operation path triggered on every audio stream
shutdown.
### Step 5.5: Similar patterns
Verified: at least 4 other drivers already have identical 0Hz guards
(`if (!freq) return 0` or `if (freq == 0) return 0`).
## PHASE 6: STABLE TREE ANALYSIS
### Step 6.1: Buggy code in stable?
- **v6.1.y**: Does NOT have 9f138bb2eaf661 (`clk_set_rate()` not added)
- bug does not exist
- **v6.6.y**: HAS both 2458adb8f92a and 9f138bb2eaf661 - **BUG EXISTS**
- **v6.12.y**: Same code, **BUG EXISTS**
### Step 6.2: Backport complications
The `rt5640_set_dai_sysclk()` code is **identical** in v6.6, v6.12, and
v7.0. The patch will apply cleanly without any modifications.
### Step 6.3: Related fixes already in stable
No other fix for this specific issue in rt5640 found.
## PHASE 7: SUBSYSTEM CONTEXT
### Step 7.1: Subsystem criticality
- **Subsystem**: ASoC codecs (sound/soc/codecs/) - audio driver
- **Criticality**: IMPORTANT - rt5640 is a widely-used Realtek audio
codec found in many embedded platforms (Tegra, Chromebooks, etc.)
### Step 7.2: Subsystem activity
Active development - multiple recent commits for rt5640.
## PHASE 8: IMPACT AND RISK ASSESSMENT
### Step 8.1: Affected users
Users of platforms combining:
- rt5640 codec with SoC-driven MCLK (Tegra platforms specifically
mentioned)
- simple-card or audio-graph-card machine driver with `mclk-fs` property
### Step 8.2: Trigger conditions
- Triggered on **every audio stream shutdown** - extremely common
operation
- No special conditions needed beyond the hardware configuration above
- This is a normal-operation failure, not an edge case
### Step 8.3: Failure mode severity
- **Clock controller firmware fault** - platform-dependent, but can
cause:
- Error returns from `clk_set_rate()` breaking audio shutdown
- On some platforms (ep93xx), division by zero causing kernel crash
- On Tegra, firmware-level faults that may affect system stability
- **Severity**: HIGH (firmware fault on every stream close)
### Step 8.4: Risk-benefit ratio
- **Benefit**: HIGH - prevents firmware fault/crash on every audio
stream shutdown on affected platforms
- **Risk**: VERY LOW - 4-line early return guard, only affects the
freq==0 case which was already broken
- **Ratio**: Excellent - this is exactly the kind of fix stable trees
are meant to carry
## PHASE 9: FINAL SYNTHESIS
### Step 9.1: Evidence compilation
**FOR backporting**:
- Fixes a real bug: clock controller firmware faults during normal audio
stream shutdown
- Well-established fix pattern: identical fixes exist for 4+ other
drivers
- Concrete crash evidence from ep93xx (division by zero stack trace)
- Tiny, surgical fix: 4 lines, single early-return guard
- Obviously correct: follows exact pattern validated in other drivers
- Clean applicability: code is identical in v6.6, v6.12, and v7.0
- Applied by Mark Brown (ASoC maintainer)
- Author is from the affected platform team (NVIDIA/Tegra)
**AGAINST backporting**:
- None identified
### Step 9.2: Stable rules checklist
1. Obviously correct and tested? **YES** - identical pattern used in 4+
other drivers, applied by maintainer
2. Fixes a real bug? **YES** - firmware faults on every stream shutdown
3. Important issue? **YES** - prevents crash/fault on normal audio
operations
4. Small and contained? **YES** - 4 lines, single function
5. No new features/APIs? **YES** - pure defensive guard
6. Can apply to stable? **YES** - verified identical code in v6.6 and
v6.12
### Step 9.3: Exception categories
Not an exception category - this is a straightforward bug fix.
## Verification
- [Phase 1] Parsed tags: Link to patch.msgid.link, Signed-off-by Mark
Brown (maintainer)
- [Phase 2] Diff analysis: 4 lines added, inserts `if (!freq) {
rt5640->sysclk = 0; return 0; }` before switch in
`rt5640_set_dai_sysclk()`
- [Phase 3] git blame: `clk_set_rate()` added by 9f138bb2eaf661 (Sameer
Pujar, v6.3); 0Hz shutdown added by 2458adb8f92ad (v5.10)
- [Phase 3] `git merge-base --is-ancestor`: 9f138bb2eaf661 IS in v6.6
(YES), NOT in v6.1 (NO)
- [Phase 3] `git merge-base --is-ancestor`: 2458adb8f92ad IS in v6.6
(YES) - both triggering commits present
- [Phase 3] git log: verified 4+ identical fixes (rockchip
f1879d7b98dc9, ep93xx 66dc3b9b9a6f4, stm32 sai 9b7a7f921689d, wm8904
2a0bda276c642)
- [Phase 3] git show 66dc3b9b9a6f4: confirmed ep93xx fix includes
division-by-zero stack trace as concrete crash evidence
- [Phase 4] b4 dig: could not find original submission by message-id
(recent commit)
- [Phase 4] lore: blocked by Anubis anti-scraping
- [Phase 5] grep: `rt5640_set_dai_sysclk` registered as `.set_sysclk` in
DAI ops, called from ASoC framework during shutdown
- [Phase 6] `git show v6.6:sound/soc/codecs/rt5640.c`: code is identical
to v7.0 - patch applies cleanly
- [Phase 6] `git show v6.12:sound/soc/codecs/rt5640.c`: code is
identical to v7.0 - patch applies cleanly
- [Phase 8] Trigger: every audio stream shutdown on affected platforms;
severity HIGH (firmware fault)
The fix is small (4 lines), surgical, obviously correct (proven pattern
in 4+ other drivers), fixes a real firmware fault on every audio stream
shutdown, and applies cleanly to all active stable trees containing the
buggy code (v6.6+).
**YES**
sound/soc/codecs/rt5640.c | 5 +++++
1 file changed, 5 insertions(+)
diff --git a/sound/soc/codecs/rt5640.c b/sound/soc/codecs/rt5640.c
index db2222e6f2e75..f6c6294e15880 100644
--- a/sound/soc/codecs/rt5640.c
+++ b/sound/soc/codecs/rt5640.c
@@ -1838,6 +1838,11 @@ static int rt5640_set_dai_sysclk(struct snd_soc_dai *dai,
unsigned int pll_bit = 0;
int ret;
+ if (!freq) {
+ rt5640->sysclk = 0;
+ return 0;
+ }
+
switch (clk_id) {
case RT5640_SCLK_S_MCLK:
ret = clk_set_rate(rt5640->mclk, freq);
--
2.53.0
^ permalink raw reply related
* [PATCH AUTOSEL 7.0-5.10] ALSA: compress: Refuse to update timestamps for unconfigured streams
From: Sasha Levin @ 2026-04-20 13:17 UTC (permalink / raw)
To: patches, stable
Cc: Mark Brown, Vinod Koul, Takashi Iwai, Sasha Levin, perex, tiwai,
linux-sound, linux-kernel
In-Reply-To: <20260420132314.1023554-1-sashal@kernel.org>
From: Mark Brown <broonie@kernel.org>
[ Upstream commit cf6c18cf83e48986ac40a053d09d3c33624135f6 ]
There are a number of mechanisms, including the userspace accessible
timestamp and buffer availability ioctl()s, which allow us to trigger
a timestamp update on a stream before it has been configured. Since
drivers might rely on stream configuration for reporting of pcm_io_frames,
including potentially doing a division by the number of channels, and
these operations are not meaningful for an unconfigured stream reject
attempts to read timestamps before any configuration is done.
Signed-off-by: Mark Brown <broonie@kernel.org>
Acked-by: Vinod Koul <vkoul@kernel.org>
Signed-off-by: Takashi Iwai <tiwai@suse.de>
Link: https://patch.msgid.link/20260401-alsa-unconfigured-tstamp-v1-1-694c2cb5f71d@kernel.org
Signed-off-by: Sasha Levin <sashal@kernel.org>
---
LLM Generated explanations, may be completely bogus:
This confirms: `snd_compr_ioctl_avail` calls `snd_compr_calc_avail`
(line 267) which calls `snd_compr_update_tstamp` **before** the state
check at line 275. So even the AVAIL ioctl triggers the `->pointer()`
callback before checking state. The fix in `snd_compr_update_tstamp`
itself properly guards *both* paths (TSTAMP and AVAIL).
Now let me finalize the analysis.
---
## PHASE 1: COMMIT MESSAGE FORENSICS
**Step 1.1: Subject Line**
- Subsystem: `ALSA: compress`
- Action verb: "Refuse" (defensive/preventive - indicates preventing
incorrect behavior)
- Summary: Refuse timestamp updates for streams that haven't been
configured yet.
Record: [ALSA: compress] [Refuse] [Prevent timestamp update on
unconfigured streams]
**Step 1.2: Tags**
- `Signed-off-by: Mark Brown <broonie@kernel.org>` - Author is the ASoC
subsystem maintainer
- `Acked-by: Vinod Koul <vkoul@kernel.org>` - Vinod Koul is the original
compress_offload author
- `Signed-off-by: Takashi Iwai <tiwai@suse.de>` - Takashi Iwai is the
ALSA top-level maintainer
- `Link:` to patch.msgid.link (the submission)
Record: Authored by ASoC subsystem maintainer, Acked by compress_offload
author, merged by ALSA maintainer. Strong review pedigree.
**Step 1.3: Commit Body**
The commit describes: userspace-accessible timestamp and buffer
availability ioctls can trigger a timestamp update on a stream before it
has been configured. Drivers might rely on stream configuration for
reporting `pcm_io_frames`, including "potentially doing a division by
the number of channels." Since these operations are not meaningful for
unconfigured streams, reject attempts to read timestamps before
configuration.
Record: Bug = calling driver `->pointer()` callback before `set_params`
has been called. Symptom = potential divide-by-zero in drivers (number
of channels = 0 before configuration). Root cause = missing state
validation in `snd_compr_update_tstamp()`.
**Step 1.4: Hidden Bug Fix?**
This is explicitly a defensive fix preventing a divide-by-zero crash.
The "Refuse" language hides what is actually a crash prevention fix.
Record: YES, this is a hidden bug fix. The commit message explicitly
says "doing a division by the number of channels" which is the crash
mechanism.
## PHASE 2: DIFF ANALYSIS
**Step 2.1: Inventory**
- Single file: `sound/core/compress_offload.c`
- Lines added: ~7 (switch statement checking SNDRV_PCM_STATE_OPEN)
- Lines removed: 0
- Function modified: `snd_compr_update_tstamp()`
- Classification: Single-file surgical fix
**Step 2.2: Code Flow Change**
Before: `snd_compr_update_tstamp()` immediately calls
`stream->ops->pointer()` with no state validation.
After: A switch statement checks if `stream->runtime->state ==
SNDRV_PCM_STATE_OPEN` (pre-configuration state) and returns `-EBADFD` if
so, preventing the `->pointer()` call.
**Step 2.3: Bug Mechanism**
Category: Divide-by-zero / uninitialized state access. When a compress
offload stream is just opened (state = OPEN), driver parameters
(channels, sample_container_bytes) are zero. Calling `->pointer()` in
this state causes drivers like SOF's `sof_compr_pointer()` to execute
`div_u64(..., sstream->channels * sstream->sample_container_bytes)`
which divides by zero.
**Step 2.4: Fix Quality**
- Obviously correct: The state check pattern is used extensively
throughout this same file (lines 276, 391, 446, 652, 846, 946, 998)
- Minimal: 7 lines added
- No regression risk: Returning -EBADFD for OPEN state is the standard
pattern used by all other ioctls
- The caller `snd_compr_calc_avail` already has comment "Still need to
return avail even if tstamp can't be filled in" showing it handles the
error
## PHASE 3: GIT HISTORY
**Step 3.1: Blame**
- `snd_compr_update_tstamp()` was introduced by commit b21c60a4edd22e
(2011, v3.3-era) by Vinod Koul as part of the original
compress_offload support
- The bug has existed since the very beginning (~2011)
**Step 3.2: Fixes tag**
- No Fixes: tag present (expected for autosel candidates)
**Step 3.3: File history**
- Recent changes to the file: 64-bit timestamp infrastructure
(2c92e2fbe9e22c, Sept 2025) made the issue more visible since it
changed the pointer callback signature, but the underlying bug
predates that
**Step 3.4: Author**
- Mark Brown (`broonie@kernel.org`) is the ASoC subsystem maintainer -
one of the most trusted kernel developers
**Step 3.5: Dependencies**
- This fix applies independently. It only adds a state check using
existing infrastructure (`SNDRV_PCM_STATE_OPEN`).
- NOTE: In older stable trees (pre-6.12ish), the function signature may
differ (uses `snd_compr_tstamp` instead of `snd_compr_tstamp64`), but
the state check logic is identical and would need only trivial
adaptation.
## PHASE 4: MAILING LIST
Lore was blocked by Anubis anti-bot protection. However:
- The patch was v1 (no revisions needed, indicating it was clean from
the start)
- It was Acked by the compress_offload author Vinod Koul
- Merged directly by ALSA maintainer Takashi Iwai
Record: Could not fetch lore discussion. Strong review indicators from
tags.
## PHASE 5: CODE SEMANTIC ANALYSIS
**Step 5.1: Functions modified**
- `snd_compr_update_tstamp()` - the internal timestamp update helper
**Step 5.2: Callers**
- `snd_compr_calc_avail()` (line 209) - called from AVAIL/AVAIL64 ioctls
- `snd_compr_tstamp()` (line 760) - called from TSTAMP/TSTAMP64 ioctls
- Both paths are directly reachable from userspace via ioctl
**Step 5.3-5.4: Call chain**
`open() → ioctl(SNDRV_COMPRESS_TSTAMP) → snd_compr_tstamp() →
snd_compr_update_tstamp() → stream->ops->pointer()` - This is directly
triggerable by any user with access to the compress device.
**Step 5.5: Similar patterns**
- Confirmed: `sof_compr_pointer()` does `div_u64(..., sstream->channels
* sstream->sample_container_bytes)` at line 384-385
- `sst_cdev_tstamp()` does `div_u64(fw_tstamp.hardware_counter,
stream->num_ch * ...)` at line 348-349
- Both would divide by zero if called before set_params
## PHASE 6: STABLE TREE ANALYSIS
**Step 6.1: Code exists in stable trees**
- The buggy function `snd_compr_update_tstamp()` has existed since 2011
(v3.3)
- Present in ALL active stable trees
- NOTE: The 64-bit tstamp infrastructure is only in v6.12+. Older trees
use `snd_compr_tstamp` struct. The fix concept applies to all, but the
exact patch applies cleanly only to v7.0 and trees with the 64-bit
tstamp change.
**Step 6.2: Backport complications**
- For v7.0: Applies cleanly (verified function matches exactly)
- For older trees: Needs minor adaptation (different struct type in
signature)
**Step 6.3: Related fixes**
- No similar state check has been applied to `snd_compr_update_tstamp`
before
## PHASE 7: SUBSYSTEM AND MAINTAINER CONTEXT
**Step 7.1: Subsystem**
- `sound/core/` - ALSA core, IMPORTANT criticality
- Compress offload is used by mobile/embedded platforms (Qualcomm, Intel
Atom, SOF)
**Step 7.2: Activity**
- Active subsystem with recent work (64-bit tstamp infrastructure added
in 2025)
## PHASE 8: IMPACT AND RISK ASSESSMENT
**Step 8.1: Who is affected**
- Users of ALSA compressed audio (primarily mobile/embedded, Android,
Intel Atom, SOF platforms)
**Step 8.2: Trigger conditions**
- Trivially triggerable: open compress device, call TSTAMP or AVAIL
ioctl before SET_PARAMS
- Can be triggered by unprivileged user with access to the audio device
- No special timing required - completely deterministic
**Step 8.3: Failure severity**
- Divide-by-zero → kernel oops/crash → **CRITICAL**
**Step 8.4: Risk-benefit**
- BENEFIT: Very high - prevents deterministic kernel crash from
userspace
- RISK: Very low - 7 lines, uses established pattern, authored by ASoC
maintainer, acked by compress_offload author
## PHASE 9: FINAL SYNTHESIS
**Evidence FOR:**
1. Fixes a divide-by-zero crash triggerable from userspace (CRITICAL)
2. Extremely small and surgical fix (7 lines)
3. Uses an existing, well-established pattern (same check at 6+ other
locations in the file)
4. Authored by ASoC subsystem maintainer Mark Brown
5. Acked by compress_offload original author Vinod Koul
6. Bug exists since 2011 - affects all stable trees
7. Multiple drivers have the vulnerable division (SOF, Intel Atom)
**Evidence AGAINST:**
1. Needs minor adaptation for trees without 64-bit tstamp (pre-6.12ish)
**Stable rules checklist:**
1. Obviously correct? YES - uses identical pattern to 6+ other ioctls in
same file
2. Fixes a real bug? YES - divide-by-zero crash
3. Important issue? YES - kernel crash triggerable from userspace
4. Small and contained? YES - 7 lines, single function
5. No new features/APIs? Correct - purely defensive check
6. Applies to stable? YES for 7.0 (clean apply); needs minor rework for
older trees
## Verification
- [Phase 1] Parsed tags: Signed-off-by Mark Brown (ASoC maintainer),
Acked-by Vinod Koul (compress_offload author), Signed-off-by Takashi
Iwai (ALSA maintainer)
- [Phase 2] Diff analysis: 7 lines added in `snd_compr_update_tstamp()`,
adds state check for SNDRV_PCM_STATE_OPEN
- [Phase 3] git blame: `snd_compr_update_tstamp()` introduced in commit
b21c60a4edd22e (2011), present in all stable trees
- [Phase 3] git show b21c60a4edd22e: confirmed original compress_offload
commit
- [Phase 3] git merge-base: confirmed 64-bit tstamp infrastructure
(2c92e2fbe9e22c) is ancestor of v7.0
- [Phase 4] Lore blocked by Anubis; relied on tag analysis for review
evidence
- [Phase 5] Grep callers: `snd_compr_update_tstamp` called from lines
209 and 760, both ioctl-reachable
- [Phase 5] Verified SOF driver `sof_compr_pointer()` does `div_u64(...,
sstream->channels * sstream->sample_container_bytes)` at line 384-385
- [Phase 5] Verified Intel Atom `sst_cdev_tstamp()` does `div_u64(...,
stream->num_ch * ...)` at line 348-349
- [Phase 5] Verified `snd_compr_set_params()` sets channels (line 256 in
sof compress.c), confirming channels=0 before set_params
- [Phase 6] git show v7.0:compress_offload.c: function matches current
HEAD exactly, clean apply confirmed
- [Phase 6] Existing divide-by-zero protection commit 678e2b44c8e3fec
(2018) validates this class of bug is real
- [Phase 7] Identified `snd_compr_ioctl_avail` line 267 calls
update_tstamp BEFORE its own state check at line 275, confirming both
AVAIL and TSTAMP paths are vulnerable
- [Phase 8] Failure mode: divide-by-zero → kernel oops, severity
CRITICAL, trivially triggerable from userspace
This is a textbook stable backport: a small, surgical fix preventing a
deterministic kernel crash (divide-by-zero) that is trivially
triggerable from userspace via ioctl. Authored and reviewed by the top
three relevant maintainers.
**YES**
sound/core/compress_offload.c | 8 ++++++++
1 file changed, 8 insertions(+)
diff --git a/sound/core/compress_offload.c b/sound/core/compress_offload.c
index fdba6e4b25fdc..67b3b1a3b5261 100644
--- a/sound/core/compress_offload.c
+++ b/sound/core/compress_offload.c
@@ -192,6 +192,14 @@ static int snd_compr_update_tstamp(struct snd_compr_stream *stream,
{
if (!stream->ops->pointer)
return -ENOTSUPP;
+
+ switch (stream->runtime->state) {
+ case SNDRV_PCM_STATE_OPEN:
+ return -EBADFD;
+ default:
+ break;
+ }
+
stream->ops->pointer(stream, tstamp);
pr_debug("dsp consumed till %u total %llu bytes\n", tstamp->byte_offset,
tstamp->copied_total);
--
2.53.0
^ permalink raw reply related
* [PATCH AUTOSEL 7.0-5.10] ASoC: Intel: cht_bsw_rt5672: Fix MCLK leak on platform_clock_control error
From: Sasha Levin @ 2026-04-20 13:17 UTC (permalink / raw)
To: patches, stable
Cc: Aravind Anilraj, Cezary Rojewski, Mark Brown, Sasha Levin,
liam.r.girdwood, peter.ujfalusi, yung-chuan.liao,
ranjani.sridharan, kai.vehmanen, perex, tiwai, linux-sound,
linux-kernel
In-Reply-To: <20260420132314.1023554-1-sashal@kernel.org>
From: Aravind Anilraj <aravindanilraj0702@gmail.com>
[ Upstream commit dced5a373a96cfd9f3bd0ffcf5339a7579d1473a ]
If snd_soc_dai_set_pll() or snd_soc_dai_set_sysclk() fail inside the
EVENT_ON path, the function returns without calling
clk_disable_unprepare() on ctx->mclk, which was already enabled earlier
in the same code path. Add the missing clk_disable_unprepare() calls
before returning the error.
Signed-off-by: Aravind Anilraj <aravindanilraj0702@gmail.com>
Reviewed-by: Cezary Rojewski <cezary.rojewski@intel.com>
Link: https://patch.msgid.link/20260401220507.23557-4-aravindanilraj0702@gmail.com
Signed-off-by: Mark Brown <broonie@kernel.org>
Signed-off-by: Sasha Levin <sashal@kernel.org>
---
LLM Generated explanations, may be completely bogus:
I now have all the information needed for a complete analysis. Let me
compile the full assessment.
---
## PHASE 1: COMMIT MESSAGE FORENSICS
### Step 1.1: PARSE THE SUBJECT LINE
Record: [ASoC: Intel: cht_bsw_rt5672] [Fix] [MCLK leak on
platform_clock_control error path]
### Step 1.2: PARSE ALL COMMIT MESSAGE TAGS
- **Signed-off-by:** Aravind Anilraj (author)
- **Reviewed-by:** Cezary Rojewski (Intel ASoC reviewer)
- **Link:** patch.msgid.link to the original patch
- **Signed-off-by:** Mark Brown (ASoC subsystem maintainer, committer)
- No Fixes: tag (expected for manual review candidates)
- No Reported-by: tag
- No Cc: stable tag
Record: Reviewed by Intel ASoC developer and merged by ASoC subsystem
maintainer. No syzbot or external reporter — this is a code-inspection-
found bug.
### Step 1.3: ANALYZE THE COMMIT BODY TEXT
The commit message clearly states: if `snd_soc_dai_set_pll()` or
`snd_soc_dai_set_sysclk()` fail in the EVENT_ON path, the function
returns without calling `clk_disable_unprepare()` on `ctx->mclk`, which
was already enabled. The fix adds the missing cleanup calls.
Record: Bug: clock resource leak on error paths. Symptom: MCLK left
enabled after error, preventing proper power management. Root cause:
missing cleanup in two error return paths.
### Step 1.4: DETECT HIDDEN BUG FIXES
This is explicitly labeled as a "Fix" and describes a resource leak — no
disguise here.
Record: This is an overt bug fix for a resource leak.
## PHASE 2: DIFF ANALYSIS
### Step 2.1: INVENTORY THE CHANGES
- **File:** `sound/soc/intel/boards/cht_bsw_rt5672.c`
- **Lines added:** +4 (two `if (ctx->mclk)
clk_disable_unprepare(ctx->mclk);` blocks)
- **Lines removed:** 0
- **Functions modified:** `platform_clock_control()`
- **Scope:** Single-file, surgical fix
Record: 1 file, 4 lines added, 0 removed. Single function modified.
Extremely contained.
### Step 2.2: UNDERSTAND THE CODE FLOW CHANGE
- **Before:** In the EVENT_ON path, after
`clk_prepare_enable(ctx->mclk)` succeeds, if either
`snd_soc_dai_set_pll()` or `snd_soc_dai_set_sysclk()` fails, the
function returns the error without disabling the clock.
- **After:** Both error paths now call
`clk_disable_unprepare(ctx->mclk)` (guarded by `if (ctx->mclk)`)
before returning the error.
Record: Error paths now properly clean up the enabled clock before
returning.
### Step 2.3: IDENTIFY THE BUG MECHANISM
**Category: Error path / resource leak fix.**
The clock `ctx->mclk` is enabled via `clk_prepare_enable()` at line 67.
If subsequent calls fail (lines 78-81 or 86-89), the function returns
without the matching `clk_disable_unprepare()`. This leaves the platform
clock running, preventing proper power management. The EVENT_OFF path at
line 103-104 already properly calls `clk_disable_unprepare()`,
confirming the intended pattern.
Record: Resource leak — MCLK left in enabled/prepared state on error
paths.
### Step 2.4: ASSESS THE FIX QUALITY
- **Obviously correct:** Yes. Mirrors the existing cleanup pattern in
the EVENT_OFF path (line 103-104). The `if (ctx->mclk)` guard is
consistent with the guard at line 66.
- **Minimal/surgical:** Yes. Only 4 lines added, no unrelated changes.
- **Regression risk:** Extremely low. Only affects error paths. The
`clk_disable_unprepare()` call is the exact counterpart to
`clk_prepare_enable()` that was already called.
Record: Fix is obviously correct, minimal, and has negligible regression
risk.
## PHASE 3: GIT HISTORY INVESTIGATION
### Step 3.1: BLAME THE CHANGED LINES
The MCLK handling was introduced by commit `c25695ae88ce26` (Pierre-
Louis Bossart, 2017-06-23) — "ASoC: Intel: cht_bsw_rt5672: 19.2MHz clock
for Baytrail platforms". The bug has existed since that commit, which
was included in v4.13-rc1.
Record: Buggy code introduced in c25695ae88ce26 (v4.13-rc1, 2017).
Present in all active stable trees.
### Step 3.2: FOLLOW THE FIXES: TAG
No Fixes: tag present. However, the logical "Fixes:" target is
`c25695ae88ce26`, which introduced the MCLK handling without proper
error path cleanup.
Record: The implicit Fixes target (c25695ae88ce26) is in all stable
trees since v4.13.
### Step 3.3: CHECK FILE HISTORY
The file has had ~20 commits over the years, mostly cleanups and
conversions. No prior attempt to fix this specific leak was found (no
commits matching "MCLK leak" in the history).
Record: Standalone fix, no prerequisites needed. No prior fix for this
issue exists.
### Step 3.4: CHECK THE AUTHOR
The author (Aravind Anilraj) appears to be a new contributor. However,
the patch was reviewed by Cezary Rojewski (Intel ASoC reviewer) and
merged by Mark Brown (ASoC subsystem maintainer).
Record: Reviewed by experienced subsystem developers despite being from
a new contributor.
### Step 3.5: CHECK FOR DEPENDENCIES
The fix only adds `clk_disable_unprepare()` calls to existing error
paths. No new functions, structures, or APIs are used. The fix is
completely self-contained.
Record: No dependencies. Can apply standalone.
## PHASE 4: MAILING LIST RESEARCH
### Step 4.1-4.5: PATCH DISCUSSION
The lore.kernel.org site was not reachable due to bot protection. b4 dig
could not find the commit by message-id (it doesn't exist in this tree
as a commit). However, the commit tags show:
- **Reviewed-by:** Cezary Rojewski (Intel) — an active ASoC reviewer
- **Signed-off-by:** Mark Brown — the ASoC subsystem maintainer who
merged it
These signatures indicate the patch went through proper review.
Record: Proper review by Intel ASoC developer and subsystem maintainer.
Could not access lore discussion directly.
## PHASE 5: CODE SEMANTIC ANALYSIS
### Step 5.1-5.2: FUNCTION AND CALLERS
`platform_clock_control()` is registered as a DAPM supply widget
callback:
```114:116:sound/soc/intel/boards/cht_bsw_rt5672.c
SND_SOC_DAPM_SUPPLY("Platform Clock", SND_SOC_NOPM, 0, 0,
platform_clock_control, SND_SOC_DAPM_PRE_PMU |
SND_SOC_DAPM_POST_PMD),
```
It is called by the DAPM framework whenever the "Platform Clock" supply
widget powers up (PRE_PMU) or down (POST_PMD). This happens during every
audio playback/capture start and stop operation. All audio paths
("Headphone", "Headset Mic", "Int Mic", "Ext Spk") depend on this widget
(lines 130-133).
Record: Called on every audio stream open/close. High frequency for
audio-active systems.
### Step 5.3-5.5: SIMILAR PATTERNS
I found the same bug pattern in sibling drivers `bytcr_rt5651.c` and
`bytcr_rt5640.c`, though those have slightly different code structure.
The `cht_bsw_rt5672.c` fix is specific to this driver and doesn't
require changes to siblings.
Record: Similar pattern exists in sibling drivers but this fix is self-
contained for cht_bsw_rt5672.
## PHASE 6: STABLE TREE ANALYSIS
### Step 6.1: BUGGY CODE IN STABLE
The buggy code was introduced in v4.13-rc1 (commit c25695ae88ce26). It
exists in all active stable trees (5.10.y, 5.15.y, 6.1.y, 6.6.y, 6.12.y,
etc.).
Record: Bug affects all active stable trees.
### Step 6.2: BACKPORT COMPLICATIONS
The file has been relatively stable. The current code at the fix site
matches what was introduced in c25695ae88ce26, with only minor changes
(the `if (ctx->mclk)` guard and the `65b2df10a1e62` commit that changed
the EVENT_OFF path). The fix should apply cleanly or with minimal
adjustment.
Record: Expected clean or near-clean apply to all stable trees.
### Step 6.3: RELATED FIXES
No prior fix for this specific issue was found in any tree.
Record: No existing fix for this leak.
## PHASE 7: SUBSYSTEM AND MAINTAINER CONTEXT
### Step 7.1: SUBSYSTEM CRITICALITY
- **Subsystem:** sound/soc/intel/boards — ASoC Intel machine driver
- **Criticality:** PERIPHERAL (specific to Cherryview/Baytrail platforms
with RT5672 codec)
- **Affected hardware:** Lenovo and other Cherryview/Baytrail-based
laptops/tablets with RT5670/RT5672 codec (reasonably common consumer
devices)
Record: ASoC Intel board driver, PERIPHERAL but for consumer devices
(laptops/tablets).
### Step 7.2: SUBSYSTEM ACTIVITY
The file sees occasional updates (cleanups and fixes). It's a mature
driver.
Record: Mature, stable driver with occasional maintenance commits.
## PHASE 8: IMPACT AND RISK ASSESSMENT
### Step 8.1: AFFECTED USERS
Users of Cherryview/Baytrail platforms with RT5670/RT5672 codec. This
includes Lenovo and similar consumer laptops/tablets from the Baytrail
era.
Record: Platform-specific but for real consumer devices.
### Step 8.2: TRIGGER CONDITIONS
The bug triggers when `snd_soc_dai_set_pll()` or
`snd_soc_dai_set_sysclk()` fails during audio stream start. While these
failures are not common in normal operation, they can occur during
hardware errors, suspend/resume transitions, or codec communication
issues.
Record: Trigger requires PLL/sysclk configuration failure during audio
start. Uncommon but possible.
### Step 8.3: FAILURE MODE SEVERITY
When triggered, the platform clock remains enabled (leaked), preventing
proper power management. Repeated triggering could cause increased power
consumption. The clock framework may also track prepare/enable counts
incorrectly, potentially affecting system suspend or causing warnings.
Record: Severity: MEDIUM (resource leak affecting power management, no
crash).
### Step 8.4: RISK-BENEFIT RATIO
- **Benefit:** Fixes a real resource leak on error paths, improves power
management reliability.
- **Risk:** Very low — 4 lines added, only on error paths, mirrors
existing cleanup patterns.
- **Ratio:** Favorable. Even though the trigger is uncommon, the fix is
trivially safe.
Record: Benefit outweighs risk. Minimal fix with no regression
potential.
## PHASE 9: FINAL SYNTHESIS
### Step 9.1: EVIDENCE COMPILATION
**FOR backporting:**
- Fixes a real clock resource leak (missing `clk_disable_unprepare()` on
error paths)
- Extremely small and surgical (4 lines, single file, single function)
- Obviously correct — mirrors existing cleanup pattern in the same
function
- Bug has existed since v4.13-rc1 (2017), affects all stable trees
- Reviewed by Intel ASoC developer, merged by subsystem maintainer
- No dependencies, self-contained
- No regression risk
**AGAINST backporting:**
- No user reports of the issue (code-inspection find)
- Trigger requires error conditions that are uncommon in normal
operation
- Driver-specific (not core kernel)
### Step 9.2: STABLE RULES CHECKLIST
1. Obviously correct and tested? **YES** — reviewed by Intel developer,
merged by maintainer
2. Fixes a real bug? **YES** — clock resource leak on error paths
3. Important issue? **MEDIUM** — resource leak, not a crash
4. Small and contained? **YES** — 4 lines, single file
5. No new features or APIs? **YES** — only adds cleanup calls
6. Can apply to stable? **YES** — code unchanged since introduction
### Step 9.3: EXCEPTION CATEGORIES
Not an exception category — this is a straightforward bug fix.
### Step 9.4: DECISION
This is a small, obvious, and safe fix for a real resource leak. The fix
adds missing `clk_disable_unprepare()` calls on two error paths where
the clock was already enabled. It's reviewed by the right people, has no
dependencies, and applies to all stable trees.
## Verification
- [Phase 1] Parsed tags: Reviewed-by Cezary Rojewski (Intel), merged by
Mark Brown (ASoC maintainer)
- [Phase 2] Diff analysis: 4 lines added, two `if (ctx->mclk)
clk_disable_unprepare(ctx->mclk)` blocks on error paths
- [Phase 3] git blame: buggy code introduced in c25695ae88ce26
(v4.13-rc1, 2017), present in all stable trees
- [Phase 3] git show c25695ae88ce26: confirmed original commit added
MCLK handling without error path cleanup
- [Phase 3] git describe --contains c25695ae88ce26:
v4.13-rc1~142^2~1^2~5^2~8 — confirmed it's been in mainline since
v4.13
- [Phase 3] git log --oneline -20 -- file: no prior MCLK leak fix found
- [Phase 4] b4 dig: could not find commit by message-id in local tree;
lore blocked by bot protection
- [Phase 5] DAPM widget registration at lines 114-116 confirms
`platform_clock_control` is called on every audio stream start/stop
- [Phase 5] DAPM routes at lines 130-133 confirm all audio paths depend
on Platform Clock
- [Phase 5] Grep for clk_disable_unprepare in sibling drivers confirms
the same cleanup pattern is expected
- [Phase 6] Code at the fix site is essentially unchanged since
c25695ae88ce26 — clean apply expected
- [Phase 8] Failure mode: clock resource leak preventing proper power
management, severity MEDIUM
- UNVERIFIED: Could not access lore.kernel.org discussion for reviewer
commentary on stable suitability
**YES**
sound/soc/intel/boards/cht_bsw_rt5672.c | 4 ++++
1 file changed, 4 insertions(+)
diff --git a/sound/soc/intel/boards/cht_bsw_rt5672.c b/sound/soc/intel/boards/cht_bsw_rt5672.c
index 359723f2700e4..57d6997eb12ff 100644
--- a/sound/soc/intel/boards/cht_bsw_rt5672.c
+++ b/sound/soc/intel/boards/cht_bsw_rt5672.c
@@ -77,6 +77,8 @@ static int platform_clock_control(struct snd_soc_dapm_widget *w,
CHT_PLAT_CLK_3_HZ, 48000 * 512);
if (ret < 0) {
dev_err(card->dev, "can't set codec pll: %d\n", ret);
+ if (ctx->mclk)
+ clk_disable_unprepare(ctx->mclk);
return ret;
}
@@ -85,6 +87,8 @@ static int platform_clock_control(struct snd_soc_dapm_widget *w,
48000 * 512, SND_SOC_CLOCK_IN);
if (ret < 0) {
dev_err(card->dev, "can't set codec sysclk: %d\n", ret);
+ if (ctx->mclk)
+ clk_disable_unprepare(ctx->mclk);
return ret;
}
} else {
--
2.53.0
^ permalink raw reply related
page: next (older) | prev (newer) | latest
- recent:[subjects (threaded)|topics (new)|topics (active)]
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox