On Fri, 24 Jan 2020 22:36:21 +0100, Pierre-Louis Bossart wrote: > > The initial intent of releasing resources in the .remove does not work > well with HDaudio codecs. If the probe_continue() fails in a work > queue, e.g. due to missing firmware or authentication issues, we don't > release any resources, and as a result the kernel oopses during > suspend operations. > > The suggested fix is to release all resources during errors in > probe_continue(), and use fw_state to track resource allocation > state, so that .remove does not attempt to release the same > hardware resources twice. PM operations are also modified so that > no action is done if DSP resources have been freed due to > an error at probe. > > Reported-by: Takashi Iwai <tiwai@xxxxxxx> > Co-developed-by: Kai Vehmanen <kai.vehmanen@xxxxxxxxxxxxxxx> > Signed-off-by: Kai Vehmanen <kai.vehmanen@xxxxxxxxxxxxxxx> > Bugzilla: http://bugzilla.suse.com/show_bug.cgi?id=1161246 > Signed-off-by: Pierre-Louis Bossart <pierre-louis.bossart@xxxxxxxxxxxxxxx> This deserves for Cc to stable, as the bug already hits on both 5.4 and 5.5 kernels. Reviewed-by: Takashi Iwai <tiwai@xxxxxxx> thanks, Takashi > --- > sound/soc/sof/core.c | 33 ++++++++++++--------------------- > sound/soc/sof/pm.c | 4 ++++ > 2 files changed, 16 insertions(+), 21 deletions(-) > > diff --git a/sound/soc/sof/core.c b/sound/soc/sof/core.c > index f517ab448a1d..34cefbaf2d2a 100644 > --- a/sound/soc/sof/core.c > +++ b/sound/soc/sof/core.c > @@ -244,7 +244,6 @@ static int sof_probe_continue(struct snd_sof_dev *sdev) > > return 0; > > -#if !IS_ENABLED(CONFIG_SND_SOC_SOF_PROBE_WORK_QUEUE) > fw_trace_err: > snd_sof_free_trace(sdev); > fw_run_err: > @@ -255,22 +254,10 @@ static int sof_probe_continue(struct snd_sof_dev *sdev) > snd_sof_free_debug(sdev); > dbg_err: > snd_sof_remove(sdev); > -#else > > - /* > - * when the probe_continue is handled in a work queue, the > - * probe does not fail so we don't release resources here. > - * They will be released with an explicit call to > - * snd_sof_device_remove() when the PCI/ACPI device is removed > - */ > - > -fw_trace_err: > -fw_run_err: > -fw_load_err: > -ipc_err: > -dbg_err: > - > -#endif > + /* all resources freed, update state to match */ > + sdev->fw_state = SOF_FW_BOOT_NOT_STARTED; > + sdev->first_boot = true; > > return ret; > } > @@ -353,10 +340,12 @@ int snd_sof_device_remove(struct device *dev) > if (IS_ENABLED(CONFIG_SND_SOC_SOF_PROBE_WORK_QUEUE)) > cancel_work_sync(&sdev->probe_work); > > - snd_sof_fw_unload(sdev); > - snd_sof_ipc_free(sdev); > - snd_sof_free_debug(sdev); > - snd_sof_free_trace(sdev); > + if (sdev->fw_state > SOF_FW_BOOT_NOT_STARTED) { > + snd_sof_fw_unload(sdev); > + snd_sof_ipc_free(sdev); > + snd_sof_free_debug(sdev); > + snd_sof_free_trace(sdev); > + } > > /* > * Unregister machine driver. This will unbind the snd_card which > @@ -364,13 +353,15 @@ int snd_sof_device_remove(struct device *dev) > * before freeing the snd_card. > */ > snd_sof_machine_unregister(sdev, pdata); > + > /* > * Unregistering the machine driver results in unloading the topology. > * Some widgets, ex: scheduler, attempt to power down the core they are > * scheduled on, when they are unloaded. Therefore, the DSP must be > * removed only after the topology has been unloaded. > */ > - snd_sof_remove(sdev); > + if (sdev->fw_state > SOF_FW_BOOT_NOT_STARTED) > + snd_sof_remove(sdev); > > /* release firmware */ > release_firmware(pdata->fw); > diff --git a/sound/soc/sof/pm.c b/sound/soc/sof/pm.c > index 84290bbeebdd..a0cde053b61a 100644 > --- a/sound/soc/sof/pm.c > +++ b/sound/soc/sof/pm.c > @@ -56,6 +56,10 @@ static int sof_resume(struct device *dev, bool runtime_resume) > if (!sof_ops(sdev)->resume || !sof_ops(sdev)->runtime_resume) > return 0; > > + /* DSP was never successfully started, nothing to resume */ > + if (sdev->first_boot) > + return 0; > + > /* > * if the runtime_resume flag is set, call the runtime_resume routine > * or else call the system resume routine > -- > 2.20.1 > _______________________________________________ Alsa-devel mailing list Alsa-devel@xxxxxxxxxxxxxxxx https://mailman.alsa-project.org/mailman/listinfo/alsa-devel
