Re: [PATCH v4 2/7] ASoC: qcom: q6apm-lpass-dais: add TDM DAI operations

[Prasad Kumpatla]

On 7/14/2026 2:02 PM, Srinivas Kandagatla wrote:
> On 7/12/26 2:41 PM, Prasad Kumpatla wrote:
> > Add TDM DAI operations to q6apm-lpass-dais so AudioReach TDM
> > backends can be configured through the normal ASoC hw_params and DAI
> > setup flow.
> >
> > The TDM set_tdm_slot() callback validates the supported slot width and
> > slot count, stores the active slot mask in the AudioReach module
> > configuration, and leaves existing DMA, I2S and HDMI paths unchanged.
> >
> > Reuse the existing LPASS child-clock handling for TDM nodes as well as
> > MI2S nodes, since TDM backends also request optional backend clocks
> > through the machine driver set_sysclk() path.
> >
> > Signed-off-by: Prasad Kumpatla <prasad.kumpatla@xxxxxxxxxxxxxxxx>
> > ---
> >   sound/soc/qcom/qdsp6/q6apm-lpass-dais.c | 57 +++++++++++++++++++++++++
> >   1 file changed, 57 insertions(+)
> >
> > diff --git a/sound/soc/qcom/qdsp6/q6apm-lpass-dais.c b/sound/soc/qcom/qdsp6/q6apm-lpass-dais.c
> > index 5743586ff..672189625 100644
> > --- a/sound/soc/qcom/qdsp6/q6apm-lpass-dais.c
> > +++ b/sound/soc/qcom/qdsp6/q6apm-lpass-dais.c
> > @@ -372,6 +372,50 @@ static int q6i2s_set_fmt(struct snd_soc_dai *dai, unsigned int fmt)
> >   	return 0;
> >   }
> >   
> > +static int q6tdm_set_tdm_slot(struct snd_soc_dai *dai, unsigned int tx_mask,
> > +			      unsigned int rx_mask, int slots, int slot_width)
> > +{
> > +	struct q6apm_lpass_dai_data *dai_data = dev_get_drvdata(dai->dev);
> > +	struct audioreach_module_config *cfg = &dai_data->module_config[dai->id];
> > +	unsigned int cap_mask;
> > +
> > +	if (slot_width != 16 && slot_width != 32) {
> > +		dev_err(dai->dev, "%s: invalid slot_width %d\n", __func__, slot_width);
> > +		return -EINVAL;
> > +	}
> > +
> > +	switch (slots) {
> > +	case 2:
> > +		cap_mask = 0x03;
> > +		break;
> > +	case 4:
> > +		cap_mask = 0x0f;
> > +		break;
> > +	case 8:
> > +		cap_mask = 0xff;
> > +		break;
> > +	case 16:
> > +		cap_mask = 0xffff;
> > +		break;
> switch (slots) {
> 	case 2:
> 	case 4:
> 	case 8:
> 	case 16:
> 		cap_mask = GENMASK(slots - 1, 0);
> 		break;
> ?

Hi Srini,

Thanks for review.

Good suggestion. The mask is derived directly from the number of
slots, so using GENMASK(slots - 1, 0) is cleaner and avoids
maintaining hardcoded values. I'll update this in the next revision.

> > +	default:
> > +		dev_err(dai->dev, "%s: invalid slots %d\n", __func__, slots);
> > +		return -EINVAL;
> > +	}
> > +
> > +	switch (dai->id) {
> > +	case PRIMARY_TDM_RX_0 ... QUINARY_TDM_TX_7:
> > +		cfg->nslots_per_frame = slots;
> > +		cfg->slot_width = slot_width;
> > +		cfg->slot_mask = ((dai->id & 0x1) ? tx_mask : rx_mask) & cap_mask;
> Should we validate the tx/rx mask here if its with in cap_mask range?
That's a good point. The current implementation silently truncates
any bits outside the valid slot range through & cap_mask. I'll add
validation to reject masks containing bits beyond the configured
number of slots and return -EINVAL instead of silently modifying
the requested configuration.

Thanks,
Prasad
> > +		break;
> > +	default:
> > +		dev_err(dai->dev, "%s: invalid dai id 0x%x\n", __func__, dai->id);
> > +		return -EINVAL;
> > +	}
> > +
> > +	return 0;
> > +}
> > +
> >   static const struct snd_soc_dai_ops q6dma_ops = {
> >   	.prepare	= q6apm_lpass_dai_prepare,
> >   	.startup	= q6apm_lpass_dai_startup,
> > @@ -401,6 +445,17 @@ static const struct snd_soc_dai_ops q6hdmi_ops = {
> >   	.trigger	= q6apm_lpass_dai_trigger,
> >   };
> >   
> > +static const struct snd_soc_dai_ops q6tdm_ops = {
> > +	.prepare	= q6apm_lpass_dai_prepare,
> > +	.startup	= q6apm_lpass_dai_startup,
> > +	.shutdown	= q6i2s_lpass_dai_shutdown,
> > +	.set_tdm_slot	= q6tdm_set_tdm_slot,
> > +	.hw_params	= q6dma_hw_params,
> > +	.set_fmt	= q6i2s_set_fmt,
> > +	.set_sysclk	= q6i2s_set_sysclk,
> > +	.trigger	= q6apm_lpass_dai_trigger,
> > +};
> > +
> >   static const struct snd_soc_component_driver q6apm_lpass_dai_component = {
> >   	.name = "q6apm-be-dai-component",
> >   	.of_xlate_dai_name = q6dsp_audio_ports_of_xlate_dai_name,
> > @@ -429,6 +484,7 @@ static int of_q6apm_parse_dai_data(struct device *dev,
> >   		case PRIMARY_MI2S_RX ... QUATERNARY_MI2S_TX:
> >   		case QUINARY_MI2S_RX ... QUINARY_MI2S_TX:
> >   		case SENARY_MI2S_RX ... SENARY_MI2S_TX:
> > +		case PRIMARY_TDM_RX_0 ... QUINARY_TDM_TX_7:
> >   			priv = &data->priv[id];
> >   			priv->mclk = of_clk_get_by_name(node, "mclk");
> >   			if (IS_ERR(priv->mclk)) {
> > @@ -490,6 +546,7 @@ static int q6apm_lpass_dai_dev_probe(struct platform_device *pdev)
> >   	cfg.q6i2s_ops = &q6i2s_ops;
> >   	cfg.q6dma_ops = &q6dma_ops;
> >   	cfg.q6hdmi_ops = &q6hdmi_ops;
> > +	cfg.q6tdm_ops = &q6tdm_ops;
> >   	dais = q6dsp_audio_ports_set_config(dev, &cfg, &num_dais);
> >   
> >   	ret = devm_snd_soc_register_component(dev, &q6apm_lpass_dai_component, dais, num_dais);