[PATCH 0/5] vexpress: modularize power reset driver

[PATCH 0/5] vexpress: modularize power reset driver

[Anders Roxell]

Hi,

This patchset contains a bugfixe, a cleanup and fixes allmodconfig build breakages
on arm and arm64. Also making the vexpress power reset driver a module.

Cheers,
Anders

Anders Roxell (5):
  power: vexpress: add suppress_bind_attrs to true
  power: vexpress: cleanup: use builtin_platform_driver
  Revert "ARM: vexpress: Don't select VEXPRESS_CONFIG"
  power: reset: vexpress: fix build issue
  power: vexpress: make the reset driver a module

 arch/arm/mach-vexpress/Kconfig          | 2 +-
 drivers/power/reset/Kconfig             | 3 ++-
 drivers/power/reset/vexpress-poweroff.c | 8 +++++++-
 3 files changed, 10 insertions(+), 3 deletions(-)

-- 
2.26.2

[PATCH 1/5] power: vexpress: add suppress_bind_attrs to true

[Anders Roxell]

Make sure that the POWER_RESET_VEXPRESS driver won't have bind/unbind
attributes available via the sysfs, so lets be explicit here and use
".suppress_bind_attrs = true" to prevent userspace from doing something
silly.

Signed-off-by: Anders Roxell <anders.roxell@linaro.org>
---
 drivers/power/reset/vexpress-poweroff.c | 1 +
 1 file changed, 1 insertion(+)

diff --git a/drivers/power/reset/vexpress-poweroff.c b/drivers/power/reset/vexpress-poweroff.c
index 90cbaa8341e3..0bf9ab8653ae 100644
--- a/drivers/power/reset/vexpress-poweroff.c
+++ b/drivers/power/reset/vexpress-poweroff.c
@@ -143,6 +143,7 @@ static struct platform_driver vexpress_reset_driver = {
 	.driver = {
 		.name = "vexpress-reset",
 		.of_match_table = vexpress_reset_of_match,
+		.suppress_bind_attrs = true,
 	},
 };
 
-- 
2.26.2

Re: [PATCH 1/5] power: vexpress: add suppress_bind_attrs to true

[Arnd Bergmann]

On Wed, May 27, 2020 at 3:27 PM Rob Herring <robh@kernel.org> wrote:
>
> On Wed, May 27, 2020 at 5:26 AM Anders Roxell <anders.roxell@linaro.org> wrote:
> >
> > Make sure that the POWER_RESET_VEXPRESS driver won't have bind/unbind
> > attributes available via the sysfs, so lets be explicit here and use
> > ".suppress_bind_attrs = true" to prevent userspace from doing something
> > silly.
>
> This doesn't really make sense if we're going to make this a module.
> Module unloading and unbind introduce the same requirements of
> cleaning up (undoing whatever probe did).

I still want this change as a separate patch so we can backport it to stable
kernels for correctness.

Also, as long as we don't have a working (and tested) .remove callback,
we can only allow making the driver a loadable module but not also
allowing the module to be removed.

      Arnd

[PATCH 2/5] power: vexpress: cleanup: use builtin_platform_driver

[Anders Roxell]

Use the helper macro for builtin drivers taht don't do anything special
in driver init. This removes some boilerplate code.

Signed-off-by: Anders Roxell <anders.roxell@linaro.org>
---
 drivers/power/reset/vexpress-poweroff.c | 7 +------
 1 file changed, 1 insertion(+), 6 deletions(-)

diff --git a/drivers/power/reset/vexpress-poweroff.c b/drivers/power/reset/vexpress-poweroff.c
index 0bf9ab8653ae..1fdbcbd95fc2 100644
--- a/drivers/power/reset/vexpress-poweroff.c
+++ b/drivers/power/reset/vexpress-poweroff.c
@@ -146,9 +146,4 @@ static struct platform_driver vexpress_reset_driver = {
 		.suppress_bind_attrs = true,
 	},
 };
-
-static int __init vexpress_reset_init(void)
-{
-	return platform_driver_register(&vexpress_reset_driver);
-}
-device_initcall(vexpress_reset_init);
+builtin_platform_driver(vexpress_reset_driver);
-- 
2.26.2

