linux-kernel - Re: [PATCH] PM: wakeirq: fix wake irq warning in system suspend stage

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: <1390e743-2216-4435-b2ef-7d92a55605b1@ti.com>
Date: Wed, 28 Feb 2024 11:34:23 +0530
From: Dhruva Gole <d-gole@...com>
To: Qingliang Li <qingliang.li@...iatek.com>,
        "Rafael J . Wysocki"
	<rafael@...nel.org>, Pavel Machek <pavel@....cz>,
        Len Brown
	<len.brown@...el.com>,
        Greg Kroah-Hartman <gregkh@...uxfoundation.org>,
        Matthias Brugger <matthias.bgg@...il.com>,
        AngeloGioacchino Del Regno
	<angelogioacchino.delregno@...labora.com>,
        Johan Hovold
	<johan+linaro@...nel.org>,
        Tony Lindgren <tony@...mide.com>, Dhruva Gole
	<d-gole@...com>
CC: <linux-pm@...r.kernel.org>, <linux-kernel@...r.kernel.org>,
        <linux-arm-kernel@...ts.infradead.org>,
        <linux-mediatek@...ts.infradead.org>
Subject: Re: [PATCH] PM: wakeirq: fix wake irq warning in system suspend stage

Hi,

On 28/02/24 07:30, Qingliang Li wrote:
> When driver registers the wake irq with reverse enable ordering,
> the wake irq will be re-enabled when entering system suspend, triggering
> an 'Unbalanced enable for IRQ xxx' warning. The wake irq will be
> enabled in both dev_pm_enable_wake_irq_complete() and dev_pm_arm_wake_irq()
> 
> To fix this issue, complete the setting of WAKE_IRQ_DEDICATED_ENABLED flag
> in dev_pm_enable_wake_irq_complete() to avoid redundant irq enablement.


Just trying to understand, why not in dev_pm_arm_wake_irq ?
Is it cuz it's called much after dev_pm_enable_wake_irq_complete ?
Not sure what's the exact call order, but I am assuming
dev_pm_enable_wake_irq_complete is more of a runtime thing and
dev_pm_arm_wake_irq happens finally at system suspend?

> 
> Fixes: 8527beb12087 ("PM: sleep: wakeirq: fix wake irq arming")
> Signed-off-by: Qingliang Li <qingliang.li@...iatek.com>
> ---

$subject: Most recent convention used for this file is:
"PM: sleep: wakeirq:  ..."

>   drivers/base/power/wakeirq.c | 4 +++-
>   1 file changed, 3 insertions(+), 1 deletion(-)
> 
> diff --git a/drivers/base/power/wakeirq.c b/drivers/base/power/wakeirq.c
> index 42171f766dcb..5a5a9e978e85 100644
> --- a/drivers/base/power/wakeirq.c
> +++ b/drivers/base/power/wakeirq.c
> @@ -313,8 +313,10 @@ void dev_pm_enable_wake_irq_complete(struct device *dev)
>   		return;
>   
>   	if (wirq->status & WAKE_IRQ_DEDICATED_MANAGED &&
> -	    wirq->status & WAKE_IRQ_DEDICATED_REVERSE)
> +	    wirq->status & WAKE_IRQ_DEDICATED_REVERSE) {
>   		enable_irq(wirq->irq);
> +		wirq->status |= WAKE_IRQ_DEDICATED_ENABLED;
> +	}

But this does make sense to make sure status is updated,
You can pick my R-by.

Reviewed-by: Dhruva Gole <d-gole@...com>

-- 
Thanks and Regards,
Dhruva Gole