| Message ID | 20231104211501.3676352-17-u.kleine-koenig@pengutronix.de |
|---|---|
| State | Accepted |
| Commit | 099806de68b75a0fe114376b1ee162fdff572ecc |
| 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-poweroff.c | 9 +++++---- > 1 file changed, 5 insertions(+), 4 deletions(-) > > diff --git a/drivers/power/reset/at91-poweroff.c b/drivers/power/reset/at91-poweroff.c > index dd5399785b69..83567428ab43 100644 > --- a/drivers/power/reset/at91-poweroff.c > +++ b/drivers/power/reset/at91-poweroff.c > @@ -149,7 +149,7 @@ static void at91_poweroff_dt_set_wakeup_mode(struct platform_device *pdev) > writel(wakeup_mode | mode, at91_shdwc.shdwc_base + AT91_SHDW_MR); > } > > -static int __init at91_poweroff_probe(struct platform_device *pdev) > +static int at91_poweroff_probe(struct platform_device *pdev) > { > struct device_node *np; > u32 ddr_type; > @@ -202,7 +202,7 @@ static int __init at91_poweroff_probe(struct platform_device *pdev) > return ret; > } > > -static int __exit at91_poweroff_remove(struct platform_device *pdev) > +static int at91_poweroff_remove(struct platform_device *pdev) > { > if (pm_power_off == at91_poweroff) > pm_power_off = NULL; > @@ -224,13 +224,14 @@ static const struct of_device_id at91_poweroff_of_match[] = { > MODULE_DEVICE_TABLE(of, at91_poweroff_of_match); > > static struct platform_driver at91_poweroff_driver = { > - .remove = __exit_p(at91_poweroff_remove), > + .probe = at91_poweroff_probe, > + .remove = at91_poweroff_remove, > .driver = { > .name = "at91-poweroff", > .of_match_table = at91_poweroff_of_match, > }, > }; > -module_platform_driver_probe(at91_poweroff_driver, at91_poweroff_probe); > +module_platform_driver(at91_poweroff_driver); > > MODULE_AUTHOR("Atmel Corporation"); > MODULE_DESCRIPTION("Shutdown driver for Atmel SoCs");
diff --git a/drivers/power/reset/at91-poweroff.c b/drivers/power/reset/at91-poweroff.c index dd5399785b69..83567428ab43 100644 --- a/drivers/power/reset/at91-poweroff.c +++ b/drivers/power/reset/at91-poweroff.c @@ -149,7 +149,7 @@ static void at91_poweroff_dt_set_wakeup_mode(struct platform_device *pdev) writel(wakeup_mode | mode, at91_shdwc.shdwc_base + AT91_SHDW_MR); } -static int __init at91_poweroff_probe(struct platform_device *pdev) +static int at91_poweroff_probe(struct platform_device *pdev) { struct device_node *np; u32 ddr_type; @@ -202,7 +202,7 @@ static int __init at91_poweroff_probe(struct platform_device *pdev) return ret; } -static int __exit at91_poweroff_remove(struct platform_device *pdev) +static int at91_poweroff_remove(struct platform_device *pdev) { if (pm_power_off == at91_poweroff) pm_power_off = NULL; @@ -224,13 +224,14 @@ static const struct of_device_id at91_poweroff_of_match[] = { MODULE_DEVICE_TABLE(of, at91_poweroff_of_match); static struct platform_driver at91_poweroff_driver = { - .remove = __exit_p(at91_poweroff_remove), + .probe = at91_poweroff_probe, + .remove = at91_poweroff_remove, .driver = { .name = "at91-poweroff", .of_match_table = at91_poweroff_of_match, }, }; -module_platform_driver_probe(at91_poweroff_driver, at91_poweroff_probe); +module_platform_driver(at91_poweroff_driver); MODULE_AUTHOR("Atmel Corporation"); MODULE_DESCRIPTION("Shutdown 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-poweroff.c | 9 +++++---- 1 file changed, 5 insertions(+), 4 deletions(-)