linux-kernel - Re: [PATCH] ASoC: wm8962: Force suspend/resume during system suspend/resume

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Message-ID: <20200424104347.GJ44490@ediswmail.ad.cirrus.com>
Date:   Fri, 24 Apr 2020 10:43:47 +0000
From:   Charles Keepax <ckeepax@...nsource.cirrus.com>
To:     Shengjiu Wang <shengjiu.wang@....com>
CC:     <lgirdwood@...il.com>, <broonie@...nel.org>, <perex@...ex.cz>,
        <tiwai@...e.com>, <tglx@...utronix.de>, <allison@...utok.net>,
        <info@...ux.net>, <patches@...nsource.cirrus.com>,
        <alsa-devel@...a-project.org>, <linux-kernel@...r.kernel.org>
Subject: Re: [PATCH] ASoC: wm8962: Force suspend/resume during system
 suspend/resume

On Fri, Apr 24, 2020 at 02:12:07PM +0800, Shengjiu Wang wrote:
> Use force_suspend/resume to make sure clocks are disabled/enabled
> accordingly during system suspend/resume.
> 
> Signed-off-by: Shengjiu Wang <shengjiu.wang@....com>
> ---
>  sound/soc/codecs/wm8962.c | 2 ++
>  1 file changed, 2 insertions(+)
> 
> diff --git a/sound/soc/codecs/wm8962.c b/sound/soc/codecs/wm8962.c
> index 0a2cfff44441..80332c147da9 100644
> --- a/sound/soc/codecs/wm8962.c
> +++ b/sound/soc/codecs/wm8962.c
> @@ -3854,6 +3854,8 @@ static int wm8962_runtime_suspend(struct device *dev)
>  
>  static const struct dev_pm_ops wm8962_pm = {
>  	SET_RUNTIME_PM_OPS(wm8962_runtime_suspend, wm8962_runtime_resume, NULL)
> +	SET_SYSTEM_SLEEP_PM_OPS(pm_runtime_force_suspend,
> +				pm_runtime_force_resume)

I would be interested in a little more explaination of why this is
the correct fix here?

My concern is this presumably precludes the CODEC being used in any
application where it is expected to process audio during system
suspend. Admittedly, the target market for this device probably
makes that unlikely, but I am slightly uncomfortable just assuming
that will always be true.

As far as I can see the expectation is that DAPM shutdowns down
all audio streams during system suspend (except those marked to
ignore suspend, as per my above concerns) and that will cause
a pm_runtime_put on each DAPM context which I would imagine is
intended to put the CODEC into runtime suspend. I am guessing the
reason you have problems is because of the driver doing its own
pm_runtime operations in set_fll, which probably takes an extra
reference preventing the power down. Is a more complex fix
correcting the interaction with DAPM the correct solution here?

Thanks,
Charles