[PATCH 0/4] ASoC: SDCA: class-driver fixes and PDM data-port support

[PATCH 0/4] ASoC: SDCA: class-driver fixes and PDM data-port support

[Srinivas Kandagatla]

Three small fixes in the SDCA class driver (propagating a read_prop
failure, correcting a pm_runtime ordering, and re-enabling the IRQ on
a failed system suspend) plus one SDCA class_function change that
clears sconfig.bps on PDM data-port terminals so the master's
compute_params() can drive the per-port wire word length.

The three fixes carry Fixes: tags and are Cc'd to stable; the PDM
clear is deliberately left without a Fixes: tag as it is a semantic
adjustment for PDM-terminal SDCA codecs rather than a regression.

The three issues were flagged during a sashiko review during WCD9378
SDCA codec driver patch review.

Srinivas Kandagatla (4):
  ASoC: SDCA: propagate sdw_slave_read_prop() failure in class_read_prop
  ASoC: SDCA: enable pm_runtime before taking the initial reference
  ASoC: SDCA: re-enable IRQ on pm_runtime_force_suspend failure
  ASoC: SDCA: class_function: clear sconfig.bps on PDM data-port
    terminals

 sound/soc/sdca/sdca_class.c          |  9 +++++++--
 sound/soc/sdca/sdca_class_function.c | 15 +++++++++++++++
 2 files changed, 22 insertions(+), 2 deletions(-)


base-commit: 2efb7a11f1aa4d2dc285fe2680cca22cb9f72f64
-- 
2.53.0

[PATCH 1/4] ASoC: SDCA: propagate sdw_slave_read_prop() failure in class_read_prop

[Srinivas Kandagatla]

sdw_slave_read_prop() can fail with -ENOMEM; propagate the return
instead of leaving slave->prop partially initialised.

Fixes: 2d877d0659cb ("ASoC: SDCA: Add basic SDCA class driver")
Cc: stable@vger.kernel.org
Assisted-by: LLM
Signed-off-by: Srinivas Kandagatla <srinivas.kandagatla@oss.qualcomm.com>
---
 sound/soc/sdca/sdca_class.c | 5 ++++-
 1 file changed, 4 insertions(+), 1 deletion(-)

diff --git a/sound/soc/sdca/sdca_class.c b/sound/soc/sdca/sdca_class.c
index 79fdb672be1d..bcc660aeb739 100644
--- a/sound/soc/sdca/sdca_class.c
+++ b/sound/soc/sdca/sdca_class.c
@@ -27,8 +27,11 @@
 static int class_read_prop(struct sdw_slave *sdw)
 {
 	struct sdw_slave_prop *prop = &sdw->prop;
+	int ret;
 
-	sdw_slave_read_prop(sdw);
+	ret = sdw_slave_read_prop(sdw);
+	if (ret)
+		return ret;
 
 	prop->use_domain_irq = true;
 	prop->scp_int1_mask = SDW_SCP_INT1_BUS_CLASH | SDW_SCP_INT1_PARITY |
-- 
2.53.0

[PATCH 2/4] ASoC: SDCA: enable pm_runtime before taking the initial reference

[Srinivas Kandagatla]

If devm_pm_runtime_enable() failed, the usage count taken earlier by
pm_runtime_get_noresume() was never dropped. Reorder so the get only
happens once enable has succeeded.

Fixes: 2d877d0659cb ("ASoC: SDCA: Add basic SDCA class driver")
Cc: stable@vger.kernel.org
Assisted-by: LLM
Signed-off-by: Srinivas Kandagatla <srinivas.kandagatla@oss.qualcomm.com>
---
 sound/soc/sdca/sdca_class.c | 3 ++-
 1 file changed, 2 insertions(+), 1 deletion(-)

diff --git a/sound/soc/sdca/sdca_class.c b/sound/soc/sdca/sdca_class.c
index bcc660aeb739..3361db188046 100644
--- a/sound/soc/sdca/sdca_class.c
+++ b/sound/soc/sdca/sdca_class.c
@@ -194,12 +194,13 @@ int sdca_class_probe(struct sdw_slave *sdw,
 	pm_runtime_set_autosuspend_delay(dev, 250);
 	pm_runtime_use_autosuspend(dev);
 	pm_runtime_set_active(dev);
-	pm_runtime_get_noresume(dev);
 
 	ret = devm_pm_runtime_enable(dev);
 	if (ret)
 		return ret;
 
+	pm_runtime_get_noresume(dev);
+
 	queue_work(system_long_wq, &drv->boot_work);
 
 	return 0;
-- 
2.53.0

[PATCH 3/4] ASoC: SDCA: re-enable IRQ on pm_runtime_force_suspend failure

[Srinivas Kandagatla]

The SoundWire IRQ was disabled before pm_runtime_force_suspend() but
never re-enabled when the force suspend failed, leaving the codec
unable to receive interrupts.

Fixes: 2d877d0659cb ("ASoC: SDCA: Add basic SDCA class driver")
Cc: stable@vger.kernel.org
Assisted-by: LLM
Signed-off-by: Srinivas Kandagatla <srinivas.kandagatla@oss.qualcomm.com>
---
 sound/soc/sdca/sdca_class.c | 1 +
 1 file changed, 1 insertion(+)

diff --git a/sound/soc/sdca/sdca_class.c b/sound/soc/sdca/sdca_class.c
index 3361db188046..8396a4c058ec 100644
--- a/sound/soc/sdca/sdca_class.c
+++ b/sound/soc/sdca/sdca_class.c
@@ -264,6 +264,7 @@ int sdca_class_system_suspend(struct sdca_class_drv *drv)
 	ret = pm_runtime_force_suspend(drv->dev);
 	if (ret) {
 		dev_err(drv->dev, "failed to force suspend: %d\n", ret);
+		enable_irq(drv->sdw->irq);
 		return ret;
 	}
 
-- 
2.53.0

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

[Srinivas Kandagatla]

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@oss.qualcomm.com>
---
 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;
+	}
+}
+
 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;
