>>> 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? >> 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.