@@ -495,13 +645,27 @@ int hda_dsp_resume(struct snd_sof_dev *sdev) } /* init hda controller. DSP cores will be powered up during fw boot */ - return hda_resume(sdev, false); + ret = hda_resume(sdev, false); + if (ret < 0) + return ret; + + hda_dsp_set_power_state(sdev, &target_state);Return value of hda_dsp_set_power_state() seems to be checked or directly returned in other functions, any reason to not do it here?
This seems like a miss. We should have returned the value of hda_set_power_state() directly here. And to address your point, Pierre. This is the platform-specific code, so the use of hda_dsp_set_power_state() should be permitted no? Whereas, the part that uses this function in ipc.c is not platform-specific.
Well, what do we mean by 'platform-specific'? Here we have two different architectures (APL and CNL) and we'll likely see more. We can also consider that all this code is a common library for all HDaudio platforms. I just wanted to make sure we don't take shortcuts.
At any rate, the return value needs to be fixed, we can discuss further on the HDaudio code partition in future evolutions. I am not 100% happy with which file deals with what, things are a bit convoluted between hda.c hda-ctrl.c hda-dsp.c hda-pcm.c, etc.
Thanks -Pierre _______________________________________________ Alsa-devel mailing list Alsa-devel@xxxxxxxxxxxxxxxx https://mailman.alsa-project.org/mailman/listinfo/alsa-devel
