Skip to content

Commit 5c5a768

Browse files
Uwe Kleine-Königgregkh
Uwe Kleine-König
authored andcommitted
platform: Provide a remove callback that returns no value
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. So this is an source for errors typically yielding resource leaks in the error path. As there are too many platform drivers to neatly convert them all to return void in a single go, do it in several steps after this patch: a) Convert all drivers to implement .remove_new() returning void instead of .remove() returning int; b) Change struct platform_driver::remove() to return void and so make it identical to .remove_new(); c) Change all drivers back to .remove() now with the better prototype; d) drop struct platform_driver::remove_new(). While this touches all drivers eventually twice, steps a) and c) can be done one driver after another and so reduces coordination efforts immensely and simplifies review. Signed-off-by: Uwe Kleine-König <[email protected]> Link: https://lore.kernel.org/r/[email protected] Signed-off-by: Greg Kroah-Hartman <[email protected]>
1 parent 7bbb89b commit 5c5a768

File tree

2 files changed

+14
-1
lines changed

2 files changed

+14
-1
lines changed

drivers/base/platform.c

+3-1
Original file line numberDiff line numberDiff line change
@@ -1416,7 +1416,9 @@ static void platform_remove(struct device *_dev)
14161416
struct platform_driver *drv = to_platform_driver(_dev->driver);
14171417
struct platform_device *dev = to_platform_device(_dev);
14181418

1419-
if (drv->remove) {
1419+
if (drv->remove_new) {
1420+
drv->remove_new(dev);
1421+
} else if (drv->remove) {
14201422
int ret = drv->remove(dev);
14211423

14221424
if (ret)

include/linux/platform_device.h

+11
Original file line numberDiff line numberDiff line change
@@ -207,7 +207,18 @@ extern void platform_device_put(struct platform_device *pdev);
207207

208208
struct platform_driver {
209209
int (*probe)(struct platform_device *);
210+
211+
/*
212+
* Traditionally the remove callback returned an int which however is
213+
* ignored by the driver core. This led to wrong expectations by driver
214+
* authors who thought returning an error code was a valid error
215+
* handling strategy. To convert to a callback returning void, new
216+
* drivers should implement .remove_new() until the conversion it done
217+
* that eventually makes .remove() return void.
218+
*/
210219
int (*remove)(struct platform_device *);
220+
void (*remove_new)(struct platform_device *);
221+
211222
void (*shutdown)(struct platform_device *);
212223
int (*suspend)(struct platform_device *, pm_message_t state);
213224
int (*resume)(struct platform_device *);

0 commit comments

Comments
 (0)