Re: [PATCH v5 1/7] ASoC: qcom: sdm845: Demystify TDM masks a bit

[sashiko-bot@kernel.org]

Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:
- [Medium] ASoC: qcom: semantic coupling of speaker and mic TDM masks
--

commit 69e096818ddd09fdfe14d66beb32de5193e59c35
Author: David Heidelberg <david@ixit.cz>
Subject: ASoC: qcom: sdm845: Demystify TDM masks a bit

This commit updates the sdm845 driver to replace magic numbers with explicit
bitmask definitions for the TDM slots. It aims to clarify which bits are used
for each RX and TX stream.

> 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: Medium]
Since microphones act as a transmission source (TX), is it conceptually
incorrect to define their slots as an RX mask?

Naming microphone transmission slots as an RX mask seems to obscure the
true hardware architecture.

> +#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

[ ... ]

> @@ -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: Medium]
Does applying the speaker playback mask to the microphone capture
configuration create a false semantic coupling?

Modifying the speaker playback slot assignments via the speaker mask will
unintentionally break the microphone capture slot assignments, as the
capture mask now directly aliases the playback mask.

>  		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/20260925-pixel3-audio-v5-0-7ffb32c980bf@ixit.cz?part=1