* [PATCH v1 1/5] dt-bindings: sound: qcom,q6dsp-lpass-ports: Rename QAIF clock IDs
2026-08-08 18:19 [PATCH v1 0/5] ASoC: qcom: Fix AudioReach TDM review findings Prasad Kumpatla
@ 2026-08-08 18:19 ` Prasad Kumpatla
2026-08-08 18:45 ` sashiko-bot
2026-08-08 18:19 ` [PATCH v1 2/5] ASoC: qcom: q6prm: Fix QAIF clock ID typo Prasad Kumpatla
` (3 subsequent siblings)
4 siblings, 1 reply; 14+ messages in thread
From: Prasad Kumpatla @ 2026-08-08 18:19 UTC (permalink / raw)
To: Liam Girdwood, Mark Brown, Rob Herring, Krzysztof Kozlowski,
Conor Dooley, Srinivas Kandagatla, Jaroslav Kysela, Takashi Iwai
Cc: linux-arm-msm, linux-sound, devicetree, linux-kernel,
Prasad Kumpatla, Sashiko
The QAIF clock IDs were added with a LAPSS prefix typo. These IDs are
new, so rename them directly to the existing LPASS naming convention
instead of keeping misspelled aliases.
Reported-by: Sashiko <sashiko-bot@kernel.org>
Link: https://sashiko.dev/#/patchset/20260804070307.117119-1-prasad.kumpatla@oss.qualcomm.com
Signed-off-by: Prasad Kumpatla <prasad.kumpatla@oss.qualcomm.com>
---
.../sound/qcom,q6dsp-lpass-ports.h | 56 +++++++++----------
1 file changed, 28 insertions(+), 28 deletions(-)
diff --git a/include/dt-bindings/sound/qcom,q6dsp-lpass-ports.h b/include/dt-bindings/sound/qcom,q6dsp-lpass-ports.h
index 2e879a3c09ae..3a99703dbc4a 100644
--- a/include/dt-bindings/sound/qcom,q6dsp-lpass-ports.h
+++ b/include/dt-bindings/sound/qcom,q6dsp-lpass-ports.h
@@ -237,34 +237,34 @@
/* Clock ID for RX CORE MCLK2 2X MCLK */
#define LPASS_CLK_ID_RX_CORE_MCLK2_2X_MCLK 70
-#define LAPSS_CLK_ID_QAIF_IF0_IBIT 71
-#define LAPSS_CLK_ID_QAIF_IF0_EBIT 72
-#define LAPSS_CLK_ID_QAIF_IF1_IBIT 73
-#define LAPSS_CLK_ID_QAIF_IF1_EBIT 74
-#define LAPSS_CLK_ID_QAIF_IF2_IBIT 75
-#define LAPSS_CLK_ID_QAIF_IF2_EBIT 76
-#define LAPSS_CLK_ID_QAIF_IF3_IBIT 77
-#define LAPSS_CLK_ID_QAIF_IF3_EBIT 78
-#define LAPSS_CLK_ID_QAIF_IF4_IBIT 79
-#define LAPSS_CLK_ID_QAIF_IF4_EBIT 80
-#define LAPSS_CLK_ID_QAIF_IF5_IBIT 81
-#define LAPSS_CLK_ID_QAIF_IF5_EBIT 82
-#define LAPSS_CLK_ID_QAIF_IF6_IBIT 83
-#define LAPSS_CLK_ID_QAIF_IF6_EBIT 84
-#define LAPSS_CLK_ID_QAIF_IF7_IBIT 85
-#define LAPSS_CLK_ID_QAIF_IF7_EBIT 86
-#define LAPSS_CLK_ID_QAIF_IF8_IBIT 87
-#define LAPSS_CLK_ID_QAIF_IF8_EBIT 88
-#define LAPSS_CLK_ID_QAIF_IF9_IBIT 89
-#define LAPSS_CLK_ID_QAIF_IF9_EBIT 90
-#define LAPSS_CLK_ID_QAIF_IF10_IBIT 91
-#define LAPSS_CLK_ID_QAIF_IF10_EBIT 92
-#define LAPSS_CLK_ID_QAIF_IF11_IBIT 93
-#define LAPSS_CLK_ID_QAIF_IF11_EBIT 94
-#define LAPSS_CLK_ID_QAIF_IF12_IBIT 95
-#define LAPSS_CLK_ID_QAIF_IF12_EBIT 96
-#define LAPSS_CLK_ID_VA_QAIF_IF0_IBIT 97
-#define LAPSS_CLK_ID_VA_QAIF_IF0_EBIT 98
+#define LPASS_CLK_ID_QAIF_IF0_IBIT 71
+#define LPASS_CLK_ID_QAIF_IF0_EBIT 72
+#define LPASS_CLK_ID_QAIF_IF1_IBIT 73
+#define LPASS_CLK_ID_QAIF_IF1_EBIT 74
+#define LPASS_CLK_ID_QAIF_IF2_IBIT 75
+#define LPASS_CLK_ID_QAIF_IF2_EBIT 76
+#define LPASS_CLK_ID_QAIF_IF3_IBIT 77
+#define LPASS_CLK_ID_QAIF_IF3_EBIT 78
+#define LPASS_CLK_ID_QAIF_IF4_IBIT 79
+#define LPASS_CLK_ID_QAIF_IF4_EBIT 80
+#define LPASS_CLK_ID_QAIF_IF5_IBIT 81
+#define LPASS_CLK_ID_QAIF_IF5_EBIT 82
+#define LPASS_CLK_ID_QAIF_IF6_IBIT 83
+#define LPASS_CLK_ID_QAIF_IF6_EBIT 84
+#define LPASS_CLK_ID_QAIF_IF7_IBIT 85
+#define LPASS_CLK_ID_QAIF_IF7_EBIT 86
+#define LPASS_CLK_ID_QAIF_IF8_IBIT 87
+#define LPASS_CLK_ID_QAIF_IF8_EBIT 88
+#define LPASS_CLK_ID_QAIF_IF9_IBIT 89
+#define LPASS_CLK_ID_QAIF_IF9_EBIT 90
+#define LPASS_CLK_ID_QAIF_IF10_IBIT 91
+#define LPASS_CLK_ID_QAIF_IF10_EBIT 92
+#define LPASS_CLK_ID_QAIF_IF11_IBIT 93
+#define LPASS_CLK_ID_QAIF_IF11_EBIT 94
+#define LPASS_CLK_ID_QAIF_IF12_IBIT 95
+#define LPASS_CLK_ID_QAIF_IF12_EBIT 96
+#define LPASS_CLK_ID_VA_QAIF_IF0_IBIT 97
+#define LPASS_CLK_ID_VA_QAIF_IF0_EBIT 98
#define LPASS_HW_AVTIMER_VOTE 101
#define LPASS_HW_MACRO_VOTE 102
--
2.34.1
^ permalink raw reply related [flat|nested] 14+ messages in thread* Re: [PATCH v1 1/5] dt-bindings: sound: qcom,q6dsp-lpass-ports: Rename QAIF clock IDs
2026-08-08 18:19 ` [PATCH v1 1/5] dt-bindings: sound: qcom,q6dsp-lpass-ports: Rename QAIF clock IDs Prasad Kumpatla
@ 2026-08-08 18:45 ` sashiko-bot
2026-08-09 12:25 ` Prasad Kumpatla
0 siblings, 1 reply; 14+ messages in thread
From: sashiko-bot @ 2026-08-08 18:45 UTC (permalink / raw)
To: Prasad Kumpatla; +Cc: conor+dt, devicetree, robh
Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:
- [Low] Renaming macro definitions in the dt-bindings header without updating corresponding driver usages breaks the kernel build.
--
--- Patch [1]: [PATCH v1 1/5] dt-bindings: sound: qcom,q6dsp-lpass-ports: Rename QAIF clock IDs ---
commit 24389528fddee532f4b72dd537118240033f4a3e
Author: Prasad Kumpatla <prasad.kumpatla@oss.qualcomm.com>
dt-bindings: sound: qcom,q6dsp-lpass-ports: Rename QAIF clock IDs
The patch renames the LAPSS_CLK_ID_QAIF_* macros to LPASS_CLK_ID_QAIF_*
in the dt-bindings header to fix a spelling typo in the prefix.
Link: https://sashiko.dev/#/patchset/20260804070307.117119-1-prasad.kumpatla@oss.qualcomm.com
> diff --git a/include/dt-bindings/sound/qcom,q6dsp-lpass-ports.h b/include/dt-bindings/sound/qcom,q6dsp-lpass-ports.h
> index 2e879a3c09aef..3a99703dbc4a0 100644
> --- a/include/dt-bindings/sound/qcom,q6dsp-lpass-ports.h
> +++ b/include/dt-bindings/sound/qcom,q6dsp-lpass-ports.h
[ ... ]
> -#define LAPSS_CLK_ID_QAIF_IF0_IBIT 71
> -#define LAPSS_CLK_ID_QAIF_IF0_EBIT 72
> -#define LAPSS_CLK_ID_QAIF_IF1_IBIT 73
> -#define LAPSS_CLK_ID_QAIF_IF1_EBIT 74
[ ... ]
> -#define LAPSS_CLK_ID_VA_QAIF_IF0_IBIT 97
> -#define LAPSS_CLK_ID_VA_QAIF_IF0_EBIT 98
> +#define LPASS_CLK_ID_QAIF_IF0_IBIT 71
> +#define LPASS_CLK_ID_QAIF_IF0_EBIT 72
> +#define LPASS_CLK_ID_QAIF_IF1_IBIT 73
> +#define LPASS_CLK_ID_QAIF_IF1_EBIT 74
[Severity: Low]
Does renaming these macros here without updating the dependent driver files
break the build?
Looking at q6prm_clks in sound/soc/qcom/qdsp6/q6prm-clocks.c, it appears
to still reference the old macro names:
sound/soc/qcom/qdsp6/q6prm-clocks.c:q6prm_clks() {
...
Q6PRM_CLK(LAPSS_CLK_ID_QAIF_IF0_IBIT),
Q6PRM_CLK(LAPSS_CLK_ID_QAIF_IF0_EBIT),
...
}
Because Q6PRM_CLK(id) concatenates Q6PRM_##id, the corresponding target macros
in sound/soc/qcom/qdsp6/q6prm.h also still use the LAPSS spelling:
sound/soc/qcom/qdsp6/q6prm.h:
#define Q6PRM_LAPSS_CLK_ID_QAIF_IF0_IBIT 0x500
#define Q6PRM_LAPSS_CLK_ID_QAIF_IF0_EBIT 0x501
Do these C headers and source files need to be updated in this same commit to
prevent undeclared identifier errors during compilation?
--
Sashiko AI review · https://sashiko.dev/#/patchset/20260808181948.2489187-1-prasad.kumpatla@oss.qualcomm.com?part=1
^ permalink raw reply [flat|nested] 14+ messages in thread* Re: [PATCH v1 1/5] dt-bindings: sound: qcom,q6dsp-lpass-ports: Rename QAIF clock IDs
2026-08-08 18:45 ` sashiko-bot
@ 2026-08-09 12:25 ` Prasad Kumpatla
0 siblings, 0 replies; 14+ messages in thread
From: Prasad Kumpatla @ 2026-08-09 12:25 UTC (permalink / raw)
To: sashiko-reviews; +Cc: conor+dt, devicetree, robh
On 8/9/2026 12:15 AM, sashiko-bot@kernel.org wrote:
> Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:
> - [Low] Renaming macro definitions in the dt-bindings header without updating corresponding driver usages breaks the kernel build.
> --
>
> --- Patch [1]: [PATCH v1 1/5] dt-bindings: sound: qcom,q6dsp-lpass-ports: Rename QAIF clock IDs ---
> commit 24389528fddee532f4b72dd537118240033f4a3e
> Author: Prasad Kumpatla <prasad.kumpatla@oss.qualcomm.com>
>
> dt-bindings: sound: qcom,q6dsp-lpass-ports: Rename QAIF clock IDs
>
> The patch renames the LAPSS_CLK_ID_QAIF_* macros to LPASS_CLK_ID_QAIF_*
> in the dt-bindings header to fix a spelling typo in the prefix.
>
> Link: https://sashiko.dev/#/patchset/20260804070307.117119-1-prasad.kumpatla@oss.qualcomm.com
>
>> diff --git a/include/dt-bindings/sound/qcom,q6dsp-lpass-ports.h b/include/dt-bindings/sound/qcom,q6dsp-lpass-ports.h
>> index 2e879a3c09aef..3a99703dbc4a0 100644
>> --- a/include/dt-bindings/sound/qcom,q6dsp-lpass-ports.h
>> +++ b/include/dt-bindings/sound/qcom,q6dsp-lpass-ports.h
> [ ... ]
>
>> -#define LAPSS_CLK_ID_QAIF_IF0_IBIT 71
>> -#define LAPSS_CLK_ID_QAIF_IF0_EBIT 72
>> -#define LAPSS_CLK_ID_QAIF_IF1_IBIT 73
>> -#define LAPSS_CLK_ID_QAIF_IF1_EBIT 74
> [ ... ]
>> -#define LAPSS_CLK_ID_VA_QAIF_IF0_IBIT 97
>> -#define LAPSS_CLK_ID_VA_QAIF_IF0_EBIT 98
>> +#define LPASS_CLK_ID_QAIF_IF0_IBIT 71
>> +#define LPASS_CLK_ID_QAIF_IF0_EBIT 72
>> +#define LPASS_CLK_ID_QAIF_IF1_IBIT 73
>> +#define LPASS_CLK_ID_QAIF_IF1_EBIT 74
> [Severity: Low]
> Does renaming these macros here without updating the dependent driver files
> break the build?
Hi Sashiko,
Thanks for the review.
The old LAPSS_* definitions are not used by any in-tree users, so
removing them does not introduce a build regression. The
corresponding Q6PRM updates are handled in the related changes.
Thanks,
Prasad
>
> Looking at q6prm_clks in sound/soc/qcom/qdsp6/q6prm-clocks.c, it appears
> to still reference the old macro names:
>
> sound/soc/qcom/qdsp6/q6prm-clocks.c:q6prm_clks() {
> ...
> Q6PRM_CLK(LAPSS_CLK_ID_QAIF_IF0_IBIT),
> Q6PRM_CLK(LAPSS_CLK_ID_QAIF_IF0_EBIT),
> ...
> }
>
> Because Q6PRM_CLK(id) concatenates Q6PRM_##id, the corresponding target macros
> in sound/soc/qcom/qdsp6/q6prm.h also still use the LAPSS spelling:
>
> sound/soc/qcom/qdsp6/q6prm.h:
> #define Q6PRM_LAPSS_CLK_ID_QAIF_IF0_IBIT 0x500
> #define Q6PRM_LAPSS_CLK_ID_QAIF_IF0_EBIT 0x501
>
> Do these C headers and source files need to be updated in this same commit to
> prevent undeclared identifier errors during compilation?
>
^ permalink raw reply [flat|nested] 14+ messages in thread
* [PATCH v1 2/5] ASoC: qcom: q6prm: Fix QAIF clock ID typo
2026-08-08 18:19 [PATCH v1 0/5] ASoC: qcom: Fix AudioReach TDM review findings Prasad Kumpatla
2026-08-08 18:19 ` [PATCH v1 1/5] dt-bindings: sound: qcom,q6dsp-lpass-ports: Rename QAIF clock IDs Prasad Kumpatla
@ 2026-08-08 18:19 ` Prasad Kumpatla
2026-08-08 18:45 ` sashiko-bot
2026-08-08 18:19 ` [PATCH v1 3/5] ASoC: qcom: qdsp6: Zero-initialize AudioReach module config Prasad Kumpatla
` (2 subsequent siblings)
4 siblings, 1 reply; 14+ messages in thread
From: Prasad Kumpatla @ 2026-08-08 18:19 UTC (permalink / raw)
To: Liam Girdwood, Mark Brown, Rob Herring, Krzysztof Kozlowski,
Conor Dooley, Srinivas Kandagatla, Jaroslav Kysela, Takashi Iwai
Cc: linux-arm-msm, linux-sound, devicetree, linux-kernel,
Prasad Kumpatla, Sashiko
The Audio IF QAIF clock IDs were introduced with a "LAPSS"
prefix instead of "LPASS".
Rename the definitions and update their users accordingly.
This also fixes the clock names generated by Q6PRM_CLK().
Reported-by: Sashiko <sashiko-bot@kernel.org>
Link: https://sashiko.dev/#/patchset/20260804070307.117119-1-prasad.kumpatla@oss.qualcomm.com
Signed-off-by: Prasad Kumpatla <prasad.kumpatla@oss.qualcomm.com>
---
sound/soc/qcom/qdsp6/q6prm-clocks.c | 56 ++++++++++++++---------------
sound/soc/qcom/qdsp6/q6prm.h | 56 ++++++++++++++---------------
2 files changed, 56 insertions(+), 56 deletions(-)
diff --git a/sound/soc/qcom/qdsp6/q6prm-clocks.c b/sound/soc/qcom/qdsp6/q6prm-clocks.c
index f613e2aee75e..b4f9abbe02cb 100644
--- a/sound/soc/qcom/qdsp6/q6prm-clocks.c
+++ b/sound/soc/qcom/qdsp6/q6prm-clocks.c
@@ -64,34 +64,34 @@ static const struct q6dsp_clk_init q6prm_clks[] = {
Q6PRM_CLK(LPASS_CLK_ID_WSA2_CORE_TX_MCLK),
Q6PRM_CLK(LPASS_CLK_ID_WSA2_CORE_TX_2X_MCLK),
Q6PRM_CLK(LPASS_CLK_ID_RX_CORE_MCLK2_2X_MCLK),
- Q6PRM_CLK(LAPSS_CLK_ID_QAIF_IF0_IBIT),
- Q6PRM_CLK(LAPSS_CLK_ID_QAIF_IF0_EBIT),
- Q6PRM_CLK(LAPSS_CLK_ID_QAIF_IF1_IBIT),
- Q6PRM_CLK(LAPSS_CLK_ID_QAIF_IF1_EBIT),
- Q6PRM_CLK(LAPSS_CLK_ID_QAIF_IF2_IBIT),
- Q6PRM_CLK(LAPSS_CLK_ID_QAIF_IF2_EBIT),
- Q6PRM_CLK(LAPSS_CLK_ID_QAIF_IF3_IBIT),
- Q6PRM_CLK(LAPSS_CLK_ID_QAIF_IF3_EBIT),
- Q6PRM_CLK(LAPSS_CLK_ID_QAIF_IF4_IBIT),
- Q6PRM_CLK(LAPSS_CLK_ID_QAIF_IF4_EBIT),
- Q6PRM_CLK(LAPSS_CLK_ID_QAIF_IF5_IBIT),
- Q6PRM_CLK(LAPSS_CLK_ID_QAIF_IF5_EBIT),
- Q6PRM_CLK(LAPSS_CLK_ID_QAIF_IF6_IBIT),
- Q6PRM_CLK(LAPSS_CLK_ID_QAIF_IF6_EBIT),
- Q6PRM_CLK(LAPSS_CLK_ID_QAIF_IF7_IBIT),
- Q6PRM_CLK(LAPSS_CLK_ID_QAIF_IF7_EBIT),
- Q6PRM_CLK(LAPSS_CLK_ID_QAIF_IF8_IBIT),
- Q6PRM_CLK(LAPSS_CLK_ID_QAIF_IF8_EBIT),
- Q6PRM_CLK(LAPSS_CLK_ID_QAIF_IF9_IBIT),
- Q6PRM_CLK(LAPSS_CLK_ID_QAIF_IF9_EBIT),
- Q6PRM_CLK(LAPSS_CLK_ID_QAIF_IF10_IBIT),
- Q6PRM_CLK(LAPSS_CLK_ID_QAIF_IF10_EBIT),
- Q6PRM_CLK(LAPSS_CLK_ID_QAIF_IF11_IBIT),
- Q6PRM_CLK(LAPSS_CLK_ID_QAIF_IF11_EBIT),
- Q6PRM_CLK(LAPSS_CLK_ID_QAIF_IF12_IBIT),
- Q6PRM_CLK(LAPSS_CLK_ID_QAIF_IF12_EBIT),
- Q6PRM_CLK(LAPSS_CLK_ID_VA_QAIF_IF0_IBIT),
- Q6PRM_CLK(LAPSS_CLK_ID_VA_QAIF_IF0_EBIT),
+ Q6PRM_CLK(LPASS_CLK_ID_QAIF_IF0_IBIT),
+ Q6PRM_CLK(LPASS_CLK_ID_QAIF_IF0_EBIT),
+ Q6PRM_CLK(LPASS_CLK_ID_QAIF_IF1_IBIT),
+ Q6PRM_CLK(LPASS_CLK_ID_QAIF_IF1_EBIT),
+ Q6PRM_CLK(LPASS_CLK_ID_QAIF_IF2_IBIT),
+ Q6PRM_CLK(LPASS_CLK_ID_QAIF_IF2_EBIT),
+ Q6PRM_CLK(LPASS_CLK_ID_QAIF_IF3_IBIT),
+ Q6PRM_CLK(LPASS_CLK_ID_QAIF_IF3_EBIT),
+ Q6PRM_CLK(LPASS_CLK_ID_QAIF_IF4_IBIT),
+ Q6PRM_CLK(LPASS_CLK_ID_QAIF_IF4_EBIT),
+ Q6PRM_CLK(LPASS_CLK_ID_QAIF_IF5_IBIT),
+ Q6PRM_CLK(LPASS_CLK_ID_QAIF_IF5_EBIT),
+ Q6PRM_CLK(LPASS_CLK_ID_QAIF_IF6_IBIT),
+ Q6PRM_CLK(LPASS_CLK_ID_QAIF_IF6_EBIT),
+ Q6PRM_CLK(LPASS_CLK_ID_QAIF_IF7_IBIT),
+ Q6PRM_CLK(LPASS_CLK_ID_QAIF_IF7_EBIT),
+ Q6PRM_CLK(LPASS_CLK_ID_QAIF_IF8_IBIT),
+ Q6PRM_CLK(LPASS_CLK_ID_QAIF_IF8_EBIT),
+ Q6PRM_CLK(LPASS_CLK_ID_QAIF_IF9_IBIT),
+ Q6PRM_CLK(LPASS_CLK_ID_QAIF_IF9_EBIT),
+ Q6PRM_CLK(LPASS_CLK_ID_QAIF_IF10_IBIT),
+ Q6PRM_CLK(LPASS_CLK_ID_QAIF_IF10_EBIT),
+ Q6PRM_CLK(LPASS_CLK_ID_QAIF_IF11_IBIT),
+ Q6PRM_CLK(LPASS_CLK_ID_QAIF_IF11_EBIT),
+ Q6PRM_CLK(LPASS_CLK_ID_QAIF_IF12_IBIT),
+ Q6PRM_CLK(LPASS_CLK_ID_QAIF_IF12_EBIT),
+ Q6PRM_CLK(LPASS_CLK_ID_VA_QAIF_IF0_IBIT),
+ Q6PRM_CLK(LPASS_CLK_ID_VA_QAIF_IF0_EBIT),
Q6DSP_VOTE_CLK(LPASS_HW_MACRO_VOTE, Q6PRM_HW_CORE_ID_LPASS,
"LPASS_HW_MACRO"),
Q6DSP_VOTE_CLK(LPASS_HW_DCODEC_VOTE, Q6PRM_HW_CORE_ID_DCODEC,
diff --git a/sound/soc/qcom/qdsp6/q6prm.h b/sound/soc/qcom/qdsp6/q6prm.h
index cca77cd92bc1..cf619ef6561a 100644
--- a/sound/soc/qcom/qdsp6/q6prm.h
+++ b/sound/soc/qcom/qdsp6/q6prm.h
@@ -97,34 +97,34 @@
/* Clock ID for RX CORE MCLK2 2X MCLK */
#define Q6PRM_LPASS_CLK_ID_RX_CORE_MCLK2_2X_MCLK 0x318
-#define Q6PRM_LAPSS_CLK_ID_QAIF_IF0_IBIT 0x500
-#define Q6PRM_LAPSS_CLK_ID_QAIF_IF0_EBIT 0x501
-#define Q6PRM_LAPSS_CLK_ID_QAIF_IF1_IBIT 0x502
-#define Q6PRM_LAPSS_CLK_ID_QAIF_IF1_EBIT 0x503
-#define Q6PRM_LAPSS_CLK_ID_QAIF_IF2_IBIT 0x504
-#define Q6PRM_LAPSS_CLK_ID_QAIF_IF2_EBIT 0x505
-#define Q6PRM_LAPSS_CLK_ID_QAIF_IF3_IBIT 0x506
-#define Q6PRM_LAPSS_CLK_ID_QAIF_IF3_EBIT 0x507
-#define Q6PRM_LAPSS_CLK_ID_QAIF_IF4_IBIT 0x508
-#define Q6PRM_LAPSS_CLK_ID_QAIF_IF4_EBIT 0x509
-#define Q6PRM_LAPSS_CLK_ID_QAIF_IF5_IBIT 0x50A
-#define Q6PRM_LAPSS_CLK_ID_QAIF_IF5_EBIT 0x50B
-#define Q6PRM_LAPSS_CLK_ID_QAIF_IF6_IBIT 0x50C
-#define Q6PRM_LAPSS_CLK_ID_QAIF_IF6_EBIT 0x50D
-#define Q6PRM_LAPSS_CLK_ID_QAIF_IF7_IBIT 0x50E
-#define Q6PRM_LAPSS_CLK_ID_QAIF_IF7_EBIT 0x50F
-#define Q6PRM_LAPSS_CLK_ID_QAIF_IF8_IBIT 0x510
-#define Q6PRM_LAPSS_CLK_ID_QAIF_IF8_EBIT 0x511
-#define Q6PRM_LAPSS_CLK_ID_QAIF_IF9_IBIT 0x512
-#define Q6PRM_LAPSS_CLK_ID_QAIF_IF9_EBIT 0x513
-#define Q6PRM_LAPSS_CLK_ID_QAIF_IF10_IBIT 0x514
-#define Q6PRM_LAPSS_CLK_ID_QAIF_IF10_EBIT 0x515
-#define Q6PRM_LAPSS_CLK_ID_QAIF_IF11_IBIT 0x516
-#define Q6PRM_LAPSS_CLK_ID_QAIF_IF11_EBIT 0x517
-#define Q6PRM_LAPSS_CLK_ID_QAIF_IF12_IBIT 0x518
-#define Q6PRM_LAPSS_CLK_ID_QAIF_IF12_EBIT 0x519
-#define Q6PRM_LAPSS_CLK_ID_VA_QAIF_IF0_IBIT 0x550
-#define Q6PRM_LAPSS_CLK_ID_VA_QAIF_IF0_EBIT 0x551
+#define Q6PRM_LPASS_CLK_ID_QAIF_IF0_IBIT 0x500
+#define Q6PRM_LPASS_CLK_ID_QAIF_IF0_EBIT 0x501
+#define Q6PRM_LPASS_CLK_ID_QAIF_IF1_IBIT 0x502
+#define Q6PRM_LPASS_CLK_ID_QAIF_IF1_EBIT 0x503
+#define Q6PRM_LPASS_CLK_ID_QAIF_IF2_IBIT 0x504
+#define Q6PRM_LPASS_CLK_ID_QAIF_IF2_EBIT 0x505
+#define Q6PRM_LPASS_CLK_ID_QAIF_IF3_IBIT 0x506
+#define Q6PRM_LPASS_CLK_ID_QAIF_IF3_EBIT 0x507
+#define Q6PRM_LPASS_CLK_ID_QAIF_IF4_IBIT 0x508
+#define Q6PRM_LPASS_CLK_ID_QAIF_IF4_EBIT 0x509
+#define Q6PRM_LPASS_CLK_ID_QAIF_IF5_IBIT 0x50A
+#define Q6PRM_LPASS_CLK_ID_QAIF_IF5_EBIT 0x50B
+#define Q6PRM_LPASS_CLK_ID_QAIF_IF6_IBIT 0x50C
+#define Q6PRM_LPASS_CLK_ID_QAIF_IF6_EBIT 0x50D
+#define Q6PRM_LPASS_CLK_ID_QAIF_IF7_IBIT 0x50E
+#define Q6PRM_LPASS_CLK_ID_QAIF_IF7_EBIT 0x50F
+#define Q6PRM_LPASS_CLK_ID_QAIF_IF8_IBIT 0x510
+#define Q6PRM_LPASS_CLK_ID_QAIF_IF8_EBIT 0x511
+#define Q6PRM_LPASS_CLK_ID_QAIF_IF9_IBIT 0x512
+#define Q6PRM_LPASS_CLK_ID_QAIF_IF9_EBIT 0x513
+#define Q6PRM_LPASS_CLK_ID_QAIF_IF10_IBIT 0x514
+#define Q6PRM_LPASS_CLK_ID_QAIF_IF10_EBIT 0x515
+#define Q6PRM_LPASS_CLK_ID_QAIF_IF11_IBIT 0x516
+#define Q6PRM_LPASS_CLK_ID_QAIF_IF11_EBIT 0x517
+#define Q6PRM_LPASS_CLK_ID_QAIF_IF12_IBIT 0x518
+#define Q6PRM_LPASS_CLK_ID_QAIF_IF12_EBIT 0x519
+#define Q6PRM_LPASS_CLK_ID_VA_QAIF_IF0_IBIT 0x550
+#define Q6PRM_LPASS_CLK_ID_VA_QAIF_IF0_EBIT 0x551
#define Q6PRM_LPASS_CLK_SRC_INTERNAL 1
#define Q6PRM_LPASS_CLK_ROOT_DEFAULT 0
--
2.34.1
^ permalink raw reply related [flat|nested] 14+ messages in thread* Re: [PATCH v1 2/5] ASoC: qcom: q6prm: Fix QAIF clock ID typo
2026-08-08 18:19 ` [PATCH v1 2/5] ASoC: qcom: q6prm: Fix QAIF clock ID typo Prasad Kumpatla
@ 2026-08-08 18:45 ` sashiko-bot
2026-08-09 12:27 ` Prasad Kumpatla
0 siblings, 1 reply; 14+ messages in thread
From: sashiko-bot @ 2026-08-08 18:45 UTC (permalink / raw)
To: Prasad Kumpatla; +Cc: conor+dt, devicetree, robh
Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:
- [Low] The patch series breaks `git bisect` by splitting a macro rename across commits without retaining aliases, causing an intermediate build failure.
--
--- Patch [2]: [PATCH v1 2/5] ASoC: qcom: q6prm: Fix QAIF clock ID typo ---
commit 2d0653a87462572279fa5b7912ec981aed567a40
Author: Prasad Kumpatla <prasad.kumpatla@oss.qualcomm.com>
ASoC: qcom: q6prm: Fix QAIF clock ID typo
The Audio IF QAIF clock IDs were introduced with a "LAPSS" prefix instead
of "LPASS". This commit renames the definitions and updates their users
accordingly.
Link: https://sashiko.dev/#/patchset/20260804070307.117119-1-prasad.kumpatla@oss.qualcomm.com
> diff --git a/sound/soc/qcom/qdsp6/q6prm-clocks.c b/sound/soc/qcom/qdsp6/q6prm-clocks.c
> index f613e2aee75e6..b4f9abbe02cb0 100644
> --- a/sound/soc/qcom/qdsp6/q6prm-clocks.c
> +++ b/sound/soc/qcom/qdsp6/q6prm-clocks.c
> @@ -64,34 +64,34 @@ static const struct q6dsp_clk_init q6prm_clks[] = {
> Q6PRM_CLK(LPASS_CLK_ID_WSA2_CORE_TX_MCLK),
> Q6PRM_CLK(LPASS_CLK_ID_WSA2_CORE_TX_2X_MCLK),
> Q6PRM_CLK(LPASS_CLK_ID_RX_CORE_MCLK2_2X_MCLK),
> - Q6PRM_CLK(LAPSS_CLK_ID_QAIF_IF0_IBIT),
[Severity: Low]
Does this patch series break git bisect by splitting a macro rename across
commits?
While this regression is fixed in this commit, it appears the preceding
dt-bindings commit removes LAPSS_CLK_ID_QAIF_IF0_IBIT and related macros
from include/dt-bindings/sound/qcom,q6dsp-lpass-ports.h.
Because q6prm_clks still references those removed macros at the preceding
dt-bindings commit, it causes an intermediate build failure when compiling
the kernel at that point.
Should the dt-bindings commit temporarily retain the old misspelled macros as
aliases until the driver is migrated, or should these changes be squashed?
> - Q6PRM_CLK(LAPSS_CLK_ID_QAIF_IF0_EBIT),
[ ... ]
--
Sashiko AI review · https://sashiko.dev/#/patchset/20260808181948.2489187-1-prasad.kumpatla@oss.qualcomm.com?part=2
^ permalink raw reply [flat|nested] 14+ messages in thread* Re: [PATCH v1 2/5] ASoC: qcom: q6prm: Fix QAIF clock ID typo
2026-08-08 18:45 ` sashiko-bot
@ 2026-08-09 12:27 ` Prasad Kumpatla
0 siblings, 0 replies; 14+ messages in thread
From: Prasad Kumpatla @ 2026-08-09 12:27 UTC (permalink / raw)
To: sashiko-reviews; +Cc: conor+dt, devicetree, robh
On 8/9/2026 12:15 AM, sashiko-bot@kernel.org wrote:
> Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:
> - [Low] The patch series breaks `git bisect` by splitting a macro rename across commits without retaining aliases, causing an intermediate build failure.
> --
>
> --- Patch [2]: [PATCH v1 2/5] ASoC: qcom: q6prm: Fix QAIF clock ID typo ---
> commit 2d0653a87462572279fa5b7912ec981aed567a40
> Author: Prasad Kumpatla <prasad.kumpatla@oss.qualcomm.com>
>
> ASoC: qcom: q6prm: Fix QAIF clock ID typo
>
> The Audio IF QAIF clock IDs were introduced with a "LAPSS" prefix instead
> of "LPASS". This commit renames the definitions and updates their users
> accordingly.
>
> Link: https://sashiko.dev/#/patchset/20260804070307.117119-1-prasad.kumpatla@oss.qualcomm.com
>
>> diff --git a/sound/soc/qcom/qdsp6/q6prm-clocks.c b/sound/soc/qcom/qdsp6/q6prm-clocks.c
>> index f613e2aee75e6..b4f9abbe02cb0 100644
>> --- a/sound/soc/qcom/qdsp6/q6prm-clocks.c
>> +++ b/sound/soc/qcom/qdsp6/q6prm-clocks.c
>> @@ -64,34 +64,34 @@ static const struct q6dsp_clk_init q6prm_clks[] = {
>> Q6PRM_CLK(LPASS_CLK_ID_WSA2_CORE_TX_MCLK),
>> Q6PRM_CLK(LPASS_CLK_ID_WSA2_CORE_TX_2X_MCLK),
>> Q6PRM_CLK(LPASS_CLK_ID_RX_CORE_MCLK2_2X_MCLK),
>> - Q6PRM_CLK(LAPSS_CLK_ID_QAIF_IF0_IBIT),
> [Severity: Low]
> Does this patch series break git bisect by splitting a macro rename across
> commits?
>
> While this regression is fixed in this commit, it appears the preceding
> dt-bindings commit removes LAPSS_CLK_ID_QAIF_IF0_IBIT and related macros
> from include/dt-bindings/sound/qcom,q6dsp-lpass-ports.h.
>
> Because q6prm_clks still references those removed macros at the preceding
> dt-bindings commit, it causes an intermediate build failure when compiling
> the kernel at that point.
>
> Should the dt-bindings commit temporarily retain the old misspelled macros as
> aliases until the driver is migrated, or should these changes be squashed?
Hi Sashiko,
Thanks for reviewing.
The series remains bisectable as the renamed clock IDs are not
referenced by any in-tree users before the corresponding Q6PRM updates.
Therefore, i think aliases or squashing the changes are not required.
Thanks,
Prasad
>
>> - Q6PRM_CLK(LAPSS_CLK_ID_QAIF_IF0_EBIT),
> [ ... ]
>
^ permalink raw reply [flat|nested] 14+ messages in thread
* [PATCH v1 3/5] ASoC: qcom: qdsp6: Zero-initialize AudioReach module config
2026-08-08 18:19 [PATCH v1 0/5] ASoC: qcom: Fix AudioReach TDM review findings Prasad Kumpatla
2026-08-08 18:19 ` [PATCH v1 1/5] dt-bindings: sound: qcom,q6dsp-lpass-ports: Rename QAIF clock IDs Prasad Kumpatla
2026-08-08 18:19 ` [PATCH v1 2/5] ASoC: qcom: q6prm: Fix QAIF clock ID typo Prasad Kumpatla
@ 2026-08-08 18:19 ` Prasad Kumpatla
2026-08-08 18:46 ` sashiko-bot
2026-08-08 18:19 ` [PATCH v1 4/5] ASoC: qcom: common: Distinguish missing and invalid TDM slot configuration Prasad Kumpatla
2026-08-08 18:19 ` [PATCH v1 5/5] ASoC: qcom: sc8280xp: Fix TDM hw_params error handling Prasad Kumpatla
4 siblings, 1 reply; 14+ messages in thread
From: Prasad Kumpatla @ 2026-08-08 18:19 UTC (permalink / raw)
To: Liam Girdwood, Mark Brown, Rob Herring, Krzysztof Kozlowski,
Conor Dooley, Srinivas Kandagatla, Jaroslav Kysela, Takashi Iwai
Cc: linux-arm-msm, linux-sound, devicetree, linux-kernel,
Prasad Kumpatla, Sashiko
q6apm_dai_prepare() and q6apm_dai_compr_set_params() build an
AudioReach module config on the stack and populate only the fields
needed for the stream type.
Audio IF media-format setup treats zero slot_mask, nslots_per_frame and
slot_width as a request to use the topology defaults. Leave those fields
uninitialized and random stack data can override the topology values sent
to the DSP.
Initialize the whole config structure so omitted fields reliably retain
their zero/default meaning.
Reported-by: Sashiko <sashiko-bot@kernel.org>
Link: https://sashiko.dev/#/patchset/20260804070307.117119-1-prasad.kumpatla@oss.qualcomm.com
Signed-off-by: Prasad Kumpatla <prasad.kumpatla@oss.qualcomm.com>
---
sound/soc/qcom/qdsp6/q6apm-dai.c | 4 ++--
1 file changed, 2 insertions(+), 2 deletions(-)
diff --git a/sound/soc/qcom/qdsp6/q6apm-dai.c b/sound/soc/qcom/qdsp6/q6apm-dai.c
index bf1f872a09f4..033197bdd870 100644
--- a/sound/soc/qcom/qdsp6/q6apm-dai.c
+++ b/sound/soc/qcom/qdsp6/q6apm-dai.c
@@ -216,7 +216,7 @@ static int q6apm_dai_prepare(struct snd_soc_component *component,
{
struct snd_pcm_runtime *runtime = substream->runtime;
struct q6apm_dai_rtd *prtd = runtime->private_data;
- struct audioreach_module_config cfg;
+ struct audioreach_module_config cfg = {};
struct device *dev = component->dev;
struct q6apm_dai_data *pdata;
int ret;
@@ -814,7 +814,7 @@ static int q6apm_dai_compr_set_params(struct snd_soc_component *component,
struct snd_compr_runtime *runtime = stream->runtime;
struct q6apm_dai_rtd *prtd = runtime->private_data;
struct q6apm_dai_data *pdata;
- struct audioreach_module_config cfg;
+ struct audioreach_module_config cfg = {};
struct snd_codec *codec = ¶ms->codec;
int dir = stream->direction;
int ret;
--
2.34.1
^ permalink raw reply related [flat|nested] 14+ messages in thread* Re: [PATCH v1 3/5] ASoC: qcom: qdsp6: Zero-initialize AudioReach module config
2026-08-08 18:19 ` [PATCH v1 3/5] ASoC: qcom: qdsp6: Zero-initialize AudioReach module config Prasad Kumpatla
@ 2026-08-08 18:46 ` sashiko-bot
2026-08-09 12:30 ` Prasad Kumpatla
0 siblings, 1 reply; 14+ messages in thread
From: sashiko-bot @ 2026-08-08 18:46 UTC (permalink / raw)
To: Prasad Kumpatla; +Cc: robh, conor+dt, devicetree
Thank you for your contribution! Sashiko AI review found 2 potential issue(s) to consider:
Pre-existing issues:
- [High] Memory leak of audio_buffer array when q6apm_dai_prepare fails.
- [High] Concurrent Use-After-Free in gpr_do_rx_callback() due to lack of synchronization with port teardown.
--
--- Patch [3]: [PATCH v1 3/5] ASoC: qcom: qdsp6: Zero-initialize AudioReach module config ---
commit f7d8f4b6351377745201f54e092ff89f2f6f845e
Author: Prasad Kumpatla <prasad.kumpatla@oss.qualcomm.com>
ASoC: qcom: qdsp6: Zero-initialize AudioReach module config
This commit updates q6apm_dai_prepare() and q6apm_dai_compr_set_params() to
zero-initialize the AudioReach module config structure. This ensures that
uninitialized stack data doesn't override topology defaults sent to the DSP.
Link: https://sashiko.dev/#/patchset/20260804070307.117119-1-prasad.kumpatla@oss.qualcomm.com
> diff --git a/sound/soc/qcom/qdsp6/q6apm-dai.c b/sound/soc/qcom/qdsp6/q6apm-dai.c
> index bf1f872a09f45..033197bdd8705 100644
> --- a/sound/soc/qcom/qdsp6/q6apm-dai.c
> +++ b/sound/soc/qcom/qdsp6/q6apm-dai.c
> @@ -216,7 +216,7 @@ static int q6apm_dai_prepare(struct snd_soc_component *component,
> {
> struct snd_pcm_runtime *runtime = substream->runtime;
> struct q6apm_dai_rtd *prtd = runtime->private_data;
> - struct audioreach_module_config cfg;
> + struct audioreach_module_config cfg = {};
[Severity: High]
This isn't a bug introduced by this patch, but while reviewing the changes
in q6apm_dai_prepare(), I noticed a potential memory leak on the error path.
If q6apm_alloc_fragments() succeeds, but a subsequent command like
q6apm_graph_media_format_pcm() fails:
sound/soc/qcom/qdsp6/q6apm-dai.c:q6apm_dai_prepare() {
...
ret = q6apm_alloc_fragments(...);
if (ret < 0) {
...
}
ret = q6apm_graph_media_format_pcm(prtd->graph, &cfg);
if (ret < 0) {
dev_err(dev, "%s: CMD Format block failed\n", __func__);
return ret;
}
...
prtd->state = Q6APM_STREAM_RUNNING;
}
The function will return without setting prtd->state to Q6APM_STREAM_RUNNING.
Later, when the stream is closed, q6apm_dai_close() skips freeing the
fragments because prtd->state was never set:
sound/soc/qcom/qdsp6/q6apm-dai.c:q6apm_dai_close() {
...
if (prtd->state) {
q6apm_graph_stop(prtd->graph);
q6apm_free_fragments(prtd->graph, substream->stream);
}
q6apm_graph_close(prtd->graph);
}
Does this leave the allocated audio buffer arrays permanently leaked?
[Severity: High]
This is another pre-existing issue I noticed related to closing the stream.
Can an unprivileged local user trigger a use-after-free by repeatedly
opening and closing a stream while the DSP generates events?
When userspace closes the stream, q6apm_graph_close() eventually invokes
gpr_free_port(), which frees the port immediately:
drivers/soc/qcom/apr.c:gpr_free_port() {
spin_lock_irqsave(&gpr->svcs_lock, flags);
idr_remove(&gpr->svcs_idr, port->id);
spin_unlock_irqrestore(&gpr->svcs_lock, flags);
kfree(port);
}
However, gpr_do_rx_callback() processes DSP IPC messages in a workqueue,
dropping the lock before dereferencing the callback:
drivers/soc/qcom/apr.c:gpr_do_rx_callback() {
...
spin_lock_irqsave(&gpr->svcs_lock, flags);
svc = idr_find(&gpr->svcs_idr, hdr->dest_port);
spin_unlock_irqrestore(&gpr->svcs_lock, flags);
if (!svc) { ... }
if (svc->callback)
svc->callback(&resp, svc->priv, 0);
}
If the workqueue is executing concurrently, could it dereference the
already-freed port memory to invoke the callback since the workqueue
isn't flushed prior to freeing the port?
> struct device *dev = component->dev;
> struct q6apm_dai_data *pdata;
> int ret;
[ ... ]
--
Sashiko AI review · https://sashiko.dev/#/patchset/20260808181948.2489187-1-prasad.kumpatla@oss.qualcomm.com?part=3
^ permalink raw reply [flat|nested] 14+ messages in thread* Re: [PATCH v1 3/5] ASoC: qcom: qdsp6: Zero-initialize AudioReach module config
2026-08-08 18:46 ` sashiko-bot
@ 2026-08-09 12:30 ` Prasad Kumpatla
0 siblings, 0 replies; 14+ messages in thread
From: Prasad Kumpatla @ 2026-08-09 12:30 UTC (permalink / raw)
To: sashiko-reviews; +Cc: robh, conor+dt, devicetree
On 8/9/2026 12:16 AM, sashiko-bot@kernel.org wrote:
> Thank you for your contribution! Sashiko AI review found 2 potential issue(s) to consider:
>
> Pre-existing issues:
> - [High] Memory leak of audio_buffer array when q6apm_dai_prepare fails.
> - [High] Concurrent Use-After-Free in gpr_do_rx_callback() due to lack of synchronization with port teardown.
> --
>
> --- Patch [3]: [PATCH v1 3/5] ASoC: qcom: qdsp6: Zero-initialize AudioReach module config ---
> commit f7d8f4b6351377745201f54e092ff89f2f6f845e
> Author: Prasad Kumpatla <prasad.kumpatla@oss.qualcomm.com>
>
> ASoC: qcom: qdsp6: Zero-initialize AudioReach module config
>
> This commit updates q6apm_dai_prepare() and q6apm_dai_compr_set_params() to
> zero-initialize the AudioReach module config structure. This ensures that
> uninitialized stack data doesn't override topology defaults sent to the DSP.
>
> Link: https://sashiko.dev/#/patchset/20260804070307.117119-1-prasad.kumpatla@oss.qualcomm.com
>
>> diff --git a/sound/soc/qcom/qdsp6/q6apm-dai.c b/sound/soc/qcom/qdsp6/q6apm-dai.c
>> index bf1f872a09f45..033197bdd8705 100644
>> --- a/sound/soc/qcom/qdsp6/q6apm-dai.c
>> +++ b/sound/soc/qcom/qdsp6/q6apm-dai.c
>> @@ -216,7 +216,7 @@ static int q6apm_dai_prepare(struct snd_soc_component *component,
>> {
>> struct snd_pcm_runtime *runtime = substream->runtime;
>> struct q6apm_dai_rtd *prtd = runtime->private_data;
>> - struct audioreach_module_config cfg;
>> + struct audioreach_module_config cfg = {};
> [Severity: High]
> This isn't a bug introduced by this patch, but while reviewing the changes
> in q6apm_dai_prepare(), I noticed a potential memory leak on the error path.
>
> If q6apm_alloc_fragments() succeeds, but a subsequent command like
> q6apm_graph_media_format_pcm() fails:
>
> sound/soc/qcom/qdsp6/q6apm-dai.c:q6apm_dai_prepare() {
> ...
> ret = q6apm_alloc_fragments(...);
> if (ret < 0) {
> ...
> }
>
> ret = q6apm_graph_media_format_pcm(prtd->graph, &cfg);
> if (ret < 0) {
> dev_err(dev, "%s: CMD Format block failed\n", __func__);
> return ret;
> }
> ...
> prtd->state = Q6APM_STREAM_RUNNING;
> }
>
> The function will return without setting prtd->state to Q6APM_STREAM_RUNNING.
> Later, when the stream is closed, q6apm_dai_close() skips freeing the
> fragments because prtd->state was never set:
>
> sound/soc/qcom/qdsp6/q6apm-dai.c:q6apm_dai_close() {
> ...
> if (prtd->state) {
> q6apm_graph_stop(prtd->graph);
> q6apm_free_fragments(prtd->graph, substream->stream);
> }
>
> q6apm_graph_close(prtd->graph);
> }
>
> Does this leave the allocated audio buffer arrays permanently leaked?
>
> [Severity: High]
> This is another pre-existing issue I noticed related to closing the stream.
> Can an unprivileged local user trigger a use-after-free by repeatedly
> opening and closing a stream while the DSP generates events?
Hi Sashiko,
Thanks for Reviewing.
These issues are pre-existing and not introduced by this change,
which only fixes the initialization of audioreach_module_config.
I'd prefer to keep this patch focused and handle the error-path
and teardown concerns separately.
Thanks,
Prasad
>
> When userspace closes the stream, q6apm_graph_close() eventually invokes
> gpr_free_port(), which frees the port immediately:
>
> drivers/soc/qcom/apr.c:gpr_free_port() {
> spin_lock_irqsave(&gpr->svcs_lock, flags);
> idr_remove(&gpr->svcs_idr, port->id);
> spin_unlock_irqrestore(&gpr->svcs_lock, flags);
>
> kfree(port);
> }
>
> However, gpr_do_rx_callback() processes DSP IPC messages in a workqueue,
> dropping the lock before dereferencing the callback:
>
> drivers/soc/qcom/apr.c:gpr_do_rx_callback() {
> ...
> spin_lock_irqsave(&gpr->svcs_lock, flags);
> svc = idr_find(&gpr->svcs_idr, hdr->dest_port);
> spin_unlock_irqrestore(&gpr->svcs_lock, flags);
>
> if (!svc) { ... }
>
> if (svc->callback)
> svc->callback(&resp, svc->priv, 0);
> }
>
> If the workqueue is executing concurrently, could it dereference the
> already-freed port memory to invoke the callback since the workqueue
> isn't flushed prior to freeing the port?
>
>> struct device *dev = component->dev;
>> struct q6apm_dai_data *pdata;
>> int ret;
> [ ... ]
>
^ permalink raw reply [flat|nested] 14+ messages in thread
* [PATCH v1 4/5] ASoC: qcom: common: Distinguish missing and invalid TDM slot configuration
2026-08-08 18:19 [PATCH v1 0/5] ASoC: qcom: Fix AudioReach TDM review findings Prasad Kumpatla
` (2 preceding siblings ...)
2026-08-08 18:19 ` [PATCH v1 3/5] ASoC: qcom: qdsp6: Zero-initialize AudioReach module config Prasad Kumpatla
@ 2026-08-08 18:19 ` Prasad Kumpatla
2026-08-08 18:39 ` sashiko-bot
2026-08-08 18:19 ` [PATCH v1 5/5] ASoC: qcom: sc8280xp: Fix TDM hw_params error handling Prasad Kumpatla
4 siblings, 1 reply; 14+ messages in thread
From: Prasad Kumpatla @ 2026-08-08 18:19 UTC (permalink / raw)
To: Liam Girdwood, Mark Brown, Rob Herring, Krzysztof Kozlowski,
Conor Dooley, Srinivas Kandagatla, Jaroslav Kysela, Takashi Iwai
Cc: linux-arm-msm, linux-sound, devicetree, linux-kernel,
Prasad Kumpatla, Sashiko
qcom_snd_parse_dai_tdm_slots() uses -EINVAL for both missing DAI-link
TDM configuration and malformed TDM slot properties. As a result,
qcom_snd_apply_dai_tdm_slots() silently ignores configuration errors.
Return -ENOENT for missing DAI-link configuration and preserve -EINVAL
for malformed TDM slot properties.
Reported-by: Sashiko <sashiko-bot@kernel.org>
Link: https://sashiko.dev/#/patchset/20260804070307.117119-1-prasad.kumpatla@oss.qualcomm.com
Signed-off-by: Prasad Kumpatla <prasad.kumpatla@oss.qualcomm.com>
---
sound/soc/qcom/common.c | 6 +++---
1 file changed, 3 insertions(+), 3 deletions(-)
diff --git a/sound/soc/qcom/common.c b/sound/soc/qcom/common.c
index d231024206db..d9f256d51973 100644
--- a/sound/soc/qcom/common.c
+++ b/sound/soc/qcom/common.c
@@ -103,14 +103,14 @@ static int qcom_snd_parse_dai_tdm_slots(struct snd_soc_pcm_runtime *rtd,
int ret;
if (!link_np)
- return -EINVAL;
+ return -ENOENT;
struct device_node *cpu_np __free(device_node) =
of_get_child_by_name(link_np, "cpu");
struct device_node *codec_np __free(device_node) =
of_get_child_by_name(link_np, "codec");
if (!cpu_np || !codec_np)
- return -EINVAL;
+ return -ENOENT;
ret = qcom_snd_parse_tdm_slot(cpu_np, cpu_cfg);
if (ret)
@@ -172,7 +172,7 @@ int qcom_snd_apply_dai_tdm_slots(struct snd_soc_pcm_runtime *rtd)
ret = qcom_snd_get_dai_tdm_slots(rtd, &cpu_cfg, &codec_cfg);
if (ret)
- return ret == -EINVAL ? 0 : ret;
+ return ret == -ENOENT ? 0 : ret;
return qcom_snd_apply_dai_tdm_slots_cfg(rtd, &cpu_cfg, &codec_cfg);
}
--
2.34.1
^ permalink raw reply related [flat|nested] 14+ messages in thread* Re: [PATCH v1 4/5] ASoC: qcom: common: Distinguish missing and invalid TDM slot configuration
2026-08-08 18:19 ` [PATCH v1 4/5] ASoC: qcom: common: Distinguish missing and invalid TDM slot configuration Prasad Kumpatla
@ 2026-08-08 18:39 ` sashiko-bot
2026-08-09 12:32 ` Prasad Kumpatla
0 siblings, 1 reply; 14+ messages in thread
From: sashiko-bot @ 2026-08-08 18:39 UTC (permalink / raw)
To: Prasad Kumpatla; +Cc: devicetree, robh, conor+dt
Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:
- [High] Incomplete API update breaks TDM configuration parsing and error handling in the sc8280xp machine driver.
--
--- Patch [4]: [PATCH v1 4/5] ASoC: qcom: common: Distinguish missing and invalid TDM slot configuration ---
commit 2828c15bbb01105bac7c7698b0dd46a75894895f
Author: Prasad Kumpatla <prasad.kumpatla@oss.qualcomm.com>
ASoC: qcom: common: Distinguish missing and invalid TDM slot configuration
This commit modifies the TDM slot configuration parser to return -ENOENT for
missing DAI-link configuration while preserving -EINVAL for malformed
properties. It also updates qcom_snd_apply_dai_tdm_slots() to handle the new
-ENOENT return code.
Link: https://sashiko.dev/#/patchset/20260804070307.117119-1-prasad.kumpatla@oss.qualcomm.com
> diff --git a/sound/soc/qcom/common.c b/sound/soc/qcom/common.c
> index d231024206dbf..d9f256d519733 100644
> --- a/sound/soc/qcom/common.c
> +++ b/sound/soc/qcom/common.c
[ ... ]
> @@ -172,7 +172,7 @@ int qcom_snd_apply_dai_tdm_slots(struct snd_soc_pcm_runtime *rtd)
>
> ret = qcom_snd_get_dai_tdm_slots(rtd, &cpu_cfg, &codec_cfg);
> if (ret)
> - return ret == -EINVAL ? 0 : ret;
> + return ret == -ENOENT ? 0 : ret;
[Severity: High]
Does this API change break other callers that were not updated?
Looking at sc8280xp_tdm_hw_params() in sound/soc/qcom/sc8280xp.c, it still
expects -EINVAL for missing TDM configuration:
sound/soc/qcom/sc8280xp.c:sc8280xp_tdm_hw_params() {
...
ret = qcom_snd_get_dai_tdm_slots(rtd, &cpu_cfg, &codec_cfg);
if (ret)
return ret == -EINVAL ? 0 : ret;
...
}
Will this cause hw_params to fail when TDM configuration is intentionally
missing, since it now receives -ENOENT?
Could this also silently succeed on malformed configurations that return
-EINVAL, defeating the intent of the patch?
>
> return qcom_snd_apply_dai_tdm_slots_cfg(rtd, &cpu_cfg, &codec_cfg);
> }
--
Sashiko AI review · https://sashiko.dev/#/patchset/20260808181948.2489187-1-prasad.kumpatla@oss.qualcomm.com?part=4
^ permalink raw reply [flat|nested] 14+ messages in thread* Re: [PATCH v1 4/5] ASoC: qcom: common: Distinguish missing and invalid TDM slot configuration
2026-08-08 18:39 ` sashiko-bot
@ 2026-08-09 12:32 ` Prasad Kumpatla
0 siblings, 0 replies; 14+ messages in thread
From: Prasad Kumpatla @ 2026-08-09 12:32 UTC (permalink / raw)
To: sashiko-reviews; +Cc: devicetree, robh, conor+dt
On 8/9/2026 12:09 AM, sashiko-bot@kernel.org wrote:
> Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:
> - [High] Incomplete API update breaks TDM configuration parsing and error handling in the sc8280xp machine driver.
> --
>
> --- Patch [4]: [PATCH v1 4/5] ASoC: qcom: common: Distinguish missing and invalid TDM slot configuration ---
> commit 2828c15bbb01105bac7c7698b0dd46a75894895f
> Author: Prasad Kumpatla <prasad.kumpatla@oss.qualcomm.com>
>
> ASoC: qcom: common: Distinguish missing and invalid TDM slot configuration
>
> This commit modifies the TDM slot configuration parser to return -ENOENT for
> missing DAI-link configuration while preserving -EINVAL for malformed
> properties. It also updates qcom_snd_apply_dai_tdm_slots() to handle the new
> -ENOENT return code.
>
> Link: https://sashiko.dev/#/patchset/20260804070307.117119-1-prasad.kumpatla@oss.qualcomm.com
>
>> diff --git a/sound/soc/qcom/common.c b/sound/soc/qcom/common.c
>> index d231024206dbf..d9f256d519733 100644
>> --- a/sound/soc/qcom/common.c
>> +++ b/sound/soc/qcom/common.c
> [ ... ]
>> @@ -172,7 +172,7 @@ int qcom_snd_apply_dai_tdm_slots(struct snd_soc_pcm_runtime *rtd)
>>
>> ret = qcom_snd_get_dai_tdm_slots(rtd, &cpu_cfg, &codec_cfg);
>> if (ret)
>> - return ret == -EINVAL ? 0 : ret;
>> + return ret == -ENOENT ? 0 : ret;
> [Severity: High]
> Does this API change break other callers that were not updated?
>
> Looking at sc8280xp_tdm_hw_params() in sound/soc/qcom/sc8280xp.c, it still
> expects -EINVAL for missing TDM configuration:
>
> sound/soc/qcom/sc8280xp.c:sc8280xp_tdm_hw_params() {
> ...
> ret = qcom_snd_get_dai_tdm_slots(rtd, &cpu_cfg, &codec_cfg);
> if (ret)
> return ret == -EINVAL ? 0 : ret;
> ...
> }
>
> Will this cause hw_params to fail when TDM configuration is intentionally
> missing, since it now receives -ENOENT?
>
> Could this also silently succeed on malformed configurations that return
> -EINVAL, defeating the intent of the patch?
Hi Sashiko,
Thanks for Reviewing.
qcom_snd_get_dai_tdm_slots() has only two callers. Both are updated
in this series to treat -ENOENT as "configuration absent", while
invalid DT properties continue to return -EINVAL. As a result,
malformed configurations are still reported as errors and are not
silently accepted.
Thanks,
Prasad
>
>>
>> return qcom_snd_apply_dai_tdm_slots_cfg(rtd, &cpu_cfg, &codec_cfg);
>> }
^ permalink raw reply [flat|nested] 14+ messages in thread
* [PATCH v1 5/5] ASoC: qcom: sc8280xp: Fix TDM hw_params error handling
2026-08-08 18:19 [PATCH v1 0/5] ASoC: qcom: Fix AudioReach TDM review findings Prasad Kumpatla
` (3 preceding siblings ...)
2026-08-08 18:19 ` [PATCH v1 4/5] ASoC: qcom: common: Distinguish missing and invalid TDM slot configuration Prasad Kumpatla
@ 2026-08-08 18:19 ` Prasad Kumpatla
4 siblings, 0 replies; 14+ messages in thread
From: Prasad Kumpatla @ 2026-08-08 18:19 UTC (permalink / raw)
To: Liam Girdwood, Mark Brown, Rob Herring, Krzysztof Kozlowski,
Conor Dooley, Srinivas Kandagatla, Jaroslav Kysela, Takashi Iwai
Cc: linux-arm-msm, linux-sound, devicetree, linux-kernel,
Prasad Kumpatla, Sashiko
Treat -ENOENT from TDM slot parsing as the optional "configuration not
present" case and continue to propagate real configuration errors.
Also ignore -ENOTSUPP from optional DAI format and codec sysclk
callbacks, apply codec_dai_fmt to codec DAIs on TDM links, and use a
signed bclk_freq variable so errors from snd_soc_tdm_params_to_bclk()
are handled correctly.
Reported-by: Sashiko <sashiko-bot@kernel.org>
Link: https://sashiko.dev/#/patchset/20260804070307.117119-1-prasad.kumpatla@oss.qualcomm.com
Signed-off-by: Prasad Kumpatla <prasad.kumpatla@oss.qualcomm.com>
---
sound/soc/qcom/sc8280xp.c | 21 +++++++++++++++------
1 file changed, 15 insertions(+), 6 deletions(-)
diff --git a/sound/soc/qcom/sc8280xp.c b/sound/soc/qcom/sc8280xp.c
index 597c0d887d2f..0120443da418 100644
--- a/sound/soc/qcom/sc8280xp.c
+++ b/sound/soc/qcom/sc8280xp.c
@@ -115,33 +115,42 @@ static int sc8280xp_tdm_hw_params(struct snd_pcm_substream *substream,
struct snd_soc_dai *codec_dai;
struct qcom_snd_tdm_slot_cfg cpu_cfg;
struct qcom_snd_tdm_slot_cfg codec_cfg;
- unsigned int bclk_freq;
+ int bclk_freq;
int ret;
int i;
ret = qcom_snd_get_dai_tdm_slots(rtd, &cpu_cfg, &codec_cfg);
if (ret)
- return ret == -EINVAL ? 0 : ret;
+ return ret == -ENOENT ? 0 : ret;
if (!cpu_cfg.slots)
return 0;
ret = snd_soc_dai_set_fmt(cpu_dai, SND_SOC_DAIFMT_BP_FP);
- if (ret)
+ if (ret && ret != -ENOTSUPP)
return ret;
+ if (data->priv->codec_dai_fmt) {
+ for_each_rtd_codec_dais(rtd, i, codec_dai) {
+ ret = snd_soc_dai_set_fmt(codec_dai,
+ data->priv->codec_dai_fmt);
+ if (ret && ret != -ENOTSUPP)
+ return ret;
+ }
+ }
+
ret = qcom_snd_apply_dai_tdm_slots_cfg(rtd, &cpu_cfg, &codec_cfg);
if (ret)
return ret;
bclk_freq = snd_soc_tdm_params_to_bclk(params, cpu_cfg.slot_width, cpu_cfg.slots, 1);
- if (!bclk_freq)
+ if (bclk_freq <= 0)
return -EINVAL;
if (data->priv->mi2s_bclk_enable) {
ret = snd_soc_dai_set_sysclk(cpu_dai, LPAIF_MI2S_BCLK, bclk_freq,
SND_SOC_CLOCK_IN);
- if (ret) {
+ if (ret && ret != -ENOTSUPP) {
dev_err(rtd->dev, "%s: failed to set cpu sysclk: %d\n",
__func__, ret);
return ret;
@@ -152,7 +161,7 @@ static int sc8280xp_tdm_hw_params(struct snd_pcm_substream *substream,
for_each_rtd_codec_dais(rtd, i, codec_dai) {
ret = snd_soc_dai_set_sysclk(codec_dai, 0, bclk_freq,
SND_SOC_CLOCK_IN);
- if (ret) {
+ if (ret && ret != -ENOTSUPP) {
dev_err(rtd->dev, "%s: failed to set codec sysclk on %s: %d\n",
__func__, codec_dai->name, ret);
return ret;
--
2.34.1
^ permalink raw reply related [flat|nested] 14+ messages in thread