linux-kernel.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
* [RFC RESEND PATCH v2 1/8] workqueue: Add resource managed version of delayed work init
  2021-03-22  7:41 [RFC RESEND PATCH v2 0/8] Add managed version of delayed work init Matti Vaittinen
@ 2021-03-22  7:36 ` Matti Vaittinen
  2021-03-22  7:36 ` [RFC RESEND PATCH v2 2/8] MAINTAINERS: Add entry for devm helpers Matti Vaittinen
                   ` (7 subsequent siblings)
  8 siblings, 0 replies; 14+ messages in thread
From: Matti Vaittinen @ 2021-03-22  7:36 UTC (permalink / raw)
  To: Matti Vaittinen, Matti Vaittinen
  Cc: MyungJoo Ham, Chanwoo Choi, Andy Gross, Bjorn Andersson,
	Jean Delvare, Guenter Roeck, Hans de Goede, Mark Gross,
	Sebastian Reichel, Chen-Yu Tsai, Liam Girdwood, Mark Brown,
	Wim Van Sebroeck, Matti Vaittinen, gregkh, linux-kernel,
	linux-arm-msm, linux-hwmon, platform-driver-x86, linux-pm,
	linux-watchdog

A few drivers which need a delayed work-queue must cancel work at driver
detach. Some of those implement remove() solely for this purpose. Help
drivers to avoid unnecessary remove and error-branch implementation by
adding managed verision of delayed work initialization. This will also
help drivers to avoid mixing manual and devm based unwinding when other
resources are handled by devm.

Signed-off-by: Matti Vaittinen <matti.vaittinen@fi.rohmeurope.com>
---
 include/linux/devm-helpers.h | 53 ++++++++++++++++++++++++++++++++++++
 1 file changed, 53 insertions(+)
 create mode 100644 include/linux/devm-helpers.h

diff --git a/include/linux/devm-helpers.h b/include/linux/devm-helpers.h
new file mode 100644
index 000000000000..f64e0c9f3763
--- /dev/null
+++ b/include/linux/devm-helpers.h
@@ -0,0 +1,53 @@
+/* SPDX-License-Identifier: GPL-2.0-only */
+#ifndef __LINUX_DEVM_HELPERS_H
+#define __LINUX_DEVM_HELPERS_H
+
+/*
+ * Functions which do automatically cancel operations or release resources upon
+ * driver detach.
+ *
+ * These should be helpful to avoid mixing the manual and devm-based resource
+ * management which can be source of annoying, rarely occurring,
+ * hard-to-reproduce bugs.
+ *
+ * Please take into account that devm based cancellation may be performed some
+ * time after the remove() is ran.
+ *
+ * Thus mixing devm and manual resource management can easily cause problems
+ * when unwinding operations with dependencies. IRQ scheduling a work in a queue
+ * is typical example where IRQs are often devm-managed and WQs are manually
+ * cleaned at remove(). If IRQs are not manually freed at remove() (and this is
+ * often the case when we use devm for IRQs) we have a period of time after
+ * remove() - and before devm managed IRQs are freed - where new IRQ may fire
+ * and schedule a work item which won't be cancelled because remove() was
+ * already ran.
+ */
+
+#include <linux/device.h>
+#include <linux/workqueue.h>
+
+static inline void devm_delayed_work_drop(void *res)
+{
+	cancel_delayed_work_sync(res);
+}
+
+/**
+ * devm_delayed_work_autocancel - Resource-managed work allocation
+ * @dev: Device which lifetime work is bound to
+ * @pdata: work to be cancelled when driver is detached
+ *
+ * Initialize work which is automatically cancelled when driver is detached.
+ * A few drivers need delayed work which must be cancelled before driver
+ * is detached to avoid accessing removed resources.
+ * devm_delayed_work_autocancel() can be used to omit the explicit
+ * cancelleation when driver is detached.
+ */
+static inline int devm_delayed_work_autocancel(struct device *dev,
+					       struct delayed_work *w,
+					       work_func_t worker)
+{
+	INIT_DELAYED_WORK(w, worker);
+	return devm_add_action(dev, devm_delayed_work_drop, w);
+}
+
+#endif
-- 
2.25.4


-- 
Matti Vaittinen, Linux device drivers
ROHM Semiconductors, Finland SWDC
Kiviharjunlenkki 1E
90220 OULU
FINLAND

~~~ "I don't think so," said Rene Descartes. Just then he vanished ~~~
Simon says - in Latin please.
~~~ "non cogito me" dixit Rene Descarte, deinde evanescavit ~~~
Thanks to Simon Glass for the translation =] 

^ permalink raw reply related	[flat|nested] 14+ messages in thread

* [RFC RESEND PATCH v2 2/8] MAINTAINERS: Add entry for devm helpers
  2021-03-22  7:41 [RFC RESEND PATCH v2 0/8] Add managed version of delayed work init Matti Vaittinen
  2021-03-22  7:36 ` [RFC RESEND PATCH v2 1/8] workqueue: Add resource " Matti Vaittinen
@ 2021-03-22  7:36 ` Matti Vaittinen
  2021-03-22  7:37 ` [RFC RESEND PATCH v2 3/8] extconn: Clean-up few drivers by using managed work init Matti Vaittinen
                   ` (6 subsequent siblings)
  8 siblings, 0 replies; 14+ messages in thread
From: Matti Vaittinen @ 2021-03-22  7:36 UTC (permalink / raw)
  To: Matti Vaittinen, Matti Vaittinen
  Cc: MyungJoo Ham, Chanwoo Choi, Andy Gross, Bjorn Andersson,
	Jean Delvare, Guenter Roeck, Hans de Goede, Mark Gross,
	Sebastian Reichel, Chen-Yu Tsai, Liam Girdwood, Mark Brown,
	Wim Van Sebroeck, Matti Vaittinen, gregkh, linux-kernel,
	linux-arm-msm, linux-hwmon, platform-driver-x86, linux-pm,
	linux-watchdog

Devm helper header containing small inline helpers was added.
Hans promised to maintain it.

Add Hans as maintainer and myself as designated reviewer.

Signed-off-by: Matti Vaittinen <matti.vaittinen@fi.rohmeurope.com>
---
 MAINTAINERS | 6 ++++++
 1 file changed, 6 insertions(+)

diff --git a/MAINTAINERS b/MAINTAINERS
index 9e876927c60d..fa5ac3164678 100644
--- a/MAINTAINERS
+++ b/MAINTAINERS
@@ -5169,6 +5169,12 @@ M:	Torben Mathiasen <device@lanana.org>
 S:	Maintained
 W:	http://lanana.org/docs/device-list/index.html
 
+DEVICE RESOURCE MANAGEMENT HELPERS
+M:	Hans de Goede <hdegoede@redhat.com>
+R:	Matti Vaittinen <matti.vaittinen@fi.rohmeurope.com>
+S:	Maintained
+F:	include/linux/devm-helpers.h
+
 DEVICE-MAPPER  (LVM)
 M:	Alasdair Kergon <agk@redhat.com>
 M:	Mike Snitzer <snitzer@redhat.com>
-- 
2.25.4


-- 
Matti Vaittinen, Linux device drivers
ROHM Semiconductors, Finland SWDC
Kiviharjunlenkki 1E
90220 OULU
FINLAND

~~~ "I don't think so," said Rene Descartes. Just then he vanished ~~~
Simon says - in Latin please.
~~~ "non cogito me" dixit Rene Descarte, deinde evanescavit ~~~
Thanks to Simon Glass for the translation =] 

^ permalink raw reply related	[flat|nested] 14+ messages in thread

* [RFC RESEND PATCH v2 3/8] extconn: Clean-up few drivers by using managed work init
  2021-03-22  7:41 [RFC RESEND PATCH v2 0/8] Add managed version of delayed work init Matti Vaittinen
  2021-03-22  7:36 ` [RFC RESEND PATCH v2 1/8] workqueue: Add resource " Matti Vaittinen
  2021-03-22  7:36 ` [RFC RESEND PATCH v2 2/8] MAINTAINERS: Add entry for devm helpers Matti Vaittinen
@ 2021-03-22  7:37 ` Matti Vaittinen
  2021-03-22  7:37 ` [RFC RESEND PATCH v2 4/8] hwmon: raspberry-pi: " Matti Vaittinen
                   ` (5 subsequent siblings)
  8 siblings, 0 replies; 14+ messages in thread
From: Matti Vaittinen @ 2021-03-22  7:37 UTC (permalink / raw)
  To: Matti Vaittinen, Matti Vaittinen
  Cc: MyungJoo Ham, Chanwoo Choi, Andy Gross, Bjorn Andersson,
	Guenter Roeck, Hans de Goede, Mark Gross, Matti Vaittinen,
	gregkh, linux-kernel, linux-arm-msm, platform-driver-x86

