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

[Dhruva Gole]

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@xxxxxxxxxxxx>
> ---

$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@xxxxxx>

--
Thanks and Regards,
Dhruva Gole