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

[Prasad Kumpatla <prasad.kumpatla@oss.qualcomm.com>]

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@oss.qualcomm.com>
>> ---
>>   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);