Modularizing Versatile Express

CONFIG_COMMON_CLK_VERSATILE doesn't really do anything other than hiding Arm Ltd reference platform clock drivers. It is both selected by the platforms that need it and has a 'depends on' for those platforms. It selects REGMAP_MMIO, but really CONFIG_ICST should do that. Also, CONFIG_ICST can't be enabled for COMPILE_TEST unless something else selected it.

Cc Catalin Marinas <[email protected]>

Cc Will Deacon <[email protected]>

Cc Liviu Dudau <[email protected]>

Cc Sudeep Holla <[email protected]>

Cc Lorenzo Pieralisi <[email protected]>

Cc Linus Walleij <[email protected]>

Cc Stephen Boyd <[email protected]>

Cc [email protected]

Signed-off-by Rob Herring <[email protected]>

The only thing that vexpress-syscfg does is provide a regmap to vexpress-config bus child devices. There's little reason to have 2 components for this. The current structure with initcall ordering requirements makes turning these components into modules more difficult.

So let's start to simplify things and merge vexpress-syscfg into vexpress-config. There's no functional change in this commit and it's still separate components until subsequent commits.

Cc Liviu Dudau <[email protected]>

Cc Sudeep Holla <[email protected]>

Cc Lorenzo Pieralisi <[email protected]>

Cc Linus Walleij <[email protected]>

Cc Arnd Bergmann <[email protected]>

Cc Greg Kroah-Hartman <[email protected]>

Signed-off-by Rob Herring <[email protected]>

---
 arch/arm/mach-vexpress/Kconfig |   1 -
 drivers/bus/vexpress-config.c  | 283 +++++++++++++++++++++++++++++++--
 drivers/misc/Kconfig           |   9 --
 drivers/misc/Makefile          |   1 -
 drivers/misc/vexpress-syscfg.c | 280 --------------------------------
 include/linux/vexpress.h       |  17 --
 6 files changed, 274 insertions(+), 317 deletions(-)
 delete mode 100644 drivers/misc/vexpress-syscfg.c

vexpress_flags_set() is only used by the platform SMP related code and has nothing to do with the vexpress-sysreg MFD driver other than both access the same h/w block. It's also only needed for 32-bit systems and must be built-in for them. Let's move vexpress_flags_set() closer to where it is being used. This will allow for vexpress-sysreg to be built as a module.

Cc Liviu Dudau <[email protected]>

Cc Sudeep Holla <[email protected]>

Cc Lorenzo Pieralisi <[email protected]>

Cc Linus Walleij <[email protected]>

Cc Lee Jones <[email protected]>

Signed-off-by Rob Herring <[email protected]>

The VExpress power-off/reset driver is not needed on 64-bit platforms as PSCI power-off and reset can be used instead. Stop selecting it so it can be disabled and not always built-in.

Cc Catalin Marinas <[email protected]>

Cc Will Deacon <[email protected]>

Cc Liviu Dudau <[email protected]>

Cc Sudeep Holla <[email protected]>

Cc Lorenzo Pieralisi <[email protected]>

Cc Linus Walleij <[email protected]>

Signed-off-by Rob Herring <[email protected]>

CONFIG_VEXPRESS_CONFIG has 'default y if ARCH_VEXPRESS', so selecting is unnecessary. It also prevents setting CONFIG_VEXPRESS_CONFIG to a module.

Cc Catalin Marinas <[email protected]>

Cc Will Deacon <[email protected]>

Cc Liviu Dudau <[email protected]>

Cc Sudeep Holla <[email protected]>

Cc Lorenzo Pieralisi <[email protected]>

Cc Linus Walleij <[email protected]>

Signed-off-by Rob Herring <[email protected]>

---
 arch/arm64/Kconfig.platforms | 1 -
 1 file changed, 1 deletion(-)

arch/arm64/Kconfig.platforms +0 -0

index 9c084f995e0f..5c38dc56b808 100644

Enable building the VExpress power-off/reset driver as a module.

While 64-bit Arm reference platforms have SP810 for clocks for SP804 timers, they are not needed since the arch timers are used instead.

Cc Catalin Marinas <[email protected]>

Cc Will Deacon <[email protected]>

Cc Liviu Dudau <[email protected]>

Cc Sudeep Holla <[email protected]>

Cc Lorenzo Pieralisi <[email protected]>

Cc Linus Walleij <[email protected]>

Cc Stephen Boyd <[email protected]>

Cc [email protected]

Signed-off-by Rob Herring <[email protected]>

In preparation to enable the vexpress-osc clock driver as a module, convert the driver to use the managed devres clock API variants. With this, a driver .remove() hook is not needed.

Cc Liviu Dudau <[email protected]>

Cc Sudeep Holla <[email protected]>

Cc Lorenzo Pieralisi <[email protected]>

Cc Linus Walleij <[email protected]>

Cc Stephen Boyd <[email protected]>

Cc [email protected]

Signed-off-by Rob Herring <[email protected]>

Nothing in the VExpress sysregs nor the MFD child drivers use CONFIG_CLKSRC_MMIO. There's the 24MHz counter, but that's handled by drivers/clocksource/timer-versatile.c which doesn't use CONFIG_CLKSRC_MMIO either. So let's just drop CONFIG_CLKSRC_MMIO.

As the !ARCH_USES_GETTIMEOFFSET dependency was added for CONFIG_CLKSRC_MMIO, that can be dropped, too.

Cc Liviu Dudau <[email protected]>

Cc Sudeep Holla <[email protected]>

Cc Lorenzo Pieralisi <[email protected]>

Cc Linus Walleij <[email protected]>

Cc Lee Jones <[email protected]>

Signed-off-by Rob Herring <[email protected]>

Cc Liviu Dudau <[email protected]>

Cc Sudeep Holla <[email protected]>

Cc Lorenzo Pieralisi <[email protected]>

Cc Linus Walleij <[email protected]>

Cc Lee Jones <[email protected]>

Signed-off-by Rob Herring <[email protected]>

Use the managed devm_gpiochip_add_data() and devm_mfd_add_devices() instead of their unmanaged counterparts. With this, no .remove() hook is needed for driver unbind.

Cc Liviu Dudau <[email protected]>

Cc Sudeep Holla <[email protected]>

Cc Lorenzo Pieralisi <[email protected]>

Cc Linus Walleij <[email protected]>

Cc Lee Jones <[email protected]>

Signed-off-by Rob Herring <[email protected]>

