[PATCH v2 16/17] ASoC: Intel: bdw_rt286: Refactor suspend/resume
Cezary Rojewski
cezary.rojewski at intel.com
Wed Jun 15 14:57:41 CEST 2022
On 2022-06-15 3:27 AM, Pierre-Louis Bossart wrote:
> On 6/13/22 04:15, Cezary Rojewski wrote:
>> Make use of card->remove() rather than pdev->remove() to unassign jack
>> during card unbind procedure.
>>
>> To reduce code size, define unified jack setter in form of
>> bdw_rt286_set_jack() and invoke it during remove(), suspend_pre() and
>> resume_port().
>>
>> Signed-off-by: Cezary Rojewski <cezary.rojewski at intel.com>
>> Reviewed-by: Amadeusz Sławiński <amadeuszx.slawinski at linux.intel.com>
>
> This seems to have rather negative side effects in our modprobe/modprobe
> -r tests?
>
> The pattern of disabling the jack in the platform device .remove is
> fairly common, I don't recall having seen a machine driver doing this in
> the card .remove step. Are you sure this is equivalent?
>
> Reverting this patch removes the kernel oops.
>
> I don't have time to debug further - but this adds to my point of
> minimizing risk on legacy code, doesn't it? suspend-resume is difficult
> to get right, and easy to break. I have done the latter more often that
> the former.
>
> if you want to reproduce the issue, see
> https://github.com/thesofproject/linux/pull/3696
>
> and use sof-test:
> /root/sof-test/test-case/check-kmod-load-unload.sh -l 1
Thanks for the report. Indeed, the latest "optimization" broke the
card->remove() path.
Jacks are often initialized during dai_link initialization which is
completely out of platform_device area. This report made me think
further - if we assign jack in dai_link->init(), we should be able to
drop it in dai_link->exit().
Not exactly! ->init() is done once card components are already accounted
for (available for use) but snd_soc_link_exit() is called during
snd_soc_remove_pcm_runtime() when card components are available no
longer - soc_remove_link_components().
TLDR: teardown path is not symmetric with its counterpart, perhaps a
problem yet to be addressed. I'll see if moving the jack-NULLing to
codec's DAI ->remove() won't be a better temporary (?) solution than
reverting to platform_device->remove() usage.
Mark,
Is it fine to leave v2 series as is, just ignoring this single 16/17
patch? Or should I resend entire series as v3 without this very patch?
I'd like to address the problem via a separate change.
Regards,
Czarek
More information about the Alsa-devel
mailing list