Re: [PATCH 4/4] ASoC: SDCA: class_function: clear sconfig.bps on PDM data-port terminals

[Srinivas Kandagatla]

On 10/19/26 10:02 AM, Charles Keepax wrote:
> On Sun, Oct 18, 2026 at 08:23:59PM +0100, Srinivas Kandagatla wrote:
>> snd_sdw_params_to_config() derives sconfig.bps from the PCM container
>> width, which is not meaningful on PDM data-port terminals and clashes
>> with masters that declare their own view of the wire word length (e.g.
>> the Qualcomm SoundWire master hardcodes sconfig.bps = 1 for PDM).
>>
>> Clear sconfig.bps when the DAI's SDCA terminal is PDM so compute_params()
>> or the slave silicon drives the per-port wire word length.
>>
>> Assisted-by: LLM
>> Signed-off-by: Srinivas Kandagatla <srinivas.kandagatla@xxxxxxxxxxxxxxxx>
>> ---
>>  sound/soc/sdca/sdca_class_function.c | 15 +++++++++++++++
>>  1 file changed, 15 insertions(+)
>>
>> diff --git a/sound/soc/sdca/sdca_class_function.c b/sound/soc/sdca/sdca_class_function.c
>> index 13b70f0f0dbf..acaadd12432a 100644
>> --- a/sound/soc/sdca/sdca_class_function.c
>> +++ b/sound/soc/sdca/sdca_class_function.c
>> @@ -126,6 +126,17 @@ static int class_function_startup(struct snd_pcm_substream *substream,
>>  					 substream, dai);
>>  }
>>  
>> +static bool class_function_terminal_is_pdm(enum sdca_terminal_type t)
>> +{
>> +	switch (t) {
>> +	case SDCA_TERM_TYPE_RAW_PDM_MIC:
>> +	case SDCA_TERM_TYPE_PDM_RENDER_STREAM:
>> +		return true;
>> +	default:
>> +		return false;
>> +	}
>> +}
> 
> I think this would make sense as a generic helper in sdca_asoc.c
> probably taking the function and the DAI as arguments.

v2 will have this change.
> 
>>  static int class_function_sdw_add_peripheral(struct snd_pcm_substream *substream,
>>  					     struct snd_pcm_hw_params *params,
>>  					     struct snd_soc_dai *dai)
>> @@ -133,6 +144,7 @@ static int class_function_sdw_add_peripheral(struct snd_pcm_substream *substream
>>  	struct class_function_drv *drv = snd_soc_component_get_drvdata(dai->component);
>>  	struct sdw_stream_runtime *sdw_stream = snd_soc_dai_get_dma_data(dai, substream);
>>  	struct sdw_slave *sdw = dev_to_sdw_dev(drv->dev->parent);
>> +	struct sdca_entity *entity = &drv->function->entities[dai->id];
>>  	struct sdw_stream_config sconfig = {0};
>>  	struct sdw_port_config pconfig = {0};
>>  	int ret;
>> @@ -152,6 +164,9 @@ static int class_function_sdw_add_peripheral(struct snd_pcm_substream *substream
>>  
>>  	pconfig.num = ret;
>>  
>> +	if (class_function_terminal_is_pdm(entity->iot.type))
>> +		sconfig.bps = 0;
> 
> I think this is fine, as you say bps makes no sense for a PDM
> stream but I might need to spend a little more time working out
> how this all propagates through the core. I guess my concern is
> if other drivers don't handle PDM in the same way.
in v2, i have now set the stream type correctly based on terminal type.

--srini
> 
> Thanks,
> Charles