Re: [PATCH v3 1/8] 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:
- [Low] sdm845: MIC_TDM_RX_MASK incorrectly implies microphone receives data
--

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

ASoC: qcom: sdm845: Demystify TDM masks a bit

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

> 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]
Does this naming convention accurately reflect the data direction?

The file appears to establish a peripheral-centric naming convention, where
SPK_TDM_RX_MASK is used for playback (speaker receives) and
LEFT_SPK_TDM_TX_MASK is used for IV sense (speaker transmits).

By this convention, since a microphone is an input device that transmits data
to the SoC, using an RX mask implies the microphone is receiving data. Would
it be more accurate to name these microphone slots MIC_TDM_TX_MASK to align
with the physical direction of the data flow?

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

[Severity: Low]
Is this using the correct mask name for the CPU capture path?

This forms the CPU capture mask using the inappropriately named 
MIC_TDM_RX_MASK, which contradicts the commit's stated intent to clarify 
RX/TX directions. Could this be updated to use a TX mask instead?

> +					       NUM_TDM_SLOTS, slot_width);
>  		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/20260924-pixel3-audio-v3-0-5d06460fc8a9@ixit.cz?part=1