Skip to contentSkip to conversation

ASoC: SDCA: class-driver fixes and PDM data-port support

PATCH Applied Srinivas Kandagatla 4 participants 13 messages Oct 20 – Nov 2

Subsystems (5)
Stable branch Sound ASoC SoundWire +1 more subsystems
lore
Share & tools
lore
#0 lore raw

v2 cover ASoC: SDCA: class-driver fixes and PDM data-port support

Three small fixes in the SDCA class driver (propagating a read_prop failure, dropping an extra pm_runtime reference on probe failure, and re-enabling the IRQ on a failed system suspend), plus three patches that teach the SDCA class_function path to tag PDM data-port terminals and the SoundWire core to treat PDM streams correctly.

The three fixes carry Fixes: tags and are Cc'd to stable; the PDM support patches have no Fixes: tag as they add new behaviour rather than fix a regression.

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

Merge strategy: patches 1-4 and patch 6 go via ASoC, patch 5 goes via SoundWire. There is no compile-time dependency between the SoundWire and ASoC changes: patch 5 can land in SoundWire while patches 1-4, 6 land in ASoC in any order. Patch 6's runtime behaviour depends on patch 5 to work end-to-end on Qualcomm SDCA targets, but each tree compiles and tests cleanly on its own.

Changes in v2:
- 2/6: switched from reordering pm_runtime_get_noresume() to dropping the extra reference on devm_pm_runtime_enable() failure, per
- Replaces v1 4/4 ("clear sconfig.bps on PDM data-port terminals") with v2 4-6/6:
- Moved the PDM terminal-type check out of sdca_class_function.c into a new sdca_asoc_dai_is_pdm() helper, per Charles.
- Signal PDM via sconfig.type = SDW_STREAM_PDM instead of clearing sconfig.bps, per Pierre-Louis Bossart (use the type field that already exists in struct sdw_stream_config).
- Added a core SoundWire change that skips the bps mismatch check for SDW_STREAM_PDM so the slave's PCM-container-width bps and the master's PDM bps convention can co-exist without -EINVAL.
- v1: https://lore.kernel.org/all/[v1-cover-letter-message-id]/

Srinivas Kandagatla (6):
ASoC: SDCA: propagate sdw_slave_read_prop() failure in class_read_prop
ASoC: SDCA: drop pm_runtime reference on probe failure path
ASoC: SDCA: re-enable IRQ on pm_runtime_force_suspend failure
ASoC: SDCA: add sdca_asoc_dai_is_pdm() helper
soundwire: stream: skip bps mismatch check for PDM streams
ASoC: SDCA: class_function: mark PDM data-port terminals

 drivers/soundwire/stream.c           |  4 +++-
 include/sound/sdca_asoc.h            |  2 ++
 sound/soc/sdca/sdca_asoc.c           | 23 +++++++++++++++++++++++
 sound/soc/sdca/sdca_class.c          | 10 ++++++++--
 sound/soc/sdca/sdca_class_function.c |  3 +++
 5 files changed, 39 insertions(+), 3 deletions(-)

base-commit: 3d229ce89862b8e82d71a9e11d6f840ca272a6d6

Signature · 1 line
2.53.0
#1 lore raw

PATCH v2 1/6 ASoC: SDCA: propagate sdw_slave_read_prop() failure in class_read_prop

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")
Assisted-by LLM
---
 sound/soc/sdca/sdca_class.c | 5 ++++-
 1 file changed, 4 insertions(+), 1 deletion(-)
sound/soc/sdca/sdca_class.c +4 -1
index 79fdb672be1d..bcc660aeb739 100644
@@ -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 |
Signature · 1 line
2.53.0
#2 lore raw

PATCH v2 2/6 ASoC: SDCA: drop pm_runtime reference on probe failure path

In reply to Srinivas Kandagatla

