Skip to contentSkip to conversation

notifier: convert remaining drivers to device-managed registration

PATCH Applied Eliav Farber 4 participants 9 messages Nov 13

Subsystems (9)
ACPI HID GPIO Input drivers +5 more subsystems
lore
Share & tools
lore
#0 lore raw

v6 cover notifier: convert remaining drivers to device-managed registration

The devm_atomic_notifier_chain_register() and devm_blocking_notifier_chain_register() helpers this series added in v5 are now in mainline, together with 6 of the 11 driver conversions. This revision carries the 6 conversions that were never picked up, plus one new conversion for a driver that grew the old pattern after v5 was posted.

Background, unchanged from v5. Many drivers repeat the same boilerplate when registering notifiers with device lifetime:

1. Register the notifier with *_notifier_chain_register()
2. Check for error
3. Register a devm action to unregister on teardown
4. Implement a per-driver static unregister callback

The devm_*_notifier_chain_register() helpers reduce that to a single call with one error path, eliminating the per-driver unregister callback. Each conversion drops ~15 lines; this revision removes 117 lines and adds 18.

Already in mainline, so dropped from this revision:

  v5       commit        subject
  1/13     e38b9eda475f  notifier: add device-managed registration APIs
  2/13     6b0c6ff76795  pwm: iqs620a
  3/13     8c77589a7274  iio: light: iqs621-als
  4/13     c9f54ead5793  iio: position: iqs624
  5/13     870f46a42afd  gpio: adp5585
  11/13    323906906101  gpio: eic-sprd
  12/13    df6501bf6fc4  gpio: gpiolib-kunit

What is left, with the v5 numbering for anyone who reviewed it then:

  v6   v5      subject
  1/7  6/13    platform/x86: bitland-mifs-wmi
  2/7  7/13    Input: adp5585
  3/7  8/13    ACPI: APEI: GHES: remove unused ghes_{,un}register_...
  4/7  9/13    ACPI: APEI: GHES: use devm_...
  5/7  10/13   platform/x86: uniwill-wmi
  6/7  13/13   reboot
  7/7  -       power: supply: cros_pchg (new in v6)

The patches are independent and can go through their own trees. The only ordering constraint is that patch 3 must precede patch 4: patch 3 removes the unused exports, patch 4 converts what is left. The devm helpers are already upstream, so unlike v5 this revision needs no base commit or pull request to build.

Suggested routing, since the series now spans five trees:

  1/7, 5/7  platform-drivers-x86
  2/7       input
  3/7, 4/7  acpi, in this order
  6/7       kernel/reboot.c has no MAINTAINERS entry, so it falls to

THE REST
7/7 power-supply

Each patch is Cc'd to its own subsystem lists only, so no list receives patches it does not own. linux-kernel receives all of them.

v5: https://lore.kernel.org/all/[email protected]/

Changes in v6:
- Dropped the 7 patches now in mainline, listed above.
- Rebased onto v7.4-rc1. No conflicts; the 6 carried patches are functionally unchanged from v5.
- Patch 3: replaced an em dash in the commit message with an ASCII hyphen. No change to the code.
- Patch 6: the commit message now records that the removed callback's WARN_ON() on a failed unregister goes away, because devm_blocking_notifier_chain_register() ignores the unregister return value. No in-tree caller can trigger it. No change to the code.
- Patch 7: new. cros_pchg grew the blocking_notifier_chain_register() + devm_add_action_or_reset() pattern in 631adfa03595 ("power: supply: cros_pchg: unregister EC notifier"), which reached mainline in v7.3-rc1, five days after v5 was posted.
- Carried forward the Acked-by and Reviewed-by tags given on v1 and v4. Each was checked against the patch it was given for: all four are byte-identical to the version here.

Changes in v5:
- Patch 1: rename notifier_block parameter from 'n' to 'nb' to match the devres struct field name (Uwe Kleine-König)
- Patch 2: remove extra blank line left after deleting the unregister callback (Uwe Kleine-König)

Changes in v4:
- Patch 8: split the removal of unused ghes_register_vendor_record_notifier() and ghes_unregister_vendor_record_notifier() into a separate preceding patch (Andy Shevchenko)

Changes in v3:
- Patch 1: drop devm_raw_notifier_chain_register() since raw notifiers require caller-provided locking which is incompatible with the devres teardown callback (Sashiko)
- Patch 12: fix commit message to accurately describe the old code as using devres_alloc() + register_reboot_notifier() (Sashiko)

Changes in v2:
- Patch 1: drop 'extern' from new function prototypes (Bart Van Assche)
- Patch 1: fix kerneldoc to use 'Return:' format (Bart Van Assche)
- Patch 1: use <linux/device/devres.h> instead of <linux/device.h> (Andy Shevchenko)
- Patch 8: also remove unused ghes_register_vendor_record_notifier() and ghes_unregister_vendor_record_notifier() along with their exports and ghes.h declarations (Jonathan Cameron)

