[PATCH RFC v2 0/6] Speakers for Pixel 3 / 3 XL

[PATCH RFC v2 0/6] Speakers for Pixel 3 / 3 XL

[David Heidelberg via B4 Relay]

This series address:

0. demistify the hex to BIT() macro and add left/right rx spk
1. most likely wrongly set DSP_B in sdm845.c
   my guess this is because TDM was never used in the mainline with
   sdm845
2. applying correct mask to left/right speaker for the TDM in sdm845.c,
   so both speakers can work simultaneusly
3. setting sysclk at startups, which some codecs need (such as cs35l36)
4. setting proper tdm slot in the cs35l36 driver
5. device-tree wiring for Pixel 3 / 3 XL

Consider my knowledge here limited, thus sending RFC. Some parts we're
heavily assisted by LLM.

David

Signed-off-by: David Heidelberg <david@ixit.cz>
---
Changes in v2:
- Device-tree name sorting, with exception keeping mm1-dai-link in it's
  place. (Konrad)
- Make set_tdm_slot cleaner and less leveled. (Konrad)
- Added TDM MIC defines in sdm845.c.
  Should we in capture use speaker mask?
- Renamed model from 'Google Pixel 3' to 'blueline', I would love to
  hear feedback on this one.
- Link to v1: https://lore.kernel.org/r/20260705-pixel3-audio-v1-0-3b66f33859f1@ixit.cz

---
David Heidelberg (6):
      ASoC: qcom: sdm845: Demystify TDM masks a bit
      ASoC: qcom: sdm845: use DSP_A format for TDM codec DAIs
      ASoC: qcom: sdm845: Use per-speaker RX masks for TDM slot assignment
      ASoC: qcom: sdm845: Set codec dai and component sysclk during startup
      ASoC: cs35l36: Implement set_tdm_slot to program RX and TX slots
      arm64: dts: qcom: sdm845-google: Add basic audio support

 arch/arm64/boot/dts/qcom/sdm845-google-common.dtsi | 132 ++++++++++++++++++++-
 sound/soc/codecs/cs35l36.c                         |  44 +++++++
 sound/soc/qcom/sdm845.c                            |  49 ++++++--
 3 files changed, 213 insertions(+), 12 deletions(-)
---
base-commit: 7079a12d7506b07fb53b54a664bfad5fa9b16d70
change-id: 20260613-pixel3-audio-bdbfb49e8037

Best regards,
--  
David Heidelberg <david@ixit.cz>

[PATCH RFC v2 1/6] ASoC: qcom: sdm845: Demystify TDM masks a bit

[David Heidelberg via B4 Relay]

From: David Heidelberg <david@ixit.cz>

Describe the mask with the bits used for each RX/TX.

Signed-off-by: David Heidelberg <david@ixit.cz>
---
 sound/soc/qcom/sdm845.c | 21 ++++++++++++++-------
 1 file changed, 14 insertions(+), 7 deletions(-)

diff --git a/sound/soc/qcom/sdm845.c b/sound/soc/qcom/sdm845.c
index 6843ab8ba017e..0ce8265ab1c1f 100644
--- a/sound/soc/qcom/sdm845.c
+++ b/sound/soc/qcom/sdm845.c
@@ -18,19 +18,24 @@
 #include "sdw.h"
 #include "../codecs/rt5663.h"
 
 #define DRIVER_NAME	"sdm845"
 #define DEFAULT_SAMPLE_RATE_48K		48000
 #define DEFAULT_MCLK_RATE		24576000
 #define TDM_BCLK_RATE		6144000
 #define MI2S_BCLK_RATE		1536000