sdca_class_probe() takes an extra usage count with pm_runtime_get_noresume() to keep the device powered on until the deferred boot work has run. If devm_pm_runtime_enable() then fails, that reference is never dropped.

Drop it on the error path.

Fixes 2d877d0659cb ("ASoC: SDCA: Add basic SDCA class driver")
Assisted-by LLM
---
 sound/soc/sdca/sdca_class.c | 4 +++-
 1 file changed, 3 insertions(+), 1 deletion(-)
sound/soc/sdca/sdca_class.c +3 -1
index bcc660aeb739..ca311209c6d1 100644
@@ -197,8 +197,10 @@ int sdca_class_probe(struct sdw_slave *sdw,
 	pm_runtime_get_noresume(dev);
 
 	ret = devm_pm_runtime_enable(dev);
-	if (ret)
+	if (ret) {
+		pm_runtime_put_sync(dev);
 		return ret;
+	}
 
 	queue_work(system_long_wq, &drv->boot_work);
 
Signature · 1 line
2.53.0
maintainer Reviewed-by on v2 2/6
#3 lore raw

Re: [PATCH v2 2/6] ASoC: SDCA: drop pm_runtime reference on probe failure path

Quoted 5 lines · Srinivas Kandagatla

sdca_class_probe() takes an extra usage count with pm_runtime_get_noresume() to keep the device powered on until the deferred boot work has run. If devm_pm_runtime_enable() then fails, that reference is never dropped.

Quoted 7 lines

Drop it on the error path.

Fixes 2d877d0659cb ("ASoC: SDCA: Add basic SDCA class driver")
Assisted-by LLM

---

Thanks,
Charles

#4 lore raw

PATCH v2 3/6 ASoC: SDCA: re-enable IRQ on pm_runtime_force_suspend failure

In reply to 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")
Assisted-by LLM
---
 sound/soc/sdca/sdca_class.c | 1 +
 1 file changed, 1 insertion(+)
sound/soc/sdca/sdca_class.c +1 -0
index ca311209c6d1..ee61c983dc57 100644
@@ -265,6 +265,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;
 	}
 
Signature · 1 line
2.53.0
#5 lore raw

PATCH v2 4/6 ASoC: SDCA: add sdca_asoc_dai_is_pdm() helper

In reply to Srinivas Kandagatla

Resolve the SDCA terminal type for a DAI and return true for the spec-defined PDM terminals (SDCA_TERM_TYPE_RAW_PDM_MIC and SDCA_TERM_TYPE_PDM_RENDER_STREAM).

Used by sdca_class_function to tag PDM streams; expose it so other SDCA codec drivers can share the check.

Assisted-by LLM
---
 include/sound/sdca_asoc.h  |  2 ++
 sound/soc/sdca/sdca_asoc.c | 23 +++++++++++++++++++++++
 2 files changed, 25 insertions(+)
include/sound/sdca_asoc.h +2 -0
index 599a72f33515..99c39b5944e3 100644
@@ -112,4 +112,6 @@ int sdca_asoc_pde_poll_actual_ps(struct regmap *regmap,
 				 int from_ps, int to_ps,
 				 const struct sdca_pde_delay *pde_delays,
 				 int num_delays);
+bool sdca_asoc_dai_is_pdm(struct sdca_function_data *function,
+			  struct snd_soc_dai *dai);
 #endif // __SDCA_ASOC_H__
sound/soc/sdca/sdca_asoc.c +23 -0
index ccc579f878ff..559a0fdfd98e 100644
@@ -1781,3 +1781,26 @@ int sdca_asoc_hw_params(struct device *dev, struct regmap *regmap,
 	return 0;
 }
 EXPORT_SYMBOL_NS(sdca_asoc_hw_params, "SND_SOC_SDCA");