Eliav Farber (7):
platform/x86: bitland-mifs-wmi: use
devm_blocking_notifier_chain_register()
Input: adp5585: use devm_blocking_notifier_chain_register()
ACPI: APEI: GHES: remove unused
ghes_{,un}register_vendor_record_notifier()
ACPI: APEI: GHES: use devm_blocking_notifier_chain_register() platform/x86: uniwill-wmi: use devm_blocking_notifier_chain_register()
reboot: use devm_blocking_notifier_chain_register()
power: supply: cros_pchg: use devm_blocking_notifier_chain_register()

 drivers/acpi/apei/ghes.c                      | 27 +++----------------
 drivers/input/keyboard/adp5585-keys.c         | 18 +++----------
 drivers/platform/x86/bitland-mifs-wmi.c       | 17 +++---------
 drivers/platform/x86/uniwill/uniwill-wmi.c    | 17 +++---------
 .../power/supply/cros_peripheral_charger.c    | 16 +++--------
 include/acpi/ghes.h                           | 16 -----------
 kernel/reboot.c                               | 24 ++---------------
 7 files changed, 18 insertions(+), 117 deletions(-)

base-commit: 067c5c24cef23d913b67eaaf71ae67e7169a6620

Signature · 1 line
2.47.3
#1 lore raw

PATCH v6 1/7 platform/x86: bitland-mifs-wmi: use devm_blocking_notifier_chain_register()

Replace the blocking_notifier_chain_register() + devm_add_action_or_reset() pattern with a single call to devm_blocking_notifier_chain_register(), removing the bitland_notifier_unregister() callback.

---
Changes in v6:
- Rebased onto v7.4-rc1. No functional change since v5.

 drivers/platform/x86/bitland-mifs-wmi.c | 17 +++--------------
 1 file changed, 3 insertions(+), 14 deletions(-)
drivers/platform/x86/bitland-mifs-wmi.c +3 -14
index 6dd901e54d9d..617c369c63c5 100644
@@ -621,13 +621,6 @@ static const struct key_entry bitland_mifs_wmi_keymap[] = {
 	{ KE_END, 0 }
 };
 