drivers/mfd/vexpress-sysreg.c static struct platform_driver vexpress_sysreg_driver = { · line 169

Quoted 7 lines · Rob Herring

Enable building the vexpress-sysreg driver as a module.

drivers/mfd/vexpress-sysreg.c +0 -4

-
-       /* Need the sysreg early, before any other device... */
-       for_each_matching_node(node, vexpress_sysreg_match)
-               of_platform_device_create(node, NULL, NULL);

Is this safe to remove? If yes, please mention it in the changelog.

Quoted 4 lines · Rob Herring

Enable building the vexpress-osc clock driver as a module.

drivers/clk/versatile/Kconfig config CLK_SP810 · line 19

Quoted 5 lines

config CLK_VEXPRESS_OSC

drivers/clk/versatile/Kconfig +2 -2

-       bool "Clock driver for Versatile Express OSC clock generators"
-       depends on VEXPRESS_CONFIG || COMPILE_TEST
+       tristate "Clock driver for Versatile Express OSC clock generators"
+       depends on VEXPRESS_CONFIG

Why do you remove the "|| COMPILE_TEST" here?

Quoted 2 lines · Rob Herring

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.

Quoted 4 lines · Rob Herring

While 64-bit Arm reference platforms have SP810 for clocks for SP804 timers, they are not needed since the arch timers are used instead.

Quoted 9 lines

Cc Catalin Marinas <[email protected]>

Cc Will Deacon <[email protected]>

Cc Liviu Dudau <[email protected]>

Cc Sudeep Holla <[email protected]>

Cc Lorenzo Pieralisi <[email protected]>

Cc Linus Walleij <[email protected]>

Cc Stephen Boyd <[email protected]>

Cc [email protected]

Signed-off-by Rob Herring <[email protected]>

Acked-by Arnd Bergmann <[email protected]>

Quoted 1 line

drivers/clk/versatile/Kconfig config ICST · line 12

Quoted 4 lines

config CLK_SP810
bool "Clock driver for ARM SP810 System Controller"

drivers/clk/versatile/Kconfig +1 -1

-       default y if ARCH_VEXPRESS
+       default y if (ARCH_VEXPRESS && ARM)

But maybe add "|| (COMPILE_TEST && OF)" for extra points.

Quoted 19 lines · Arnd Bergmann

Quoted 4 lines · Rob Herring

While 64-bit Arm reference platforms have SP810 for clocks for SP804 timers, they are not needed since the arch timers are used instead.

Quoted 9 lines

Cc Catalin Marinas <[email protected]>

Cc Will Deacon <[email protected]>

Cc Liviu Dudau <[email protected]>

Cc Sudeep Holla <[email protected]>

Cc Lorenzo Pieralisi <[email protected]>

Cc Linus Walleij <[email protected]>

Cc Stephen Boyd <[email protected]>

Cc [email protected]

Signed-off-by Rob Herring <[email protected]>

Acked-by Arnd Bergmann <[email protected]>

Quoted 1 line

Quoted 6 lines

drivers/clk/versatile/Kconfig config ICST · line 12

Quoted 4 lines

config CLK_SP810
bool "Clock driver for ARM SP810 System Controller"

drivers/clk/versatile/Kconfig +1 -1

-       default y if ARCH_VEXPRESS
+       default y if (ARCH_VEXPRESS && ARM)

But maybe add "|| (COMPILE_TEST && OF)" for extra points.

On a 'default y'? Not necessary.

drivers/clk/versatile/Kconfig +1 -7

index c2618f1477a2..3465fb291998 100644
@@ -1,22 +1,14 @@
 # SPDX-License-Identifier: GPL-2.0-only
 config ICST
-       bool
-
-config COMMON_CLK_VERSATILE
-       bool "Clock driver for ARM Reference designs"
-       depends on ARCH_INTEGRATOR || ARCH_REALVIEW || \
-               ARCH_VERSATILE || ARCH_VEXPRESS || ARM64 || \
-               COMPILE_TEST
+       bool "Clock driver for ARM Reference designs ICST" if COMPILE_TEST

drivers/clk/versatile/Kconfig +1 -2

        default y if ARCH_VEXPRESS
        ---help---
          Supports clock muxing (REFCLK/TIMCLK to TIMERCLKEN0-3) capabilities

@@ -24,8 +16,7 @@ config CLK_SP810
 
 config CLK_VEXPRESS_OSC
        bool "Clock driver for Versatile Express OSC clock generators"
-       depends on COMMON_CLK_VERSATILE
-       depends on VEXPRESS_CONFIG
+       depends on VEXPRESS_CONFIG || COMPILE_TEST
        default y if ARCH_VEXPRESS
        ---help---
          Simple regmap-based driver driving clock generators on Versatile

While 64-bit Arm reference platforms have SP810 for clocks for SP804 timers, they are not needed since the arch timers are used instead.

Cc Catalin Marinas <[email protected]>

Cc Will Deacon <[email protected]>

Cc Liviu Dudau <[email protected]>

Cc Sudeep Holla <[email protected]>

Cc Lorenzo Pieralisi <[email protected]>

Cc Linus Walleij <[email protected]>

Cc Stephen Boyd <[email protected]>

Cc [email protected]

Signed-off-by Rob Herring <[email protected]>

---

There's only a single caller of vexpress_config_set_master() from vexpress-sysreg.c. Let's just make the registers needed available to vexpress-config and move all the code there. The registers needed aren't used anywhere else either. With this, we can get rid of the private API between these 2 drivers.

Cc Liviu Dudau <[email protected]>

Cc Sudeep Holla <[email protected]>

Cc Lorenzo Pieralisi <[email protected]>

Cc Linus Walleij <[email protected]>

Cc Lee Jones <[email protected]>

Cc Greg Kroah-Hartman <[email protected]>

Signed-off-by Rob Herring <[email protected]>

In preparation to enable the vexpress-osc clock driver as a module, convert the driver to use the managed devres clock API variants. With this, a driver .remove() hook is not needed.

Cc Liviu Dudau <[email protected]>

Cc Sudeep Holla <[email protected]>

Cc Lorenzo Pieralisi <[email protected]>

Cc Linus Walleij <[email protected]>

Cc Stephen Boyd <[email protected]>

Cc [email protected]

Signed-off-by Rob Herring <[email protected]>

---

The VExpress power-off/reset driver is not needed on 64-bit platforms as PSCI power-off and reset can be used instead. Stop selecting it so it can be disabled and not always built-in.

Cc Catalin Marinas <[email protected]>

Cc Will Deacon <[email protected]>

Cc Liviu Dudau <[email protected]>

Cc Sudeep Holla <[email protected]>

Cc Lorenzo Pieralisi <[email protected]>

Cc Linus Walleij <[email protected]>

Signed-off-by Rob Herring <[email protected]>

---
 arch/arm64/Kconfig.platforms | 1 -
 1 file changed, 1 deletion(-)

arch/arm64/Kconfig.platforms +0 -1

index 55d70cfe0f9e..9c084f995e0f 100644
@@ -278,7 +278,6 @@ config ARCH_VEXPRESS
 	select GPIOLIB
 	select PM
 	select PM_GENERIC_DOMAINS
-	select POWER_RESET_VEXPRESS
 	select VEXPRESS_CONFIG
 	help
 	  This enables support for the ARMv8 software model (Versatile

Signature · 1 line

2.20.1

vexpress_flags_set() is only used by the platform SMP related code and has nothing to do with the vexpress-sysreg MFD driver other than both access the same h/w block. It's also only needed for 32-bit systems and must be built-in for them. Let's move vexpress_flags_set() closer to where it is being used. This will allow for vexpress-sysreg to be built as a module.

Cc Liviu Dudau <[email protected]>

Cc Sudeep Holla <[email protected]>

Cc Lorenzo Pieralisi <[email protected]>

Cc Linus Walleij <[email protected]>

Cc Lee Jones <[email protected]>

Signed-off-by Rob Herring <[email protected]>

---
 arch/arm/mach-vexpress/Kconfig |  1 -
 arch/arm/mach-vexpress/core.h  |  1 +
 arch/arm/mach-vexpress/dcscb.c |  1 +
 arch/arm/mach-vexpress/v2m.c   | 23 +++++++++++++++++++++++
 drivers/mfd/vexpress-sysreg.c  | 19 -------------------
 include/linux/vexpress.h       |  4 ----
 6 files changed, 25 insertions(+), 24 deletions(-)

arch/arm/mach-vexpress/Kconfig +0 -1

index 726a68085c3b..18951cd20d9d 100644
@@ -22,7 +22,6 @@ menuconfig ARCH_VEXPRESS
 	select REGULATOR_FIXED_VOLTAGE if REGULATOR
 	select VEXPRESS_CONFIG
 	select VEXPRESS_SYSCFG
-	select MFD_VEXPRESS_SYSREG
 	help
 	  This option enables support for systems using Cortex processor based
 	  ARM core and logic (FPGA) tiles on the Versatile Express motherboard,

arch/arm/mach-vexpress/core.h +1 -0

index f4a7519084f1..bda78675c55d 100644
@@ -1,3 +1,4 @@
 bool vexpress_smp_init_ops(void);
+void vexpress_flags_set(u32 data);
 
 extern const struct smp_operations vexpress_smp_dt_ops;

arch/arm/mach-vexpress/dcscb.c +1 -0

index 46a903c88c6a..a0554d7d04f7 100644
@@ -20,6 +20,7 @@
 #include <asm/cputype.h>
 #include <asm/cp15.h>
 
+#include "core.h"
 
 #define RST_HOLD0	0x0
 #define RST_HOLD1	0x4

arch/arm/mach-vexpress/v2m.c +23 -0

index 95886b3bb9dd..ffe7c7a85ae9 100644
@@ -1,8 +1,31 @@
 // SPDX-License-Identifier: GPL-2.0
+#include <linux/of.h>
+#include <linux/of_address.h>
 #include <asm/mach/arch.h>
 
 #include "core.h"
 
+#define SYS_FLAGSSET		0x030
+#define SYS_FLAGSCLR		0x034
+
+void vexpress_flags_set(u32 data)
+{
+	static void __iomem *base;
+
+	if (!base) {
+		struct device_node *node = of_find_compatible_node(NULL, NULL,
+				"arm,vexpress-sysreg");
+
+		base = of_iomap(node, 0);
+	}
+
+	if (WARN_ON(!base))
+		return;
+
+	writel(~0, base + SYS_FLAGSCLR);
+	writel(data, base + SYS_FLAGSSET);
+}
+
 static const char * const v2m_dt_match[] __initconst = {
 	"arm,vexpress",
 	NULL,

drivers/mfd/vexpress-sysreg.c +0 -19

index c68ff56dbdb1..0b9cc67706c7 100644
@@ -8,7 +8,6 @@
 #include <linux/err.h>
 #include <linux/io.h>
 #include <linux/mfd/core.h>
-#include <linux/of_address.h>
 #include <linux/of_platform.h>
 #include <linux/platform_data/syscon.h>
 #include <linux/platform_device.h>

@@ -42,24 +41,6 @@
 
 #define SYS_MISC_MASTERSITE	(1 << 14)
 
-void vexpress_flags_set(u32 data)
-{
-	static void __iomem *base;
-
-	if (!base) {
-		struct device_node *node = of_find_compatible_node(NULL, NULL,
-				"arm,vexpress-sysreg");
-
-		base = of_iomap(node, 0);
-	}
-
-	if (WARN_ON(!base))
-		return;
-
-	writel(~0, base + SYS_FLAGSCLR);
-	writel(data, base + SYS_FLAGSSET);
-}
-
 /* The sysreg block is just a random collection of various functions... */
 
 static struct syscon_platform_data vexpress_sysreg_sys_id_pdata = {

include/linux/vexpress.h +0 -4

index 0e130b5077a5..2ec7992b054c 100644
@@ -40,8 +40,4 @@ struct device *vexpress_config_bridge_register(struct device *parent,
 
 struct regmap *devm_regmap_init_vexpress_config(struct device *dev);
 
-/* Platform control */
-
-void vexpress_flags_set(u32 data);
-
 #endif

Signature · 1 line

2.20.1

While 64-bit Arm reference platforms have SP810 for clocks for SP804 timers, they are not needed since the arch timers are used instead.

Cc Catalin Marinas <[email protected]>

Cc Will Deacon <[email protected]>

Cc Liviu Dudau <[email protected]>

Cc Sudeep Holla <[email protected]>

Cc Lorenzo Pieralisi <[email protected]>

Cc Linus Walleij <[email protected]>

Cc Stephen Boyd <[email protected]>

Cc [email protected]

Signed-off-by Rob Herring <[email protected]>

---
 drivers/clk/versatile/Kconfig | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

drivers/clk/versatile/Kconfig +1 -1

index 3465fb291998..5bdd5c98990b 100644
@@ -9,7 +9,7 @@ config ICST
 
 config CLK_SP810
 	bool "Clock driver for ARM SP810 System Controller"
-	default y if ARCH_VEXPRESS
+	default y if (ARCH_VEXPRESS && ARM)
 	---help---
 	  Supports clock muxing (REFCLK/TIMCLK to TIMERCLKEN0-3) capabilities
 	  of the ARM SP810 System Controller cell.

Signature · 1 line

2.20.1

In preparation to enable the vexpress-osc clock driver as a module, convert the driver to use the managed devres clock API variants. With this, a driver .remove() hook is not needed.

Cc Liviu Dudau <[email protected]>

Cc Sudeep Holla <[email protected]>

Cc Lorenzo Pieralisi <[email protected]>

Cc Linus Walleij <[email protected]>

Cc Stephen Boyd <[email protected]>

Cc [email protected]

Signed-off-by Rob Herring <[email protected]>

---
 drivers/clk/versatile/clk-vexpress-osc.c | 10 +++++-----
 1 file changed, 5 insertions(+), 5 deletions(-)

drivers/clk/versatile/clk-vexpress-osc.c +5 -5

index 7ade146a3ea9..5bb1d5a714d0 100644
@@ -65,8 +65,8 @@ static int vexpress_osc_probe(struct platform_device *pdev)
 {
 	struct clk_init_data init;
 	struct vexpress_osc *osc;
-	struct clk *clk;
 	u32 range[2];
+	int ret;
 
 	osc = devm_kzalloc(&pdev->dev, sizeof(*osc), GFP_KERNEL);
 	if (!osc)

@@ -92,11 +92,11 @@ static int vexpress_osc_probe(struct platform_device *pdev)
 
 	osc->hw.init = &init;
 
-	clk = clk_register(NULL, &osc->hw);
-	if (IS_ERR(clk))
-		return PTR_ERR(clk);
+	ret = devm_clk_hw_register(&pdev->dev, &osc->hw);
+	if (ret < 0)
+		return ret;
 
-	of_clk_add_provider(pdev->dev.of_node, of_clk_src_simple_get, clk);
+	devm_of_clk_add_hw_provider(&pdev->dev, of_clk_hw_simple_get, &osc->hw);
 	clk_hw_set_rate_range(&osc->hw, osc->rate_min, osc->rate_max);
 
 	dev_dbg(&pdev->dev, "Registered clock '%s'\n", init.name);

Signature · 1 line

2.20.1

Enable building the vexpress-osc clock driver as a module.

Cc Linus Walleij <[email protected]>

Cc Liviu Dudau <[email protected]>

Cc Sudeep Holla <[email protected]>

Cc Lorenzo Pieralisi <[email protected]>

Cc Michael Turquette <[email protected]>

Cc Stephen Boyd <[email protected]>

Cc [email protected]

Signed-off-by Rob Herring <[email protected]>

---
 drivers/clk/versatile/Kconfig            |  4 ++--
 drivers/clk/versatile/clk-vexpress-osc.c | 10 ++++------
 2 files changed, 6 insertions(+), 8 deletions(-)

drivers/clk/versatile/Kconfig +0 -0

index 5bdd5c98990b..9de2396dcf9b 100644

drivers/clk/versatile/Kconfig +0 -0

 	default y if ARCH_VEXPRESS
 	---help---
 	  Simple regmap-based driver driving clock generators on Versatile

drivers/clk/versatile/clk-vexpress-osc.c +4 -6

index 5bb1d5a714d0..b2b32fa2d7c3 100644
@@ -7,6 +7,7 @@
 #include <linux/clkdev.h>
 #include <linux/clk-provider.h>
 #include <linux/err.h>
+#include <linux/module.h>
 #include <linux/of.h>
 #include <linux/platform_device.h>
 #include <linux/slab.h>

@@ -108,6 +109,7 @@ static const struct of_device_id vexpress_osc_of_match[] = {
 	{ .compatible = "arm,vexpress-osc", },
 	{}
 };
+MODULE_DEVICE_TABLE(of, vexpress_osc_of_match);
 
 static struct platform_driver vexpress_osc_driver = {
 	.driver	= {

@@ -116,9 +118,5 @@ static struct platform_driver vexpress_osc_driver = {
 	},
 	.probe = vexpress_osc_probe,
 };
-
-static int __init vexpress_osc_init(void)
-{
-	return platform_driver_register(&vexpress_osc_driver);
-}
-core_initcall(vexpress_osc_init);
+module_platform_driver(vexpress_osc_driver);
+MODULE_LICENSE("GPL v2");

Signature · 1 line

2.20.1

Nothing in the VExpress sysregs nor the MFD child drivers use CONFIG_CLKSRC_MMIO. There's the 24MHz counter, but that's handled by drivers/clocksource/timer-versatile.c which doesn't use CONFIG_CLKSRC_MMIO either. So let's just drop CONFIG_CLKSRC_MMIO.

As the !ARCH_USES_GETTIMEOFFSET dependency was added for CONFIG_CLKSRC_MMIO, that can be dropped, too.

Cc Liviu Dudau <[email protected]>

Cc Sudeep Holla <[email protected]>

Cc Lorenzo Pieralisi <[email protected]>

Cc Linus Walleij <[email protected]>

Cc Lee Jones <[email protected]>

Signed-off-by Rob Herring <[email protected]>

---
 drivers/mfd/Kconfig | 3 +--
 1 file changed, 1 insertion(+), 2 deletions(-)

drivers/mfd/Kconfig +1 -2

index 0a59249198d3..b1311dea2da1 100644
@@ -2029,9 +2029,8 @@ endmenu
 
 config MFD_VEXPRESS_SYSREG
 	bool "Versatile Express System Registers"
-	depends on VEXPRESS_CONFIG && GPIOLIB && !ARCH_USES_GETTIMEOFFSET
+	depends on VEXPRESS_CONFIG && GPIOLIB
 	default y
-	select CLKSRC_MMIO
 	select GPIO_GENERIC_PLATFORM
 	select MFD_CORE
 	select MFD_SYSCON

Signature · 1 line

2.20.1

Cc Liviu Dudau <[email protected]>

Cc Sudeep Holla <[email protected]>

Cc Lorenzo Pieralisi <[email protected]>

Cc Linus Walleij <[email protected]>

Cc Lee Jones <[email protected]>

Signed-off-by Rob Herring <[email protected]>

---
 drivers/mfd/vexpress-sysreg.c | 36 -----------------------------------
 1 file changed, 36 deletions(-)

drivers/mfd/vexpress-sysreg.c +0 -26

index 0b9cc67706c7..90a4eda2ba2b 100644
@@ -43,10 +43,6 @@
 
 /* The sysreg block is just a random collection of various functions... */
 
-static struct syscon_platform_data vexpress_sysreg_sys_id_pdata = {
-	.label = "sys_id",
-};
-
 static struct bgpio_pdata vexpress_sysreg_sys_led_pdata = {
 	.label = "sys_led",
 	.base = -1,

@@ -65,24 +61,8 @@ static struct bgpio_pdata vexpress_sysreg_sys_flash_pdata = {
 	.ngpio = 1,
 };
 
-static struct syscon_platform_data vexpress_sysreg_sys_misc_pdata = {
-	.label = "sys_misc",
-};
-
-static struct syscon_platform_data vexpress_sysreg_sys_procid_pdata = {
-	.label = "sys_procid",
-};
-
 static struct mfd_cell vexpress_sysreg_cells[] = {
 	{
-		.name = "syscon",
-		.num_resources = 1,
-		.resources = (struct resource []) {
-			DEFINE_RES_MEM(SYS_ID, 0x4),
-		},
-		.platform_data = &vexpress_sysreg_sys_id_pdata,
-		.pdata_size = sizeof(vexpress_sysreg_sys_id_pdata),
-	}, {
 		.name = "basic-mmio-gpio",
 		.of_compatible = "arm,vexpress-sysreg,sys_led",
 		.num_resources = 1,

@@ -109,22 +89,6 @@ static struct mfd_cell vexpress_sysreg_cells[] = {
 		},
 		.platform_data = &vexpress_sysreg_sys_flash_pdata,
 		.pdata_size = sizeof(vexpress_sysreg_sys_flash_pdata),
-	}, {
-		.name = "syscon",
-		.num_resources = 1,
-		.resources = (struct resource []) {
-			DEFINE_RES_MEM(SYS_MISC, 0x4),
-		},

drivers/mfd/vexpress-sysreg.c +0 -0

 	}, {
 		.name = "vexpress-syscfg",
 		.num_resources = 1,

Signature · 1 line

2.20.1

Use the managed devm_gpiochip_add_data() and devm_mfd_add_devices() instead of their unmanaged counterparts. With this, no .remove() hook is needed for driver unbind.

Cc Liviu Dudau <[email protected]>

Cc Sudeep Holla <[email protected]>

Cc Lorenzo Pieralisi <[email protected]>

Cc Linus Walleij <[email protected]>

Cc Lee Jones <[email protected]>

Signed-off-by Rob Herring <[email protected]>

---
 drivers/mfd/vexpress-sysreg.c | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

drivers/mfd/vexpress-sysreg.c +2 -2

index 90a4eda2ba2b..9fb37fa689e0 100644
@@ -140,9 +140,9 @@ static int vexpress_sysreg_probe(struct platform_device *pdev)
 	bgpio_init(mmc_gpio_chip, &pdev->dev, 0x4, base + SYS_MCI,
 			NULL, NULL, NULL, NULL, 0);
 	mmc_gpio_chip->ngpio = 2;
-	gpiochip_add_data(mmc_gpio_chip, NULL);
+	devm_gpiochip_add_data(&pdev->dev, mmc_gpio_chip, NULL);
 
-	return mfd_add_devices(&pdev->dev, PLATFORM_DEVID_AUTO,
+	return devm_mfd_add_devices(&pdev->dev, PLATFORM_DEVID_AUTO,
 			vexpress_sysreg_cells,
 			ARRAY_SIZE(vexpress_sysreg_cells), mem, 0, NULL);
 }

Signature · 1 line

2.20.1

The only thing that vexpress-syscfg does is provide a regmap to vexpress-config bus child devices. There's little reason to have 2 components for this. The current structure with initcall ordering requirements makes turning these components into modules more difficult.

So let's start to simplify things and merge vexpress-syscfg into vexpress-config. There's no functional change in this commit and it's still separate components until subsequent commits.

Cc Liviu Dudau <[email protected]>

Cc Sudeep Holla <[email protected]>

Cc Lorenzo Pieralisi <[email protected]>

Cc Linus Walleij <[email protected]>

Cc Arnd Bergmann <[email protected]>

Cc Greg Kroah-Hartman <[email protected]>

Signed-off-by Rob Herring <[email protected]>

---
 arch/arm/mach-vexpress/Kconfig |   1 -
 drivers/bus/vexpress-config.c  | 283 +++++++++++++++++++++++++++++++--
 drivers/misc/Kconfig           |   9 --
 drivers/misc/Makefile          |   1 -
 drivers/misc/vexpress-syscfg.c | 280 --------------------------------
 include/linux/vexpress.h       |  17 --
 6 files changed, 274 insertions(+), 317 deletions(-)
 delete mode 100644 drivers/misc/vexpress-syscfg.c

arch/arm/mach-vexpress/Kconfig +0 -1

index 2d1fdec4c230..065e12991663 100644
@@ -20,7 +20,6 @@ menuconfig ARCH_VEXPRESS
 	select REGULATOR if MMC_ARMMMCI
 	select REGULATOR_FIXED_VOLTAGE if REGULATOR
 	select VEXPRESS_CONFIG
-	select VEXPRESS_SYSCFG
 	help
 	  This option enables support for systems using Cortex processor based
 	  ARM core and logic (FPGA) tiles on the Versatile Express motherboard,

drivers/bus/vexpress-config.c +274 -9

index ff70575b2db6..43f5beac9811 100644
@@ -6,10 +6,48 @@
 
 #include <linux/err.h>
 #include <linux/init.h>
+#include <linux/io.h>
 #include <linux/of.h>
+#include <linux/platform_device.h>
 #include <linux/of_device.h>
+#include <linux/sched/signal.h>
+#include <linux/slab.h>
 #include <linux/vexpress.h>
 
+#define SYS_CFGDATA		0x0
+
+#define SYS_CFGCTRL		0x4
+#define SYS_CFGCTRL_START	(1 << 31)
+#define SYS_CFGCTRL_WRITE	(1 << 30)
+#define SYS_CFGCTRL_DCC(n)	(((n) & 0xf) << 26)
+#define SYS_CFGCTRL_FUNC(n)	(((n) & 0x3f) << 20)
+#define SYS_CFGCTRL_SITE(n)	(((n) & 0x3) << 16)
+#define SYS_CFGCTRL_POSITION(n)	(((n) & 0xf) << 12)
+#define SYS_CFGCTRL_DEVICE(n)	(((n) & 0xfff) << 0)
+
+#define SYS_CFGSTAT		0x8
+#define SYS_CFGSTAT_ERR		(1 << 1)
+#define SYS_CFGSTAT_COMPLETE	(1 << 0)
+
+
+struct vexpress_syscfg {
+	struct device *dev;
+	void __iomem *base;
+	struct list_head funcs;
+};
+
+struct vexpress_syscfg_func {
+	struct list_head list;
+	struct vexpress_syscfg *syscfg;
+	struct regmap *regmap;
+	int num_templates;
+	u32 template[]; /* Keep it last! */
+};
+
+struct vexpress_config_bridge_ops {
+	struct regmap * (*regmap_init)(struct device *dev, void *context);
+	void (*regmap_exit)(struct regmap *regmap, void *context);
+};
 
 struct vexpress_config_bridge {
 	struct vexpress_config_bridge_ops *ops;

@@ -27,17 +65,12 @@ void vexpress_config_set_master(u32 site)
 	vexpress_config_site_master = site;
 }
 
-u32 vexpress_config_get_master(void)
-{
-	return vexpress_config_site_master;
-}
-
-void vexpress_config_lock(void *arg)
+static void vexpress_config_lock(void *arg)
 {
 	mutex_lock(&vexpress_config_mutex);
 }
 
-void vexpress_config_unlock(void *arg)
+static void vexpress_config_unlock(void *arg)
 {
 	mutex_unlock(&vexpress_config_mutex);
 }

@@ -59,7 +92,7 @@ static void vexpress_config_find_prop(struct device_node *node,
 	}
 }
 
-int vexpress_config_get_topo(struct device_node *node, u32 *site,
+static int vexpress_config_get_topo(struct device_node *node, u32 *site,
 		u32 *position, u32 *dcc)
 {
 	vexpress_config_find_prop(node, "arm,vexpress,site", site);

@@ -113,7 +146,7 @@ struct regmap *devm_regmap_init_vexpress_config(struct device *dev)
 }
 EXPORT_SYMBOL_GPL(devm_regmap_init_vexpress_config);
 
-struct device *vexpress_config_bridge_register(struct device *parent,
+static struct device *vexpress_config_bridge_register(struct device *parent,
 		struct vexpress_config_bridge_ops *ops, void *context)
 {
 	struct device *dev;

@@ -201,3 +234,235 @@ static int __init vexpress_config_init(void)
 }
 postcore_initcall(vexpress_config_init);
 
+static int vexpress_syscfg_exec(struct vexpress_syscfg_func *func,
+		int index, bool write, u32 *data)
+{
+	struct vexpress_syscfg *syscfg = func->syscfg;
+	u32 command, status;
+	int tries;
+	long timeout;
+
+	if (WARN_ON(index >= func->num_templates))
+		return -EINVAL;
+
+	command = readl(syscfg->base + SYS_CFGCTRL);
+	if (WARN_ON(command & SYS_CFGCTRL_START))
+		return -EBUSY;
+
+	command = func->template[index];
+	command |= SYS_CFGCTRL_START;
+	command |= write ? SYS_CFGCTRL_WRITE : 0;
+
+	/* Use a canary for reads */
+	if (!write)
+		*data = 0xdeadbeef;
+
+	dev_dbg(syscfg->dev, "func %p, command %x, data %x\n",
+			func, command, *data);
+	writel(*data, syscfg->base + SYS_CFGDATA);
+	writel(0, syscfg->base + SYS_CFGSTAT);
+	writel(command, syscfg->base + SYS_CFGCTRL);
+	mb();
+
+	/* The operation can take ages... Go to sleep, 100us initially */
+	tries = 100;
+	timeout = 100;
+	do {
+		if (!irqs_disabled()) {
+			set_current_state(TASK_INTERRUPTIBLE);
+			schedule_timeout(usecs_to_jiffies(timeout));
+			if (signal_pending(current))
+				return -EINTR;
+		} else {
+			udelay(timeout);
+		}
+
+		status = readl(syscfg->base + SYS_CFGSTAT);
+		if (status & SYS_CFGSTAT_ERR)
+			return -EFAULT;
+
+		if (timeout > 20)
+			timeout -= 20;
+	} while (--tries && !(status & SYS_CFGSTAT_COMPLETE));
+	if (WARN_ON_ONCE(!tries))
+		return -ETIMEDOUT;
+
+	if (!write) {
+		*data = readl(syscfg->base + SYS_CFGDATA);
+		dev_dbg(syscfg->dev, "func %p, read data %x\n", func, *data);
+	}
+
+	return 0;
+}
+
+static int vexpress_syscfg_read(void *context, unsigned int index,
+		unsigned int *val)
+{
+	struct vexpress_syscfg_func *func = context;
+
+	return vexpress_syscfg_exec(func, index, false, val);
+}
+
+static int vexpress_syscfg_write(void *context, unsigned int index,
+		unsigned int val)
+{
+	struct vexpress_syscfg_func *func = context;
+
+	return vexpress_syscfg_exec(func, index, true, &val);
+}
+
+static struct regmap_config vexpress_syscfg_regmap_config = {
+	.lock = vexpress_config_lock,
+	.unlock = vexpress_config_unlock,
+	.reg_bits = 32,
+	.val_bits = 32,
+	.reg_read = vexpress_syscfg_read,
+	.reg_write = vexpress_syscfg_write,
+	.reg_format_endian = REGMAP_ENDIAN_LITTLE,
+	.val_format_endian = REGMAP_ENDIAN_LITTLE,
+};
+
+
+static struct regmap *vexpress_syscfg_regmap_init(struct device *dev,
+		void *context)
+{
+	int err;
+	struct vexpress_syscfg *syscfg = context;
+	struct vexpress_syscfg_func *func;
+	struct property *prop;
+	const __be32 *val = NULL;
+	__be32 energy_quirk[4];
+	int num;
+	u32 site, position, dcc;
+	int i;
+
+	err = vexpress_config_get_topo(dev->of_node, &site,
+				&position, &dcc);
+	if (err)
+		return ERR_PTR(err);
+
+	prop = of_find_property(dev->of_node,
+			"arm,vexpress-sysreg,func", NULL);
+	if (!prop)
+		return ERR_PTR(-EINVAL);
+
+	num = prop->length / sizeof(u32) / 2;
+	val = prop->value;
+
+	/*
+	 * "arm,vexpress-energy" function used to be described
+	 * by its first device only, now it requires both
+	 */
+	if (num == 1 && of_device_is_compatible(dev->of_node,
+			"arm,vexpress-energy")) {
+		num = 2;
+		energy_quirk[0] = *val;
+		energy_quirk[2] = *val++;
+		energy_quirk[1] = *val;
+		energy_quirk[3] = cpu_to_be32(be32_to_cpup(val) + 1);
+		val = energy_quirk;
+	}
+
+	func = kzalloc(struct_size(func, template, num), GFP_KERNEL);
+	if (!func)
+		return ERR_PTR(-ENOMEM);
+
+	func->syscfg = syscfg;
+	func->num_templates = num;
+
+	for (i = 0; i < num; i++) {
+		u32 function, device;
+
+		function = be32_to_cpup(val++);
+		device = be32_to_cpup(val++);
+
+		dev_dbg(dev, "func %p: %u/%u/%u/%u/%u\n",
+				func, site, position, dcc,
+				function, device);
+
+		func->template[i] = SYS_CFGCTRL_DCC(dcc);
+		func->template[i] |= SYS_CFGCTRL_SITE(site);
+		func->template[i] |= SYS_CFGCTRL_POSITION(position);
+		func->template[i] |= SYS_CFGCTRL_FUNC(function);
+		func->template[i] |= SYS_CFGCTRL_DEVICE(device);
+	}
+
+	vexpress_syscfg_regmap_config.max_register = num - 1;
+
+	func->regmap = regmap_init(dev, NULL, func,
+			&vexpress_syscfg_regmap_config);
+
+	if (IS_ERR(func->regmap)) {
+		void *err = func->regmap;
+
+		kfree(func);
+		return err;
+	}
+
+	list_add(&func->list, &syscfg->funcs);
+
+	return func->regmap;
+}
+
+static void vexpress_syscfg_regmap_exit(struct regmap *regmap, void *context)
+{
+	struct vexpress_syscfg *syscfg = context;
+	struct vexpress_syscfg_func *func, *tmp;
+
+	regmap_exit(regmap);
+
+	list_for_each_entry_safe(func, tmp, &syscfg->funcs, list) {
+		if (func->regmap == regmap) {
+			list_del(&syscfg->funcs);
+			kfree(func);
+			break;
+		}
+	}
+}
+
+static struct vexpress_config_bridge_ops vexpress_syscfg_bridge_ops = {
+	.regmap_init = vexpress_syscfg_regmap_init,
+	.regmap_exit = vexpress_syscfg_regmap_exit,
+};
+
+
+static int vexpress_syscfg_probe(struct platform_device *pdev)
+{
+	struct vexpress_syscfg *syscfg;
+	struct resource *res;
+	struct device *bridge;
+
+	syscfg = devm_kzalloc(&pdev->dev, sizeof(*syscfg), GFP_KERNEL);
+	if (!syscfg)
+		return -ENOMEM;
+	syscfg->dev = &pdev->dev;
+	INIT_LIST_HEAD(&syscfg->funcs);
+
+	res = platform_get_resource(pdev, IORESOURCE_MEM, 0);
+	syscfg->base = devm_ioremap_resource(&pdev->dev, res);
+	if (IS_ERR(syscfg->base))
+		return PTR_ERR(syscfg->base);
+
+	/* Must use dev.parent (MFD), as that's where DT phandle points at... */
+	bridge = vexpress_config_bridge_register(pdev->dev.parent,
+			&vexpress_syscfg_bridge_ops, syscfg);
+
+	return PTR_ERR_OR_ZERO(bridge);
+}
+
+static const struct platform_device_id vexpress_syscfg_id_table[] = {
+	{ "vexpress-syscfg", },
+	{},
+};
+
+static struct platform_driver vexpress_syscfg_driver = {
+	.driver.name = "vexpress-syscfg",
+	.id_table = vexpress_syscfg_id_table,
+	.probe = vexpress_syscfg_probe,
+};
+
+static int __init vexpress_syscfg_init(void)
+{
+	return platform_driver_register(&vexpress_syscfg_driver);
+}
+core_initcall(vexpress_syscfg_init);

drivers/misc/Kconfig +0 -9

index 99e151475d8f..edd5dd5ebfdc 100644
@@ -423,15 +423,6 @@ config SRAM
 config SRAM_EXEC
 	bool
 
-config VEXPRESS_SYSCFG
-	bool "Versatile Express System Configuration driver"
-	depends on VEXPRESS_CONFIG
-	default y
-	help
-	  ARM Ltd. Versatile Express uses specialised platform configuration
-	  bus. System Configuration interface is one of the possible means
-	  of generating transactions on this bus.
-
 config PCI_ENDPOINT_TEST
 	depends on PCI
 	select CRC32

drivers/misc/Makefile +0 -1

index 9abf2923d831..c7bd01ac6291 100644
@@ -49,7 +49,6 @@ obj-$(CONFIG_SRAM_EXEC)		+= sram-exec.o
 obj-y				+= mic/
 obj-$(CONFIG_GENWQE)		+= genwqe/
 obj-$(CONFIG_ECHO)		+= echo/
-obj-$(CONFIG_VEXPRESS_SYSCFG)	+= vexpress-syscfg.o
 obj-$(CONFIG_CXL_BASE)		+= cxl/
 obj-$(CONFIG_PCI_ENDPOINT_TEST)	+= pci_endpoint_test.o
 obj-$(CONFIG_OCXL)		+= ocxl/

drivers/misc/vexpress-syscfg.c deleted +0 -280

deleted file mode 100644
index a431787c0898..000000000000
@@ -1,280 +0,0 @@
-// SPDX-License-Identifier: GPL-2.0-only
-/*
- *
- * Copyright (C) 2014 ARM Limited
- */
-
-#include <linux/delay.h>
-#include <linux/err.h>
-#include <linux/io.h>
-#include <linux/of.h>
-#include <linux/platform_device.h>
-#include <linux/sched/signal.h>
-#include <linux/slab.h>
-#include <linux/syscore_ops.h>
-#include <linux/vexpress.h>
-
-
-#define SYS_CFGDATA		0x0
-
-#define SYS_CFGCTRL		0x4
-#define SYS_CFGCTRL_START	(1 << 31)
-#define SYS_CFGCTRL_WRITE	(1 << 30)
-#define SYS_CFGCTRL_DCC(n)	(((n) & 0xf) << 26)
-#define SYS_CFGCTRL_FUNC(n)	(((n) & 0x3f) << 20)
-#define SYS_CFGCTRL_SITE(n)	(((n) & 0x3) << 16)
-#define SYS_CFGCTRL_POSITION(n)	(((n) & 0xf) << 12)
-#define SYS_CFGCTRL_DEVICE(n)	(((n) & 0xfff) << 0)
-
-#define SYS_CFGSTAT		0x8
-#define SYS_CFGSTAT_ERR		(1 << 1)
-#define SYS_CFGSTAT_COMPLETE	(1 << 0)
-
-
-struct vexpress_syscfg {
-	struct device *dev;
-	void __iomem *base;
-	struct list_head funcs;
-};
-
-struct vexpress_syscfg_func {
-	struct list_head list;
-	struct vexpress_syscfg *syscfg;
-	struct regmap *regmap;
-	int num_templates;
-	u32 template[]; /* Keep it last! */
-};
-
-
-static int vexpress_syscfg_exec(struct vexpress_syscfg_func *func,
-		int index, bool write, u32 *data)
-{
-	struct vexpress_syscfg *syscfg = func->syscfg;
-	u32 command, status;
-	int tries;
-	long timeout;
-
-	if (WARN_ON(index >= func->num_templates))
-		return -EINVAL;
-
-	command = readl(syscfg->base + SYS_CFGCTRL);
-	if (WARN_ON(command & SYS_CFGCTRL_START))
-		return -EBUSY;
-
-	command = func->template[index];
-	command |= SYS_CFGCTRL_START;
-	command |= write ? SYS_CFGCTRL_WRITE : 0;
-
-	/* Use a canary for reads */
-	if (!write)
-		*data = 0xdeadbeef;
-
-	dev_dbg(syscfg->dev, "func %p, command %x, data %x\n",
-			func, command, *data);
-	writel(*data, syscfg->base + SYS_CFGDATA);
-	writel(0, syscfg->base + SYS_CFGSTAT);
-	writel(command, syscfg->base + SYS_CFGCTRL);
-	mb();
-
-	/* The operation can take ages... Go to sleep, 100us initially */
-	tries = 100;
-	timeout = 100;
-	do {
-		if (!irqs_disabled()) {
-			set_current_state(TASK_INTERRUPTIBLE);
-			schedule_timeout(usecs_to_jiffies(timeout));
-			if (signal_pending(current))
-				return -EINTR;
-		} else {
-			udelay(timeout);
-		}
-
-		status = readl(syscfg->base + SYS_CFGSTAT);
-		if (status & SYS_CFGSTAT_ERR)
-			return -EFAULT;
-
-		if (timeout > 20)
-			timeout -= 20;
-	} while (--tries && !(status & SYS_CFGSTAT_COMPLETE));
-	if (WARN_ON_ONCE(!tries))
-		return -ETIMEDOUT;
-
-	if (!write) {
-		*data = readl(syscfg->base + SYS_CFGDATA);
-		dev_dbg(syscfg->dev, "func %p, read data %x\n", func, *data);
-	}
-
-	return 0;
-}
-
-static int vexpress_syscfg_read(void *context, unsigned int index,
-		unsigned int *val)
-{
-	struct vexpress_syscfg_func *func = context;
-
-	return vexpress_syscfg_exec(func, index, false, val);
-}
-
-static int vexpress_syscfg_write(void *context, unsigned int index,
-		unsigned int val)
-{
-	struct vexpress_syscfg_func *func = context;
-
-	return vexpress_syscfg_exec(func, index, true, &val);
-}
-
-static struct regmap_config vexpress_syscfg_regmap_config = {
-	.lock = vexpress_config_lock,
-	.unlock = vexpress_config_unlock,
-	.reg_bits = 32,
-	.val_bits = 32,
-	.reg_read = vexpress_syscfg_read,
-	.reg_write = vexpress_syscfg_write,
-	.reg_format_endian = REGMAP_ENDIAN_LITTLE,
-	.val_format_endian = REGMAP_ENDIAN_LITTLE,
-};
-
-
-static struct regmap *vexpress_syscfg_regmap_init(struct device *dev,
-		void *context)
-{
-	int err;
-	struct vexpress_syscfg *syscfg = context;
-	struct vexpress_syscfg_func *func;
-	struct property *prop;
-	const __be32 *val = NULL;
-	__be32 energy_quirk[4];
-	int num;
-	u32 site, position, dcc;
-	int i;
-
-	err = vexpress_config_get_topo(dev->of_node, &site,
-				&position, &dcc);
-	if (err)
-		return ERR_PTR(err);
-
-	prop = of_find_property(dev->of_node,
-			"arm,vexpress-sysreg,func", NULL);
-	if (!prop)
-		return ERR_PTR(-EINVAL);
-
-	num = prop->length / sizeof(u32) / 2;
-	val = prop->value;
-
-	/*
-	 * "arm,vexpress-energy" function used to be described
-	 * by its first device only, now it requires both
-	 */
-	if (num == 1 && of_device_is_compatible(dev->of_node,
-			"arm,vexpress-energy")) {
-		num = 2;
-		energy_quirk[0] = *val;
-		energy_quirk[2] = *val++;
-		energy_quirk[1] = *val;
-		energy_quirk[3] = cpu_to_be32(be32_to_cpup(val) + 1);
-		val = energy_quirk;
-	}
-
-	func = kzalloc(struct_size(func, template, num), GFP_KERNEL);
-	if (!func)
-		return ERR_PTR(-ENOMEM);
-
-	func->syscfg = syscfg;
-	func->num_templates = num;
-
-	for (i = 0; i < num; i++) {
-		u32 function, device;
-
-		function = be32_to_cpup(val++);
-		device = be32_to_cpup(val++);
-
-		dev_dbg(dev, "func %p: %u/%u/%u/%u/%u\n",
-				func, site, position, dcc,
-				function, device);
-
-		func->template[i] = SYS_CFGCTRL_DCC(dcc);
-		func->template[i] |= SYS_CFGCTRL_SITE(site);
-		func->template[i] |= SYS_CFGCTRL_POSITION(position);
-		func->template[i] |= SYS_CFGCTRL_FUNC(function);
-		func->template[i] |= SYS_CFGCTRL_DEVICE(device);
-	}
-
-	vexpress_syscfg_regmap_config.max_register = num - 1;
-
-	func->regmap = regmap_init(dev, NULL, func,
-			&vexpress_syscfg_regmap_config);
-
-	if (IS_ERR(func->regmap)) {
-		void *err = func->regmap;
-
-		kfree(func);
-		return err;
-	}
-
-	list_add(&func->list, &syscfg->funcs);
-
-	return func->regmap;
-}
-
-static void vexpress_syscfg_regmap_exit(struct regmap *regmap, void *context)
-{
-	struct vexpress_syscfg *syscfg = context;
-	struct vexpress_syscfg_func *func, *tmp;
-
-	regmap_exit(regmap);
-
-	list_for_each_entry_safe(func, tmp, &syscfg->funcs, list) {
-		if (func->regmap == regmap) {
-			list_del(&syscfg->funcs);
-			kfree(func);
-			break;
-		}
-	}
-}
-
-static struct vexpress_config_bridge_ops vexpress_syscfg_bridge_ops = {
-	.regmap_init = vexpress_syscfg_regmap_init,
-	.regmap_exit = vexpress_syscfg_regmap_exit,
-};
-
-
-static int vexpress_syscfg_probe(struct platform_device *pdev)
-{
-	struct vexpress_syscfg *syscfg;
-	struct resource *res;
-	struct device *bridge;
-
-	syscfg = devm_kzalloc(&pdev->dev, sizeof(*syscfg), GFP_KERNEL);
-	if (!syscfg)
-		return -ENOMEM;
-	syscfg->dev = &pdev->dev;
-	INIT_LIST_HEAD(&syscfg->funcs);
-
-	res = platform_get_resource(pdev, IORESOURCE_MEM, 0);
-	syscfg->base = devm_ioremap_resource(&pdev->dev, res);
-	if (IS_ERR(syscfg->base))
-		return PTR_ERR(syscfg->base);
-
-	/* Must use dev.parent (MFD), as that's where DT phandle points at... */
-	bridge = vexpress_config_bridge_register(pdev->dev.parent,
-			&vexpress_syscfg_bridge_ops, syscfg);
-
-	return PTR_ERR_OR_ZERO(bridge);
-}
-
-static const struct platform_device_id vexpress_syscfg_id_table[] = {
-	{ "vexpress-syscfg", },
-	{},
-};
-
-static struct platform_driver vexpress_syscfg_driver = {
-	.driver.name = "vexpress-syscfg",
-	.id_table = vexpress_syscfg_id_table,
-	.probe = vexpress_syscfg_probe,
-};
-
-static int __init vexpress_syscfg_init(void)
-{
-	return platform_driver_register(&vexpress_syscfg_driver);
-}
-core_initcall(vexpress_syscfg_init);

include/linux/vexpress.h +0 -17

index 2ec7992b054c..65096c792d57 100644
@@ -18,23 +18,6 @@
 /* Config infrastructure */
 
 void vexpress_config_set_master(u32 site);
-u32 vexpress_config_get_master(void);
-
-void vexpress_config_lock(void *arg);
-void vexpress_config_unlock(void *arg);
-
-int vexpress_config_get_topo(struct device_node *node, u32 *site,
-		u32 *position, u32 *dcc);
-
-/* Config bridge API */
-
-struct vexpress_config_bridge_ops {
-	struct regmap * (*regmap_init)(struct device *dev, void *context);
-	void (*regmap_exit)(struct regmap *regmap, void *context);
-};
-
-struct device *vexpress_config_bridge_register(struct device *parent,
-		struct vexpress_config_bridge_ops *ops, void *context);
 
 /* Config regmap API */
 

Signature · 1 line

2.20.1

The vexpress-config initialization is dependent on the vexpress-syscfg driver probing. As vexpress-config was not a driver, deferred probe could not be used and instead initcall ordering was relied upon. This is fragile and doesn't work for modules.

Let's move the config bus init into the vexpress-syscfg probe. This eliminates the initcall ordering requirement and the need to create a struct device and the "vexpress-config" class.

Cc Liviu Dudau <[email protected]>

Cc Sudeep Holla <[email protected]>

Cc Lorenzo Pieralisi <[email protected]>

Cc Linus Walleij <[email protected]>

Cc Arnd Bergmann <[email protected]>

Cc Greg Kroah-Hartman <[email protected]>

Signed-off-by Rob Herring <[email protected]>

---
 drivers/bus/vexpress-config.c | 118 ++++++----------------------------
 1 file changed, 21 insertions(+), 97 deletions(-)

drivers/bus/vexpress-config.c +21 -97

index 43f5beac9811..43deb4df140b 100644
@@ -56,7 +56,6 @@ struct vexpress_config_bridge {
 
 
 static DEFINE_MUTEX(vexpress_config_mutex);
-static struct class *vexpress_config_class;
 static u32 vexpress_config_site_master = VEXPRESS_SITE_MASTER;
 
 

@@ -121,9 +120,6 @@ struct regmap *devm_regmap_init_vexpress_config(struct device *dev)
 	struct regmap *regmap;
 	struct regmap **res;
 
-	if (WARN_ON(dev->parent->class != vexpress_config_class))
-		return ERR_PTR(-ENODEV);
-
 	bridge = dev_get_drvdata(dev->parent);
 	if (WARN_ON(!bridge))
 		return ERR_PTR(-EINVAL);

@@ -146,94 +142,6 @@ struct regmap *devm_regmap_init_vexpress_config(struct device *dev)
 }
 EXPORT_SYMBOL_GPL(devm_regmap_init_vexpress_config);
 
-static struct device *vexpress_config_bridge_register(struct device *parent,
-		struct vexpress_config_bridge_ops *ops, void *context)
-{
-	struct device *dev;
-	struct vexpress_config_bridge *bridge;
-
-	if (!vexpress_config_class) {
-		vexpress_config_class = class_create(THIS_MODULE,
-				"vexpress-config");
-		if (IS_ERR(vexpress_config_class))
-			return (void *)vexpress_config_class;
-	}
-
-	dev = device_create(vexpress_config_class, parent, 0,
-			NULL, "%s.bridge", dev_name(parent));
-
-	if (IS_ERR(dev))
-		return dev;
-
-	bridge = devm_kmalloc(dev, sizeof(*bridge), GFP_KERNEL);
-	if (!bridge) {
-		put_device(dev);
-		device_unregister(dev);
-		return ERR_PTR(-ENOMEM);
-	}
-	bridge->ops = ops;
-	bridge->context = context;
-
-	dev_set_drvdata(dev, bridge);
-
-	dev_dbg(parent, "Registered bridge '%s', parent node %p\n",
-			dev_name(dev), parent->of_node);
-
-	return dev;
-}
-
-
-static int vexpress_config_node_match(struct device *dev, const void *data)
-{
-	const struct device_node *node = data;
-
-	dev_dbg(dev, "Parent node %p, looking for %p\n",
-			dev->parent->of_node, node);
-
-	return dev->parent->of_node == node;
-}
-
-static int vexpress_config_populate(struct device_node *node)
-{
-	struct device_node *bridge;
-	struct device *parent;
-	int ret;
-
-	bridge = of_parse_phandle(node, "arm,vexpress,config-bridge", 0);
-	if (!bridge)
-		return -EINVAL;
-
-	parent = class_find_device(vexpress_config_class, NULL, bridge,
-			vexpress_config_node_match);
-	of_node_put(bridge);
-	if (WARN_ON(!parent))
-		return -ENODEV;
-
-	ret = of_platform_populate(node, NULL, NULL, parent);
-
-	put_device(parent);
-
-	return ret;
-}
-
-static int __init vexpress_config_init(void)
-{
-	int err = 0;
-	struct device_node *node;
-
-	/* Need the config devices early, before the "normal" devices... */
-	for_each_compatible_node(node, NULL, "arm,vexpress,config-bus") {
-		err = vexpress_config_populate(node);
-		if (err) {
-			of_node_put(node);
-			break;
-		}
-	}
-
-	return err;
-}
-postcore_initcall(vexpress_config_init);
-
 static int vexpress_syscfg_exec(struct vexpress_syscfg_func *func,
 		int index, bool write, u32 *data)
 {

@@ -430,7 +338,8 @@ static int vexpress_syscfg_probe(struct platform_device *pdev)
 {
 	struct vexpress_syscfg *syscfg;
 	struct resource *res;
-	struct device *bridge;
+	struct vexpress_config_bridge *bridge;
+	struct device_node *node;
 
 	syscfg = devm_kzalloc(&pdev->dev, sizeof(*syscfg), GFP_KERNEL);
 	if (!syscfg)

@@ -443,11 +352,26 @@ static int vexpress_syscfg_probe(struct platform_device *pdev)
 	if (IS_ERR(syscfg->base))
 		return PTR_ERR(syscfg->base);
 
-	/* Must use dev.parent (MFD), as that's where DT phandle points at... */
-	bridge = vexpress_config_bridge_register(pdev->dev.parent,
-			&vexpress_syscfg_bridge_ops, syscfg);
+	bridge = devm_kmalloc(&pdev->dev, sizeof(*bridge), GFP_KERNEL);
+	if (!bridge)
+		return -ENOMEM;
+
+	bridge->ops = &vexpress_syscfg_bridge_ops;
+	bridge->context = syscfg;
+
+	dev_set_drvdata(&pdev->dev, bridge);
 
-	return PTR_ERR_OR_ZERO(bridge);
+	for_each_compatible_node(node, NULL, "arm,vexpress,config-bus") {
+		struct device_node *bridge_np;
+
+		bridge_np = of_parse_phandle(node, "arm,vexpress,config-bridge", 0);
+		if (bridge_np != pdev->dev.parent->of_node)
+			continue;
+
+		of_platform_populate(node, NULL, NULL, &pdev->dev);
+	}
+
+	return 0;
 }
 
 static const struct platform_device_id vexpress_syscfg_id_table[] = {

Signature · 1 line

2.20.1

Is the subject supposed to say 'set master' vs. 'site master'?

Quoting Rob Herring (2020-04-19 10:08:08)

Quoted 13 lines

There's only a single caller of vexpress_config_set_master() from vexpress-sysreg.c. Let's just make the registers needed available to vexpress-config and move all the code there. The registers needed aren't used anywhere else either. With this, we can get rid of the private API between these 2 drivers.

Cc Liviu Dudau <[email protected]>

Cc Sudeep Holla <[email protected]>

Cc Lorenzo Pieralisi <[email protected]>

Cc Linus Walleij <[email protected]>

Cc Lee Jones <[email protected]>

Cc Greg Kroah-Hartman <[email protected]>

Signed-off-by Rob Herring <[email protected]>

Enable building vexpress-config driver as a module.

Cc Liviu Dudau <[email protected]>

Cc Sudeep Holla <[email protected]>

Cc Lorenzo Pieralisi <[email protected]>

Cc Linus Walleij <[email protected]>

Cc Greg Kroah-Hartman <[email protected]>

Signed-off-by Rob Herring <[email protected]>

---
 drivers/bus/Kconfig           |  2 +-
 drivers/bus/vexpress-config.c | 10 ++++------
 2 files changed, 5 insertions(+), 7 deletions(-)

drivers/bus/Kconfig +1 -1

index 6d4e4497b59b..c16268c53831 100644
@@ -183,7 +183,7 @@ config UNIPHIER_SYSTEM_BUS
 	  needed to use on-board devices connected to UniPhier SoCs.
 
 config VEXPRESS_CONFIG
-	bool "Versatile Express configuration bus"
+	tristate "Versatile Express configuration bus"
 	default y if ARCH_VEXPRESS
 	depends on ARM || ARM64
 	depends on OF

drivers/bus/vexpress-config.c +4 -6

index caa35a4cb34d..a58ac0c8e282 100644
@@ -7,6 +7,7 @@
 #include <linux/err.h>
 #include <linux/init.h>
 #include <linux/io.h>
+#include <linux/module.h>
 #include <linux/of.h>
 #include <linux/platform_device.h>
 #include <linux/of_device.h>

@@ -407,15 +408,12 @@ static const struct platform_device_id vexpress_syscfg_id_table[] = {
 	{ "vexpress-syscfg", },
 	{},
 };
+MODULE_DEVICE_TABLE(platform, vexpress_syscfg_id_table);
 
 static struct platform_driver vexpress_syscfg_driver = {
 	.driver.name = "vexpress-syscfg",
 	.id_table = vexpress_syscfg_id_table,
 	.probe = vexpress_syscfg_probe,
 };
-
-static int __init vexpress_syscfg_init(void)
-{
-	return platform_driver_register(&vexpress_syscfg_driver);
-}
-core_initcall(vexpress_syscfg_init);
+module_platform_driver(vexpress_syscfg_driver);
+MODULE_LICENSE("GPL v2");

Signature · 1 line

2.20.1

CONFIG_VEXPRESS_CONFIG has 'default y if ARCH_VEXPRESS', so selecting is unnecessary. Selecting it also prevents setting CONFIG_VEXPRESS_CONFIG to a module.

Cc Liviu Dudau <[email protected]>

Cc Sudeep Holla <[email protected]>

Cc Lorenzo Pieralisi <[email protected]>

Cc Linus Walleij <[email protected]>

Signed-off-by Rob Herring <[email protected]>

---
 arch/arm/mach-vexpress/Kconfig | 1 -
 1 file changed, 1 deletion(-)

arch/arm/mach-vexpress/Kconfig +0 -1

index 065e12991663..8391a5b3cd78 100644
@@ -19,7 +19,6 @@ 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,

Signature · 1 line

2.20.1

vexpress_flags_set() is only used by the platform SMP related code and has nothing to do with the vexpress-sysreg MFD driver other than both access the same h/w block. It's also only needed for 32-bit systems and must be built-in for them. Let's move vexpress_flags_set() closer to where it is being used. This will allow for vexpress-sysreg to be built as a module.

Cc Liviu Dudau <[email protected]>

Cc Sudeep Holla <[email protected]>

The VExpress power-off/reset driver is not needed on 64-bit platforms as PSCI power-off and reset can be used instead. Stop selecting it so it can be disabled and not always built-in.

Cc Catalin Marinas <[email protected]>

Cc Will Deacon <[email protected]>

Cc Liviu Dudau <[email protected]>

Cc Sudeep Holla <[email protected]>

CONFIG_VEXPRESS_CONFIG has 'default y if ARCH_VEXPRESS', so selecting is unnecessary. It also prevents setting CONFIG_VEXPRESS_CONFIG to a module.

Cc Catalin Marinas <[email protected]>

Cc Will Deacon <[email protected]>

Cc Liviu Dudau <[email protected]>

Cc Sudeep Holla <[email protected]>