Few drivers implement remove call-back only for ensuring a delayed
work gets cancelled prior driver removal. Clean-up these by switching
to use devm_delayed_work_autocancel() instead.

Additionally, this helps avoiding mixing devm and manual resource
management and cleans up a (theoretical?) bug from extconn-palmas.c
and extcon-qcom-spmi-misc.c where (devm managed)IRQ might schedule
new work item after wq was cleaned at remove().

This change is compile-tested only. All testing is appreciated.

Signed-off-by: Matti Vaittinen <matti.vaittinen@fi.rohmeurope.com>
---
 drivers/extcon/extcon-gpio.c           | 15 ++++-----------
 drivers/extcon/extcon-intel-int3496.c  | 16 ++++------------
 drivers/extcon/extcon-palmas.c         | 17 ++++++-----------
 drivers/extcon/extcon-qcom-spmi-misc.c | 17 ++++++-----------
 4 files changed, 20 insertions(+), 45 deletions(-)

diff --git a/drivers/extcon/extcon-gpio.c b/drivers/extcon/extcon-gpio.c
index c211222f5d0c..4105df74f2b0 100644
--- a/drivers/extcon/extcon-gpio.c
+++ b/drivers/extcon/extcon-gpio.c
@@ -9,6 +9,7 @@
  * (originally switch class is supported)
  */
 
+#include <linux/devm-helpers.h>
 #include <linux/extcon-provider.h>
 #include <linux/gpio/consumer.h>
 #include <linux/init.h>
@@ -112,7 +113,9 @@ static int gpio_extcon_probe(struct platform_device *pdev)
 	if (ret < 0)
 		return ret;
 
-	INIT_DELAYED_WORK(&data->work, gpio_extcon_work);
+	ret = devm_delayed_work_autocancel(dev, &data->work, gpio_extcon_work);
+	if (ret)
+		return ret;
 
 	/*
 	 * Request the interrupt of gpio to detect whether external connector
@@ -131,15 +134,6 @@ static int gpio_extcon_probe(struct platform_device *pdev)
 	return 0;
 }
 
-static int gpio_extcon_remove(struct platform_device *pdev)
-{
-	struct gpio_extcon_data *data = platform_get_drvdata(pdev);
-
-	cancel_delayed_work_sync(&data->work);
-
-	return 0;
-}
-
 #ifdef CONFIG_PM_SLEEP
 static int gpio_extcon_resume(struct device *dev)
 {
@@ -158,7 +152,6 @@ static SIMPLE_DEV_PM_OPS(gpio_extcon_pm_ops, NULL, gpio_extcon_resume);
 
 static struct platform_driver gpio_extcon_driver = {
 	.probe		= gpio_extcon_probe,
-	.remove		= gpio_extcon_remove,
 	.driver		= {
 		.name	= "extcon-gpio",
 		.pm	= &gpio_extcon_pm_ops,
diff --git a/drivers/extcon/extcon-intel-int3496.c b/drivers/extcon/extcon-intel-int3496.c
index 80c9abcc3f97..fb527c23639e 100644
--- a/drivers/extcon/extcon-intel-int3496.c
+++ b/drivers/extcon/extcon-intel-int3496.c
@@ -11,6 +11,7 @@
  */
 
 #include <linux/acpi.h>
+#include <linux/devm-helpers.h>
 #include <linux/extcon-provider.h>
 #include <linux/gpio/consumer.h>
 #include <linux/interrupt.h>
@@ -101,7 +102,9 @@ static int int3496_probe(struct platform_device *pdev)
 		return -ENOMEM;
 
 	data->dev = dev;
-	INIT_DELAYED_WORK(&data->work, int3496_do_usb_id);
+	ret = devm_delayed_work_autocancel(dev, &data->work, int3496_do_usb_id);
+	if (ret)
+		return ret;
 
 	data->gpio_usb_id = devm_gpiod_get(dev, "id", GPIOD_IN);
 	if (IS_ERR(data->gpio_usb_id)) {
@@ -155,16 +158,6 @@ static int int3496_probe(struct platform_device *pdev)
 	return 0;
 }
 
-static int int3496_remove(struct platform_device *pdev)
-{
-	struct int3496_data *data = platform_get_drvdata(pdev);
-
-	devm_free_irq(&pdev->dev, data->usb_id_irq, data);
-	cancel_delayed_work_sync(&data->work);
-
-	return 0;
-}
-
 static const struct acpi_device_id int3496_acpi_match[] = {
 	{ "INT3496" },
 	{ }
@@ -177,7 +170,6 @@ static struct platform_driver int3496_driver = {
 		.acpi_match_table = int3496_acpi_match,
 	},
 	.probe = int3496_probe,
-	.remove = int3496_remove,
 };
 
 module_platform_driver(int3496_driver);
diff --git a/drivers/extcon/extcon-palmas.c b/drivers/extcon/extcon-palmas.c
index a2852bcc5f0d..d2c1a8b89c08 100644
--- a/drivers/extcon/extcon-palmas.c
+++ b/drivers/extcon/extcon-palmas.c
@@ -9,6 +9,7 @@
  * Author: Hema HK <hemahk@ti.com>
  */
 
+#include <linux/devm-helpers.h>
 #include <linux/module.h>
 #include <linux/interrupt.h>
 #include <linux/platform_device.h>
@@ -237,7 +238,11 @@ static int palmas_usb_probe(struct platform_device *pdev)
 			palmas_usb->sw_debounce_jiffies = msecs_to_jiffies(debounce);
 	}
 
-	INIT_DELAYED_WORK(&palmas_usb->wq_detectid, palmas_gpio_id_detect);
+	status = devm_delayed_work_autocancel(&pdev->dev,
+					      &palmas_usb->wq_detectid,
+					      palmas_gpio_id_detect);
+	if (status)
+		return status;
 
 	palmas->usb = palmas_usb;
 	palmas_usb->palmas = palmas;
@@ -359,15 +364,6 @@ static int palmas_usb_probe(struct platform_device *pdev)
 	return 0;
 }
 
