Skip to content

Commit

Permalink
platform: Provide a remove callback that returns no value
Browse files Browse the repository at this point in the history
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 <u.kleine-koenig@pengutronix.de>
Link: https://lore.kernel.org/r/20221209150914.3557650-1-u.kleine-koenig@pengutronix.de
Signed-off-by: Greg Kroah-Hartman <gregkh@linuxfoundation.org>
  • Loading branch information
Uwe Kleine-König authored and gregkh committed Jan 17, 2023
1 parent 7bbb89b commit 5c5a768
Show file tree
Hide file tree
Showing 2 changed files with 14 additions and 1 deletion.
4 changes: 3 additions & 1 deletion drivers/base/platform.c
Original file line number Diff line number Diff line change
Expand Up @@ -1416,7 +1416,9 @@ 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) {
if (drv->remove_new) {
drv->remove_new(dev);
} else if (drv->remove) {
int ret = drv->remove(dev);

if (ret)
Expand Down
11 changes: 11 additions & 0 deletions include/linux/platform_device.h
Original file line number Diff line number Diff line change
Expand Up @@ -207,7 +207,18 @@ extern void platform_device_put(struct platform_device *pdev);

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.
*/
int (*remove)(struct platform_device *);
void (*remove_new)(struct platform_device *);

void (*shutdown)(struct platform_device *);
int (*suspend)(struct platform_device *, pm_message_t state);
int (*resume)(struct platform_device *);
Expand Down

0 comments on commit 5c5a768

Please sign in to comment.