Re: [PATCH V6 8/8] soundwire: amd: add pm_prepare callback and pm ops support

[Mukunda,Vijendar]

On 08/03/23 19:53, Pierre-Louis Bossart wrote:
>>>> device_for_each_child() will invoke amd_resume_child_device() function callback
>>>> for each device which will try to resume the child device in this case.
>>>> By definition, device_for_each_child() Iterate over @parent's child devices,
>>>> and invokes the callback for each. We check the return of amd_resume_child_device()
>>>> each time.
>>>> If it returns anything other than 0, we break out and return that value.
>>>>
>>>> In current scenario, As AMP codec is not in runtime suspend state,
>>>> pm_request_resume() will return a value as 1. This will break the
>>>> sequence for resuming rest of the child devices(JACK codec in our case).
>>> Well, yes, now that makes sense, thanks for the details.
>>>
>>> I think the reason why we didn't see the problem with the Intel code is
>>> that both amplifiers are on the same dailink, so they are by
>>> construction either both suspended or both active. We never had
>>> different types of devices on the same link.
>>>
>>> I would however suggest this simpler alternative, where only negative
>>> return values are returned:
>>>
>>> ret = pm_request_resume(dev);
>>> if (ret < 0) {
>>> 	dev_err(dev, "pm_request_resume failed: %d\n", ret);
>>>         return ret;
>>> }
>>> return 0;
>>>
>>> this would work just fine, no?
Sorry its my bad. This would work fine.
We will fix it and respin the patch series.
>>> No, As explained, pm_request_resume() return value is 1 for active device.
>>>> As mentioned in an earlier thread, there are two possible solutions.
>>>> 1. check pm runtime suspend state and return 0 if it is not suspended
>>>> 2. simply always return 0 for amd_resume_child_device() function callback.
>>>>
>>>> We opted first one as solution.
>>> My suggestion looks like your option 2. It's cleaner IMHO.
>> To use option 2, we need to respin the patch series.
>> Is it okay if we fix it as supplement patch?
> I would vote for re-spinning a new version and ask others to review.