| Message ID | 20231104211501.3676352-19-u.kleine-koenig@pengutronix.de |
|---|---|
| State | Accepted |
| Commit | dde74a5de817e0a011e4783cf26295d7f6fdca26 |
| 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> Reviewed-by: Claudiu Beznea <claudiu.beznea@tuxon.dev> > --- > drivers/power/reset/at91-sama5d2_shdwc.c | 9 +++++---- > 1 file changed, 5 insertions(+), 4 deletions(-) > > diff --git a/drivers/power/reset/at91-sama5d2_shdwc.c b/drivers/power/reset/at91-sama5d2_shdwc.c > index e76b102b57b1..ef8add623363 100644 > --- a/drivers/power/reset/at91-sama5d2_shdwc.c > +++ b/drivers/power/reset/at91-sama5d2_shdwc.c > @@ -329,7 +329,7 @@ static const struct of_device_id at91_pmc_ids[] = { > { /* Sentinel. */ } > }; > > -static int __init at91_shdwc_probe(struct platform_device *pdev) > +static int at91_shdwc_probe(struct platform_device *pdev) > { > const struct of_device_id *match; > struct device_node *np; > @@ -421,7 +421,7 @@ static int __init at91_shdwc_probe(struct platform_device *pdev) > return ret; > } > > -static int __exit at91_shdwc_remove(struct platform_device *pdev) > +static int at91_shdwc_remove(struct platform_device *pdev) > { > struct shdwc *shdw = platform_get_drvdata(pdev); > > @@ -442,13 +442,14 @@ static int __exit at91_shdwc_remove(struct platform_device *pdev) > } > > static struct platform_driver at91_shdwc_driver = { > - .remove = __exit_p(at91_shdwc_remove), > + .probe = at91_shdwc_probe, > + .remove = at91_shdwc_remove, > .driver = { > .name = "at91-shdwc", > .of_match_table = at91_shdwc_of_match, > }, > }; > -module_platform_driver_probe(at91_shdwc_driver, at91_shdwc_probe); > +module_platform_driver(at91_shdwc_driver); > > MODULE_AUTHOR("Nicolas Ferre <nicolas.ferre@atmel.com>"); > MODULE_DESCRIPTION("Atmel shutdown controller driver");
diff --git a/drivers/power/reset/at91-sama5d2_shdwc.c b/drivers/power/reset/at91-sama5d2_shdwc.c index e76b102b57b1..ef8add623363 100644 --- a/drivers/power/reset/at91-sama5d2_shdwc.c +++ b/drivers/power/reset/at91-sama5d2_shdwc.c @@ -329,7 +329,7 @@ static const struct of_device_id at91_pmc_ids[] = { { /* Sentinel. */ } }; -static int __init at91_shdwc_probe(struct platform_device *pdev) +static int at91_shdwc_probe(struct platform_device *pdev) { const struct of_device_id *match; struct device_node *np; @@ -421,7 +421,7 @@ static int __init at91_shdwc_probe(struct platform_device *pdev) return ret; } -static int __exit at91_shdwc_remove(struct platform_device *pdev) +static int at91_shdwc_remove(struct platform_device *pdev) { struct shdwc *shdw = platform_get_drvdata(pdev); @@ -442,13 +442,14 @@ static int __exit at91_shdwc_remove(struct platform_device *pdev) } static struct platform_driver at91_shdwc_driver = { - .remove = __exit_p(at91_shdwc_remove), + .probe = at91_shdwc_probe, + .remove = at91_shdwc_remove, .driver = { .name = "at91-shdwc", .of_match_table = at91_shdwc_of_match, }, }; -module_platform_driver_probe(at91_shdwc_driver, at91_shdwc_probe); +module_platform_driver(at91_shdwc_driver); MODULE_AUTHOR("Nicolas Ferre <nicolas.ferre@atmel.com>"); MODULE_DESCRIPTION("Atmel shutdown controller driver");
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-sama5d2_shdwc.c | 9 +++++---- 1 file changed, 5 insertions(+), 4 deletions(-)