mirror of https://lore.kernel.org/lkml/
 help / color / mirror / Atom feed
* [PATCH v3] ASoC: cs35l56: Fix probe deadlock waiting for SoundWire enumeration
@ 2026-09-01 12:26 Richard Fitzgerald
  2026-09-01 15:18 ` Mark Brown
  2026-09-01 15:21 ` Mark Brown
  0 siblings, 2 replies; 6+ messages in thread
From: Richard Fitzgerald @ 2026-09-01 12:26 UTC (permalink / raw)
  To: broonie; +Cc: linux-sound, linux-kernel, patches

On SoundWire, don't call snd_soc_register_component() from driver probe().
Instead, queue a work item after first SoundWire attach to call
snd_soc_register_component(). This prevents a deadlock if
snd_soc_register_component() directly calls cs35l56_component_probe().

On SoundWire, the registers are not accessible during driver probe().
Drivers must return from their probe() and wait for the SoundWire core
to call their update_status() callback to report an ATTACHED status. The
cs35l56 driver handled this by calling snd_soc_register_component() from
driver probe() as usual, and cs35l56_component_probe() waited for
init_completion to be signalled. A SoundWire attach calls cs35l56_init()
which then signals init_completion.

This created a deadlock if this was the last component needed to complete
a card. In that case, snd_soc_register_component() directly called
cs35l56_component_probe() which led to this:

    driver probe()
        calls snd_soc_register_component()
            calls cs35l56_component_probe()
                waits for init_completion

In this case the driver probe() has not returned, so the SoundWire core
would not call update_status() and init_completion would not be signalled.

Fortunately, snd_soc_register_component() never returns -EPROBE_DEFER, so
it doesn't need to be called from a driver probe(). It can be deferred to
a work item. This work is queued after the first completed pass through
cs35l56_init(), so there is no need for it to wait for init_completion.
snd_soc_register_component() isn't called directly from cs35l56_init()
because cs35l56_init() runs in the SoundWire bus driver thread, and there
would be a risk of nested locking or lock inversion.

The work item is queued on a freezable workqueue to prevent a race between
the work item and system_suspend of another instance. If the workqueue
were not frozen it would be possible for the work item of one driver
instance to call snd_soc_register_component() which then calls
cs35l56_component_probe() of another instance while that instance is
already executing its system suspend functions.

The non-SoundWire case still calls snd_soc_register_component() from
cs35l56_common_probe() so that it is the last initialization action.
There's no need defer the call for I2S/SPI buses so we can also leave it
able to return errors during probe.

Fixes: 440c2d38950f7 ("ASoC: cs35l56: Wait for init_complete in cs35l56_component_probe()")
Signed-off-by: Richard Fitzgerald <rf@opensource.cirrus.com>
---
Changes in V3:
Rewritten to defer snd_soc_register_component() to a work item and only
queue the item after the device has enumerated.

Changes in V2:
- Fixed missing flush_work() in cs35l56_system_suspend() by calling
  cs35l56_wait_dsp_ready() which already does the correct flushes.

 sound/soc/codecs/cs35l56.c | 71 +++++++++++++++++++++++++++++++-------
 sound/soc/codecs/cs35l56.h |  2 ++
 2 files changed, 60 insertions(+), 13 deletions(-)

diff --git a/sound/soc/codecs/cs35l56.c b/sound/soc/codecs/cs35l56.c
index b9118ad8fab54..890429ab0dfba 100644
--- a/sound/soc/codecs/cs35l56.c
+++ b/sound/soc/codecs/cs35l56.c
@@ -1365,12 +1365,6 @@ static int _cs35l56_component_probe(struct snd_soc_component *component)
 
 	BUILD_BUG_ON(ARRAY_SIZE(cs35l56_tx_input_texts) != ARRAY_SIZE(cs35l56_tx_input_values));
 
-	if (!wait_for_completion_timeout(&cs35l56->init_completion,
-					 msecs_to_jiffies(5000))) {
-		dev_err(cs35l56->base.dev, "%s: init_completion timed out\n", __func__);
-		return -ENODEV;
-	}
-
 	cs35l56->dsp.part = kasprintf(GFP_KERNEL, "cs35l%02x", cs35l56->base.type);
 	if (!cs35l56->dsp.part)
 		return -ENOMEM;
@@ -1939,6 +1933,40 @@ static int cs35l56_try_get_broken_sdca_spkid_gpio(struct cs35l56_private *cs35l5
 	return ret;
 }
 