-static int palmas_usb_remove(struct platform_device *pdev)
-{
-	struct palmas_usb *palmas_usb = platform_get_drvdata(pdev);
-
-	cancel_delayed_work_sync(&palmas_usb->wq_detectid);
-
-	return 0;
-}
-
 #ifdef CONFIG_PM_SLEEP
 static int palmas_usb_suspend(struct device *dev)
 {
@@ -422,7 +418,6 @@ static const struct of_device_id of_palmas_match_tbl[] = {
 
 static struct platform_driver palmas_usb_driver = {
 	.probe = palmas_usb_probe,
-	.remove = palmas_usb_remove,
 	.driver = {
 		.name = "palmas-usb",
 		.of_match_table = of_palmas_match_tbl,
diff --git a/drivers/extcon/extcon-qcom-spmi-misc.c b/drivers/extcon/extcon-qcom-spmi-misc.c
index 6b836ae62176..74d57d951b68 100644
--- a/drivers/extcon/extcon-qcom-spmi-misc.c
+++ b/drivers/extcon/extcon-qcom-spmi-misc.c
@@ -7,6 +7,7 @@
  * Stephen Boyd <stephen.boyd@linaro.org>
  */
 
+#include <linux/devm-helpers.h>
 #include <linux/extcon-provider.h>
 #include <linux/init.h>
 #include <linux/interrupt.h>
@@ -80,7 +81,11 @@ static int qcom_usb_extcon_probe(struct platform_device *pdev)
 	}
 
 	info->debounce_jiffies = msecs_to_jiffies(USB_ID_DEBOUNCE_MS);
-	INIT_DELAYED_WORK(&info->wq_detcable, qcom_usb_extcon_detect_cable);
+
+	ret = devm_delayed_work_autocancel(dev, &info->wq_detcable,
+					   qcom_usb_extcon_detect_cable);
+	if (ret)
+		return ret;
 
 	info->irq = platform_get_irq_byname(pdev, "usb_id");
 	if (info->irq < 0)
@@ -105,15 +110,6 @@ static int qcom_usb_extcon_probe(struct platform_device *pdev)
 	return 0;
 }
 
-static int qcom_usb_extcon_remove(struct platform_device *pdev)
-{
-	struct qcom_usb_extcon_info *info = platform_get_drvdata(pdev);
-
-	cancel_delayed_work_sync(&info->wq_detcable);
-
-	return 0;
-}
-
 #ifdef CONFIG_PM_SLEEP
 static int qcom_usb_extcon_suspend(struct device *dev)
 {
@@ -149,7 +145,6 @@ MODULE_DEVICE_TABLE(of, qcom_usb_extcon_dt_match);
 
 static struct platform_driver qcom_usb_extcon_driver = {
 	.probe		= qcom_usb_extcon_probe,
-	.remove		= qcom_usb_extcon_remove,
 	.driver		= {
 		.name	= "extcon-pm8941-misc",
 		.pm	= &qcom_usb_extcon_pm_ops,
-- 
2.25.4


-- 
Matti Vaittinen, Linux device drivers
ROHM Semiconductors, Finland SWDC
Kiviharjunlenkki 1E
90220 OULU
FINLAND

~~~ "I don't think so," said Rene Descartes. Just then he vanished ~~~
Simon says - in Latin please.
~~~ "non cogito me" dixit Rene Descarte, deinde evanescavit ~~~
Thanks to Simon Glass for the translation =] 

^ permalink raw reply related	[flat|nested] 14+ messages in thread

* [RFC RESEND PATCH v2 4/8] hwmon: raspberry-pi: Clean-up few drivers by using managed work init
  2021-03-22  7:41 [RFC RESEND PATCH v2 0/8] Add managed version of delayed work init Matti Vaittinen
                   ` (2 preceding siblings ...)
  2021-03-22  7:37 ` [RFC RESEND PATCH v2 3/8] extconn: Clean-up few drivers by using managed work init Matti Vaittinen
@ 2021-03-22  7:37 ` Matti Vaittinen
  2021-03-22 14:05   ` Guenter Roeck
  2021-03-22  7:37 ` [RFC RESEND PATCH v2 5/8] platform/x86: gpd pocket fan: Clean-up " Matti Vaittinen
                   ` (4 subsequent siblings)
  8 siblings, 1 reply; 14+ messages in thread
From: Matti Vaittinen @ 2021-03-22  7:37 UTC (permalink / raw)
  To: Matti Vaittinen, Matti Vaittinen
  Cc: Jean Delvare, Guenter Roeck, Hans de Goede, Matti Vaittinen,
	gregkh, linux-kernel, linux-hwmon

Few drivers implement remove call-back only for ensuring a delayed
work gets cancelled prior driver removal. Clean-up these by switching
to use devm_delayed_work_autocancel() instead.

This change is compile-tested only. All testing is appreciated.

Signed-off-by: Matti Vaittinen <matti.vaittinen@fi.rohmeurope.com>
---
 drivers/hwmon/raspberrypi-hwmon.c | 17 ++++++-----------
 1 file changed, 6 insertions(+), 11 deletions(-)

diff --git a/drivers/hwmon/raspberrypi-hwmon.c b/drivers/hwmon/raspberrypi-hwmon.c
index d3a64a35f7a9..805d396aa81b 100644
--- a/drivers/hwmon/raspberrypi-hwmon.c
+++ b/drivers/hwmon/raspberrypi-hwmon.c
@@ -7,6 +7,7 @@
  * Copyright (C) 2018 Stefan Wahren <stefan.wahren@i2se.com>
  */
 #include <linux/device.h>
+#include <linux/devm-helpers.h>
 #include <linux/err.h>
 #include <linux/hwmon.h>
 #include <linux/module.h>
@@ -106,6 +107,7 @@ static int rpi_hwmon_probe(struct platform_device *pdev)
 {
 	struct device *dev = &pdev->dev;
 	struct rpi_hwmon_data *data;
+	int ret;
 
 	data = devm_kzalloc(dev, sizeof(*data), GFP_KERNEL);
 	if (!data)
@@ -119,7 +121,10 @@ static int rpi_hwmon_probe(struct platform_device *pdev)
 							       &rpi_chip_info,
 							       NULL);
 
-	INIT_DELAYED_WORK(&data->get_values_poll_work, get_values_poll);
+	ret = devm_delayed_work_autocancel(dev, &data->get_values_poll_work,
+					   get_values_poll);
+	if (ret)
+		return ret;
 	platform_set_drvdata(pdev, data);
 
 	if (!PTR_ERR_OR_ZERO(data->hwmon_dev))
@@ -128,18 +133,8 @@ static int rpi_hwmon_probe(struct platform_device *pdev)
 	return PTR_ERR_OR_ZERO(data->hwmon_dev);
 }
 
-static int rpi_hwmon_remove(struct platform_device *pdev)
-{
-	struct rpi_hwmon_data *data = platform_get_drvdata(pdev);
-
-	cancel_delayed_work_sync(&data->get_values_poll_work);
-
-	return 0;
-}
-
 static struct platform_driver rpi_hwmon_driver = {
 	.probe = rpi_hwmon_probe,
-	.remove = rpi_hwmon_remove,
 	.driver = {
 		.name = "raspberrypi-hwmon",
 	},
-- 
2.25.4


-- 
Matti Vaittinen, Linux device drivers
ROHM Semiconductors, Finland SWDC
Kiviharjunlenkki 1E
90220 OULU
FINLAND

~~~ "I don't think so," said Rene Descartes. Just then he vanished ~~~
Simon says - in Latin please.
~~~ "non cogito me" dixit Rene Descarte, deinde evanescavit ~~~
Thanks to Simon Glass for the translation =] 

^ permalink raw reply related	[flat|nested] 14+ messages in thread

* [RFC RESEND PATCH v2 5/8] platform/x86: gpd pocket fan: Clean-up by using managed work init
  2021-03-22  7:41 [RFC RESEND PATCH v2 0/8] Add managed version of delayed work init Matti Vaittinen
                   ` (3 preceding siblings ...)
  2021-03-22  7:37 ` [RFC RESEND PATCH v2 4/8] hwmon: raspberry-pi: " Matti Vaittinen
@ 2021-03-22  7:37 ` Matti Vaittinen
  2021-03-22  7:37 ` [RFC RESEND PATCH v2 6/8] power: supply: Clean-up few drivers " Matti Vaittinen
                   ` (3 subsequent siblings)
  8 siblings, 0 replies; 14+ messages in thread
From: Matti Vaittinen @ 2021-03-22  7:37 UTC (permalink / raw)
  To: Matti Vaittinen, Matti Vaittinen
  Cc: Hans de Goede, Mark Gross, Matti Vaittinen, gregkh, linux-kernel,
	platform-driver-x86

Few drivers implement remove call-back only for ensuring a delayed
work gets cancelled prior driver removal. Clean-up these by switching
to use devm_delayed_work_autocancel() instead.

This change is compile-tested only. All testing is appreciated.

Signed-off-by: Matti Vaittinen <matti.vaittinen@fi.rohmeurope.com>
---
 drivers/platform/x86/gpd-pocket-fan.c | 17 ++++++-----------
 1 file changed, 6 insertions(+), 11 deletions(-)

diff --git a/drivers/platform/x86/gpd-pocket-fan.c b/drivers/platform/x86/gpd-pocket-fan.c
index 5b516e4c2bfb..7a20f68ae206 100644
--- a/drivers/platform/x86/gpd-pocket-fan.c
+++ b/drivers/platform/x86/gpd-pocket-fan.c
@@ -6,6 +6,7 @@
  */
 
 #include <linux/acpi.h>
+#include <linux/devm-helpers.h>
 #include <linux/gpio/consumer.h>
 #include <linux/module.h>
 #include <linux/moduleparam.h>
@@ -124,7 +125,7 @@ static void gpd_pocket_fan_force_update(struct gpd_pocket_fan_data *fan)
 static int gpd_pocket_fan_probe(struct platform_device *pdev)
 {
 	struct gpd_pocket_fan_data *fan;
-	int i;
+	int i, ret;
 
 	for (i = 0; i < ARRAY_SIZE(temp_limits); i++) {
 		if (temp_limits[i] < 20000 || temp_limits[i] > 90000) {
@@ -152,7 +153,10 @@ static int gpd_pocket_fan_probe(struct platform_device *pdev)
 		return -ENOMEM;
 
 	fan->dev = &pdev->dev;
-	INIT_DELAYED_WORK(&fan->work, gpd_pocket_fan_worker);
+	ret = devm_delayed_work_autocancel(&pdev->dev, &fan->work,
+					   gpd_pocket_fan_worker);
+	if (ret)
+		return ret;
 
 	/* Note this returns a "weak" reference which we don't need to free */
 	fan->dts0 = thermal_zone_get_zone_by_name("soc_dts0");
@@ -177,14 +181,6 @@ static int gpd_pocket_fan_probe(struct platform_device *pdev)
 	return 0;
 }
 
-static int gpd_pocket_fan_remove(struct platform_device *pdev)
-{
-	struct gpd_pocket_fan_data *fan = platform_get_drvdata(pdev);
-
-	cancel_delayed_work_sync(&fan->work);
-	return 0;
-}
-
 #ifdef CONFIG_PM_SLEEP
 static int gpd_pocket_fan_suspend(struct device *dev)
 {
@@ -215,7 +211,6 @@ MODULE_DEVICE_TABLE(acpi, gpd_pocket_fan_acpi_match);
 
 static struct platform_driver gpd_pocket_fan_driver = {
 	.probe	= gpd_pocket_fan_probe,
-	.remove	= gpd_pocket_fan_remove,
 	.driver	= {
 		.name			= "gpd_pocket_fan",
 		.acpi_match_table	= gpd_pocket_fan_acpi_match,
-- 
2.25.4


-- 
Matti Vaittinen, Linux device drivers
ROHM Semiconductors, Finland SWDC
Kiviharjunlenkki 1E
90220 OULU
FINLAND

~~~ "I don't think so," said Rene Descartes. Just then he vanished ~~~
Simon says - in Latin please.
~~~ "non cogito me" dixit Rene Descarte, deinde evanescavit ~~~
Thanks to Simon Glass for the translation =] 

^ permalink raw reply related	[flat|nested] 14+ messages in thread

* [RFC RESEND PATCH v2 6/8] power: supply: Clean-up few drivers by using managed work init
  2021-03-22  7:41 [RFC RESEND PATCH v2 0/8] Add managed version of delayed work init Matti Vaittinen
                   ` (4 preceding siblings ...)
  2021-03-22  7:37 ` [RFC RESEND PATCH v2 5/8] platform/x86: gpd pocket fan: Clean-up " Matti Vaittinen
@ 2021-03-22  7:37 ` Matti Vaittinen
  2021-03-22 14:08   ` Chen-Yu Tsai
  2021-03-22  7:38 ` [RFC RESEND PATCH v2 7/8] regulator: qcom_spmi-regulator: Clean-up " Matti Vaittinen
                   ` (2 subsequent siblings)
  8 siblings, 1 reply; 14+ messages in thread
From: Matti Vaittinen @ 2021-03-22  7:37 UTC (permalink / raw)
  To: Matti Vaittinen, Matti Vaittinen
  Cc: Hans de Goede, Sebastian Reichel, Chen-Yu Tsai, Matti Vaittinen,
	gregkh, linux-kernel, linux-pm

Few drivers implement remove call-back only for ensuring a delayed
work gets cancelled prior driver removal. Clean-up these by switching
to use devm_delayed_work_autocancel() instead.

This change is compile-tested only. All testing is appreciated.

Signed-off-by: Matti Vaittinen <matti.vaittinen@fi.rohmeurope.com>
Acked-by: Sebastian Reichel <sebastian.reichel@collabora.com>
---
 drivers/power/supply/axp20x_usb_power.c      | 15 +++++----------
 drivers/power/supply/bq24735-charger.c       | 18 ++++++------------
 drivers/power/supply/ltc2941-battery-gauge.c | 20 +++++++-------------
 drivers/power/supply/sbs-battery.c           | 16 +++++-----------
 4 files changed, 23 insertions(+), 46 deletions(-)

diff --git a/drivers/power/supply/axp20x_usb_power.c b/drivers/power/supply/axp20x_usb_power.c
index 8933ae26c3d6..4259709e3491 100644
--- a/drivers/power/supply/axp20x_usb_power.c
+++ b/drivers/power/supply/axp20x_usb_power.c
@@ -8,6 +8,7 @@
 
 #include <linux/bitops.h>
 #include <linux/device.h>
+#include <linux/devm-helpers.h>
 #include <linux/init.h>
 #include <linux/interrupt.h>
 #include <linux/kernel.h>
@@ -646,21 +647,16 @@ static int axp20x_usb_power_probe(struct platform_device *pdev)
 		}
 	}
 
+	ret = devm_delayed_work_autocancel(&pdev->dev, &power->vbus_detect,
+					   axp20x_usb_power_poll_vbus);
+	if (ret)
+		return ret;
 	if (axp20x_usb_vbus_needs_polling(power))
 		queue_delayed_work(system_power_efficient_wq, &power->vbus_detect, 0);
 
 	return 0;
 }
 
-static int axp20x_usb_power_remove(struct platform_device *pdev)
-{
-	struct axp20x_usb_power *power = platform_get_drvdata(pdev);
-
-	cancel_delayed_work_sync(&power->vbus_detect);
-
-	return 0;
-}
-
 static const struct of_device_id axp20x_usb_power_match[] = {
 	{
 		.compatible = "x-powers,axp202-usb-power-supply",
@@ -680,7 +676,6 @@ MODULE_DEVICE_TABLE(of, axp20x_usb_power_match);
 
 static struct platform_driver axp20x_usb_power_driver = {
 	.probe = axp20x_usb_power_probe,
-	.remove = axp20x_usb_power_remove,
 	.driver = {
 		.name		= DRVNAME,
 		.of_match_table	= axp20x_usb_power_match,
diff --git a/drivers/power/supply/bq24735-charger.c b/drivers/power/supply/bq24735-charger.c
index ab2f4bf8f603..b5d619db79f6 100644
--- a/drivers/power/supply/bq24735-charger.c
+++ b/drivers/power/supply/bq24735-charger.c
@@ -17,6 +17,7 @@
  * 51 Franklin Street, Fifth Floor, Boston, MA  02110-1301, USA.
  */
 
+#include <linux/devm-helpers.h>
 #include <linux/err.h>
 #include <linux/i2c.h>
 #include <linux/init.h>
@@ -473,7 +474,11 @@ static int bq24735_charger_probe(struct i2c_client *client,
 		if (!charger->poll_interval)
 			return 0;
 
-		INIT_DELAYED_WORK(&charger->poll, bq24735_poll);
+		ret = devm_delayed_work_autocancel(&client->dev, &charger->poll,
+						   bq24735_poll);
+		if (ret)
+			return ret;
+
 		schedule_delayed_work(&charger->poll,
 				      msecs_to_jiffies(charger->poll_interval));
 	}
@@ -481,16 +486,6 @@ static int bq24735_charger_probe(struct i2c_client *client,
 	return 0;
 }
 
-static int bq24735_charger_remove(struct i2c_client *client)
-{
-	struct bq24735 *charger = i2c_get_clientdata(client);
-
-	if (charger->poll_interval)
-		cancel_delayed_work_sync(&charger->poll);
-
-	return 0;
-}
-
 static const struct i2c_device_id bq24735_charger_id[] = {
 	{ "bq24735-charger", 0 },
 	{}
@@ -509,7 +504,6 @@ static struct i2c_driver bq24735_charger_driver = {
 		.of_match_table = bq24735_match_ids,
 	},
 	.probe = bq24735_charger_probe,
-	.remove = bq24735_charger_remove,
 	.id_table = bq24735_charger_id,
 };
 
diff --git a/drivers/power/supply/ltc2941-battery-gauge.c b/drivers/power/supply/ltc2941-battery-gauge.c
index 10cd617516ec..09f3e78af4e0 100644
--- a/drivers/power/supply/ltc2941-battery-gauge.c
+++ b/drivers/power/supply/ltc2941-battery-gauge.c
@@ -8,6 +8,7 @@
  * Author: Auryn Verwegen
  * Author: Mike Looijmans
  */
+#include <linux/devm-helpers.h>
 #include <linux/kernel.h>
 #include <linux/module.h>
 #include <linux/of_device.h>
@@ -445,15 +446,6 @@ static enum power_supply_property ltc294x_properties[] = {
 	POWER_SUPPLY_PROP_CURRENT_NOW,
 };
 
-static int ltc294x_i2c_remove(struct i2c_client *client)
-{
-	struct ltc294x_info *info = i2c_get_clientdata(client);
-
-	cancel_delayed_work_sync(&info->work);
-	power_supply_unregister(info->supply);
-	return 0;
-}
-
 static int ltc294x_i2c_probe(struct i2c_client *client,
 	const struct i2c_device_id *id)
 {
@@ -547,7 +539,10 @@ static int ltc294x_i2c_probe(struct i2c_client *client,
 
 	psy_cfg.drv_data = info;
 
-	INIT_DELAYED_WORK(&info->work, ltc294x_work);
+	ret = devm_delayed_work_autocancel(&client->dev, &info->work,
+					   ltc294x_work);
+	if (ret)
+		return ret;
 
 	ret = ltc294x_reset(info, prescaler_exp);
 	if (ret < 0) {
@@ -555,8 +550,8 @@ static int ltc294x_i2c_probe(struct i2c_client *client,
 		return ret;
 	}
 
-	info->supply = power_supply_register(&client->dev, &info->supply_desc,
-					     &psy_cfg);
+	info->supply = devm_power_supply_register(&client->dev,
+						  &info->supply_desc, &psy_cfg);
 	if (IS_ERR(info->supply)) {
 		dev_err(&client->dev, "failed to register ltc2941\n");
 		return PTR_ERR(info->supply);
@@ -655,7 +650,6 @@ static struct i2c_driver ltc294x_driver = {
 		.pm	= LTC294X_PM_OPS,
 	},
 	.probe		= ltc294x_i2c_probe,
-	.remove		= ltc294x_i2c_remove,
 	.shutdown	= ltc294x_i2c_shutdown,
 	.id_table	= ltc294x_i2c_id,
 };
diff --git a/drivers/power/supply/sbs-battery.c b/drivers/power/supply/sbs-battery.c
index b6a538ebb378..70ea404b2a36 100644
--- a/drivers/power/supply/sbs-battery.c
+++ b/drivers/power/supply/sbs-battery.c
@@ -7,6 +7,7 @@
 
 #include <linux/bits.h>
 #include <linux/delay.h>
+#include <linux/devm-helpers.h>
 #include <linux/err.h>
 #include <linux/gpio/consumer.h>
 #include <linux/i2c.h>
@@ -1165,7 +1166,10 @@ static int sbs_probe(struct i2c_client *client)
 		}
 	}
 
-	INIT_DELAYED_WORK(&chip->work, sbs_delayed_work);
+	rc = devm_delayed_work_autocancel(&client->dev, &chip->work,
+					  sbs_delayed_work);
+	if (rc)
+		return rc;
 
 	chip->power_supply = devm_power_supply_register(&client->dev, sbs_desc,
 						   &psy_cfg);
@@ -1185,15 +1189,6 @@ static int sbs_probe(struct i2c_client *client)
 	return rc;
 }
 
-static int sbs_remove(struct i2c_client *client)
-{
-	struct sbs_info *chip = i2c_get_clientdata(client);
-
-	cancel_delayed_work_sync(&chip->work);
-
-	return 0;
-}
-
 #if defined CONFIG_PM_SLEEP
 
 static int sbs_suspend(struct device *dev)
@@ -1248,7 +1243,6 @@ MODULE_DEVICE_TABLE(of, sbs_dt_ids);
 
 static struct i2c_driver sbs_battery_driver = {
 	.probe_new	= sbs_probe,
-	.remove		= sbs_remove,
 	.alert		= sbs_alert,
 	.id_table	= sbs_id,
 	.driver = {
-- 
2.25.4


-- 
Matti Vaittinen, Linux device drivers
ROHM Semiconductors, Finland SWDC
Kiviharjunlenkki 1E
90220 OULU
FINLAND

~~~ "I don't think so," said Rene Descartes. Just then he vanished ~~~
Simon says - in Latin please.
~~~ "non cogito me" dixit Rene Descarte, deinde evanescavit ~~~
Thanks to Simon Glass for the translation =] 

^ permalink raw reply related	[flat|nested] 14+ messages in thread

* [RFC RESEND PATCH v2 7/8] regulator: qcom_spmi-regulator: Clean-up by using managed work init
  2021-03-22  7:41 [RFC RESEND PATCH v2 0/8] Add managed version of delayed work init Matti Vaittinen
                   ` (5 preceding siblings ...)
  2021-03-22  7:37 ` [RFC RESEND PATCH v2 6/8] power: supply: Clean-up few drivers " Matti Vaittinen
@ 2021-03-22  7:38 ` Matti Vaittinen
  2021-03-22  7:38 ` [RFC RESEND PATCH v2 8/8] watchdog: retu_wdt: " Matti Vaittinen
  2021-03-23 12:43 ` [RFC RESEND PATCH v2 0/8] Add managed version of delayed " Greg KH
  8 siblings, 0 replies; 14+ messages in thread
From: Matti Vaittinen @ 2021-03-22  7:38 UTC (permalink / raw)
  To: Matti Vaittinen, Matti Vaittinen
  Cc: Hans de Goede, Liam Girdwood, Mark Brown, Matti Vaittinen,
	gregkh, linux-kernel

Few drivers implement remove call-back only for ensuring a delayed
work gets cancelled prior driver removal. Clean-up these by switching
to use devm_delayed_work_autocancel() instead.

Additionally, this helps avoiding mixing devm and manual resource
management and cleans up a (theoretical?) bug where devm managed
over-current IRQ might schedule a new work item after wq was cleaned
at remove().

This change is compile-tested only. All testing is appreciated.

Signed-off-by: Matti Vaittinen <matti.vaittinen@fi.rohmeurope.com>
---
 drivers/regulator/qcom_spmi-regulator.c | 34 ++++++-------------------
 1 file changed, 8 insertions(+), 26 deletions(-)

diff --git a/drivers/regulator/qcom_spmi-regulator.c b/drivers/regulator/qcom_spmi-regulator.c
index e62e1d72d943..c2442d7798ab 100644
--- a/drivers/regulator/qcom_spmi-regulator.c
+++ b/drivers/regulator/qcom_spmi-regulator.c
@@ -5,6 +5,7 @@
 
 #include <linux/module.h>
 #include <linux/delay.h>
+#include <linux/devm-helpers.h>
 #include <linux/err.h>
 #include <linux/kernel.h>
 #include <linux/interrupt.h>
@@ -1842,7 +1843,10 @@ static int spmi_regulator_of_parse(struct device_node *node,
 			return ret;
 		}
 
-		INIT_DELAYED_WORK(&vreg->ocp_work, spmi_regulator_vs_ocp_work);
+		ret = devm_delayed_work_autocancel(dev, &vreg->ocp_work,
+						   spmi_regulator_vs_ocp_work);
+		if (ret)
+			return ret;
 	}
 
 	return 0;
@@ -2157,10 +2161,8 @@ static int qcom_spmi_regulator_probe(struct platform_device *pdev)
 		vreg->regmap = regmap;
 		if (reg->ocp) {
 			vreg->ocp_irq = platform_get_irq_byname(pdev, reg->ocp);
-			if (vreg->ocp_irq < 0) {
-				ret = vreg->ocp_irq;
-				goto err;
-			}
+			if (vreg->ocp_irq < 0)
+				return vreg->ocp_irq;
 		}
 		vreg->desc.id = -1;
 		vreg->desc.owner = THIS_MODULE;
@@ -2203,8 +2205,7 @@ static int qcom_spmi_regulator_probe(struct platform_device *pdev)
 		rdev = devm_regulator_register(dev, &vreg->desc, &config);
 		if (IS_ERR(rdev)) {
 			dev_err(dev, "failed to register %s\n", name);
-			ret = PTR_ERR(rdev);
-			goto err;
+			return PTR_ERR(rdev);
 		}
 
 		INIT_LIST_HEAD(&vreg->node);
@@ -2212,24 +2213,6 @@ static int qcom_spmi_regulator_probe(struct platform_device *pdev)
 	}
 
 	return 0;
-
-err:
-	list_for_each_entry(vreg, vreg_list, node)
-		if (vreg->ocp_irq)
-			cancel_delayed_work_sync(&vreg->ocp_work);
-	return ret;
-}
-
-static int qcom_spmi_regulator_remove(struct platform_device *pdev)
-{
-	struct spmi_regulator *vreg;
-	struct list_head *vreg_list = platform_get_drvdata(pdev);
-
-	list_for_each_entry(vreg, vreg_list, node)
-		if (vreg->ocp_irq)
-			cancel_delayed_work_sync(&vreg->ocp_work);
-
-	return 0;
 }
 
 static struct platform_driver qcom_spmi_regulator_driver = {
@@ -2238,7 +2221,6 @@ static struct platform_driver qcom_spmi_regulator_driver = {
 		.of_match_table = qcom_spmi_regulator_match,
 	},
 	.probe		= qcom_spmi_regulator_probe,
-	.remove		= qcom_spmi_regulator_remove,
 };
 module_platform_driver(qcom_spmi_regulator_driver);
 
