* [PATCH v2 0/3] ASoC: qcom: lpass: Switch VA/WSA macros to PM clock framework
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
This series converts LPASS WSA and VA macro codec drivers to the PM
clock framework for runtime PM clock handling.
Clock enable/disable sequencing during runtime suspend/resume is moved
to pm_clk helpers, while regcache state handling remains in the driver
runtime PM callbacks. This aligns these codec drivers with common LPASS
runtime PM patterns and reduces idle clock voting.
Patch 3 switches WSA MCLK clock registration to devm-managed
clk_hw registration.
---
v1: lore.kernel.org/r/20260413121824.375473-1-ajay.nandam@oss.qualcomm.com
Changes since v1:
- Fixed runtime resume error handling in WSA/VA paths:
- keep regcache cache-only on pm_clk_resume() failure
- unwind clocks with pm_clk_suspend() if regcache_sync() fails
- Dropped the separate "Guard optional NPL clock rate programming"
patch as discussed on list
Ajay Kumar Nandam (3):
ASoC: codecs: lpass-wsa-macro: Switch to PM clock framework for
runtime PM
ASoC: codecs: lpass-va-macro: Switch to PM clock framework for runtime
PM
ASoC: codecs: lpass-wsa-macro: Use devm_clk_hw_register() for MCLK
output
sound/soc/codecs/lpass-va-macro.c | 123 ++++++++++++++---------------
sound/soc/codecs/lpass-wsa-macro.c | 115 +++++++++------------------
2 files changed, 97 insertions(+), 141 deletions(-)
--
2.34.1
^ permalink raw reply
* [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
* Re: [PATCH RFC 06/10] arm64: dts: qcom: msm8939-asus-z00t: add Venus
From: Dmitry Baryshkov @ 2026-04-20 12:31 UTC (permalink / raw)
To: André Apitzsch
Cc: Erikas Bitovtas, Konrad Dybcio, Bryan O'Donoghue,
Vikash Garodia, Dikshita Agarwal, Mauro Carvalho Chehab,
Rob Herring, Krzysztof Kozlowski, Conor Dooley, Bjorn Andersson,
Konrad Dybcio, Michael Turquette, Stephen Boyd, linux-media,
linux-arm-msm, devicetree, linux-kernel, linux-clk,
~postmarketos/upstreaming, phone-devel
In-Reply-To: <a09485c54946b754741259540257393734b518c3.camel@apitzsch.eu>
On Sun, Apr 19, 2026 at 10:26:56PM +0200, André Apitzsch wrote:
> Hi Dmitry,
>
> Am Samstag, dem 18.04.2026 um 02:40 +0300 schrieb Dmitry Baryshkov:
> > On Thu, Apr 16, 2026 at 07:57:30PM +0300, Erikas Bitovtas wrote:
> > >
> > >
> > > On 4/16/26 6:17 PM, Konrad Dybcio wrote:
> > > > On 4/16/26 3:43 PM, Erikas Bitovtas wrote:
> > > > > Enable Venus video encoder/decoder for Asus ZenFone 2
> > > > > Laser/Selfie.
> > > > >
> > > > > Signed-off-by: Erikas Bitovtas <xerikasxx@gmail.com>
> > > > > ---
> > > > > arch/arm64/boot/dts/qcom/msm8939-asus-z00t.dts | 8 ++++++++
> > > > > 1 file changed, 8 insertions(+)
> > > > >
> > > > > diff --git a/arch/arm64/boot/dts/qcom/msm8939-asus-z00t.dts
> > > > > b/arch/arm64/boot/dts/qcom/msm8939-asus-z00t.dts
> > > > > index 90e966242720..231a3e9c1929 100644
> > > > > --- a/arch/arm64/boot/dts/qcom/msm8939-asus-z00t.dts
> > > > > +++ b/arch/arm64/boot/dts/qcom/msm8939-asus-z00t.dts
> > > > > @@ -267,6 +267,14 @@ &usb_hs_phy {
> > > > > extcon = <&usb_id>;
> > > > > };
> > > > >
> > > > > +&venus {
> > > > > + status = "okay";
> > > >
> > > > You need a firmware path here
> > >
> > > When I tested Venus on my device, it loaded without one specified -
> > > msm-firmware-loader creates a symbolic link from modem partition
> > > for firmware. Additionally, none of the MSM8916 devices seem to
> > > include a firmware name. Has something changed since then?
> >
> > Us becoming more strict? Or more caring? The default file paths are
> > supposed to be used for unfused devices. So if they don't work with
> > yours (most likely they don't), please add firmware-name:
> >
> > firmware-name = "qcom/msm8916/Asus/z00t/venus.mbn";
>
> For BQ M5 (msm8939-longcheer-l9100), venus works with the firmware
> provided by linux-firmware [1] and the default path (here "qcom/venus-
> 1.8/venus.mbn").
>
> Just to be clear, should the firmware-name still be added in this case,
> even if it is the default one?
No need to, but it would be nice to mention in the commit message.
>
> Best regards,
> André
>
>
> [1] https://git.kernel.org/pub/scm/linux/kernel/git/firmware/linux-firmware.git/tree/qcom/venus-1.8
--
With best wishes
Dmitry
^ permalink raw reply
* [PATCH v2 0/3] pinctrl: qcom: lpass-lpi: Switch to PM clock framework
From: Ajay Kumar Nandam @ 2026-04-20 12:31 UTC (permalink / raw)
To: Bjorn Andersson, Linus Walleij
Cc: linux-arm-msm, linux-gpio, linux-kernel, mohammad.rafi.shaik,
ajay.nandam
This series moves LPASS LPI pinctrl runtime clock control to the PM
clock framework and ensures GPIO register accesses runtime-resume the
block before MMIO.
The common LPASS LPI core now uses PM clocks and autosuspend. Runtime PM
callbacks are wired for all LPASS LPI variant drivers sharing the common
core so behavior is consistent across SoCs using DT-provided clocks via
of_pm_clk_add_clks().
---
v1: lore.kernel.org/r/20260413122233.375945-1-ajay.nandam@oss.qualcomm.com
Changes since v1:
- Kept GPIO helper return type adjustments in the runtime PM access
patch and switched access paths to pm_runtime_resume_and_get()
- Added runtime PM callback wiring for remaining LPASS LPI SoC drivers
(sc8280xp, sm4250, sm6115, sm8250, sm8450, sm8550, sm8650)
- Addressed style nits in GPIO access helper declaration formatting
Testing:
- Runtime behavior validated on Kodiak (sc7280)
- Other LPASS LPI variants compile-tested; wider runtime testing welcome
Ajay Kumar Nandam (3):
pinctrl: qcom: lpass-lpi: Switch to PM clock framework for runtime PM
pinctrl: qcom: lpass-lpi: Enable runtime PM hooks on remaining SoCs
pinctrl: qcom: lpass-lpi: Resume clocks for GPIO access
drivers/pinctrl/qcom/pinctrl-lpass-lpi.c | 48 ++++++++++++-------
.../pinctrl/qcom/pinctrl-sc7280-lpass-lpi.c | 7 +++
.../pinctrl/qcom/pinctrl-sc8280xp-lpass-lpi.c | 11 ++++-
.../pinctrl/qcom/pinctrl-sm4250-lpass-lpi.c | 7 +++
.../pinctrl/qcom/pinctrl-sm6115-lpass-lpi.c | 7 +++
.../pinctrl/qcom/pinctrl-sm8250-lpass-lpi.c | 11 ++++-
.../pinctrl/qcom/pinctrl-sm8450-lpass-lpi.c | 11 ++++-
.../pinctrl/qcom/pinctrl-sm8550-lpass-lpi.c | 11 ++++-
.../pinctrl/qcom/pinctrl-sm8650-lpass-lpi.c | 11 ++++-
9 files changed, 97 insertions(+), 27 deletions(-)
--
2.34.1
^ permalink raw reply
* [PATCH v2 1/3] pinctrl: qcom: lpass-lpi: Switch to PM clock framework for runtime PM
From: Ajay Kumar Nandam @ 2026-04-20 12:31 UTC (permalink / raw)
To: Bjorn Andersson, Linus Walleij
Cc: linux-arm-msm, linux-gpio, linux-kernel, mohammad.rafi.shaik,
ajay.nandam
In-Reply-To: <20260420123135.350446-1-ajay.nandam@oss.qualcomm.com>
Convert the LPASS LPI pinctrl driver to use the PM clock framework for
runtime power management.
This allows the LPASS LPI pinctrl driver to drop clock votes when idle,
improves power efficiency on platforms using LPASS LPI island mode, and
aligns the driver with common runtime PM patterns used across Qualcomm
LPASS subsystems.
Signed-off-by: Ajay Kumar Nandam <ajay.nandam@oss.qualcomm.com>
---
drivers/pinctrl/qcom/pinctrl-lpass-lpi.c | 22 +++++++++----------
.../pinctrl/qcom/pinctrl-sc7280-lpass-lpi.c | 7 ++++++
2 files changed, 18 insertions(+), 11 deletions(-)
diff --git a/drivers/pinctrl/qcom/pinctrl-lpass-lpi.c b/drivers/pinctrl/qcom/pinctrl-lpass-lpi.c
index 76aed3296279..2b0956ff2ae0 100644
--- a/drivers/pinctrl/qcom/pinctrl-lpass-lpi.c
+++ b/drivers/pinctrl/qcom/pinctrl-lpass-lpi.c
@@ -15,6 +15,8 @@
#include <linux/pinctrl/pinconf-generic.h>
#include <linux/pinctrl/pinconf.h>
#include <linux/pinctrl/pinmux.h>
+#include <linux/pm_clock.h>
+#include <linux/pm_runtime.h>
#include "../pinctrl-utils.h"
@@ -22,7 +24,6 @@
#define MAX_NR_GPIO 32
#define GPIO_FUNC 0
-#define MAX_LPI_NUM_CLKS 2
struct lpi_pinctrl {
struct device *dev;
@@ -31,7 +32,6 @@ struct lpi_pinctrl {
struct pinctrl_desc desc;
char __iomem *tlmm_base;
char __iomem *slew_base;
- struct clk_bulk_data clks[MAX_LPI_NUM_CLKS];
/* Protects from concurrent register updates */
struct mutex lock;
DECLARE_BITMAP(ever_gpio, MAX_NR_GPIO);
@@ -480,9 +480,6 @@ int lpi_pinctrl_probe(struct platform_device *pdev)
pctrl->data = data;
pctrl->dev = &pdev->dev;
- pctrl->clks[0].id = "core";
- pctrl->clks[1].id = "audio";
-
pctrl->tlmm_base = devm_platform_ioremap_resource(pdev, 0);
if (IS_ERR(pctrl->tlmm_base))
return dev_err_probe(dev, PTR_ERR(pctrl->tlmm_base),
@@ -495,13 +492,17 @@ int lpi_pinctrl_probe(struct platform_device *pdev)
"Slew resource not provided\n");
}
- ret = devm_clk_bulk_get_optional(dev, MAX_LPI_NUM_CLKS, pctrl->clks);
+ ret = devm_pm_clk_create(dev);
if (ret)
return ret;
- ret = clk_bulk_prepare_enable(MAX_LPI_NUM_CLKS, pctrl->clks);
- if (ret)
- return dev_err_probe(dev, ret, "Can't enable clocks\n");
+ ret = of_pm_clk_add_clks(dev);
+ if (ret < 0)
+ return ret;
+
+ pm_runtime_set_autosuspend_delay(dev, 100);
+ pm_runtime_use_autosuspend(dev);
+ devm_pm_runtime_enable(dev);
pctrl->desc.pctlops = &lpi_gpio_pinctrl_ops;
pctrl->desc.pmxops = &lpi_gpio_pinmux_ops;
@@ -539,8 +540,8 @@ int lpi_pinctrl_probe(struct platform_device *pdev)
return 0;
err_pinctrl:
+ pm_runtime_disable(dev);
mutex_destroy(&pctrl->lock);
- clk_bulk_disable_unprepare(MAX_LPI_NUM_CLKS, pctrl->clks);
return ret;
}
@@ -552,7 +553,6 @@ void lpi_pinctrl_remove(struct platform_device *pdev)
int i;
mutex_destroy(&pctrl->lock);
- clk_bulk_disable_unprepare(MAX_LPI_NUM_CLKS, pctrl->clks);
for (i = 0; i < pctrl->data->npins; i++)
pinctrl_generic_remove_group(pctrl->ctrl, i);
diff --git a/drivers/pinctrl/qcom/pinctrl-sc7280-lpass-lpi.c b/drivers/pinctrl/qcom/pinctrl-sc7280-lpass-lpi.c
index 750f410311a8..64a200dd8f41 100644
--- a/drivers/pinctrl/qcom/pinctrl-sc7280-lpass-lpi.c
+++ b/drivers/pinctrl/qcom/pinctrl-sc7280-lpass-lpi.c
@@ -7,6 +7,8 @@
#include <linux/gpio/driver.h>
#include <linux/module.h>
#include <linux/platform_device.h>
+#include <linux/pm_clock.h>
+#include <linux/pm_runtime.h>
#include "pinctrl-lpass-lpi.h"
@@ -139,10 +141,15 @@ static const struct of_device_id lpi_pinctrl_of_match[] = {
};
MODULE_DEVICE_TABLE(of, lpi_pinctrl_of_match);
+static const struct dev_pm_ops lpi_pinctrl_pm_ops = {
+ RUNTIME_PM_OPS(pm_clk_suspend, pm_clk_resume, NULL)
+};
+
static struct platform_driver lpi_pinctrl_driver = {
.driver = {
.name = "qcom-sc7280-lpass-lpi-pinctrl",
.of_match_table = lpi_pinctrl_of_match,
+ .pm = pm_ptr(&lpi_pinctrl_pm_ops),
},
.probe = lpi_pinctrl_probe,
.remove = lpi_pinctrl_remove,
--
2.34.1
^ permalink raw reply related
* [PATCH v2 2/3] pinctrl: qcom: lpass-lpi: Enable runtime PM hooks on remaining SoCs
From: Ajay Kumar Nandam @ 2026-04-20 12:31 UTC (permalink / raw)
To: Bjorn Andersson, Linus Walleij
Cc: linux-arm-msm, linux-gpio, linux-kernel, mohammad.rafi.shaik,
ajay.nandam
In-Reply-To: <20260420123135.350446-1-ajay.nandam@oss.qualcomm.com>
The LPASS LPI core was switched to the PM clock framework and runtime PM,
but only the sc7280 variant driver wired runtime PM callbacks.
Hook up runtime PM callbacks for the remaining LPASS LPI variant
drivers so all SoCs using the common core get consistent pm_clk based
clock handling:
- sc8280xp
- sm4250
- sm6115
- sm8250
- sm8450
- sm8550
- sm8650
This is a mechanical per-variant driver update that relies on the
same generic PM clock flow (of_pm_clk_add_clks() + pm_clk_suspend/
pm_clk_resume()) and DT-provided clocks.
Runtime behavior was validated on Kodiak (sc7280).
Signed-off-by: Ajay Kumar Nandam <ajay.nandam@oss.qualcomm.com>
---
drivers/pinctrl/qcom/pinctrl-sc8280xp-lpass-lpi.c | 11 +++++++++--
drivers/pinctrl/qcom/pinctrl-sm4250-lpass-lpi.c | 7 +++++++
drivers/pinctrl/qcom/pinctrl-sm6115-lpass-lpi.c | 7 +++++++
drivers/pinctrl/qcom/pinctrl-sm8250-lpass-lpi.c | 11 +++++++++--
drivers/pinctrl/qcom/pinctrl-sm8450-lpass-lpi.c | 11 +++++++++--
drivers/pinctrl/qcom/pinctrl-sm8550-lpass-lpi.c | 11 +++++++++--
drivers/pinctrl/qcom/pinctrl-sm8650-lpass-lpi.c | 11 +++++++++--
7 files changed, 59 insertions(+), 10 deletions(-)
diff --git a/drivers/pinctrl/qcom/pinctrl-sc8280xp-lpass-lpi.c b/drivers/pinctrl/qcom/pinctrl-sc8280xp-lpass-lpi.c
index 0e839b6aaaf4..1a61316c8c47 100644
--- a/drivers/pinctrl/qcom/pinctrl-sc8280xp-lpass-lpi.c
+++ b/drivers/pinctrl/qcom/pinctrl-sc8280xp-lpass-lpi.c
@@ -6,6 +6,8 @@
#include <linux/gpio/driver.h>
#include <linux/module.h>
#include <linux/platform_device.h>
+#include <linux/pm_clock.h>
+#include <linux/pm_runtime.h>
#include "pinctrl-lpass-lpi.h"
@@ -173,10 +175,15 @@ static const struct of_device_id lpi_pinctrl_of_match[] = {
};
MODULE_DEVICE_TABLE(of, lpi_pinctrl_of_match);
+static const struct dev_pm_ops lpi_pinctrl_pm_ops = {
+ RUNTIME_PM_OPS(pm_clk_suspend, pm_clk_resume, NULL)
+};
+
static struct platform_driver lpi_pinctrl_driver = {
.driver = {
- .name = "qcom-sc8280xp-lpass-lpi-pinctrl",
- .of_match_table = lpi_pinctrl_of_match,
+ .name = "qcom-sc8280xp-lpass-lpi-pinctrl",
+ .of_match_table = lpi_pinctrl_of_match,
+ .pm = pm_ptr(&lpi_pinctrl_pm_ops),
},
.probe = lpi_pinctrl_probe,
.remove = lpi_pinctrl_remove,
diff --git a/drivers/pinctrl/qcom/pinctrl-sm4250-lpass-lpi.c b/drivers/pinctrl/qcom/pinctrl-sm4250-lpass-lpi.c
index c0e178be9cfc..75bafa62426a 100644
--- a/drivers/pinctrl/qcom/pinctrl-sm4250-lpass-lpi.c
+++ b/drivers/pinctrl/qcom/pinctrl-sm4250-lpass-lpi.c
@@ -7,6 +7,8 @@
#include <linux/gpio/driver.h>
#include <linux/module.h>
#include <linux/platform_device.h>
+#include <linux/pm_clock.h>
+#include <linux/pm_runtime.h>
#include "pinctrl-lpass-lpi.h"
@@ -221,10 +223,15 @@ static const struct of_device_id lpi_pinctrl_of_match[] = {
};
MODULE_DEVICE_TABLE(of, lpi_pinctrl_of_match);
+static const struct dev_pm_ops lpi_pinctrl_pm_ops = {
+ RUNTIME_PM_OPS(pm_clk_suspend, pm_clk_resume, NULL)
+};
+
static struct platform_driver lpi_pinctrl_driver = {
.driver = {
.name = "qcom-sm4250-lpass-lpi-pinctrl",
.of_match_table = lpi_pinctrl_of_match,
+ .pm = pm_ptr(&lpi_pinctrl_pm_ops),
},
.probe = lpi_pinctrl_probe,
.remove = lpi_pinctrl_remove,
diff --git a/drivers/pinctrl/qcom/pinctrl-sm6115-lpass-lpi.c b/drivers/pinctrl/qcom/pinctrl-sm6115-lpass-lpi.c
index b7d9186861a2..05435ea6e17a 100644
--- a/drivers/pinctrl/qcom/pinctrl-sm6115-lpass-lpi.c
+++ b/drivers/pinctrl/qcom/pinctrl-sm6115-lpass-lpi.c
@@ -7,6 +7,8 @@
#include <linux/gpio/driver.h>
#include <linux/module.h>
#include <linux/platform_device.h>
+#include <linux/pm_clock.h>
+#include <linux/pm_runtime.h>
#include "pinctrl-lpass-lpi.h"
@@ -141,10 +143,15 @@ static const struct of_device_id lpi_pinctrl_of_match[] = {
};
MODULE_DEVICE_TABLE(of, lpi_pinctrl_of_match);
+static const struct dev_pm_ops lpi_pinctrl_pm_ops = {
+ RUNTIME_PM_OPS(pm_clk_suspend, pm_clk_resume, NULL)
+};
+
static struct platform_driver lpi_pinctrl_driver = {
.driver = {
.name = "qcom-sm6115-lpass-lpi-pinctrl",
.of_match_table = lpi_pinctrl_of_match,
+ .pm = pm_ptr(&lpi_pinctrl_pm_ops),
},
.probe = lpi_pinctrl_probe,
.remove = lpi_pinctrl_remove,
diff --git a/drivers/pinctrl/qcom/pinctrl-sm8250-lpass-lpi.c b/drivers/pinctrl/qcom/pinctrl-sm8250-lpass-lpi.c
index c27452eece3e..656f22da7dde 100644
--- a/drivers/pinctrl/qcom/pinctrl-sm8250-lpass-lpi.c
+++ b/drivers/pinctrl/qcom/pinctrl-sm8250-lpass-lpi.c
@@ -7,6 +7,8 @@
#include <linux/gpio/driver.h>
#include <linux/module.h>
#include <linux/platform_device.h>
+#include <linux/pm_clock.h>
+#include <linux/pm_runtime.h>
#include "pinctrl-lpass-lpi.h"
@@ -134,10 +136,15 @@ static const struct of_device_id lpi_pinctrl_of_match[] = {
};
MODULE_DEVICE_TABLE(of, lpi_pinctrl_of_match);
+static const struct dev_pm_ops lpi_pinctrl_pm_ops = {
+ RUNTIME_PM_OPS(pm_clk_suspend, pm_clk_resume, NULL)
+};
+
static struct platform_driver lpi_pinctrl_driver = {
.driver = {
- .name = "qcom-sm8250-lpass-lpi-pinctrl",
- .of_match_table = lpi_pinctrl_of_match,
+ .name = "qcom-sm8250-lpass-lpi-pinctrl",
+ .of_match_table = lpi_pinctrl_of_match,
+ .pm = pm_ptr(&lpi_pinctrl_pm_ops),
},
.probe = lpi_pinctrl_probe,
.remove = lpi_pinctrl_remove,
diff --git a/drivers/pinctrl/qcom/pinctrl-sm8450-lpass-lpi.c b/drivers/pinctrl/qcom/pinctrl-sm8450-lpass-lpi.c
index 439f6541622e..a79f99ec6df9 100644
--- a/drivers/pinctrl/qcom/pinctrl-sm8450-lpass-lpi.c
+++ b/drivers/pinctrl/qcom/pinctrl-sm8450-lpass-lpi.c
@@ -6,6 +6,8 @@
#include <linux/gpio/driver.h>
#include <linux/module.h>
#include <linux/platform_device.h>
+#include <linux/pm_clock.h>
+#include <linux/pm_runtime.h>
#include "pinctrl-lpass-lpi.h"
@@ -202,10 +204,15 @@ static const struct of_device_id lpi_pinctrl_of_match[] = {
};
MODULE_DEVICE_TABLE(of, lpi_pinctrl_of_match);
+static const struct dev_pm_ops lpi_pinctrl_pm_ops = {
+ RUNTIME_PM_OPS(pm_clk_suspend, pm_clk_resume, NULL)
+};
+
static struct platform_driver lpi_pinctrl_driver = {
.driver = {
- .name = "qcom-sm8450-lpass-lpi-pinctrl",
- .of_match_table = lpi_pinctrl_of_match,
+ .name = "qcom-sm8450-lpass-lpi-pinctrl",
+ .of_match_table = lpi_pinctrl_of_match,
+ .pm = pm_ptr(&lpi_pinctrl_pm_ops),
},
.probe = lpi_pinctrl_probe,
.remove = lpi_pinctrl_remove,
diff --git a/drivers/pinctrl/qcom/pinctrl-sm8550-lpass-lpi.c b/drivers/pinctrl/qcom/pinctrl-sm8550-lpass-lpi.c
index 73065919c8c2..9037ef0020da 100644
--- a/drivers/pinctrl/qcom/pinctrl-sm8550-lpass-lpi.c
+++ b/drivers/pinctrl/qcom/pinctrl-sm8550-lpass-lpi.c
@@ -6,6 +6,8 @@
#include <linux/gpio/driver.h>
#include <linux/module.h>
#include <linux/platform_device.h>
+#include <linux/pm_clock.h>
+#include <linux/pm_runtime.h>
#include "pinctrl-lpass-lpi.h"
@@ -210,10 +212,15 @@ static const struct of_device_id lpi_pinctrl_of_match[] = {
};
MODULE_DEVICE_TABLE(of, lpi_pinctrl_of_match);
+static const struct dev_pm_ops lpi_pinctrl_pm_ops = {
+ RUNTIME_PM_OPS(pm_clk_suspend, pm_clk_resume, NULL)
+};
+
static struct platform_driver lpi_pinctrl_driver = {
.driver = {
- .name = "qcom-sm8550-lpass-lpi-pinctrl",
- .of_match_table = lpi_pinctrl_of_match,
+ .name = "qcom-sm8550-lpass-lpi-pinctrl",
+ .of_match_table = lpi_pinctrl_of_match,
+ .pm = pm_ptr(&lpi_pinctrl_pm_ops),
},
.probe = lpi_pinctrl_probe,
.remove = lpi_pinctrl_remove,
diff --git a/drivers/pinctrl/qcom/pinctrl-sm8650-lpass-lpi.c b/drivers/pinctrl/qcom/pinctrl-sm8650-lpass-lpi.c
index f9fcedf5a65d..513ddc99dd37 100644
--- a/drivers/pinctrl/qcom/pinctrl-sm8650-lpass-lpi.c
+++ b/drivers/pinctrl/qcom/pinctrl-sm8650-lpass-lpi.c
@@ -6,6 +6,8 @@
#include <linux/gpio/driver.h>
#include <linux/module.h>
#include <linux/platform_device.h>
+#include <linux/pm_clock.h>
+#include <linux/pm_runtime.h>
#include "pinctrl-lpass-lpi.h"
@@ -217,10 +219,15 @@ static const struct of_device_id lpi_pinctrl_of_match[] = {
};
MODULE_DEVICE_TABLE(of, lpi_pinctrl_of_match);
+static const struct dev_pm_ops lpi_pinctrl_pm_ops = {
+ RUNTIME_PM_OPS(pm_clk_suspend, pm_clk_resume, NULL)
+};
+
static struct platform_driver lpi_pinctrl_driver = {
.driver = {
- .name = "qcom-sm8650-lpass-lpi-pinctrl",
- .of_match_table = lpi_pinctrl_of_match,
+ .name = "qcom-sm8650-lpass-lpi-pinctrl",
+ .of_match_table = lpi_pinctrl_of_match,
+ .pm = pm_ptr(&lpi_pinctrl_pm_ops),
},
.probe = lpi_pinctrl_probe,
.remove = lpi_pinctrl_remove,
--
2.34.1
^ permalink raw reply related
* [PATCH v2 3/3] pinctrl: qcom: lpass-lpi: Resume clocks for GPIO access
From: Ajay Kumar Nandam @ 2026-04-20 12:31 UTC (permalink / raw)
To: Bjorn Andersson, Linus Walleij
Cc: linux-arm-msm, linux-gpio, linux-kernel, mohammad.rafi.shaik,
ajay.nandam
In-Reply-To: <20260420123135.350446-1-ajay.nandam@oss.qualcomm.com>
Ensure the LPI pinctrl device clocks are runtime resumed
before accessing GPIO registers and autosuspended after
the access completes.
Guard GPIO register read and write helpers with synchronous
runtime PM calls so the device is active during MMIO
operations.
Signed-off-by: Ajay Kumar Nandam <ajay.nandam@oss.qualcomm.com>
---
drivers/pinctrl/qcom/pinctrl-lpass-lpi.c | 26 ++++++++++++++++++------
1 file changed, 20 insertions(+), 6 deletions(-)
diff --git a/drivers/pinctrl/qcom/pinctrl-lpass-lpi.c b/drivers/pinctrl/qcom/pinctrl-lpass-lpi.c
index 2b0956ff2ae0..75ae21478fa4 100644
--- a/drivers/pinctrl/qcom/pinctrl-lpass-lpi.c
+++ b/drivers/pinctrl/qcom/pinctrl-lpass-lpi.c
@@ -38,32 +38,46 @@ struct lpi_pinctrl {
const struct lpi_pinctrl_variant_data *data;
};
-static int lpi_gpio_read(struct lpi_pinctrl *state, unsigned int pin,
+static u32 lpi_gpio_read(struct lpi_pinctrl *state, unsigned int pin,
unsigned int addr)
{
- u32 pin_offset;
+ u32 pin_offset, val;
+ int ret;
if (state->data->flags & LPI_FLAG_USE_PREDEFINED_PIN_OFFSET)
pin_offset = state->data->groups[pin].pin_offset;
else
pin_offset = LPI_TLMM_REG_OFFSET * pin;
- return ioread32(state->tlmm_base + pin_offset + addr);
+ ret = pm_runtime_resume_and_get(state->dev);
+ if (ret < 0)
+ return 0;
+
+ val = ioread32(state->tlmm_base + pin_offset + addr);
+ pm_runtime_put_autosuspend(state->dev);
+
+ return val;
}
-static int lpi_gpio_write(struct lpi_pinctrl *state, unsigned int pin,
- unsigned int addr, unsigned int val)
+static void lpi_gpio_write(struct lpi_pinctrl *state, unsigned int pin,
+ unsigned int addr, unsigned int val)
{
u32 pin_offset;
+ int ret;
if (state->data->flags & LPI_FLAG_USE_PREDEFINED_PIN_OFFSET)
pin_offset = state->data->groups[pin].pin_offset;
else
pin_offset = LPI_TLMM_REG_OFFSET * pin;
+ ret = pm_runtime_resume_and_get(state->dev);
+ if (ret < 0)
+ return;
+
iowrite32(val, state->tlmm_base + pin_offset + addr);
+ pm_runtime_put_autosuspend(state->dev);
- return 0;
+ return;
}
static const struct pinctrl_ops lpi_gpio_pinctrl_ops = {
--
2.34.1
^ permalink raw reply related
* Re: [PATCH v3 1/2] phy: qcom: edp: Add eDP/DP mode switch support
From: Yongxing Mou @ 2026-04-20 12:47 UTC (permalink / raw)
To: Dmitry Baryshkov
Cc: Vinod Koul, Neil Armstrong, Stephen Boyd, Bjorn Andersson,
linux-arm-msm, linux-phy, linux-kernel, stable
In-Reply-To: <islxoe4wbqx5pl54difetdcl5lrqvfd5ysbaicxz5lv235sfmd@6hwrq3rmqx7c>
On 3/20/2026 2:36 PM, Dmitry Baryshkov wrote:
> On Mon, Mar 02, 2026 at 04:28:29PM +0800, Yongxing Mou wrote:
>> The eDP PHY supports both eDP&DP modes, each requires a different table.
>> The current driver doesn't fully support every combo PHY mode and use
>> either the eDP or DP table when enable the platform. In addition, some
>> platforms mismatch between the mode and the table where DP mode uses
>> the eDP table or eDP mode use the DP table.
>>
>> Clean up and correct the tables for currently supported platforms based on
>> the HPG specification.
>>
>> Here lists the tables can be reused across current platforms.
>> DP mode：
>> -sa8775p/sc7280/sc8280xp/x1e80100
>> -glymur
>> eDP mode(low vdiff):
>
> Separate question: should we extend phy_configure_dp_opts with the
> low/high vdiff? Is there a point in providing the ability to toggle
> between low vdiff and high vdiff?
>
Emm ,i haven't found any platform using high vdiff so far, and I'm not
clear in which cases switching between low and high vdiff would be needed.
>> -glymur/sa8775p/sc8280xp/x1e80100
>> -sc7280
>
> I understand your wish to perform all the changes in a single patch, but
> there is one problem with that. Consider this patch regresses one of the
> platforms (I'm looking at Kodiak and SC8180X as they get the biggest set
> of changes). It would be almost impossible to separate, which particular
> change caused the regression. I'd suggest splitting this patch into a
> set of more atomic changes. E.g. the AUX_CFG8 is definitely a separate
> change. Writing swing / pre_emph tables on Kodiak and SC8180X is a
> separate change (or two). Switching each of the platforms to the
> corrected set of tables ideally also should come as a separate change,
> so that in case of a regression the issue would be easier to identify.
>
Thank for point this, will separate the change.
I mostly overlooked SC8180X here, since I assumed it shares the same PHY
as SC7280. However, they are using different PHY sub‑versions. Will add
proper support for it in the next version.
>>
>> Cc: stable@vger.kernel.org
>> Fixes: f199223cb490 ("phy: qcom: Introduce new eDP PHY driver")
>> Signed-off-by: Yongxing Mou <yongxing.mou@oss.qualcomm.com>
>> ---
>> drivers/phy/qualcomm/phy-qcom-edp.c | 90 ++++++++++++++++++++++---------------
>> 1 file changed, 53 insertions(+), 37 deletions(-)
>>
>> diff --git a/drivers/phy/qualcomm/phy-qcom-edp.c b/drivers/phy/qualcomm/phy-qcom-edp.c
>> index 7372de05a0b8..36998326bae6 100644
>> --- a/drivers/phy/qualcomm/phy-qcom-edp.c
>> +++ b/drivers/phy/qualcomm/phy-qcom-edp.c
>> @@ -87,7 +87,8 @@ struct qcom_edp_phy_cfg {
>> bool is_edp;
>> const u8 *aux_cfg;
>> const u8 *vco_div_cfg;
>> - const struct qcom_edp_swing_pre_emph_cfg *swing_pre_emph_cfg;
>> + const struct qcom_edp_swing_pre_emph_cfg *dp_swing_pre_emph_cfg;
>> + const struct qcom_edp_swing_pre_emph_cfg *edp_swing_pre_emph_cfg;
>> const struct phy_ver_ops *ver_ops;
>> };
>>
>> @@ -116,17 +117,17 @@ struct qcom_edp {
>> };
>>
>> static const u8 dp_swing_hbr_rbr[4][4] = {
>> - { 0x08, 0x0f, 0x16, 0x1f },
>> + { 0x07, 0x0f, 0x16, 0x1f },
>> { 0x11, 0x1e, 0x1f, 0xff },
>> { 0x16, 0x1f, 0xff, 0xff },
>> { 0x1f, 0xff, 0xff, 0xff }
>> };
>>
>> static const u8 dp_pre_emp_hbr_rbr[4][4] = {
>> - { 0x00, 0x0d, 0x14, 0x1a },
>> + { 0x00, 0x0e, 0x15, 0x1a },
>> { 0x00, 0x0e, 0x15, 0xff },
>> { 0x00, 0x0e, 0xff, 0xff },
>> - { 0x03, 0xff, 0xff, 0xff }
>> + { 0x04, 0xff, 0xff, 0xff }
>> };
>
> I've checked, at least this table doesn't match SC8180X configuration.
>
Got it.
>>
>> static const u8 dp_swing_hbr2_hbr3[4][4] = {
>> @@ -150,6 +151,20 @@ static const struct qcom_edp_swing_pre_emph_cfg dp_phy_swing_pre_emph_cfg = {
>> .pre_emphasis_hbr3_hbr2 = &dp_pre_emp_hbr2_hbr3,
>> };
>>
>> +static const u8 dp_pre_emp_hbr_rbr_v8[4][4] = {
>> + { 0x00, 0x0e, 0x15, 0x1a },
>> + { 0x00, 0x0e, 0x15, 0xff },
>> + { 0x00, 0x0e, 0xff, 0xff },
>> + { 0x00, 0xff, 0xff, 0xff }
>> +};
>> +
>> +static const struct qcom_edp_swing_pre_emph_cfg dp_phy_swing_pre_emph_cfg_v8 = {
>> + .swing_hbr_rbr = &dp_swing_hbr_rbr,
>> + .swing_hbr3_hbr2 = &dp_swing_hbr2_hbr3,
>> + .pre_emphasis_hbr_rbr = &dp_pre_emp_hbr_rbr_v8,
>> + .pre_emphasis_hbr3_hbr2 = &dp_pre_emp_hbr2_hbr3,
>> +};
>> +
>> static const u8 edp_swing_hbr_rbr[4][4] = {
>> { 0x07, 0x0f, 0x16, 0x1f },
>> { 0x0d, 0x16, 0x1e, 0xff },
>> @@ -158,7 +173,7 @@ static const u8 edp_swing_hbr_rbr[4][4] = {
>> };
>>
>> static const u8 edp_pre_emp_hbr_rbr[4][4] = {
>> - { 0x05, 0x12, 0x17, 0x1d },
>> + { 0x05, 0x11, 0x17, 0x1d },
>
> This was changed only for Kodiak. For SC8180X, I assume, we should be
> using the older table.
>
Emm, for SC8180X, eDP low VDIFF (High HBR) only S1 (250mV) P0-0dB
Emphasis Settings "0x08" different from other "generic" tables which is
"0x0B".
>> { 0x05, 0x11, 0x18, 0xff },
>> { 0x06, 0x11, 0xff, 0xff },
>> { 0x00, 0xff, 0xff, 0xff }
>> @@ -172,10 +187,10 @@ static const u8 edp_swing_hbr2_hbr3[4][4] = {
>> };
>>
>> static const u8 edp_pre_emp_hbr2_hbr3[4][4] = {
>
> I think it becomes worth adding version to the "generic" tables. They
> are not that generic anyway.
>
Got it. SC8180X here need a different table..
>> - { 0x08, 0x11, 0x17, 0x1b },
>> - { 0x00, 0x0c, 0x13, 0xff },
>> - { 0x05, 0x10, 0xff, 0xff },
>> - { 0x00, 0xff, 0xff, 0xff }
>> + { 0x0c, 0x15, 0x19, 0x1e },
>> + { 0x0b, 0x15, 0x19, 0xff },
>> + { 0x0e, 0x14, 0xff, 0xff },
>> + { 0x0d, 0xff, 0xff, 0xff }
>
> Current table indeed doesn't match the swing table. Please take care
> about the SC8180X differences (I think, it will need separate set of
> tables).
>
Got it.
>> };
>>
>> static const struct qcom_edp_swing_pre_emph_cfg edp_phy_swing_pre_emph_cfg = {
>> @@ -193,25 +208,25 @@ static const u8 edp_phy_vco_div_cfg_v4[4] = {
>> 0x01, 0x01, 0x02, 0x00,
>> };
>>
>> -static const u8 edp_pre_emp_hbr_rbr_v5[4][4] = {
>> - { 0x05, 0x11, 0x17, 0x1d },
>> - { 0x05, 0x11, 0x18, 0xff },
>> - { 0x06, 0x11, 0xff, 0xff },
>> - { 0x00, 0xff, 0xff, 0xff }
>> +static const u8 edp_swing_hbr2_hbr3_v3[4][4] = {
>> + { 0x06, 0x11, 0x16, 0x1b },
>> + { 0x0b, 0x19, 0x1f, 0xff },
>> + { 0x18, 0x1f, 0xff, 0xff },
>> + { 0x1f, 0xff, 0xff, 0xff }
>> };
>>
>> -static const u8 edp_pre_emp_hbr2_hbr3_v5[4][4] = {
>> +static const u8 edp_pre_emp_hbr2_hbr3_v3[4][4] = {
>> { 0x0c, 0x15, 0x19, 0x1e },
>> - { 0x0b, 0x15, 0x19, 0xff },
>> - { 0x0e, 0x14, 0xff, 0xff },
>> + { 0x09, 0x14, 0x19, 0xff },
>> + { 0x0f, 0x14, 0xff, 0xff },
>> { 0x0d, 0xff, 0xff, 0xff }
>> };
>>
>> -static const struct qcom_edp_swing_pre_emph_cfg edp_phy_swing_pre_emph_cfg_v5 = {
>> +static const struct qcom_edp_swing_pre_emph_cfg edp_phy_swing_pre_emph_cfg_v3 = {
>> .swing_hbr_rbr = &edp_swing_hbr_rbr,
>> - .swing_hbr3_hbr2 = &edp_swing_hbr2_hbr3,
>> - .pre_emphasis_hbr_rbr = &edp_pre_emp_hbr_rbr_v5,
>> - .pre_emphasis_hbr3_hbr2 = &edp_pre_emp_hbr2_hbr3_v5,
>> + .swing_hbr3_hbr2 = &edp_swing_hbr2_hbr3_v3,
>> + .pre_emphasis_hbr_rbr = &edp_pre_emp_hbr_rbr,
>> + .pre_emphasis_hbr3_hbr2 = &edp_pre_emp_hbr2_hbr3_v3,
>> };
>>
>> static const u8 edp_phy_aux_cfg_v5[DP_AUX_CFG_SIZE] = {
>> @@ -262,12 +277,7 @@ static int qcom_edp_phy_init(struct phy *phy)
>> DP_PHY_PD_CTL_PLL_PWRDN | DP_PHY_PD_CTL_DP_CLAMP_EN,
>> edp->edp + DP_PHY_PD_CTL);
>>
>> - /*
>> - * TODO: Re-work the conditions around setting the cfg8 value
>> - * when more information becomes available about why this is
>> - * even needed.
>> - */
>> - if (edp->cfg->swing_pre_emph_cfg && !edp->is_edp)
>> + if (!edp->is_edp)
>> aux_cfg[8] = 0xb7;
>
> This is a separate fix, as it changes the aux_cfg[8] value for Kodiak
> and SC8180X.
>
Got it.
>>
>> writel(0xfc, edp->edp + DP_PHY_MODE);
>> @@ -291,7 +301,7 @@ static int qcom_edp_phy_init(struct phy *phy)
>>
>> static int qcom_edp_set_voltages(struct qcom_edp *edp, const struct phy_configure_opts_dp *dp_opts)
>> {
>> - const struct qcom_edp_swing_pre_emph_cfg *cfg = edp->cfg->swing_pre_emph_cfg;
>> + const struct qcom_edp_swing_pre_emph_cfg *cfg;
>> unsigned int v_level = 0;
>> unsigned int p_level = 0;
>> u8 ldo_config;
>> @@ -299,11 +309,10 @@ static int qcom_edp_set_voltages(struct qcom_edp *edp, const struct phy_configur
>> u8 emph;
>> int i;
>>
>> - if (!cfg)
>> - return 0;
>> -
>> if (edp->is_edp)
>> - cfg = &edp_phy_swing_pre_emph_cfg;
>> + cfg = edp->cfg->edp_swing_pre_emph_cfg;
>> + else
>> + cfg = edp->cfg->dp_swing_pre_emph_cfg;
>>
>> for (i = 0; i < dp_opts->lanes; i++) {
>> v_level = max(v_level, dp_opts->voltage[i]);
>> @@ -564,20 +573,24 @@ static const struct qcom_edp_phy_cfg sa8775p_dp_phy_cfg = {
>> .is_edp = false,
>> .aux_cfg = edp_phy_aux_cfg_v5,
>> .vco_div_cfg = edp_phy_vco_div_cfg_v4,
>> - .swing_pre_emph_cfg = &edp_phy_swing_pre_emph_cfg_v5,
>> + .dp_swing_pre_emph_cfg = &dp_phy_swing_pre_emph_cfg,
>> + .edp_swing_pre_emph_cfg = &edp_phy_swing_pre_emph_cfg,
>> .ver_ops = &qcom_edp_phy_ops_v4,
>> };
>>
>> static const struct qcom_edp_phy_cfg sc7280_dp_phy_cfg = {
>> .aux_cfg = edp_phy_aux_cfg_v4,
>> .vco_div_cfg = edp_phy_vco_div_cfg_v4,
>> + .dp_swing_pre_emph_cfg = &dp_phy_swing_pre_emph_cfg,
>> + .edp_swing_pre_emph_cfg = &edp_phy_swing_pre_emph_cfg_v3,
>> .ver_ops = &qcom_edp_phy_ops_v4,
>> };
>>
>> static const struct qcom_edp_phy_cfg sc8280xp_dp_phy_cfg = {
>> .aux_cfg = edp_phy_aux_cfg_v4,
>> .vco_div_cfg = edp_phy_vco_div_cfg_v4,
>> - .swing_pre_emph_cfg = &dp_phy_swing_pre_emph_cfg,
>> + .dp_swing_pre_emph_cfg = &dp_phy_swing_pre_emph_cfg,
>> + .edp_swing_pre_emph_cfg = &edp_phy_swing_pre_emph_cfg,
>> .ver_ops = &qcom_edp_phy_ops_v4,
>> };
>>
>> @@ -585,7 +598,8 @@ static const struct qcom_edp_phy_cfg sc8280xp_edp_phy_cfg = {
>> .is_edp = true,
>> .aux_cfg = edp_phy_aux_cfg_v4,
>> .vco_div_cfg = edp_phy_vco_div_cfg_v4,
>> - .swing_pre_emph_cfg = &edp_phy_swing_pre_emph_cfg,
>> + .dp_swing_pre_emph_cfg = &dp_phy_swing_pre_emph_cfg,
>> + .edp_swing_pre_emph_cfg = &edp_phy_swing_pre_emph_cfg,
>
> Ok, we are going to continue using eDP table because of is_edp = true.
>
>> .ver_ops = &qcom_edp_phy_ops_v4,
>> };
>>
>> @@ -766,7 +780,8 @@ static const struct phy_ver_ops qcom_edp_phy_ops_v6 = {
>> static struct qcom_edp_phy_cfg x1e80100_phy_cfg = {
>> .aux_cfg = edp_phy_aux_cfg_v4,
>> .vco_div_cfg = edp_phy_vco_div_cfg_v4,
>> - .swing_pre_emph_cfg = &dp_phy_swing_pre_emph_cfg,
>> + .dp_swing_pre_emph_cfg = &dp_phy_swing_pre_emph_cfg,
>> + .edp_swing_pre_emph_cfg = &edp_phy_swing_pre_emph_cfg,
>> .ver_ops = &qcom_edp_phy_ops_v6,
>> };
>>
>> @@ -945,7 +960,8 @@ static const struct phy_ver_ops qcom_edp_phy_ops_v8 = {
>> static struct qcom_edp_phy_cfg glymur_phy_cfg = {
>> .aux_cfg = edp_phy_aux_cfg_v8,
>> .vco_div_cfg = edp_phy_vco_div_cfg_v8,
>> - .swing_pre_emph_cfg = &edp_phy_swing_pre_emph_cfg_v5,
>> + .dp_swing_pre_emph_cfg = &dp_phy_swing_pre_emph_cfg_v8,
>> + .edp_swing_pre_emph_cfg = &edp_phy_swing_pre_emph_cfg,
>> .ver_ops = &qcom_edp_phy_ops_v8,
>> };
>>
>>
>> --
>> 2.43.0
>>
>>
>> --
>> linux-phy mailing list
>> linux-phy@lists.infradead.org
>> https://lists.infradead.org/mailman/listinfo/linux-phy
>
^ permalink raw reply
* Re: [PATCH v3 1/2] phy: qcom: edp: Add eDP/DP mode switch support
From: Yongxing Mou @ 2026-04-20 12:48 UTC (permalink / raw)
To: Konrad Dybcio, Vinod Koul, Neil Armstrong, Stephen Boyd,
Bjorn Andersson
Cc: linux-arm-msm, linux-phy, linux-kernel, stable
In-Reply-To: <245f4589-d7ca-4d6b-8162-a86972752bd8@oss.qualcomm.com>
On 4/16/2026 5:34 PM, Konrad Dybcio wrote:
> On 3/2/26 9:28 AM, Yongxing Mou wrote:
>> The eDP PHY supports both eDP&DP modes, each requires a different table.
>> The current driver doesn't fully support every combo PHY mode and use
>> either the eDP or DP table when enable the platform. In addition, some
>> platforms mismatch between the mode and the table where DP mode uses
>> the eDP table or eDP mode use the DP table.
>>
>> Clean up and correct the tables for currently supported platforms based on
>> the HPG specification.
>>
>> Here lists the tables can be reused across current platforms.
>> DP mode：
>> -sa8775p/sc7280/sc8280xp/x1e80100
>> -glymur
>> eDP mode(low vdiff):
>> -glymur/sa8775p/sc8280xp/x1e80100
>> -sc7280
>>
>> Cc: stable@vger.kernel.org
>> Fixes: f199223cb490 ("phy: qcom: Introduce new eDP PHY driver")
>> Signed-off-by: Yongxing Mou <yongxing.mou@oss.qualcomm.com>
>> ---
>
> I went through everything and all the sequences are OK.
>
> SC8180X will need changes, but it's already incorrect so this
> doesn't necessarily affect it
>
> Thanks!
>
> Reviewed-by: Konrad Dybcio <konrad.dybcio@oss.qualcomm.com>
>
> Konrad
Thanks here.. I didn’t notice before that SC8180X is different from
SC7280, and will correct it in the next version.
^ permalink raw reply
* Re: [PATCH v3 1/2] phy: qcom: edp: Add eDP/DP mode switch support
From: Konrad Dybcio @ 2026-04-20 12:49 UTC (permalink / raw)
To: Yongxing Mou, Vinod Koul, Neil Armstrong, Stephen Boyd,
Bjorn Andersson
Cc: linux-arm-msm, linux-phy, linux-kernel, stable
In-Reply-To: <12e8a8de-49b8-463a-8b88-2cbbf1ab901c@oss.qualcomm.com>
On 4/20/26 2:48 PM, Yongxing Mou wrote:
>
>
> On 4/16/2026 5:34 PM, Konrad Dybcio wrote:
>> On 3/2/26 9:28 AM, Yongxing Mou wrote:
>>> The eDP PHY supports both eDP&DP modes, each requires a different table.
>>> The current driver doesn't fully support every combo PHY mode and use
>>> either the eDP or DP table when enable the platform. In addition, some
>>> platforms mismatch between the mode and the table where DP mode uses
>>> the eDP table or eDP mode use the DP table.
>>>
>>> Clean up and correct the tables for currently supported platforms based on
>>> the HPG specification.
>>>
>>> Here lists the tables can be reused across current platforms.
>>> DP mode：
>>> -sa8775p/sc7280/sc8280xp/x1e80100
>>> -glymur
>>> eDP mode(low vdiff):
>>> -glymur/sa8775p/sc8280xp/x1e80100
>>> -sc7280
>>>
>>> Cc: stable@vger.kernel.org
>>> Fixes: f199223cb490 ("phy: qcom: Introduce new eDP PHY driver")
>>> Signed-off-by: Yongxing Mou <yongxing.mou@oss.qualcomm.com>
>>> ---
>>
>> I went through everything and all the sequences are OK.
>>
>> SC8180X will need changes, but it's already incorrect so this
>> doesn't necessarily affect it
>>
>> Thanks!
>>
>> Reviewed-by: Konrad Dybcio <konrad.dybcio@oss.qualcomm.com>
>>
>> Konrad
> Thanks here.. I didn’t notice before that SC8180X is different from SC7280, and will correct it in the next version.
Please make it another patch on top of these two, so that we don't have
to re-validate the existing diff
Konrad
^ permalink raw reply
* Re: [PATCH] dt-bindings: crypto: qcom,inline-crypto-engine: Document Nord ICE
From: Shawn Guo @ 2026-04-20 12:57 UTC (permalink / raw)
To: Krzysztof Kozlowski
Cc: Herbert Xu, Rob Herring, Krzysztof Kozlowski, Conor Dooley,
Bjorn Andersson, Konrad Dybcio, Dmitry Baryshkov,
Bartosz Golaszewski, Deepti Jaggi, linux-crypto, devicetree,
linux-arm-msm, linux-kernel
In-Reply-To: <4b074757-ac44-4077-8ab4-5a983d1be50b@kernel.org>
On Mon, Apr 20, 2026 at 01:56:46PM +0200, Krzysztof Kozlowski wrote:
> On 20/04/2026 10:39, Shawn Guo wrote:
> > On Mon, Apr 20, 2026 at 10:27:56AM +0200, Krzysztof Kozlowski wrote:
> >> On 20/04/2026 09:33, Shawn Guo wrote:
> >>> Add compatible for Inline Crypto Engine (ICE) on Qualcomm Nord SoC
> >>> witha fallback on qcom,inline-crypto-engine.
> >>
> >> Don't explain what the diff is doing. Explain why. Why do you use fallback?
> >>
> >> What is Nord? It's nowhere explained. First posting was 1.5 months ago
> >> and it did not provide any explanation. I don't see any information
> >> being posted in the series sent now.
> >
> > I'm still checking internally to see how we can get the best socinfo
> > patch describing Nord which is a SoC family covering both SA8997P and
> > IQ10 variant. Hopefully I will get it soon.
>
> I found the DTS on:
> https://github.com/qualcomm-linux/kernel-topics/commits/early/hwe/nord/
> so it should be mentioned somewhere, which I kind of asked when we
> discussed about adding compatibles used by that DTS. You would solve
> yourself all my questions from three threads.
Ah, I see. I thought only patches posted to list count.
Thanks!
Shawn
^ permalink raw reply
* Re: [PATCH 3/3] arm64: dts: qcom: eliza-mtp: Enable USB and ADSP support
From: Abel Vesa @ 2026-04-20 13:03 UTC (permalink / raw)
To: Alexander Koskovich
Cc: Bjorn Andersson, Konrad Dybcio, Rob Herring, Krzysztof Kozlowski,
Conor Dooley, linux-arm-msm, devicetree, linux-kernel
In-Reply-To: <adyfWzVHsg3qo9cH@prism>
On 26-04-13 07:52:27, Alexander Koskovich wrote:
> On Tue, Mar 31, 2026 at 01:37:24PM +0300, Abel Vesa wrote:
> > The Eliza MTP features a single USB Type-C port. Its USB 2.0 lines are
> > routed through an eUSB2 repeater provided by the PM7750BA PMIC.
> >
> > Describe the port and repeater, and enable the USB controller and PHYs.
> >
> > Also specify the ADSP firmware and enable the remoteproc.
> >
> > Signed-off-by: Abel Vesa <abel.vesa@oss.qualcomm.com>
> > ---
> > arch/arm64/boot/dts/qcom/eliza-mtp.dts | 83 ++++++++++++++++++++++++++++++++++
> > 1 file changed, 83 insertions(+)
> >
> > diff --git a/arch/arm64/boot/dts/qcom/eliza-mtp.dts b/arch/arm64/boot/dts/qcom/eliza-mtp.dts
> > index 90f629800cb0..c31f00e36eee 100644
> > --- a/arch/arm64/boot/dts/qcom/eliza-mtp.dts
> > +++ b/arch/arm64/boot/dts/qcom/eliza-mtp.dts
> > @@ -6,9 +6,12 @@
> > /dts-v1/;
> ...
> >
> > +&usb_hsphy {
> > + vdd-supply = <&vreg_l2b>;
>
> Shouldn't this be l7k? Looking at kera-usb.dtsi I see the vdd-supply for
> eusb2_phy0 as l7k. I don't see this being overriden by anything else
> downstream either.
>
> Just bringing it up since I copied this part for another Eliza platform
> (USB 2 only) and USB failed until I fixed this supply.
Yep. That seems to be the case. Thanks for pointing this out.
Will fix when respinning.
^ permalink raw reply
* [PATCH AUTOSEL 7.0-5.10] net: qrtr: fix endian handling of confirm_rx field
From: Sasha Levin @ 2026-04-20 13:07 UTC (permalink / raw)
To: patches, stable
Cc: Alexander Wilhelm, Manivannan Sadhasivam, Simon Horman,
David S. Miller, Sasha Levin, edumazet, kuba, pabeni,
linux-arm-msm, netdev, linux-kernel
In-Reply-To: <20260420131539.986432-1-sashal@kernel.org>
From: Alexander Wilhelm <alexander.wilhelm@westermo.com>
[ Upstream commit e4cf6087cab382c7031e6b436ec55202fa9f2d7b ]
Convert confirm_rx to little endian when enqueueing and convert it back on
receive. This fixes control flow on big endian hosts, little endian is
unaffected.
On transmit, store confirm_rx as __le32 using cpu_to_le32(). On receive,
apply le32_to_cpu() before using the value. !! ensures the value is 0 or 1
in native endianness, so the conversion isn’t strictly required here, but
it is kept for consistency and clarity.
Reviewed-by: Manivannan Sadhasivam <mani@kernel.org>
Signed-off-by: Alexander Wilhelm <alexander.wilhelm@westermo.com>
Reviewed-by: Simon Horman <horms@kernel.org>
Signed-off-by: David S. Miller <davem@davemloft.net>
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.
---
## PHASE 1: COMMIT MESSAGE FORENSICS
### Step 1.1: PARSE THE SUBJECT LINE
Record: **[net: qrtr]** **[fix]** Fix endian handling of `confirm_rx`
field — the word "fix" is used explicitly, targeting an endianness bug
in QRTR protocol.
### Step 1.2: PARSE ALL COMMIT MESSAGE TAGS
- **Reviewed-by: Manivannan Sadhasivam <mani@kernel.org>** —
QRTR/Qualcomm ARM SoC maintainer
- **Signed-off-by: Alexander Wilhelm <alexander.wilhelm@westermo.com>**
— Author (Westermo, embedded networking)
- **Reviewed-by: Simon Horman <horms@kernel.org>** — Netdev reviewer
- **Signed-off-by: David S. Miller <davem@davemloft.net>** — Net
subsystem maintainer merged it
No Fixes: tag (it was deliberately removed at Simon Horman's request
during review — see Phase 4). No Reported-by tag. No Cc: stable tag.
### Step 1.3: ANALYZE THE COMMIT BODY TEXT
The commit explains that:
- On transmit, `confirm_rx` needs `cpu_to_le32()` because the header
struct field is `__le32`
- On receive, `le32_to_cpu()` should be applied before use
- The `!!` ensures the value is 0 or 1, so the receive-side conversion
isn't strictly required but is kept for consistency
- This "fixes control flow on big endian hosts"
Record: Bug: Missing endian conversion for the `confirm_rx` field on
both TX and RX paths. Symptom: Broken flow control on big-endian hosts.
Little-endian unaffected. Root cause: `confirm_rx` was stored/read as
native endian into a `__le32` field.
### Step 1.4: DETECT HIDDEN BUG FIXES
Not hidden — this is an explicit endianness bug fix. The subject says
"fix" directly.
---
## PHASE 2: DIFF ANALYSIS — LINE BY LINE
### Step 2.1: INVENTORY THE CHANGES
- **File**: `net/qrtr/af_qrtr.c` — 2 lines changed (1 modified in TX
path, 1 modified in RX path)
- **Functions modified**: `qrtr_node_enqueue()` (TX),
`qrtr_endpoint_post()` (RX)
- **Scope**: Single-file, extremely surgical fix
### Step 2.2: UNDERSTAND THE CODE FLOW CHANGE
**Hunk 1 (line 364, TX path in `qrtr_node_enqueue`):**
- Before: `hdr->confirm_rx = !!confirm_rx;` — stores native-endian int
into `__le32` field
- After: `hdr->confirm_rx = cpu_to_le32(!!confirm_rx);` — properly
converts to little-endian
- On LE hosts: `cpu_to_le32` is a no-op, identical behavior
- On BE hosts: Value 1 was stored as `0x00000001` in native (big-endian)
byte order = `0x01000000` in LE interpretation. Now correctly stored
as LE 1.
**Hunk 2 (line 465, RX path in `qrtr_endpoint_post`):**
- Before: `cb->confirm_rx = !!v1->confirm_rx;` — reads `__le32` as
native int
- After: `cb->confirm_rx = !!le32_to_cpu(v1->confirm_rx);` — properly
converts from LE first
- Due to `!!`, the result on the receive side was already correct (any
non-zero becomes 1). The fix adds the conversion for
correctness/consistency.
### Step 2.3: IDENTIFY THE BUG MECHANISM
Category: **Endianness/type bug (f)**. The `qrtr_hdr_v1` struct declares
`confirm_rx` as `__le32`, and every other field in the struct uses
proper `cpu_to_le32()`/`le32_to_cpu()` conversions — except
`confirm_rx`. This is the one field that was missed.
### Step 2.4: ASSESS THE FIX QUALITY
- **Obviously correct**: Yes — it follows the exact same pattern as all
adjacent fields (type, src_node_id, etc.)
- **Minimal**: Yes — 2 lines, exactly matching the existing code pattern
- **Regression risk**: Essentially zero. On LE hosts (the vast
majority), these are no-ops. On BE hosts, this makes the behavior
correct.
---
## PHASE 3: GIT HISTORY INVESTIGATION
### Step 3.1: BLAME THE CHANGED LINES
- TX line (364): `hdr->confirm_rx = !!confirm_rx;` — introduced by
commit **5fdeb0d372ab** ("net: qrtr: Implement outgoing flow
control"), authored 2020-01-13, first appeared in **v5.6-rc1**
- RX line (465): `cb->confirm_rx = !!v1->confirm_rx;` — introduced by
commit **194ccc88297ae** ("net: qrtr: Support decoding incoming v2
packets"), authored 2017-10-10, first appeared in **v4.15**
Record: The buggy TX code has been present since v5.6. The buggy RX code
since v4.15. Both are in all active stable trees (5.10, 5.15, 6.1, 6.6,
6.12, 7.0).
### Step 3.2: FOLLOW THE FIXES: TAG
The v2 submission HAD `Fixes: 5fdeb0d372ab` but it was removed at Simon
Horman's request. The original buggy commit 5fdeb0d372ab ("Implement
outgoing flow control") is present in v5.6+ and all active stable trees.
### Step 3.3: CHECK FILE HISTORY
Recent changes to `af_qrtr.c` are unrelated refactoring (xarray
conversion, treewide changes, proto_ops changes). No recent endianness
fixes.
### Step 3.4: CHECK THE AUTHOR'S OTHER COMMITS
Alexander Wilhelm from Westermo has a clear pattern of fixing endianness
bugs in Qualcomm subsystems: QMI encoding/decoding, MHI BHI vector
table, ath12k QMI data. This is part of an effort to make Qualcomm
subsystems work on big-endian platforms.
### Step 3.5: CHECK FOR DEPENDENT/PREREQUISITE COMMITS
None. The fix applies directly to the original buggy lines without any
prerequisites.
---
## PHASE 4: MAILING LIST AND EXTERNAL RESEARCH
### Step 4.1: FIND THE ORIGINAL PATCH DISCUSSION
Found via yhbt.net mirror of lore.kernel.org. The patch went through 3
versions:
- **v1** (2026-03-20): Initial submission with Fixes tag, targeted at
`net`
- **v2** (2026-03-24): Rebase on latest net tree, improved commit
message, still had Fixes tag
- **v3** (2026-03-26): Rebase on `net-next`, Fixes tag removed at Simon
Horman's request
### Step 4.2: KEY REVIEWER FEEDBACK
**Simon Horman** (netdev reviewer): "But as this isn't strictly
necessary let's target net-next and drop the Fixes tag." This is a
**negative signal** for stable backport — the netdev reviewer explicitly
downgraded from fix to enhancement.
**Manivannan Sadhasivam** (QRTR maintainer) disagreed: "FWIW: Adding
Fixes tag doesn't mean that the patch should be queued for -rcS." Mani
thought the Fixes tag was appropriate.
### Step 4.3: BUG REPORT
No external bug report. The author found this during systematic
endianness auditing.
### Step 4.4: RELATED PATCHES
This is a standalone fix. Not part of a series.
### Step 4.5: STABLE MAILING LIST HISTORY
No stable-specific discussion found.
---
## PHASE 5: CODE SEMANTIC ANALYSIS
### Step 5.1: IDENTIFY KEY FUNCTIONS
- `qrtr_node_enqueue()` — TX path
- `qrtr_endpoint_post()` — RX path
### Step 5.2: TRACE CALLERS
- `qrtr_node_enqueue()` is called from: `qrtr_sendmsg()` (the main
sendmsg path), `qrtr_send_resume_tx()`, and broadcast path. It's the
core TX function.
- `qrtr_endpoint_post()` is called from: MHI driver (`qrtr_mhi.c`), SMD
driver (`qrtr_smd.c`), tun driver (`qrtr_tun.c`). It's the core RX
entry point — called for EVERY incoming QRTR packet.
### Step 5.3-5.4: CALL CHAIN
`qrtr_endpoint_post()` is called directly from hardware transport
drivers on every received packet. `qrtr_node_enqueue()` is called on
every transmitted packet. Both are hot-path functions.
### Step 5.5: SIMILAR PATTERNS
All other fields in `qrtr_hdr_v1` already use proper endian conversions.
`confirm_rx` was the only one missed.
---
## PHASE 6: CROSS-REFERENCING AND STABLE TREE ANALYSIS
### Step 6.1: DOES THE BUGGY CODE EXIST IN STABLE TREES?
The TX bug (5fdeb0d372ab) exists in **v5.6+**, so all active stable
trees: 5.10.y, 5.15.y, 6.1.y, 6.6.y, 6.12.y.
The RX bug (194ccc88297ae) exists since **v4.15**.
### Step 6.2: BACKPORT COMPLICATIONS
The code at these two lines has not changed since introduction. The
patch should apply cleanly to all active stable trees.
### Step 6.3: RELATED FIXES ALREADY IN STABLE
None found.
---
## PHASE 7: SUBSYSTEM AND MAINTAINER CONTEXT
### Step 7.1: SUBSYSTEM CRITICALITY
**net/qrtr** — Qualcomm IPC Router, used for communication between Linux
and Qualcomm firmware (modem, WiFi, etc.).
Criticality: **PERIPHERAL** — affects users of Qualcomm SoC platforms
running big-endian kernels (very niche). Qualcomm SoCs are little-endian
ARM, so the primary users are unaffected.
### Step 7.2: SUBSYSTEM ACTIVITY
Moderate activity — mostly maintenance fixes, not heavy development.
---
## PHASE 8: IMPACT AND RISK ASSESSMENT
### Step 8.1: WHO IS AFFECTED
Only big-endian hosts that use QRTR. This is extremely niche — Qualcomm
SoCs are LE ARM. However, Westermo (author's company) apparently runs BE
systems with QRTR, and there could be other embedded platforms.
### Step 8.2: TRIGGER CONDITIONS
Every QRTR data transmission on a big-endian host. The TX side stores
the wrong endianness, which means the remote end receives a malformed
`confirm_rx` value. The RX side is actually mitigated by `!!` (any non-
zero normalizes to 1).
### Step 8.3: FAILURE MODE SEVERITY
On big-endian hosts: The flow control mechanism (confirm_rx/resume_tx)
breaks. The TX side sends `confirm_rx` in wrong byte order. If the
remote firmware compares `confirm_rx` directly to 1 (rather than using
`!!`), it won't send RESUME_TX messages, causing the sender to
eventually block or exhaust remote resources.
Severity: **MEDIUM** — broken flow control on a niche platform.
### Step 8.4: RISK-BENEFIT RATIO
- **Benefit**: Fixes real protocol-level correctness bug on BE hosts.
Low impact population but real for those affected.
- **Risk**: Essentially zero. `cpu_to_le32`/`le32_to_cpu` are no-ops on
LE. The fix follows the established pattern used by every other field
in the same struct.
---
## PHASE 9: FINAL SYNTHESIS
### Step 9.1: COMPILE THE EVIDENCE
**FOR backporting:**
- Real endianness correctness bug
- Extremely small fix (2 lines), obviously correct
- Zero regression risk on LE systems (no-op), fixes BE systems
- Follows the identical pattern of all adjacent header fields
- Present in all stable trees since v5.6
- Reviewed by 2 reviewers, merged by David Miller
- Manivannan Sadhasivam (QRTR maintainer) considered the Fixes tag
appropriate
**AGAINST backporting:**
- Simon Horman explicitly said "not strictly necessary," asked to drop
Fixes tag and target net-next
- Big-endian QRTR usage is extremely niche
- The `!!` on the RX path mitigates the receive side (only TX is truly
broken)
- No user bug report — found during code audit
### Step 9.2: STABLE RULES CHECKLIST
1. Obviously correct and tested? **Yes** — standard endian conversion
pattern
2. Fixes a real bug? **Yes** — broken wire protocol on BE hosts
3. Important issue? **Borderline** — broken flow control, but only on BE
hosts using QRTR
4. Small and contained? **Yes** — 2 lines in 1 file
5. No new features? **Correct**
6. Can apply to stable? **Yes** — clean apply expected
### Step 9.3: EXCEPTION CATEGORIES
None applicable.
### Step 9.4: DECISION
This is a genuine endianness bug fix with zero regression risk. While
the affected population is niche, the fix is so small and obviously
correct that the risk/benefit ratio strongly favors inclusion. The
netdev reviewer's pushback was about targeting net vs. net-next (merge
window process), not about whether the fix is correct. The QRTR
maintainer (Mani) disagreed with dropping the Fixes tag. For stable
users on big-endian platforms running QRTR, this fixes a real protocol-
level correctness issue in the flow control mechanism.
---
## Verification
- [Phase 1] Parsed tags: Reviewed-by from Manivannan Sadhasivam and
Simon Horman, SOB from David S. Miller (merger)
- [Phase 2] Diff analysis: 2 lines changed — adding `cpu_to_le32()` on
TX, `le32_to_cpu()` on RX, matching pattern of all other fields
- [Phase 2] Verified `qrtr_hdr_v1.confirm_rx` is declared as `__le32`
(line 44 of af_qrtr.c)
- [Phase 3] git blame: TX line introduced by 5fdeb0d372ab (v5.6,
2020-01-13), RX line by 194ccc88297ae (v4.15, 2017-10-10)
- [Phase 3] git merge-base: Confirmed 5fdeb0d372ab is in v5.10, v5.15,
v6.1, v6.6 (all active stable trees)
- [Phase 3] Author check: Alexander Wilhelm has 7 commits all fixing
Qualcomm endianness bugs
- [Phase 4] Mailing list (yhbt.net mirror): Found full v2 thread. Simon
Horman said "not strictly necessary," Mani disagreed
- [Phase 4] Patch went v1->v2->v3; v3 dropped Fixes tag, targeted net-
next at reviewer request
- [Phase 5] Callers verified: `qrtr_node_enqueue` is core TX path,
`qrtr_endpoint_post` is core RX entry point (EXPORT_SYMBOL_GPL)
- [Phase 5] Verified all other `qrtr_hdr_v1` fields use proper endian
conversions — only `confirm_rx` was missed
- [Phase 6] Code is unchanged at buggy lines since introduction — clean
apply expected
- [Phase 8] Risk assessment: zero risk on LE (no-op conversions), fixes
correctness on BE
**YES**
net/qrtr/af_qrtr.c | 4 ++--
1 file changed, 2 insertions(+), 2 deletions(-)
diff --git a/net/qrtr/af_qrtr.c b/net/qrtr/af_qrtr.c
index d77e9c8212da5..7cec6a7859b03 100644
--- a/net/qrtr/af_qrtr.c
+++ b/net/qrtr/af_qrtr.c
@@ -361,7 +361,7 @@ static int qrtr_node_enqueue(struct qrtr_node *node, struct sk_buff *skb,
}
hdr->size = cpu_to_le32(len);
- hdr->confirm_rx = !!confirm_rx;
+ hdr->confirm_rx = cpu_to_le32(!!confirm_rx);
rc = skb_put_padto(skb, ALIGN(len, 4) + sizeof(*hdr));
@@ -462,7 +462,7 @@ int qrtr_endpoint_post(struct qrtr_endpoint *ep, const void *data, size_t len)
cb->type = le32_to_cpu(v1->type);
cb->src_node = le32_to_cpu(v1->src_node_id);
cb->src_port = le32_to_cpu(v1->src_port_id);
- cb->confirm_rx = !!v1->confirm_rx;
+ cb->confirm_rx = !!le32_to_cpu(v1->confirm_rx);
cb->dst_node = le32_to_cpu(v1->dst_node_id);
cb->dst_port = le32_to_cpu(v1->dst_port_id);
--
2.53.0
^ permalink raw reply related
* [PATCH AUTOSEL 7.0-6.1] Bluetooth: hci_qca: disable power control for WCN7850 when bt_en is not defined
From: Sasha Levin @ 2026-04-20 13:08 UTC (permalink / raw)
To: patches, stable
Cc: Shuai Zhang, Bartosz Golaszewski, Luiz Augusto von Dentz,
Sasha Levin, brgl, marcel, luiz.dentz, linux-arm-msm,
linux-bluetooth, linux-kernel
In-Reply-To: <20260420131539.986432-1-sashal@kernel.org>
From: Shuai Zhang <shuai.zhang@oss.qualcomm.com>
[ Upstream commit 7b75867803a8712bdf7683c31d71d3d5e28ce821 ]
On platforms using an M.2 slot with both UART and USB support, bt_en is
pulled high by hardware. In this case, software-based power control
should be disabled. The current platforms are Lemans-EVK and Monaco-EVK.
Add QCA_WCN7850 to the existing condition so that power_ctrl_enabled is
cleared when bt_en is not software-controlled (or absent), aligning its
behavior with WCN6750 and WCN6855
Signed-off-by: Shuai Zhang <shuai.zhang@oss.qualcomm.com>
Reviewed-by: Bartosz Golaszewski <bartosz.golaszewski@oss.qualcomm.com>
Signed-off-by: Luiz Augusto von Dentz <luiz.von.dentz@intel.com>
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. Let me
compile the results.
---
## PHASE 1: COMMIT MESSAGE FORENSICS
**Step 1.1:** [Subsystem: Bluetooth/hci_qca] [Action: "disable" / "add"]
[Summary: Disable software power control for WCN7850 when bt_en GPIO is
not defined (HW-managed)]
**Step 1.2:** Tags found:
- `Signed-off-by: Shuai Zhang <shuai.zhang@oss.qualcomm.com>` - Author,
Qualcomm BT developer
- `Reviewed-by: Bartosz Golaszewski
<bartosz.golaszewski@oss.qualcomm.com>` - This is the author of the
prerequisite commit `0fb410c914eb03` that introduced the
WCN6750/WCN6855-only check. His review is highly significant.
- `Signed-off-by: Luiz Augusto von Dentz <luiz.von.dentz@intel.com>` -
BT subsystem maintainer applied it
- No Fixes: tag, no Cc: stable, no Reported-by (expected for candidates)
**Step 1.3:** The commit body explains: On Lemans-EVK and Monaco-EVK
platforms (M.2 slot with UART+USB), bt_en is pulled high by hardware.
Software power control must be disabled. Without this,
`power_ctrl_enabled` remains true for WCN7850, causing
`HCI_QUIRK_NON_PERSISTENT_SETUP` and `qca_power_off` shutdown handler to
be set incorrectly.
**Step 1.4:** This IS a bug fix disguised as "aligning behavior." The
commit adds WCN7850 to a condition that was already handling the same
scenario for WCN6750/WCN6855, making WCN7850 broken on affected
platforms.
## PHASE 2: DIFF ANALYSIS
**Step 2.1:** Single file changed: `drivers/bluetooth/hci_qca.c`, +2/-1
lines. Function modified: `qca_serdev_probe()`. Scope: single-file,
single-hunk surgical fix.
**Step 2.2:** Before: When `bt_en` is NULL, only WCN6750 and WCN6855 got
`power_ctrl_enabled=false`. After: WCN7850 also gets
`power_ctrl_enabled=false`. This affects the probe path where the power
control strategy is decided.
**Step 2.3:** Bug category: Logic/correctness fix - missing SoC type in
a condition. When `power_ctrl_enabled` remains incorrectly true:
- `HCI_QUIRK_NON_PERSISTENT_SETUP` is set (line 2532)
- `hdev->shutdown = qca_power_off` is set (line 2533)
- The SSR recovery in `fce1a9244a0f8` checks this quirk and takes the
wrong path
**Step 2.4:** Fix is obviously correct - follows established pattern.
Zero regression risk (only adds a SoC type to an OR chain). Reviewed by
the author of the prerequisite code.
## PHASE 3: GIT HISTORY INVESTIGATION
**Step 3.1:** `git blame` shows the condition was introduced by
`0fb410c914eb03` (Bartosz Golaszewski, 2025-05-27), which restructured
the code to restrict the `power_ctrl_enabled=false` check to
WCN6750/WCN6855 only. WCN7850 was inadvertently omitted.
**Step 3.2:** No Fixes: tag. The root cause is `0fb410c914eb03` which
has its own `Fixes: 3d05fc82237a` and `Cc: stable@vger.kernel.org`.
WCN7850 was missed when `0fb410c914eb03` restricted the condition.
**Step 3.3:** Related commits by same author: `fce1a9244a0f8` "Fix SSR
fail when BT_EN is pulled up by hw" - this is the companion fix that
depends on `HCI_QUIRK_NON_PERSISTENT_SETUP` being correctly set. This
commit is standalone but paired with `0fb410c914eb03`.
**Step 3.4:** Shuai Zhang is a regular Qualcomm BT contributor. Bartosz
Golaszewski (reviewer) wrote the prerequisite code.
**Step 3.5:** This commit depends on `0fb410c914eb03` being present.
That commit is in mainline (first tagged in v6.16/v6.17) but NOT yet in
any stable tree (not in v6.6, v6.12, or v6.14). In stable trees without
`0fb410c914eb03`, the code has an unconditional check (`if
(!qcadev->bt_en) power_ctrl_enabled = false;`) that covers ALL SoC types
including WCN7850. The bug only manifests after `0fb410c914eb03` is
applied.
## PHASE 4: MAILING LIST RESEARCH
The patch went through v1 -> v2 -> v3. v1 had review feedback from
Dmitry Baryshkov requesting more context about affected platforms. v2/v3
added platform details (Lemans-EVK, Monaco-EVK). Bartosz Golaszewski
(who wrote the prerequisite commit) gave Reviewed-by on v3. Luiz von
Dentz (BT maintainer) applied it to bluetooth-next. No NAKs, no concerns
about the code change itself.
## PHASE 5: CODE SEMANTIC ANALYSIS
`power_ctrl_enabled` controls two behaviors in `qca_serdev_probe()`:
1. Setting `HCI_QUIRK_NON_PERSISTENT_SETUP` quirk
2. Registering `qca_power_off` as shutdown handler
When `power_ctrl_enabled` is incorrectly true for WCN7850 with HW-
managed bt_en:
- `qca_power_off` -> `qca_power_shutdown()` falls to default case:
`gpiod_set_value_cansleep(NULL, 0)` which is a no-op (safe)
- But the quirk `HCI_QUIRK_NON_PERSISTENT_SETUP` being set causes the
SSR recovery code (`fce1a9244a0f8`) to skip critical recovery steps,
leading to SSR failure (HCI reset timeout)
## PHASE 6: STABLE TREE ANALYSIS
- **v6.6**: WCN7850 exists (12 references). Code structure is completely
different (`IS_ERR_OR_NULL` pattern). Bug exists differently but this
patch wouldn't apply without significant rework.
- **v6.12/v6.14**: `3d05fc82237aa9` is present but `0fb410c914eb03` is
NOT. The check is unconditional (`if (!qcadev->bt_en)
power_ctrl_enabled = false;`), so the bug does NOT exist yet. However,
when `0fb410c914eb03` (tagged `Cc: stable`) is backported, it WILL
introduce this bug by restricting the check to WCN6750/WCN6855 only.
- This patch must be paired with `0fb410c914eb03` when backporting.
## PHASE 7: SUBSYSTEM AND MAINTAINER CONTEXT
- Subsystem: Bluetooth driver (IMPORTANT, affects Qualcomm BT hardware
users)
- Criticality: Driver-specific, but WCN7850 is a widely-used Qualcomm BT
chip (SM8550 platforms and others)
- Active subsystem with regular contributions
## PHASE 8: IMPACT AND RISK ASSESSMENT
- **Who is affected**: WCN7850 users on platforms where bt_en is HW-
controlled (Lemans-EVK, Monaco-EVK with M.2 slot)
- **Trigger**: Always, during probe on affected hardware. Not timing-
dependent.
- **Failure mode**: SSR failure - BT controller cannot recover from
firmware crash. HCI reset times out. MEDIUM-HIGH severity (Bluetooth
becomes non-functional after FW crash until reboot)
- **Benefit**: High for affected hardware users
- **Risk**: Very low - 1 line addition to an OR condition, obviously
correct pattern
## PHASE 9: FINAL SYNTHESIS
**Evidence FOR:**
- Fixes real hardware issue on WCN7850 platforms with HW-managed bt_en
- Trivially small (1 line), obviously correct
- Reviewed by the author of the prerequisite code
- Applied by BT subsystem maintainer
- Follows established pattern (WCN6750/WCN6855 already handled)
- Without this fix, SSR recovery fails on affected platforms
- Falls under "hardware quirk/workaround" exception category
**Evidence AGAINST:**
- Depends on `0fb410c914eb03` (not yet in stable trees)
- Limited platform scope (Lemans-EVK, Monaco-EVK)
- In current stable trees, the bug doesn't exist yet (unconditional
check)
**Stable rules checklist:**
1. Obviously correct and tested? YES (trivial 1-line addition, reviewed)
2. Fixes a real bug? YES (SSR failure on affected hardware)
3. Important issue? YES (BT becomes non-functional after FW crash)
4. Small and contained? YES (1 line change)
5. No new features? YES (just extends existing condition)
6. Can apply to stable? YES, when paired with `0fb410c914eb03`
## Verification
- [Phase 1] Parsed tags: Reviewed-by from Bartosz Golaszewski
(prerequisite commit author), applied by BT maintainer Luiz von Dentz
- [Phase 2] Diff: 1 line added to OR condition in `qca_serdev_probe()`,
adding `QCA_WCN7850`
- [Phase 3] `git blame`: condition introduced by `0fb410c914eb03`
(2025-05-27), which restricted check to WCN6750/WCN6855
- [Phase 3] `0fb410c914eb03` has `Cc: stable`, `Fixes: 3d05fc82237a` -
will be backported
- [Phase 3] `0fb410c914eb03` NOT in v6.6, v6.12, v6.14 (verified with
`git merge-base`)
- [Phase 3] WCN7850 support (`e0c1278ac89b0`) is in v6.6 (2023-08-24)
- [Phase 4] Patchew: v3 reviewed by Bartosz Golaszewski, applied by Luiz
von Dentz. No NAKs.
- [Phase 5] `power_ctrl_enabled=true` causes incorrect
`HCI_QUIRK_NON_PERSISTENT_SETUP` and SSR recovery failure
- [Phase 5] Verified `qca_power_shutdown()` default case:
`gpiod_set_value_cansleep(NULL, 0)` is safe (no-op)
- [Phase 6] v6.12/v6.14: unconditional check exists, bug not present.
Bug will appear when `0fb410c914eb03` is backported
- [Phase 6] v6.6: different code structure, would need rework
- [Phase 8] Failure mode: SSR failure (HCI reset timeout, BT non-
functional until reboot), MEDIUM-HIGH severity
**YES**
drivers/bluetooth/hci_qca.c | 3 ++-
1 file changed, 2 insertions(+), 1 deletion(-)
diff --git a/drivers/bluetooth/hci_qca.c b/drivers/bluetooth/hci_qca.c
index bb9f002aa85e9..edc907c4e870a 100644
--- a/drivers/bluetooth/hci_qca.c
+++ b/drivers/bluetooth/hci_qca.c
@@ -2471,7 +2471,8 @@ static int qca_serdev_probe(struct serdev_device *serdev)
if (!qcadev->bt_en &&
(data->soc_type == QCA_WCN6750 ||
- data->soc_type == QCA_WCN6855))
+ data->soc_type == QCA_WCN6855 ||
+ data->soc_type == QCA_WCN7850))
power_ctrl_enabled = false;
qcadev->sw_ctrl = devm_gpiod_get_optional(&serdev->dev, "swctrl",
--
2.53.0
^ permalink raw reply related
* [PATCH AUTOSEL 7.0-6.6] Bluetooth: hci_qca: Fix missing wakeup during SSR memdump handling
From: Sasha Levin @ 2026-04-20 13:08 UTC (permalink / raw)
To: patches, stable
Cc: Shuai Zhang, Bartosz Golaszewski, Paul Menzel,
Luiz Augusto von Dentz, Sasha Levin, brgl, marcel, luiz.dentz,
linux-arm-msm, linux-bluetooth, linux-kernel
In-Reply-To: <20260420131539.986432-1-sashal@kernel.org>
From: Shuai Zhang <shuai.zhang@oss.qualcomm.com>
[ Upstream commit c347ca17d62a32c25564fee0ca3a2a7bc2d5fd6f ]
When a Bluetooth controller encounters a coredump, it triggers the
Subsystem Restart (SSR) mechanism. The controller first reports the
coredump data and, once the upload is complete, sends a hw_error
event. The host relies on this event to proceed with subsequent
recovery actions.
If the host has not finished processing the coredump data when the
hw_error event is received, it waits until either the processing is
complete or the 8-second timeout expires before handling the event.
The current implementation clears QCA_MEMDUMP_COLLECTION using
clear_bit(), which does not wake up waiters sleeping in
wait_on_bit_timeout(). As a result, the waiting thread may remain
blocked until the timeout expires even if the coredump collection
has already completed.
Fix this by clearing QCA_MEMDUMP_COLLECTION with
clear_and_wake_up_bit(), which also wakes up the waiting thread and
allows the hw_error handling to proceed immediately.
Test case:
- Trigger a controller coredump using:
hcitool cmd 0x3f 0c 26
- Tested on QCA6390.
- Capture HCI logs using btmon.
- Verify that the delay between receiving the hw_error event and
initiating the power-off sequence is reduced compared to the
timeout-based behavior.
Reviewed-by: Bartosz Golaszewski <bartosz.golaszewski@oss.qualcomm.com>
Reviewed-by: Paul Menzel <pmenzel@molgen.mpg.de>
Signed-off-by: Shuai Zhang <shuai.zhang@oss.qualcomm.com>
Signed-off-by: Luiz Augusto von Dentz <luiz.von.dentz@intel.com>
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. Let me
compile it.
---
## PHASE 1: COMMIT MESSAGE FORENSICS
### Step 1.1: PARSE THE SUBJECT LINE
- **Subsystem prefix:** Bluetooth: hci_qca
- **Action verb:** "Fix" - explicitly a bug fix
- **One-line summary:** Fix missing wakeup during SSR memdump handling -
`clear_bit()` doesn't wake up waiters sleeping in
`wait_on_bit_timeout()`.
### Step 1.2: PARSE ALL COMMIT MESSAGE TAGS
- **Reviewed-by:** Bartosz Golaszewski (Qualcomm contributor,
knowledgeable in this driver)
- **Reviewed-by:** Paul Menzel (known active reviewer)
- **Signed-off-by:** Shuai Zhang <shuai.zhang@oss.qualcomm.com> (author,
Qualcomm - QCA chipset vendor)
- **Signed-off-by:** Luiz Augusto von Dentz <luiz.von.dentz@intel.com>
(Bluetooth maintainer)
- No Fixes: tag, no Reported-by, no syzbot. Absence of Fixes: is
expected.
### Step 1.3: ANALYZE THE COMMIT BODY TEXT
- **Bug:** When Bluetooth controller encounters a coredump (SSR), it
sends memdump data then sends `hw_error` event. The host calls
`wait_on_bit_timeout()` on `QCA_MEMDUMP_COLLECTION` to wait for the
collection to complete. But the collection worker clears the bit with
`clear_bit()`, which does NOT wake up the waiter.
- **Symptom:** The waiting thread blocks for the full 8-second timeout
(`MEMDUMP_TIMEOUT_MS = 8000`) even when collection finishes early.
- **Root cause:** API misuse - `wait_on_bit_timeout()` documentation
explicitly requires wakeup via `wake_up_bit()` or
`clear_and_wake_up_bit()`.
- **Test:** Tested on QCA6390 hardware using `hcitool` and btmon.
### Step 1.4: DETECT HIDDEN BUG FIXES
This is an explicitly stated bug fix, not disguised. The
`wait_on_bit_timeout` API documentation (in `include/linux/wait_bit.h`)
states: "The clearing of the bit must be signalled with wake_up_bit(),
often as clear_and_wake_up_bit()." Using plain `clear_bit()` is an API
violation.
---
## PHASE 2: DIFF ANALYSIS - LINE BY LINE
### Step 2.1: INVENTORY THE CHANGES
- **File:** `drivers/bluetooth/hci_qca.c` only
- **Changes:** 2 lines changed (2 `clear_bit` → `clear_and_wake_up_bit`)
- **Functions modified:** `qca_controller_memdump()` (2 locations)
- **Scope:** Single-file, single-function surgical fix
### Step 2.2: UNDERSTAND THE CODE FLOW CHANGE
**Hunk 1 (line 1108):** Error path when `hci_devcd_init()` fails:
- Before: `clear_bit(QCA_MEMDUMP_COLLECTION, &qca->flags)` — clears bit
but no wakeup
- After: `clear_and_wake_up_bit(QCA_MEMDUMP_COLLECTION, &qca->flags)` —
clears bit AND wakes waiting thread
**Hunk 2 (line 1186):** Normal completion path (last sequence received):
- Before: same `clear_bit()` without wakeup
- After: same `clear_and_wake_up_bit()` with wakeup
### Step 2.3: IDENTIFY THE BUG MECHANISM
This is a **synchronization bug**: missing wakeup. The
`qca_wait_for_dump_collection()` function calls `wait_on_bit_timeout()`
which puts the thread to sleep waiting for the bit to be cleared AND a
wakeup signal. Without the wakeup, the thread sleeps for the full
8-second timeout.
### Step 2.4: ASSESS THE FIX QUALITY
- **Obviously correct:** Yes. The `wait_on_bit_timeout` documentation
explicitly states wakeup is required.
- **Minimal:** Yes, 2 line changes.
- **Regression risk:** Negligible. `clear_and_wake_up_bit()` does
exactly what `clear_bit()` does plus a wakeup. No new side effects.
---
## PHASE 3: GIT HISTORY INVESTIGATION
### Step 3.1: BLAME THE CHANGED LINES
- Line 1108 (`clear_bit`): Introduced by `06d3fdfcdf5cef` (Sai Teja
Aluvala, 2023-06-14) — v6.6-rc1
- Line 1186 (`clear_bit`): Introduced by `7c2c3e63e1e97c` (Venkata
Lakshmi, 2020-02-14) — v5.7-rc1
- `wait_on_bit_timeout` (line 1606): Introduced by `d841502c79e3fd`
(Balakrishna Godavarthi, 2020-01-02) — v5.6-rc1
So the bug at line 1186 has existed since v5.7, and the bug at line 1108
since v6.6.
### Step 3.2: FOLLOW THE FIXES: TAG
No Fixes: tag (expected).
### Step 3.3: CHECK FILE HISTORY
Recent changes to `hci_qca.c` are active (73 commits since v5.15). The
file sees regular activity.
### Step 3.4: CHECK THE AUTHOR'S OTHER COMMITS
Shuai Zhang is a Qualcomm contributor with multiple commits to the QCA
Bluetooth stack. The fix was reviewed by the Bluetooth maintainer (Luiz
Augusto von Dentz).
### Step 3.5: CHECK FOR DEPENDENT/PREREQUISITE COMMITS
None. `clear_and_wake_up_bit()` has existed since v4.17. The fix is a
drop-in replacement for `clear_bit()` at two locations.
---
## PHASE 4: MAILING LIST AND EXTERNAL RESEARCH
### Step 4.1-4.2: FIND THE ORIGINAL PATCH DISCUSSION
Found at:
https://yhbt.net/lore/lkml/177583080679.2077665.8641347877052929776.git-
patchwork-notify@kernel.org/T/
The patch went through **7 revisions** (v1 through v7), indicating
extensive review:
- v5→v6: Changed from `wake_up_bit` to `clear_and_wake_up_bit` (the
proper API)
- Applied to bluetooth-next by Luiz Augusto von Dentz (Bluetooth
maintainer)
- Commit in bluetooth-next: `9f07d5d04826`
### Step 4.3: BUG REPORT
No external bug report — the author identified the issue through
code/testing.
### Step 4.4-4.5: RELATED PATCHES AND STABLE HISTORY
This is a standalone single-patch fix. No series dependencies.
---
## PHASE 5: CODE SEMANTIC ANALYSIS
### Step 5.1-5.4: KEY FUNCTIONS AND CALL CHAINS
The affected path:
1. Bluetooth controller crashes → sends memdump data → sends `hw_error`
event
2. `qca_hw_error()` or `qca_reset()` → calls
`qca_wait_for_dump_collection()` → `wait_on_bit_timeout()` on
`QCA_MEMDUMP_COLLECTION`
3. Concurrently, `qca_controller_memdump()` (workqueue) processes dump
packets
4. On completion, `qca_controller_memdump()` clears
`QCA_MEMDUMP_COLLECTION` — but without waking up the waiter in step 2
5. Result: waiter in step 2 sleeps for full 8 seconds even though
collection finished
Both `qca_hw_error()` and `qca_reset()` call
`qca_wait_for_dump_collection()`, so both paths are affected.
### Step 5.5: SIMILAR PATTERNS
No other `clear_bit`/`wait_on_bit_timeout` mismatches found in this
file.
---
## PHASE 6: CROSS-REFERENCING AND STABLE TREE ANALYSIS
### Step 6.1: DOES THE BUGGY CODE EXIST IN STABLE TREES?
- The `clear_bit` at the completion path (line 1186) has been present
since v5.7, so it exists in stable trees 5.10.y, 5.15.y, 6.1.y, 6.6.y,
6.12.y.
- The `clear_bit` at the error path (line 1108) was introduced in v6.6,
so only in 6.6.y, 6.12.y.
### Step 6.2: BACKPORT COMPLICATIONS
The patch should apply cleanly or with minor context adjustments. The
two lines being changed are simple API call replacements. Older trees
may not have the first hunk (line 1108) since that code was added in
v6.6.
### Step 6.3: RELATED FIXES ALREADY IN STABLE
No related fixes for this specific bug found.
---
## PHASE 7: SUBSYSTEM AND MAINTAINER CONTEXT
### Step 7.1: SUBSYSTEM CRITICALITY
- **Subsystem:** drivers/bluetooth — Bluetooth driver for Qualcomm
chipsets
- **Criticality:** IMPORTANT — QCA Bluetooth chipsets are widely used in
laptops, phones, and embedded systems
### Step 7.2: SUBSYSTEM ACTIVITY
Active subsystem with regular commits.
---
## PHASE 8: IMPACT AND RISK ASSESSMENT
### Step 8.1: AFFECTED POPULATION
Users of QCA Bluetooth chipsets (QCA6390 and similar) — a significant
population in the Android and laptop ecosystem.
### Step 8.2: TRIGGER CONDITIONS
Triggered when the Bluetooth controller crashes and SSR begins. Not
common in normal operation, but when it happens (coredump, hw error),
the 8-second unnecessary delay is always present.
### Step 8.3: FAILURE MODE SEVERITY
- **Failure mode:** Unnecessary 8-second delay during Bluetooth recovery
after controller crash
- **Severity:** MEDIUM — Not a crash, not data corruption, not a
security issue. It's a latency bug during error recovery that affects
usability.
### Step 8.4: RISK-BENEFIT RATIO
- **Benefit:** Eliminates unnecessary 8-second delay during SSR
recovery. Correct API usage per documentation.
- **Risk:** Very low — 2-line change, `clear_and_wake_up_bit` is a
strict superset of `clear_bit` (clear + wakeup). No new side effects.
- **Ratio:** Very favorable — minimal risk for a real improvement in
error recovery behavior.
---
## PHASE 9: FINAL SYNTHESIS
### Step 9.1: COMPILE THE EVIDENCE
**FOR backporting:**
- Fixes a genuine API misuse bug (`clear_bit` instead of
`clear_and_wake_up_bit`)
- 2-line surgical fix, trivially correct per API documentation
- Extensively reviewed (7 revisions), 2 Reviewed-by tags
- Applied by Bluetooth maintainer
- Tested on real hardware (QCA6390)
- Very low regression risk
- Bug exists since v5.7 for one path and v6.6 for the other
- `clear_and_wake_up_bit()` exists since v4.17, no dependency issues
**AGAINST backporting:**
- The bug doesn't cause crashes, data corruption, or security issues
- Only manifests during SSR (error recovery), not during normal
operation
- It's a latency/usability improvement rather than a fix for a hard
failure
### Step 9.2: STABLE RULES CHECKLIST
1. **Obviously correct and tested?** YES — API documentation is
explicit, tested on hardware
2. **Fixes a real bug?** YES — missing wakeup causing unnecessary
8-second delay
3. **Important issue?** MEDIUM — recovery delay, not crash/corruption
4. **Small and contained?** YES — 2 lines in 1 file
5. **No new features or APIs?** YES — just corrects API usage
6. **Can apply to stable trees?** YES — `clear_and_wake_up_bit` exists
since v4.17
### Step 9.3: EXCEPTION CATEGORIES
Not applicable — this is a standard bug fix.
### Step 9.4: DECISION
The fix is tiny, obviously correct, well-reviewed, and addresses a real
bug where `wait_on_bit_timeout()` is used with `clear_bit()` instead of
`clear_and_wake_up_bit()`, causing an unnecessary 8-second blocking
delay during Bluetooth error recovery. While the impact is "only" a
latency issue during recovery (not a crash or data loss), the extremely
low risk of the fix and the real user-visible improvement make this
appropriate for stable.
---
## Verification
- [Phase 1] Parsed tags: Reviewed-by from Bartosz Golaszewski and Paul
Menzel; SOB from author and Bluetooth maintainer
- [Phase 2] Diff analysis: 2 lines changed in
`qca_controller_memdump()`, replacing `clear_bit()` with
`clear_and_wake_up_bit()`
- [Phase 3] git blame: Line 1108 introduced in `06d3fdfcdf5cef`
(v6.6-rc1); Line 1186 introduced in `7c2c3e63e1e97c` (v5.7-rc1);
`wait_on_bit_timeout` introduced in `d841502c79e3fd` (v5.6-rc1)
- [Phase 3] git describe: `clear_and_wake_up_bit` introduced in
`8236b0ae31c83` (v4.17-rc4), present in all active stable trees
- [Phase 4] lore thread found: patch went through v1→v7, applied to
bluetooth-next by maintainer as `9f07d5d04826`
- [Phase 4] No NAKs or objections in the discussion thread
- [Phase 5] Call chain: `qca_hw_error()`/`qca_reset()` →
`qca_wait_for_dump_collection()` → `wait_on_bit_timeout()` waits for
bit cleared by `qca_controller_memdump()` workqueue
- [Phase 5] Verified `wait_on_bit_timeout()` documentation in
`include/linux/wait_bit.h` lines 118-120 explicitly requires
`clear_and_wake_up_bit()`
- [Phase 6] Buggy code exists in stable trees 5.10+, 5.15+, 6.1+, 6.6+,
6.12+ (second hunk); 6.6+, 6.12+ (first hunk)
- [Phase 6] `MEMDUMP_TIMEOUT_MS` is 8000 (8 seconds) — confirmed at line
54
- [Phase 8] Failure mode: 8-second unnecessary delay during Bluetooth
SSR recovery, severity MEDIUM
**YES**
drivers/bluetooth/hci_qca.c | 4 ++--
1 file changed, 2 insertions(+), 2 deletions(-)
diff --git a/drivers/bluetooth/hci_qca.c b/drivers/bluetooth/hci_qca.c
index edc907c4e870a..524e47392f919 100644
--- a/drivers/bluetooth/hci_qca.c
+++ b/drivers/bluetooth/hci_qca.c
@@ -1105,7 +1105,7 @@ static void qca_controller_memdump(struct work_struct *work)
qca->qca_memdump = NULL;
qca->memdump_state = QCA_MEMDUMP_COLLECTED;
cancel_delayed_work(&qca->ctrl_memdump_timeout);
- clear_bit(QCA_MEMDUMP_COLLECTION, &qca->flags);
+ clear_and_wake_up_bit(QCA_MEMDUMP_COLLECTION, &qca->flags);
clear_bit(QCA_IBS_DISABLED, &qca->flags);
mutex_unlock(&qca->hci_memdump_lock);
return;
@@ -1183,7 +1183,7 @@ static void qca_controller_memdump(struct work_struct *work)
kfree(qca->qca_memdump);
qca->qca_memdump = NULL;
qca->memdump_state = QCA_MEMDUMP_COLLECTED;
- clear_bit(QCA_MEMDUMP_COLLECTION, &qca->flags);
+ clear_and_wake_up_bit(QCA_MEMDUMP_COLLECTION, &qca->flags);
}
mutex_unlock(&qca->hci_memdump_lock);
--
2.53.0
^ permalink raw reply related
* [PATCH AUTOSEL 7.0-5.15] remoteproc: qcom: Fix minidump out-of-bounds access on subsystems array
From: Sasha Levin @ 2026-04-20 13:16 UTC (permalink / raw)
To: patches, stable
Cc: Mukesh Ojha, Konrad Dybcio, Bjorn Andersson, Sasha Levin,
mathieu.poirier, linux-arm-msm, linux-remoteproc, linux-kernel
In-Reply-To: <20260420132314.1023554-1-sashal@kernel.org>
From: Mukesh Ojha <mukesh.ojha@oss.qualcomm.com>
[ Upstream commit 743cfae79d2458e241b06ed523c28a09f1449b75 ]
MAX_NUM_OF_SS was hardcoded to 10 in the minidump_global_toc struct,
which is a direct overlay on an SMEM item allocated by the firmware.
Newer Qualcomm SoC firmware allocates space for more subsystems, while
older firmware only allocates space for 10. Bumping the constant would
cause Linux to read/write beyond the SMEM item boundary on older
platforms.
Fix this by converting subsystems[] to a flexible array member and
deriving the actual number of subsystems at runtime from the size
returned by qcom_smem_get(). Add a bounds check on minidump_id against
the derived count before indexing into the array.
Signed-off-by: Mukesh Ojha <mukesh.ojha@oss.qualcomm.com>
Acked-by: Konrad Dybcio <konrad.dybcio@oss.qualcomm.com>
Link: https://lore.kernel.org/r/20260331171243.1962067-1-mukesh.ojha@oss.qualcomm.com
Signed-off-by: Bjorn Andersson <andersson@kernel.org>
Signed-off-by: Sasha Levin <sashal@kernel.org>
---
LLM Generated explanations, may be completely bogus:
Now I have enough information to complete my analysis.
## PHASE 1: COMMIT MESSAGE FORENSICS
**Step 1.1 - Subject Line Parse:**
Record: Subsystem `remoteproc: qcom:`; action verb `Fix`; summary: fix
out-of-bounds access on minidump subsystems array.
**Step 1.2 - Tags:**
Record:
- `Signed-off-by: Mukesh Ojha <mukesh.ojha@oss.qualcomm.com>` (author,
Qualcomm/remoteproc contributor)
- `Acked-by: Konrad Dybcio <konrad.dybcio@oss.qualcomm.com>` (subsystem
contributor)
- `Link: https://lore.kernel.org/r/20260331171243.1962067-1-
mukesh.ojha@oss.qualcomm.com`
- `Signed-off-by: Bjorn Andersson <andersson@kernel.org>` (remoteproc
co-maintainer)
- No `Cc: stable@vger.kernel.org`, no `Fixes:` tag (expected for review
candidates)
**Step 1.3 - Commit Body:**
Record: Bug description: `MAX_NUM_OF_SS` hardcoded to 10 but
`minidump_global_toc` overlays SMEM item allocated by firmware; newer
firmware allocates more subsystems; bumping constant would overflow SMEM
on older platforms. Fix: convert to flexible array, derive count at
runtime from `qcom_smem_get()` size, add bounds check.
**Step 1.4 - Hidden Fix:**
Record: Explicit bug fix ("Fix ... out-of-bounds access") - not hidden.
## PHASE 2: DIFF ANALYSIS
**Step 2.1 - Inventory:**
Record: Single file `drivers/remoteproc/qcom_common.c`; +14/-3 lines;
modifies `qcom_minidump()` function and `struct minidump_global_toc`;
surgical, single-file fix.
**Step 2.2 - Code Flow:**
Record: Before: `qcom_smem_get(...,NULL)` ignores size; directly indexes
`toc->subsystems[minidump_id]` which is declared `[MAX_NUM_OF_SS=10]`.
After: requests actual SMEM size via `&toc_size`; computes `num_ss` from
`(toc_size - offsetof(...,subsystems))/sizeof(subsystem)`; returns early
with error if `minidump_id >= num_ss`.
**Step 2.3 - Bug Mechanism:**
Record: Category: **memory safety / out-of-bounds access**. With
`MAX_NUM_OF_SS=10`, accessing `toc->subsystems[20]` (as done for SA8775p
CDSP1) is out-of-bounds per the C array type. Fix converts to flexible
array member and validates index at runtime.
**Step 2.4 - Fix Quality:**
Record: Obviously correct; minimal/surgical; preserves behavior for
valid indexes (< actual count); no regression risk for existing devices
using minidump_id 3/4/5/7; depends on `qcom_smem_get()` reporting
accurate size which is its contract.
## PHASE 3: GIT HISTORY INVESTIGATION
**Step 3.1 - Blame:**
Record: `MAX_NUM_OF_SS=10` was introduced by `8ed8485c4f056
("remoteproc: qcom: Add capability to collect minidumps")` in Nov 2020
(v5.11). The buggy code has been present since v5.11.
**Step 3.2 - Fixes Tag:**
Record: No `Fixes:` tag present. The bug was technically present since
minidump was added in v5.11, but only becomes an observable OOB when a
device's `minidump_id` exceeds 9.
**Step 3.3 - File History:**
Record: The file has been minimally modified. Key related commits:
- `9091225ba28c0` ("remoteproc: qcom: pas: Add support for SA8775p ADSP,
CDSP and GPDSP") in v6.12-rc1 added `minidump_id = 20, 21, 22` for
sa8775p CDSP1, GPDSP0, GPDSP1 → this is when the bug becomes
triggerable.
- `318da1371246f` ("remoteproc: qcom: Expand MD_* as MINIDUMP_*") - same
author, minidump cleanup
- Standalone fix, not part of a series.
**Step 3.4 - Author Context:**
Record: Mukesh Ojha is a regular Qualcomm remoteproc contributor and the
primary author of minidump-related work (multiple minidump cleanup
commits). Fix from a knowledgeable author.
**Step 3.5 - Dependencies:**
Record: Only dependency is that `qcom_smem_get()` supports returning
size via pointer, which it has done since inception (verified in all
stable trees).
## PHASE 4: MAILING LIST RESEARCH
**Step 4.1 - b4 dig:**
Record: Found at `https://lore.kernel.org/all/20260331171243.1962067-1-
mukesh.ojha@oss.qualcomm.com/`. Only v2 was applied.
**Step 4.2 - Reviewers:**
Record: Sent to Bjorn Andersson (co-maintainer), Mathieu Poirier (co-
maintainer), linux-arm-msm, linux-remoteproc. Reviewed/Acked by Konrad
Dybcio.
**Step 4.3 - History - v1 Discussion:**
Record: Fetched v1 thread
(`20250808164417.4105659-1-mukesh.ojha@oss.qualcomm.com`). V1 was a
naïve fix bumping `MAX_NUM_OF_SS` from 10 to 30. Bjorn Andersson
objected: "this number is used to size the minidump_global_toc struct...
Doesn't this imply that on older platforms you've now told Linux (and
your debugger) that it's fine to write beyond the smem item?" and
suggested "check the returned size of the qcom_smem_get() call." Author
Mukesh specifically asked **"do you think it should a fix (cc
stable)?"**. V2 implemented exactly Bjorn's suggestion, received Ack,
and was applied.
**Step 4.4 - Series:**
Record: Standalone single-patch submission.
**Step 4.5 - Stable Mailing List:**
Record: No separate stable discussion visible; author explicitly
considered it a stable fix but final v2 lacks Cc:stable tag.
## PHASE 5: CODE SEMANTIC ANALYSIS
**Step 5.1 - Key Functions:**
Record: `qcom_minidump()` is the modified function; `struct
minidump_global_toc` is the modified type.
**Step 5.2 - Callers:**
Record: `qcom_minidump()` is called from exactly one place -
`qcom_pas_minidump()` in `drivers/remoteproc/qcom_q6v5_pas.c:151`. This
is registered as `.coredump` callback in `qcom_pas_minidump_ops` (line
531), which is invoked by `rproc_boot_recovery()` in
`remoteproc_core.c:1798` when a crashed remoteproc is being recovered.
**Step 5.3 - Callees:**
Record: Calls `qcom_smem_get()` (SMEM item retrieval), `dev_err()`, and
then `rproc_coredump()` or `qcom_add_minidump_segments()` +
`rproc_coredump_using_sections()`.
**Step 5.4 - Call Chain / Reachability:**
Record: Reachable from remoteproc crash recovery: remoteproc crash →
`rproc_boot_recovery()` → `rproc->ops->coredump()` →
`qcom_pas_minidump()` → `qcom_minidump(rproc, pas->minidump_id, ...)`.
With `pas->minidump_id = 20/21/22`, this triggers OOB on
`toc->subsystems[20]` indexing. This path is triggered automatically
when the remoteproc crashes (real-world trigger, not obscure).
**Step 5.5 - Similar Patterns:**
Record: Verified the file contains only this one use of the
`subsystems[]` array. Checked `minidump_id` values in `qcom_q6v5_pas.c`:
values 3, 4, 5, 7, 20, 21, 22 are used. The 20/21/22 are all for
`sa8775p` CDSP1/GPDSP0/GPDSP1 resources.
## PHASE 6: CROSS-REFERENCING STABLE TREES
**Step 6.1 - Does Buggy Code Exist?**
Record: Verified by inspecting each stable branch:
| Stable | MAX_NUM_OF_SS=10 | minidump_id 20/21/22 | Bug triggers? |
|--------|------------------|----------------------|---------------|
| p-6.1 | Yes | No | No (harmless) |
| p-6.6 | Yes | Yes (3 entries) | **Yes** |
| p-6.12 | Yes | Yes (3 entries) | **Yes** |
| p-6.15 | Yes | Yes (3 entries) | **Yes** |
| p-6.16 | Yes | Yes (3 entries) | **Yes** |
| p-6.17 | Yes | Yes (3 entries) | **Yes** |
Stable 6.6.y through 6.17.y all have the buggy configuration. 6.1.y has
the limit but no triggering minidump_ids, so safe.
**Step 6.2 - Backport Complications:**
Record: Diffed `qcom_common.c` across p-6.12, p-6.15, p-6.17 - file is
identical across these stable trees. Fix should apply cleanly. For p-6.6
the surrounding code is functionally identical. `qcom_smem_get()`
signature with `size_t *size` parameter is unchanged across all trees
(verified in p-6.6).
**Step 6.3 - Related Fixes in Stable:**
Record: No prior fix for this specific issue exists in stable trees; the
MAX_NUM_OF_SS limit has been 10 since minidump was added.
## PHASE 7: SUBSYSTEM CONTEXT
**Step 7.1 - Subsystem:**
Record: `drivers/remoteproc/qcom_common.c` - PERIPHERAL (Qualcomm-
specific remoteproc common helpers, affects only Qualcomm SoC users,
primarily SA8775p automotive).
**Step 7.2 - Activity:**
Record: Actively maintained subsystem with recent activity.
## PHASE 8: IMPACT AND RISK
**Step 8.1 - Affected Users:**
Record: Driver-specific - SA8775p SoC users (Qualcomm automotive
platform) using remoteproc/minidump for CDSP1, GPDSP0, or GPDSP1
subsystems.
**Step 8.2 - Trigger:**
Record: Triggered when a SA8775p CDSP1/GPDSP0/GPDSP1 remoteproc crashes
and `rproc_boot_recovery()` invokes the coredump callback. Cannot be
triggered by unprivileged users directly; occurs on hardware/firmware-
induced remoteproc crashes.
**Step 8.3 - Failure Severity:**
Record: With `MAX_NUM_OF_SS=10` and `minidump_id=20`,
`&toc->subsystems[20]` reads past the declared struct end. On SA8775p
the firmware's SMEM item is sized for 23+ entries, so reads land within
SMEM but access memory not described by the Linux struct (UBSAN would
flag this). With a UBSAN-enabled kernel → WARN/report. Without UBSAN →
reads memory bytes 320+ from struct start, potentially interprets them
as `status`/`enabled`/`regions_baseptr`. If `regions_baseptr` has non-
zero bits from surrounding SMEM, the code proceeds to
`qcom_add_minidump_segments()` which calls `ioremap()` with attacker-
uncontrolled but wrong values, and `rproc_coredump_add_custom_segment`
with garbage `da`/`size`. Severity: **MEDIUM-HIGH** - not a routine
crash but a real OOB in crash-recovery path that can cause incorrect
behavior, broken minidump collection, and potentially ioremap of wrong
addresses.
**Step 8.4 - Risk-Benefit:**
Record:
- Benefit: Fixes real OOB on SA8775p automotive SoC users; enables
correct minidump collection on these subsystems.
- Risk: Very low - fix is +14/-3 lines, surgical, only affects the path
that was broken; does not change behavior for valid minidump_ids (<
real count); doesn't alter the SMEM overlay layout (flexible array
occupies the same offset as the previous fixed array).
- Assessment: Positive risk-benefit.
## PHASE 9: FINAL SYNTHESIS
**Step 9.1 - Evidence:**
For:
- Real OOB bug reachable from remoteproc crash path on SA8775p
- Verified triggering config (minidump_id 20/21/22) is present in stable
trees 6.6.y–6.17.y
- Small, contained fix (single file, single function, ~17 lines)
- Reviewed and Acked by subsystem contributor
- Author explicitly considered it stable-worthy
- Applied cleanly across stable trees (file identical across
6.12/6.15/6.17)
- Fix doesn't change behavior for existing working configurations
- Addresses exact concern raised by maintainer Bjorn in v1 review
Against:
- No `Cc: stable` tag on final patch
- No explicit `Fixes:` tag
- Only affects SA8775p users (peripheral driver)
- Crash-recovery path, not hot path
Unresolved: None material.
**Step 9.2 - Stable Rules Checklist:**
1. Obviously correct? Yes - proper size-derivation from SMEM + bounds
check.
2. Fixes real bug? Yes - OOB access verified via source inspection.
3. Important issue? Medium - OOB with potential for misbehavior in crash
recovery.
4. Small/contained? Yes - +14/-3 in one file.
5. No new features? Correct - pure bug fix.
6. Applies to stable? Yes - verified file identical in 6.12/6.15/6.17;
p-6.6 is compatible.
**Step 9.3 - Exceptions:**
Not a device ID/quirk/DT/build/doc exception; a standard bug fix.
**Step 9.4 - Decision:**
YES - this fixes a real, verifiable out-of-bounds memory access
reachable in a realistic path (remoteproc crash recovery on SA8775p). It
satisfies all stable kernel rules.
## Verification
- [Phase 1] Parsed commit message: confirmed Acked-by Konrad, SoB from
Bjorn Andersson, Link to lore; no stable/Fixes tag (expected).
- [Phase 2] Read the diff: verified `MAX_NUM_OF_SS` removed,
`subsystems[]` changed to flexible array, `qcom_smem_get()` now
requests `toc_size`, `num_ss` derived via `offsetof`, and bounds check
added.
- [Phase 3.1] Ran `git blame` on lines 30-35 of
`drivers/remoteproc/qcom_common.c`: confirmed `MAX_NUM_OF_SS`
originated from `8ed8485c4f056` (Siddharth Gupta, 2020-11-19).
- [Phase 3.1] `git describe --contains 8ed8485c4f056` →
`v5.11-rc1~148^2~10`: minidump feature introduced in v5.11.
- [Phase 3.3] `git log -S "minidump_id = 20"` → `9091225ba28c0` (SA8775p
support) in v6.12-rc1 (`git describe --contains`).
- [Phase 3.3] `git show 9091225ba28c0` confirmed minidump_id = 20, 21,
22 added for sa8775p CDSP1/GPDSP0/GPDSP1.
- [Phase 4.1] `b4 dig -c 743cfae79d245` → found at lore v2 URL; `-a`
shows only v2 in this series.
- [Phase 4.3] `b4 mbox` on v1 URL `20250808164417.4105659-1-...`:
retrieved v1 discussion; confirmed Bjorn objected to v1's naïve
constant bump and suggested the exact approach used in v2; author
asked about Cc: stable.
- [Phase 5.2] `grep "qcom_minidump"` in drivers/remoteproc: confirmed
single caller in `qcom_q6v5_pas.c:151` via
`qcom_pas_minidump_ops.coredump`.
- [Phase 5.2] `grep "coredump"` in remoteproc_core.c: confirmed
invocation from `rproc_boot_recovery()` line 1798.
- [Phase 5.5] `grep "minidump_id = "` in `qcom_q6v5_pas.c`: confirmed
values 3, 4, 5, 7, 20, 21, 22 - three are out-of-bounds for
MAX_NUM_OF_SS=10.
- [Phase 6.1] `git show p-6.X:drivers/remoteproc/qcom_common.c | grep
MAX_NUM_OF_SS`: all stable branches (6.1, 6.6, 6.12, 6.15, 6.16, 6.17)
have `MAX_NUM_OF_SS=10`.
- [Phase 6.1] `git show p-6.X:drivers/remoteproc/qcom_q6v5_pas.c | grep
-c "minidump_id = 2[012]"`: p-6.1=0, p-6.6=3, p-6.12=3, p-6.15=3,
p-6.16=3, p-6.17=3. So bug triggers in 6.6.y through 6.17.y.
- [Phase 6.2] Diffed `qcom_common.c` between p-6.12, p-6.15, p-6.17 -
files identical, fix should apply cleanly.
- [Phase 6.2] `git show p-6.6:drivers/soc/qcom/smem.c | grep
qcom_smem_get`: `qcom_smem_get(unsigned host, unsigned item, size_t
*size)` signature present.
- [Phase 6.2] Verified p-6.6 qcom_minidump function structure matches
mainline pre-fix version.
- [Phase 8.3] Confirmed callers of `qcom_minidump`: only
`qcom_pas_minidump`, invoked on remoteproc recovery - real reachable
path.
- UNVERIFIED: Could not empirically reproduce UBSAN output or observe a
crash; severity assessment based on code reading and standard C
semantics.
**YES**
drivers/remoteproc/qcom_common.c | 17 ++++++++++++++---
1 file changed, 14 insertions(+), 3 deletions(-)
diff --git a/drivers/remoteproc/qcom_common.c b/drivers/remoteproc/qcom_common.c
index 6c31140268acb..fd2b6824ad265 100644
--- a/drivers/remoteproc/qcom_common.c
+++ b/drivers/remoteproc/qcom_common.c
@@ -28,7 +28,6 @@
#define to_ssr_subdev(d) container_of(d, struct qcom_rproc_ssr, subdev)
#define to_pdm_subdev(d) container_of(d, struct qcom_rproc_pdm, subdev)
-#define MAX_NUM_OF_SS 10
#define MAX_REGION_NAME_LENGTH 16
#define SBL_MINIDUMP_SMEM_ID 602
#define MINIDUMP_REGION_VALID ('V' << 24 | 'A' << 16 | 'L' << 8 | 'I' << 0)
@@ -80,7 +79,7 @@ struct minidump_global_toc {
__le32 status;
__le32 md_revision;
__le32 enabled;
- struct minidump_subsystem subsystems[MAX_NUM_OF_SS];
+ struct minidump_subsystem subsystems[];
};
struct qcom_ssr_subsystem {
@@ -151,9 +150,11 @@ void qcom_minidump(struct rproc *rproc, unsigned int minidump_id,
int ret;
struct minidump_subsystem *subsystem;
struct minidump_global_toc *toc;
+ unsigned int num_ss;
+ size_t toc_size;
/* Get Global minidump ToC*/
- toc = qcom_smem_get(QCOM_SMEM_HOST_ANY, SBL_MINIDUMP_SMEM_ID, NULL);
+ toc = qcom_smem_get(QCOM_SMEM_HOST_ANY, SBL_MINIDUMP_SMEM_ID, &toc_size);
/* check if global table pointer exists and init is set */
if (IS_ERR(toc) || !toc->status) {
@@ -161,6 +162,16 @@ void qcom_minidump(struct rproc *rproc, unsigned int minidump_id,
return;
}
+ /* Derive the number of subsystems from the actual SMEM item size */
+ num_ss = (toc_size - offsetof(struct minidump_global_toc, subsystems)) /
+ sizeof(struct minidump_subsystem);
+
+ if (minidump_id >= num_ss) {
+ dev_err(&rproc->dev, "Minidump id %d is out of range: %d\n",
+ minidump_id, num_ss);
+ return;
+ }
+
/* Get subsystem table of contents using the minidump id */
subsystem = &toc->subsystems[minidump_id];
--
2.53.0
^ permalink raw reply related
* Re: [PATCH v3 1/5] dt-bindings: phy: qcom,sc8280xp-qmp-pcie-phy: Add support for glymur Gen5 x8 bifurcation mode
From: Krzysztof Kozlowski @ 2026-04-20 13:23 UTC (permalink / raw)
To: Qiang Yu
Cc: Vinod Koul, Neil Armstrong, Rob Herring, Krzysztof Kozlowski,
Conor Dooley, Philipp Zabel, Bjorn Andersson, Konrad Dybcio,
linux-arm-msm, linux-phy, devicetree, linux-kernel
In-Reply-To: <aeXUZ1uSEJ9InUtw@hu-qianyu-lv.qualcomm.com>
On Mon, Apr 20, 2026 at 12:23:19AM -0700, Qiang Yu wrote:
> On Fri, Apr 17, 2026 at 11:18:08AM +0200, Krzysztof Kozlowski wrote:
> > On Wed, Apr 15, 2026 at 07:58:13PM -0700, Qiang Yu wrote:
> > > On Wed, Apr 15, 2026 at 09:50:28AM +0200, Krzysztof Kozlowski wrote:
> > > > On Sun, Apr 12, 2026 at 11:25:56PM -0700, Qiang Yu wrote:
> > > > > The Glymur SoC has pcie3a and pcie3b PHYs that can operate in two modes:
> > > > >
> > > > > 1. Independent 4-lane mode: Each PHY operates as a separate PCIe Gen5
> > > > > 4-lane interface, compatible with qcom,glymur-qmp-gen5x4-pcie-phy
> > > > > 2. Bifurcation mode (8-lane): pcie3a phy acts as leader and pcie3b phy as
> > > > > follower to form a single 8-lane PCIe Gen5 interface
> > > > >
> > > > > In bifurcation mode, the hardware design requires controlling additional
> > > > > resources beyond the standard pcie3a PHY configuration:
> > > > >
> > > > > - pcie3b's aux_clk (phy_b_aux)
> > > > > - pcie3b's phy_gdsc power domain
> > > > > - pcie3b's bcr/nocsr reset
> > > > >
> > > > > Add qcom,glymur-qmp-gen5x8-pcie-phy compatible string to document this
> > > > > 8-lane bifurcation configuration.
> > > >
> > > > Do you describe PCI3A or PCI3B or something combined PCI3?
> > >
> > > I describe a single x8 PHY with resources from both the pcie3a and pcie3b
> > > PHY blocks for x8 operation.
> > >
> > > >
> > > > >
> > > > > The phy_b_aux clock is used as the 6th clock instead of pipediv2,
> > > > > requiring the clock-names enum to be extended to support both
> > > > > [phy_b_aux, pipediv2] options at index 5. This follows the existing
> > > > > pattern used for [rchng, refgen] clocks at index 3.
> > > > >
> > > > > Signed-off-by: Qiang Yu <qiang.yu@oss.qualcomm.com>
> > > > > ---
> > > > > .../bindings/phy/qcom,sc8280xp-qmp-pcie-phy.yaml | 53 ++++++++++++++++++----
> > > > > 1 file changed, 45 insertions(+), 8 deletions(-)
> > > > >
> > > > > diff --git a/Documentation/devicetree/bindings/phy/qcom,sc8280xp-qmp-pcie-phy.yaml b/Documentation/devicetree/bindings/phy/qcom,sc8280xp-qmp-pcie-phy.yaml
> > > > > index 3a35120a77ec0ceb814a1cdcacff32fef32b4f7b..14eba5d705b1956c1bb00cc8c95171ed6488299b 100644
> > > > > --- a/Documentation/devicetree/bindings/phy/qcom,sc8280xp-qmp-pcie-phy.yaml
> > > > > +++ b/Documentation/devicetree/bindings/phy/qcom,sc8280xp-qmp-pcie-phy.yaml
> > > > > @@ -18,6 +18,7 @@ properties:
> > > > > enum:
> > > > > - qcom,glymur-qmp-gen4x2-pcie-phy
> > > > > - qcom,glymur-qmp-gen5x4-pcie-phy
> > > > > + - qcom,glymur-qmp-gen5x8-pcie-phy
> > > >
> > > > That's the same device as 5x4, no? One device, one compatible and this
> > > > suggests you will have three PCI phys in the DT - two 5x4 and one 5x8?
> > > >
> > >
> > > It is not the same as the 5x4 PHY. In DT, we model three PHY nodes:
> > > phy_3a (1x4), phy_3b (1x4), and a separate phy_1x8 node for x8 mode.
> >
> > OK, that's what I wanted to hear. And that's what should not be done,
> >
> > You should not have a separate node for the same hardware. First, DTC
> > will give you a W=1 warning, although warning itself should be moved to
> > W=2.
> >
> > Second, the warning tells important story - same hardware is described
> > twice.
> >
> > You only need phy_3a and phy_3b, so only two in total.
>
> We can keep only phy_3a and phy_3b, but still add new compatible
> qcom,glymur-qmp-gen5x8-pcie-phy in binding, right?
>
> For boards that support pcie3a(1x4) + pcie3b(1x4), DTS would be:
>
> pcie3a_phy { compatible = "qcom,glymur-qmp-gen5x4-pcie-phy"; };
> pcie3b_phy { compatible = "qcom,glymur-qmp-gen5x4-pcie-phy"; };
>
> For boards that support 1x8, we would override pcie3a_phy with:
>
> pcie3a_phy { compatible = "qcom,glymur-qmp-gen5x8-pcie-phy"; /* additional resources */ };
> pcie3b_phy { compatible = "qcom,glymur-qmp-gen5x4-pcie-phy"; };
>
> This still uses only two PHY nodes and DTC will not report warning.
IMO, you do not need another compatible. Device is exactly the same. If
wiring on the board differs, e.g. you have 8x instead of 4x, you:
1. disable unused 3B phy.
2. Add to 3A missing resources or the phandle to companion node.
At least that is what I tought till now, when I opened the HPG/manual
for Glymur phy. Someone skipped important information when PCIe PHY was
upstreamed first and glymur.dtsi already got PHY 3B described.
Reminder: writing bindings asks you explicitly to post COMPLETE
bindings.
If you posted COMPLETE bindings we would question all this and you would
have to check in user manual that this is actually ONE device.
There is no 5x4 phy 3A and 3B, at least HPG is pretty clear here.
And you should start with that.
But you posted first incomplete binding, hiding the rest and now you
have 5x4 merged into DTSI.
So let's rephrase based on manual:
You have only one PCIE phy3. Not 3A + 3B. That one phy3 can be
configured by consumers (board) differently, e.g. by requesting 8-lane
or twice 4-lane phys.
Let me send correction note for glymur.dtsi.
Best regards,
Krzysztof
^ permalink raw reply
* [PATCH AUTOSEL 6.18] arm64: dts: qcom: monaco: Reserve full Gunyah metadata region
From: Sasha Levin @ 2026-04-20 13:17 UTC (permalink / raw)
To: patches, stable
Cc: Loic Poulain, Konrad Dybcio, Dmitry Baryshkov, Bjorn Andersson,
Sasha Levin, konradybcio, robh, krzk+dt, conor+dt, quic_jingyw,
linux-arm-msm, devicetree, linux-kernel
In-Reply-To: <20260420132314.1023554-1-sashal@kernel.org>
From: Loic Poulain <loic.poulain@oss.qualcomm.com>
[ Upstream commit 85d98669fa7f1d3041d962515e45ee6e392db6f8 ]
We observe spurious "Synchronous External Abort" exceptions
(ESR=0x96000010) and kernel crashes on Monaco-based platforms.
These faults are caused by the kernel inadvertently accessing
hypervisor-owned memory that is not properly marked as reserved.
>From boot log, The Qualcomm hypervisor reports the memory range
at 0x91a80000 of size 0x80000 (512 KiB) as hypervisor-owned:
qhee_hyp_assign_remove_memory: 0x91a80000/0x80000 -> ret 0
However, the EFI memory map provided by firmware only reserves the
subrange 0x91a40000–0x91a87fff (288 KiB). The remaining portion
(0x91a88000–0x91afffff) is incorrectly reported as conventional
memory (from efi debug):
efi: 0x000091a40000-0x000091a87fff [Reserved...]
efi: 0x000091a88000-0x0000938fffff [Conventional...]
As a result, the allocator may hand out PFNs inside the hypervisor
owned region, causing fatal aborts when the kernel accesses those
addresses.
Add a reserved-memory carveout for the Gunyah hypervisor metadata
at 0x91a80000 (512 KiB) and mark it as no-map so Linux does not
map or allocate from this area.
For the record:
Hyp version: gunyah-e78adb36e debug (2025-11-17 05:38:05 UTC)
UEFI Ver: 6.0.260122.BOOT.MXF.1.0.c1-00449-KODIAKLA-1
Fixes: 7be190e4bdd2 ("arm64: dts: qcom: add QCS8300 platform")
Signed-off-by: Loic Poulain <loic.poulain@oss.qualcomm.com>
Reviewed-by: Konrad Dybcio <konrad.dybcio@oss.qualcomm.com>
Reviewed-by: Dmitry Baryshkov <dmitry.baryshkov@oss.qualcomm.com>
Link: https://lore.kernel.org/r/20260302142603.1113355-1-loic.poulain@oss.qualcomm.com
Signed-off-by: Bjorn Andersson <andersson@kernel.org>
Signed-off-by: Sasha Levin <sashal@kernel.org>
---
LLM Generated explanations, may be completely bogus:
Error: Failed to generate final synthesis
arch/arm64/boot/dts/qcom/qcs8300.dtsi | 5 +++++
1 file changed, 5 insertions(+)
diff --git a/arch/arm64/boot/dts/qcom/qcs8300.dtsi b/arch/arm64/boot/dts/qcom/qcs8300.dtsi
index b8d4a75baee22..7a4c3e872d8ee 100644
--- a/arch/arm64/boot/dts/qcom/qcs8300.dtsi
+++ b/arch/arm64/boot/dts/qcom/qcs8300.dtsi
@@ -756,6 +756,11 @@ smem_mem: smem@90900000 {
hwlocks = <&tcsr_mutex 3>;
};
+ gunyah_md_mem: gunyah-md-region@91a80000 {
+ reg = <0x0 0x91a80000 0x0 0x80000>;
+ no-map;
+ };
+
lpass_machine_learning_mem: lpass-machine-learning-region@93b00000 {
reg = <0x0 0x93b00000 0x0 0xf00000>;
no-map;
--
2.53.0
^ permalink raw reply related
* [PATCH AUTOSEL 7.0-6.18] firmware: qcom: scm: Allow QSEECOM on Lenovo IdeaCentre Mini X
From: Sasha Levin @ 2026-04-20 13:17 UTC (permalink / raw)
To: patches, stable
Cc: Bjorn Andersson, Dmitry Baryshkov, Bjorn Andersson, Sasha Levin,
konradybcio, linux-arm-msm, linux-kernel
In-Reply-To: <20260420132314.1023554-1-sashal@kernel.org>
From: Bjorn Andersson <bjorn.andersson@oss.qualcomm.com>
[ Upstream commit a31ad9339eff4ce401dec816b01a94b4e3c47898 ]
The Hamoa-based Lenovo IdeaCentre Mini X provides the same UEFI variable
access through uefisecapp as other Hamoa devices, add it to the
allowlist.
Reviewed-by: Dmitry Baryshkov <dmitry.baryshkov@oss.qualcomm.com>
Signed-off-by: Bjorn Andersson <bjorn.andersson@oss.qualcomm.com>
Link: https://lore.kernel.org/r/20260401-ideacentre-v2-3-5745fe2c764e@oss.qualcomm.com
Signed-off-by: Bjorn Andersson <andersson@kernel.org>
Signed-off-by: Sasha Levin <sashal@kernel.org>
---
LLM Generated explanations, may be completely bogus:
Now I have a thorough picture. Let me compile the analysis.
---
## PHASE 1: COMMIT MESSAGE FORENSICS
**Step 1.1: Subject Line**
- Subsystem: `firmware: qcom: scm`
- Action verb: "Allow" (enable a device on an allowlist)
- Summary: Adds the Lenovo IdeaCentre Mini X to the QSEECOM allowlist
for UEFI variable access.
**Step 1.2: Tags**
- `Reviewed-by: Dmitry Baryshkov` — Qualcomm maintainer reviewed it
- `Signed-off-by: Bjorn Andersson` — both as author and as committer;
he's the Qualcomm SCM subsystem maintainer
- `Link:` to lore.kernel.org for the v2 series
- No `Fixes:` tag (expected for a device ID / allowlist addition)
- No `Cc: stable`
**Step 1.3: Commit Body**
The commit explains this is a Hamoa-based device that provides the same
UEFI variable access (through `uefisecapp`) as other Hamoa devices
already on the allowlist. The purpose is to enable UEFI variable access
on this specific hardware.
**Step 1.4: Hidden Bug Fix?**
This is not a hidden bug fix. It's a straightforward device allowlist
addition — enabling existing, validated functionality on a new machine.
Record: This is a device allowlist addition, akin to adding a device ID.
## PHASE 2: DIFF ANALYSIS
**Step 2.1: Inventory**
- 1 file changed: `drivers/firmware/qcom/qcom_scm.c`
- 1 line added: `{ .compatible = "lenovo,ideacentre-mini-01q8x10" }`
- No lines removed (other than context shift)
- Function affected: static array `qcom_scm_qseecom_allowlist[]`
**Step 2.2: Code Flow Change**
Before: The Lenovo IdeaCentre Mini X is not in the allowlist →
`qcom_scm_qseecom_machine_is_allowed()` returns false →
QSEECOM/uefisecapp is not initialized → no UEFI variable access on this
device.
After: The device is matched → QSEECOM is enabled → UEFI variable access
works.
**Step 2.3: Bug Mechanism**
Category: Hardware enablement / device ID addition. This is analogous to
adding a PCI/USB device ID to an existing driver. The driver and feature
exist; only the allowlist entry is new.
**Step 2.4: Fix Quality**
- Trivially correct — a single compatible string added alphabetically to
an existing table
- Zero regression risk — only affects this specific machine
- Minimal change (1 line)
## PHASE 3: GIT HISTORY
**Step 3.1: Blame**
The allowlist was introduced by `00b1248606ba39` (Maximilian Luz,
2023-08-27) with just ThinkPad X13s. It has been incrementally expanded
since then with many similar one-line additions. The table has existed
since approximately v6.7.
**Step 3.2: No Fixes: tag** — expected for allowlist additions.
**Step 3.3: File History**
Many similar QSEECOM allowlist additions have been committed: Dell XPS
13, Surface Pro, HP Omnibook, ASUS Vivobook, Lenovo Thinkbook 16, etc.
This is a well-established pattern. Each is a standalone one-liner.
**Step 3.4: Author**
Bjorn Andersson is the Qualcomm SCM subsystem maintainer and the main
committer for this driver. High trust level.
**Step 3.5: Dependencies**
No dependencies. This is a self-contained one-line addition to an
existing table.
## PHASE 4: MAILING LIST
b4 dig failed to find the commit (it's not yet in the tree as a
committed hash). The lore link was blocked by anti-bot protection.
However, the Link: tag shows this is v2 of the series, reviewed by
Dmitry Baryshkov, confirming proper review.
Record: Could not fetch lore discussion due to anti-scraping protection.
The commit has proper review tags.
## PHASE 5: CODE SEMANTIC ANALYSIS
The allowlist is checked by `qcom_scm_qseecom_machine_is_allowed()`
which is called during `qcom_scm_qseecom_init()`. This function gates
whether the QSEECOM/uefisecapp platform device is created. Without the
allowlist entry, the specific machine gets an "untested machine,
skipping" message and UEFI variable access is completely unavailable.
This is critical for the device: without this entry, the user cannot
access UEFI variables at all (including boot configuration).
## PHASE 6: STABLE TREE ANALYSIS
**Step 6.1: Code existence in stable**
- v6.6: The allowlist does NOT exist (0 matches) → Not applicable
- v6.8+: The allowlist exists (3 matches found)
- The allowlist has been present and growing since approximately
v6.7/v6.8
The commit is relevant for stable trees v6.8+. However, the current file
in mainline has additional entries that may not all be in older stable
trees. The diff should apply cleanly or with trivial context adjustment
since it's just inserting one line in alphabetical order in a table.
**Step 6.2: Backport complications**
Minor context differences possible (some entries in the list may not be
present in older stables), but since this is adding a line to a simple
table, a backport is trivial — worst case requires manual insertion at
the right alphabetical position.
## PHASE 7: SUBSYSTEM CONTEXT
- Subsystem: Qualcomm firmware / SCM (Secure Channel Manager)
- Criticality: IMPORTANT for Qualcomm ARM laptop users. The QSEECOM
interface provides UEFI variable access, which is essential for boot
configuration on these devices.
- Very active subsystem with regular allowlist additions.
## PHASE 8: IMPACT AND RISK ASSESSMENT
**Step 8.1: Affected Users**
Users of the Lenovo IdeaCentre Mini X (Snapdragon/Hamoa-based). Without
this entry, they have no UEFI variable access.
**Step 8.2: Trigger**
Every boot — the allowlist is checked during driver initialization.
**Step 8.3: Severity**
Without this fix: the user cannot access UEFI variables on their
hardware. This is a functional hardware enablement issue.
**Step 8.4: Risk-Benefit**
- Benefit: HIGH — enables essential functionality on real hardware
- Risk: VERY LOW — 1-line addition to a device table, affects only one
specific machine
- Ratio: Excellent — high benefit, near-zero risk
## PHASE 9: FINAL SYNTHESIS
**Evidence FOR backporting:**
- This is a device allowlist addition — directly analogous to adding a
device ID, which is an established exception category for stable
- 1-line change, trivially correct, zero regression risk
- Written and committed by the subsystem maintainer (Bjorn Andersson)
- Reviewed by Qualcomm maintainer (Dmitry Baryshkov)
- Follows a well-established pattern (20+ similar commits in history)
- Enables essential UEFI variable access on real shipping hardware
**Evidence AGAINST backporting:**
- The allowlist doesn't exist in v6.6 and earlier (limits applicability
to v6.8+ stable trees)
- This is technically "new hardware enablement" rather than a "bug fix"
**Decision:** This falls squarely into the **device ID / hardware
allowlist addition** exception category. It's a single-line addition to
an existing allowlist table in an existing driver, enabling known-good
functionality on a new machine. The pattern is identical to adding
PCI/USB device IDs. It has proper review, is written by the maintainer,
and carries essentially zero risk.
## Verification
- [Phase 1] Parsed tags: Reviewed-by Dmitry Baryshkov, Signed-off-by
Bjorn Andersson (2x — author and committer)
- [Phase 2] Diff: 1 line added to `qcom_scm_qseecom_allowlist[]` array —
a compatible string
- [Phase 3] git blame: allowlist created by commit `00b1248606ba39`
(v6.7 era, 2023-08-27)
- [Phase 3] git tag --contains: allowlist first in `next-20251219`, not
in v6.6
- [Phase 3] v6.8 check: allowlist exists with 3 references (confirmed
with `git show v6.8:...`)
- [Phase 3] git log: 20+ similar QSEECOM allowlist additions in history
(established pattern)
- [Phase 4] b4 dig: failed to find commit (not yet in tree); lore
blocked by anti-bot
- [Phase 5] Allowlist checked in `qcom_scm_qseecom_machine_is_allowed()`
→ gates QSEECOM init → without entry, device prints "untested machine,
skipping" and UEFI access is disabled
- [Phase 6] Applicable to v6.8+ stable trees only; trivial backport
(table insertion)
- [Phase 8] Risk: VERY LOW (1 line, only affects one machine); Benefit:
HIGH (enables UEFI access)
- UNVERIFIED: Exact lore discussion content (anti-bot blocked); does not
affect decision
**YES**
drivers/firmware/qcom/qcom_scm.c | 1 +
1 file changed, 1 insertion(+)
diff --git a/drivers/firmware/qcom/qcom_scm.c b/drivers/firmware/qcom/qcom_scm.c
index f06bed79666b9..4450741b8221b 100644
--- a/drivers/firmware/qcom/qcom_scm.c
+++ b/drivers/firmware/qcom/qcom_scm.c
@@ -2301,6 +2301,7 @@ static const struct of_device_id qcom_scm_qseecom_allowlist[] __maybe_unused = {
{ .compatible = "hp,omnibook-x14" },
{ .compatible = "huawei,gaokun3" },
{ .compatible = "lenovo,flex-5g" },
+ { .compatible = "lenovo,ideacentre-mini-01q8x10" },
{ .compatible = "lenovo,thinkbook-16" },
{ .compatible = "lenovo,thinkpad-t14s" },
{ .compatible = "lenovo,thinkpad-x13s", },
--
2.53.0
^ permalink raw reply related
* [PATCH AUTOSEL 7.0-5.10] ASoC: codecs: wcd-clsh: Always update buck/flyback on transitions on transitions
From: Sasha Levin @ 2026-04-20 13:17 UTC (permalink / raw)
To: patches, stable
Cc: Cédric Bellegarde, Mark Brown, Sasha Levin, srini, lgirdwood,
perex, tiwai, linux-sound, linux-arm-msm, linux-kernel
In-Reply-To: <20260420132314.1023554-1-sashal@kernel.org>
From: Cédric Bellegarde <cedric.bellegarde@adishatz.org>
[ Upstream commit f8d51e903a6c97d8d298f14d9f8b4fff808670e3 ]
The WCD934x audio outputs (earpiece, headphone, speaker) share two power
supply converters, a buck and a flyback, managed by reference counters
(buck_users, flyback_users) in the Class-H controller.
The early return in wcd_clsh_ctrl_set_state() when nstate == ctrl->state
prevented _wcd_clsh_ctrl_set_state() from being called when switching
between outputs sharing the same state value. As a result, the buck and
flyback reference counters were never decremented on disable, leaving the
converters active and their counters out of sync with the actual hardware
state.
This caused audible distortion on the earpiece output and spurious MBHC
over-current protection interrupts on HPHL/HPHR during output switching.
Remove the early return so that CLSH_REQ_ENABLE and CLSH_REQ_DISABLE are
always dispatched, keeping the buck and flyback reference counters
consistent on every state transition.
Signed-off-by: Cédric Bellegarde <cedric.bellegarde@adishatz.org>
Link: https://patch.msgid.link/20260304141006.280894-1-cedric.bellegarde@adishatz.org
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 I need. Let me compile the full analysis.
---
## PHASE 1: COMMIT MESSAGE FORENSICS
### Step 1.1: Subject Line
- **Subsystem:** ASoC: codecs: wcd-clsh
- **Action verb:** "Always update" (implies something was incorrectly
not being updated = bug fix)
- **Summary:** Remove an incorrect early return that prevented
buck/flyback power supply reference counters from being updated during
audio output transitions.
### Step 1.2: Tags
- **Signed-off-by:** Cédric Bellegarde (author)
- **Link:** https://patch.msgid.link/20260304141006.280894-1-
cedric.bellegarde@adishatz.org
- **Signed-off-by:** Mark Brown (ASoC maintainer, applied the patch)
- No Fixes: tag, no Reported-by, no Cc: stable — all expected for
candidate review.
### Step 1.3: Commit Body Analysis
The commit message is detailed and clearly explains:
- **Bug:** Early return in `wcd_clsh_ctrl_set_state()` when `nstate ==
ctrl->state` prevented `_wcd_clsh_ctrl_set_state()` from being called
during disable transitions.
- **Root cause:** Each audio output (earpiece, HPHL, HPHR) calls
`set_state` with the same `nstate` for both enable (PRE_DAC) and
disable (POST_PA). The early return silently skips the disable call.
- **Symptom:** Buck/flyback reference counters never decremented →
converters left active → audible distortion on earpiece + spurious
MBHC over-current interrupts on HPHL/HPHR.
- **Fix:** Remove the 3-line early return.
### Step 1.4: Hidden Bug Fix?
No — this is explicitly described as a bug fix with clear user-visible
symptoms. The commit message thoroughly explains the bug mechanism.
---
## PHASE 2: DIFF ANALYSIS
### Step 2.1: Inventory
- **Files changed:** 1 (`sound/soc/codecs/wcd-clsh-v2.c`)
- **Lines:** -3, +0 (pure deletion)
- **Function modified:** `wcd_clsh_ctrl_set_state()`
- **Scope:** Single-file, surgical fix
### Step 2.2: Code Flow Change
**Before:** When `nstate == ctrl->state`, the function returns
immediately without calling `_wcd_clsh_ctrl_set_state()`. This means
neither CLSH_REQ_ENABLE nor CLSH_REQ_DISABLE is dispatched.
**After:** The function always proceeds to the switch on `clsh_event`,
dispatching either CLSH_REQ_ENABLE or CLSH_REQ_DISABLE to
`_wcd_clsh_ctrl_set_state()`.
### Step 2.3: Bug Mechanism
This is a **reference counting bug**. Looking at the actual call pattern
in wcd934x.c:
1. **Enable EAR** (PRE_PMU): `set_state(ctrl, PRE_DAC,
WCD_CLSH_STATE_EAR, CLS_H_NORMAL)` → state=EAR, buck_users++,
flyback_users++
2. **Disable EAR** (POST_PMD): `set_state(ctrl, POST_PA,
WCD_CLSH_STATE_EAR, CLS_H_NORMAL)` → nstate=EAR == ctrl->state=EAR →
**EARLY RETURN!** Buck/flyback never decremented.
The same pattern affects ALL outputs (HPHL, HPHR, LO, AUX) across ALL
WCD codec drivers (wcd9335, wcd934x, wcd937x, wcd938x, wcd939x).
### Step 2.4: Fix Quality
- **Obviously correct:** Yes. The early return was clearly wrong — the
function uses `clsh_event` (enable vs disable) to dispatch different
operations, and the early return bypasses this dispatch.
- **Minimal/surgical:** Maximum surgical — 3-line deletion.
- **Regression risk:** Very low. The removed check was a premature
optimization that incorrectly assumed same nstate means no-op. The
`_wcd_clsh_ctrl_set_state` sub-functions use reference counting
(buck_users, flyback_users) which already handles idempotency
correctly.
---
## PHASE 3: GIT HISTORY INVESTIGATION
### Step 3.1: Blame
The buggy early return at line 851-852 was introduced in commit
`cc2e324d39b26` ("ASoC: wcd9335: add CLASS-H Controller support") by
Srinivas Kandagatla, merged in **v5.1-rc1**. This code has been present
since the initial creation of the file.
### Step 3.2: Fixes Tag
No Fixes: tag present. However, the bug was clearly introduced by
`cc2e324d39b26` (v5.1-rc1).
### Step 3.3: File History
9 commits to `wcd-clsh-v2.c` since initial creation. Changes have been
minor: unused function removal, new codec version support, symbol
renaming, GENMASK fixes. No prior fix to this early return logic.
### Step 3.4: Author
Cédric Bellegarde has one other commit in the tree (ASoC: qcom: q6asm:
drop DSP responses for closed data streams). Not the subsystem
maintainer, but the patch was accepted by Mark Brown (ASoC maintainer).
### Step 3.5: Dependencies
None. This is a standalone 3-line deletion with no dependencies on other
patches.
---
## PHASE 4: MAILING LIST RESEARCH
### Step 4.1: Original Discussion
Found via web search at yhbt.net/lore mirror. The patch was submitted on
2026-03-04 and applied by Mark Brown on 2026-03-16 to `broonie/sound
for-7.1` (commit `f8d51e903a6c`).
### Step 4.2: Reviewer Feedback
Mark Brown applied directly with no review comments or objections — a
clean accept from the ASoC subsystem maintainer. No NAKs or concerns
raised.
### Step 4.3: Bug Report
No separate bug report; the author discovered this through direct
debugging (audio distortion and spurious interrupts during output
switching).
### Step 4.4: Series Context
Single standalone patch, not part of any series.
### Step 4.5: Stable Discussion
No stable-specific discussion found.
---
## PHASE 5: CODE SEMANTIC ANALYSIS
### Step 5.1: Modified Function
`wcd_clsh_ctrl_set_state()` — exported function, the main API for the
Class-H controller.
### Step 5.2: Callers
`wcd_clsh_ctrl_set_state()` is called from **5 different WCD codec
drivers**:
- `wcd9335.c` — 8 call sites (EAR, HPHL, HPHR, LO)
- `wcd934x.c` — 8 call sites (EAR, HPHL, HPHR, LO)
- `wcd937x.c` — 8 call sites (EAR, HPHL, HPHR, AUX)
- `wcd938x.c` — 10 call sites (EAR, HPHL, HPHR, AUX)
- `wcd939x.c` — 6 call sites (EAR, HPHL, HPHR)
All follow the same pattern: PRE_DAC enable on PMU, POST_PA disable on
PMD.
### Step 5.3-5.4: Call Chain
These are called from DAPM widget event handlers, triggered during
normal audio routing changes. Every user who plays audio through
earpiece, headphones, or speaker on a Qualcomm WCD93xx-based device
triggers this code path.
### Step 5.5: Similar Patterns
The reference counting pattern in `wcd_clsh_buck_ctrl()` and
`wcd_clsh_flyback_ctrl()` (and v3 variants) all use increment-on-
enable/decrement-on-disable with `buck_users`/`flyback_users`. The early
return prevented the decrement path from ever executing.
---
## PHASE 6: STABLE TREE ANALYSIS
### Step 6.1: Buggy Code in Stable
The bug was introduced in `cc2e324d39b26` (v5.1-rc1). This code exists
in **all active stable trees**: 5.10.y, 5.15.y, 6.1.y, 6.6.y, 6.12.y,
and any other LTS/stable branches.
### Step 6.2: Backport Complications
The file has had only minor changes (renaming, cleanup). The patch is a
simple 3-line deletion that should apply cleanly to all stable trees.
### Step 6.3: Related Fixes
No prior fix for this issue in any stable tree.
---
## PHASE 7: SUBSYSTEM CONTEXT
### Step 7.1: Subsystem
- **Path:** sound/soc/codecs/
- **Subsystem:** ASoC (Audio System on Chip) — audio codec drivers
- **Criticality:** IMPORTANT — affects audio on all Qualcomm WCD93xx
codec-based phones and devices (many Android devices, some embedded
systems)
### Step 7.2: Activity
Moderately active subsystem with steady fixes and improvements.
---
## PHASE 8: IMPACT AND RISK ASSESSMENT
### Step 8.1: Affected Users
All users of Qualcomm WCD93xx-series audio codecs (WCD9335, WCD934x,
WCD937x, WCD938x, WCD939x). This includes many Android phones and
Qualcomm-based embedded systems.
### Step 8.2: Trigger Conditions
- **Trigger:** Any normal audio output switching (e.g., call on
earpiece, then play music through headphones) — extremely common
operation.
- **Unprivileged trigger:** Yes — any userspace audio playback triggers
this.
### Step 8.3: Failure Mode Severity
- **Audible distortion** on earpiece — MEDIUM-HIGH (user-perceivable
audio quality issue)
- **Spurious MBHC over-current interrupts** — MEDIUM (can cause spurious
headphone detection events)
- **Power supply reference counter leak** — MEDIUM (converters left
active, wasted power, potential for undefined hardware behavior)
- Overall severity: **HIGH** — directly affects audio functionality
### Step 8.4: Risk-Benefit Ratio
- **Benefit:** HIGH — fixes audio distortion and spurious interrupts on
a wide class of devices, bug present since v5.1
- **Risk:** VERY LOW — 3-line deletion, obviously correct, no regression
risk (the removed check was always wrong)
- **Ratio:** Very favorable for backporting
---
## PHASE 9: FINAL SYNTHESIS
### Step 9.1: Evidence Compilation
**FOR backporting:**
- Fixes a real, user-visible bug: audio distortion and spurious
interrupts
- Reference counting bug — buck/flyback users never decremented on
disable
- Affects ALL WCD93xx codec users (5 codec drivers, ~40 call sites)
- Bug present since v5.1-rc1 — affects all active stable trees
- Fix is minimal: 3-line deletion, obviously correct
- Applied by ASoC maintainer Mark Brown with no concerns
- No dependencies — standalone fix
- Will apply cleanly to all stable trees
**AGAINST backporting:**
- No Fixes: tag (expected — that's why it's under manual review)
- Author is not the subsystem maintainer
- No formal Reported-by (author found it themselves)
### Step 9.2: Stable Rules Checklist
1. **Obviously correct and tested?** YES — 3-line deletion, logic
clearly wrong, applied by maintainer
2. **Fixes a real bug?** YES — audio distortion, spurious interrupts,
reference counter leak
3. **Important issue?** YES — user-perceivable audio quality degradation
on common hardware
4. **Small and contained?** YES — 3 lines removed from 1 file, 1
function
5. **No new features or APIs?** YES — pure bug fix
6. **Can apply to stable?** YES — clean apply expected
### Step 9.3: Exception Categories
Not applicable — this is a straightforward bug fix, not an exception
category.
### Step 9.4: Decision
This is a clear, minimal fix for a real reference counting bug that
causes audible audio distortion on all Qualcomm WCD93xx codec-based
devices. The bug has existed since v5.1. The fix is a 3-line deletion
that is obviously correct and was accepted by the ASoC maintainer.
---
## Verification
- [Phase 1] Parsed tags: Signed-off-by (author + Mark Brown), Link to
patch.msgid.link
- [Phase 2] Diff analysis: 3 lines removed from
`wcd_clsh_ctrl_set_state()`, removes incorrect early return
- [Phase 2] Verified bug mechanism: PRE_DAC sets state=EAR, POST_PA
passes same nstate=EAR, early return prevents DISABLE dispatch
- [Phase 2] Verified reference counting: `wcd_clsh_buck_ctrl()` (line
182-183) and `wcd_clsh_flyback_ctrl()` (line 231-232) use
increment/decrement of `buck_users`/`flyback_users`
- [Phase 3] git blame: buggy code introduced in cc2e324d39b26 (v5.1-rc1)
— present since file creation
- [Phase 3] git log: only 9 commits to this file since creation, no
prior fix for this issue
- [Phase 4] Found original submission at yhbt.net lore mirror: single
patch, clean accept by Mark Brown for-7.1
- [Phase 4] No NAKs, no review concerns raised
- [Phase 5] Verified callers: 5 codec drivers (wcd9335, wcd934x,
wcd937x, wcd938x, wcd939x), ~40 call sites, all follow enable/disable
pattern
- [Phase 6] Code exists in all active stable trees (v5.1+)
- [Phase 6] File has minimal changes — clean apply expected
- [Phase 8] Failure mode: audio distortion + spurious interrupts,
severity HIGH
**YES**
sound/soc/codecs/wcd-clsh-v2.c | 3 ---
1 file changed, 3 deletions(-)
diff --git a/sound/soc/codecs/wcd-clsh-v2.c b/sound/soc/codecs/wcd-clsh-v2.c
index 13d07296916f6..62ca22ea0f3b6 100644
--- a/sound/soc/codecs/wcd-clsh-v2.c
+++ b/sound/soc/codecs/wcd-clsh-v2.c
@@ -848,9 +848,6 @@ int wcd_clsh_ctrl_set_state(struct wcd_clsh_ctrl *ctrl,
{
struct snd_soc_component *comp = ctrl->comp;
- if (nstate == ctrl->state)
- return 0;
-
if (!wcd_clsh_is_state_valid(nstate)) {
dev_err(comp->dev, "Class-H not a valid new state:\n");
return -EINVAL;
--
2.53.0
^ permalink raw reply related
* [PATCH AUTOSEL 7.0-6.1] drm/msm/dpu: fix vblank IRQ registration before atomic_mode_set
From: Sasha Levin @ 2026-04-20 13:18 UTC (permalink / raw)
To: patches, stable
Cc: Cédric Bellegarde, Dmitry Baryshkov, Sasha Levin,
robin.clark, lumag, airlied, simona, linux-arm-msm, dri-devel,
freedreno, linux-kernel
In-Reply-To: <20260420132314.1023554-1-sashal@kernel.org>
From: Cédric Bellegarde <cedric.bellegarde@adishatz.org>
[ Upstream commit 961c900628fef77ad07b4bc4c868e47b9a1269c7 ]
dpu_encoder_toggle_vblank_for_crtc() can call control_vblank_irq()
at any time in response to a userspace vblank request, independently
of the atomic commit sequence. If this happens before the encoder's
first atomic_mode_set(), irq[INTR_IDX_RDPTR] is still zero.
Passing irq_idx=0 to dpu_core_irq_register_callback() is treated as
invalid, and DPU_IRQ_REG(0) and DPU_IRQ_BIT(0) produce misleading
values of 134217727 and 31 respectively due to unsigned wraparound
in the (irq_idx - 1) macros, resulting in the confusing error:
[dpu error]invalid IRQ=[134217727, 31]
Since irq[INTR_IDX_RDPTR] will be properly populated by
atomic_mode_set() and registered by irq_enable() as part of the
normal modeset sequence, silently skip the vblank IRQ registration
when the index has not yet been initialized. This matches the
existing pattern of the master encoder check above it.
Signed-off-by: Cédric Bellegarde <cedric.bellegarde@adishatz.org>
Reviewed-by: Dmitry Baryshkov <dmitry.baryshkov@oss.qualcomm.com>
Patchwork: https://patchwork.freedesktop.org/patch/712752/
Link: https://lore.kernel.org/r/20260318171700.394945-1-cedric.bellegarde@adishatz.org
Signed-off-by: Dmitry Baryshkov <dmitry.baryshkov@oss.qualcomm.com>
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. Let me
synthesize the findings.
---
## PHASE 1: COMMIT MESSAGE FORENSICS
**Step 1.1**: Subject: `drm/msm/dpu:` subsystem prefix. Action verb:
`fix`. Summary: Fix vblank IRQ registration when called before
atomic_mode_set() initializes the IRQ index.
**Step 1.2**: Tags:
- `Signed-off-by: Cédric Bellegarde` (author)
- `Reviewed-by: Dmitry Baryshkov` (subsystem maintainer - strong quality
signal)
- `Patchwork: https://patchwork.freedesktop.org/patch/712752/`
- `Link: https://lore.kernel.org/r/20260318171700.394945-1-
cedric.bellegarde@adishatz.org`
- `Signed-off-by: Dmitry Baryshkov` (merged by maintainer)
- No Fixes: tag, no Cc: stable, no Reported-by. Absence of these tags is
expected.
**Step 1.3**: The commit body explains:
- Bug: `dpu_encoder_toggle_vblank_for_crtc()` can call
`control_vblank_irq()` at any time via a vblank workqueue,
independently of the atomic commit sequence.
- Root cause: Before the encoder's first `atomic_mode_set()`,
`irq[INTR_IDX_RDPTR]` is zero.
- Symptom: Passing irq_idx=0 to `dpu_core_irq_register_callback()`
produces confusing error: `[dpu error]invalid IRQ=[134217727, 31]` due
to unsigned wraparound in `(irq_idx - 1)` macros.
- Fix approach: Early return when irq index is 0, matching the existing
master encoder check pattern.
**Step 1.4**: This is explicitly labeled as a fix, not hidden.
## PHASE 2: DIFF ANALYSIS
**Step 2.1**: Single file changed: `dpu_encoder_phys_cmd.c`. +6 lines
added (including blank line). One function modified:
`dpu_encoder_phys_cmd_control_vblank_irq()`. Scope: single-file surgical
fix.
**Step 2.2**: The change inserts a guard check between the slave encoder
check and the refcount-negative check:
- **Before**: If `irq[INTR_IDX_RDPTR]` is 0, the code proceeds to call
`dpu_core_irq_register_callback(dpu_kms, 0, ...)`, which fails with
confusing error messages.
- **After**: The new check catches irq_idx=0 early, returns -EINVAL via
`goto end`, skipping the confusing `dpu_core_irq_register_callback()`
error path.
**Step 2.3**: Bug category: **Logic/correctness fix** (missing guard for
uninitialized state). The function can be called via the vblank
workqueue before IRQs are initialized. The macros `DPU_IRQ_REG(0) =
(0-1)/32 = 134217727` and `DPU_IRQ_BIT(0) = (0-1)%32 = 31` produce
wildly misleading error values.
**Step 2.4**: Fix quality: Obviously correct. The check
`!phys_enc->irq[INTR_IDX_RDPTR]` is the simplest possible guard. No
regression risk - returns the same -EINVAL that the existing code path
produces (via `dpu_core_irq_is_valid(0)` returning false), just without
the confusing intermediate error message. Follows the pattern of the
slave encoder check above it.
## PHASE 3: GIT HISTORY INVESTIGATION
**Step 3.1**: `git blame` shows the `control_vblank_irq()` function was
introduced by Jeykumar Sankaran in commit `25fdd5933e4c0f` (June 2018),
the original DPU driver submission. The function has been present since
v5.1.
**Step 3.2**: No Fixes: tag present.
**Step 3.3**: Related commits:
- `d13f638c9b88e` (v6.9): Dropped `atomic_mode_set()`, moving IRQ init
to `irq_enable()` — introduced the bug more acutely
- `35322c39a653c` (v6.11): Reverted the above, re-adding
`atomic_mode_set()` — partially fixed the issue
- The current fix addresses the remaining race window even after the
revert, since `control_vblank_irq()` can be called before the first
`atomic_mode_set()`
**Step 3.4**: The author (Cédric Bellegarde) is not the maintainer but
the patch is reviewed and merged by Dmitry Baryshkov, who is the DPU
subsystem maintainer.
**Step 3.5**: No prerequisites needed. The fix applies to the code as it
exists in the current tree. For older stable trees, the
`vblank_ctl_lock` mutex (added in v6.8 by `45284ff733e4c`) must exist
for the `goto end` pattern to work correctly.
## PHASE 4: MAILING LIST RESEARCH
**Step 4.1**: Both lore.kernel.org and patchwork.freedesktop.org were
blocked by anti-bot protection. The b4 dig search didn't find the commit
directly. However, the patchwork link in the commit metadata
(`patch/712752/`) and the lore link confirm it was submitted and
reviewed through the normal DRM/MSM workflow.
**Step 4.2**: Reviewed by Dmitry Baryshkov (DPU subsystem maintainer),
who also merged the patch. This is the appropriate reviewer.
**Step 4.3-4.5**: Could not fully verify due to anti-bot protections on
lore/patchwork.
## PHASE 5: CODE SEMANTIC ANALYSIS
**Step 5.1**: Modified function:
`dpu_encoder_phys_cmd_control_vblank_irq()`
**Step 5.2**: Call chain traced:
1. Userspace vblank request → DRM framework
2. `msm_crtc_enable_vblank()` → `vblank_ctrl_queue_work()` (queues work
item)
3. `vblank_ctrl_worker()` (async workqueue) →
`kms->funcs->enable_vblank()`
4. `dpu_kms_enable_vblank()` → `dpu_crtc_vblank()` →
`dpu_encoder_toggle_vblank_for_crtc()`
5. → `phys->ops.control_vblank_irq(phys, enable)` (the function being
fixed)
This is a common user-reachable path — any userspace app requesting
vblank events.
**Step 5.3-5.4**: The vblank worker runs asynchronously. If it fires
before the first `atomic_mode_set()` in the atomic commit path,
`irq[INTR_IDX_RDPTR]` is still zero. Confirmed at line 159:
```149:163:drivers/gpu/drm/msm/disp/dpu1/dpu_encoder_phys_cmd.c
static void dpu_encoder_phys_cmd_atomic_mode_set(
struct dpu_encoder_phys *phys_enc,
struct drm_crtc_state *crtc_state,
struct drm_connector_state *conn_state)
{
// ... sets irq[INTR_IDX_RDPTR] here
if (phys_enc->has_intf_te)
phys_enc->irq[INTR_IDX_RDPTR] =
phys_enc->hw_intf->cap->intr_tear_rd_ptr;
else
phys_enc->irq[INTR_IDX_RDPTR] =
phys_enc->hw_pp->caps->intr_rdptr;
// ...
}
```
**Step 5.5**: The video encoder
(`dpu_encoder_phys_vid_control_vblank_irq`) has a similar pattern with
`INTR_IDX_VSYNC` but lacks this guard. Potentially a related issue
exists there too.
## PHASE 6: STABLE TREE ANALYSIS
**Step 6.1**: The buggy code (`control_vblank_irq` without the guard)
exists in all stable trees since v5.1. The async vblank workqueue path
that triggers it also exists in all DPU-capable stable trees.
**Step 6.2**: Backport complications:
- v6.12.y and later: Should apply cleanly (mutex locking exists since
v6.8)
- v6.6.y: The `vblank_ctl_lock` mutex doesn't exist; function uses
different locking. Would need adaptation.
**Step 6.3**: The related revert `35322c39a653c` (v6.11) fixed the acute
version of this problem but didn't address the remaining race window
this fix covers.
## PHASE 7: SUBSYSTEM CONTEXT
**Step 7.1**: Subsystem: `drivers/gpu/drm/msm/disp/dpu1/` — DPU display
driver for Qualcomm SoCs. Criticality: IMPORTANT. Used in Qualcomm-based
phones, Chromebooks, and development boards (Dragonboard, Robotics RB
series).
**Step 7.2**: Active subsystem with regular commits from Dmitry
Baryshkov and other contributors.
## PHASE 8: IMPACT AND RISK ASSESSMENT
**Step 8.1**: Affects users of Qualcomm command-mode DSI panels (common
in mobile devices and some Chromebooks).
**Step 8.2**: Trigger: Userspace requesting vblank events before the
first atomic modeset completes. This can happen during display
initialization if applications request vblank timing early. The vblank
workqueue makes this asynchronous and timing-dependent.
**Step 8.3**: Failure mode: Confusing error messages in dmesg (`invalid
IRQ=[134217727, 31]`). Not a crash, not data corruption, not a security
issue. Severity: **MEDIUM** — the error messages are misleading and can
cause confusion during debugging, but the system still functions
correctly because `dpu_core_irq_is_valid(0)` catches the invalid index.
**Step 8.4**:
- **Benefit**: MEDIUM — eliminates confusing error messages for CMD DSI
panel users; makes the code path cleaner and more intentional
- **Risk**: VERY LOW — 5 lines, obviously correct guard check, returns
same error code, no change in functional behavior
- **Ratio**: Favorable — low risk fix with meaningful user-facing
improvement
## PHASE 9: FINAL SYNTHESIS
**Evidence FOR backporting:**
- Small, surgical fix (5 lines in one function)
- Obviously correct — simple NULL/zero check before use
- Reviewed and merged by subsystem maintainer (Dmitry Baryshkov)
- Fixes a real race condition between async vblank workqueue and atomic
commit
- Eliminates confusing error messages (`invalid IRQ=[134217727, 31]`)
caused by unsigned wraparound
- Follows existing code pattern in the same function (master encoder
guard)
- Affects real hardware (Qualcomm CMD DSI panels in mobile/Chromebook)
- No regression risk — returns same error as existing path
**Evidence AGAINST backporting:**
- Not fixing a crash, security issue, or data corruption
- Primarily an error message cleanup (existing code already handles
irq_idx=0 correctly via `dpu_core_irq_is_valid`)
- May need adaptation for older stable trees (v6.6.y lacks mutex)
**Stable rules checklist:**
1. Obviously correct and tested? **YES** — trivial guard, reviewed by
maintainer
2. Fixes a real bug? **YES** — race condition producing confusing errors
3. Important issue? **MEDIUM** — not crash/security, but user-visible
error messages on real hardware
4. Small and contained? **YES** — 5 lines in one function
5. No new features or APIs? **YES**
6. Can apply to stable trees? **YES** for v6.12.y+; needs adaptation for
v6.6.y
## Verification
- [Phase 1] Parsed tags: Reviewed-by Dmitry Baryshkov (maintainer), Link
to lore, Patchwork link. No Fixes/Cc:stable (expected).
- [Phase 2] Diff analysis: 5 lines added to
`dpu_encoder_phys_cmd_control_vblank_irq()`, guards against
irq[INTR_IDX_RDPTR]==0 before calling dpu_core_irq_register_callback.
- [Phase 2] Verified DPU_IRQ_REG/DPU_IRQ_BIT macros at
`dpu_hw_interrupts.h:40-41`: `(irq_idx - 1) / 32` and `(irq_idx - 1) %
32` — confirmed unsigned wraparound when irq_idx=0.
- [Phase 2] Verified `dpu_core_irq_is_valid(0)` at
`dpu_hw_interrupts.c:289-292`: returns false, so existing code catches
it but after printing confusing values.
- [Phase 3] git blame: `control_vblank_irq()` introduced by
`25fdd5933e4c0f` (v5.1, June 2018), present in all stable trees.
- [Phase 3] git show `35322c39a653c`: Confirmed revert of
`d13f638c9b88e`, with identical call trace showing the same race
condition this fix addresses.
- [Phase 3] `d13f638c9b88e` (v6.9) dropped atomic_mode_set;
`35322c39a653c` (v6.11) reverted it. Current fix addresses remaining
race window.
- [Phase 4] Lore/patchwork blocked by anti-bot protection — could not
verify full discussion.
- [Phase 5] Traced full call chain: `msm_crtc_enable_vblank →
vblank_ctrl_queue_work → vblank_ctrl_worker → dpu_kms_enable_vblank →
dpu_crtc_vblank → dpu_encoder_toggle_vblank_for_crtc →
control_vblank_irq`. Confirmed async workqueue makes this race
realistic.
- [Phase 5] Verified `atomic_mode_set()` at line 149-164 initializes
`irq[INTR_IDX_RDPTR]` — confirmed it's the only place this IRQ index
is set.
- [Phase 5] Verified `dpu_encoder_toggle_vblank_for_crtc()` ignores the
return value from `control_vblank_irq()`.
- [Phase 6] `vblank_ctl_lock` mutex added in v6.8 (`45284ff733e4c`) —
confirmed this dependency for backport to v6.6.y.
- [Phase 7] Confirmed active subsystem with regular Dmitry Baryshkov
contributions.
- [Phase 8] Failure mode: confusing error messages, no crash/corruption.
Severity: MEDIUM.
- UNVERIFIED: Full mailing list discussion (lore blocked). Cannot
confirm if stable was discussed or if there are NAKs.
The fix is small, obviously correct, reviewed by the maintainer, and
addresses a real race condition on Qualcomm command-mode DSI panels.
While the primary impact is eliminating confusing error messages rather
than preventing crashes, the fix improves code robustness with
negligible regression risk.
**YES**
drivers/gpu/drm/msm/disp/dpu1/dpu_encoder_phys_cmd.c | 6 ++++++
1 file changed, 6 insertions(+)
diff --git a/drivers/gpu/drm/msm/disp/dpu1/dpu_encoder_phys_cmd.c b/drivers/gpu/drm/msm/disp/dpu1/dpu_encoder_phys_cmd.c
index 93db1484f6069..45079ee59cf67 100644
--- a/drivers/gpu/drm/msm/disp/dpu1/dpu_encoder_phys_cmd.c
+++ b/drivers/gpu/drm/msm/disp/dpu1/dpu_encoder_phys_cmd.c
@@ -257,6 +257,12 @@ static int dpu_encoder_phys_cmd_control_vblank_irq(
if (!dpu_encoder_phys_cmd_is_master(phys_enc))
goto end;
+ /* IRQ not yet initialized */
+ if (!phys_enc->irq[INTR_IDX_RDPTR]) {
+ ret = -EINVAL;
+ goto end;
+ }
+
/* protect against negative */
if (!enable && refcount == 0) {
ret = -EINVAL;
--
2.53.0
^ permalink raw reply related
* [PATCH AUTOSEL 7.0-6.18] firmware: qcom: scm: Allow QSEECOM on ECS LIVA QC710
From: Sasha Levin @ 2026-04-20 13:18 UTC (permalink / raw)
To: patches, stable
Cc: Val Packett, Dmitry Baryshkov, Bjorn Andersson, Sasha Levin,
konradybcio, linux-arm-msm, linux-kernel
In-Reply-To: <20260420132314.1023554-1-sashal@kernel.org>
From: Val Packett <val@packett.cool>
[ Upstream commit 34a49e8508b5d00816d25fe3758b474471e7e051 ]
Allow this machine to access efivars through qseecom/uefisecapp.
Reviewed-by: Dmitry Baryshkov <dmitry.baryshkov@oss.qualcomm.com>
Signed-off-by: Val Packett <val@packett.cool>
Link: https://lore.kernel.org/r/20260120234029.419825-11-val@packett.cool
Signed-off-by: Bjorn Andersson <andersson@kernel.org>
Signed-off-by: Sasha Levin <sashal@kernel.org>
---
LLM Generated explanations, may be completely bogus:
Now I have a clear picture. Let me finalize my analysis.
---
## PHASE 1: COMMIT MESSAGE FORENSICS
**Step 1.1: Subject Line**
- Subsystem: `firmware: qcom: scm`
- Action: "Allow" — enabling hardware functionality
- Summary: Add ECS LIVA QC710 to the QSEECOM allowlist for EFI variable
access.
**Step 1.2: Tags**
- `Reviewed-by: Dmitry Baryshkov` — Qualcomm subsystem reviewer
- `Signed-off-by: Val Packett` — patch author
- `Link:` to lore (message-id:
`20260120234029.419825-11-val@packett.cool`) — patch 11 of a series
- `Signed-off-by: Bjorn Andersson` — subsystem maintainer who merged it
- No Fixes: tag (expected for an allowlist addition)
- No Cc: stable (expected for this review)
**Step 1.3: Commit Body**
The body is a single sentence: "Allow this machine to access efivars
through qseecom/uefisecapp." Without this entry, the ECS LIVA QC710
cannot access UEFI EFI variables, which are needed for firmware settings
and boot configuration.
**Step 1.4: Hidden Bug Fix Detection**
This is not a hidden bug fix. It is a straightforward hardware
enablement / device ID addition.
Record: This is a device allowlist addition, enabling EFI variable
access on a specific Qualcomm-based machine.
## PHASE 2: DIFF ANALYSIS
**Step 2.1: Inventory**
- 1 file changed: `drivers/firmware/qcom/qcom_scm.c`
- 1 line added: `{ .compatible = "ecs,liva-qc710" },`
- Function: N/A — it's a static data table entry
- Scope: Trivially small, single-line addition to an allowlist array
**Step 2.2: Code Flow Change**
Before: ECS LIVA QC710 is not in the allowlist, so
`qcom_scm_qseecom_init()` prints "untested machine, skipping" and
QSEECOM/uefisecapp is not initialized.
After: The machine is matched, QSEECOM is initialized, and EFI variables
become accessible.
**Step 2.3: Bug Mechanism**
Category: Hardware enablement / device ID addition. Not a bug fix per
se, but enables critical firmware functionality (EFI variable access) on
a specific machine.
**Step 2.4: Fix Quality**
- Obviously correct — adding one compatible string to a sorted allowlist
- Minimal / surgical — one line
- Zero regression risk — only affects the specific machine by compatible
string match
- Reviewed by Qualcomm reviewer, merged by subsystem maintainer
## PHASE 3: GIT HISTORY
**Step 3.1: Blame**
The allowlist was introduced in commit `00b1248606ba39` ("Add support
for Qualcomm Secure Execution Environment SCM interface"), tagged
`v6.6-rc1-2`, which means it appeared in v6.7. Multiple similar entries
have been added since.
**Step 3.3: File History**
There are 17+ similar "Allow QSEECOM on <machine>" commits — this is a
well-established pattern.
**Step 3.5: Dependencies**
The diff also shows `asus,vivobook-s15-x1p4` and `qcom,purwa-iot-evk`
entries that don't exist in the current tree. These come from other
patches in the same series (patch 11 of a multi-patch series). However,
the `ecs,liva-qc710` line itself is independent — it's just an addition
to an alphabetically-sorted list with no dependencies on other entries.
## PHASE 4: MAILING LIST
The Link header references `20260120234029.419825-11-val@packett.cool`,
indicating this is patch 11 of a larger series. The patch was reviewed
by Dmitry Baryshkov (Qualcomm reviewer) and merged by Bjorn Andersson
(the subsystem maintainer).
## PHASE 5: CODE SEMANTIC ANALYSIS
The allowlist is used in `qcom_scm_qseecom_init()` which checks
`of_machine_device_match(qcom_scm_qseecom_allowlist)` (in mainline) or
`qcom_scm_qseecom_machine_is_allowed()` (in older stable trees). If the
machine isn't in the list, QSEECOM is skipped entirely. The change only
affects the specific ECS LIVA QC710 machine.
## PHASE 6: STABLE TREE ANALYSIS
**Step 6.1: Code Existence in Stable Trees**
- v6.6: Does NOT have the allowlist (code doesn't exist) — not
applicable
- v6.7 through v6.13+: DO have the allowlist
- Active stable trees with this code: 6.12.y and any other LTS ≥ 6.7
**Step 6.2: Backport Complications**
The one-line addition should apply cleanly. In older stable trees, the
surrounding list entries may differ slightly (fewer entries), but the
addition of a new compatible string to the sorted list is trivial to
resolve if there's a minor context conflict. The function mechanism (old
`qcom_scm_qseecom_machine_is_allowed` vs new `of_machine_device_match`)
doesn't matter — only the data table is modified.
## PHASE 7: SUBSYSTEM CONTEXT
- Subsystem: Qualcomm SCM firmware interface
- Criticality: PERIPHERAL — affects users of specific Qualcomm ARM
platforms
- This is an actively developed subsystem with many contributors adding
machine support
## PHASE 8: IMPACT AND RISK ASSESSMENT
**Step 8.1: Who is Affected**
Only users of the ECS LIVA QC710 (Qualcomm-based mini PC).
**Step 8.2: Trigger**
Every boot — without this entry, EFI variables are completely
inaccessible on this machine.
**Step 8.3: Failure Mode**
Without the fix: EFI variables are inaccessible, which means users
cannot manage boot configuration, firmware settings, or Secure Boot
through the standard Linux efivars interface. Severity: MEDIUM-HIGH for
users of this specific hardware.
**Step 8.4: Risk-Benefit Ratio**
- BENEFIT: Enables critical firmware functionality for a specific
machine
- RISK: Essentially zero — only a data table entry, only affects the
specific machine by exact compatible string match
- Assessment: Very favorable ratio
## PHASE 9: FINAL SYNTHESIS
**Evidence FOR backporting:**
- Classic device ID / allowlist addition pattern — exception category
that is routinely backported
- One-line change with zero regression risk
- Enables critical firmware functionality (EFI variable access)
- Well-reviewed by subsystem reviewer and maintainer
- Established pattern with 17+ identical precedent commits
**Evidence AGAINST backporting:**
- This is hardware enablement, not a bug fix in the traditional sense
- Only affects a single specific machine
- The allowlist doesn't exist in v6.6.y (oldest active LTS)
**Stable Rules Checklist:**
1. Obviously correct? YES — single allowlist entry
2. Fixes a real bug? It enables required functionality — comparable to a
device ID addition
3. Important issue? Medium-important for affected hardware users
4. Small and contained? YES — 1 line
5. No new features? Correct — just enables existing feature on
additional hardware
6. Can apply to stable? YES for 6.12.y and later stable trees
**Exception Category:** Device ID / hardware allowlist addition to
existing driver — this is an explicitly allowed exception for stable
trees.
## Verification
- [Phase 1] Parsed tags: Reviewed-by Dmitry Baryshkov, Link to lore,
Signed-off-by Bjorn Andersson (maintainer)
- [Phase 2] Diff: Single line `{ .compatible = "ecs,liva-qc710" }` added
to allowlist table
- [Phase 3] git blame: allowlist introduced in 00b1248606ba39 at
v6.6-rc1-2, present from v6.7+
- [Phase 3] git log: 17+ identical "Allow QSEECOM on <machine>" commits
exist as precedent
- [Phase 4] b4 dig for similar commit (30ee285f1e3cd): confirmed review
and merge pattern
- [Phase 5] Allowlist controls access in qcom_scm_qseecom_init(), only
matched machine is affected
- [Phase 6] Allowlist exists in v6.7+ (verified via git show for v6.6
through v6.13); not in v6.6
- [Phase 6] v6.12 uses `qcom_scm_qseecom_machine_is_allowed()` — data
table change applies regardless
- [Phase 8] Risk: zero — one-line data table entry, only affects
specific machine by exact compatible match
This is a textbook device ID / allowlist addition. It's a single line,
zero-risk, enables important firmware functionality for a specific
machine, and follows a long-established pattern of identical commits
that are routinely backported.
**YES**
drivers/firmware/qcom/qcom_scm.c | 1 +
1 file changed, 1 insertion(+)
diff --git a/drivers/firmware/qcom/qcom_scm.c b/drivers/firmware/qcom/qcom_scm.c
index 2fe1632f06e90..f06bed79666b9 100644
--- a/drivers/firmware/qcom/qcom_scm.c
+++ b/drivers/firmware/qcom/qcom_scm.c
@@ -2296,6 +2296,7 @@ static const struct of_device_id qcom_scm_qseecom_allowlist[] __maybe_unused = {
{ .compatible = "dell,inspiron-14-plus-7441" },
{ .compatible = "dell,latitude-7455" },
{ .compatible = "dell,xps13-9345" },
+ { .compatible = "ecs,liva-qc710" },
{ .compatible = "hp,elitebook-ultra-g1q" },
{ .compatible = "hp,omnibook-x14" },
{ .compatible = "huawei,gaokun3" },
--
2.53.0
^ permalink raw reply related
* [PATCH AUTOSEL 6.18] arm64: dts: qcom: hamoa/x1: fix idle exit latency
From: Sasha Levin @ 2026-04-20 13:19 UTC (permalink / raw)
To: patches, stable
Cc: Daniel J Blueman, Konrad Dybcio, Bjorn Andersson, Sasha Levin,
konradybcio, robh, krzk+dt, conor+dt, linux-arm-msm, devicetree,
linux-kernel
In-Reply-To: <20260420132314.1023554-1-sashal@kernel.org>
From: Daniel J Blueman <daniel@quora.org>
[ Upstream commit 3ecea84d2b90bbf934d5ca75514fa902fd71e03f ]
Designs based on the Qualcomm X1 Hamoa reference platform report:
driver: Idle state 1 target residency too low
This is because the declared X1 idle entry plus exit latency of 680us
exceeds the declared minimum 600us residency time:
entry-latency-us = <180>;
exit-latency-us = <500>;
min-residency-us = <600>;
Fix this to be 320us so the sum of the entry and exit latencies matches
the downstream 500us exit latency, as directed by Maulik.
Tested on a Lenovo Yoga Slim 7x with Qualcomm X1E-80-100.
Fixes: 2e65616ef07f ("arm64: dts: qcom: x1e80100: Update C4/C5 residency/exit numbers")
Signed-off-by: Daniel J Blueman <daniel@quora.org>
Reviewed-by: Konrad Dybcio <konrad.dybcio@oss.qualcomm.com>
Link: https://lore.kernel.org/r/20260220124626.8611-1-daniel@quora.org
Signed-off-by: Bjorn Andersson <andersson@kernel.org>
Signed-off-by: Sasha Levin <sashal@kernel.org>
---
LLM Generated explanations, may be completely bogus:
Error: Failed to generate final synthesis
arch/arm64/boot/dts/qcom/x1e80100.dtsi | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/arch/arm64/boot/dts/qcom/x1e80100.dtsi b/arch/arm64/boot/dts/qcom/x1e80100.dtsi
index 6d97329995fe7..efe8d5e7079fe 100644
--- a/arch/arm64/boot/dts/qcom/x1e80100.dtsi
+++ b/arch/arm64/boot/dts/qcom/x1e80100.dtsi
@@ -281,7 +281,7 @@ cluster_c4: cpu-sleep-0 {
idle-state-name = "ret";
arm,psci-suspend-param = <0x00000004>;
entry-latency-us = <180>;
- exit-latency-us = <500>;
+ exit-latency-us = <320>;
min-residency-us = <600>;
};
};
--
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