Date: Mon, 27 May 2024 10:34:15 +0200
From: Uwe Kleine-König <u.kleine-koenig@...gutronix.de>
To: Greg Kroah-Hartman <gregkh@...uxfoundation.org>
Cc: "Rafael J. Wysocki" <rafael@...nel.org>,
	Mark Brown <broonie@...nel.org>,
	Takashi Iwai <tiwai@...e.de>,
	Uwe Kleine-König <uwe@...ine-koenig.org>,
	David Gow <davidgow@...gle.com>,
	linux-kernel@...r.kernel.org,
	kernel@...gutronix.de,
	Philipp Zabel <p.zabel@...gutronix.de>,
	Nishanth Menon <nm@...com>,
	Jeremy Kerr <jk@...abs.org>,
	Rodolfo Giometti <giometti@...eenne.com>,
	Thierry Reding <treding@...dia.com>,
	Matt Coster <matt.coster@...tec.com>,
	Thomas Zimmermann <tzimmermann@...e.de>,
	Dave Jiang <dave.jiang@...el.com>,
	Dan Williams <dan.j.williams@...el.com>,
	Vishal Verma <vishal.l.verma@...el.com>,
	Bjorn Andersson <andersson@...nel.org>
Subject: [PATCH] platform: Make platform_driver::remove() return void

struct platform_driver::remove returning an integer made driver authors
expect that returning an error code was proper error handling. However
the driver core ignores the error and continues to remove the device
because there is nothing the core could do anyhow and reentering the
remove callback again is only calling for trouble.

To prevent such wrong assumptions, change the return type of the remove
callback to void. This was prepared by introducing an alternative remove
callback returning void and converting all drivers to that. So .remove()
can be changed without further changes in drivers.

This corresponds to step b) of the plan outlined in commit
5c5a7680e67b ("platform: Provide a remove callback that returns no value").

Signed-off-by: Uwe Kleine-König <u.kleine-koenig@...gutronix.de>
---
Hello Greg,

there are only very little platform drivers left in v6.10-rc1 that need
to be changed to .remove_new() before this patch can be applied. They
were all sent out to the respective maintainers, most of them suggested
to apply the patches together with this one.

You can fetch this patch together with all necessary commits from:

	https://git.kernel.org/pub/scm/linux/kernel/git/ukleinek/linux.git platform-remove-void

If you have no concerns, I can also provide you a signed tag for
pulling. I think that's easier than indiviually applying them, but I can
also send out the complete series if you prefer.

The overall shortlog looks as follows:

Uwe Kleine-König (18):
      reset: meson-audio-arb: Convert to platform remove callback returning void
      reset: rzg2l-usbphy-ctrl: Convert to platform remove callback returning void
      reset: ti-sci: Convert to platform remove callback returning void
      Merge branch 'reset/next' of git://git.pengutronix.de/pza/linux
      fsi: master-aspeed: Convert to platform remove callback returning void
      fsi: master-ast-cf: Convert to platform remove callback returning void
      fsi: master-gpio: Convert to platform remove callback returning void
      fsi: occ: Convert to platform remove callback returning void
      pps: clients: gpio: Convert to platform remove callback returning void
      gpu: host1x: mipi: Benefit from devm_clk_get_prepared()
      drm/imagination: Convert to platform remove callback returning void
      drm/mediatek: Convert to platform remove callback returning void
      gpu: host1x: Convert to platform remove callback returning void
      gpu: ipu-v3: Convert to platform remove callback returning void
      nvdimm/e820: Convert to platform remove callback returning void
      nvdimm/of_pmem: Convert to platform remove callback returning void
      samples: qmi: Convert to platform remove callback returning void
      platform: Make platform_driver::remove() return void

The patches to drivers/reset were applied for next, but Philipp didn't
come around to care for them entering v6.10-rc1. I merged them on top of
v6.10 with his consent.

For the samples patch I didn't receive any feedback even after two
pings. Also the nvdimm patches didn't get a maintainer blessing. The
others are fine.

I'd like to have this in next early now. I don't expect any fallouts
(allmodconfig builds done for arm64, riscv, m68k, x86_64 without
problems, mips failed for unrelated reasons) but it might affect new
code entering in the upcoming cycle.