-- 
2.25.4


-- 
Matti Vaittinen, Linux device drivers
ROHM Semiconductors, Finland SWDC
Kiviharjunlenkki 1E
90220 OULU
FINLAND

~~~ "I don't think so," said Rene Descartes. Just then he vanished ~~~
Simon says - in Latin please.
~~~ "non cogito me" dixit Rene Descarte, deinde evanescavit ~~~
Thanks to Simon Glass for the translation =] 

^ permalink raw reply related	[flat|nested] 14+ messages in thread

* [RFC RESEND PATCH v2 8/8] watchdog: retu_wdt: Clean-up by using managed work init
  2021-03-22  7:41 [RFC RESEND PATCH v2 0/8] Add managed version of delayed work init Matti Vaittinen
                   ` (6 preceding siblings ...)
  2021-03-22  7:38 ` [RFC RESEND PATCH v2 7/8] regulator: qcom_spmi-regulator: Clean-up " Matti Vaittinen
@ 2021-03-22  7:38 ` Matti Vaittinen
  2021-03-22 14:05   ` Guenter Roeck
  2021-03-23 12:43 ` [RFC RESEND PATCH v2 0/8] Add managed version of delayed " Greg KH
  8 siblings, 1 reply; 14+ messages in thread
From: Matti Vaittinen @ 2021-03-22  7:38 UTC (permalink / raw)
  To: Matti Vaittinen, Matti Vaittinen
  Cc: Guenter Roeck, Hans de Goede, Wim Van Sebroeck, Matti Vaittinen,
	gregkh, linux-kernel, linux-watchdog

Few drivers implement remove call-back only for ensuring a delayed
work gets cancelled prior driver removal. Clean-up these by switching
to use devm_delayed_work_autocancel() instead.

This change is compile-tested only. All testing is appreciated.

Signed-off-by: Matti Vaittinen <matti.vaittinen@fi.rohmeurope.com>
---
 drivers/watchdog/retu_wdt.c | 22 ++++++----------------
 1 file changed, 6 insertions(+), 16 deletions(-)

diff --git a/drivers/watchdog/retu_wdt.c b/drivers/watchdog/retu_wdt.c
index 258dfcf9cbda..2b9017e1cd91 100644
--- a/drivers/watchdog/retu_wdt.c
+++ b/drivers/watchdog/retu_wdt.c
@@ -8,6 +8,7 @@
  * Rewritten by Aaro Koskinen.
  */
 
+#include <linux/devm-helpers.h>
 #include <linux/slab.h>
 #include <linux/errno.h>
 #include <linux/device.h>
@@ -127,9 +128,12 @@ static int retu_wdt_probe(struct platform_device *pdev)
 	wdev->rdev		= rdev;
 	wdev->dev		= &pdev->dev;
 
-	INIT_DELAYED_WORK(&wdev->ping_work, retu_wdt_ping_work);
+	ret = devm_delayed_work_autocancel(&pdev->dev, &wdev->ping_work,
+					   retu_wdt_ping_work);
+	if (ret)
+		return ret;
 
-	ret = watchdog_register_device(retu_wdt);
+	ret = devm_watchdog_register_device(&pdev->dev, retu_wdt);
 	if (ret < 0)
 		return ret;
 
@@ -138,25 +142,11 @@ static int retu_wdt_probe(struct platform_device *pdev)
 	else
 		retu_wdt_ping_enable(wdev);
 
-	platform_set_drvdata(pdev, retu_wdt);
-
-	return 0;
-}
-
-static int retu_wdt_remove(struct platform_device *pdev)
-{
-	struct watchdog_device *wdog = platform_get_drvdata(pdev);
-	struct retu_wdt_dev *wdev = watchdog_get_drvdata(wdog);
-
-	watchdog_unregister_device(wdog);
-	cancel_delayed_work_sync(&wdev->ping_work);
-
 	return 0;
 }
 
 static struct platform_driver retu_wdt_driver = {
 	.probe		= retu_wdt_probe,
-	.remove		= retu_wdt_remove,
 	.driver		= {
 		.name	= "retu-wdt",
 	},
-- 
2.25.4


-- 
Matti Vaittinen, Linux device drivers
ROHM Semiconductors, Finland SWDC
Kiviharjunlenkki 1E
90220 OULU
FINLAND

~~~ "I don't think so," said Rene Descartes. Just then he vanished ~~~
Simon says - in Latin please.
~~~ "non cogito me" dixit Rene Descarte, deinde evanescavit ~~~
Thanks to Simon Glass for the translation =] 

^ permalink raw reply related	[flat|nested] 14+ messages in thread

* [RFC RESEND PATCH v2 0/8] Add managed version of delayed work init
@ 2021-03-22  7:41 Matti Vaittinen
  2021-03-22  7:36 ` [RFC RESEND PATCH v2 1/8] workqueue: Add resource " Matti Vaittinen
                   ` (8 more replies)
  0 siblings, 9 replies; 14+ messages in thread
From: Matti Vaittinen @ 2021-03-22  7:41 UTC (permalink / raw)
  To: Matti Vaittinen, Matti Vaittinen
  Cc: MyungJoo Ham, Chanwoo Choi, Andy Gross, Bjorn Andersson,
	Jean Delvare, Guenter Roeck, Hans de Goede, Mark Gross,
	Sebastian Reichel, Chen-Yu Tsai, Liam Girdwood, Mark Brown,
	Wim Van Sebroeck, Matti Vaittinen, gregkh, linux-kernel,
	linux-arm-msm, linux-hwmon, platform-driver-x86, linux-pm,
	linux-watchdog

It's not rare that device drivers need delayed work.
It's not rare that this work needs driver's data.

Often this means that driver must ensure the work is not queued when
driver is detached. Often it is done by ensuring new work is not added and
then calling cancel_delayed_work_sync() at remove(). In many cases this
may also require cleanup at probe error path - which is easy to forget.

Also the "by ensuring new work is not added" has a gotcha.

It is not strange to see devm managed IRQs scheduling (delayed) work.
Mixing this with manua wq clean-up is hard to do correctly because the
devm is likely to free the IRQ only after the remove() is ran. So manual
wq cancellation and devm-based IRQ management do not mix well - there is
a short(?) time-window after the wq clean-up when IRQs are still not
freed and may schedule new work.

When both WQs and IRQs are managed by devm things are likely to just
work. WQs should be initialized before IRQs (when IRQs need to schedule
work) and devm unwinds things in "FILO" order.

This series implements delayed wq cancellation on top of devm and replaces
the obvious cases where only thing remove call-back in a driver does is
cancelling the work. There might be other cases where we could switch
more than just work cancellation to use managed version and thus get rid
of remove or mixed (manual and devm) resource management.

This RFC does also introduce include/linux/devm-helpers.h file which
hopefully works as a place where this kind of helpers can be inlined.

Please see previous discussion here:
RFC v1:
https://lore.kernel.org/lkml/cover.1613216412.git.matti.vaittinen@fi.rohmeurope.com/

Changelog v2 resend:
  - rebased on 5.12-rc4

Changelog v2:
  - used correct terminology ("driver detach" instead of "exit, ...")
  - inlined the devm_delayed_work_autocancel() in a header
  - added Hans as a maintainer for the new header + myself as a reviewer
  - used devm_add_action() instead of using plain devres_add()

---

Matti Vaittinen (8):
  workqueue: Add resource managed version of delayed work init
  MAINTAINERS: Add entry for devm helpers
  extconn: Clean-up few drivers by using managed work init
  hwmon: raspberry-pi: Clean-up few drivers by using managed work init
  platform/x86: gpd pocket fan: Clean-up by using managed work init
  power: supply: Clean-up few drivers by using managed work init
  regulator: qcom_spmi-regulator: Clean-up by using managed work init
  watchdog: retu_wdt: Clean-up by using managed work init

 MAINTAINERS                                  |  6 +++
 drivers/extcon/extcon-gpio.c                 | 15 ++----
 drivers/extcon/extcon-intel-int3496.c        | 16 ++----
 drivers/extcon/extcon-palmas.c               | 17 +++----
 drivers/extcon/extcon-qcom-spmi-misc.c       | 17 +++----
 drivers/hwmon/raspberrypi-hwmon.c            | 17 +++----
 drivers/platform/x86/gpd-pocket-fan.c        | 17 +++----
 drivers/power/supply/axp20x_usb_power.c      | 15 ++----
 drivers/power/supply/bq24735-charger.c       | 18 +++----
 drivers/power/supply/ltc2941-battery-gauge.c | 20 +++-----
 drivers/power/supply/sbs-battery.c           | 16 ++----
 drivers/regulator/qcom_spmi-regulator.c      | 34 +++----------
 drivers/watchdog/retu_wdt.c                  | 22 +++-----
 include/linux/devm-helpers.h                 | 53 ++++++++++++++++++++
 14 files changed, 128 insertions(+), 155 deletions(-)
 create mode 100644 include/linux/devm-helpers.h


base-commit: 0d02ec6b3136c73c09e7859f0d0e4e2c4c07b49b
-- 
2.25.4


-- 
Matti Vaittinen, Linux device drivers
ROHM Semiconductors, Finland SWDC
Kiviharjunlenkki 1E
90220 OULU
FINLAND

~~~ "I don't think so," said Rene Descartes. Just then he vanished ~~~
Simon says - in Latin please.
~~~ "non cogito me" dixit Rene Descarte, deinde evanescavit ~~~
Thanks to Simon Glass for the translation =] 

^ permalink raw reply	[flat|nested] 14+ messages in thread

* Re: [RFC RESEND PATCH v2 4/8] hwmon: raspberry-pi: Clean-up few drivers by using managed work init
  2021-03-22  7:37 ` [RFC RESEND PATCH v2 4/8] hwmon: raspberry-pi: " Matti Vaittinen
