| Message ID | cover.1613216412.git.matti.vaittinen@fi.rohmeurope.com (mailing list archive) |
|---|---|
| Headers | show |
| Series | Add managed version of delayed work init | expand |
On Sat, Feb 13, 2021 at 01:58:17PM +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. > > Often this means that driver must ensure the work is not queued when > driver exits. Usually this 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. > > It might be helpful for (a) few drivers if there was a work init why the (a) and not just a? > function which would ensure cancel_delayed_work_sync() is called at > driver exit. So this series implements one 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. > > Main reson why this is RFC is that I had hard time deciding where this > function should be introduced. It's not nice to include all device stuff > in workqueue - because many workqueue users are not interested in > devices. In same way, not all of the devices are interested in WQs. > OTOH, adding own file just for this sounds like an overkill. s/own/one --mark > > This time I decided that it is more correct that devices use WQs than > that WQs use devices. Hence the function is introduced in > include/linux/device.h and drivers/base/devres.c > > -- > > Matti Vaittinen (7): > drivers: base: Add resource managed version of delayed work init > 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 > > drivers/base/devres.c | 33 ++++++++++++++++++++ > drivers/extcon/extcon-gpio.c | 14 ++------- > drivers/extcon/extcon-intel-int3496.c | 15 ++------- > drivers/extcon/extcon-palmas.c | 16 +++------- > drivers/extcon/extcon-qcom-spmi-misc.c | 16 +++------- > drivers/hwmon/raspberrypi-hwmon.c | 16 +++------- > drivers/platform/x86/gpd-pocket-fan.c | 16 +++------- > drivers/power/supply/axp20x_usb_power.c | 15 +++------ > drivers/power/supply/bq24735-charger.c | 17 +++------- > drivers/power/supply/ltc2941-battery-gauge.c | 19 ++++------- > drivers/power/supply/sbs-battery.c | 15 +++------ > drivers/regulator/qcom_spmi-regulator.c | 33 +++++--------------- > drivers/watchdog/retu_wdt.c | 21 +++---------- > include/linux/device.h | 5 +++ > 14 files changed, 95 insertions(+), 156 deletions(-) > > > base-commit: 92bf22614b21a2706f4993b278017e437f7785b3 > -- > 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 =]
Hello Mark, Thanks for taking a look at the series! This is the first time anyone has been commenting on a cover-letter which is likely to fade away and never be looked at again. Guess you are a thorough person :) On Thu, 2021-02-18 at 08:28 -0800, mark gross wrote: > On Sat, Feb 13, 2021 at 01:58:17PM +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. > > > > Often this means that driver must ensure the work is not queued > > when > > driver exits. Usually this 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. > > > > It might be helpful for (a) few drivers if there was a work init > why the (a) and not just a? I am not sure how many drivers are needed to change it from 'few' to 'a few'. Additionally, this series converted only the drivers which I found could easily get rid of the .remove() - I did not analyze how many drivers would benefit from this by getting rid of mixed devm/manual resource management. So to sum up - I don't know how many drivers will benefit and what people think makes 'few' to turn to 'a few'. '(a) few' leaves this decision to readers - and (a) few of them know the drivers better than I do. > > Main reson why this is RFC is that I had hard time deciding where > > this > > function should be introduced. It's not nice to include all device > > stuff > > in workqueue - because many workqueue users are not interested in > > devices. In same way, not all of the devices are interested in WQs. > > OTOH, adding own file just for this sounds like an overkill. > s/own/one Hm. The 'own file for XXX' does not make sense for native English speakers? Didn't now that. Thanks for pointing it out. I will edit the cover letter when I respin this rebased on v5.12-rc1 - and it is likely the series v2 will add this function inlined in a new header dedicated for devm-helpers (as was suggested by Hans de Goede). Best Regards --Matti