+static int cs35l56_component_register(struct cs35l56_private *cs35l56)
+{
+	int ret;
+
+	ret = snd_soc_register_component(cs35l56->base.dev,
+					 &soc_component_dev_cs35l56,
+					 cs35l56_dai, ARRAY_SIZE(cs35l56_dai));
+	if (ret < 0) {
+		dev_err(cs35l56->base.dev, "Register codec failed: %d\n", ret);
+		return ret;
+	}
+
+	cs35l56->component_registered = true;
+
+	return 0;
+}
+
+static void cs35l56_component_register_work(struct work_struct *work)
+{
+	struct cs35l56_private *cs35l56 = container_of(work,
+						       struct cs35l56_private,
+						       component_register_work);
+	int ret;
+
+	PM_RUNTIME_ACQUIRE_AUTOSUSPEND(cs35l56->base.dev, pm_err);
+	ret = PM_RUNTIME_ACQUIRE_ERR(&pm_err);
+	if (ret) {
+		dev_err(cs35l56->base.dev, "register_work failed to get pm_runtime: %d\n", ret);
+		return;
+	}
+
+	cs35l56_component_register(cs35l56);
+}
+
 int cs35l56_common_probe(struct cs35l56_private *cs35l56, int irq)
 {
 	int ret;
@@ -1947,6 +1975,7 @@ int cs35l56_common_probe(struct cs35l56_private *cs35l56, int irq)
 	mutex_init(&cs35l56->base.irq_lock);
 	cs35l56->base.cal_index = -1;
 	cs35l56->speaker_id = -ENOENT;
+	INIT_WORK(&cs35l56->component_register_work, cs35l56_component_register_work);
 
 	dev_set_drvdata(cs35l56->base.dev, cs35l56);
 
@@ -2020,12 +2049,17 @@ int cs35l56_common_probe(struct cs35l56_private *cs35l56, int irq)
 	if (ret)
 		goto err_remove_wm_adsp;
 
-	ret = snd_soc_register_component(cs35l56->base.dev,
-					 &soc_component_dev_cs35l56,
-					 cs35l56_dai, ARRAY_SIZE(cs35l56_dai));
-	if (ret < 0) {
-		dev_err_probe(cs35l56->base.dev, ret, "Register codec failed\n");
-		goto err_free_irq;
+	/*
+	 * Defer calling snd_soc_register_component() on SoundWire to prevent
+	 * a deadlock where it calls our component_probe(), which requires the
+	 * SoundWire enumeration to complete, but because we are still in probe()
+	 * the SoundWire core will not call the update_status() callback. At time
+	 * of writing snd_soc_register_component() never returns EPROBE_DEFER.
+	 */
+	if (!cs35l56->sdw_peripheral) {
+		ret = cs35l56_component_register(cs35l56);
+		if (ret < 0)
+			goto err_free_irq;
 	}
 
 	return 0;
@@ -2055,6 +2089,7 @@ EXPORT_SYMBOL_NS_GPL(cs35l56_common_probe, "SND_SOC_CS35L56_CORE");
 
 int cs35l56_init(struct cs35l56_private *cs35l56)
 {
+	bool first_time_init = !cs35l56->base.init_done;
 	int ret;
 
 	/*
@@ -2131,13 +2166,23 @@ int cs35l56_init(struct cs35l56_private *cs35l56)
 	cs35l56->base.init_done = true;
 	complete_all(&cs35l56->init_completion);
 
+	if (cs35l56->sdw_peripheral && first_time_init) {
+		/*
+		 * Hardware now accessible, queue work to call
+		 * snd_soc_register_component().
+		 */
+		queue_work(system_freezable_wq, &cs35l56->component_register_work);
+	}
+
 	return 0;
 }
 EXPORT_SYMBOL_NS_GPL(cs35l56_init, "SND_SOC_CS35L56_CORE");
 
 void cs35l56_remove(struct cs35l56_private *cs35l56)
 {
-	snd_soc_unregister_component(cs35l56->base.dev);
+	cancel_work_sync(&cs35l56->component_register_work);
+	if (cs35l56->component_registered)
+		snd_soc_unregister_component(cs35l56->base.dev);
 
 	cs35l56->base.init_done = false;
 
diff --git a/sound/soc/codecs/cs35l56.h b/sound/soc/codecs/cs35l56.h
index 35c02ae17de3d..f7cf8aa653e28 100644
--- a/sound/soc/codecs/cs35l56.h
+++ b/sound/soc/codecs/cs35l56.h
@@ -32,6 +32,7 @@ struct sdw_slave;
 struct cs35l56_private {
 	struct wm_adsp dsp; /* must be first member */
 	struct cs35l56_base base;
+	struct work_struct component_register_work;
 	struct work_struct dsp_work;
 	struct workqueue_struct *dsp_wq;
 	struct snd_soc_component *component;
@@ -41,6 +42,7 @@ struct cs35l56_private {
 	const char *fallback_fw_suffix;
 	bool soft_resetting;
 	bool sdw_attached;
+	bool component_registered;
 	struct completion init_completion;
 
 	int speaker_id;
-- 
2.47.3


^ permalink raw reply	[flat|nested] 6+ messages in thread
end of thread, other threads:[~2026-09-01 21:09 UTC | newest]

Thread overview: 6+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2026-09-01 12:26 [PATCH v3] ASoC: cs35l56: Fix probe deadlock waiting for SoundWire enumeration Richard Fitzgerald
2026-09-01 15:18 ` Mark Brown
2026-09-01 15:38   ` Richard Fitzgerald
2026-09-01 15:48     ` Mark Brown
2026-09-01 15:50       ` Richard Fitzgerald
2026-09-01 15:21 ` Mark Brown

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox
all inboxes | Powered by JetHome®