@ 2021-03-22 14:05   ` Guenter Roeck
  0 siblings, 0 replies; 14+ messages in thread
From: Guenter Roeck @ 2021-03-22 14:05 UTC (permalink / raw)
  To: Matti Vaittinen
  Cc: Matti Vaittinen, Jean Delvare, Hans de Goede, gregkh,
	linux-kernel, linux-hwmon

On Mon, Mar 22, 2021 at 09:37:18AM +0200, Matti Vaittinen wrote:
> Few drivers implement remove call-back only for ensuring a delayed
> work gets cancelled prior driver removal. Clean-up these by switching
> to use devm_delayed_work_autocancel() instead.
> 
> This change is compile-tested only. All testing is appreciated.
> 
> Signed-off-by: Matti Vaittinen <matti.vaittinen@fi.rohmeurope.com>

Acked-by: Guenter Roeck <linux@roeck-us.net>

> ---
>  drivers/hwmon/raspberrypi-hwmon.c | 17 ++++++-----------
>  1 file changed, 6 insertions(+), 11 deletions(-)
> 
> diff --git a/drivers/hwmon/raspberrypi-hwmon.c b/drivers/hwmon/raspberrypi-hwmon.c
> index d3a64a35f7a9..805d396aa81b 100644
> --- a/drivers/hwmon/raspberrypi-hwmon.c
> +++ b/drivers/hwmon/raspberrypi-hwmon.c
> @@ -7,6 +7,7 @@
>   * Copyright (C) 2018 Stefan Wahren <stefan.wahren@i2se.com>
>   */
>  #include <linux/device.h>
> +#include <linux/devm-helpers.h>
>  #include <linux/err.h>
>  #include <linux/hwmon.h>
>  #include <linux/module.h>
> @@ -106,6 +107,7 @@ static int rpi_hwmon_probe(struct platform_device *pdev)
>  {
>  	struct device *dev = &pdev->dev;
>  	struct rpi_hwmon_data *data;
> +	int ret;
>  
>  	data = devm_kzalloc(dev, sizeof(*data), GFP_KERNEL);
>  	if (!data)
> @@ -119,7 +121,10 @@ static int rpi_hwmon_probe(struct platform_device *pdev)
>  							       &rpi_chip_info,
>  							       NULL);
>  
> -	INIT_DELAYED_WORK(&data->get_values_poll_work, get_values_poll);
> +	ret = devm_delayed_work_autocancel(dev, &data->get_values_poll_work,
> +					   get_values_poll);
> +	if (ret)
> +		return ret;
>  	platform_set_drvdata(pdev, data);
>  
>  	if (!PTR_ERR_OR_ZERO(data->hwmon_dev))
> @@ -128,18 +133,8 @@ static int rpi_hwmon_probe(struct platform_device *pdev)
>  	return PTR_ERR_OR_ZERO(data->hwmon_dev);
>  }
>  
> -static int rpi_hwmon_remove(struct platform_device *pdev)
> -{
> -	struct rpi_hwmon_data *data = platform_get_drvdata(pdev);
> -
> -	cancel_delayed_work_sync(&data->get_values_poll_work);
> -
> -	return 0;
> -}
> -
>  static struct platform_driver rpi_hwmon_driver = {
>  	.probe = rpi_hwmon_probe,
> -	.remove = rpi_hwmon_remove,
>  	.driver = {
>  		.name = "raspberrypi-hwmon",
>  	},
> -- 
> 2.25.4
> 
> 
> -- 
> Matti Vaittinen, Linux device drivers
> ROHM Semiconductors, Finland SWDC
> Kiviharjunlenkki 1E
> 90220 OULU
> FINLAND
> 
> ~~~ "I don't think so," said Rene Descartes. Just then he vanished ~~~
> Simon says - in Latin please.
> ~~~ "non cogito me" dixit Rene Descarte, deinde evanescavit ~~~
> Thanks to Simon Glass for the translation =] 

^ permalink raw reply	[flat|nested] 14+ messages in thread

* Re: [RFC RESEND PATCH v2 8/8] watchdog: retu_wdt: Clean-up by using managed work init
  2021-03-22  7:38 ` [RFC RESEND PATCH v2 8/8] watchdog: retu_wdt: " Matti Vaittinen
