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

[Richard Fitzgerald]

On 18/6/26 14:22, Marek Szyprowski wrote:
> On 18.06.2026 15:12, Richard Fitzgerald wrote:
>> On 18/6/26 13:58, Marek Szyprowski wrote:
>>> On 18.06.2026 13:22, Richard Fitzgerald wrote:
>>>> On 17/6/26 15:10, Marek Szyprowski wrote:
>>>>> On 11.06.2026 13:08, 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.
>>>>>>
>>>>>> 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 at opensource.cirrus.com>
>>>>>> ---
>>>>>
>>>>>
>>>>> This patch landed recently in linux-next as commit 0f54ce994b23 ("ASoC:
>>>>> soc-core: Create device_link to ensure correct suspend order"). In my
>>>>> tests I found that it breaks probing of VC4 DRM subsystem on Raspberry Pi
>>>>> 3 and 4 boards due to an issue with hdmi-audio-codec:
>>>>>
>>>>> # dmesg | grep vc4
>>>>> vc4-drm gpu: bound fe400000.hvs (ops vc4_hvs_ops [vc4])
>>>>> vc4_hdmi fef00700.hdmi: Failed to create device link to hdmi-audio-codec.1.auto
>>>>> vc4_hdmi fef00700.hdmi: error -EINVAL: Could not register sound card
>>>>> vc4-drm gpu: failed to bind fef00700.hdmi (ops vc4_hdmi_ops [vc4]): -22
>>>>> vc4-drm gpu: adev bind failed: -22
>>>>> vc4-drm gpu: probe with driver vc4-drm failed with error -22
>>>>>
>>>>
>>>> Where in device_link_add() does it fail?
>>>>
>>>
>>> It fails the following check at drivers/base/core.c line 766:
>>>
>>>          if (!device_pm_initialized(supplier)
>>>               || (!(flags & DL_FLAG_SYNC_STATE_ONLY) &&
>>>                     device_is_dependent(consumer, supplier))) {
>>>                   link = NULL;
>>>                   goto out;
>>>           }
>>>
>>> because device_is_dependent(consumer, supplier) is true for fef00700.hdmi and
>>> hdmi-audio-codec.1.auto.
>>>
>>>
>>> Best regards
>>
>> Is the machine driver the parent of the codec driver?
> 
> 
> Yes, so this change fixes the issue:

Right. I thought I'd tested that. But I haven't got any hardware that
uses that "for real", so obviously I messed up my testing there.

It makes this whole device_link stuff less useful if you can't use it to
reorder a parent-child suspend order for when the parent-child
relationship isn't relevant to the order they should suspend.

If we skip entries that we can't re-order, is that simply adding
confusing complications and _mostly_ ASoC forces the correct order, but
in some cases it cannot and then it is up to those affected drivers to
figure it out themselves? Can we always guarantee that those drivers
will know which they are, or can it all break subtly because a driver
doesn't know that in some systems it becomes related to some other
driver?

Mark - should we revert this because of too many corner cases in the
device_link code and go back to every codec driver with a system_suspend
installing its own device_link to ensure correct suspend order compared
to the machine driver?