From mboxrd@z Thu Jan 1 00:00:00 1970 From: maxime.ripard@free-electrons.com (Maxime Ripard) Date: Tue, 18 Nov 2014 09:21:51 +0100 Subject: [RFC PATCHv2] ARM: mvebu: Let the device-tree determine smp_ops In-Reply-To: <546A9378.2080907@alliedtelesis.co.nz> References: <20141106201603.GF4974@lunn.ch> <1415327626-16079-1-git-send-email-chris.packham@alliedtelesis.co.nz> <20141117095640.03e25d17@free-electrons.com> <546A9378.2080907@alliedtelesis.co.nz> Message-ID: <20141118082151.GX6414@lukather> To: linux-arm-kernel@lists.infradead.org List-Id: linux-arm-kernel.lists.infradead.org On Tue, Nov 18, 2014 at 12:31:52AM +0000, Chris Packham wrote: > > > On 11/18/2014 12:34 PM, Chris Packham wrote: > > 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 > >> > >> 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. > > (I really need to figure out how to tell Thunderbird how to wrap some > parts but not others) > > > > > ---- 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 > > --- > > 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 > > #include > > #include > > +#include > > +#include > > #include > > #include > > #include > > @@ -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")); > > 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 :) Maxime -- Maxime Ripard, Free Electrons Embedded Linux, Kernel and Android engineering http://free-electrons.com -------------- next part -------------- A non-text attachment was scrubbed... Name: signature.asc Type: application/pgp-signature Size: 819 bytes Desc: Digital signature URL: