| Message ID | 20200419170810.5738-5-robh@kernel.org (mailing list archive) |
|---|---|
| State | Not Applicable, archived |
| Headers | show |
| Series | Modularizing Versatile Express | expand |
On Sun, Apr 19, 2020 at 7:08 PM Rob Herring <robh@kernel.org> wrote: > > Enable building the VExpress power-off/reset driver as a module. Your change allows loading and unloading the driver, but actually unloading is a bug with the current implementation, as there is no 'release' handler to undo the _vexpress_register_restart_handler() function. It should not be hard to add a release handler, or you could just mark the function as non-unloadable by only having a module_init() but no module_exit() function. I suppose if you do the latter, there should also be a suppress_bind_attrs flag in the device_driver. This is a preexisting bug. Arnd
On Mon, Apr 20, 2020 at 10:24 AM Arnd Bergmann <arnd@arndb.de> wrote: > > On Sun, Apr 19, 2020 at 7:08 PM Rob Herring <robh@kernel.org> wrote: > > > > Enable building the VExpress power-off/reset driver as a module. > > Your change allows loading and unloading the driver, but actually > unloading is a bug with the current implementation, as there is no > 'release' handler to undo the _vexpress_register_restart_handler() > function. And also to save and restore pm_power_off... > It should not be hard to add a release handler, or you could just > mark the function as non-unloadable by only having a module_init() > but no module_exit() function. > > I suppose if you do the latter, there should also be a suppress_bind_attrs > flag in the device_driver. This is a preexisting bug. I may just drop this patch. I wrote it and then realized I don't need it as PSCI can be used instead. Rob
On Sun, Apr 19, 2020 at 12:07:57PM -0500, Rob Herring wrote: > Enable building the VExpress power-off/reset driver as a module. > > Cc: Catalin Marinas <catalin.marinas@arm.com> > Cc: Will Deacon <will@kernel.org> > Cc: Liviu Dudau <liviu.dudau@arm.com> > Cc: Sudeep Holla <sudeep.holla@arm.com> Reviewed-by: Sudeep Holla <sudeep.holla@arm.com> -- Regards, Sudeep
Hi, On Mon, Apr 20, 2020 at 12:23:13PM -0500, Rob Herring wrote: > On Mon, Apr 20, 2020 at 10:24 AM Arnd Bergmann <arnd@arndb.de> wrote: > > > > On Sun, Apr 19, 2020 at 7:08 PM Rob Herring <robh@kernel.org> wrote: > > > > > > Enable building the VExpress power-off/reset driver as a module. > > > > Your change allows loading and unloading the driver, but actually > > unloading is a bug with the current implementation, as there is no > > 'release' handler to undo the _vexpress_register_restart_handler() > > function. > > And also to save and restore pm_power_off... I'm fine with providing Acked-by for this for merging through a different tree or taking it through my tree. > > It should not be hard to add a release handler, or you could just > > mark the function as non-unloadable by only having a module_init() > > but no module_exit() function. > > > > I suppose if you do the latter, there should also be a suppress_bind_attrs > > flag in the device_driver. This is a preexisting bug. > > I may just drop this patch. I wrote it and then realized I don't need > it as PSCI can be used instead. So is the driver useless on arm64 and depends can be reduced to arm32? -- Sebastian
diff --git a/drivers/power/reset/Kconfig b/drivers/power/reset/Kconfig index 890380302080..0ba18221ba40 100644 --- a/drivers/power/reset/Kconfig +++ b/drivers/power/reset/Kconfig @@ -182,7 +182,7 @@ config POWER_RESET_VERSATILE reference boards. config POWER_RESET_VEXPRESS - bool "ARM Versatile Express power-off and reset driver" + tristate "ARM Versatile Express power-off and reset driver" depends on ARM || ARM64 depends on VEXPRESS_CONFIG help diff --git a/drivers/power/reset/vexpress-poweroff.c b/drivers/power/reset/vexpress-poweroff.c index 90cbaa8341e3..8a6d3add894c 100644 --- a/drivers/power/reset/vexpress-poweroff.c +++ b/drivers/power/reset/vexpress-poweroff.c @@ -5,6 +5,7 @@ */ #include <linux/delay.h> +#include <linux/module.h> #include <linux/notifier.h> #include <linux/of.h> #include <linux/of_device.h> @@ -87,6 +88,7 @@ static const struct of_device_id vexpress_reset_of_match[] = { }, {} }; +MODULE_DEVICE_TABLE(of, vexpress_reset_of_match); static int _vexpress_register_restart_handler(struct device *dev) { @@ -145,9 +147,5 @@ static struct platform_driver vexpress_reset_driver = { .of_match_table = vexpress_reset_of_match, }, }; - -static int __init vexpress_reset_init(void) -{ - return platform_driver_register(&vexpress_reset_driver); -} -device_initcall(vexpress_reset_init); +module_platform_driver(vexpress_reset_driver); +MODULE_LICENSE("GPL v2");
Enable building the VExpress power-off/reset driver as a module. Cc: Catalin Marinas <catalin.marinas@arm.com> Cc: Will Deacon <will@kernel.org> Cc: Liviu Dudau <liviu.dudau@arm.com> Cc: Sudeep Holla <sudeep.holla@arm.com> Cc: Lorenzo Pieralisi <lorenzo.pieralisi@arm.com> Cc: Linus Walleij <linus.walleij@linaro.org> Cc: Sebastian Reichel <sre@kernel.org> Cc: linux-pm@vger.kernel.org Signed-off-by: Rob Herring <robh@kernel.org> --- drivers/power/reset/Kconfig | 2 +- drivers/power/reset/vexpress-poweroff.c | 10 ++++------ 2 files changed, 5 insertions(+), 7 deletions(-)