Re: [PATCH RFC 5/6] ASoC: cs35l36: Implement set_tdm_slot to program RX slot register

[sashiko-bot@kernel.org]

Thank you for your contribution! Sashiko AI review found 3 potential issue(s) to consider:
- [High] The newly added set_tdm_slot function conflicts with the existing DAPM 'Channel Mux' kcontrol, causing TDM slot state corruption.
- [High] The set_tdm_slot callback silently ignores the tx_mask parameter, failing to configure TX TDM slots and causing potential DOUT bus contention.
- [Medium] The return value of regmap_update_bits is ignored, masking potential hardware communication failures.
--

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

ASoC: cs35l36: Implement set_tdm_slot to program RX slot register

Program ASP_RX1_SLOT with the first set bit of rx_mask so the amplifier
listens on the correct TDM slot when configured by machine drivers.

> diff --git a/sound/soc/codecs/cs35l36.c b/sound/soc/codecs/cs35l36.c
> index 89645327945f1..944f4cb5763e4 100644
> --- a/sound/soc/codecs/cs35l36.c
> +++ b/sound/soc/codecs/cs35l36.c
> @@ -947,10 +947,26 @@ static const struct cs35l36_pll_config *cs35l36_get_clk_config(
>  	return NULL;
>  }
>  
> +static int cs35l36_set_tdm_slot(struct snd_soc_dai *dai,
> +				    unsigned int tx_mask, unsigned int rx_mask,
> +				    int slots, int slot_width)
> +{

[Severity: High]
Does this safely handle scenarios where multiple codecs share a TDM bus?

The function accepts a tx_mask parameter but doesn't use it, nor does it
return an error if a non-default tx_mask is requested.

If a machine driver sets up multiple CS35L36 codecs on a shared TDM bus and
calls set_tdm_slot with distinct tx_mask values, the codecs will remain on
their overlapping default TX slots. This could cause multiple codecs to drive
the I2S DOUT line simultaneously when the capture stream starts.

> +	struct cs35l36_private *cs35l36 =
> +			snd_soc_component_get_drvdata(dai->component);
> +
> +	if (rx_mask)
> +		regmap_update_bits(cs35l36->regmap, CS35L36_ASP_RX1_SLOT,
> +				   CS35L36_ASP_RX1_SLOT_MASK,
> +				   ffs(rx_mask) - 1);

[Severity: High]
Will this conflict with the existing DAPM channel mux kcontrol?

The preexisting chansel_enum DAPM mux targets the exact same register field:

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

Since chansel_enum uses a 1-bit mask and set_tdm_slot uses a 6-bit mask
(CS35L36_ASP_RX1_SLOT_MASK), these interfaces seem incompatible. 

If a machine driver sets the slot to 4, later toggling the DAPM kcontrol
from userspace will flip only bit 0, corrupting the slot configuration.

[Severity: Medium]
Should the return value of regmap_update_bits be checked here?

If the hardware write fails with an error like -EIO, the function still
returns 0. This falsely reports success to the ALSA core, allowing the stream
to proceed with the amplifier listening to the wrong TDM slot instead of
gracefully aborting.

> +
> +	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,
>  };

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20260705-pixel3-audio-v1-0-3b66f33859f1@ixit.cz?part=5