| Message ID | 20210817190057.255264-3-pierre-louis.bossart@linux.intel.com |
|---|---|
| State | New |
| Headers | show |
| Series | driver core: kick deferred probe from delayed context | expand |
On Tue, Aug 17, 2021 at 02:00:57PM -0500, Pierre-Louis Bossart wrote: > +++ b/sound/soc/sof/core.c > @@ -251,6 +251,9 @@ static int sof_probe_continue(struct snd_sof_dev *sdev) > > sdev->probe_completed = true; > > + /* kick-off re-probing of deferred devices */ > + driver_deferred_probe_trigger(); > + I think we should move this into snd_soc_register_component() - the same issue could occur with any other component, the only other thing I can see kicking in here is the machine driver registration but that ought to kick probe itself anyway. Or is there some other case here?
On 8/18/21 7:07 AM, Mark Brown wrote: > On Tue, Aug 17, 2021 at 02:00:57PM -0500, Pierre-Louis Bossart wrote: > >> +++ b/sound/soc/sof/core.c >> @@ -251,6 +251,9 @@ static int sof_probe_continue(struct snd_sof_dev *sdev) >> >> sdev->probe_completed = true; >> >> + /* kick-off re-probing of deferred devices */ >> + driver_deferred_probe_trigger(); >> + > > I think we should move this into snd_soc_register_component() - the same > issue could occur with any other component, the only other thing I can > see kicking in here is the machine driver registration but that ought to > kick probe itself anyway. Or is there some other case here? Thanks for the suggestion Mark, it would be more consistent indeed to kick a re-evaluation of the deferred probe list when ASoC components are successfully registered with something like this: diff --git a/sound/soc/soc-core.c b/sound/soc/soc-core.c index c830e96afba2..9d6feea7719c 100644 --- a/sound/soc/soc-core.c +++ b/sound/soc/soc-core.c @@ -2677,7 +2677,14 @@ int snd_soc_register_component(struct device *dev, if (ret < 0) return ret; - return snd_soc_add_component(component, dai_drv, num_dai); + ret = snd_soc_add_component(component, dai_drv, num_dai); + if (ret < 0) + return ret; + + /* kick-off re-probing of deferred devices */ + driver_deferred_probe_trigger(); + + return 0; } EXPORT_SYMBOL_GPL(snd_soc_register_component); In the case of this SOF driver, it'd be completely equivalent to what this patch suggested, the snd_soc_register_component() is what we do last in the workqueue. In the case of 'regular' drivers, the component registration is typically done last as well before the end of the probe. This would result in 2 evaluations (one on successful ASoC component registration and one on successful probe), and maybe on the second evaluation there's nothing to do. I can't think of any negative side-effects.
diff --git a/sound/soc/sof/core.c b/sound/soc/sof/core.c index 3e4dd4a86363..cecc0e914807 100644 --- a/sound/soc/sof/core.c +++ b/sound/soc/sof/core.c @@ -251,6 +251,9 @@ static int sof_probe_continue(struct snd_sof_dev *sdev) sdev->probe_completed = true; + /* kick-off re-probing of deferred devices */ + driver_deferred_probe_trigger(); + return 0; fw_trace_err:
Audio drivers such as HDaudio legacy and SOF rely on a workqueue to split the probe into two, with a first pass returning success immediately, and the second pass taking a lot more time due to the use of request_module() and the DSP initializations. This workqueue-based solution helps deal with conflicting requirements a) other drivers should not be blocked by a long probe b) a PROBE_PREFER_ASYNCHRONOUS probe_type is explicitly not allowed to avoid a deadlock when request_module() is used. This patch makes sure the deferred probe framework is triggered when the provider of resources successfully completes its initialization. Signed-off-by: Pierre-Louis Bossart <pierre-louis.bossart@linux.intel.com> --- sound/soc/sof/core.c | 3 +++ 1 file changed, 3 insertions(+)