sound/soc/soc-core.c | 15 +++++++++++++++ 1 file changed, 15 insertions(+)
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.
Signed-off-by: Richard Fitzgerald <rf@opensource.cirrus.com>
---
Mark,
I think it would be ok to put this into -next and let it get some wider
testing before it goes into 7.1 or older.
sound/soc/soc-core.c | 15 +++++++++++++++
1 file changed, 15 insertions(+)
diff --git a/sound/soc/soc-core.c b/sound/soc/soc-core.c
index c0599031a3e4..1820143a144a 100644
--- a/sound/soc/soc-core.c
+++ b/sound/soc/soc-core.c
@@ -2297,6 +2297,21 @@ static int snd_soc_bind_card(struct snd_soc_card *card)
if (!snd_soc_component_active(component))
pinctrl_pm_select_sleep_state(component->dev);
+ /*
+ * 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 (!device_link_add(card->dev, component->dev,
+ DL_FLAG_AUTOREMOVE_CONSUMER)) {
+ dev_warn(card->dev, "Failed to create device link to %s\n",
+ dev_name(component->dev));
+ ret = -EINVAL;
+ goto probe_end;
+ }
+ }
+
probe_end:
if (ret < 0)
soc_cleanup_card_resources(card);
--
2.47.3On 01/06/2026 3:28 pm, Richard Fitzgerald wrote: > 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. > > Signed-off-by: Richard Fitzgerald <rf@opensource.cirrus.com> > --- > Mark, > I think it would be ok to put this into -next and let it get some wider > testing before it goes into 7.1 or older. I was looking at the Sashiko review for this patch. If Sashiko can be trusted, device_links seems to be thoroughly booby-trapped. Short summary seems to be: only safe way to use them is with the STATELESS flag and do manual cleanup.
On Fri, Jun 05, 2026 at 09:55:07AM +0100, Richard Fitzgerald wrote: > On 01/06/2026 3:28 pm, Richard Fitzgerald wrote: > > I think it would be ok to put this into -next and let it get some wider > > testing before it goes into 7.1 or older. > I was looking at the Sashiko review for this patch. > If Sashiko can be trusted, device_links seems to be thoroughly > booby-trapped. > Short summary seems to be: only safe way to use them is with the > STATELESS flag and do manual cleanup. Yeah, I saw that but didn't put the time in to confirming how reliable the specific analysis is. The booby trapped bit seems about right though.
On Mon, Jun 01, 2026 at 03:28:35PM +0100, Richard Fitzgerald wrote:
> + for_each_card_components(card, component) {
> + if (!device_link_add(card->dev, component->dev,
> + DL_FLAG_AUTOREMOVE_CONSUMER)) {
> + dev_warn(card->dev, "Failed to create device link to %s\n",
> + dev_name(component->dev));
> + ret = -EINVAL;
> + goto probe_end;
Do we still have devices registering separate components with the same
device for DMA and DAI operations? They'd end up duplicating the links
here.
On 02/06/2026 2:31 pm, Mark Brown wrote:
> On Mon, Jun 01, 2026 at 03:28:35PM +0100, Richard Fitzgerald wrote:
>
>> + for_each_card_components(card, component) {
>> + if (!device_link_add(card->dev, component->dev,
>> + DL_FLAG_AUTOREMOVE_CONSUMER)) {
>> + dev_warn(card->dev, "Failed to create device link to %s\n",
>> + dev_name(component->dev));
>> + ret = -EINVAL;
>> + goto probe_end;
>
> Do we still have devices registering separate components with the same
> device for DMA and DAI operations? They'd end up duplicating the links
> here.
Ok, weird, but I agree it's possible for one driver to register multiple
components. I'll make a V2 that skips the device link if the component
dev is the same as the card dev.
On Tue, Jun 02, 2026 at 02:34:53PM +0100, Richard Fitzgerald wrote: > On 02/06/2026 2:31 pm, Mark Brown wrote: > > Do we still have devices registering separate components with the same > > device for DMA and DAI operations? They'd end up duplicating the links > > here. > Ok, weird, but I agree it's possible for one driver to register multiple > components. I'll make a V2 that skips the device link if the component > dev is the same as the card dev. That wouldn't avoid the issue I think? The card is a separate device, but the DMA and DAI are two devices registered separately. This used to be because we had completely separate driver types for DMA and DAI, prior to the component refactoring. A simple move to component would keep two separate components, this could be refactored into a single component but that'd need to actually happen and I'm not sure we did that for everything. I was wondering if you'd checked already, sounds like no?
On 02/06/2026 2:46 pm, Mark Brown wrote: > On Tue, Jun 02, 2026 at 02:34:53PM +0100, Richard Fitzgerald wrote: >> On 02/06/2026 2:31 pm, Mark Brown wrote: > >>> Do we still have devices registering separate components with the same >>> device for DMA and DAI operations? They'd end up duplicating the links >>> here. > >> Ok, weird, but I agree it's possible for one driver to register multiple >> components. I'll make a V2 that skips the device link if the component >> dev is the same as the card dev. > > That wouldn't avoid the issue I think? The card is a separate device, > but the DMA and DAI are two devices registered separately. This used to > be because we had completely separate driver types for DMA and DAI, > prior to the component refactoring. A simple move to component would > keep two separate components, this could be refactored into a single > component but that'd need to actually happen and I'm not sure we did > that for everything. I was wondering if you'd checked already, sounds > like no? Oh, you mean two components with the same dev, so that the device_link_add() gets called for the same (card,component) devices multiple times? I tested multiple calls for the same device pair, and it's ok. This also covers the case where ASoC core now creates this device_link and the codec driver also creates the same device_link between the card and itself (hdac_hdmi does this). From the kernel doc for device_link_add(): "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"
On Tue, Jun 02, 2026 at 02:55:09PM +0100, Richard Fitzgerald wrote: > Oh, you mean two components with the same dev, so that the > device_link_add() gets called for the same (card,component) devices > multiple times? > I tested multiple calls for the same device pair, and it's ok. > This also covers the case where ASoC core now creates this device_link > and the codec driver also creates the same device_link between the > card and itself (hdac_hdmi does this). > From the kernel doc for device_link_add(): > "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" Ah, good - I'd remembered there was an issue with duplicates there. I guess it's worth adding the defensiveness for card and component being the same in any case, just to be on the safe side. There were some SoCs (sunxi I think?) with integrated DACs that I can see doing something like that, can't remember if they actually do or not.
On Mon, Jun 01, 2026 at 03:28:35PM +0100, Richard Fitzgerald wrote: > 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. > > Signed-off-by: Richard Fitzgerald <rf@opensource.cirrus.com> > --- Reviewed-by: Charles Keepax <ckeepax@opensource.cirrus.com> Thanks, Charles
© 2016 - 2026 Red Hat, Inc.