Please tell me how you want to handle that, I'll do the necessary
preparations then.

Best regards
Uwe

 drivers/base/platform.c         | 10 ++--------
 include/linux/platform_device.h | 15 +++++++--------
 2 files changed, 9 insertions(+), 16 deletions(-)

diff --git a/drivers/base/platform.c b/drivers/base/platform.c
index 10c577963418..c8aa1be70526 100644
--- a/drivers/base/platform.c
+++ b/drivers/base/platform.c
@@ -1420,14 +1420,8 @@ static void platform_remove(struct device *_dev)
 	struct platform_driver *drv = to_platform_driver(_dev->driver);
 	struct platform_device *dev = to_platform_device(_dev);
 
-	if (drv->remove_new) {
-		drv->remove_new(dev);
-	} else if (drv->remove) {
-		int ret = drv->remove(dev);
-
-		if (ret)
-			dev_warn(_dev, "remove callback returned a non-zero value. This will be ignored.\n");
-	}
+	if (drv->remove)
+		drv->remove(dev);
 	dev_pm_domain_detach(_dev, true);
 }
 
diff --git a/include/linux/platform_device.h b/include/linux/platform_device.h
index 7a41c72c1959..d422db6eec63 100644
--- a/include/linux/platform_device.h
+++ b/include/linux/platform_device.h
@@ -237,15 +237,14 @@ struct platform_driver {
 	int (*probe)(struct platform_device *);
 
 	/*
-	 * Traditionally the remove callback returned an int which however is
-	 * ignored by the driver core. This led to wrong expectations by driver
-	 * authors who thought returning an error code was a valid error
-	 * handling strategy. To convert to a callback returning void, new
-	 * drivers should implement .remove_new() until the conversion it done
-	 * that eventually makes .remove() return void.
+	 * .remove_new() is a relic from a prototype conversion of .remove().
+	 * New drivers are supposed to implement .remove(). Once all drivers are
+	 * converted to not use .remove_new any more, it will be dropped.
 	 */
-	int (*remove)(struct platform_device *);
-	void (*remove_new)(struct platform_device *);
+	union {
+		void (*remove)(struct platform_device *);
+		void (*remove_new)(struct platform_device *);
+	};
 
 	void (*shutdown)(struct platform_device *);
 	int (*suspend)(struct platform_device *, pm_message_t state);

base-commit: 1613e604df0cd359cf2a7fbd9be7a0bcfacfabd0
prerequisite-patch-id: 34124516ca38cd2c66a8a871c499ce3ae96e09ab
prerequisite-patch-id: 8ea26f3437cebce13a4975cef291db6a2e15bb93
prerequisite-patch-id: ee4b519fc5515807ae72877467e5f1c83c93d19a
prerequisite-patch-id: 79e7647450348ef0cca53af2bc5e5a0033dcec57
prerequisite-patch-id: 45568b19e28a0cb3f46699ff945adc654eda07d2
prerequisite-patch-id: 7daf4547e4b3785959e0c024ce95141cd1d936da
prerequisite-patch-id: f3c1f5b72e3b503760d4b70bb661ceb8381c4822
prerequisite-patch-id: b63f367f8354d56916d33c4236c79cf8e1c7d67a
prerequisite-patch-id: 4b8d2995e96ae290599d752cf1c1d2537e47bfab
prerequisite-patch-id: 5915e9d3dd78f832ab0017c81df770f443b32169
prerequisite-patch-id: 5626ccc5644f4edf610c083f2ff7c1308c6aaf27
prerequisite-patch-id: 712d0c765bdecbc60d72e62b10b329b525dfd16f
prerequisite-patch-id: 85e037f468c34b7a904f926e0cc555d7863c2cc7
prerequisite-patch-id: 48554c57f1d06553af3ded861ebf9b88ee6b03c4
prerequisite-patch-id: c26315d8be3a746b04dea921d41c903d054b774f
prerequisite-patch-id: 8eca4420a223355531ddfc5331729feb5fff9812

-- 
2.43.0

Powered by blists - more mailing lists