[PATCH v2] ASoC: soc-core: Create device_link to ensure correct suspend order

[Richard Fitzgerald]

In snd_soc_bind_card() create a device_link from card to all components
to ensure correct order of system_suspend. The card is the consumer and
the components are the supplier, so that the card will system_suspend
before any of the components.

The PM core will normally system_suspend drivers in the opposite order
that they registered. This ensures children are suspended before their
parents, for example users of a bus driver should suspend before the bus
driver suspends.

For ASoC, snd_soc_suspend() shuts down any active audio, which requires
that the components are still able to communicate with their hardware.
Previously there was nothing to ensure this ordering, because there is
(usually) no relationship between a machine driver and component drivers.
If the machine driver registered before the codec drivers, the codec
drivers would be suspended before the machine driver snd_soc_suspend()
runs, so that ASoC is attempting to stop audio on a driver that has
already suspended.

Creating a device_link is safe if there is already a device_link between
those devices because of multiple components sharing the same dev.
device_link_add() kernel doc says:

 "if a device link between the given @consumer and @supplier pair
  exists already when this function is called for them, the existing link
  will be returned regardless of its current type and status ...
  The caller of this function is then expected to treat
  the link as though it has just been created, so (in particular) if
  DL_FLAG_STATELESS was passed in @flags, the link needs to be released
  explicitly when not needed any more"

For the same reason it is safe if the codec driver or machine driver
later call device_link_add() to create a link between the same two
devices.

(I have tested creating multiple links between the card->dev and a
component->dev and did not encounter any problems with suspend/resume or
module unloading.)

The DL_FLAG_AUTOREMOVE_* flags assume that they are being called from
the probe() function of that device. This isn't guaranteed in ASoC card
binding because of deferred binding. The exact behavior and consequences
of the DL_FLAG_AUTOREMOVE_* are also unclear from the documentation.
So DL_FLAG_STATELESS is used for safety, and the links are removed
explicitly when the card unbinds or if the bind fails.

Signed-off-by: Richard Fitzgerald <rf@xxxxxxxxxxxxxxxxxxxxx>
---
for-next

Changes in V2:
- Use DL_FLAG_STATELESS to avoid unclear/unstated behavior of the
  DL_FLAG_AUTOREMOVE_* flags.

- Skip creating a device link for a component that is the same device
  as the card.

 sound/soc/soc-core.c | 46 +++++++++++++++++++++++++++++++++++++++++++-
 1 file changed, 45 insertions(+), 1 deletion(-)

diff --git a/sound/soc/soc-core.c b/sound/soc/soc-core.c
index b8c9ddfc4490..ac9b2269c26e 100644
--- a/sound/soc/soc-core.c
+++ b/sound/soc/soc-core.c
@@ -2138,10 +2138,29 @@ static void soc_cleanup_card_resources(struct snd_soc_card *card)
 	}
 }
 
+static void snd_soc_remove_device_links(struct snd_soc_card *card,
+					struct snd_soc_component *stop_at)
+{
+	struct snd_soc_component *component;
+
+	for_each_card_components(card, component) {
+		if (card->dev == component->dev)
+			continue;
+
+		device_link_remove(card->dev, component->dev);
+
+		if (component == stop_at)
+			return;
+	}
+}
+
 static void snd_soc_unbind_card(struct snd_soc_card *card)
 {
 	if (snd_soc_card_is_instantiated(card)) {
 		card->instantiated = false;
+
+		snd_soc_remove_device_links(card, NULL);
+
 		soc_cleanup_card_resources(card);
 	}
 }
@@ -2151,6 +2170,7 @@ static int snd_soc_bind_card(struct snd_soc_card *card)
 	struct snd_soc_pcm_runtime *rtd;
 	struct snd_soc_component *component;
 	struct snd_soc_dapm_context *dapm = snd_soc_card_to_dapm(card);
+	struct snd_soc_component *last_devlinked_component = NULL;
 	int ret;
 
 	snd_soc_card_mutex_lock_root(card);
@@ -2275,6 +2295,25 @@ static int snd_soc_bind_card(struct snd_soc_card *card)
 		}
 	}
 
+	/*
+	 * Add device_link from card to component so that system_suspend
+	 * will be done in the correct order. The card must suspend first
+	 * to stop audio activity before the components suspend.
+	 */
+	for_each_card_components(card, component) {
+		if (card->dev == component->dev)
+			continue;
+
+		if (!device_link_add(card->dev, component->dev, DL_FLAG_STATELESS)) {
+			dev_warn(card->dev, "Failed to create device link to %s\n",
+				 dev_name(component->dev));
+			ret = -EINVAL;
+			goto probe_end;
+		}
+
+		last_devlinked_component = component;
+	}
+
 	ret = snd_soc_card_late_probe(card);
 	if (ret < 0)
 		goto probe_end;
@@ -2303,8 +2342,13 @@ static int snd_soc_bind_card(struct snd_soc_card *card)
 			pinctrl_pm_select_sleep_state(component->dev);
 
 probe_end:
-	if (ret < 0)
+	if (ret < 0) {
+		if (last_devlinked_component)
+			snd_soc_remove_device_links(card, last_devlinked_component);
+
 		soc_cleanup_card_resources(card);
+	}
+
 	if (ret == -EPROBE_DEFER) {
 		list_add(&card->list, &unbind_card_list);
 		ret = 0;
-- 
2.47.3