[PATCH 3/5] Revert "ARM: vexpress: Don't select VEXPRESS_CONFIG"

[Anders Roxell]

This reverts commit 848685c25da99d871bbd87369f3c3d6eead661ac.
Due to when I set 'depends on VEXPRESS_CONFOG=Y' in 'config
POWER_RESET_VEXPRESS' to get an allmodconfig build on arm64 to build,
and allmodconfig build on arm fails if this patch isn't reverted.

Signed-off-by: Anders Roxell <anders.roxell@linaro.org>
---
 arch/arm/mach-vexpress/Kconfig | 1 +
 1 file changed, 1 insertion(+)

diff --git a/arch/arm/mach-vexpress/Kconfig b/arch/arm/mach-vexpress/Kconfig
index 8391a5b3cd78..065e12991663 100644
--- a/arch/arm/mach-vexpress/Kconfig
+++ b/arch/arm/mach-vexpress/Kconfig
@@ -19,6 +19,7 @@ menuconfig ARCH_VEXPRESS
 	select POWER_SUPPLY
 	select REGULATOR if MMC_ARMMMCI
 	select REGULATOR_FIXED_VOLTAGE if REGULATOR
+	select VEXPRESS_CONFIG
 	help
 	  This option enables support for systems using Cortex processor based
 	  ARM core and logic (FPGA) tiles on the Versatile Express motherboard,
-- 
2.26.2

[PATCH 4/5] power: reset: vexpress: fix build issue

[Anders Roxell]

An allmodconfig kernel makes CONFIG_VEXPRESS_CONFIG a module and
CONFIG_POWER_RESET_VEXPRESS builtin. That makes us see this build
error:

