Messages in this thread | | | Date | Wed, 21 Feb 2024 22:01:44 +0100 | From | Sebastian Reichel <> | Subject | Re: [PATCH RFC] power: reset: restart-poweroff: convert to module |
| |
Hi,
On Wed, Feb 21, 2024 at 06:46:07PM +0100, A. Sverdlin wrote: > From: Alexander Sverdlin <alexander.sverdlin@siemens.com> > > The necessity of having a fake platform device for a generic, platform > independent functionality is not obvious. > Some platforms requre device tree modification for this, some would require > ACPI tables modification, while functionality may be useful even to > end-users without required expertise. Convert the platform driver to > a simple module. > > Signed-off-by: Alexander Sverdlin <alexander.sverdlin@siemens.com> > --- > This RFC is merely to understand if this approach would be accepted. > Converting to "tristate" could follow or preceed this patch.
1. You cannot easily make this tristate, because of machine_restart(). 2. This is already using module_platform_driver(), so this has nothing to do with making it a module like the subject suggests. 3. This no longer applies, since the driver is now properly using devm_register_sys_off_handler instead of pm_power_off. 4. It's intentional, that a device needs to be described. This is _not_ meant as a general purpose poweroff driver. It's intended to be used with bootloader support, which keeps the system off as described in the comment at the start of the file.
So: NAK
-- Sebastian
> diff --git a/drivers/power/reset/restart-poweroff.c b/drivers/power/reset/restart-poweroff.c > index 28f1822db1626..e1d94109f6823 100644 > --- a/drivers/power/reset/restart-poweroff.c > +++ b/drivers/power/reset/restart-poweroff.c > @@ -20,7 +20,7 @@ static void restart_poweroff_do_poweroff(void) > machine_restart(NULL); > } > > -static int restart_poweroff_probe(struct platform_device *pdev) > +static int __init restart_poweroff_init(void) > { > /* If a pm_power_off function has already been added, leave it alone */ > if (pm_power_off != NULL) { > @@ -33,12 +33,10 @@ static int restart_poweroff_probe(struct platform_device *pdev) > return 0; > } > > -static int restart_poweroff_remove(struct platform_device *pdev) > +static void __exit restart_poweroff_exit(void) > { > if (pm_power_off == &restart_poweroff_do_poweroff) > pm_power_off = NULL; > - > - return 0; > } > > static const struct of_device_id of_restart_poweroff_match[] = { > @@ -47,15 +45,8 @@ static const struct of_device_id of_restart_poweroff_match[] = { > }; > MODULE_DEVICE_TABLE(of, of_restart_poweroff_match); > > -static struct platform_driver restart_poweroff_driver = { > - .probe = restart_poweroff_probe, > - .remove = restart_poweroff_remove, > - .driver = { > - .name = "poweroff-restart", > - .of_match_table = of_restart_poweroff_match, > - }, > -}; > -module_platform_driver(restart_poweroff_driver); > +module_init(restart_poweroff_init); > +module_exit(restart_poweroff_exit); > > MODULE_AUTHOR("Andrew Lunn <andrew@lunn.ch"); > MODULE_DESCRIPTION("restart poweroff driver"); > -- > 2.43.0 > [unhandled content-type:application/pgp-signature] | |