Hello,
The purpose of this patch set is to add the SMP support for the Armada
XP SoCs. Beside the SMP support itself brought by the last 3 patches,
this patch set also adds the support for the coherency fabric unit and
the power management service unit.
The coherency fabric is responsible for ensuring hardware coherency
between all CPUs and between CPUs and I/O masters. This unit is also
available for Armada 370 and will be used in an incoming patch set
for hardware I/O cache coherency.
The power management service unit is responsible for powering down and
waking up CPUs and other SOC units.
The original code is from Yehuda Yitschak, it was reworked by myself
and reviewed by Yehuda.
This patch set is based on 3.7-rc3 and depends one the framework clock
support (the last version was posted 2 weeks ago:
http://thread.gmane.org/gmane.linux.kernel/1375701). The git branch
called ArmadaXP-SMP-for-3.8-V2 is also available at
https://github.com/MISL-EBU-System-SW/mainline-public.git.
Arnd, Olof, as this code is SoC specific will you be able to pull it
through Jason tree? Or should I push the part added in
arch/arm/mm/proc-v7.S to support PJ4B CPU through Russell tree?
Changelog:
V1 -> V2:
- Rebased on to v3.7-rc3
- Fixed typos found by Alexandre Belloni
- Added clk_prepare_enable() before getting rate clk in
set_secondary_cpus_clock()
- Add explanation in the binding documentation about the per-CPU
interrupt registers: the address of the virtual register must be
used.
- Removed the armada_xp prefix in the coherency.c file to be more
compliant with the name convention of the other files.
- Coherency_init is now called from armada_370_xp_dt_init() and is no
more an early_init() call. As the device tree is not available from
an early_init(), it was useless to call coherency_init() so
early. The need to be able to call some function very early during
the boot were already resolved by using the hard code address of the
register.
Regards,
Yehuda Yitschak (5):
arm: mvebu: Added support for coherency fabric in mach-mvebu
arm: mvebu: Added initial support for power managmement service unit
arm: mvebu: Added IPI support via doorbells
arm: mm: Added support for PJ4B cpu and init routines
arm: mvebu: Added SMP support for Armada XP
.../devicetree/bindings/arm/armada-370-xp-mpic.txt | 12 +-
.../devicetree/bindings/arm/armada-370-xp-pmsu.txt | 20 ++++
.../devicetree/bindings/arm/coherency-fabric.txt | 16 +++
arch/arm/boot/dts/armada-370-xp.dtsi | 5 +
arch/arm/boot/dts/armada-xp.dtsi | 12 +-
arch/arm/configs/mvebu_defconfig | 3 +
arch/arm/mach-mvebu/Kconfig | 3 +-
arch/arm/mach-mvebu/Makefile | 4 +-
arch/arm/mach-mvebu/armada-370-xp.c | 3 +
arch/arm/mach-mvebu/armada-370-xp.h | 10 ++
arch/arm/mach-mvebu/coherency.c | 89 ++++++++++++++
arch/arm/mach-mvebu/coherency.h | 21 ++++
arch/arm/mach-mvebu/common.h | 6 +
arch/arm/mach-mvebu/headsmp.S | 66 +++++++++++
arch/arm/mach-mvebu/hotplug.c | 30 +++++
arch/arm/mach-mvebu/irq-armada-370-xp.c | 92 ++++++++++++++-
arch/arm/mach-mvebu/platsmp.c | 124 ++++++++++++++++++++
arch/arm/mach-mvebu/pmsu.c | 78 ++++++++++++
arch/arm/mach-mvebu/pmsu.h | 16 +++
arch/arm/mm/Kconfig | 4 +
arch/arm/mm/proc-v7.S | 46 ++++++++
21 files changed, 648 insertions(+), 12 deletions(-)
create mode 100644 Documentation/devicetree/bindings/arm/armada-370-xp-pmsu.txt
create mode 100644 Documentation/devicetree/bindings/arm/coherency-fabric.txt
create mode 100644 arch/arm/mach-mvebu/coherency.c
create mode 100644 arch/arm/mach-mvebu/coherency.h
create mode 100644 arch/arm/mach-mvebu/headsmp.S
create mode 100644 arch/arm/mach-mvebu/hotplug.c
create mode 100644 arch/arm/mach-mvebu/platsmp.c
create mode 100644 arch/arm/mach-mvebu/pmsu.c
create mode 100644 arch/arm/mach-mvebu/pmsu.h
--
1.7.9.5
@@ -0,0 +1,16 @@+Coherency fabric+----------------+Available on Marvell SOCs: Armada 370 and Armada XP++Required properties:++- compatible: "marvell,coherency-fabric"+- reg: Should contain,coherency fabric registers location and length.++Example:++coherency-fabric at d0020200 {+ compatible = "marvell,coherency-fabric";+ reg = <0xd0020200 0xb0>;+};+
@@ -0,0 +1,89 @@+/*+*Coherencyfabric(Aurora)supportforArmada370andXPplatforms.+*+*Copyright(C)2012Marvell+*+*YehudaYitschak<yehuday@marvell.com>+*GregoryClement<gregory.clement@free-electrons.com>+*ThomasPetazzoni<thomas.petazzoni@free-electrons.com>+*+*ThisfileislicensedunderthetermsoftheGNUGeneralPublic+*Licenseversion2.Thisprogramislicensed"as is"withoutany+*warrantyofanykind,whetherexpressorimplied.+*+*TheArmada370andArmadaXPSOCshaveacoherencyfabricwhichis+*responsibleforensuringhardwarecoherencybetweenallCPUsandbetween+*CPUsandI/Omasters.Thisfileinitializesthecoherencyfabricand+*suppliesbasicroutinesforconfiguringandcontrollinghardwarecoherency+*/++#include<linux/kernel.h>+#include<linux/init.h>+#include<linux/of_address.h>+#include<linux/io.h>+#include<linux/smp.h>+#include<asm/smp_plat.h>+#include"armada-370-xp.h"++/* Some functions in this file are called very early during SMP+*initialization.Atthattimethedevicetreeframeworkisnotyet+*ready,anditisnotpossibletogettheregisteraddressto+*ioremapit.That'swhythepointerbelowisgivenwithaninitial+*valuematchingitsvirtualmapping+*/+staticvoid__iomem*coherency_base=ARMADA_370_XP_REGS_VIRT_BASE+0x20200;++/* Coherency fabric registers */+#define COHERENCY_FABRIC_CTL_OFFSET 0x0+#define COHERENCY_FABRIC_CFG_OFFSET 0x4++staticstructof_device_idof_coherency_table[]={+{.compatible="marvell,coherency-fabric"},+{/* end of list */},+};++intcoherency_get_cpu_count(void)+{+intreg,cnt;++reg=readl(coherency_base+COHERENCY_FABRIC_CFG_OFFSET);+cnt=(reg&0xF)+1;++returncnt;+}++intset_cpu_coherent(unsignedinthw_cpu_id,intsmp_group_id)+{+intreg;++if(!coherency_base){+pr_warn("Can't make CPU %d cache coherent.\n",hw_cpu_id);+pr_warn("Coherency fabric is not initialized\n");+return1;+}++/* Enable the CPU in coherency fabric */+reg=readl(coherency_base+COHERENCY_FABRIC_CTL_OFFSET);+reg|=1<<(24+hw_cpu_id);+writel(reg,coherency_base+COHERENCY_FABRIC_CTL_OFFSET);++/* Add CPU to SMP group */+reg=readl(coherency_base+COHERENCY_FABRIC_CFG_OFFSET);+reg|=1<<(16+hw_cpu_id+(smp_group_id==0?8:0));+writel(reg,coherency_base+COHERENCY_FABRIC_CFG_OFFSET);++return0;+}++int__initcoherency_init(void)+{+structdevice_node*np;++np=of_find_matching_node(NULL,of_coherency_table);+if(np){+pr_info("Initializing Coherency fabric\n");+coherency_base=of_iomap(np,0);+}++return0;+}
@@ -0,0 +1,20 @@+Power Management Service Unit(PMSU)+-----------------------------------+Available on Marvell SOCs: Armada 370 and Armada XP++Required properties:++- compatible: "marvell,armada-370-xp-pmsu"++- reg: Should contain PMSU registers location and length. First pair+ for the per-CPU SW Reset Control registers, second pair for the+ Power Management Service Unit.++Example:++armada-370-xp-pmsu at d0022000 {+ compatible = "marvell,armada-370-xp-pmsu";+ reg = <0xd0022100 0x430>,+ <0xd0020800 0x20>;+};+
@@ -0,0 +1,78 @@+/*+*PowerManagementServiceUnit(PMSU)supportforArmada370/XPplatforms.+*+*Copyright(C)2012Marvell+*+*YehudaYitschak<yehuday@marvell.com>+*GregoryClement<gregory.clement@free-electrons.com>+*ThomasPetazzoni<thomas.petazzoni@free-electrons.com>+*+*ThisfileislicensedunderthetermsoftheGNUGeneralPublic+*Licenseversion2.Thisprogramislicensed"as is"withoutany+*warrantyofanykind,whetherexpressorimplied.+*+*TheArmada370andArmadaXPSOCshaveapowermanagementservice+*unitwhichisresponsibleforpoweringdownandwakingupCPUsand+*otherSOCunits+*/++#include<linux/kernel.h>+#include<linux/init.h>+#include<linux/of_address.h>+#include<linux/io.h>+#include<linux/smp.h>+#include<asm/smp_plat.h>++staticvoid__iomem*pmsu_mp_base;+staticvoid__iomem*pmsu_reset_base;++#define PMSU_BOOT_ADDR_REDIRECT_OFFSET(cpu) ((cpu * 0x100) + 0x24)+#define PMSU_RESET_CTL_OFFSET(cpu) (cpu * 0x8)++staticstructof_device_idof_pmsu_table[]={+{.compatible="marvell,armada-370-xp-pmsu"},+{/* end of list */},+};++#ifdef CONFIG_SMP+intarmada_xp_boot_cpu(unsignedintcpu_id,void__iomem*boot_addr)+{+intreg,hw_cpu;++if(!pmsu_mp_base||!pmsu_reset_base){+pr_warn("Can't boot CPU. PMSU is uninitialized\n");+return1;+}++hw_cpu=cpu_logical_map(cpu_id);++writel(virt_to_phys(boot_addr),pmsu_mp_base++PMSU_BOOT_ADDR_REDIRECT_OFFSET(hw_cpu));++/* Make sure value hits memory before reset */+dsb();++/* Release CPU from reset by clearing reset bit*/+reg=readl(pmsu_reset_base+PMSU_RESET_CTL_OFFSET(hw_cpu));+reg&=(~0x1);+writel(reg,pmsu_reset_base+PMSU_RESET_CTL_OFFSET(hw_cpu));++return0;+}+#endif++int__initarmada_370_xp_pmsu_init(void)+{+structdevice_node*np;++np=of_find_matching_node(NULL,of_pmsu_table);+if(np){+pr_info("Initializing Power Management Service Unit\n");+pmsu_mp_base=of_iomap(np,0);+pmsu_reset_base=of_iomap(np,1);+}++return0;+}++early_initcall(armada_370_xp_pmsu_init);
@@ -6,9 +6,15 @@ Required properties: - interrupt-controller: Identifies the node as an interrupt controller. - #interrupt-cells: The number of cells to define the interrupts. Should be 1. The cell is the IRQ number+ - reg: Should contain PMIC registers location and length. First pair for the main interrupt registers, second pair for the per-CPU- interrupt registers+ interrupt registers. For this last pair, to be compliant with SMP+ support, the "virtual" must be use (For the record, these registers+ automatically map to the interrupt controller registers of the+ current CPU)++ Example:
@@ -12,6 +12,9 @@ CONFIG_ARCH_MVEBU=y CONFIG_MACH_ARMADA_370=y CONFIG_MACH_ARMADA_XP=y # CONFIG_CACHE_L2X0 is not set+# CONFIG_SWP_EMULATE is not set+CONFIG_SMP=y+# CONFIG_LOCAL_TIMERS is not set CONFIG_AEABI=y CONFIG_HIGHMEM=y # CONFIG_COMPACTION is not set
@@ -0,0 +1,30 @@+/*+*SymmetricMultiProcessing(SMP)supportforArmadaXP+*+*Copyright(C)2012Marvell+*+*LiorAmsalem<alior@marvell.com>+*GregoryCLEMENT<gregory.clement@free-electrons.com>+*ThomasPetazzoni<thomas.petazzoni@free-electrons.com>+*+*ThisfileislicensedunderthetermsoftheGNUGeneralPublic+*Licenseversion2.Thisprogramislicensed"as is"withoutany+*warrantyofanykind,whetherexpressorimplied.+*/+#include<linux/kernel.h>+#include<linux/errno.h>+#include<linux/smp.h>+#include<asm/proc-fns.h>++/*+*platform-specificcodetoshutdownaCPU+*+*CalledwithIRQsdisabled+*/+void__refarmada_xp_cpu_die(unsignedintcpu)+{+cpu_do_idle();++/* We should never return from idle */+panic("mvebu: cpu %d unexpectedly exit from shutdown\n",cpu);+}
@@ -0,0 +1,124 @@+/*+*SymmetricMultiProcessing(SMP)supportforArmadaXP+*+*Copyright(C)2012Marvell+*+*LiorAmsalem<alior@marvell.com>+*YehudaYitschak<yehuday@marvell.com>+*GregoryCLEMENT<gregory.clement@free-electrons.com>+*ThomasPetazzoni<thomas.petazzoni@free-electrons.com>+*+*ThisfileislicensedunderthetermsoftheGNUGeneralPublic+*Licenseversion2.Thisprogramislicensed"as is"withoutany+*warrantyofanykind,whetherexpressorimplied.+*+*TheArmadaXPSoChas4ARMv7PJ4BCPUsrunninginfullHWcoherency+*ThisfileimplementstheroutinesforpreparingtheSMPinfrastructure+*andwakingupthesecondaryCPUs+*/++#include<linux/init.h>+#include<linux/smp.h>+#include<linux/clk.h>+#include<linux/of.h>+#include<asm/cacheflush.h>+#include<asm/smp_plat.h>+#include"common.h"+#include"armada-370-xp.h"+#include"pmsu.h"+#include"coherency.h"++void__initset_secondary_cpus_clock(void)+{+intcpu;+unsignedlongrate;+structclk*cpu_clk=NULL;+structdevice_node*np=NULL;++cpu=smp_processor_id();+np=of_find_node_by_type(np,"cpu");+np=NULL;+while((np=of_find_node_by_type(np,"cpu"))){+constu32*reg;+intlen;+reg=of_get_property(np,"reg",&len);+if(!reg||len!=4){+pr_err("%s missing reg property\n",np->full_name);+continue;+}+if(be32_to_cpup(reg)==cpu){+cpu_clk=of_clk_get(np,0);+break;+}+}+WARN_ON(IS_ERR(cpu_clk));+clk_prepare_enable(cpu_clk);+rate=clk_get_rate(cpu_clk);++/* set all the other CPU clk to the same rate than the boot CPU */+np=NULL;+while((np=of_find_node_by_type(np,"cpu"))){+constu32*reg;+intlen;+reg=of_get_property(np,"reg",&len);+if(!reg||len!=4){+pr_err("%s missing reg property\n",np->full_name);+continue;+}+if(be32_to_cpup(reg)!=cpu){+cpu_clk=of_clk_get(np,0);+clk_set_rate(cpu_clk,rate);+}+}+}++staticvoid__cpuinitarmada_xp_secondary_init(unsignedintcpu)+{+armada_xp_mpic_smp_cpu_init();+}++staticint__cpuinitarmada_xp_boot_secondary(unsignedintcpu,+structtask_struct*idle)+{+pr_info("Booting CPU %d\n",cpu);++armada_xp_boot_cpu(cpu,armada_xp_secondary_startup);++return0;+}++staticvoid__initarmada_xp_smp_init_cpus(void)+{+unsignedinti,ncores;+ncores=coherency_get_cpu_count();++/* Limit possbile CPUs to defconfig */+if(ncores>nr_cpu_ids){+pr_warn("SMP: %d CPUs physically present. Only %d configured.",+ncores,nr_cpu_ids);+pr_warn("Clipping CPU count to %d\n",nr_cpu_ids);+ncores=nr_cpu_ids;+}++for(i=0;i<ncores;i++)+set_cpu_possible(i,true);++set_smp_cross_call(armada_mpic_send_doorbell);+}++void__initarmada_xp_smp_prepare_cpus(unsignedintmax_cpus)+{+set_secondary_cpus_clock();+flush_cache_all();+set_cpu_coherent(cpu_logical_map(smp_processor_id()),0);+}++structsmp_operationsarmada_xp_smp_ops__initdata={+.smp_init_cpus=armada_xp_smp_init_cpus,+.smp_prepare_cpus=armada_xp_smp_prepare_cpus,+.smp_secondary_init=armada_xp_secondary_init,+.smp_boot_secondary=armada_xp_boot_secondary,+#ifdef CONFIG_HOTPLUG_CPU+.cpu_die=armada_xp_cpu_die,+#endif+};
+int set_cpu_coherent(unsigned int hw_cpu_id, int smp_group_id)
+{
+ int reg;
+
+ if (!coherency_base) {
+ pr_warn("Can't make CPU %d cache coherent.\n", hw_cpu_id);
+ pr_warn("Coherency fabric is not initialized\n");
+ return 1;
+ }
+
+ /* Enable the CPU in coherency fabric */
+ reg = readl(coherency_base + COHERENCY_FABRIC_CTL_OFFSET);
+ reg |= 1 << (24 + hw_cpu_id);
+ writel(reg, coherency_base + COHERENCY_FABRIC_CTL_OFFSET);
+
+ /* Add CPU to SMP group */
+ reg = readl(coherency_base + COHERENCY_FABRIC_CFG_OFFSET);
+ reg |= 1 << (16 + hw_cpu_id + (smp_group_id == 0 ? 8 : 0));
+ writel(reg, coherency_base + COHERENCY_FABRIC_CFG_OFFSET);
+
+ return 0;
+}
These writels may expand to code containing calls to outer_sync(), which
will attempt to take a spinlock for the aurora l2. Given that the CPU isn't
coherent, how does this play out with the exclusive store instruction in the
lock?
Will
+int set_cpu_coherent(unsigned int hw_cpu_id, int smp_group_id)
+{
+ int reg;
+
+ if (!coherency_base) {
+ pr_warn("Can't make CPU %d cache coherent.\n", hw_cpu_id);
+ pr_warn("Coherency fabric is not initialized\n");
+ return 1;
+ }
+
+ /* Enable the CPU in coherency fabric */
+ reg = readl(coherency_base + COHERENCY_FABRIC_CTL_OFFSET);
+ reg |= 1 << (24 + hw_cpu_id);
+ writel(reg, coherency_base + COHERENCY_FABRIC_CTL_OFFSET);
+
+ /* Add CPU to SMP group */
+ reg = readl(coherency_base + COHERENCY_FABRIC_CFG_OFFSET);
+ reg |= 1 << (16 + hw_cpu_id + (smp_group_id == 0 ? 8 : 0));
+ writel(reg, coherency_base + COHERENCY_FABRIC_CFG_OFFSET);
+
+ return 0;
+}
These writels may expand to code containing calls to outer_sync(), which
will attempt to take a spinlock for the aurora l2. Given that the CPU isn't
coherent, how does this play out with the exclusive store instruction in the
lock?
--
Gregory Clement, Free Electrons
Kernel, drivers, real-time and embedded Linux
development, consulting, training and support.
http://free-electrons.com
+int set_cpu_coherent(unsigned int hw_cpu_id, int smp_group_id)
+{
+ int reg;
+
+ if (!coherency_base) {
+ pr_warn("Can't make CPU %d cache coherent.\n", hw_cpu_id);
+ pr_warn("Coherency fabric is not initialized\n");
+ return 1;
+ }
+
+ /* Enable the CPU in coherency fabric */
+ reg = readl(coherency_base + COHERENCY_FABRIC_CTL_OFFSET);
+ reg |= 1 << (24 + hw_cpu_id);
+ writel(reg, coherency_base + COHERENCY_FABRIC_CTL_OFFSET);
+
+ /* Add CPU to SMP group */
+ reg = readl(coherency_base + COHERENCY_FABRIC_CFG_OFFSET);
+ reg |= 1 << (16 + hw_cpu_id + (smp_group_id == 0 ? 8 : 0));
+ writel(reg, coherency_base + COHERENCY_FABRIC_CFG_OFFSET);
+
+ return 0;
+}
These writels may expand to code containing calls to outer_sync(), which
will attempt to take a spinlock for the aurora l2. Given that the CPU isn't
coherent, how does this play out with the exclusive store instruction in the
lock?
I dug a little this subject: and I am not sure there is problem. In SMP mode,
only the system cache mode of Aurora is used. In this mode, outer_cache.sync
is void then outer_sync() won't call any function, so there will be no
access to any spinlock.
Gregory
Hello,
On 10/29/2012 10:11 PM, Gregory CLEMENT wrote:
[...]
Arnd, Olof, as this code is SoC specific will you be able to pull it
through Jason tree? Or should I push the part added in
arch/arm/mm/proc-v7.S to support PJ4B CPU through Russell tree?
Before sending a new series with the fixes related to Will comments, I
would like to know if I need to split the patches in order to send some
parts to Russell or if you (Arnd and Olof) will be able to take the whole
series (through Jason Cooper's git tree of course).
Changelog:
V1 -> V2:
- Rebased on to v3.7-rc3
- Fixed typos found by Alexandre Belloni
- Added clk_prepare_enable() before getting rate clk in
set_secondary_cpus_clock()
- Add explanation in the binding documentation about the per-CPU
interrupt registers: the address of the virtual register must be
used.
- Removed the armada_xp prefix in the coherency.c file to be more
compliant with the name convention of the other files.
- Coherency_init is now called from armada_370_xp_dt_init() and is no
more an early_init() call. As the device tree is not available from
an early_init(), it was useless to call coherency_init() so
early. The need to be able to call some function very early during
the boot were already resolved by using the hard code address of the
register.
Regards,
Yehuda Yitschak (5):
arm: mvebu: Added support for coherency fabric in mach-mvebu
arm: mvebu: Added initial support for power managmement service unit
arm: mvebu: Added IPI support via doorbells
arm: mm: Added support for PJ4B cpu and init routines
arm: mvebu: Added SMP support for Armada XP
.../devicetree/bindings/arm/armada-370-xp-mpic.txt | 12 +-
.../devicetree/bindings/arm/armada-370-xp-pmsu.txt | 20 ++++
.../devicetree/bindings/arm/coherency-fabric.txt | 16 +++
arch/arm/boot/dts/armada-370-xp.dtsi | 5 +
arch/arm/boot/dts/armada-xp.dtsi | 12 +-
arch/arm/configs/mvebu_defconfig | 3 +
arch/arm/mach-mvebu/Kconfig | 3 +-
arch/arm/mach-mvebu/Makefile | 4 +-
arch/arm/mach-mvebu/armada-370-xp.c | 3 +
arch/arm/mach-mvebu/armada-370-xp.h | 10 ++
arch/arm/mach-mvebu/coherency.c | 89 ++++++++++++++
arch/arm/mach-mvebu/coherency.h | 21 ++++
arch/arm/mach-mvebu/common.h | 6 +
arch/arm/mach-mvebu/headsmp.S | 66 +++++++++++
arch/arm/mach-mvebu/hotplug.c | 30 +++++
arch/arm/mach-mvebu/irq-armada-370-xp.c | 92 ++++++++++++++-
arch/arm/mach-mvebu/platsmp.c | 124 ++++++++++++++++++++
arch/arm/mach-mvebu/pmsu.c | 78 ++++++++++++
arch/arm/mach-mvebu/pmsu.h | 16 +++
arch/arm/mm/Kconfig | 4 +
arch/arm/mm/proc-v7.S | 46 ++++++++
21 files changed, 648 insertions(+), 12 deletions(-)
create mode 100644 Documentation/devicetree/bindings/arm/armada-370-xp-pmsu.txt
create mode 100644 Documentation/devicetree/bindings/arm/coherency-fabric.txt
create mode 100644 arch/arm/mach-mvebu/coherency.c
create mode 100644 arch/arm/mach-mvebu/coherency.h
create mode 100644 arch/arm/mach-mvebu/headsmp.S
create mode 100644 arch/arm/mach-mvebu/hotplug.c
create mode 100644 arch/arm/mach-mvebu/platsmp.c
create mode 100644 arch/arm/mach-mvebu/pmsu.c
create mode 100644 arch/arm/mach-mvebu/pmsu.h
--
Gregory Clement, Free Electrons
Kernel, drivers, real-time and embedded Linux
development, consulting, training and support.
http://free-electrons.com
On Monday 12 November 2012, Gregory CLEMENT wrote:
Before sending a new series with the fixes related to Will comments, I
would like to know if I need to split the patches in order to send some
parts to Russell or if you (Arnd and Olof) will be able to take the whole
series (through Jason Cooper's git tree of course).
We can take the whole series through arm-soc, but it would be good to
get an Ack from Russell for patch 4.
Arnd
From: Will Deacon <hidden> Date: 2012-11-13 10:43:40
On Mon, Nov 12, 2012 at 08:21:07PM +0000, Gregory CLEMENT wrote:
On 11/05/2012 03:02 PM, Will Deacon wrote:
quoted
On Mon, Oct 29, 2012 at 09:11:44PM +0000, Gregory CLEMENT wrote:
quoted
+int set_cpu_coherent(unsigned int hw_cpu_id, int smp_group_id)
+{
+ int reg;
+
+ if (!coherency_base) {
+ pr_warn("Can't make CPU %d cache coherent.\n", hw_cpu_id);
+ pr_warn("Coherency fabric is not initialized\n");
+ return 1;
+ }
+
+ /* Enable the CPU in coherency fabric */
+ reg = readl(coherency_base + COHERENCY_FABRIC_CTL_OFFSET);
+ reg |= 1 << (24 + hw_cpu_id);
+ writel(reg, coherency_base + COHERENCY_FABRIC_CTL_OFFSET);
+
+ /* Add CPU to SMP group */
+ reg = readl(coherency_base + COHERENCY_FABRIC_CFG_OFFSET);
+ reg |= 1 << (16 + hw_cpu_id + (smp_group_id == 0 ? 8 : 0));
+ writel(reg, coherency_base + COHERENCY_FABRIC_CFG_OFFSET);
+
+ return 0;
+}
These writels may expand to code containing calls to outer_sync(), which
will attempt to take a spinlock for the aurora l2. Given that the CPU isn't
coherent, how does this play out with the exclusive store instruction in the
lock?
I dug a little this subject: and I am not sure there is problem. In SMP mode,
only the system cache mode of Aurora is used. In this mode, outer_cache.sync
is void then outer_sync() won't call any function, so there will be no
access to any spinlock.
Hmm, that is pretty subtle and it doesn't really solve the bigger picture.
printk takes logbuf_lock, for example, and I'm sure that by the time you get
to this code you will have relied on exclusives behaving correctly.
Will
Russell,
Do you have any comments on this patch?
Else do you agree to give your acked-by?
Thanks,
Gregory
On 10/29/2012 10:11 PM, Gregory CLEMENT wrote:
--
Gregory Clement, Free Electrons
Kernel, drivers, real-time and embedded Linux
development, consulting, training and support.
http://free-electrons.com
This mask seems a little excessive -- does your cpuid not contain revision
information? If so, I'm not sure we want to update the code every time there
is a new spin of the same CPU.
Will
virt_to_phys on an __iomem * doesn't feel right to me...
It isn't correct - I suspect the __iomem there is just wrong - and it
needs the callsite also checking.
quoted
+ /* Make sure value hits memory before reset */
+ dsb();
writel has barrier semantics -- you shouldn't need this dsb.
writel has a barrier before the write (to ensure that DMA agents see data
that was written to memory when they are enabled by the write). There
isn't a barrier after the write.
This just looks silly to me - setting five bits with five instructions
when they can all be done in one instruction. Yes, I know you want
to comment it, but there's other ways to achieve that.
From: Will Deacon <hidden> Date: 2012-11-14 09:46:40
On Wed, Nov 14, 2012 at 12:07:36AM +0000, Russell King - ARM Linux wrote:
On Mon, Nov 05, 2012 at 02:05:58PM +0000, Will Deacon wrote:
quoted
On Mon, Oct 29, 2012 at 09:11:45PM +0000, Gregory CLEMENT wrote:
quoted
+ /* Make sure value hits memory before reset */
+ dsb();
writel has barrier semantics -- you shouldn't need this dsb.
writel has a barrier before the write (to ensure that DMA agents see data
that was written to memory when they are enabled by the write). There
isn't a barrier after the write.
Indeed, but there's a following write to actually do the reset, so the dsb
should come from there.
Will
On Mon, Nov 12, 2012 at 08:21:07PM +0000, Gregory CLEMENT wrote:
quoted
On 11/05/2012 03:02 PM, Will Deacon wrote:
quoted
On Mon, Oct 29, 2012 at 09:11:44PM +0000, Gregory CLEMENT wrote:
quoted
+int set_cpu_coherent(unsigned int hw_cpu_id, int smp_group_id)
+{
+ int reg;
+
+ if (!coherency_base) {
+ pr_warn("Can't make CPU %d cache coherent.\n", hw_cpu_id);
+ pr_warn("Coherency fabric is not initialized\n");
+ return 1;
+ }
+
+ /* Enable the CPU in coherency fabric */
+ reg = readl(coherency_base + COHERENCY_FABRIC_CTL_OFFSET);
+ reg |= 1 << (24 + hw_cpu_id);
+ writel(reg, coherency_base + COHERENCY_FABRIC_CTL_OFFSET);
+
+ /* Add CPU to SMP group */
+ reg = readl(coherency_base + COHERENCY_FABRIC_CFG_OFFSET);
+ reg |= 1 << (16 + hw_cpu_id + (smp_group_id == 0 ? 8 : 0));
+ writel(reg, coherency_base + COHERENCY_FABRIC_CFG_OFFSET);
+
+ return 0;
+}
These writels may expand to code containing calls to outer_sync(), which
will attempt to take a spinlock for the aurora l2. Given that the CPU isn't
coherent, how does this play out with the exclusive store instruction in the
lock?
I dug a little this subject: and I am not sure there is problem. In SMP mode,
only the system cache mode of Aurora is used. In this mode, outer_cache.sync
is void then outer_sync() won't call any function, so there will be no
access to any spinlock.
Hmm, that is pretty subtle and it doesn't really solve the bigger picture.
printk takes logbuf_lock, for example, and I'm sure that by the time you get
to this code you will have relied on exclusives behaving correctly.
Hi Will,
I get an answer from Marvell engineers:
"STREX on non-shareable and/or non-cacheable memory regions is supported."
Gregory
From: Will Deacon <hidden> Date: 2012-11-15 10:17:52
On Wed, Nov 14, 2012 at 08:00:32PM +0000, Gregory CLEMENT wrote:
On 11/13/2012 11:43 AM, Will Deacon wrote:
quoted
On Mon, Nov 12, 2012 at 08:21:07PM +0000, Gregory CLEMENT wrote:
quoted
On 11/05/2012 03:02 PM, Will Deacon wrote:
quoted
These writels may expand to code containing calls to outer_sync(), which
will attempt to take a spinlock for the aurora l2. Given that the CPU isn't
coherent, how does this play out with the exclusive store instruction in the
lock?
I dug a little this subject: and I am not sure there is problem. In SMP mode,
only the system cache mode of Aurora is used. In this mode, outer_cache.sync
is void then outer_sync() won't call any function, so there will be no
access to any spinlock.
Hmm, that is pretty subtle and it doesn't really solve the bigger picture.
printk takes logbuf_lock, for example, and I'm sure that by the time you get
to this code you will have relied on exclusives behaving correctly.
Hi Will,
I get an answer from Marvell engineers:
"STREX on non-shareable and/or non-cacheable memory regions is supported."
Interesting, thanks for asking them about this. Does this mean that:
1. When not running coherently (i.e. before initialising the
coherency fabric), memory is treated as non-shareable,
non-cacheable?
2. If (1), then are exclusive accesses the only way to achieve
coherent memory accesses in this scenario?
If so, you still have a problem with write locks, where the unlock code does
a regular str to clear the status. atomic_{read,set} also uses regular
memory accesses, so I think you'll get some surprises there when you add
explicit memory barriers and expect things to be visible between threads.
Do memory barriers have different semantics depending on the state of your
coherency fabric?
Cheers,
Will
On Wed, Nov 14, 2012 at 08:00:32PM +0000, Gregory CLEMENT wrote:
quoted
On 11/13/2012 11:43 AM, Will Deacon wrote:
quoted
On Mon, Nov 12, 2012 at 08:21:07PM +0000, Gregory CLEMENT wrote:
quoted
On 11/05/2012 03:02 PM, Will Deacon wrote:
quoted
These writels may expand to code containing calls to outer_sync(), which
will attempt to take a spinlock for the aurora l2. Given that the CPU isn't
coherent, how does this play out with the exclusive store instruction in the
lock?
I dug a little this subject: and I am not sure there is problem. In SMP mode,
only the system cache mode of Aurora is used. In this mode, outer_cache.sync
is void then outer_sync() won't call any function, so there will be no
access to any spinlock.
Hmm, that is pretty subtle and it doesn't really solve the bigger picture.
printk takes logbuf_lock, for example, and I'm sure that by the time you get
to this code you will have relied on exclusives behaving correctly.
Hi Will,
I get an answer from Marvell engineers:
"STREX on non-shareable and/or non-cacheable memory regions is supported."
Interesting, thanks for asking them about this. Does this mean that:
Here come the answers to your new questions
1. When not running coherently (i.e. before initialising the
coherency fabric), memory is treated as non-shareable,
non-cacheable?
It can be cacheable. The shared memory (as defined on the page table)
will NOT be coherent by HW.
2. If (1), then are exclusive accesses the only way to achieve
coherent memory accesses in this scenario?
I quote: "I suspect there is terminology miss-use: exclusive accesses
are NOT used to achieve memory coherency - they are used to achieve
atomicity. To achieve memory coherency while fabric is configured to
be non-coherent, SW should use maintenance operations over the L1
caches.suspect there is terminology miss-use: exclusive accesses are
NOT used to achieve memory coherency - "they are used to achieve
atomicity. To achieve memory coherency while fabric is configured to
be non-coherent, SW should use maintenance operations over the L1
caches.
If so, you still have a problem with write locks, where the unlock code does
a regular str to clear the status. atomic_{read,set} also uses regular
memory accesses, so I think you'll get some surprises there when you add
explicit memory barriers and expect things to be visible between threads.
Do memory barriers have different semantics depending on the state of your
coherency fabric?
No
--
Gregory Clement, Free Electrons
Kernel, drivers, real-time and embedded Linux
development, consulting, training and support.
http://free-electrons.com
From: Will Deacon <hidden> Date: 2012-11-15 16:21:23
Hi Gregory,
On Thu, Nov 15, 2012 at 03:54:39PM +0000, Gregory CLEMENT wrote:
On 11/15/2012 11:17 AM, Will Deacon wrote:
quoted
Interesting, thanks for asking them about this. Does this mean that:
Here come the answers to your new questions
Great, thanks for the quick turn-around!
quoted
1. When not running coherently (i.e. before initialising the
coherency fabric), memory is treated as non-shareable,
non-cacheable?
It can be cacheable. The shared memory (as defined on the page table)
will NOT be coherent by HW.
Ok, so we really are incoherent before enabling the fabric.
quoted
2. If (1), then are exclusive accesses the only way to achieve
coherent memory accesses in this scenario?
I quote: "I suspect there is terminology miss-use: exclusive accesses
are NOT used to achieve memory coherency - they are used to achieve
atomicity. To achieve memory coherency while fabric is configured to
be non-coherent, SW should use maintenance operations over the L1
caches."
Ok, so if I'm understanding correctly then I don't really see the usefulness
of having working exclusives that are incoherent. Surely it means that you
can guarantee mutual exclusion on a lock variable, but the value you actually
end up reading from the lock is junk unless you litter the accessors with cache
clean operations?
Anyway, that's by-the-by as this is all called early enough that we
shouldn't care. The thing I don't like now is that the fabric initialisation
is done entirely differently on the primary CPU than the secondaries. The
primary probes the device-tree (well, it's also now hard-coded for v2) and
accesses the registers from a C function(armada_370_xp_set_cpu_coherent) whilst
the secondaries have hardcoded addresses and access via asm
(armada_xp_secondary_startup).
Will
Hi Gregory,
On Thu, Nov 15, 2012 at 03:54:39PM +0000, Gregory CLEMENT wrote:
quoted
On 11/15/2012 11:17 AM, Will Deacon wrote:
quoted
Interesting, thanks for asking them about this. Does this mean that:
Here come the answers to your new questions
Great, thanks for the quick turn-around!
quoted
quoted
1. When not running coherently (i.e. before initialising the
coherency fabric), memory is treated as non-shareable,
non-cacheable?
It can be cacheable. The shared memory (as defined on the page table)
will NOT be coherent by HW.
Ok, so we really are incoherent before enabling the fabric.
quoted
quoted
2. If (1), then are exclusive accesses the only way to achieve
coherent memory accesses in this scenario?
I quote: "I suspect there is terminology miss-use: exclusive accesses
are NOT used to achieve memory coherency - they are used to achieve
atomicity. To achieve memory coherency while fabric is configured to
be non-coherent, SW should use maintenance operations over the L1
caches."
Ok, so if I'm understanding correctly then I don't really see the usefulness
of having working exclusives that are incoherent. Surely it means that you
can guarantee mutual exclusion on a lock variable, but the value you actually
end up reading from the lock is junk unless you litter the accessors with cache
clean operations?
Anyway, that's by-the-by as this is all called early enough that we
shouldn't care. The thing I don't like now is that the fabric initialisation
is done entirely differently on the primary CPU than the secondaries. The
primary probes the device-tree (well, it's also now hard-coded for v2) and
accesses the registers from a C function(armada_370_xp_set_cpu_coherent) whilst
the secondaries have hardcoded addresses and access via asm
(armada_xp_secondary_startup).
Now it is hardcoded in both case as you pointed it. So the last
difference is setup from a C function or via asm.
The differences between primary and secondary CPU when they enable the
coherency, is due to the fact that we really are in a different
situation. For primary CPU, as it is the only CPU online it doesn't
need to enable the coherency from the beginning, so we can wait to
have MMU enable and convenient feature. Whereas for the secondary CPU
they need the coherency from the very beginning are by definition they
won't be alone. That's why this very first instruction are written in
asm and they use physical address.
I don't see how to handle it in a different way.
Gregory
From: Will Deacon <hidden> Date: 2012-11-16 18:56:10
On Thu, Nov 15, 2012 at 04:49:17PM +0000, Gregory CLEMENT wrote:
On 11/15/2012 05:21 PM, Will Deacon wrote:
quoted
Anyway, that's by-the-by as this is all called early enough that we
shouldn't care. The thing I don't like now is that the fabric initialisation
is done entirely differently on the primary CPU than the secondaries. The
primary probes the device-tree (well, it's also now hard-coded for v2) and
accesses the registers from a C function(armada_370_xp_set_cpu_coherent) whilst
the secondaries have hardcoded addresses and access via asm
(armada_xp_secondary_startup).
Now it is hardcoded in both case as you pointed it. So the last
difference is setup from a C function or via asm.
The differences between primary and secondary CPU when they enable the
coherency, is due to the fact that we really are in a different
situation. For primary CPU, as it is the only CPU online it doesn't
need to enable the coherency from the beginning, so we can wait to
have MMU enable and convenient feature. Whereas for the secondary CPU
they need the coherency from the very beginning are by definition they
won't be alone. That's why this very first instruction are written in
asm and they use physical address.
I don't see how to handle it in a different way.
The code paths are fine, I would just like to see less duplication. Can you
make the asm function PCS compliant and call it from C for the primary
(setting the link register to secondary_startup for the secondary cores)?
Will