+
+/**
+ * sdca_asoc_dai_is_pdm - True if the DAI is routed to a PDM data-port terminal
+ * @function: Pointer to the Function information.
+ * @dai: Pointer to the ASoC DAI.
+ *
+ * Return: true for SDCA_TERM_TYPE_RAW_PDM_MIC and
+ * SDCA_TERM_TYPE_PDM_RENDER_STREAM, false otherwise.
+ */
+bool sdca_asoc_dai_is_pdm(struct sdca_function_data *function,
+			  struct snd_soc_dai *dai)
+{
+	struct sdca_entity *entity = &function->entities[dai->id];
+
+	switch (entity->iot.type) {
+	case SDCA_TERM_TYPE_RAW_PDM_MIC:
+	case SDCA_TERM_TYPE_PDM_RENDER_STREAM:
+		return true;
+	default:
+		return false;
+	}
+}
+EXPORT_SYMBOL_NS(sdca_asoc_dai_is_pdm, "SND_SOC_SDCA");
Signature · 1 line
2.53.0
maintainer Reviewed-by on v2 4/6
#6 lore raw

Re: [PATCH v2 4/6] ASoC: SDCA: add sdca_asoc_dai_is_pdm() helper

Quoted 10 lines · Srinivas Kandagatla

Resolve the SDCA terminal type for a DAI and return true for the spec-defined PDM terminals (SDCA_TERM_TYPE_RAW_PDM_MIC and SDCA_TERM_TYPE_PDM_RENDER_STREAM).

Used by sdca_class_function to tag PDM streams; expose it so other SDCA codec drivers can share the check.

Assisted-by LLM

---

Thanks,
Charles

#7 lore raw

PATCH v2 5/6 soundwire: stream: skip bps mismatch check for PDM streams

In reply to Srinivas Kandagatla

PDM data-port terminals carry a wire word length defined by the stream payload, not by any host-side bps field.

Skip the sdw_config_stream() bps mismatch check for SDW_STREAM_PDM. Behaviour for SDW_STREAM_PCM and SDW_STREAM_BPT is unchanged.

Assisted-by LLM
---
 drivers/soundwire/stream.c | 4 +++-
 1 file changed, 3 insertions(+), 1 deletion(-)
drivers/soundwire/stream.c +3 -1
index 0b00fcdcab2d..31db368ba24b 100644
@@ -1374,7 +1374,9 @@ static int sdw_config_stream(struct device *dev,
 		return -EINVAL;
 	}
 
