diff mbox series

PM: wakeup: implement devm_device_init_wakeup() helper

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

Commit Message

Joe Hattori Dec. 13, 2024, 3:52 a.m. UTC
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(+)

Comments

Joe Hattori Dec. 14, 2024, 2:26 a.m. UTC | #1
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 mbox series

Patch

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 */