+
 	ret = sdw_stream_add_slave(sdw, &sconfig, &pconfig, 1, sdw_stream);
 	if (ret) {
 		dev_err(drv->dev, "failed to add sdw stream: %d\n", ret);
-- 
2.53.0

Re: [PATCH 1/4] ASoC: SDCA: propagate sdw_slave_read_prop() failure in class_read_prop

[Charles Keepax]

On Sun, Oct 18, 2026 at 08:23:56PM +0100, Srinivas Kandagatla wrote:
> sdw_slave_read_prop() can fail with -ENOMEM; propagate the return
> instead of leaving slave->prop partially initialised.
> 
> Fixes: 2d877d0659cb ("ASoC: SDCA: Add basic SDCA class driver")
> Cc: stable@vger.kernel.org
> Assisted-by: LLM
> Signed-off-by: Srinivas Kandagatla <srinivas.kandagatla@oss.qualcomm.com>
> ---

Reviewed-by: Charles Keepax <ckeepax@opensource.cirrus.com>

Thanks,
Charles

Re: [PATCH 2/4] ASoC: SDCA: enable pm_runtime before taking the initial reference

[Charles Keepax]

On Sun, Oct 18, 2026 at 08:23:57PM +0100, Srinivas Kandagatla wrote:
> If devm_pm_runtime_enable() failed, the usage count taken earlier by
> pm_runtime_get_noresume() was never dropped. Reorder so the get only
> happens once enable has succeeded.
> 
> Fixes: 2d877d0659cb ("ASoC: SDCA: Add basic SDCA class driver")
> Cc: stable@vger.kernel.org
> Assisted-by: LLM
> Signed-off-by: Srinivas Kandagatla <srinivas.kandagatla@oss.qualcomm.com>
> ---
>  sound/soc/sdca/sdca_class.c | 3 ++-
>  1 file changed, 2 insertions(+), 1 deletion(-)
> 
> diff --git a/sound/soc/sdca/sdca_class.c b/sound/soc/sdca/sdca_class.c
> index bcc660aeb739..3361db188046 100644
> --- a/sound/soc/sdca/sdca_class.c
> +++ b/sound/soc/sdca/sdca_class.c
> @@ -194,12 +194,13 @@ int sdca_class_probe(struct sdw_slave *sdw,
>  	pm_runtime_set_autosuspend_delay(dev, 250);
>  	pm_runtime_use_autosuspend(dev);
>  	pm_runtime_set_active(dev);
> -	pm_runtime_get_noresume(dev);
>  
>  	ret = devm_pm_runtime_enable(dev);
>  	if (ret)
>  		return ret;
>  
> +	pm_runtime_get_noresume(dev);

I think it would be better to drop the reference on the error
path. Doing it this way round there is a very small chance that
the device auto-suspends between the enable and get_noresume.
The intention was to keep everything powered on until bring up
was complete.

Thanks,
Charles

Re: [PATCH 3/4] ASoC: SDCA: re-enable IRQ on pm_runtime_force_suspend failure

[Charles Keepax]

On Sun, Oct 18, 2026 at 08:23:58PM +0100, Srinivas Kandagatla wrote:
> The SoundWire IRQ was disabled before pm_runtime_force_suspend() but
> never re-enabled when the force suspend failed, leaving the codec
> unable to receive interrupts.
> 
> Fixes: 2d877d0659cb ("ASoC: SDCA: Add basic SDCA class driver")
> Cc: stable@vger.kernel.org
> Assisted-by: LLM
> Signed-off-by: Srinivas Kandagatla <srinivas.kandagatla@oss.qualcomm.com>
> ---

Reviewed-by: Charles Keepax <ckeepax@opensource.cirrus.com>

Thanks,
Charles

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

[Charles Keepax]

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

>  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.

Thanks,
Charles

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

[Pierre-Louis Bossart]

>>  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.

It's a bit odd to rely on bps when we already have a type for the stream
in struct sdw_stream_config:

@type: Stream type PCM, PDM or BPT