[PATCH v2 0/4] ASoC: qcom: Fix AudioReach TDM review findings

[PATCH v2 0/4] ASoC: qcom: Fix AudioReach TDM review findings

[Prasad Kumpatla]

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

The first patch fixes a typo in the newly added QAIF clock ID names by
renaming the LAPSS-prefixed identifiers to LPASS and updating all
corresponding in-tree users.

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.

  - Fix sc8280xp TDM hw_params error handling by properly distinguishing
    optional and invalid TDM slot configurations and propagating errors
    accordingly.

All issues were reported by Sashiko.

Link:
https://sashiko.dev/#/patchset/20260804070307.117119-1-prasad.kumpatla@oss.qualcomm.com

Changes in v2:
 - Folded patches 1 and 2 into a single patch so the QAIF clock ID
   rename and its users are updated together.
 - Avoid an intermediate build failure and keep the series buildable
   on a per-patch basis.

Prasad Kumpatla (4):
  dt-bindings: sound: qcom,q6dsp-lpass-ports: Rename QAIF clock IDs
  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

[PATCH v2 1/4] dt-bindings: sound: qcom,q6dsp-lpass-ports: Rename QAIF clock IDs

[Prasad Kumpatla]

The QAIF clock IDs were introduced with a LAPSS prefix typo. Rename
them to use the existing LPASS naming convention and update all
corresponding in-tree users.

Since these IDs are new, rename them directly instead of keeping
backward-compatible 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 +++++++++----------
 sound/soc/qcom/qdsp6/q6prm-clocks.c           | 56 +++++++++----------
 sound/soc/qcom/qdsp6/q6prm.h                  | 56 +++++++++----------
 3 files changed, 84 insertions(+), 84 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
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

Re: [PATCH v2 1/4] dt-bindings: sound: qcom,q6dsp-lpass-ports: Rename QAIF clock IDs

[Mark Brown]

[-- Attachment #1: Type: text/plain, Size: 563 bytes --]

On Tue, Aug 18, 2026 at 02:16:52PM +0530, Prasad Kumpatla wrote:
> The QAIF clock IDs were introduced with a LAPSS prefix typo. Rename
> them to use the existing LPASS naming convention and update all
> corresponding in-tree users.

Please submit patches using subject lines reflecting the style for the
subsystem, this makes it easier for people to identify relevant patches.
Look at what existing commits in the area you're changing are doing and
make sure your subject lines visually resemble what they're doing.
There's no need to resubmit to fix this alone.

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 488 bytes --]

[PATCH v2 2/4] ASoC: qcom: qdsp6: Zero-initialize AudioReach module config

[Prasad Kumpatla]

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

Re: [PATCH v2 2/4] ASoC: qcom: qdsp6: Zero-initialize AudioReach module config

[sashiko-bot]

Thank you for your contribution! Sashiko AI review found 2 potential issue(s) to consider:

Pre-existing issues:
- [High] Missing dma_rmb() between reads of frame_counter and index in q6apm_dai_pointer().
- [Medium] Memory leak of dynamically allocated fragment buffers on error paths.
--

--- Patch [2]: [PATCH v2 2/4] ASoC: qcom: qdsp6: Zero-initialize AudioReach module config ---
commit 8c62450bf2680c8f23f31e1b33efb46a0ed00f8a
Author: Prasad Kumpatla <prasad.kumpatla@oss.qualcomm.com>

ASoC: qcom: qdsp6: Zero-initialize AudioReach module config

q6apm_dai_prepare() and q6apm_dai_compr_set_params() build an AudioReach
module config on the stack. Initialize the whole config structure so omitted
fields reliably retain their zero/default meaning.

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: Medium]
This is a pre-existing issue, but while looking at q6apm_dai_prepare() in
sound/soc/qcom/qdsp6/q6apm-dai.c, could there be a memory leak of the
dynamically allocated fragment buffers on the error paths?

If a failure occurs during setup, such as q6apm_graph_media_format_pcm()
returning an error:

	ret = q6apm_alloc_fragments(prtd->graph, substream->stream, prtd->phys,
				(prtd->pcm_size / prtd->periods), prtd->periods);
