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@xxxxxxxxx>
Reviewed-by: Amadeusz Sławiński <amadeuszx.slawinski@xxxxxxxxxxxxxxx>
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