@ 2021-03-22 14:05   ` Guenter Roeck
  0 siblings, 0 replies; 14+ messages in thread
From: Guenter Roeck @ 2021-03-22 14:05 UTC (permalink / raw)
  To: Matti Vaittinen
  Cc: Matti Vaittinen, Hans de Goede, Wim Van Sebroeck, gregkh,
	linux-kernel, linux-watchdog

On Mon, Mar 22, 2021 at 09:38:37AM +0200, Matti Vaittinen wrote:
> Few drivers implement remove call-back only for ensuring a delayed
> work gets cancelled prior driver removal. Clean-up these by switching
> to use devm_delayed_work_autocancel() instead.
> 
> This change is compile-tested only. All testing is appreciated.
> 
> Signed-off-by: Matti Vaittinen <matti.vaittinen@fi.rohmeurope.com>

Acked-by: Guenter Roeck <linux@roeck-us.net>

> ---
>  drivers/watchdog/retu_wdt.c | 22 ++++++----------------
>  1 file changed, 6 insertions(+), 16 deletions(-)
> 
> diff --git a/drivers/watchdog/retu_wdt.c b/drivers/watchdog/retu_wdt.c
> index 258dfcf9cbda..2b9017e1cd91 100644
> --- a/drivers/watchdog/retu_wdt.c
> +++ b/drivers/watchdog/retu_wdt.c
> @@ -8,6 +8,7 @@
>   * Rewritten by Aaro Koskinen.
>   */
>  
> +#include <linux/devm-helpers.h>
>  #include <linux/slab.h>
>  #include <linux/errno.h>
>  #include <linux/device.h>
> @@ -127,9 +128,12 @@ static int retu_wdt_probe(struct platform_device *pdev)
>  	wdev->rdev		= rdev;
>  	wdev->dev		= &pdev->dev;
>  
> -	INIT_DELAYED_WORK(&wdev->ping_work, retu_wdt_ping_work);
> +	ret = devm_delayed_work_autocancel(&pdev->dev, &wdev->ping_work,
> +					   retu_wdt_ping_work);
> +	if (ret)
> +		return ret;
>  
> -	ret = watchdog_register_device(retu_wdt);
> +	ret = devm_watchdog_register_device(&pdev->dev, retu_wdt);
>  	if (ret < 0)
>  		return ret;
>  
> @@ -138,25 +142,11 @@ static int retu_wdt_probe(struct platform_device *pdev)
>  	else
>  		retu_wdt_ping_enable(wdev);
>  
> -	platform_set_drvdata(pdev, retu_wdt);
> -
> -	return 0;
> -}
> -
> -static int retu_wdt_remove(struct platform_device *pdev)
> -{
> -	struct watchdog_device *wdog = platform_get_drvdata(pdev);
> -	struct retu_wdt_dev *wdev = watchdog_get_drvdata(wdog);
> -
> -	watchdog_unregister_device(wdog);
> -	cancel_delayed_work_sync(&wdev->ping_work);
> -
>  	return 0;
>  }
>  
>  static struct platform_driver retu_wdt_driver = {
>  	.probe		= retu_wdt_probe,
> -	.remove		= retu_wdt_remove,
>  	.driver		= {
>  		.name	= "retu-wdt",
>  	},
> -- 
> 2.25.4
> 
> 
> -- 
> Matti Vaittinen, Linux device drivers
> ROHM Semiconductors, Finland SWDC
> Kiviharjunlenkki 1E
> 90220 OULU
> FINLAND
> 
> ~~~ "I don't think so," said Rene Descartes. Just then he vanished ~~~
> Simon says - in Latin please.
> ~~~ "non cogito me" dixit Rene Descarte, deinde evanescavit ~~~
> Thanks to Simon Glass for the translation =] 

^ permalink raw reply	[flat|nested] 14+ messages in thread

* Re: [RFC RESEND PATCH v2 6/8] power: supply: Clean-up few drivers by using managed work init
  2021-03-22  7:37 ` [RFC RESEND PATCH v2 6/8] power: supply: Clean-up few drivers " Matti Vaittinen