-#define LEFT_SPK_TDM_TX_MASK    0x30
-#define RIGHT_SPK_TDM_TX_MASK   0xC0
-#define SPK_TDM_RX_MASK         0x03
+#define LEFT_SPK_TDM_RX_MASK	BIT(0)
+#define RIGHT_SPK_TDM_RX_MASK	BIT(1)
+#define SPK_TDM_RX_MASK		(LEFT_SPK_TDM_RX_MASK | RIGHT_SPK_TDM_RX_MASK)
+#define MIC1_TDM_RX_MASK	BIT(2)
+#define MIC2_TDM_RX_MASK	BIT(3)
+#define MIC_TDM_RX_MASK		(MIC1_TDM_RX_MASK | MIC2_TDM_RX_MASK)
+#define LEFT_SPK_TDM_TX_MASK	(BIT(4) | BIT(5))
+#define RIGHT_SPK_TDM_TX_MASK	(BIT(6) | BIT(7))
 #define NUM_TDM_SLOTS           8
 #define SLIM_MAX_TX_PORTS 16
 #define SLIM_MAX_RX_PORTS 13
 #define WCD934X_DEFAULT_MCLK_RATE	9600000
 
 struct sdm845_snd_data {
 	struct snd_soc_jack jack;
 	bool jack_setup;
@@ -107,34 +112,36 @@ static int sdm845_tdm_snd_hw_params(struct snd_pcm_substream *substream,
 	default:
 		dev_err(rtd->dev, "%s: invalid param format 0x%x\n",
 				__func__, params_format(params));
 		return -EINVAL;
 	}
 
 	channels = params_channels(params);
 	if (substream->stream == SNDRV_PCM_STREAM_PLAYBACK) {
-		ret = snd_soc_dai_set_tdm_slot(cpu_dai, 0, 0x3,
-				8, slot_width);
+		ret = snd_soc_dai_set_tdm_slot(cpu_dai, 0, SPK_TDM_RX_MASK,
+					       NUM_TDM_SLOTS, slot_width);
 		if (ret < 0) {
 			dev_err(rtd->dev, "%s: failed to set tdm slot, err:%d\n",
 					__func__, ret);
 			goto end;
 		}
 
 		ret = snd_soc_dai_set_channel_map(cpu_dai, 0, NULL,
 				channels, tdm_slot_offset);
 		if (ret < 0) {
 			dev_err(rtd->dev, "%s: failed to set channel map, err:%d\n",
 					__func__, ret);
 			goto end;
 		}
 	} else {
-		ret = snd_soc_dai_set_tdm_slot(cpu_dai, 0xf, 0,
-				8, slot_width);
+		ret = snd_soc_dai_set_tdm_slot(cpu_dai,
+					       SPK_TDM_RX_MASK |
+					       MIC_TDM_RX_MASK, 0,
+					       NUM_TDM_SLOTS, slot_width);
 		if (ret < 0) {
 			dev_err(rtd->dev, "%s: failed to set tdm slot, err:%d\n",
 					__func__, ret);
 			goto end;
 		}
 
 		ret = snd_soc_dai_set_channel_map(cpu_dai, channels,
 				tdm_slot_offset, 0, NULL);

-- 
2.55.0

Re: [PATCH RFC v2 1/6] ASoC: qcom: sdm845: Demystify TDM masks a bit

[sashiko-bot]

Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:
- [Low] Incorrect semantic definition and conflation of TDM RX/TX masks for the CPU capture path.
--

commit 14c4efbbf7408318c5b2824f99db089826af6b11
Author: David Heidelberg <david@ixit.cz>

ASoC: qcom: sdm845: Demystify TDM masks a bit

This commit attempts to clarify the TDM masks by defining individual bits
for left/right speakers and microphones instead of using hardcoded hex values.

> diff --git a/sound/soc/qcom/sdm845.c b/sound/soc/qcom/sdm845.c
> index 6843ab8ba017e..0ce8265ab1c1f 100644
> --- a/sound/soc/qcom/sdm845.c
> +++ b/sound/soc/qcom/sdm845.c
> @@ -23,9 +23,14 @@
>  #define DEFAULT_MCLK_RATE		24576000
>  #define TDM_BCLK_RATE		6144000
>  #define MI2S_BCLK_RATE		1536000
> -#define LEFT_SPK_TDM_TX_MASK    0x30
> -#define RIGHT_SPK_TDM_TX_MASK   0xC0
> -#define SPK_TDM_RX_MASK         0x03
> +#define LEFT_SPK_TDM_RX_MASK	BIT(0)
> +#define RIGHT_SPK_TDM_RX_MASK	BIT(1)
> +#define SPK_TDM_RX_MASK		(LEFT_SPK_TDM_RX_MASK | RIGHT_SPK_TDM_RX_MASK)
> +#define MIC1_TDM_RX_MASK	BIT(2)
> +#define MIC2_TDM_RX_MASK	BIT(3)
> +#define MIC_TDM_RX_MASK		(MIC1_TDM_RX_MASK | MIC2_TDM_RX_MASK)

[Severity: Low]
Is this naming convention accurate for the hardware routing?

Microphones capture audio, meaning they transmit data to the CPU rather than
receive it. Naming this an RX mask seems to misrepresent the physical data
flow and makes the definitions confusing.

> +#define LEFT_SPK_TDM_TX_MASK	(BIT(4) | BIT(5))
> +#define RIGHT_SPK_TDM_TX_MASK	(BIT(6) | BIT(7))

[ ... ]

> @@ -128,8 +133,10 @@ static int sdm845_tdm_snd_hw_params(struct snd_pcm_substream *substream,
>  			goto end;
>  		}
>  	} else {
> -		ret = snd_soc_dai_set_tdm_slot(cpu_dai, 0xf, 0,
> -				8, slot_width);
> +		ret = snd_soc_dai_set_tdm_slot(cpu_dai,
> +					       SPK_TDM_RX_MASK |
> +					       MIC_TDM_RX_MASK, 0,
> +					       NUM_TDM_SLOTS, slot_width);

[Severity: Low]
Does this combination of masks correctly reflect the TDM slot usage for
capture in sdm845_tdm_snd_hw_params()?

It appears the speaker mask, which defines playback slots on the DOUT wire,
is being combined with the new microphone mask merely to mathematically
reconstruct the previous 0xf value.

Using an RX mask to configure the CPU transmit mask, which reads from the
DIN wire, makes the capture path configuration difficult to follow. Could
we define a proper set of TX masks for the capture path instead?

>  		if (ret < 0) {
>  			dev_err(rtd->dev, "%s: failed to set tdm slot, err:%d\n",
>  					__func__, ret);

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20260903-pixel3-audio-v2-0-7c22ed915291@ixit.cz?part=1

[PATCH RFC v2 2/6] ASoC: qcom: sdm845: use DSP_A format for TDM codec DAIs

[David Heidelberg via B4 Relay]

From: David Heidelberg <david@ixit.cz>

Before the DSP_B only worked because the only close-to-mainline consumer
cs35l36 codec was patched to map both DSP_A and DSP_B to the same
hardware register value (asp_fmt = 0), which is inherently DSP_A timing.
Use the right codec (DSP_A) which works as expected.

Signed-off-by: David Heidelberg <david@ixit.cz>
---
 sound/soc/qcom/sdm845.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/sound/soc/qcom/sdm845.c b/sound/soc/qcom/sdm845.c
index 0ce8265ab1c1f..d1cc825fc5cdc 100644
--- a/sound/soc/qcom/sdm845.c
+++ b/sound/soc/qcom/sdm845.c
@@ -382,17 +382,17 @@ static int sdm845_snd_startup(struct snd_pcm_substream *substream)
 	case QUATERNARY_TDM_RX_0:
 	case QUATERNARY_TDM_TX_0:
 		if (++(data->quat_tdm_clk_count) == 1) {
 			snd_soc_dai_set_sysclk(cpu_dai,
 				Q6AFE_LPASS_CLK_ID_QUAD_TDM_IBIT,
 				TDM_BCLK_RATE, SNDRV_PCM_STREAM_PLAYBACK);
 		}
 
-		codec_dai_fmt |= SND_SOC_DAIFMT_IB_NF | SND_SOC_DAIFMT_DSP_B;
+		codec_dai_fmt |= SND_SOC_DAIFMT_IB_NF | SND_SOC_DAIFMT_DSP_A;
 
 		for_each_rtd_codec_dais(rtd, j, codec_dai) {
 
 			if (!strcmp(codec_dai->component->name_prefix,
 				    "Left")) {
 				ret = snd_soc_dai_set_fmt(
 						codec_dai, codec_dai_fmt);
 				if (ret < 0) {

-- 
2.55.0

[PATCH RFC v2 3/6] ASoC: qcom: sdm845: Use per-speaker RX masks for TDM slot assignment

[David Heidelberg via B4 Relay]

From: David Heidelberg <david@ixit.cz>

Both Left and Right codec DAIs were passing the same SPK_TDM_RX_MASK,
both speakers ended up on slot 0, breaking the one speaker in
configuration such as on Pixel 3.

Split SPK_TDM_RX_MASK into per-speaker masks so that the Left codec
gets slot 0 (rx_mask=0x01) and the Right codec gets slot 1
(rx_mask=0x02).

This commit is here, so later CS35L36 receives correct slot for right
and left speakers.

Assisted-by: Claude:claude-4.6-opus
Signed-off-by: David Heidelberg <david@ixit.cz>
---
 sound/soc/qcom/sdm845.c | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/sound/soc/qcom/sdm845.c b/sound/soc/qcom/sdm845.c
index d1cc825fc5cdc..59cfa3b26cd4d 100644
--- a/sound/soc/qcom/sdm845.c
+++ b/sound/soc/qcom/sdm845.c
@@ -152,29 +152,29 @@ static int sdm845_tdm_snd_hw_params(struct snd_pcm_substream *substream,
 		}
 	}
 
 	for_each_rtd_codec_dais(rtd, j, codec_dai) {
 
 		if (!strcmp(codec_dai->component->name_prefix, "Left")) {
 			ret = snd_soc_dai_set_tdm_slot(
 					codec_dai, LEFT_SPK_TDM_TX_MASK,
-					SPK_TDM_RX_MASK, NUM_TDM_SLOTS,
+					LEFT_SPK_TDM_RX_MASK, NUM_TDM_SLOTS,
 					slot_width);
 			if (ret < 0) {
 				dev_err(rtd->dev,
 					"DEV0 TDM slot err:%d\n", ret);
 				return ret;
 			}
 		}
 
 		if (!strcmp(codec_dai->component->name_prefix, "Right")) {
 			ret = snd_soc_dai_set_tdm_slot(
 					codec_dai, RIGHT_SPK_TDM_TX_MASK,
-					SPK_TDM_RX_MASK, NUM_TDM_SLOTS,
+					RIGHT_SPK_TDM_RX_MASK, NUM_TDM_SLOTS,
 					slot_width);
 			if (ret < 0) {
 				dev_err(rtd->dev,
 					"DEV1 TDM slot err:%d\n", ret);
 				return ret;
 			}
 		}
 	}

-- 
2.55.0

Re: [PATCH RFC v2 3/6] ASoC: qcom: sdm845: Use per-speaker RX masks for TDM slot assignment

[sashiko-bot]

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

Pre-existing issues:
- [High] Potential NULL pointer dereference when calling strcmp() on an unchecked codec_dai->component->name_prefix.
- [High] Improper reuse of the speaker playback mask (`SPK_TDM_RX_MASK`) for the CPU capture mask, causing dropped speaker feedback.
--

commit fc9c3ae11318fad7d402ab1b0d3e9652106e6e97
Author: David Heidelberg <david@ixit.cz>

ASoC: qcom: sdm845: Use per-speaker RX masks for TDM slot assignment
    
Both Left and Right codec DAIs were passing the same SPK_TDM_RX_MASK,
both speakers ended up on slot 0, breaking the one speaker in
configuration such as on Pixel 3.

> diff --git a/sound/soc/qcom/sdm845.c b/sound/soc/qcom/sdm845.c
> index d1cc825fc5cdc..59cfa3b26cd4d 100644
> --- a/sound/soc/qcom/sdm845.c
> +++ b/sound/soc/qcom/sdm845.c
> @@ -157,7 +157,7 @@ static int sdm845_tdm_snd_hw_params(struct snd_pcm_substream *substream,
>  		if (!strcmp(codec_dai->component->name_prefix, "Left")) {

[Severity: High]
This isn't a regression introduced by this patch, but can this cause a kernel
panic if a device tree is missing the optional sound-name-prefix property?

If a local user opens the ALSA PCM device, it triggers
sdm845_tdm_snd_hw_params() which unconditionally passes
codec_dai->component->name_prefix to strcmp().

If the property is missing, name_prefix will be NULL. Can this strcmp()
dereference the NULL pointer and cause a panic?

This same unchecked dereference also occurs during PCM startup:

sound/soc/qcom/sdm845.c:sdm845_snd_startup() {
    ...
    for_each_rtd_codec_dais(rtd, j, codec_dai) {
        if (!strcmp(codec_dai->component->name_prefix, "Left")) {
            ...
        if (!strcmp(codec_dai->component->name_prefix, "Right")) {
    ...
}

>  			ret = snd_soc_dai_set_tdm_slot(
>  					codec_dai, LEFT_SPK_TDM_TX_MASK,
> -					SPK_TDM_RX_MASK, NUM_TDM_SLOTS,
> +					LEFT_SPK_TDM_RX_MASK, NUM_TDM_SLOTS,
>  					slot_width);
>  			if (ret < 0) {
>  				dev_err(rtd->dev,
> @@ -169,7 +169,7 @@ static int sdm845_tdm_snd_hw_params(struct snd_pcm_substream *substream,
>  		if (!strcmp(codec_dai->component->name_prefix, "Right")) {

[Severity: High]
This is the same pre-existing regression as above, but does this code
similarly panic if the sound-name-prefix property is missing for the right
codec?

>  			ret = snd_soc_dai_set_tdm_slot(
>  					codec_dai, RIGHT_SPK_TDM_TX_MASK,
> -					SPK_TDM_RX_MASK, NUM_TDM_SLOTS,
> +					RIGHT_SPK_TDM_RX_MASK, NUM_TDM_SLOTS,
>  					slot_width);
>  			if (ret < 0) {
>  				dev_err(rtd->dev,

[Severity: High]
This is a pre-existing regression cemented by an earlier patch in this series,
but does the CPU DAI capture configuration miss the speaker TX slots?

When a capture stream is opened, sdm845_tdm_snd_hw_params() configures the
capture mask:

sound/soc/qcom/sdm845.c:sdm845_tdm_snd_hw_params() {
    ...
    } else {
        ret = snd_soc_dai_set_tdm_slot(cpu_dai,
                                       SPK_TDM_RX_MASK |
                                       MIC_TDM_RX_MASK, 0,
                                       NUM_TDM_SLOTS, slot_width);
    ...
}

Because it uses SPK_TDM_RX_MASK | MIC_TDM_RX_MASK, it captures from slots 0-3.

This completely ignores LEFT_SPK_TDM_TX_MASK and RIGHT_SPK_TDM_TX_MASK. Does
this break the TDM routing for speaker IV sense feedback data transmitted on
slots 4-7?

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20260903-pixel3-audio-v2-0-7c22ed915291@ixit.cz?part=3

[PATCH RFC v2 4/6] ASoC: qcom: sdm845: Set codec dai and component sysclk during startup

[David Heidelberg via B4 Relay]

From: David Heidelberg <david@ixit.cz>

The cs35l36 codec needs the codec dai and component sysclk to be set
during TDM startup. Set these for all codec DAIs on the QUATERNARY_TDM
path, gracefully handling codecs that don't support sysclk by ignoring
-ENOTSUPP returns.

Based on work of Joel Selvaraj.

Signed-off-by: David Heidelberg <david@ixit.cz>
---
 sound/soc/qcom/sdm845.c | 22 ++++++++++++++++++++++
 1 file changed, 22 insertions(+)

diff --git a/sound/soc/qcom/sdm845.c b/sound/soc/qcom/sdm845.c
index 59cfa3b26cd4d..f0fbec9c8df25 100644
--- a/sound/soc/qcom/sdm845.c
+++ b/sound/soc/qcom/sdm845.c
@@ -407,16 +407,38 @@ static int sdm845_snd_startup(struct snd_pcm_substream *substream)
 				ret = snd_soc_dai_set_fmt(
 						codec_dai, codec_dai_fmt);
 				if (ret < 0) {
 					dev_err(rtd->dev,
 						"Right TDM slot err:%d\n", ret);
 					return ret;
 				}
 			}
+
+			/* Set codec sysclk needed by codecs like cs35l36. */
+			ret = snd_soc_dai_set_sysclk(codec_dai, 0,
+						     TDM_BCLK_RATE,
+						     SND_SOC_CLOCK_IN);
+			if (ret < 0 && ret != -ENOTSUPP) {
+				dev_err(codec_dai->dev,
+					"Failed to set codec dai sysclk: %d\n",
+					ret);
+				return ret;
+			}
+
+			ret = snd_soc_component_set_sysclk(codec_dai->component,
+							   0, 0,
+							   TDM_BCLK_RATE,
+							   SND_SOC_CLOCK_IN);
+			if (ret < 0 && ret != -ENOTSUPP) {
+				dev_err(codec_dai->dev,
+					"Failed to set codec component sysclk: %d\n",
+					ret);
+				return ret;
+			}
 		}
 		break;
 	case SLIMBUS_0_RX...SLIMBUS_6_TX:
 		break;
 
 	default:
 		pr_err("%s: invalid dai id 0x%x\n", __func__, cpu_dai->id);
 		break;

-- 
2.55.0

Re: [PATCH RFC v2 4/6] ASoC: qcom: sdm845: Set codec dai and component sysclk during startup

[sashiko-bot]

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

New issues:
- [Medium] Resource leak (clock and refcount) on error path in sdm845_snd_startup.

Pre-existing issues:
- [High] Kernel panic due to NULL pointer dereference in `strcmp()`.
--

commit 620251c05b4c5ac4fb80bf96411a9fcc1f4c402d
Author: David Heidelberg <david@ixit.cz>

ASoC: qcom: sdm845: Set codec dai and component sysclk during startup

The cs35l36 codec needs the codec dai and component sysclk to be set
during TDM startup. Set these for all codec DAIs on the QUATERNARY_TDM
path, gracefully handling codecs that don't support sysclk by ignoring
-ENOTSUPP returns.

> diff --git a/sound/soc/qcom/sdm845.c b/sound/soc/qcom/sdm845.c
> index 59cfa3b26cd4d..f0fbec9c8df25 100644
> --- a/sound/soc/qcom/sdm845.c
> +++ b/sound/soc/qcom/sdm845.c
> @@ -412,6 +412,28 @@ static int sdm845_snd_startup(struct snd_pcm_substream *substream)
>  					return ret;
>  				}
>  			}

[Severity: High]
This is a pre-existing issue, but can codec_dai->component->name_prefix be
NULL when evaluated in the strcmp() calls just above this block?

If the device tree lacks a sound-name-prefix property for a codec on the
QUATERNARY_TDM link, passing a NULL name_prefix to strcmp() will cause a
kernel panic during startup.

> +
> +			/* Set codec sysclk needed by codecs like cs35l36. */
> +			ret = snd_soc_dai_set_sysclk(codec_dai, 0,
> +						     TDM_BCLK_RATE,
> +						     SND_SOC_CLOCK_IN);
> +			if (ret < 0 && ret != -ENOTSUPP) {
> +				dev_err(codec_dai->dev,
> +					"Failed to set codec dai sysclk: %d\n",
> +					ret);
> +				return ret;

[Severity: Medium]
Does this early return leak the CPU DAI clock and the clock reference count?

Earlier in sdm845_snd_startup(), the quat_tdm_clk_count is incremented and
the clock is enabled:

    if (++(data->quat_tdm_clk_count) == 1) {
        snd_soc_dai_set_sysclk(cpu_dai,
            Q6AFE_LPASS_CLK_ID_QUAD_TDM_IBIT,
            TDM_BCLK_RATE, SNDRV_PCM_STREAM_PLAYBACK);
    }

If setting the codec DAI sysclk fails and returns here, it bypasses
decrementing this reference count and leaves the CPU DAI sysclk active. The
ALSA/ASoC core does not invoke the machine driver's shutdown callback when
startup fails, meaning this state is permanently leaked.

Is there a need to unwind these resources before returning?

> +			}
> +
> +			ret = snd_soc_component_set_sysclk(codec_dai->component,
> +							   0, 0,
> +							   TDM_BCLK_RATE,
> +							   SND_SOC_CLOCK_IN);
> +			if (ret < 0 && ret != -ENOTSUPP) {
> +				dev_err(codec_dai->dev,
> +					"Failed to set codec component sysclk: %d\n",
> +					ret);
> +				return ret;

[Severity: Medium]
Does this early return also leak the CPU DAI clock and the reference count?

Similar to the previous early return, failing to set the codec component
sysclk will leave the quat_tdm_clk_count incremented and the CPU DAI sysclk
active.

> +			}
>  		}
>  		break;

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20260903-pixel3-audio-v2-0-7c22ed915291@ixit.cz?part=4

[PATCH RFC v2 5/6] ASoC: cs35l36: Implement set_tdm_slot to program RX and TX slots

[David Heidelberg via B4 Relay]

From: David Heidelberg <david@ixit.cz>

Program the ASP RX and TX slot registers from the TDM masks passed by
the machine driver. Each set bit in a mask names a slot; codec channels
are assigned to those slots in order, ASPRX1 taking the first RX slot
and ASPTX1..TX8 the first eight TX slots, with a warning if the mask
names more slots than the device has channels.

Passing slots == 0 or an empty mask restores the hardware defaults,
ASPRX1 in slot 0 and ASPTX1..TX8 in slots 0..7.

This lets a machine driver with several amplifiers on one bus, such as
sdm845 with two CS35L36, put each amplifier on its own RX slot and keep
their TX slots from colliding.

Assisted-by: Claude:claude-4.6-opus
Signed-off-by: David Heidelberg <david@ixit.cz>
---
 sound/soc/codecs/cs35l36.c | 44 ++++++++++++++++++++++++++++++++++++++++++++
 1 file changed, 44 insertions(+)

diff --git a/sound/soc/codecs/cs35l36.c b/sound/soc/codecs/cs35l36.c
index 89645327945f1..4d35ca6291548 100644
--- a/sound/soc/codecs/cs35l36.c
+++ b/sound/soc/codecs/cs35l36.c
@@ -942,20 +942,64 @@ static const struct cs35l36_pll_config *cs35l36_get_clk_config(
 	for (i = 0; i < ARRAY_SIZE(cs35l36_pll_sysclk); i++) {
 		if (cs35l36_pll_sysclk[i].freq == freq)
 			return &cs35l36_pll_sysclk[i];
 	}
 
 	return NULL;
 }
 
+static void cs35l36_mask_to_slots(struct cs35l36_private *cs35l36,
+				  unsigned long mask, unsigned int base_reg,
+				  unsigned int nchan)
+{
+	unsigned int chan = 0, shift;
+	int slot;
+
+	/* Two 6-bit slot fields per register, at bits 0 and 16 */
+	for_each_set_bit(slot, &mask, BITS_PER_TYPE(mask)) {
+		if (chan == nchan) {
+			dev_warn(cs35l36->dev,
+				 "Too many slots in TDM mask: %lx\n", mask);
+			return;
+		}
+
+		shift = (chan % 2) * CS35L36_ASP_TX2_SLOT_SHIFT;
+		regmap_update_bits(cs35l36->regmap, base_reg + (chan / 2) * 4,
+				   CS35L36_ASP_RX1_SLOT_MASK << shift,
+				   slot << shift);
+		chan++;
+	}
+}
+
+static int cs35l36_set_tdm_slot(struct snd_soc_dai *dai,
+				unsigned int tx_mask, unsigned int rx_mask,
+				int slots, int slot_width)
+{
+	struct cs35l36_private *cs35l36 =
+			snd_soc_component_get_drvdata(dai->component);
+
+	/* Note: rx/tx is from point of view of the CPU end */
+	if (!slots || !rx_mask)
+		rx_mask = BIT(0);		/* ASPRX1 in slot 0 */
+
+	if (!slots || !tx_mask)
+		tx_mask = GENMASK(7, 0);	/* ASPTX1..8 in slots 0..7 */
+
+	cs35l36_mask_to_slots(cs35l36, rx_mask, CS35L36_ASP_RX1_SLOT, 1);
+	cs35l36_mask_to_slots(cs35l36, tx_mask, CS35L36_ASP_TX1_TX2_SLOT, 8);
+
+	return 0;
+}
+
 static const struct snd_soc_dai_ops cs35l36_ops = {
 	.set_fmt = cs35l36_set_dai_fmt,
 	.hw_params = cs35l36_pcm_hw_params,
 	.set_sysclk = cs35l36_dai_set_sysclk,
+	.set_tdm_slot = cs35l36_set_tdm_slot,
 };
 
 #define CS35L36_RATES (		    \
 	SNDRV_PCM_RATE_8000_48000 | \
 	SNDRV_PCM_RATE_12000 |	    \
 	SNDRV_PCM_RATE_24000 |	    \
 	SNDRV_PCM_RATE_88200 |	    \
 	SNDRV_PCM_RATE_96000 |	    \

-- 
2.55.0

Re: [PATCH RFC v2 5/6] ASoC: cs35l36: Implement set_tdm_slot to program RX and TX slots

[sashiko-bot]

Thank you for your contribution! Sashiko AI review found 2 potential issue(s) to consider:
- [High] Unmapped TDM channels are not disabled and retain their default slot assignments, leading to TDM slot collisions.
- [High] The new set_tdm_slot configures the RX TDM slot via a register that aliases an existing ALSA user control, introducing state corruption.
--

commit af0d8ef24a330447b6254f7aa04fa7142a333d1b
Author: David Heidelberg <david@ixit.cz>

ASoC: cs35l36: Implement set_tdm_slot to program RX and TX slots

This commit implements the set_tdm_slot DAI operation for the CS35L36 codec,
allowing machine drivers to program ASP RX and TX slots. It maps bits in the
provided RX and TX masks to consecutive codec channels, and sets hardware
defaults if no slots are passed.

> diff --git a/sound/soc/codecs/cs35l36.c b/sound/soc/codecs/cs35l36.c
> index 89645327945f1..4d35ca6291548 100644
> --- a/sound/soc/codecs/cs35l36.c
> +++ b/sound/soc/codecs/cs35l36.c

[ ... ]

> @@ -947,10 +947,54 @@ static const struct cs35l36_pll_config *cs35l36_get_clk_config(
>  	return NULL;
>  }
>  
> +static void cs35l36_mask_to_slots(struct cs35l36_private *cs35l36,
> +				  unsigned long mask, unsigned int base_reg,
> +				  unsigned int nchan)
> +{
> +	unsigned int chan = 0, shift;
> +	int slot;
> +
> +	/* Two 6-bit slot fields per register, at bits 0 and 16 */
> +	for_each_set_bit(slot, &mask, BITS_PER_TYPE(mask)) {
> +		if (chan == nchan) {
> +			dev_warn(cs35l36->dev,
> +				 "Too many slots in TDM mask: %lx\n", mask);
> +			return;
> +		}
> +
> +		shift = (chan % 2) * CS35L36_ASP_TX2_SLOT_SHIFT;
> +		regmap_update_bits(cs35l36->regmap, base_reg + (chan / 2) * 4,
> +				   CS35L36_ASP_RX1_SLOT_MASK << shift,
> +				   slot << shift);
> +		chan++;
> +	}

[Severity: High]
What happens to the remaining hardware channels if the provided mask has fewer
set bits than the total number of hardware channels?

The loop finishes processing once all set bits are consumed, which seems to
ignore the remaining hardware channels and leaves them in a potentially
dangerous active state on their default slots.

For example, if a machine driver passes a tx_mask with fewer set bits than the
total number of hardware channels (8) to map a subset of slots (e.g.,
tx_mask = 0x30 to use slots 4 and 5), this loop assigns those slots to the
first channels (ASPTX1 and ASPTX2). It terminates early without modifying the
remaining channels (ASPTX3..8), leaving them at their hardware default slots
(slots 2..7).

If DAPM later enables these unmapped channels (e.g., via the 'AMP Capture'
path), they will transmit on their default slots. In this example, ASPTX5
defaults to slot 4, which collides directly with the explicitly mapped ASPTX1
on slot 4.

> +}
> +
> +static int cs35l36_set_tdm_slot(struct snd_soc_dai *dai,
> +				unsigned int tx_mask, unsigned int rx_mask,
> +				int slots, int slot_width)
> +{
> +	struct cs35l36_private *cs35l36 =
> +			snd_soc_component_get_drvdata(dai->component);
> +
> +	/* Note: rx/tx is from point of view of the CPU end */
> +	if (!slots || !rx_mask)
> +		rx_mask = BIT(0);		/* ASPRX1 in slot 0 */
> +
> +	if (!slots || !tx_mask)
> +		tx_mask = GENMASK(7, 0);	/* ASPTX1..8 in slots 0..7 */
> +
> +	cs35l36_mask_to_slots(cs35l36, rx_mask, CS35L36_ASP_RX1_SLOT, 1);

[Severity: High]
Does configuring the RX TDM slot via CS35L36_ASP_RX1_SLOT alias an existing
ALSA user control?

The driver exposes bit 0 of this exact register to userspace as a DAPM audio
routing control ('Input Mux') here:

sound/soc/codecs/cs35l36.c:
    static SOC_ENUM_SINGLE_DECL(chansel_enum, CS35L36_ASP_RX1_SLOT, 0,
                                cs35l36_chan_text);

Because both the DAI configuration callback and an ALSA user control manage the
LSB of this register, will saving and restoring ALSA state (e.g., via
alsactl restore on boot) silently overwrite bit 0 of the TDM slot configured
by the machine driver?

For example, if the machine driver configures TDM slot 4 (binary 100), and
userspace restores 'RX2' (value 1), the slot will be corrupted to 5
(binary 101), breaking the machine driver's hardware configuration.

> +	cs35l36_mask_to_slots(cs35l36, tx_mask, CS35L36_ASP_TX1_TX2_SLOT, 8);
> +
> +	return 0;
> +}

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20260903-pixel3-audio-v2-0-7c22ed915291@ixit.cz?part=5

Re: [PATCH RFC v2 5/6] ASoC: cs35l36: Implement set_tdm_slot to program RX and TX slots

[Konrad Dybcio]

On 9/3/26 8:26 PM, David Heidelberg via B4 Relay wrote:
> From: David Heidelberg <david@ixit.cz>
> 
> Program the ASP RX and TX slot registers from the TDM masks passed by
> the machine driver. Each set bit in a mask names a slot; codec channels
> are assigned to those slots in order, ASPRX1 taking the first RX slot
> and ASPTX1..TX8 the first eight TX slots, with a warning if the mask
> names more slots than the device has channels.
> 
> Passing slots == 0 or an empty mask restores the hardware defaults,
> ASPRX1 in slot 0 and ASPTX1..TX8 in slots 0..7.
> 
> This lets a machine driver with several amplifiers on one bus, such as
> sdm845 with two CS35L36, put each amplifier on its own RX slot and keep
> their TX slots from colliding.
> 
> Assisted-by: Claude:claude-4.6-opus
> Signed-off-by: David Heidelberg <david@ixit.cz>
> ---

[...]

> +static int cs35l36_set_tdm_slot(struct snd_soc_dai *dai,
> +				unsigned int tx_mask, unsigned int rx_mask,
> +				int slots, int slot_width)
> +{
> +	struct cs35l36_private *cs35l36 =
> +			snd_soc_component_get_drvdata(dai->component);
> +
> +	/* Note: rx/tx is from point of view of the CPU end */
> +	if (!slots || !rx_mask)
> +		rx_mask = BIT(0);		/* ASPRX1 in slot 0 */
> +
> +	if (!slots || !tx_mask)
> +		tx_mask = GENMASK(7, 0);	/* ASPTX1..8 in slots 0..7 */
> +
> +	cs35l36_mask_to_slots(cs35l36, rx_mask, CS35L36_ASP_RX1_SLOT, 1);

GPT says:

Findings
1. High: patch 5 makes the TDM RX slot compete with the codec’s Input Mux control.  
cs35l36_set_tdm_slot() programs CS35L36_ASP_RX1_SLOT with a six-bit TDM slot value in patch 5. That register’s bit 0 is already exposed as the Input Mux DAPM control:
- sound/soc/codecs/cs35l36.c:588 defines the control from CS35L36_ASP_RX1_SLOT, shift 0.
- sound/soc/codecs/cs35l36.c:583-586 defines its values as RX1 and RX2.

Konrad

Re: [PATCH RFC v2 5/6] ASoC: cs35l36: Implement set_tdm_slot to program RX and TX slots

[Charles Keepax]

On Fri, Sep 04, 2026 at 10:22:18AM +0200, Konrad Dybcio wrote:
> On 9/3/26 8:26 PM, David Heidelberg via B4 Relay wrote:
> > From: David Heidelberg <david@ixit.cz>
> > 
> > Program the ASP RX and TX slot registers from the TDM masks passed by
> > the machine driver. Each set bit in a mask names a slot; codec channels
> > are assigned to those slots in order, ASPRX1 taking the first RX slot
> > and ASPTX1..TX8 the first eight TX slots, with a warning if the mask
> > names more slots than the device has channels.
> > 
> > Passing slots == 0 or an empty mask restores the hardware defaults,
> > ASPRX1 in slot 0 and ASPTX1..TX8 in slots 0..7.
> > 
> > This lets a machine driver with several amplifiers on one bus, such as
> > sdm845 with two CS35L36, put each amplifier on its own RX slot and keep
> > their TX slots from colliding.
> > 
> > Assisted-by: Claude:claude-4.6-opus
> > Signed-off-by: David Heidelberg <david@ixit.cz>
> > ---
> 
> [...]
> 
> > +static int cs35l36_set_tdm_slot(struct snd_soc_dai *dai,
> > +				unsigned int tx_mask, unsigned int rx_mask,
> > +				int slots, int slot_width)
> > +{
> > +	struct cs35l36_private *cs35l36 =
> > +			snd_soc_component_get_drvdata(dai->component);
> > +
> > +	/* Note: rx/tx is from point of view of the CPU end */
> > +	if (!slots || !rx_mask)
> > +		rx_mask = BIT(0);		/* ASPRX1 in slot 0 */
> > +
> > +	if (!slots || !tx_mask)
> > +		tx_mask = GENMASK(7, 0);	/* ASPTX1..8 in slots 0..7 */
> > +
> > +	cs35l36_mask_to_slots(cs35l36, rx_mask, CS35L36_ASP_RX1_SLOT, 1);
> 
> GPT says:
> 
> Findings
> 1. High: patch 5 makes the TDM RX slot compete with the codec’s Input Mux control.  
> cs35l36_set_tdm_slot() programs CS35L36_ASP_RX1_SLOT with a
> six-bit TDM slot value in patch 5. That register’s bit 0 is
> already exposed as the Input Mux DAPM control:
> - sound/soc/codecs/cs35l36.c:588 defines the control from CS35L36_ASP_RX1_SLOT, shift 0.
> - sound/soc/codecs/cs35l36.c:583-586 defines its values as RX1 and RX2.

This is a fair point, although I am inclined to say that using an
ALSA control for this is definitely not a good thing to do. I would
be tempted to say just remove the alsa control, now we have a
proper TDM callback. But probably also fine to ignore the control
for now. It only becomes a problem if user-space starts tweaking
the control and to some extent that is on the person changing the
control.

Thanks,
Charles

Re: [PATCH RFC v2 5/6] ASoC: cs35l36: Implement set_tdm_slot to program RX and TX slots

[Charles Keepax]

On Thu, Sep 03, 2026 at 08:26:41PM +0200, David Heidelberg via B4 Relay wrote:
> From: David Heidelberg <david@ixit.cz>
> 
> Program the ASP RX and TX slot registers from the TDM masks passed by
> the machine driver. Each set bit in a mask names a slot; codec channels
> are assigned to those slots in order, ASPRX1 taking the first RX slot
> and ASPTX1..TX8 the first eight TX slots, with a warning if the mask
> names more slots than the device has channels.
> 
> Passing slots == 0 or an empty mask restores the hardware defaults,
> ASPRX1 in slot 0 and ASPTX1..TX8 in slots 0..7.
> 
> This lets a machine driver with several amplifiers on one bus, such as
> sdm845 with two CS35L36, put each amplifier on its own RX slot and keep
> their TX slots from colliding.
> 
> Assisted-by: Claude:claude-4.6-opus
> Signed-off-by: David Heidelberg <david@ixit.cz>
> ---

This patch looks good to me.

Reviewed-by: Charles Keepax <ckeepax@opensource.cirrus.com>

Thanks,
Charles

[PATCH RFC v2 6/6] arm64: dts: qcom: sdm845-google: Add basic audio support

[David Heidelberg via B4 Relay]

From: David Heidelberg <david@ixit.cz>

Introduce support for sound card and wire two CS35L36 audio codecs for
top and bottom speakers.

Inspired by commit from Joel Selvaraj.

Signed-off-by: David Heidelberg <david@ixit.cz>
---
 arch/arm64/boot/dts/qcom/sdm845-google-common.dtsi | 132 ++++++++++++++++++++-
 1 file changed, 130 insertions(+), 2 deletions(-)

diff --git a/arch/arm64/boot/dts/qcom/sdm845-google-common.dtsi b/arch/arm64/boot/dts/qcom/sdm845-google-common.dtsi
index 9e952f9862f24..cf87b582e3997 100644
--- a/arch/arm64/boot/dts/qcom/sdm845-google-common.dtsi
+++ b/arch/arm64/boot/dts/qcom/sdm845-google-common.dtsi
@@ -2,16 +2,18 @@
 
 /dts-v1/;
 
 #include <dt-bindings/arm/qcom,ids.h>
 #include <dt-bindings/dma/qcom-gpi.h>
 #include <dt-bindings/input/linux-event-codes.h>
 #include <dt-bindings/interrupt-controller/irq.h>
 #include <dt-bindings/regulator/qcom,rpmh-regulator.h>
+#include <dt-bindings/sound/qcom,q6afe.h>
+#include <dt-bindings/sound/qcom,q6asm.h>
 
 #include "sdm845.dtsi"
 #include "pm8998.dtsi"
 #include "pmi8998.dtsi"
 
 /delete-node/ &mpss_region;
 /delete-node/ &venus_mem;
 /delete-node/ &cdsp_mem;
@@ -404,19 +406,61 @@ nfc@28 {
 		firmware-gpios = <&tlmm 79 GPIO_ACTIVE_HIGH>;
 
 		pinctrl-0 = <&nfc_int_default &nfc_enable_default>;
 		pinctrl-names = "default";
 	};
 };
 
 &i2c12 {
-	/* Bottom spkr (right) CS35L36 @ 40 */
+	status = "okay";
+
+	cs35l36_bottom: audio-codec@40 {
+		compatible = "cirrus,cs35l36";
+		reg = <0x40>;
+		reset-gpios = <&tlmm 112 GPIO_ACTIVE_HIGH>;
+		#sound-dai-cells = <0>;
+		sound-name-prefix = "Right"; /* Bottom */
+		VA-supply = <&vreg_s4a_1p8>;
+		interrupts-extended = <&tlmm 115 IRQ_TYPE_LEVEL_LOW>;
+
+		pinctrl-0 = <&cs35l36_bottom_default_state>;
+		pinctrl-names = "default";
+
+		cirrus,boost-ind-nanohenry = <1000>;
+		cirrus,boost-ctl-select = <1>;
+		cirrus,boost-ctl-millivolt = <10000>;
+		cirrus,boost-peak-milliamp = <3700>;
+		cirrus,temp-warn-threshold = <1>;
+		cirrus,multi-amp-mode;
+		cirrus,irq-drive-select = <0>;
+		cirrus,irq-gpio-select = <0>;
+	};
 
-	/* Top spkr (left) CS35L36 @ 41 */
+	cs35l36_top: audio-codec@41 {
+		compatible = "cirrus,cs35l36";
+		reg = <0x41>;
+		reset-gpios = <&tlmm 75 GPIO_ACTIVE_HIGH>;
+		#sound-dai-cells = <0>;
+		sound-name-prefix = "Left"; /* Top */
+		VA-supply = <&vreg_s4a_1p8>;
+		interrupts-extended = <&tlmm 40 IRQ_TYPE_LEVEL_LOW>;
+
+		pinctrl-0 = <&cs35l36_top_default_state>;
+		pinctrl-names = "default";
+
+		cirrus,boost-ind-nanohenry = <1000>;
+		cirrus,boost-ctl-select = <0x01>;
+		cirrus,boost-ctl-millivolt = <10000>;
+		cirrus,boost-peak-milliamp = <3700>;
+		cirrus,temp-warn-threshold = <1>;
+		cirrus,multi-amp-mode;
+		cirrus,irq-drive-select = <0>;
+		cirrus,irq-gpio-select = <0>;
+	};
 };
 
 &ipa {
 	firmware-name = "qcom/sdm845/Google/blueline/ipa_fws.mbn";
 	memory-region = <&ipa_fw_mem>;
 
 	status = "okay";
 };
@@ -449,16 +493,35 @@ &pm8998_resin {
 };
 
 &pmi8998_charger {
 	monitored-battery = <&battery>;
 
 	status = "okay";
 };
 
+&q6afedai {
+	dai@72 {
+		reg = <QUATERNARY_TDM_RX_0>;
+
+		qcom,tdm-sync-mode = <0>;
+		qcom,tdm-sync-src = <1>;
+		qcom,tdm-data-out = <0>;
+		qcom,tdm-invert-sync = <0>;
+		qcom,tdm-data-delay = <1>;
+		qcom,tdm-data-align = <0>;
+	};
+};
+
+&q6asmdai {
+	dai@0 {
+		reg = <MSM_FRONTEND_DAI_MULTIMEDIA1>;
+	};
+};
+
 &qupv3_id_0 {
 	status = "okay";
 };
 
 &qupv3_id_1 {
 	status = "okay";
 };
 
@@ -467,16 +530,49 @@ &qup_uart9_rx {
 	bias-pull-up;
 };
 
 &qup_uart9_tx {
 	drive-strength = <2>;
 	bias-disable;
 };
 
+&sound {
+	compatible = "qcom,sdm845-sndcard";
+	model = "blueline";
+	pinctrl-0 = <&quat_mi2s_active>, <&quat_mi2s_sd0_active>,
+		    <&quat_mi2s_sd1_active>;
+	pinctrl-names = "default";
+
+	/* mm[0-9]-dai-link nodes must be positioned first regardless of sorting */
+	mm1-dai-link {
+		link-name = "MultiMedia1";
+
+		cpu {
+			sound-dai = <&q6asmdai MSM_FRONTEND_DAI_MULTIMEDIA1>;
+		};
+	};
+
+	cs35l36-dai-link {
+		link-name = "Speaker Playback";
+
+		codec {
+			sound-dai = <&cs35l36_bottom>, <&cs35l36_top>;
+		};
+
+		cpu {
+			sound-dai = <&q6afedai QUATERNARY_TDM_RX_0>;
+		};
+
+		platform {
+			sound-dai = <&q6routing>;
+		};
+	};
+};
+
 &tlmm {
 	gpio-reserved-ranges = < 0 4>, /* SPI (Intel MNH Pixel Visual Core) */
 			       <81 4>; /* SPI (most likely Fingerprint Cards FPC1075) */
 
 	nfc_int_default: nfc-int-default-state {
 		pins = "gpio63";
 		function = "gpio";
 		drive-strength = <2>;
@@ -505,16 +601,48 @@ touchscreen_pins: ts-pins-gpio-state {
 	};
 
 	touchscreen_i2c_pins: qup-i2c2-gpio-state {
 		pins = "gpio27", "gpio28";
 		function = "gpio";
 		drive-strength = <2>;
 		bias-disable;
 	};
+
+	cs35l36_bottom_default_state: cs35l36-bottom-default-state {
+		reset-pins {
+			pins = "gpio112";
+			function = "gpio";
+			drive-strength = <2>;
+			bias-disable;
+		};
+
+		irq-pins {
+			pins = "gpio115";
+			function = "gpio";
+			drive-strength = <2>;
+			bias-pull-up;
+		};
+	};
+
+	cs35l36_top_default_state: cs35l36-top-default-state {
+		irq-pins {
+			pins = "gpio40";
+			function = "gpio";
+			drive-strength = <2>;
+			bias-pull-up;
+		};
+
+		reset-pins {
+			pins = "gpio75";
+			function = "gpio";
+			drive-strength = <2>;
+			bias-disable;
+		};
+	};
 };
 
 &uart6 {
 	pinctrl-0 = <&qup_uart6_4pin>;
 
 	status = "okay";
 
 	bluetooth {

-- 
2.55.0