-	if (stream->params.bps &&
+	/* PDM streams have no stream-wide bps; skip the match for them. */
+	if (stream_config->type != SDW_STREAM_PDM &&
+	    stream->params.bps &&
 	    stream->params.bps != stream_config->bps) {
 		dev_err(dev, "bps not matching, stream:%s\n", stream->name);
 		return -EINVAL;
Signature · 1 line
2.53.0
maintainer Reviewed-by on v2 5/6
#8 lore raw

Re: [PATCH v2 5/6] soundwire: stream: skip bps mismatch check for PDM streams

Quoted 9 lines · Srinivas Kandagatla

PDM data-port terminals carry a wire word length defined by the stream payload, not by any host-side bps field.

Skip the sdw_config_stream() bps mismatch check for SDW_STREAM_PDM. Behaviour for SDW_STREAM_PCM and SDW_STREAM_BPT is unchanged.

Assisted-by LLM

---

Thanks,
Charles

#9 lore raw

PATCH v2 6/6 ASoC: SDCA: class_function: mark PDM data-port terminals

In reply to Srinivas Kandagatla

snd_sdw_params_to_config() leaves sconfig.type at SDW_STREAM_PCM, which is wrong when the DAI is routed to a PDM data-port terminal.

Set sconfig.type to SDW_STREAM_PDM for PDM terminals so the SoundWire core sees the right type and does not enforce bps match against the PCM-container-width value stamped by the helper.

Assisted-by LLM
---
 sound/soc/sdca/sdca_class_function.c | 3 +++
 1 file changed, 3 insertions(+)
sound/soc/sdca/sdca_class_function.c +3 -0
index 13b70f0f0dbf..41f6f0eb8563 100644
@@ -142,6 +142,9 @@ static int class_function_sdw_add_peripheral(struct snd_pcm_substream *substream
 
 	snd_sdw_params_to_config(substream, params, &sconfig, &pconfig);
 
+	if (sdca_asoc_dai_is_pdm(drv->function, dai))
+		sconfig.type = SDW_STREAM_PDM;
+
 	/*
 	 * FIXME: As also noted in sdca_asoc_get_port(), currently only
 	 * a single unshared port is supported for each DAI.
Signature · 1 line
2.53.0
maintainer Reviewed-by on v2 6/6
#10 lore raw

Re: [PATCH v2 6/6] ASoC: SDCA: class_function: mark PDM data-port terminals

Quoted 10 lines · Srinivas Kandagatla

snd_sdw_params_to_config() leaves sconfig.type at SDW_STREAM_PCM, which is wrong when the DAI is routed to a PDM data-port terminal.

Set sconfig.type to SDW_STREAM_PDM for PDM terminals so the SoundWire core sees the right type and does not enforce bps match against the PCM-container-width value stamped by the helper.

Assisted-by LLM

---

Thanks,
Charles

maintainer
#11 lore raw

Re: (subset) [PATCH v2 0/6] ASoC: SDCA: class-driver fixes and PDM data-port support

In reply to Srinivas Kandagatla
Quoted 9 lines · Srinivas Kandagatla

ASoC: SDCA: class-driver fixes and PDM data-port support

Three small fixes in the SDCA class driver (propagating a read_prop failure, dropping an extra pm_runtime reference on probe failure, and re-enabling the IRQ on a failed system suspend), plus three patches that teach the SDCA class_function path to tag PDM data-port terminals and the SoundWire core to treat PDM streams correctly.

[...]

Applied to

https://git.kernel.org/pub/scm/linux/kernel/git/broonie/sound.git for-7.4

Thanks!

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

      https://git.kernel.org/broonie/sound/c/79cfb5aebbf0
[2/6] ASoC: SDCA: drop pm_runtime reference on probe failure path
      https://git.kernel.org/broonie/sound/c/86331b76b32d
[3/6] ASoC: SDCA: re-enable IRQ on pm_runtime_force_suspend failure
      https://git.kernel.org/broonie/sound/c/d34a818a331f

All being well this means that it will be integrated into the linux-next tree (usually sometime in the next 24 hours) and sent to Linus during the next merge window (or sooner if it is a bug fix), however if problems are discovered then the patch may be dropped or reverted.

You may get further e-mails resulting from automated or manual testing and review of the tree, please engage with people reporting problems and send followup patches addressing any issues that are reported if needed.

If any updates are required or you are submitting further changes they should be sent as incremental updates against current git, existing patches will not be replaced.

Please add any relevant lists and maintainers to the CCs when replying to this mail.

Thanks,
Mark

maintainer Applied
#12 lore raw

Re: (subset) [PATCH v2 0/6] ASoC: SDCA: class-driver fixes and PDM data-port support

In reply to Srinivas Kandagatla
Quoted 11 lines · Srinivas Kandagatla

Three small fixes in the SDCA class driver (propagating a read_prop failure, dropping an extra pm_runtime reference on probe failure, and re-enabling the IRQ on a failed system suspend), plus three patches that teach the SDCA class_function path to tag PDM data-port terminals and the SoundWire core to treat PDM streams correctly.

The three fixes carry Fixes: tags and are Cc'd to stable; the PDM support patches have no Fixes: tag as they add new behaviour rather than fix a regression.

[...]

Applied, thanks!

[5/6] soundwire: stream: skip bps mismatch check for PDM streams

      commit: afd88e7ad78170040612f608203734156b8a9eb1

Best regards,

Signature · 1 line
~Vinod