...
	ret = q6apm_graph_media_format_pcm(prtd->graph, &cfg);
	if (ret < 0) {
		dev_err(dev, "%s: CMD Format block failed\n", __func__);
		return ret;
	}

The function returns without setting prtd->state to Q6APM_STREAM_RUNNING.

Later, when ALSA cleans up and calls q6apm_dai_close(), the teardown
logic skips the cleanup of the buffers because prtd->state is 0:

	if (prtd->state) {
		/* only stop graph that is started */
		q6apm_graph_stop(prtd->graph);
		q6apm_free_fragments(prtd->graph, substream->stream);
	}

	q6apm_graph_close(prtd->graph);

This frees the graph structure itself, losing the pointers to the buffers
and permanently leaking the memory.

Should the error paths in q6apm_dai_prepare() clean up the allocated
fragments, or should q6apm_dai_close() be adjusted to free them even if
the stream isn't fully running?

>  	struct device *dev = component->dev;
>  	struct q6apm_dai_data *pdata;
>  	int ret;

[ ... ]

[Severity: High]
This isn't a bug introduced by this patch, but while reviewing the changes
in sound/soc/qcom/qdsp6/q6apm-dai.c, I noticed a potential issue in
q6apm_dai_pointer(). Are read memory barriers needed in the retry loop
when reading from shared DMA memory?

q6apm_dai_pointer() contains this loop:

		/* index is valid if frame_counter does not change while reading. */
		do {
			fc1 = READ_ONCE(prtd->pos_buffer->frame_counter);
			index = READ_ONCE(prtd->pos_buffer->index);
			fc2 = READ_ONCE(prtd->pos_buffer->frame_counter);
		} while (fc1 != fc2 && --retries);

On weakly-ordered architectures like ARM64, is the CPU permitted to reorder
these reads from the coherent DMA memory? If index is read before fc1, or
fc2 is read before index, might the validation fail to detect a concurrent
update by the DSP, leading to an incorrect hardware pointer snapshot being
returned?

Would adding dma_rmb() between these reads ensure the correct ordering?

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20260818084655.3240284-1-prasad.kumpatla@oss.qualcomm.com?part=2

[PATCH v2 3/4] ASoC: qcom: common: Distinguish missing and invalid TDM slot configuration

[Prasad Kumpatla]

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

Re: [PATCH v2 3/4] ASoC: qcom: common: Distinguish missing and invalid TDM slot configuration

[sashiko-bot]

Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:
- [High] Un-updated caller sc8280xp_tdm_hw_params() causes PCM initialization failure for missing TDM configurations and ignores malformed ones.
--

--- Patch [3]: [PATCH v2 3/4] ASoC: qcom: common: Distinguish missing and invalid TDM slot configuration ---
commit 6f19acb6aca2ed0950dcc6a8cc707f166eb0eb8c
Author: Prasad Kumpatla <prasad.kumpatla@oss.qualcomm.com>

ASoC: qcom: common: Distinguish missing and invalid TDM slot configuration

Return -ENOENT for missing DAI-link configuration and preserve -EINVAL
for malformed TDM slot properties.

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
> @@ -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;

[Severity: High]
Does changing the return value here also require an update to
sc8280xp_tdm_hw_params() in sound/soc/qcom/sc8280xp.c?

Looking at sc8280xp_tdm_hw_params(), it calls qcom_snd_get_dai_tdm_slots()
which now returns -ENOENT for missing configurations. However, it still
appears to check for -EINVAL:

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 sc8280xp_tdm_hw_params() to mistakenly return an error
(-ENOENT) when the configuration is missing, and incorrectly ignore the
error (-EINVAL) when the configuration is actually malformed?

>  
>  	return qcom_snd_apply_dai_tdm_slots_cfg(rtd, &cpu_cfg, &codec_cfg);
>  }

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20260818084655.3240284-1-prasad.kumpatla@oss.qualcomm.com?part=3

[PATCH v2 4/4] ASoC: qcom: sc8280xp: Fix TDM hw_params error handling

[Prasad Kumpatla]

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 1322601659d4..4d48e1012cd4 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