[RFC PATCHv2] ARM: mvebu: Let the device-tree determine smp_ops

[Chris Packham]

Hi Thomas, Maxime

On Mon, 17 Nov 2014, Thomas Petazzoni wrote:

> Dear Chris Packham,
>
> On Fri,  7 Nov 2014 15:33:46 +1300, Chris Packham wrote:
>> The machine specific SMP operations can be configured either via
>> setup_arch or via arm_dt_init_cpu_maps. For the ARMADA_370_XP_DT devices
>> both of these are called and setup_arch wins because it is called last.
>> This means that it is not possible to substitute a different set of SMP
>> operations via the device-tree.
>>
>> For the ARMADA_370_XP_DT compatible devices add a smp_init function that
>> detects if the device tree has an enable-method defined. If it does
>> return true to indicate that the smp_ops have already been set which
>> will prevent setup_arch from overriding them.
>>
>> Signed-off-by: Chris Packham <chris.packham at alliedtelesis.co.nz>
>
> My colleague Maxime Ripard (in Cc) rightfully suggests exploring a
> different option: what about getting rid completely of the .smp field
> of the DT_MACHINE structure, and instead have some code run early
> enough that looks if an enable-method is defined, and if not, defines
> it to the default value. This way, we continue to be backward
> compatible in terms of DT, but we always use the enable-method from the
> DT, and not sometimes from DT, sometimes from the DT_MACHINE structure.
>
> Unfortunately, it will have to be done on the flattened DT, because the
> DT is unflattened right before the enable-method properties are looked
> up:
>
>        unflatten_device_tree();
>
>        arm_dt_init_cpu_maps();
>
> And manipulating the DT in its flattened format, while possible in
> ->dt_fixup(), is a pain, and probably doesn't allow adding new
> properties anyway.
>
> So, in the end, maybe this idea doesn't work, I haven't checked
> completely.
>

So yeah it's tricky to work with the flattened dt. Not impossible the 
powerpc arch code does quite a lot with it. Arm does less but still uses 
it in atags_to_fdt.c. Below is a rough attempt at an implementation that 
seems to work. Because of the flattened dt it's harder to iterate through 
the cpu nodes so I haven't implemented anything to look for an 
enable-method attached to them.

---- 8< ----
Subject: [PATCH] ARM: mvebu: use dt_fixup to provide fallback for
  enable-method

When the device tree doesn't define an enable-method insert a property
into the flattened device tree. arm_dt_init_cpu_maps() will then parse
this an set smp_ops appropriately. Now that we have this fallback it is
no longer necessary to set .smp in the DT_MACHINE definition.

Signed-off-by: Chris Packham <chris.packham at alliedtelesis.co.nz>
---
  arch/arm/mach-mvebu/Makefile   |  2 ++
  arch/arm/mach-mvebu/board-v7.c | 23 ++++++++++++++++++++++-
  2 files changed, 24 insertions(+), 1 deletion(-)

diff --git a/arch/arm/mach-mvebu/Makefile b/arch/arm/mach-mvebu/Makefile
index 1636cdb..f818eaf 100644
--- a/arch/arm/mach-mvebu/Makefile
+++ b/arch/arm/mach-mvebu/Makefile
@@ -14,3 +14,5 @@ endif

  obj-$(CONFIG_MACH_DOVE)		 += dove.o
  obj-$(CONFIG_MACH_KIRKWOOD)	 += kirkwood.o kirkwood-pm.o
+
+CFLAGS_board-v7.o = -I$(src)/../../../scripts/dtc/libfdt
\ No newline at end of file
diff --git a/arch/arm/mach-mvebu/board-v7.c 
b/arch/arm/mach-mvebu/board-v7.c
index b2524d6..45851a2 100644
--- a/arch/arm/mach-mvebu/board-v7.c
+++ b/arch/arm/mach-mvebu/board-v7.c
@@ -17,6 +17,8 @@
  #include <linux/clk-provider.h>
  #include <linux/of_address.h>
  #include <linux/of_platform.h>
+#include <linux/of_fdt.h>
+#include <linux/libfdt.h>
  #include <linux/io.h>
  #include <linux/clocksource.h>
  #include <linux/dma-mapping.h>
@@ -184,6 +186,25 @@ static void __init mvebu_dt_init(void)
  	of_platform_populate(NULL, of_default_bus_match_table, NULL, 
NULL);
  }

+static void __init armada_370_xp_dt_fixup(void)
+{
+	void *prop;
+	int offset;
+	int len;
+
+	offset = fdt_path_offset(initial_boot_params, "/cpus");
+	prop = fdt_getprop(initial_boot_params, offset, "enable-method", 
&len);
+
+	if (!prop) {
+		pr_info("No enable-method defined. "
+			"Falling back to \"marvell,armada-xp-smp\"\n");
+
+		fdt_appendprop(initial_boot_params, offset, 
"enable-method",
+			       "marvell,armada-xp-smp",
+			       sizeof("marvell,armada-xp-smp"));
+	}
+}
+
  static const char * const armada_370_xp_dt_compat[] = {
  	"marvell,armada-370-xp",
  	NULL,
@@ -192,11 +213,11 @@ static const char * const armada_370_xp_dt_compat[] 
= {
  DT_MACHINE_START(ARMADA_370_XP_DT, "Marvell Armada 370/XP (Device Tree)")
  	.l2c_aux_val	= 0,
  	.l2c_aux_mask	= ~0,
-	.smp		= smp_ops(armada_xp_smp_ops),
  	.init_machine	= mvebu_dt_init,
  	.init_irq       = mvebu_init_irq,
  	.restart	= mvebu_restart,
  	.dt_compat	= armada_370_xp_dt_compat,
+	.dt_fixup	= armada_370_xp_dt_fixup,
  MACHINE_END

  static const char * const armada_375_dt_compat[] = {
-- 
2.2.0.rc0