| Message ID | 20231104211501.3676352-18-u.kleine-koenig@pengutronix.de (mailing list archive) |
|---|---|
| State | New |
| Headers | show |
| Series | power: reset: Drop platform_driver_probe() and convert to platform remove callback returning void | expand |
On 04.11.2023 23:15, Uwe Kleine-König wrote: > On today's platforms the benefit of platform_driver_probe() isn't that > relevant any more. It allows to drop some code after booting (or module > loading) for .probe() and discard the .remove() function completely if > the driver is built-in. This typically saves a few 100k. > > The downside of platform_driver_probe() is that the driver cannot be > bound and unbound at runtime which is ancient and so slightly > complicates testing. There are also thoughts to deprecate > platform_driver_probe() because it adds some complexity in the driver > core for little gain. Also many drivers don't use it correctly. This > driver for example misses to mark the driver struct with __ref which is > needed to suppress a (W=1) modpost warning. > > Signed-off-by: Uwe Kleine-König <u.kleine-koenig@pengutronix.de> Other than "::" in title: Reviewed-by: Claudiu Beznea <claudiu.beznea@tuxon.dev> > --- > drivers/power/reset/at91-reset.c | 9 +++++---- > 1 file changed, 5 insertions(+), 4 deletions(-) > > diff --git a/drivers/power/reset/at91-reset.c b/drivers/power/reset/at91-reset.c > index aa9b012d3d00..af85f2f929ba 100644 > --- a/drivers/power/reset/at91-reset.c > +++ b/drivers/power/reset/at91-reset.c > @@ -337,7 +337,7 @@ static int at91_rcdev_init(struct at91_reset *reset, > return devm_reset_controller_register(&pdev->dev, &reset->rcdev); > } > > -static int __init at91_reset_probe(struct platform_device *pdev) > +static int at91_reset_probe(struct platform_device *pdev) > { > const struct of_device_id *match; > struct at91_reset *reset; > @@ -417,7 +417,7 @@ static int __init at91_reset_probe(struct platform_device *pdev) > return ret; > } > > -static int __exit at91_reset_remove(struct platform_device *pdev) > +static int at91_reset_remove(struct platform_device *pdev) > { > struct at91_reset *reset = platform_get_drvdata(pdev); > > @@ -428,13 +428,14 @@ static int __exit at91_reset_remove(struct platform_device *pdev) > } > > static struct platform_driver at91_reset_driver = { > - .remove = __exit_p(at91_reset_remove), > + .probe = at91_reset_probe, > + .remove = at91_reset_remove, > .driver = { > .name = "at91-reset", > .of_match_table = at91_reset_of_match, > }, > }; > -module_platform_driver_probe(at91_reset_driver, at91_reset_probe); > +module_platform_driver(at91_reset_driver); > > MODULE_AUTHOR("Atmel Corporation"); > MODULE_DESCRIPTION("Reset driver for Atmel SoCs");
diff --git a/drivers/power/reset/at91-reset.c b/drivers/power/reset/at91-reset.c index aa9b012d3d00..af85f2f929ba 100644 --- a/drivers/power/reset/at91-reset.c +++ b/drivers/power/reset/at91-reset.c @@ -337,7 +337,7 @@ static int at91_rcdev_init(struct at91_reset *reset, return devm_reset_controller_register(&pdev->dev, &reset->rcdev); } -static int __init at91_reset_probe(struct platform_device *pdev) +static int at91_reset_probe(struct platform_device *pdev) { const struct of_device_id *match; struct at91_reset *reset; @@ -417,7 +417,7 @@ static int __init at91_reset_probe(struct platform_device *pdev) return ret; } -static int __exit at91_reset_remove(struct platform_device *pdev) +static int at91_reset_remove(struct platform_device *pdev) { struct at91_reset *reset = platform_get_drvdata(pdev); @@ -428,13 +428,14 @@ static int __exit at91_reset_remove(struct platform_device *pdev) } static struct platform_driver at91_reset_driver = { - .remove = __exit_p(at91_reset_remove), + .probe = at91_reset_probe, + .remove = at91_reset_remove, .driver = { .name = "at91-reset", .of_match_table = at91_reset_of_match, }, }; -module_platform_driver_probe(at91_reset_driver, at91_reset_probe); +module_platform_driver(at91_reset_driver); MODULE_AUTHOR("Atmel Corporation"); MODULE_DESCRIPTION("Reset driver for Atmel SoCs");
On today's platforms the benefit of platform_driver_probe() isn't that relevant any more. It allows to drop some code after booting (or module loading) for .probe() and discard the .remove() function completely if the driver is built-in. This typically saves a few 100k. The downside of platform_driver_probe() is that the driver cannot be bound and unbound at runtime which is ancient and so slightly complicates testing. There are also thoughts to deprecate platform_driver_probe() because it adds some complexity in the driver core for little gain. Also many drivers don't use it correctly. This driver for example misses to mark the driver struct with __ref which is needed to suppress a (W=1) modpost warning. Signed-off-by: Uwe Kleine-König <u.kleine-koenig@pengutronix.de> --- drivers/power/reset/at91-reset.c | 9 +++++---- 1 file changed, 5 insertions(+), 4 deletions(-)