On Thu, 01 Dec 2022 14:22:12 +0100,
Oliver Neukum wrote:

On 01.12.22 14:03, Ricardo Ribalda wrote:

Hi,
 

quoted

This patchset does not modify this behaviour. It simply fixes the
stall for kexec().

The  patch that introduced the stall:
83bfc7e793b5 ("ASoC: SOF: core: unregister clients and machine drivers
in .shutdown")

That patch is problematic. I would go as far as saying that
it needs to be reverted.

... or fixed.

quoted

was sent as a generalised version of:
https://github.com/thesofproject/linux/pull/3388

AFAIK, we would need a similar patch for every single board.... which
I am not sure it is doable in a reasonable timeframe.

On the meantime this seems like a decent compromises. Yes, a
miss-behaving userspace can still stall during suspend, but that was
not introduced in this patch.

Well, I mean if you know what wrong then I'd say at least return to
a sanely broken state.

The whole approach is wrong. You need to be able to deal with user
space talking to removed devices by returning an error and keeping
the resources association with the open file allocated until
user space calls close()

As I already mentioned in another thread, if the user-space action has
to be cut off, we just need to call snd_card_disconnect() instead
without sync.  A quick hack would be like below (totally untested and
might be wrong, though).

In anyway, Ricardo, please stop spinning too frequently; v8 in a few 
days is way too much, and now the recipient list became unmanageable.
Let's give people some time to review and consider a better solution
at first.


thanks,

Takashi

-- 8< --

--- a/sound/soc/sof/core.c
+++ b/sound/soc/sof/core.c

@@ -475,7 +475,7 @@ EXPORT_SYMBOL(snd_sof_device_remove);
 int snd_sof_device_shutdown(struct device *dev)
 {
 	struct snd_sof_dev *sdev = dev_get_drvdata(dev);
-	struct snd_sof_pdata *pdata = sdev->pdata;
+	struct snd_soc_component *component;
 
 	if (IS_ENABLED(CONFIG_SND_SOC_SOF_PROBE_WORK_QUEUE))
 		cancel_work_sync(&sdev->probe_work);

@@ -484,9 +484,9 @@ int snd_sof_device_shutdown(struct device *dev)
 	 * make sure clients and machine driver(s) are unregistered to force
 	 * all userspace devices to be closed prior to the DSP shutdown sequence
 	 */
-	sof_unregister_clients(sdev);
-
-	snd_sof_machine_unregister(sdev, pdata);
+	component = snd_soc_lookup_component(sdev->dev, NULL);
+	if (component && component->card && component->card->snd_card)
+		snd_card_disconnect(component->card->snd_card);
 
 	if (sdev->fw_state == SOF_FW_BOOT_COMPLETE)
 		return snd_sof_shutdown(sdev);