@ 2021-03-22 14:08   ` Chen-Yu Tsai
  0 siblings, 0 replies; 14+ messages in thread
From: Chen-Yu Tsai @ 2021-03-22 14:08 UTC (permalink / raw)
  To: matti.vaittinen
  Cc: Matti Vaittinen, Hans de Goede, Sebastian Reichel,
	Greg Kroah-Hartman, linux-kernel, open list:THERMAL

Hi,

On Mon, Mar 22, 2021 at 3:38 PM Matti Vaittinen
<matti.vaittinen@fi.rohmeurope.com> wrote:
>
> Few drivers implement remove call-back only for ensuring a delayed
> work gets cancelled prior driver removal. Clean-up these by switching
> to use devm_delayed_work_autocancel() instead.
>
> This change is compile-tested only. All testing is appreciated.
>
> Signed-off-by: Matti Vaittinen <matti.vaittinen@fi.rohmeurope.com>
> Acked-by: Sebastian Reichel <sebastian.reichel@collabora.com>
> ---
>  drivers/power/supply/axp20x_usb_power.c      | 15 +++++----------
>  drivers/power/supply/bq24735-charger.c       | 18 ++++++------------
>  drivers/power/supply/ltc2941-battery-gauge.c | 20 +++++++-------------
>  drivers/power/supply/sbs-battery.c           | 16 +++++-----------
>  4 files changed, 23 insertions(+), 46 deletions(-)
>
> diff --git a/drivers/power/supply/axp20x_usb_power.c b/drivers/power/supply/axp20x_usb_power.c
> index 8933ae26c3d6..4259709e3491 100644
> --- a/drivers/power/supply/axp20x_usb_power.c
> +++ b/drivers/power/supply/axp20x_usb_power.c
> @@ -8,6 +8,7 @@
>
>  #include <linux/bitops.h>
>  #include <linux/device.h>
> +#include <linux/devm-helpers.h>
>  #include <linux/init.h>
>  #include <linux/interrupt.h>
>  #include <linux/kernel.h>
> @@ -646,21 +647,16 @@ static int axp20x_usb_power_probe(struct platform_device *pdev)
>                 }
>         }
>
> +       ret = devm_delayed_work_autocancel(&pdev->dev, &power->vbus_detect,
> +                                          axp20x_usb_power_poll_vbus);
> +       if (ret)
> +               return ret;

This doesn't look right. The IRQ is requested before this, and the delayed_work
struct is initialized even earlier, so you'd be re-initializing the struct,
with the work item potentially running or queued up already.


ChenYu

>         if (axp20x_usb_vbus_needs_polling(power))
>                 queue_delayed_work(system_power_efficient_wq, &power->vbus_detect, 0);
>
>         return 0;
>  }
>
> -static int axp20x_usb_power_remove(struct platform_device *pdev)
> -{
> -       struct axp20x_usb_power *power = platform_get_drvdata(pdev);
> -
> -       cancel_delayed_work_sync(&power->vbus_detect);
> -
> -       return 0;
> -}
> -
>  static const struct of_device_id axp20x_usb_power_match[] = {
>         {
>                 .compatible = "x-powers,axp202-usb-power-supply",
> @@ -680,7 +676,6 @@ MODULE_DEVICE_TABLE(of, axp20x_usb_power_match);
>
>  static struct platform_driver axp20x_usb_power_driver = {
>         .probe = axp20x_usb_power_probe,
> -       .remove = axp20x_usb_power_remove,
>         .driver = {
>                 .name           = DRVNAME,
>                 .of_match_table = axp20x_usb_power_match,

^ permalink raw reply	[flat|nested] 14+ messages in thread

* Re: [RFC RESEND PATCH v2 0/8] Add managed version of delayed work init
  2021-03-22  7:41 [RFC RESEND PATCH v2 0/8] Add managed version of delayed work init Matti Vaittinen
                   ` (7 preceding siblings ...)
  2021-03-22  7:38 ` [RFC RESEND PATCH v2 8/8] watchdog: retu_wdt: " Matti Vaittinen
@ 2021-03-23 12:43 ` Greg KH
  2021-03-23 13:16   ` Vaittinen, Matti
  8 siblings, 1 reply; 14+ messages in thread
From: Greg KH @ 2021-03-23 12:43 UTC (permalink / raw)
  To: Matti Vaittinen
  Cc: Matti Vaittinen, MyungJoo Ham, Chanwoo Choi, Andy Gross,
	Bjorn Andersson, Jean Delvare, Guenter Roeck, Hans de Goede,
	Mark Gross, Sebastian Reichel, Chen-Yu Tsai, Liam Girdwood,
	Mark Brown, Wim Van Sebroeck, linux-kernel, linux-arm-msm,
	linux-hwmon, platform-driver-x86, linux-pm, linux-watchdog

On Mon, Mar 22, 2021 at 09:41:13AM +0200, Matti Vaittinen wrote:
> It's not rare that device drivers need delayed work.
> It's not rare that this work needs driver's data.

I don't normally comment on "RFC" patch series as I can't take them and
the submitter doesn't feel right with them being merged at this point in
time.

So if you think this is all correct now, please resubmit without that so
we can review it properly :)

thanks,

greg k-h

^ permalink raw reply	[flat|nested] 14+ messages in thread

* Re: [RFC RESEND PATCH v2 0/8] Add managed version of delayed work init
  2021-03-23 12:43 ` [RFC RESEND PATCH v2 0/8] Add managed version of delayed " Greg KH
