Message ID | 20241213035235.2479642-1-joe@pf.is.s.u-tokyo.ac.jp |
---|---|
State | New |
Headers | show |
Series | PM: wakeup: implement devm_device_init_wakeup() helper | expand |
Hi Dhruva, Thank you for your review. On 12/13/24 18:26, Dhruva Gole wrote: > On Dec 13, 2024 at 12:52:35 +0900, Joe Hattori wrote: >> Some drivers that enable device wakeup fail to properly disable it >> during their cleanup, which results in a memory leak. >> >> To address this, introduce devm_device_init_wakeup(), a managed variant >> of device_init_wakeup(dev, true). With this managed helper, wakeup >> functionality will be automatically disabled when the device is >> released, ensuring a more reliable cleanup process. >> >> This need for this addition arose during a previous discussion [1]. >> >> [1]: >> https://lore.kernel.org/linux-rtc/20241212100403.3799667-1-joe@pf.is.s.u-tokyo.ac.jp/ > > CC Alexandre who I see is an important part of this thread. Yes, just sent a v2 patch with Alexandre in CC and in the "Suggested-by" tag. > >> >> Signed-off-by: Joe Hattori <joe@pf.is.s.u-tokyo.ac.jp> >> --- >> include/linux/pm_wakeup.h | 27 +++++++++++++++++++++++++++ >> 1 file changed, 27 insertions(+) >> >> diff --git a/include/linux/pm_wakeup.h b/include/linux/pm_wakeup.h >> index 222f7530806c..baf4f982858a 100644 >> --- a/include/linux/pm_wakeup.h >> +++ b/include/linux/pm_wakeup.h >> @@ -240,4 +240,31 @@ static inline int device_init_wakeup(struct device *dev, bool enable) >> return 0; >> } >> >> +static void devm_device_disable_wakeup(void *data) >> +{ >> + struct device *dev = data; >> + >> + device_wakeup_disable(dev); >> + device_set_wakeup_capable(dev, false); >> +} >> + >> +/** >> + * devm_device_init_wakeup - Resource managed device wakeup initialization. >> + * @dev: Device to handle. >> + * >> + * This function is the devm managed version of device_init_wakeup(dev, true). >> + */ >> +static inline int devm_device_init_wakeup(struct device *dev) >> +{ >> + int err; >> + >> + device_set_wakeup_capable(dev, true); >> + err = device_wakeup_enable(dev); >> + if (err) { >> + device_set_wakeup_capable(dev, false); >> + return err; >> + } >> + return devm_add_action_or_reset(dev, devm_device_disable_wakeup, dev); > > Why not just call in device_init_wakeup which already does all this for > you? Makes sense, applied in the v2 patch. > > > Then the devm_disable will just be device_init_wakeup(dev, false); > > > See for eg. how runtime pm does it: > > int devm_pm_runtime_enable(struct device *dev) > { > pm_runtime_enable(dev); > Best, Joe
diff --git a/include/linux/pm_wakeup.h b/include/linux/pm_wakeup.h index 222f7530806c..baf4f982858a 100644 --- a/include/linux/pm_wakeup.h +++ b/include/linux/pm_wakeup.h @@ -240,4 +240,31 @@ static inline int device_init_wakeup(struct device *dev, bool enable) return 0; } +static void devm_device_disable_wakeup(void *data) +{ + struct device *dev = data; + + device_wakeup_disable(dev); + device_set_wakeup_capable(dev, false); +} + +/** + * devm_device_init_wakeup - Resource managed device wakeup initialization. + * @dev: Device to handle. + * + * This function is the devm managed version of device_init_wakeup(dev, true). + */ +static inline int devm_device_init_wakeup(struct device *dev) +{ + int err; + + device_set_wakeup_capable(dev, true); + err = device_wakeup_enable(dev); + if (err) { + device_set_wakeup_capable(dev, false); + return err; + } + return devm_add_action_or_reset(dev, devm_device_disable_wakeup, dev); +} + #endif /* _LINUX_PM_WAKEUP_H */
Some drivers that enable device wakeup fail to properly disable it during their cleanup, which results in a memory leak. To address this, introduce devm_device_init_wakeup(), a managed variant of device_init_wakeup(dev, true). With this managed helper, wakeup functionality will be automatically disabled when the device is released, ensuring a more reliable cleanup process. This need for this addition arose during a previous discussion [1]. [1]: https://lore.kernel.org/linux-rtc/20241212100403.3799667-1-joe@pf.is.s.u-tokyo.ac.jp/ Signed-off-by: Joe Hattori <joe@pf.is.s.u-tokyo.ac.jp> --- include/linux/pm_wakeup.h | 27 +++++++++++++++++++++++++++ 1 file changed, 27 insertions(+)