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 <[email protected]>
---
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
On Mon, May 27, 2024 at 10:34:15AM +0200, Uwe Kleine-K?nig wrote:
> 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 <[email protected]>
> ---
> 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.
A signed tag is good, I can just pull from that, thanks!
greg k-h
Hello Greg
On Tue, Jun 04, 2024 at 06:13:54PM +0200, Greg Kroah-Hartman wrote:
> On Mon, May 27, 2024 at 10:34:15AM +0200, Uwe Kleine-K?nig wrote:
> > 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.
>
> A signed tag is good, I can just pull from that, thanks!
here it comes:
The following changes since commit 1613e604df0cd359cf2a7fbd9be7a0bcfacfabd0:
Linux 6.10-rc1 (2024-05-26 15:20:12 -0700)
are available in the Git repository at:
https://git.kernel.org/pub/scm/linux/kernel/git/ukleinek/linux.git tags/platform-remove-void-step-b
for you to fetch changes up to 0edb555a65d1ef047a9805051c36922b52a38a9d:
platform: Make platform_driver::remove() return void (2024-05-27 10:34:35 +0200)
----------------------------------------------------------------
Change struct platform_driver::remove() to return void
This is step b) of the plan outlined in commit 5c5a7680e67b ("platform:
Provide a remove callback that returns no value"), which completes the
first major step of making the remove callback return no value. Up to
now it returned an int which however was mostly ignored by the driver
core and lured driver authors to believe there is some error handling.
Note that the Linux driver model assumes that removing a device cannot
fail, so this isn't about being lazy and not implementing error handling
in the core and so making .remove return void is the right thing to do.
----------------------------------------------------------------
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
drivers/base/platform.c | 10 ++--------
drivers/fsi/fsi-master-aspeed.c | 6 ++----
drivers/fsi/fsi-master-ast-cf.c | 6 ++----
drivers/fsi/fsi-master-gpio.c | 6 ++----
drivers/fsi/fsi-occ.c | 6 ++----
drivers/gpu/drm/imagination/pvr_drv.c | 7 ++-----
drivers/gpu/drm/mediatek/mtk_padding.c | 5 ++---
drivers/gpu/host1x/dev.c | 6 ++----
drivers/gpu/host1x/mipi.c | 17 +----------------
drivers/gpu/ipu-v3/ipu-common.c | 6 ++----
drivers/gpu/ipu-v3/ipu-pre.c | 5 ++---
drivers/gpu/ipu-v3/ipu-prg.c | 6 ++----
drivers/nvdimm/e820.c | 5 ++---
drivers/nvdimm/of_pmem.c | 6 ++----
drivers/pps/clients/pps-gpio.c | 5 ++---
drivers/reset/reset-meson-audio-arb.c | 6 ++----
drivers/reset/reset-rzg2l-usbphy-ctrl.c | 6 ++----
drivers/reset/reset-ti-sci.c | 6 ++----
include/linux/platform_device.h | 15 +++++++--------
samples/qmi/qmi_sample_client.c | 6 ++----
20 files changed, 44 insertions(+), 97 deletions(-)
Thanks for considering this,
Uwe
On Wed, Jun 05, 2024 at 12:33:11PM +0200, Uwe Kleine-K?nig wrote:
> Hello Greg
>
> On Tue, Jun 04, 2024 at 06:13:54PM +0200, Greg Kroah-Hartman wrote:
> > On Mon, May 27, 2024 at 10:34:15AM +0200, Uwe Kleine-K?nig wrote:
> > > 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.
> >
> > A signed tag is good, I can just pull from that, thanks!
>
> here it comes:
>
> The following changes since commit 1613e604df0cd359cf2a7fbd9be7a0bcfacfabd0:
>
> Linux 6.10-rc1 (2024-05-26 15:20:12 -0700)
>
> are available in the Git repository at:
>
> https://git.kernel.org/pub/scm/linux/kernel/git/ukleinek/linux.git tags/platform-remove-void-step-b
Pulled and pushed out, thanks!
greg k-h