On 9/5/23 08:37, Pierre-Louis Bossart wrote:
On 9/1/23 08:44, Péter Ujfalusi wrote:
On 01/09/2023 15:15, Kai Vehmanen wrote:
Hi,
On Wed, 30 Aug 2023, Maarten Lankhorst wrote:
With the upcoming changes for i915/Xe driver relying on the -EPROBE_DEFER mechanism, we need to have a first pass of the probe which cannot be pushed to a workqueue. Introduce 2 new optional callbacks.
[...]
diff --git a/sound/soc/sof/core.c b/sound/soc/sof/core.c index 30db685cc5f4b..54c384a5d6140 100644 --- a/sound/soc/sof/core.c +++ b/sound/soc/sof/core.c @@ -327,8 +327,6 @@ static int sof_probe_continue(struct snd_sof_dev *sdev) dsp_err: snd_sof_remove(sdev); probe_err:
- sof_ops_free(sdev);
this seems a bit out-of-place in this patch. It seems a valid change, but not really related to this patch, right?
The ops needs to be preserved even if the wq fails since the patch wants to call snd_sof_remove_no_wq() unconditionally on remove.
We seem to have a related fix waiting to be sent to alsa-devel, by Peter: "ASoC: SOF: core: Only call sof_ops_free() on remove if the probe wa" https://github.com/thesofproject/linux/pull/4515
I guess we can revert that in sof-dev, if this is the preferred way?
... not yet in Mark's tree.
Otherwise patch looks good to me.
I would have not created the snd_sof_remove_no_wq() as it makes not much functional sense. It might be even better if the remove in the wq would do the hda_codec_i915_exit() as the module will remain in there until the user removes it.
I think find all this very confusing, because there is no workqueue used in the remove steps. The workqueue is only used ONCE during the probe.
Maybe we should just remove any references to workqueues, and have
probe_start (cannot run in a wq) probe (may run in a wq) remove (cannot run in a wq, needs to call cancel_work_sync() if the probe runs in a wq) remove_last (cannot run in a wq, releases all resources acquired in probe_start)
Or something similar that shows the symmetry between steps and when the wq is allowed.