@ 2021-03-23 13:16   ` Vaittinen, Matti
  0 siblings, 0 replies; 14+ messages in thread
From: Vaittinen, Matti @ 2021-03-23 13:16 UTC (permalink / raw)
  To: gregkh
  Cc: linux-hwmon, wim, linux-arm-msm, sre, jdelvare, linux-kernel,
	bjorn.andersson, lgirdwood, hdegoede, linux, platform-driver-x86,
	wens, mgross, linux-pm, myungjoo.ham, linux-watchdog, agross,
	broonie, cw00.choi

Hi Greg,

On Tue, 2021-03-23 at 13:43 +0100, Greg KH wrote:
> On Mon, Mar 22, 2021 at 09:41:13AM +0200, Matti Vaittinen wrote:
> > It's not rare that device drivers need delayed work.
> > It's not rare that this work needs driver's data.
> 
> I don't normally comment on "RFC" patch series as I can't take them
> and
> the submitter doesn't feel right with them being merged at this point
> in
> time.
> 
> So if you think this is all correct now, please resubmit without that
> so
> we can review it properly :)

Thanks for the guidance :)
I'll drop the RFC and resubmit.

Others - sorry for the noise.

Best Regards
	Matti Vaittinen


^ permalink raw reply	[flat|nested] 14+ messages in thread

end of thread, other threads:[~2021-03-23 13:17 UTC | newest]

Thread overview: 14+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2021-03-22  7:41 [RFC RESEND PATCH v2 0/8] Add managed version of delayed work init Matti Vaittinen
2021-03-22  7:36 ` [RFC RESEND PATCH v2 1/8] workqueue: Add resource " Matti Vaittinen
2021-03-22  7:36 ` [RFC RESEND PATCH v2 2/8] MAINTAINERS: Add entry for devm helpers Matti Vaittinen
2021-03-22  7:37 ` [RFC RESEND PATCH v2 3/8] extconn: Clean-up few drivers by using managed work init Matti Vaittinen
2021-03-22  7:37 ` [RFC RESEND PATCH v2 4/8] hwmon: raspberry-pi: " Matti Vaittinen
2021-03-22 14:05   ` Guenter Roeck
2021-03-22  7:37 ` [RFC RESEND PATCH v2 5/8] platform/x86: gpd pocket fan: Clean-up " Matti Vaittinen
2021-03-22  7:37 ` [RFC RESEND PATCH v2 6/8] power: supply: Clean-up few drivers " Matti Vaittinen
2021-03-22 14:08   ` Chen-Yu Tsai
2021-03-22  7:38 ` [RFC RESEND PATCH v2 7/8] regulator: qcom_spmi-regulator: Clean-up " Matti Vaittinen
2021-03-22  7:38 ` [RFC RESEND PATCH v2 8/8] watchdog: retu_wdt: " Matti Vaittinen
2021-03-22 14:05   ` Guenter Roeck
2021-03-23 12:43 ` [RFC RESEND PATCH v2 0/8] Add managed version of delayed " Greg KH
2021-03-23 13:16   ` Vaittinen, Matti

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).