Messages in this thread | | | From | "Rafael J. Wysocki" <> | Date | Thu, 29 Feb 2024 20:31:20 +0100 | Subject | Re: [PATCH] PM: wakeirq: fix wake irq warning in system suspend stage |
| |
On Wed, Feb 28, 2024 at 9:33 AM Qingliang Li (黎晴亮) <Qingliang.Li@mediatek.com> wrote: > > On Wed, 2024-02-28 at 11:34 +0530, Dhruva Gole wrote: > > > > External email : Please do not click links or open attachments until > > you have verified the sender or the content. > > 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? > > You are right, the involvement of 'dev_pm_enable_wake_irq_complete' is > due to the driver selecting 'pm_runtime_force_suspend' as the callback > function for system suspend. In this scenario, the call sequence during > system suspend is as follows: > dpm_suspend_start -> dpm_run_callback -> pm_runtime_force_suspend -> > dev_pm_enable_wake_irq_check/complete > suspend_enter -> dpm_suspend_noirq -> dev_pm_arm_wake_irq > > Based on the above, if the driver (i) chooses pm_runtime_force_suspend > as the system suspend callback function and (ii) registers wake irq > with reverse enable ordering, the wake irq will be enabled twice during > system suspend.
It would be good to put the above information into the patch changelog, as it actually explains the problem.
Thanks!
| |