| Message ID | b1030eddbf0069f2d39e951be1d8e40d6413aeeb.1616506559.git.matti.vaittinen@fi.rohmeurope.com |
|---|---|
| State | Accepted |
| Commit | f94a5becabf43e17490aded8bddc5f924b00338b |
| Headers | show |
| Series | Add managed version of delayed work init | expand |
Hi, On 3/23/21 2:57 PM, 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. > > 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> Thanks, patch looks good to me: Reviewed-by: Hans de Goede <hdegoede@redhat.com> Note that this is not just a cleanup, it also actually fixes a couple of driver-unbind races where the work was stopped before the IRQ is free-ed, so there is a race where the work can be restarted. Regards, Hans > --- > Changelog from RFCv2: > - RFC dropped. No functional changes. > > 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, >
Hi, Need to fix the work as following: s/extconn/extcon And I'd like you to use the more correct patch title like the following example: "extcon: Use resource-managed function for delayed work" Thanks, Chanwoo Choi On 3/23/21 10:57 PM, 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. > > 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> > --- > Changelog from RFCv2: > - RFC dropped. No functional changes. > > 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; Need to add the error log as following: if (ret) { dev_err(dev, "Failed to initialize delayed_work"); 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, >
Hello Chanwoo, Greg, Thanks for the review. On Wed, 2021-03-24 at 11:09 +0900, Chanwoo Choi wrote: > Hi, > > Need to fix the work as following: > s/extconn/extcon > > And I'd like you to use the more correct patch title like the > following example: > "extcon: Use resource-managed function for delayed work" I think Greg merged this already. How should we handle this? > @@ -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; > > Need to add the error log as following: > if (ret) { > dev_err(dev, "Failed to initialize delayed_work"); > return ret; > } I could send incremental patch to Greg for this but it does not change the commit message. Best Regards Matti Vaittinen
Hi, On 3/24/21 6:02 AM, Matti Vaittinen wrote: > Hello Chanwoo, Greg, > > Thanks for the review. > > On Wed, 2021-03-24 at 11:09 +0900, Chanwoo Choi wrote: >> Hi, >> >> Need to fix the work as following: >> s/extconn/extcon >> >> And I'd like you to use the more correct patch title like the >> following example: >> "extcon: Use resource-managed function for delayed work" > > I think Greg merged this already. How should we handle this? > >> @@ -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; >> >> Need to add the error log as following: >> if (ret) { >> dev_err(dev, "Failed to initialize delayed_work"); >> return ret; >> } > > I could send incremental patch to Greg for this but it does not change > the commit message. We cannot do anything about the commit message anymore, but the ordering issue which you introduced really needs to be fixed. Please send an incremental patch fixing the wrong order and the double init of the workqueue. Regards, Hans
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,
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> --- Changelog from RFCv2: - RFC dropped. No functional changes. 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(-)