-static void bitland_notifier_unregister(void *data)
-{
-	struct notifier_block *nb = data;
-
-	blocking_notifier_chain_unregister(&bitland_notifier_list, nb);
-}
-
 static int bitland_notifier_callback(struct notifier_block *nb,
 				     unsigned long action, void *data)
 {
@@ -727,13 +720,9 @@ static int bitland_mifs_wmi_probe(struct wmi_device *wdev, const void *context)
 		return ret;
 
 	drv_data->notifier.notifier_call = bitland_notifier_callback;
-	ret = blocking_notifier_chain_register(&bitland_notifier_list, &drv_data->notifier);
-	if (ret)
-		return ret;
-
-	return devm_add_action_or_reset(&wdev->dev,
-				       bitland_notifier_unregister,
-				       &drv_data->notifier);
+	return devm_blocking_notifier_chain_register(&wdev->dev,
+						     &bitland_notifier_list,
+						     &drv_data->notifier);
 }
 
 static void bitland_mifs_wmi_notify(struct wmi_device *wdev,
Signature · 1 line
2.47.3
#2 lore raw

PATCH v6 2/7 Input: adp5585: use devm_blocking_notifier_chain_register()

In reply to Eliav Farber

Replace the blocking_notifier_chain_register() + devm_add_action_or_reset() pattern with a single call to devm_blocking_notifier_chain_register(), removing the adp5585_keys_unreg_notifier() callback.

---
Changes in v6:
- Rebased onto v7.4-rc1. No functional change since v5.
- Dmitry Torokhov's Acked-by retained; the patch is unchanged since v1, when it was given.

 drivers/input/keyboard/adp5585-keys.c | 18 +++---------------
 1 file changed, 3 insertions(+), 15 deletions(-)
drivers/input/keyboard/adp5585-keys.c +3 -15
index ee2c3cb0445f..b27390d0e706 100644
@@ -254,15 +254,6 @@ static int adp5585_keys_ev_handle(struct notifier_block *nb, unsigned long key,
 	return NOTIFY_STOP;
 }
 
-static void adp5585_keys_unreg_notifier(void *data)
-{
-	struct adp5585_kpad *kpad = data;
-	struct adp5585_dev *adp5585 = dev_get_drvdata(kpad->dev->parent);
-
-	blocking_notifier_chain_unregister(&adp5585->event_notifier,
-					   &kpad->nb);
-}
-
 static int adp5585_keys_probe(struct platform_device *pdev)
 {
 	const struct platform_device_id *id = platform_get_device_id(pdev);
@@ -318,12 +309,9 @@ static int adp5585_keys_probe(struct platform_device *pdev)
 		return error;
 
 	kpad->nb.notifier_call = adp5585_keys_ev_handle;
-	error = blocking_notifier_chain_register(&adp5585->event_notifier,
-						 &kpad->nb);
-	if (error)
-		return error;
-
-	error = devm_add_action_or_reset(dev, adp5585_keys_unreg_notifier, kpad);
+	error = devm_blocking_notifier_chain_register(dev,
+						      &adp5585->event_notifier,
+						      &kpad->nb);
 	if (error)
 		return error;
 
Signature · 1 line
2.47.3
maintainer Applied
#3 lore raw

Re: [PATCH v6 2/7] Input: adp5585: use devm_blocking_notifier_chain_register()

Quoted 7 lines · Eliav Farber

Replace the blocking_notifier_chain_register() + devm_add_action_or_reset() pattern with a single call to devm_blocking_notifier_chain_register(), removing the adp5585_keys_unreg_notifier() callback.

Applied, thank you.

Signature · 1 line
Dmitry
#4 lore raw

PATCH v6 3/7 ACPI: APEI: GHES: remove unused ghes_{,un}register_vendor_record_notifier()

In reply to Eliav Farber

Remove ghes_register_vendor_record_notifier() and ghes_unregister_vendor_record_notifier() along with their EXPORT_SYMBOL_GPL()s and ghes.h declarations, since there are no remaining in-tree callers - all users go through devm_ghes_register_vendor_record_notifier() instead.

Inline the register/unregister calls directly into devm_ghes_register_vendor_record_notifier() and its destroy callback.

---
Changes in v6:
- Rebased onto v7.4-rc1.
- Replaced an em dash in the commit message with an ASCII hyphen. No change to the code.
- Jonathan Cameron's Reviewed-by retained; the code is unchanged since v4, when it was given.

Changes in v4:
- Split from the devm conversion patch into its own commit (Andy Shevchenko)

Changes in v2:
- New patch: remove unused ghes_register_vendor_record_notifier() and ghes_unregister_vendor_record_notifier() along with their EXPORT_SYMBOL_GPL()s and ghes.h declarations, since there are no in-tree callers (Jonathan Cameron)

 drivers/acpi/apei/ghes.c | 16 +++-------------
 include/acpi/ghes.h      | 16 ----------------
 2 files changed, 3 insertions(+), 29 deletions(-)
drivers/acpi/apei/ghes.c +3 -13
index 663dc2e601cf..a81ffacb16d0 100644
@@ -679,29 +679,19 @@ static void ghes_handle_aer(struct acpi_hest_generic_data *gdata)
 
 static BLOCKING_NOTIFIER_HEAD(vendor_record_notify_list);
 
-int ghes_register_vendor_record_notifier(struct notifier_block *nb)
+static void ghes_vendor_record_notifier_destroy(void *data)
 {
-	return blocking_notifier_chain_register(&vendor_record_notify_list, nb);
-}
-EXPORT_SYMBOL_GPL(ghes_register_vendor_record_notifier);
+	struct notifier_block *nb = data;
 
-void ghes_unregister_vendor_record_notifier(struct notifier_block *nb)
-{
 	blocking_notifier_chain_unregister(&vendor_record_notify_list, nb);
 }
-EXPORT_SYMBOL_GPL(ghes_unregister_vendor_record_notifier);
-
-static void ghes_vendor_record_notifier_destroy(void *nb)
-{
-	ghes_unregister_vendor_record_notifier(nb);
-}
 
 int devm_ghes_register_vendor_record_notifier(struct device *dev,
 					      struct notifier_block *nb)
 {
 	int ret;
 
-	ret = ghes_register_vendor_record_notifier(nb);
+	ret = blocking_notifier_chain_register(&vendor_record_notify_list, nb);
 	if (ret)
 		return ret;
 
include/acpi/ghes.h +0 -16
index 7acf209061ea..b01b3f6d0868 100644
@@ -55,22 +55,6 @@ enum {
 };
 
 #ifdef CONFIG_ACPI_APEI_GHES
-/**
- * ghes_register_vendor_record_notifier - register a notifier for vendor
- * records that the kernel would otherwise ignore.
- * @nb: pointer to the notifier_block structure of the event handler.
- *
- * return 0 : SUCCESS, non-zero : FAIL
- */
-int ghes_register_vendor_record_notifier(struct notifier_block *nb);
-
-/**
- * ghes_unregister_vendor_record_notifier - unregister the previously
- * registered vendor record notifier.
- * @nb: pointer to the notifier_block structure of the vendor record handler.
- */
-void ghes_unregister_vendor_record_notifier(struct notifier_block *nb);
-
 /**
  * devm_ghes_register_vendor_record_notifier - device-managed vendor
  * record notifier registration.
Signature · 1 line
2.47.3
#5 lore raw

Re: [PATCH v6 3/7] ACPI: APEI: GHES: remove unused ghes_{,un}register_vendor_record_notifier()

Quoted 9 lines · Eliav Farber

Remove ghes_register_vendor_record_notifier() and ghes_unregister_vendor_record_notifier() along with their EXPORT_SYMBOL_GPL()s and ghes.h declarations, since there are no remaining in-tree callers - all users go through devm_ghes_register_vendor_record_notifier() instead.

Inline the register/unregister calls directly into devm_ghes_register_vendor_record_notifier() and its destroy callback.

I'm just wondering, if you use --histogram diff algo when preparing patches. I think it might look better for reading with that being provided.

Signature · 2 lines
With Best Regards,
Andy Shevchenko
#6 lore raw

PATCH v6 4/7 ACPI: APEI: GHES: use devm_blocking_notifier_chain_register()

In reply to Eliav Farber

Replace the blocking_notifier_chain_register() + devm_add_action_or_reset() pattern with a single call to devm_blocking_notifier_chain_register(), removing the ghes_vendor_record_notifier_destroy() callback.

---
Changes in v6:
- Rebased onto v7.4-rc1. No functional change since v4.
- Jonathan Cameron's Reviewed-by retained; the code is unchanged since v4, when it was given.

Changes in v4:
- Split removal of unused code into a separate preceding patch (Andy Shevchenko)

 drivers/acpi/apei/ghes.c | 17 +++--------------
 1 file changed, 3 insertions(+), 14 deletions(-)
drivers/acpi/apei/ghes.c +3 -14
index a81ffacb16d0..92184ced12b8 100644
@@ -679,23 +679,12 @@ static void ghes_handle_aer(struct acpi_hest_generic_data *gdata)
 
 static BLOCKING_NOTIFIER_HEAD(vendor_record_notify_list);
 
-static void ghes_vendor_record_notifier_destroy(void *data)
-{
-	struct notifier_block *nb = data;
-
-	blocking_notifier_chain_unregister(&vendor_record_notify_list, nb);
-}
-
 int devm_ghes_register_vendor_record_notifier(struct device *dev,
 					      struct notifier_block *nb)
 {
-	int ret;
-
-	ret = blocking_notifier_chain_register(&vendor_record_notify_list, nb);
-	if (ret)
-		return ret;
-
-	return devm_add_action_or_reset(dev, ghes_vendor_record_notifier_destroy, nb);
+	return devm_blocking_notifier_chain_register(dev,
+						     &vendor_record_notify_list,
+						     nb);
 }
 EXPORT_SYMBOL_GPL(devm_ghes_register_vendor_record_notifier);
 
Signature · 1 line
2.47.3
#7 lore raw
In reply to Eliav Farber

Patches 5/7, 6/7 and 7/7 are not in this thread. My mail server refused the second half of the series with a submission rate limit, and the resend went out as a separate thread:

5/7 https://lore.kernel.org/all/[email protected]/
6/7 https://lore.kernel.org/all/[email protected]/
7/7 https://lore.kernel.org/all/[email protected]/

All seven patches were delivered, so I am not resending them. Sorry for the split thread.

--
Thanks, Eliav

maintainer
#8 lore raw
In reply to Eliav Farber
Quoted 8 lines · Eliav Farber

The devm_atomic_notifier_chain_register() and devm_blocking_notifier_chain_register() helpers this series added in v5 are now in mainline, together with 6 of the 11 driver conversions. This revision carries the 6 conversions that were never picked up, plus one new conversion for a driver that grew the old pattern after v5 was posted.

[...]

Thank you for your contribution, it has been applied to my local review-ilpo-next branch. Note it will show up in the public platform-drivers-x86/review-ilpo-next branch only once I've pushed my local branch there, which might take a while.

FYI [if applicable to your patch], as per Linus' policy change, also fixes are mostly routed through for-next unless the fix is for a commit introduced in the most recent cycle or is clearly a regression fix.

The list of commits applied:
[1/7] platform/x86: bitland-mifs-wmi: use devm_blocking_notifier_chain_register()

      commit: a362539b42193dcdb17f8fba40031a515c53885c

[2/7] Input: adp5585: use devm_blocking_notifier_chain_register()
(no commit info)
[3/7] ACPI: APEI: GHES: remove unused ghes_{,un}register_vendor_record_notifier()
(no commit info)
[4/7] ACPI: APEI: GHES: use devm_blocking_notifier_chain_register()
(no commit info)

--
i.