[alsa-devel] [PATCH 5/9] ASoC: SOF: Move DSP power state transitions to platform-specific ops

[Pierre-Louis Bossart]

>>>> @@ -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