aarch64-linux-gnu-ld: drivers/power/reset/vexpress-poweroff.o: in function `vexpress_reset_probe':
../drivers/power/reset/vexpress-poweroff.c:119: undefined reference to `devm_regmap_init_vexpress_config'
../drivers/power/reset/vexpress-poweroff.c:119:(.text+0x48c): relocation truncated to fit: R_AARCH64_CALL26 against undefined symbol
`devm_regmap_init_vexpress_config'
make[1]: *** [/srv/src/kernel/next/Makefile:1126: vmlinux] Error 1

Rework so that POWER_RESET_VEXPRESS depends on 'VEXPRESS_CONFIG=y'.

Fixes: d06cfe3f123c ("bus: vexpress-config: Merge vexpress-syscfg into vexpress-config")
Signed-off-by: Anders Roxell <anders.roxell@linaro.org>
Acked-by: Rob Herring <robh@kernel.org>
---
 drivers/power/reset/Kconfig | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/drivers/power/reset/Kconfig b/drivers/power/reset/Kconfig
index 4dfac618b942..f07b982c8dff 100644
--- a/drivers/power/reset/Kconfig
+++ b/drivers/power/reset/Kconfig
@@ -191,7 +191,7 @@ config POWER_RESET_VERSATILE
 config POWER_RESET_VEXPRESS
 	bool "ARM Versatile Express power-off and reset driver"
 	depends on ARM || ARM64
-	depends on VEXPRESS_CONFIG
+	depends on VEXPRESS_CONFIG=y
 	help
 	  Power off and reset support for the ARM Ltd. Versatile
 	  Express boards.
-- 
2.26.2

[PATCH 5/5] power: vexpress: make the reset driver a module

[Anders Roxell]

Today the vexpress power driver can only be builtin.  Rework so it's
possible for the vexpress power driver to be a module.

Signed-off-by: Anders Roxell <anders.roxell@linaro.org>
---
 arch/arm/mach-vexpress/Kconfig          |  1 -
 drivers/power/reset/Kconfig             |  5 +++--
 drivers/power/reset/vexpress-poweroff.c | 12 +++++++++++-
 3 files changed, 14 insertions(+), 4 deletions(-)

diff --git a/arch/arm/mach-vexpress/Kconfig b/arch/arm/mach-vexpress/Kconfig
index 065e12991663..4b54d8cf897d 100644
--- a/arch/arm/mach-vexpress/Kconfig
+++ b/arch/arm/mach-vexpress/Kconfig
@@ -15,7 +15,6 @@ menuconfig ARCH_VEXPRESS
 	select NO_IOPORT_MAP
 	select PLAT_VERSATILE
 	select POWER_RESET
-	select POWER_RESET_VEXPRESS
 	select POWER_SUPPLY
 	select REGULATOR if MMC_ARMMMCI
 	select REGULATOR_FIXED_VOLTAGE if REGULATOR
diff --git a/drivers/power/reset/Kconfig b/drivers/power/reset/Kconfig
index f07b982c8dff..8468d42b0198 100644
--- a/drivers/power/reset/Kconfig
+++ b/drivers/power/reset/Kconfig
@@ -189,9 +189,10 @@ 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=y
+	depends on VEXPRESS_CONFIG
+	default VEXPRESS_CONFIG
 	help
 	  Power off and reset support for the ARM Ltd. Versatile
 	  Express boards.
diff --git a/drivers/power/reset/vexpress-poweroff.c b/drivers/power/reset/vexpress-poweroff.c
index 1fdbcbd95fc2..b1eef95132d9 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>
@@ -146,4 +147,13 @@ static struct platform_driver vexpress_reset_driver = {
 		.suppress_bind_attrs = true,
 	},
 };
-builtin_platform_driver(vexpress_reset_driver);
+
+static int __init vexpress_reset_init(void)
+{
+	return platform_driver_register(&vexpress_reset_driver);
+}
+module_init(vexpress_reset_init);
+
+MODULE_AUTHOR("Pawel Moll <pawel.moll@arm.com>");
+MODULE_DESCRIPTION("Vexpress reset driver");
+MODULE_LICENSE("GPL v2");
-- 
2.26.2

Re: [PATCH 5/5] power: vexpress: make the reset driver a module

[Arnd Bergmann]

On Wed, May 27, 2020 at 3:32 PM Rob Herring <robh@kernel.org> wrote:
>
> On Wed, May 27, 2020 at 5:26 AM Anders Roxell <anders.roxell@linaro.org> wrote:
> >
> > Today the vexpress power driver can only be builtin.  Rework so it's
> > possible for the vexpress power driver to be a module.
>
> This is the same incomplete patch I did[1]. As a module, it needs to
> clean-up everything probe did like overwriting global variables.
>
> Rob
>
> [1] https://lore.kernel.org/linux-arm-kernel/20200419170810.5738-5-robh@kernel.org/

Your version was actually broken because it allowed unloading the
driver again. The version that Anders sent is a bit better because it
explicitly forbids unloading by having a module_init but not module_exit
function, so as long as the .suppress_bind_attrs flag is set, this will
not crash the kernel.

It would be nice to have a .remove callback, but for the merge window
I'm happy with a patch that fixes the build regression.

        Arnd

Re: [PATCH 0/5] vexpress: modularize power reset driver

[Arnd Bergmann]

On Wed, May 27, 2020 at 3:35 PM Rob Herring <robh@kernel.org> wrote:
>
> On Wed, May 27, 2020 at 5:26 AM Anders Roxell <anders.roxell@linaro.org> wrote:
> >
> > Hi,
> >
> > This patchset contains a bugfixe, a cleanup and fixes allmodconfig build breakages
> > on arm and arm64. Also making the vexpress power reset driver a module.
> >
> > Cheers,
> > Anders
> >
> > Anders Roxell (5):
> >   power: vexpress: add suppress_bind_attrs to true
> >   power: vexpress: cleanup: use builtin_platform_driver
> >   Revert "ARM: vexpress: Don't select VEXPRESS_CONFIG"
> >   power: reset: vexpress: fix build issue
> >   power: vexpress: make the reset driver a module
>
> IMO, patches 3 and 4 should be applied to fix the kconfig issues.
> Making the driver a module can be addressed separately.

I've applied patches 1 through 4 now but left the last one.

If we can find someone to write and test a .remove callback,
I'll prefer that for v5.9, otherwise I'd consider taking Anders'
version instead.

     Arnd