ARM: mvebu: Let the device-tree determine smp_ops

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.

All of the ARMADA_370_XP_DT compatible devices are either single core or declare an enable-method in the dts (via one of armada-xp-mv78230.dtsi, armada-xp-mv78260.dtsi or armada-xp-mv78460.dtsi). Remove the smp assignment from board-v7.c so that the SMP operations set via the device-tree aren't discarded.

Signed-off-by Chris Packham <[email protected]>

---
Hi,

(This is the first patch I've sent to the arm-lkml so beware of my newbie-ness)

I'm working on a new board that uses a integrated CPU that according to the vendor is "based on the ARMADA-XP", more on that to come in the future hopefully.

For the most part things just work if I use a .dts based on the mv78260.

This is my attempt at fixing the problem by not setting mdesc.smp for the ARMADA_370_XP_DT machines thus preventing setup_arch from overriding what has been setup by arm_dt_init_cpu_maps.

Quoted 16 lines · Chris Packham

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.

All of the ARMADA_370_XP_DT compatible devices are either single core or declare an enable-method in the dts (via one of armada-xp-mv78230.dtsi, armada-xp-mv78260.dtsi or armada-xp-mv78460.dtsi). Remove the smp assignment from board-v7.c so that the SMP operations set via the device-tree aren't discarded.

Signed-off-by Chris Packham <[email protected]>

---
Hi,

Quoted 1 line

(This is the first patch I've sent to the arm-lkml so beware of my newbie-ness)

Hi Chris

Congratulations on a good looking patch. Nice changelog, has a signed-off-by, comments are bellow the ---. All good.

It is however a good idea to Cc: the mvebu maintainers. I added a few people to Cc:.

Quoted 3 lines

I'm working on a new board that uses a integrated CPU that according to the vendor is "based on the ARMADA-XP", more on that to come in the future hopefully.

A packet processor chip with an embedded CPU? Next generation Prestera? No need to answer that if you don't want to.

Quoted 6 lines

For the most part things just work if I use a .dts based on the mv78260.

This is my attempt at fixing the problem by not setting mdesc.smp for the ARMADA_370_XP_DT machines thus preventing setup_arch from overriding what has been setup by arm_dt_init_cpu_maps.

You really need Thomas or Gregory to Ack this patch, but to me it looks correct. Interestingly, this machine descriptor is used by 370, which is a single processor. The smp operations are useless, so your patch is also partly a cleanup.

Andrew

Quoted 5 lines

Quoted 3 lines

I'm working on a new board that uses a integrated CPU that according to the vendor is "based on the ARMADA-XP", more on that to come in the future hopefully.

A packet processor chip with an embedded CPU? Next generation Prestera? No need to answer that if you don't want to.

You guess correctly. Not sure how much I can say without falling foul of our NDA but they know I intend on trying to get this stuff upstream rather than being locked into their particular SDK.

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.

All of the ARMADA_370_XP_DT compatible devices are either single core or declare an enable-method in the dts (via one of armada-xp-mv78230.dtsi, armada-xp-mv78260.dtsi or armada-xp-mv78460.dtsi). Remove the smp assignment from board-v7.c so that the SMP operations set via the device-tree aren't discarded.

Signed-off-by Chris Packham <[email protected]>

---
Hi,

(This is the first patch I've sent to the arm-lkml so beware of my newbie-ness)

I'm working on a new board that uses a integrated CPU that according to the vendor is "based on the ARMADA-XP", more on that to come in the future hopefully. For the most part things just work if I use a .dts based on the mv78260.

One difference I've encountered is in the way the secondary CPU is initialised, which requires me to supply a slightly different set of machine specific SMP operations so I can bring up the 2nd core. It looks like I should be able to supply a different /cpus/enable-method in the .dts and once I define the corresponding set of operations it should be picked up.

Which leads me to what I think could be a bug (or just a lack of clear specification). The smp ops are set by both arm_dt_init_cpu_maps (from the enable-method) and setup_arch (from mdesc). The latter always wins because it is called last and doesn't check to see if smp_ops has already been set.

This is my attempt at fixing the problem by not setting mdesc.smp for the ARMADA_370_XP_DT machines thus preventing setup_arch from overriding what has been setup by arm_dt_init_cpu_maps.

Thanks,
Chris

 arch/arm/mach-mvebu/board-v7.c | 1 -
 1 file changed, 1 deletion(-)

arch/arm/mach-mvebu/board-v7.c +0 -0

index 6478626..894974b 100644

arch/arm/mach-mvebu/board-v7.c static const char * const armada_370_xp_dt_compat[] = { · line 211

Quoted 12 lines

arch/arm/mach-mvebu/board-v7.c +0 -1

index 6478626..894974b 100644
@@ -206,7 +206,6 @@ 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,

There is a very good reason to keep this .smp initialization. The Device Tree for Armada XP used to *not* have the enable-method property, since the SMP enable-method binding did not exist at the time we introduced the Armada XP SMP support. Therefore, if we want to keep backward compatibility with the old Armada XP DTs and continue to have SMP support working with those, we need to keep this .smp initialization essentially forever.

Any idea what order things are done?

Would it be possible to remove the .smp entry, and then in mvebu_dt_init() check if there are smp operations set. If not, then set them to armada_xp_smp_ops?

Dear Chris Packham,

Quoted 49 lines · Chris Packham

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.

All of the ARMADA_370_XP_DT compatible devices are either single core or declare an enable-method in the dts (via one of armada-xp-mv78230.dtsi, armada-xp-mv78260.dtsi or armada-xp-mv78460.dtsi). Remove the smp assignment from board-v7.c so that the SMP operations set via the device-tree aren't discarded.

Signed-off-by Chris Packham <[email protected]>

---
Hi,

(This is the first patch I've sent to the arm-lkml so beware of my newbie-ness)

I'm working on a new board that uses a integrated CPU that according to the vendor is "based on the ARMADA-XP", more on that to come in the future hopefully. For the most part things just work if I use a .dts based on the mv78260.

One difference I've encountered is in the way the secondary CPU is initialised, which requires me to supply a slightly different set of machine specific SMP operations so I can bring up the 2nd core. It looks like I should be able to supply a different /cpus/enable-method in the .dts and once I define the corresponding set of operations it should be picked up.

Which leads me to what I think could be a bug (or just a lack of clear specification). The smp ops are set by both arm_dt_init_cpu_maps (from the enable-method) and setup_arch (from mdesc). The latter always wins because it is called last and doesn't check to see if smp_ops has already been set.

This is my attempt at fixing the problem by not setting mdesc.smp for the ARMADA_370_XP_DT machines thus preventing setup_arch from overriding what has been setup by arm_dt_init_cpu_maps.

Thanks,
Chris

  arch/arm/mach-mvebu/board-v7.c | 1 -
  1 file changed, 1 deletion(-)

arch/arm/mach-mvebu/board-v7.c +0 -0

index 6478626..894974b 100644

arch/arm/mach-mvebu/board-v7.c static const char * const armada_370_xp_dt_compat[] = { · line 211

Quoted 7 lines

arch/arm/mach-mvebu/board-v7.c +0 -1

@@ -206,7 +206,6 @@ 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,

There is a very good reason to keep this .smp initialization. The Device Tree for Armada XP used to *not* have the enable-method property, since the SMP enable-method binding did not exist at the time we introduced the Armada XP SMP support. Therefore, if we want to keep backward compatibility with the old Armada XP DTs and continue to have SMP support working with those, we need to keep this .smp initialization essentially forever.

Yes, backward compatibility sometimes sucks :-/

Darn. I thought that might be the case but I wanted to keep the scope of my change small. How about making setup_arch check if smp_ops is already set?

Alternatively if we gave ARMADA_370_XP_DT a smp_init function that returned non-zero if smp_ops is already set then I think it'd be backwards compatible and keep the scope of the change restricted to the 370-XP platforms.

Quoted 3 lines

Darn. I thought that might be the case but I wanted to keep the scope of my change small. How about making setup_arch check if smp_ops is already set?

I think that would be last resort solution. Try not to touch core code unless you really have to.

Quoted 4 lines

Alternatively if we gave ARMADA_370_XP_DT a smp_init function that returned non-zero if smp_ops is already set then I think it'd be backwards compatible and keep the scope of the change restricted to the 370-XP platforms.

This sounds more reasonable.

Andrew

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 <[email protected]>

Dear Chris Packham,

Quoted 6 lines · Chris Packham

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.

Quoted 6 lines

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 <[email protected]>

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.

Best regards,

Thomas

Dear Chris Packham,

Quoted 6 lines · Chris Packham

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.

Quoted 6 lines

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 <[email protected]>

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.

Hi Thomas, Maxime

Quoted 32 lines · Thomas Petazzoni

Dear Chris Packham,

Quoted 6 lines · Chris Packham

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.

Quoted 6 lines

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 <[email protected]>

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();

Quoted 7 lines

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 <[email protected]>

---
  arch/arm/mach-mvebu/Makefile   |  2 ++
  arch/arm/mach-mvebu/board-v7.c | 23 ++++++++++++++++++++++-
  2 files changed, 24 insertions(+), 1 deletion(-)

arch/arm/mach-mvebu/Makefile +2 -0

index 1636cdb..f818eaf 100644
@@ -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

arch/arm/mach-mvebu/board-v7.c +9 -0

index b2524d6..45851a2 100644
@@ -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");

arch/arm/mach-mvebu/board-v7.c +3 -0

+    }
+}
+
  static const char * const armada_370_xp_dt_compat[] = {
      "marvell,armada-370-xp",

NULL,

arch/arm/mach-mvebu/board-v7.c +1 -1

@@ -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[] = {

Quoted 44 lines · Chris Packham

Hi Thomas, Maxime

Quoted 32 lines · Thomas Petazzoni

Dear Chris Packham,

Quoted 6 lines · Chris Packham

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.

Quoted 6 lines

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 <[email protected]>

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();

Quoted 7 lines

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.

Quoted 6 lines

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.

(I really need to figure out how to tell Thunderbird how to wrap some parts but not others)

Quoted 58 lines

---- 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 <[email protected]>

---
  arch/arm/mach-mvebu/Makefile   |  2 ++
  arch/arm/mach-mvebu/board-v7.c | 23 ++++++++++++++++++++++-
  2 files changed, 24 insertions(+), 1 deletion(-)

arch/arm/mach-mvebu/Makefile +2 -0

index 1636cdb..f818eaf 100644
@@ -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

arch/arm/mach-mvebu/board-v7.c +14 -0

index b2524d6..45851a2 100644
@@ -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");

arch/arm/mach-mvebu/board-v7.c static void __init mvebu_dt_init(void) · line 205

Quoted 4 lines

arch/arm/mach-mvebu/board-v7.c +4 -0

+
+        fdt_appendprop(initial_boot_params, offset, "enable-method",
+                   "marvell,armada-xp-smp",
+                   sizeof("marvell,armada-xp-smp"));

Instead of inserting something into the device tree I could just call set_smp_ops here. That might be safer than trying to insert something into the device tree.

Quoted 115 lines · Chris Packham

Quoted 44 lines · Chris Packham

Hi Thomas, Maxime

Quoted 32 lines · Thomas Petazzoni

Dear Chris Packham,

Quoted 6 lines · Chris Packham

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.

Quoted 6 lines

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 <[email protected]>

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();

Quoted 7 lines

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.

Quoted 6 lines

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.

(I really need to figure out how to tell Thunderbird how to wrap some parts but not others)

Quoted 58 lines

---- 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 <[email protected]>

---
   arch/arm/mach-mvebu/Makefile   |  2 ++
   arch/arm/mach-mvebu/board-v7.c | 23 ++++++++++++++++++++++-
   2 files changed, 24 insertions(+), 1 deletion(-)

arch/arm/mach-mvebu/Makefile +2 -0

index 1636cdb..f818eaf 100644
@@ -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

arch/arm/mach-mvebu/board-v7.c +14 -0

index b2524d6..45851a2 100644
@@ -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");

Quoted 8 lines

arch/arm/mach-mvebu/board-v7.c static void __init mvebu_dt_init(void) · line 205

Quoted 4 lines

arch/arm/mach-mvebu/board-v7.c +4 -0

+
+        fdt_appendprop(initial_boot_params, offset, "enable-method",
+                   "marvell,armada-xp-smp",
+                   sizeof("marvell,armada-xp-smp"));

Instead of inserting something into the device tree I could just call set_smp_ops here. That might be safer than trying to insert something into the device tree.

I don't think this is necessary. Injecting something in the DT is safe, u-boot does that at every boot :)

Quoted 7 lines

Quoted 3 lines

Instead of inserting something into the device tree I could just call set_smp_ops here. That might be safer than trying to insert something into the device tree.

I don't think this is necessary. Injecting something in the DT is safe, u-boot does that at every boot :)

Actually I thought a bit more about this option this morning. What I'm trying to do is provide a fallback that defines smp_ops when there isn't an enable-method in the device tree. I don't actually need do do anything to the incoming device tree, I don't even need to look at it. I can unconditionally call set_smp_ops() and if the device tree has an enable-method it will override whatever has been configured. If the device tree doesn't define an enable-method it will use the default that I've configured here. That's actually very little code and can all be contained in board-v7.c.

Hi Thomas, Maxime

Quoted 32 lines · Thomas Petazzoni

Dear Chris Packham,

Quoted 6 lines · Chris Packham

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.

Quoted 6 lines

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 <[email protected]>

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();

Quoted 7 lines

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 <[email protected]>

---
 arch/arm/mach-mvebu/Makefile   |  2 ++
 arch/arm/mach-mvebu/board-v7.c | 23 ++++++++++++++++++++++-
 2 files changed, 24 insertions(+), 1 deletion(-)

arch/arm/mach-mvebu/Makefile +2 -0

index 1636cdb..f818eaf 100644
@@ -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

arch/arm/mach-mvebu/board-v7.c +11 -0

index b2524d6..45851a2 100644
@@ -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);
+