Devicetree
 help / color / mirror / Atom feed
* [PATCH v1 0/5] ASoC: qcom: Fix AudioReach TDM review findings
@ 2026-08-08 18:19 Prasad Kumpatla
  2026-08-08 18:19 ` [PATCH v1 1/5] dt-bindings: sound: qcom,q6dsp-lpass-ports: Rename QAIF clock IDs Prasad Kumpatla
                   ` (4 more replies)
  0 siblings, 5 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

Address review findings for the recently merged AudioReach TDM backend
support.

The first 2 patch fixes typo in the newly added QAIF clock ID names by
renaming the LAPSS-prefixed identifiers to LPASS and updating the q6prm
users accordingly.

The remaining patches address issues identified in the AudioReach TDM
configuration and machine-driver paths:

  - Zero-initialize AudioReach module configuration structures so
    unspecified slot parameters correctly fall back to topology defaults.

  - Distinguish between missing TDM configuration and malformed TDM slot
    properties by using separate return codes, preventing valid DT
    configuration errors from being silently ignored.

  - Align sc8280xp TDM hw_params error handling with the existing MI2S
    implementation, including proper handling of optional callbacks and
    error propagation.

All these Issues are reported by sashiko. 
Link: https://sashiko.dev/#/patchset/20260804070307.117119-1-prasad.kumpatla@oss.qualcomm.com

Prasad Kumpatla (5):
  dt-bindings: sound: qcom,q6dsp-lpass-ports: Rename QAIF clock IDs
  ASoC: qcom: q6prm: Fix QAIF clock ID typo
  ASoC: qcom: qdsp6: Zero-initialize AudioReach module config
  ASoC: qcom: common: Distinguish missing and invalid TDM slot
    configuration
  ASoC: qcom: sc8280xp: Fix TDM hw_params error handling

 .../sound/qcom,q6dsp-lpass-ports.h            | 56 +++++++++----------
 sound/soc/qcom/common.c                       |  6 +-
 sound/soc/qcom/qdsp6/q6apm-dai.c              |  4 +-
 sound/soc/qcom/qdsp6/q6prm-clocks.c           | 56 +++++++++----------
 sound/soc/qcom/qdsp6/q6prm.h                  | 56 +++++++++----------
 sound/soc/qcom/sc8280xp.c                     | 21 +++++--
 6 files changed, 104 insertions(+), 95 deletions(-)

-- 
2.34.1


^ permalink raw reply	[flat|nested] 14+ messages in thread

* [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

* [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

* [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 = &params->codec;
 	int dir = stream->direction;
 	int ret;
-- 
2.34.1

^ permalink raw reply related	[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

* [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

* 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 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 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 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 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

* 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

* 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

* 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

end of thread, other threads:[~2026-08-09 12:32 UTC | newest]

Thread overview: 14+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
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:45   ` sashiko-bot
2026-08-09 12:25     ` 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:45   ` sashiko-bot
2026-08-09 12:27     ` Prasad Kumpatla
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
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
2026-08-08 18:19 ` [PATCH v1 5/5] ASoC: qcom: sc8280xp: Fix TDM hw_params error handling Prasad Kumpatla

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox