Hello everyone,
This is a V3 of my series that adds SMP support for Allwinner sun8i-a83t.
Based on sunxi's tree, sunxi/for-next branch.
Changes since v2:
- Rebased my modifications according to new Chen-Yu Tsai's patch series
that adds SMP support for sun9i-a80 (without MCPM).
- Split the device-tree patches into 3 patches for CPUCFG, R_CPUCFG
and PRCM registers.
- The hotplug of CPU0 is currently not working (even after trying what
Allwinner's code is doing) so remove the possibility of disabling
this CPU. Created a new patch for it.
Changes since v1:
- Add Chen Yu's patch in my series (see path 01)
- Add new compatibles for prcm and cpucfg registers for sun8i-a83t.
Create two functions to separate the DT parsing of sun9i-a80 and
sun8i-a83t.
- Thanks to Maxime's review: order device tree's nodes according
to physical addresses, remove unused label and fix registers' sizes.
Update the commit log and commit title of my last patch (see
patch 05).
Patch 01: Convert the sunxi SMP driver to add support for A83T.
This SoC has a bit flip that needs to be handled.
Patch 02-03-04: Add registers nodes (prcm, cpucfg and r_cpucfg) needed
for SMP bringup.
Patch 05: Add CCI-400 node for a83t.
Patch 06: Fix the use of virtual timers that hangs the kernel in
case of SMP support.
Patch 07: Remove the possibility to disable the CPU0 because it is
currently not working for sun8i-a83t.
If you have any remarks/questions, let me know.
Thank you in advance,
Mylène
Mylène Josserand (7):
ARM: sun8i: smp: Add support for A83T
ARM: dts: sun8i: Add CPUCFG device node for A83T dtsi
ARM: dts: sun8i: Add PRCM device node for the A83T dtsi
ARM: dts: sun8i: Add R_CPUCFG device node for the A83T dtsi
arm: dts: sun8i: a83t: Add CCI-400 node
arm: dts: sun8i: a83t: Fix undefined offset with virtual timer
ARM: sun8i: smp: Remove the disabling of CPU0
arch/arm/boot/dts/sun8i-a83t.dtsi | 57 +++++++++
arch/arm/mach-sunxi/Kconfig | 2 +-
arch/arm/mach-sunxi/mc_smp.c | 245 +++++++++++++++++++++++++++++++-------
3 files changed, 260 insertions(+), 44 deletions(-)
--
2.11.0
On sun8i-a63t, hotplug CPU for CPU0 is currently not working.
Remove the possibility to disable CPU0 only for sun8i-a83t.
Signed-off-by: Mylène Josserand <[email protected]>
---
arch/arm/mach-sunxi/mc_smp.c | 6 +++++-
1 file changed, 5 insertions(+), 1 deletion(-)
diff --git a/arch/arm/mach-sunxi/mc_smp.c b/arch/arm/mach-sunxi/mc_smp.c
index fec592bf68b4..69d5ae5b3d72 100644
--- a/arch/arm/mach-sunxi/mc_smp.c
+++ b/arch/arm/mach-sunxi/mc_smp.c
@@ -661,8 +661,12 @@ static int sunxi_mc_smp_cpu_kill(unsigned int l_cpu)
return !ret;
}
-static bool sunxi_mc_smp_cpu_can_disable(unsigned int __unused)
+static bool sunxi_mc_smp_cpu_can_disable(unsigned int cpu)
{
+ /* CPU0 hotplug handled only for sun9i */
+ if (of_machine_is_compatible("allwinner,sun8i-a83t"))
+ if (cpu == 0)
+ return false;
return true;
}
#endif
--
2.11.0
The ARM architected timers use an offset between their physical and
virtual counters. That offset should be configured by the bootloader
in CNTVOFF.
However, the A83t bootloader fails to do so, and we end up with an
undefined offset (which in our case is random), meaning that each CPU
will have a different time, which isn't working very well.
Fix that by setting the arm,cpu-registers-not-fw-configured that will
make Linux use the physical timers instead of the virtual ones. One
possible side effect would be that the virtualization features would
be disabled. However, due to the way the GIC has been integrated in
the system, it is already unusable so we're effectively not losing any
feature.
Signed-off-by: Mylène Josserand <[email protected]>
---
arch/arm/boot/dts/sun8i-a83t.dtsi | 1 +
1 file changed, 1 insertion(+)
diff --git a/arch/arm/boot/dts/sun8i-a83t.dtsi b/arch/arm/boot/dts/sun8i-a83t.dtsi
index e97a6d17b8d0..b9bdb891cf2f 100644
--- a/arch/arm/boot/dts/sun8i-a83t.dtsi
+++ b/arch/arm/boot/dts/sun8i-a83t.dtsi
@@ -123,6 +123,7 @@
<GIC_PPI 14 (GIC_CPU_MASK_SIMPLE(8) | IRQ_TYPE_LEVEL_LOW)>,
<GIC_PPI 11 (GIC_CPU_MASK_SIMPLE(8) | IRQ_TYPE_LEVEL_LOW)>,
<GIC_PPI 10 (GIC_CPU_MASK_SIMPLE(8) | IRQ_TYPE_LEVEL_LOW)>;
+ arm,cpu-registers-not-fw-configured;
};
clocks {
--
2.11.0
Add CCI-400 node and control-port on CPUs needed by SMP bringup.
Signed-off-by: Mylène Josserand <[email protected]>
---
arch/arm/boot/dts/sun8i-a83t.dtsi | 41 +++++++++++++++++++++++++++++++++++++++
1 file changed, 41 insertions(+)
diff --git a/arch/arm/boot/dts/sun8i-a83t.dtsi b/arch/arm/boot/dts/sun8i-a83t.dtsi
index 7ae631fc04d5..e97a6d17b8d0 100644
--- a/arch/arm/boot/dts/sun8i-a83t.dtsi
+++ b/arch/arm/boot/dts/sun8i-a83t.dtsi
@@ -64,48 +64,56 @@
compatible = "arm,cortex-a7";
device_type = "cpu";
reg = <0>;
+ cci-control-port = <&cci_control0>;
};
cpu@1 {
compatible = "arm,cortex-a7";
device_type = "cpu";
reg = <1>;
+ cci-control-port = <&cci_control0>;
};
cpu@2 {
compatible = "arm,cortex-a7";
device_type = "cpu";
reg = <2>;
+ cci-control-port = <&cci_control0>;
};
cpu@3 {
compatible = "arm,cortex-a7";
device_type = "cpu";
reg = <3>;
+ cci-control-port = <&cci_control0>;
};
cpu@100 {
compatible = "arm,cortex-a7";
device_type = "cpu";
reg = <0x100>;
+ cci-control-port = <&cci_control1>;
};
cpu@101 {
compatible = "arm,cortex-a7";
device_type = "cpu";
reg = <0x101>;
+ cci-control-port = <&cci_control1>;
};
cpu@102 {
compatible = "arm,cortex-a7";
device_type = "cpu";
reg = <0x102>;
+ cci-control-port = <&cci_control1>;
};
cpu@103 {
compatible = "arm,cortex-a7";
device_type = "cpu";
reg = <0x103>;
+ cci-control-port = <&cci_control1>;
};
};
@@ -213,6 +221,39 @@
reg = <0x01700000 0x400>;
};
+ cci@1790000 {
+ compatible = "arm,cci-400";
+ #address-cells = <1>;
+ #size-cells = <1>;
+ reg = <0x01790000 0x10000>;
+ ranges = <0x0 0x01790000 0x10000>;
+
+ cci_control0: slave-if@4000 {
+ compatible = "arm,cci-400-ctrl-if";
+ interface-type = "ace";
+ reg = <0x4000 0x1000>;
+ };
+
+ cci_control1: slave-if@5000 {
+ compatible = "arm,cci-400-ctrl-if";
+ interface-type = "ace";
+ reg = <0x5000 0x1000>;
+ };
+
+ pmu@9000 {
+ compatible = "arm,cci-400-pmu,r1";
+ reg = <0x9000 0x5000>;
+ interrupts = <GIC_SPI 132 IRQ_TYPE_LEVEL_HIGH>,
+ <GIC_SPI 133 IRQ_TYPE_LEVEL_HIGH>,
+ <GIC_SPI 134 IRQ_TYPE_LEVEL_HIGH>,
+ <GIC_SPI 135 IRQ_TYPE_LEVEL_HIGH>,
+ <GIC_SPI 151 IRQ_TYPE_LEVEL_HIGH>,
+ <GIC_SPI 152 IRQ_TYPE_LEVEL_HIGH>,
+ <GIC_SPI 153 IRQ_TYPE_LEVEL_HIGH>,
+ <GIC_SPI 154 IRQ_TYPE_LEVEL_HIGH>;
+ };
+ };
+
syscon: syscon@1c00000 {
compatible = "allwinner,sun8i-a83t-system-controller",
"syscon";
--
2.11.0
Add the support for A83T.
A83T SoC has an additional register than A80 to handle CPU configurations:
R_CPUS_CFG. Information about the register comes from Allwinner's BSP
driver.
An important difference is the Power Off Gating register for clusters
which is BIT(4) in case of SUN9I-A80 and BIT(0) in case of SUN8I-A83T.
Signed-off-by: Mylène Josserand <[email protected]>
---
arch/arm/mach-sunxi/Kconfig | 2 +-
arch/arm/mach-sunxi/mc_smp.c | 239 +++++++++++++++++++++++++++++++++++--------
2 files changed, 198 insertions(+), 43 deletions(-)
diff --git a/arch/arm/mach-sunxi/Kconfig b/arch/arm/mach-sunxi/Kconfig
index ce53ceaf4cc5..a0ad35c41c02 100644
--- a/arch/arm/mach-sunxi/Kconfig
+++ b/arch/arm/mach-sunxi/Kconfig
@@ -51,7 +51,7 @@ config MACH_SUN9I
config ARCH_SUNXI_MC_SMP
bool
depends on SMP
- default MACH_SUN9I
+ default y if MACH_SUN9I || MACH_SUN8I
select ARM_CCI400_PORT_CTRL
select ARM_CPU_SUSPEND
diff --git a/arch/arm/mach-sunxi/mc_smp.c b/arch/arm/mach-sunxi/mc_smp.c
index 11e46c6efb90..fec592bf68b4 100644
--- a/arch/arm/mach-sunxi/mc_smp.c
+++ b/arch/arm/mach-sunxi/mc_smp.c
@@ -55,20 +55,29 @@
#define CPUCFG_CX_RST_CTRL_L2_RST BIT(8)
#define CPUCFG_CX_RST_CTRL_CX_RST(n) BIT(4 + (n))
#define CPUCFG_CX_RST_CTRL_CORE_RST(n) BIT(n)
+#define CPUCFG_CX_RST_CTRL_CORE_RST_ALL (0xf << 0)
#define PRCM_CPU_PO_RST_CTRL(c) (0x4 + 0x4 * (c))
#define PRCM_CPU_PO_RST_CTRL_CORE(n) BIT(n)
#define PRCM_CPU_PO_RST_CTRL_CORE_ALL 0xf
#define PRCM_PWROFF_GATING_REG(c) (0x100 + 0x4 * (c))
-#define PRCM_PWROFF_GATING_REG_CLUSTER BIT(4)
+/* The power off register for clusters are different from SUN9I and SUN8I */
+#define PRCM_PWROFF_GATING_REG_CLUSTER_SUN8I BIT(0)
+#define PRCM_PWROFF_GATING_REG_CLUSTER_SUN9I BIT(4)
#define PRCM_PWROFF_GATING_REG_CORE(n) BIT(n)
#define PRCM_PWR_SWITCH_REG(c, cpu) (0x140 + 0x10 * (c) + 0x4 * (cpu))
#define PRCM_CPU_SOFT_ENTRY_REG 0x164
+/* R_CPUCFG registers, specific to SUN8I */
+#define R_CPUCFG_CLUSTER_PO_RST_CTRL(c) (0x30 + (c) * 0x4)
+#define R_CPUCFG_CLUSTER_PO_RST_CTRL_CORE(n) BIT(n)
+#define R_CPUCFG_CPU_SOFT_ENTRY_REG 0x01a4
+
#define CPU0_SUPPORT_HOTPLUG_MAGIC0 0xFA50392F
#define CPU0_SUPPORT_HOTPLUG_MAGIC1 0x790DCA3A
static void __iomem *cpucfg_base;
+static void __iomem *r_cpucfg_base;
static void __iomem *prcm_base;
static void __iomem *sram_b_smp_base;
@@ -157,6 +166,16 @@ static int sunxi_cpu_powerup(unsigned int cpu, unsigned int cluster)
reg &= ~PRCM_CPU_PO_RST_CTRL_CORE(cpu);
writel(reg, prcm_base + PRCM_CPU_PO_RST_CTRL(cluster));
+ if (r_cpucfg_base) {
+ /* assert cpu power-on reset */
+ reg = readl(r_cpucfg_base +
+ R_CPUCFG_CLUSTER_PO_RST_CTRL(cluster));
+ reg &= ~(R_CPUCFG_CLUSTER_PO_RST_CTRL_CORE(cpu));
+ writel(reg, r_cpucfg_base +
+ R_CPUCFG_CLUSTER_PO_RST_CTRL(cluster));
+ udelay(10);
+ }
+
/* Cortex-A7: hold L1 reset disable signal low */
if (!sunxi_core_is_cortex_a15(cpu, cluster)) {
reg = readl(cpucfg_base + CPUCFG_CX_CTRL_REG0(cluster));
@@ -180,17 +199,37 @@ static int sunxi_cpu_powerup(unsigned int cpu, unsigned int cluster)
/* open power switch */
sunxi_cpu_power_switch_set(cpu, cluster, true);
+ /* Handle A83T bit swap */
+ if (of_machine_is_compatible("allwinner,sun8i-a83t")) {
+ if (cpu == 0)
+ cpu = 4;
+ }
+
/* clear processor power gate */
reg = readl(prcm_base + PRCM_PWROFF_GATING_REG(cluster));
reg &= ~PRCM_PWROFF_GATING_REG_CORE(cpu);
writel(reg, prcm_base + PRCM_PWROFF_GATING_REG(cluster));
udelay(20);
+ if (of_machine_is_compatible("allwinner,sun8i-a83t")) {
+ if (cpu == 4)
+ cpu = 0;
+ }
+
/* de-assert processor power-on reset */
reg = readl(prcm_base + PRCM_CPU_PO_RST_CTRL(cluster));
reg |= PRCM_CPU_PO_RST_CTRL_CORE(cpu);
writel(reg, prcm_base + PRCM_CPU_PO_RST_CTRL(cluster));
+ if (r_cpucfg_base) {
+ reg = readl(r_cpucfg_base +
+ R_CPUCFG_CLUSTER_PO_RST_CTRL(cluster));
+ reg |= R_CPUCFG_CLUSTER_PO_RST_CTRL_CORE(cpu);
+ writel(reg, r_cpucfg_base +
+ R_CPUCFG_CLUSTER_PO_RST_CTRL(cluster));
+ udelay(10);
+ }
+
/* de-assert all processor resets */
reg = readl(cpucfg_base + CPUCFG_CX_RST_CTRL(cluster));
reg |= CPUCFG_CX_RST_CTRL_DBG_RST(cpu);
@@ -212,6 +251,14 @@ static int sunxi_cluster_powerup(unsigned int cluster)
if (cluster >= SUNXI_NR_CLUSTERS)
return -EINVAL;
+ /* For A83T, assert cluster cores resets */
+ if (of_machine_is_compatible("allwinner,sun8i-a83t")) {
+ reg = readl(cpucfg_base + CPUCFG_CX_RST_CTRL(cluster));
+ reg &= ~CPUCFG_CX_RST_CTRL_CORE_RST_ALL; /* Core Reset */
+ writel(reg, cpucfg_base + CPUCFG_CX_RST_CTRL(cluster));
+ udelay(10);
+ }
+
/* assert ACINACTM */
reg = readl(cpucfg_base + CPUCFG_CX_CTRL_REG1(cluster));
reg |= CPUCFG_CX_CTRL_REG1_ACINACTM;
@@ -222,6 +269,16 @@ static int sunxi_cluster_powerup(unsigned int cluster)
reg &= ~PRCM_CPU_PO_RST_CTRL_CORE_ALL;
writel(reg, prcm_base + PRCM_CPU_PO_RST_CTRL(cluster));
+ /* assert cluster cores resets */
+ if (r_cpucfg_base) {
+ reg = readl(r_cpucfg_base +
+ R_CPUCFG_CLUSTER_PO_RST_CTRL(cluster));
+ reg &= ~CPUCFG_CX_RST_CTRL_CORE_RST_ALL;
+ writel(reg, r_cpucfg_base +
+ R_CPUCFG_CLUSTER_PO_RST_CTRL(cluster));
+ udelay(10);
+ }
+
/* assert cluster resets */
reg = readl(cpucfg_base + CPUCFG_CX_RST_CTRL(cluster));
reg &= ~CPUCFG_CX_RST_CTRL_DBG_SOC_RST;
@@ -252,7 +309,10 @@ static int sunxi_cluster_powerup(unsigned int cluster)
/* clear cluster power gate */
reg = readl(prcm_base + PRCM_PWROFF_GATING_REG(cluster));
- reg &= ~PRCM_PWROFF_GATING_REG_CLUSTER;
+ if (of_machine_is_compatible("allwinner,sun8i-a83t"))
+ reg &= ~PRCM_PWROFF_GATING_REG_CLUSTER_SUN8I;
+ else
+ reg &= ~PRCM_PWROFF_GATING_REG_CLUSTER_SUN9I;
writel(reg, prcm_base + PRCM_PWROFF_GATING_REG(cluster));
udelay(20);
@@ -516,7 +576,10 @@ static int sunxi_cluster_powerdown(unsigned int cluster)
/* gate cluster power */
pr_debug("%s: gate cluster power\n", __func__);
reg = readl(prcm_base + PRCM_PWROFF_GATING_REG(cluster));
- reg |= PRCM_PWROFF_GATING_REG_CLUSTER;
+ if (of_machine_is_compatible("allwinner,sun8i-a83t"))
+ reg |= PRCM_PWROFF_GATING_REG_CLUSTER_SUN8I;
+ else
+ reg |= PRCM_PWROFF_GATING_REG_CLUSTER_SUN9I;
writel(reg, prcm_base + PRCM_PWROFF_GATING_REG(cluster));
udelay(20);
@@ -684,37 +747,25 @@ static int __init sunxi_mc_smp_lookback(void)
return ret;
}
-static int __init sunxi_mc_smp_init(void)
+static int sun9i_dt_parsing(struct resource res)
{
- struct device_node *cpucfg_node, *sram_node, *node;
- struct resource res;
+ struct device_node *prcm_node, *cpucfg_node, *sram_node;
int ret;
- if (!of_machine_is_compatible("allwinner,sun9i-a80"))
- return -ENODEV;
-
- if (!sunxi_mc_smp_cpu_table_init())
- return -EINVAL;
-
- if (!cci_probed()) {
- pr_err("%s: CCI-400 not available\n", __func__);
- return -ENODEV;
- }
-
- node = of_find_compatible_node(NULL, NULL, "allwinner,sun9i-a80-prcm");
- if (!node) {
- pr_err("%s: PRCM not available\n", __func__);
+ prcm_node = of_find_compatible_node(NULL, NULL,
+ "allwinner,sun9i-a80-prcm");
+ if (!prcm_node)
return -ENODEV;
- }
/*
* Unfortunately we can not request the I/O region for the PRCM.
* It is shared with the PRCM clock.
*/
- prcm_base = of_iomap(node, 0);
- of_node_put(node);
+ prcm_base = of_iomap(prcm_node, 0);
+ of_node_put(prcm_node);
if (!prcm_base) {
pr_err("%s: failed to map PRCM registers\n", __func__);
+ iounmap(prcm_base);
return -ENOMEM;
}
@@ -748,35 +799,88 @@ static int __init sunxi_mc_smp_init(void)
goto err_put_sram_node;
}
- /* Configure CCI-400 for boot cluster */
- ret = sunxi_mc_smp_lookback();
- if (ret) {
- pr_err("%s: failed to configure boot cluster: %d\n",
- __func__, ret);
- goto err_unmap_release_secure_sram;
- }
+ r_cpucfg_base = NULL;
/* We don't need the CPUCFG and SRAM device nodes anymore */
of_node_put(cpucfg_node);
of_node_put(sram_node);
- /* Set the hardware entry point address */
- writel(__pa_symbol(sunxi_mc_smp_secondary_startup),
- prcm_base + PRCM_CPU_SOFT_ENTRY_REG);
-
- /* Actually enable multi cluster SMP */
- smp_set_ops(&sunxi_mc_smp_smp_ops);
-
- pr_info("sunxi multi cluster SMP support installed\n");
-
return 0;
-err_unmap_release_secure_sram:
- iounmap(sram_b_smp_base);
- of_address_to_resource(sram_node, 0, &res);
+err_unmap_release_cpucfg:
+ iounmap(cpucfg_base);
+ of_address_to_resource(cpucfg_node, 0, &res);
release_mem_region(res.start, resource_size(&res));
err_put_sram_node:
of_node_put(sram_node);
+err_put_cpucfg_node:
+ of_node_put(cpucfg_node);
+err_unmap_prcm:
+ iounmap(prcm_base);
+
+ return ret;
+}
+
+static int sun8i_dt_parsing(struct resource res)
+{
+ struct device_node *node, *cpucfg_node;
+ int ret;
+
+ node = of_find_compatible_node(NULL, NULL,
+ "allwinner,sun8i-a83t-prcm");
+ if (!node)
+ return -ENODEV;
+
+ /*
+ * Unfortunately we can not request the I/O region for the PRCM.
+ * It is shared with the PRCM clock.
+ */
+ prcm_base = of_iomap(node, 0);
+ of_node_put(node);
+ if (!prcm_base) {
+ pr_err("%s: failed to map PRCM registers\n", __func__);
+ iounmap(prcm_base);
+ return -ENOMEM;
+ }
+
+ cpucfg_node = of_find_compatible_node(NULL, NULL,
+ "allwinner,sun8i-a83t-cpucfg");
+ if (!cpucfg_node) {
+ ret = -ENODEV;
+ pr_err("%s: CPUCFG not available\n", __func__);
+ goto err_unmap_prcm;
+ }
+
+ cpucfg_base = of_io_request_and_map(cpucfg_node, 0, "sunxi-mc-smp");
+ if (IS_ERR(cpucfg_base)) {
+ ret = PTR_ERR(cpucfg_base);
+ pr_err("%s: failed to map CPUCFG registers: %d\n",
+ __func__, ret);
+ goto err_put_cpucfg_node;
+ }
+
+ node = of_find_compatible_node(NULL, NULL,
+ "allwinner,sun8i-a83t-r-cpucfg");
+ if (!node) {
+ ret = -ENODEV;
+ goto err_unmap_release_cpucfg;
+ }
+
+ r_cpucfg_base = of_iomap(node, 0);
+ if (!r_cpucfg_base) {
+ pr_err("%s: failed to map R-CPUCFG registers\n",
+ __func__);
+ ret = -ENOMEM;
+ goto err_put_node;
+ }
+
+ /* We don't need the CPUCFG device node anymore */
+ of_node_put(cpucfg_node);
+ of_node_put(node);
+
+ return 0;
+err_put_node:
+ of_node_put(node);
err_unmap_release_cpucfg:
iounmap(cpucfg_base);
of_address_to_resource(cpucfg_node, 0, &res);
@@ -785,6 +889,57 @@ static int __init sunxi_mc_smp_init(void)
of_node_put(cpucfg_node);
err_unmap_prcm:
iounmap(prcm_base);
+
+ return ret;
+}
+
+static int __init sunxi_mc_smp_init(void)
+{
+ struct resource res;
+ int ret;
+
+ if (!of_machine_is_compatible("allwinner,sun9i-a80") &&
+ !of_machine_is_compatible("allwinner,sun8i-a83t"))
+ return -ENODEV;
+
+ if (!sunxi_mc_smp_cpu_table_init())
+ return -EINVAL;
+
+ if (!cci_probed()) {
+ pr_err("%s: CCI-400 not available\n", __func__);
+ return -ENODEV;
+ }
+
+ if (of_machine_is_compatible("allwinner,sun9i-a80"))
+ ret = sun9i_dt_parsing(res);
+ else
+ ret = sun8i_dt_parsing(res);
+ if (ret) {
+ pr_err("%s: failed to parse DT: %d\n", __func__, ret);
+ return ret;
+ }
+
+ /* Configure CCI-400 for boot cluster */
+ ret = sunxi_mc_smp_lookback();
+ if (ret) {
+ pr_err("%s: failed to configure boot cluster: %d\n",
+ __func__, ret);
+ return ret; /* MYMY: Need to handle this error case */
+ }
+
+ /* Set the hardware entry point address */
+ if (of_machine_is_compatible("allwinner,sun9i-a80"))
+ writel(__pa_symbol(sunxi_mc_smp_secondary_startup),
+ prcm_base + PRCM_CPU_SOFT_ENTRY_REG);
+ else
+ writel(__pa_symbol(sunxi_mc_smp_secondary_startup),
+ r_cpucfg_base + R_CPUCFG_CPU_SOFT_ENTRY_REG);
+
+ /* Actually enable multi cluster SMP */
+ smp_set_ops(&sunxi_mc_smp_smp_ops);
+
+ pr_info("sunxi multi cluster SMP support installed\n");
+
return ret;
}
--
2.11.0
As we found in Sun9i-A80, PRCM is a collection of clock controls,
reset controls, and various power switches/gates.
It is used with CPUCFG for SMP bringup and CPU hotplugging.
Signed-off-by: Mylène Josserand <[email protected]>
---
arch/arm/boot/dts/sun8i-a83t.dtsi | 5 +++++
1 file changed, 5 insertions(+)
diff --git a/arch/arm/boot/dts/sun8i-a83t.dtsi b/arch/arm/boot/dts/sun8i-a83t.dtsi
index e45bab836620..3e7317ec3e43 100644
--- a/arch/arm/boot/dts/sun8i-a83t.dtsi
+++ b/arch/arm/boot/dts/sun8i-a83t.dtsi
@@ -699,6 +699,11 @@
interrupts = <GIC_SPI 32 IRQ_TYPE_LEVEL_HIGH>;
};
+ prcm@1f01400 {
+ compatible = "allwinner,sun8i-a83t-prcm";
+ reg = <0x1f01400 0x400>;
+ };
+
r_ccu: clock@1f01400 {
compatible = "allwinner,sun8i-a83t-r-ccu";
reg = <0x01f01400 0x400>;
--
2.11.0
The R_CPUCFG is a collection of registers needed for SMP bringup
on clusters and cluster's reset.
For the moment, documentation about this register is found in
Allwinner's code only.
Signed-off-by: Mylène Josserand <[email protected]>
---
arch/arm/boot/dts/sun8i-a83t.dtsi | 5 +++++
1 file changed, 5 insertions(+)
diff --git a/arch/arm/boot/dts/sun8i-a83t.dtsi b/arch/arm/boot/dts/sun8i-a83t.dtsi
index 3e7317ec3e43..7ae631fc04d5 100644
--- a/arch/arm/boot/dts/sun8i-a83t.dtsi
+++ b/arch/arm/boot/dts/sun8i-a83t.dtsi
@@ -714,6 +714,11 @@
#reset-cells = <1>;
};
+ r_cpucfg@1f01c00 {
+ compatible = "allwinner,sun8i-a83t-r-cpucfg";
+ reg = <0x1f01c00 0x100>;
+ };
+
r_pio: pinctrl@1f02c00 {
compatible = "allwinner,sun8i-a83t-r-pinctrl";
reg = <0x01f02c00 0x400>;
--
2.11.0
As we found in sun9i-a80, CPUCFG is a collection of registers that are
mapped to the SoC's signals from each individual processor core and
associated peripherals.
These registers are used for SMP bringup and CPU hotplugging.
Signed-off-by: Mylène Josserand <[email protected]>
---
arch/arm/boot/dts/sun8i-a83t.dtsi | 5 +++++
1 file changed, 5 insertions(+)
diff --git a/arch/arm/boot/dts/sun8i-a83t.dtsi b/arch/arm/boot/dts/sun8i-a83t.dtsi
index 7f4955a5fab7..e45bab836620 100644
--- a/arch/arm/boot/dts/sun8i-a83t.dtsi
+++ b/arch/arm/boot/dts/sun8i-a83t.dtsi
@@ -208,6 +208,11 @@
};
};
+ cpucfg@1700000 {
+ compatible = "allwinner,sun8i-a83t-cpucfg";
+ reg = <0x01700000 0x400>;
+ };
+
syscon: syscon@1c00000 {
compatible = "allwinner,sun8i-a83t-system-controller",
"syscon";
--
2.11.0
Hi,
On Mon, Feb 19, 2018 at 09:18:33AM +0100, Myl?ne Josserand wrote:
> As we found in Sun9i-A80, PRCM is a collection of clock controls,
> reset controls, and various power switches/gates.
> It is used with CPUCFG for SMP bringup and CPU hotplugging.
>
> Signed-off-by: Myl?ne Josserand <[email protected]>
> ---
> arch/arm/boot/dts/sun8i-a83t.dtsi | 5 +++++
> 1 file changed, 5 insertions(+)
>
> diff --git a/arch/arm/boot/dts/sun8i-a83t.dtsi b/arch/arm/boot/dts/sun8i-a83t.dtsi
> index e45bab836620..3e7317ec3e43 100644
> --- a/arch/arm/boot/dts/sun8i-a83t.dtsi
> +++ b/arch/arm/boot/dts/sun8i-a83t.dtsi
> @@ -699,6 +699,11 @@
> interrupts = <GIC_SPI 32 IRQ_TYPE_LEVEL_HIGH>;
> };
>
> + prcm@1f01400 {
> + compatible = "allwinner,sun8i-a83t-prcm";
> + reg = <0x1f01400 0x400>;
> + };
> +
> r_ccu: clock@1f01400 {
> compatible = "allwinner,sun8i-a83t-r-ccu";
> reg = <0x01f01400 0x400>;
These two are exactly the same nodes.
Thanks!
Maxime
--
Maxime Ripard, Bootlin (formerly Free Electrons)
Embedded Linux and Kernel engineering
http://bootlin.com
Hi,
On Mon, Feb 19, 2018 at 09:18:35AM +0100, Myl?ne Josserand wrote:
> + interrupts = <GIC_SPI 132 IRQ_TYPE_LEVEL_HIGH>,
> + <GIC_SPI 133 IRQ_TYPE_LEVEL_HIGH>,
> + <GIC_SPI 134 IRQ_TYPE_LEVEL_HIGH>,
> + <GIC_SPI 135 IRQ_TYPE_LEVEL_HIGH>,
> + <GIC_SPI 151 IRQ_TYPE_LEVEL_HIGH>,
> + <GIC_SPI 152 IRQ_TYPE_LEVEL_HIGH>,
> + <GIC_SPI 153 IRQ_TYPE_LEVEL_HIGH>,
> + <GIC_SPI 154 IRQ_TYPE_LEVEL_HIGH>;
You should align these blocks on the first one in the array.
Thanks!
Maxime
--
Maxime Ripard, Bootlin (formerly Free Electrons)
Embedded Linux and Kernel engineering
http://bootlin.com
Hi,
On Mon, Feb 19, 2018 at 09:18:36AM +0100, Myl?ne Josserand wrote:
> The ARM architected timers use an offset between their physical and
> virtual counters. That offset should be configured by the bootloader
> in CNTVOFF.
>
> However, the A83t bootloader fails to do so, and we end up with an
> undefined offset (which in our case is random), meaning that each CPU
> will have a different time, which isn't working very well.
>
> Fix that by setting the arm,cpu-registers-not-fw-configured that will
> make Linux use the physical timers instead of the virtual ones. One
> possible side effect would be that the virtualization features would
> be disabled. However, due to the way the GIC has been integrated in
> the system, it is already unusable so we're effectively not losing any
> feature.
>
> Signed-off-by: Myl?ne Josserand <[email protected]>
One small nitpick on this one (and the previous one), arm in the
subject prefix should be uppercase.
Thanks!
Maxime
--
Maxime Ripard, Bootlin (formerly Free Electrons)
Embedded Linux and Kernel engineering
http://bootlin.com
On Mon, Feb 19, 2018 at 09:18:37AM +0100, Myl?ne Josserand wrote:
> On sun8i-a63t, hotplug CPU for CPU0 is currently not working.
> Remove the possibility to disable CPU0 only for sun8i-a83t.
>
> Signed-off-by: Myl?ne Josserand <[email protected]>
> ---
> arch/arm/mach-sunxi/mc_smp.c | 6 +++++-
> 1 file changed, 5 insertions(+), 1 deletion(-)
>
> diff --git a/arch/arm/mach-sunxi/mc_smp.c b/arch/arm/mach-sunxi/mc_smp.c
> index fec592bf68b4..69d5ae5b3d72 100644
> --- a/arch/arm/mach-sunxi/mc_smp.c
> +++ b/arch/arm/mach-sunxi/mc_smp.c
> @@ -661,8 +661,12 @@ static int sunxi_mc_smp_cpu_kill(unsigned int l_cpu)
> return !ret;
> }
>
> -static bool sunxi_mc_smp_cpu_can_disable(unsigned int __unused)
> +static bool sunxi_mc_smp_cpu_can_disable(unsigned int cpu)
> {
> + /* CPU0 hotplug handled only for sun9i */
> + if (of_machine_is_compatible("allwinner,sun8i-a83t"))
> + if (cpu == 0)
> + return false;
This also means that you have a window in your patch sequence where
you can disable the CPU0 on the A83t. This patch should be earlier.
Maxime
--
Maxime Ripard, Bootlin (formerly Free Electrons)
Embedded Linux and Kernel engineering
http://bootlin.com
Hi,
On Mon, Feb 19, 2018 at 09:18:31AM +0100, Myl?ne Josserand wrote:
> Add the support for A83T.
>
> A83T SoC has an additional register than A80 to handle CPU configurations:
> R_CPUS_CFG. Information about the register comes from Allwinner's BSP
> driver.
> An important difference is the Power Off Gating register for clusters
> which is BIT(4) in case of SUN9I-A80 and BIT(0) in case of SUN8I-A83T.
>
> Signed-off-by: Myl?ne Josserand <[email protected]>
This is some high-level review, but you should split this patch in
three:
- One to refactor the current code to split the DT parsing function
(and rename the function to sun9i_smp_dt_parse) and the variable renames
- One to enable the A83t SMP (with the function called sun8i_a83t_smp_dt_parse)
- One to enable the A83t hotplug (merged with your last patch)
Also, you're calling a lot of times of_machine_is_compatible, and this
is quite inefficient. You should call it once and store the result.
Thanks!
Maxime
--
Maxime Ripard, Bootlin (formerly Free Electrons)
Embedded Linux and Kernel engineering
http://bootlin.com
Hello Maxime,
On Mon, 19 Feb 2018 10:04:01 +0100
Maxime Ripard <[email protected]> wrote:
> Hi,
>
> On Mon, Feb 19, 2018 at 09:18:31AM +0100, Mylène Josserand wrote:
> > Add the support for A83T.
> >
> > A83T SoC has an additional register than A80 to handle CPU configurations:
> > R_CPUS_CFG. Information about the register comes from Allwinner's BSP
> > driver.
> > An important difference is the Power Off Gating register for clusters
> > which is BIT(4) in case of SUN9I-A80 and BIT(0) in case of SUN8I-A83T.
> >
> > Signed-off-by: Mylène Josserand <[email protected]>
>
> This is some high-level review, but you should split this patch in
> three:
> - One to refactor the current code to split the DT parsing function
> (and rename the function to sun9i_smp_dt_parse) and the variable renames
> - One to enable the A83t SMP (with the function called sun8i_a83t_smp_dt_parse)
> - One to enable the A83t hotplug (merged with your last patch)
Thank you for the review.
Sure, I will split my patch in three.
>
> Also, you're calling a lot of times of_machine_is_compatible, and this
> is quite inefficient. You should call it once and store the result.
True.
Thanks,
Mylène
--
Mylène Josserand, Bootlin (formerly Free Electrons)
Embedded Linux and Kernel engineering
http://bootlin.com
Hi,
On Mon, 19 Feb 2018 09:48:22 +0100
Maxime Ripard <[email protected]> wrote:
> Hi,
>
> On Mon, Feb 19, 2018 at 09:18:33AM +0100, Mylène Josserand wrote:
> > As we found in Sun9i-A80, PRCM is a collection of clock controls,
> > reset controls, and various power switches/gates.
> > It is used with CPUCFG for SMP bringup and CPU hotplugging.
> >
> > Signed-off-by: Mylène Josserand <[email protected]>
> > ---
> > arch/arm/boot/dts/sun8i-a83t.dtsi | 5 +++++
> > 1 file changed, 5 insertions(+)
> >
> > diff --git a/arch/arm/boot/dts/sun8i-a83t.dtsi b/arch/arm/boot/dts/sun8i-a83t.dtsi
> > index e45bab836620..3e7317ec3e43 100644
> > --- a/arch/arm/boot/dts/sun8i-a83t.dtsi
> > +++ b/arch/arm/boot/dts/sun8i-a83t.dtsi
> > @@ -699,6 +699,11 @@
> > interrupts = <GIC_SPI 32 IRQ_TYPE_LEVEL_HIGH>;
> > };
> >
> > + prcm@1f01400 {
> > + compatible = "allwinner,sun8i-a83t-prcm";
> > + reg = <0x1f01400 0x400>;
> > + };
> > +
> > r_ccu: clock@1f01400 {
> > compatible = "allwinner,sun8i-a83t-r-ccu";
> > reg = <0x01f01400 0x400>;
>
> These two are exactly the same nodes.
Oh, yes, I did not pay attention to that.
Thanks,
Mylène
--
Mylène Josserand, Bootlin (formerly Free Electrons)
Embedded Linux and Kernel engineering
http://bootlin.com
Hi,
On Mon, 19 Feb 2018 09:49:44 +0100
Maxime Ripard <[email protected]> wrote:
> Hi,
>
> On Mon, Feb 19, 2018 at 09:18:35AM +0100, Mylène Josserand wrote:
> > + interrupts = <GIC_SPI 132 IRQ_TYPE_LEVEL_HIGH>,
> > + <GIC_SPI 133 IRQ_TYPE_LEVEL_HIGH>,
> > + <GIC_SPI 134 IRQ_TYPE_LEVEL_HIGH>,
> > + <GIC_SPI 135 IRQ_TYPE_LEVEL_HIGH>,
> > + <GIC_SPI 151 IRQ_TYPE_LEVEL_HIGH>,
> > + <GIC_SPI 152 IRQ_TYPE_LEVEL_HIGH>,
> > + <GIC_SPI 153 IRQ_TYPE_LEVEL_HIGH>,
> > + <GIC_SPI 154 IRQ_TYPE_LEVEL_HIGH>;
>
> You should align these blocks on the first one in the array.
Yep, thanks.
Mylène
--
Mylène Josserand, Bootlin (formerly Free Electrons)
Embedded Linux and Kernel engineering
http://bootlin.com
Hi,
On Mon, 19 Feb 2018 09:52:05 +0100
Maxime Ripard <[email protected]> wrote:
> Hi,
>
> On Mon, Feb 19, 2018 at 09:18:36AM +0100, Mylène Josserand wrote:
> > The ARM architected timers use an offset between their physical and
> > virtual counters. That offset should be configured by the bootloader
> > in CNTVOFF.
> >
> > However, the A83t bootloader fails to do so, and we end up with an
> > undefined offset (which in our case is random), meaning that each CPU
> > will have a different time, which isn't working very well.
> >
> > Fix that by setting the arm,cpu-registers-not-fw-configured that will
> > make Linux use the physical timers instead of the virtual ones. One
> > possible side effect would be that the virtualization features would
> > be disabled. However, due to the way the GIC has been integrated in
> > the system, it is already unusable so we're effectively not losing any
> > feature.
> >
> > Signed-off-by: Mylène Josserand <[email protected]>
>
> One small nitpick on this one (and the previous one), arm in the
> subject prefix should be uppercase.
Okay, I will fix that in V4.
Mylène
--
Mylène Josserand, Bootlin (formerly Free Electrons)
Embedded Linux and Kernel engineering
http://bootlin.com
Hi,
On Mon, 19 Feb 2018 09:54:08 +0100
Maxime Ripard <[email protected]> wrote:
> On Mon, Feb 19, 2018 at 09:18:37AM +0100, Mylène Josserand wrote:
> > On sun8i-a63t, hotplug CPU for CPU0 is currently not working.
> > Remove the possibility to disable CPU0 only for sun8i-a83t.
> >
> > Signed-off-by: Mylène Josserand <[email protected]>
> > ---
> > arch/arm/mach-sunxi/mc_smp.c | 6 +++++-
> > 1 file changed, 5 insertions(+), 1 deletion(-)
> >
> > diff --git a/arch/arm/mach-sunxi/mc_smp.c b/arch/arm/mach-sunxi/mc_smp.c
> > index fec592bf68b4..69d5ae5b3d72 100644
> > --- a/arch/arm/mach-sunxi/mc_smp.c
> > +++ b/arch/arm/mach-sunxi/mc_smp.c
> > @@ -661,8 +661,12 @@ static int sunxi_mc_smp_cpu_kill(unsigned int l_cpu)
> > return !ret;
> > }
> >
> > -static bool sunxi_mc_smp_cpu_can_disable(unsigned int __unused)
> > +static bool sunxi_mc_smp_cpu_can_disable(unsigned int cpu)
> > {
> > + /* CPU0 hotplug handled only for sun9i */
> > + if (of_machine_is_compatible("allwinner,sun8i-a83t"))
> > + if (cpu == 0)
> > + return false;
>
> This also means that you have a window in your patch sequence where
> you can disable the CPU0 on the A83t. This patch should be earlier.
It is true.
With your comment in my first patch, it will be done earlier.
Thanks,
Mylène
--
Mylène Josserand, Bootlin (formerly Free Electrons)
Embedded Linux and Kernel engineering
http://bootlin.com
On Mon, Feb 19, 2018 at 4:18 PM, Mylène Josserand
<[email protected]> wrote:
> Add the support for A83T.
>
> A83T SoC has an additional register than A80 to handle CPU configurations:
> R_CPUS_CFG. Information about the register comes from Allwinner's BSP
> driver.
> An important difference is the Power Off Gating register for clusters
> which is BIT(4) in case of SUN9I-A80 and BIT(0) in case of SUN8I-A83T.
>
> Signed-off-by: Mylène Josserand <[email protected]>
> ---
> arch/arm/mach-sunxi/Kconfig | 2 +-
> arch/arm/mach-sunxi/mc_smp.c | 239 +++++++++++++++++++++++++++++++++++--------
The same high-level comments as Maxime. Splitting the patch
will make this much easier to understand.
> 2 files changed, 198 insertions(+), 43 deletions(-)
>
> diff --git a/arch/arm/mach-sunxi/Kconfig b/arch/arm/mach-sunxi/Kconfig
> index ce53ceaf4cc5..a0ad35c41c02 100644
> --- a/arch/arm/mach-sunxi/Kconfig
> +++ b/arch/arm/mach-sunxi/Kconfig
> @@ -51,7 +51,7 @@ config MACH_SUN9I
> config ARCH_SUNXI_MC_SMP
> bool
> depends on SMP
> - default MACH_SUN9I
> + default y if MACH_SUN9I || MACH_SUN8I
> select ARM_CCI400_PORT_CTRL
> select ARM_CPU_SUSPEND
>
> diff --git a/arch/arm/mach-sunxi/mc_smp.c b/arch/arm/mach-sunxi/mc_smp.c
> index 11e46c6efb90..fec592bf68b4 100644
> --- a/arch/arm/mach-sunxi/mc_smp.c
> +++ b/arch/arm/mach-sunxi/mc_smp.c
> @@ -55,20 +55,29 @@
> #define CPUCFG_CX_RST_CTRL_L2_RST BIT(8)
> #define CPUCFG_CX_RST_CTRL_CX_RST(n) BIT(4 + (n))
> #define CPUCFG_CX_RST_CTRL_CORE_RST(n) BIT(n)
> +#define CPUCFG_CX_RST_CTRL_CORE_RST_ALL (0xf << 0)
>
> #define PRCM_CPU_PO_RST_CTRL(c) (0x4 + 0x4 * (c))
> #define PRCM_CPU_PO_RST_CTRL_CORE(n) BIT(n)
> #define PRCM_CPU_PO_RST_CTRL_CORE_ALL 0xf
> #define PRCM_PWROFF_GATING_REG(c) (0x100 + 0x4 * (c))
> -#define PRCM_PWROFF_GATING_REG_CLUSTER BIT(4)
> +/* The power off register for clusters are different from SUN9I and SUN8I */
> +#define PRCM_PWROFF_GATING_REG_CLUSTER_SUN8I BIT(0)
> +#define PRCM_PWROFF_GATING_REG_CLUSTER_SUN9I BIT(4)
> #define PRCM_PWROFF_GATING_REG_CORE(n) BIT(n)
> #define PRCM_PWR_SWITCH_REG(c, cpu) (0x140 + 0x10 * (c) + 0x4 * (cpu))
> #define PRCM_CPU_SOFT_ENTRY_REG 0x164
>
> +/* R_CPUCFG registers, specific to SUN8I */
> +#define R_CPUCFG_CLUSTER_PO_RST_CTRL(c) (0x30 + (c) * 0x4)
> +#define R_CPUCFG_CLUSTER_PO_RST_CTRL_CORE(n) BIT(n)
> +#define R_CPUCFG_CPU_SOFT_ENTRY_REG 0x01a4
> +
> #define CPU0_SUPPORT_HOTPLUG_MAGIC0 0xFA50392F
> #define CPU0_SUPPORT_HOTPLUG_MAGIC1 0x790DCA3A
>
> static void __iomem *cpucfg_base;
> +static void __iomem *r_cpucfg_base;
> static void __iomem *prcm_base;
> static void __iomem *sram_b_smp_base;
>
> @@ -157,6 +166,16 @@ static int sunxi_cpu_powerup(unsigned int cpu, unsigned int cluster)
> reg &= ~PRCM_CPU_PO_RST_CTRL_CORE(cpu);
> writel(reg, prcm_base + PRCM_CPU_PO_RST_CTRL(cluster));
>
> + if (r_cpucfg_base) {
> + /* assert cpu power-on reset */
> + reg = readl(r_cpucfg_base +
> + R_CPUCFG_CLUSTER_PO_RST_CTRL(cluster));
> + reg &= ~(R_CPUCFG_CLUSTER_PO_RST_CTRL_CORE(cpu));
> + writel(reg, r_cpucfg_base +
> + R_CPUCFG_CLUSTER_PO_RST_CTRL(cluster));
> + udelay(10);
> + }
> +
> /* Cortex-A7: hold L1 reset disable signal low */
> if (!sunxi_core_is_cortex_a15(cpu, cluster)) {
> reg = readl(cpucfg_base + CPUCFG_CX_CTRL_REG0(cluster));
> @@ -180,17 +199,37 @@ static int sunxi_cpu_powerup(unsigned int cpu, unsigned int cluster)
> /* open power switch */
> sunxi_cpu_power_switch_set(cpu, cluster, true);
>
> + /* Handle A83T bit swap */
> + if (of_machine_is_compatible("allwinner,sun8i-a83t")) {
> + if (cpu == 0)
> + cpu = 4;
> + }
> +
> /* clear processor power gate */
> reg = readl(prcm_base + PRCM_PWROFF_GATING_REG(cluster));
> reg &= ~PRCM_PWROFF_GATING_REG_CORE(cpu);
> writel(reg, prcm_base + PRCM_PWROFF_GATING_REG(cluster));
> udelay(20);
>
> + if (of_machine_is_compatible("allwinner,sun8i-a83t")) {
> + if (cpu == 4)
> + cpu = 0;
> + }
> +
> /* de-assert processor power-on reset */
> reg = readl(prcm_base + PRCM_CPU_PO_RST_CTRL(cluster));
> reg |= PRCM_CPU_PO_RST_CTRL_CORE(cpu);
> writel(reg, prcm_base + PRCM_CPU_PO_RST_CTRL(cluster));
>
> + if (r_cpucfg_base) {
> + reg = readl(r_cpucfg_base +
> + R_CPUCFG_CLUSTER_PO_RST_CTRL(cluster));
> + reg |= R_CPUCFG_CLUSTER_PO_RST_CTRL_CORE(cpu);
> + writel(reg, r_cpucfg_base +
> + R_CPUCFG_CLUSTER_PO_RST_CTRL(cluster));
> + udelay(10);
> + }
> +
> /* de-assert all processor resets */
> reg = readl(cpucfg_base + CPUCFG_CX_RST_CTRL(cluster));
> reg |= CPUCFG_CX_RST_CTRL_DBG_RST(cpu);
> @@ -212,6 +251,14 @@ static int sunxi_cluster_powerup(unsigned int cluster)
> if (cluster >= SUNXI_NR_CLUSTERS)
> return -EINVAL;
>
> + /* For A83T, assert cluster cores resets */
> + if (of_machine_is_compatible("allwinner,sun8i-a83t")) {
> + reg = readl(cpucfg_base + CPUCFG_CX_RST_CTRL(cluster));
> + reg &= ~CPUCFG_CX_RST_CTRL_CORE_RST_ALL; /* Core Reset */
> + writel(reg, cpucfg_base + CPUCFG_CX_RST_CTRL(cluster));
> + udelay(10);
> + }
> +
> /* assert ACINACTM */
> reg = readl(cpucfg_base + CPUCFG_CX_CTRL_REG1(cluster));
> reg |= CPUCFG_CX_CTRL_REG1_ACINACTM;
> @@ -222,6 +269,16 @@ static int sunxi_cluster_powerup(unsigned int cluster)
> reg &= ~PRCM_CPU_PO_RST_CTRL_CORE_ALL;
> writel(reg, prcm_base + PRCM_CPU_PO_RST_CTRL(cluster));
>
> + /* assert cluster cores resets */
> + if (r_cpucfg_base) {
> + reg = readl(r_cpucfg_base +
> + R_CPUCFG_CLUSTER_PO_RST_CTRL(cluster));
> + reg &= ~CPUCFG_CX_RST_CTRL_CORE_RST_ALL;
> + writel(reg, r_cpucfg_base +
> + R_CPUCFG_CLUSTER_PO_RST_CTRL(cluster));
> + udelay(10);
> + }
> +
> /* assert cluster resets */
> reg = readl(cpucfg_base + CPUCFG_CX_RST_CTRL(cluster));
> reg &= ~CPUCFG_CX_RST_CTRL_DBG_SOC_RST;
> @@ -252,7 +309,10 @@ static int sunxi_cluster_powerup(unsigned int cluster)
>
> /* clear cluster power gate */
> reg = readl(prcm_base + PRCM_PWROFF_GATING_REG(cluster));
> - reg &= ~PRCM_PWROFF_GATING_REG_CLUSTER;
> + if (of_machine_is_compatible("allwinner,sun8i-a83t"))
> + reg &= ~PRCM_PWROFF_GATING_REG_CLUSTER_SUN8I;
> + else
> + reg &= ~PRCM_PWROFF_GATING_REG_CLUSTER_SUN9I;
> writel(reg, prcm_base + PRCM_PWROFF_GATING_REG(cluster));
> udelay(20);
>
> @@ -516,7 +576,10 @@ static int sunxi_cluster_powerdown(unsigned int cluster)
> /* gate cluster power */
> pr_debug("%s: gate cluster power\n", __func__);
> reg = readl(prcm_base + PRCM_PWROFF_GATING_REG(cluster));
> - reg |= PRCM_PWROFF_GATING_REG_CLUSTER;
> + if (of_machine_is_compatible("allwinner,sun8i-a83t"))
> + reg |= PRCM_PWROFF_GATING_REG_CLUSTER_SUN8I;
> + else
> + reg |= PRCM_PWROFF_GATING_REG_CLUSTER_SUN9I;
> writel(reg, prcm_base + PRCM_PWROFF_GATING_REG(cluster));
> udelay(20);
>
> @@ -684,37 +747,25 @@ static int __init sunxi_mc_smp_lookback(void)
> return ret;
> }
>
> -static int __init sunxi_mc_smp_init(void)
> +static int sun9i_dt_parsing(struct resource res)
> {
> - struct device_node *cpucfg_node, *sram_node, *node;
> - struct resource res;
> + struct device_node *prcm_node, *cpucfg_node, *sram_node;
> int ret;
>
> - if (!of_machine_is_compatible("allwinner,sun9i-a80"))
> - return -ENODEV;
> -
> - if (!sunxi_mc_smp_cpu_table_init())
> - return -EINVAL;
> -
> - if (!cci_probed()) {
> - pr_err("%s: CCI-400 not available\n", __func__);
> - return -ENODEV;
> - }
> -
> - node = of_find_compatible_node(NULL, NULL, "allwinner,sun9i-a80-prcm");
> - if (!node) {
> - pr_err("%s: PRCM not available\n", __func__);
> + prcm_node = of_find_compatible_node(NULL, NULL,
> + "allwinner,sun9i-a80-prcm");
> + if (!prcm_node)
> return -ENODEV;
> - }
>
> /*
> * Unfortunately we can not request the I/O region for the PRCM.
> * It is shared with the PRCM clock.
> */
> - prcm_base = of_iomap(node, 0);
> - of_node_put(node);
> + prcm_base = of_iomap(prcm_node, 0);
> + of_node_put(prcm_node);
> if (!prcm_base) {
> pr_err("%s: failed to map PRCM registers\n", __func__);
> + iounmap(prcm_base);
Why are you trying to unmap the pointer that you already failed to map?
> return -ENOMEM;
> }
>
> @@ -748,35 +799,88 @@ static int __init sunxi_mc_smp_init(void)
> goto err_put_sram_node;
> }
>
> - /* Configure CCI-400 for boot cluster */
> - ret = sunxi_mc_smp_lookback();
> - if (ret) {
> - pr_err("%s: failed to configure boot cluster: %d\n",
> - __func__, ret);
> - goto err_unmap_release_secure_sram;
> - }
> + r_cpucfg_base = NULL;
This is not needed. Global static variables without initial values are
always initialized to 0.
>
> /* We don't need the CPUCFG and SRAM device nodes anymore */
> of_node_put(cpucfg_node);
> of_node_put(sram_node);
>
> - /* Set the hardware entry point address */
> - writel(__pa_symbol(sunxi_mc_smp_secondary_startup),
> - prcm_base + PRCM_CPU_SOFT_ENTRY_REG);
> -
> - /* Actually enable multi cluster SMP */
> - smp_set_ops(&sunxi_mc_smp_smp_ops);
> -
> - pr_info("sunxi multi cluster SMP support installed\n");
> -
> return 0;
>
> -err_unmap_release_secure_sram:
> - iounmap(sram_b_smp_base);
> - of_address_to_resource(sram_node, 0, &res);
> +err_unmap_release_cpucfg:
> + iounmap(cpucfg_base);
> + of_address_to_resource(cpucfg_node, 0, &res);
> release_mem_region(res.start, resource_size(&res));
> err_put_sram_node:
> of_node_put(sram_node);
> +err_put_cpucfg_node:
> + of_node_put(cpucfg_node);
> +err_unmap_prcm:
> + iounmap(prcm_base);
> +
> + return ret;
> +}
> +
> +static int sun8i_dt_parsing(struct resource res)
> +{
> + struct device_node *node, *cpucfg_node;
> + int ret;
> +
> + node = of_find_compatible_node(NULL, NULL,
> + "allwinner,sun8i-a83t-prcm");
> + if (!node)
> + return -ENODEV;
> +
> + /*
> + * Unfortunately we can not request the I/O region for the PRCM.
> + * It is shared with the PRCM clock.
> + */
> + prcm_base = of_iomap(node, 0);
> + of_node_put(node);
> + if (!prcm_base) {
> + pr_err("%s: failed to map PRCM registers\n", __func__);
> + iounmap(prcm_base);
> + return -ENOMEM;
> + }
> +
> + cpucfg_node = of_find_compatible_node(NULL, NULL,
> + "allwinner,sun8i-a83t-cpucfg");
> + if (!cpucfg_node) {
> + ret = -ENODEV;
> + pr_err("%s: CPUCFG not available\n", __func__);
> + goto err_unmap_prcm;
> + }
> +
> + cpucfg_base = of_io_request_and_map(cpucfg_node, 0, "sunxi-mc-smp");
> + if (IS_ERR(cpucfg_base)) {
> + ret = PTR_ERR(cpucfg_base);
> + pr_err("%s: failed to map CPUCFG registers: %d\n",
> + __func__, ret);
> + goto err_put_cpucfg_node;
> + }
> +
> + node = of_find_compatible_node(NULL, NULL,
> + "allwinner,sun8i-a83t-r-cpucfg");
> + if (!node) {
> + ret = -ENODEV;
> + goto err_unmap_release_cpucfg;
> + }
> +
> + r_cpucfg_base = of_iomap(node, 0);
> + if (!r_cpucfg_base) {
> + pr_err("%s: failed to map R-CPUCFG registers\n",
> + __func__);
> + ret = -ENOMEM;
> + goto err_put_node;
> + }
> +
> + /* We don't need the CPUCFG device node anymore */
> + of_node_put(cpucfg_node);
> + of_node_put(node);
> +
> + return 0;
> +err_put_node:
> + of_node_put(node);
> err_unmap_release_cpucfg:
> iounmap(cpucfg_base);
> of_address_to_resource(cpucfg_node, 0, &res);
> @@ -785,6 +889,57 @@ static int __init sunxi_mc_smp_init(void)
> of_node_put(cpucfg_node);
> err_unmap_prcm:
> iounmap(prcm_base);
> +
> + return ret;
> +}
> +
> +static int __init sunxi_mc_smp_init(void)
> +{
> + struct resource res;
> + int ret;
> +
> + if (!of_machine_is_compatible("allwinner,sun9i-a80") &&
> + !of_machine_is_compatible("allwinner,sun8i-a83t"))
> + return -ENODEV;
> +
> + if (!sunxi_mc_smp_cpu_table_init())
> + return -EINVAL;
> +
> + if (!cci_probed()) {
> + pr_err("%s: CCI-400 not available\n", __func__);
> + return -ENODEV;
> + }
> +
> + if (of_machine_is_compatible("allwinner,sun9i-a80"))
> + ret = sun9i_dt_parsing(res);
> + else
> + ret = sun8i_dt_parsing(res);
> + if (ret) {
> + pr_err("%s: failed to parse DT: %d\n", __func__, ret);
> + return ret;
> + }
> +
> + /* Configure CCI-400 for boot cluster */
> + ret = sunxi_mc_smp_lookback();
> + if (ret) {
> + pr_err("%s: failed to configure boot cluster: %d\n",
> + __func__, ret);
> + return ret; /* MYMY: Need to handle this error case */
Please add functions to release the resources. They will need to be
platform specific, since it requires you to find the device node to
get the resource that you want to release (for CPUCFG / R-CPUCFG).
For sun9i do it in the refactor patch. For A83T do it in the patch
you add support.
Thanks
ChenYu
> + }
> +
> + /* Set the hardware entry point address */
> + if (of_machine_is_compatible("allwinner,sun9i-a80"))
> + writel(__pa_symbol(sunxi_mc_smp_secondary_startup),
> + prcm_base + PRCM_CPU_SOFT_ENTRY_REG);
> + else
> + writel(__pa_symbol(sunxi_mc_smp_secondary_startup),
> + r_cpucfg_base + R_CPUCFG_CPU_SOFT_ENTRY_REG);
> +
> + /* Actually enable multi cluster SMP */
> + smp_set_ops(&sunxi_mc_smp_smp_ops);
> +
> + pr_info("sunxi multi cluster SMP support installed\n");
> +
> return ret;
> }
>
> --
> 2.11.0
>
Hi Mylène,
On 19/02/18 08:18, Mylène Josserand wrote:
> The ARM architected timers use an offset between their physical and
> virtual counters. That offset should be configured by the bootloader
> in CNTVOFF.
>
> However, the A83t bootloader fails to do so, and we end up with an
> undefined offset (which in our case is random), meaning that each CPU
> will have a different time, which isn't working very well.
>
> Fix that by setting the arm,cpu-registers-not-fw-configured that will
> make Linux use the physical timers instead of the virtual ones. One
> possible side effect would be that the virtualization features would
> be disabled. However, due to the way the GIC has been integrated in
> the system, it is already unusable so we're effectively not losing any
> feature.
>
> Signed-off-by: Mylène Josserand <[email protected]>
> ---
> arch/arm/boot/dts/sun8i-a83t.dtsi | 1 +
> 1 file changed, 1 insertion(+)
>
> diff --git a/arch/arm/boot/dts/sun8i-a83t.dtsi b/arch/arm/boot/dts/sun8i-a83t.dtsi
> index e97a6d17b8d0..b9bdb891cf2f 100644
> --- a/arch/arm/boot/dts/sun8i-a83t.dtsi
> +++ b/arch/arm/boot/dts/sun8i-a83t.dtsi
> @@ -123,6 +123,7 @@
> <GIC_PPI 14 (GIC_CPU_MASK_SIMPLE(8) | IRQ_TYPE_LEVEL_LOW)>,
> <GIC_PPI 11 (GIC_CPU_MASK_SIMPLE(8) | IRQ_TYPE_LEVEL_LOW)>,
> <GIC_PPI 10 (GIC_CPU_MASK_SIMPLE(8) | IRQ_TYPE_LEVEL_LOW)>;
> + arm,cpu-registers-not-fw-configured;
> };
>
> clocks {
>
Is the firmware dropping you in the kernel in secure or non-secure mode?
If the later, what you have is the only solution. If the former, that
I'd suggest you adopt what we already have for the Renesas stuff (see
arch/arm/mach-shmobile/headsmp-apmu.S and commit 3fd45a136ff6).
It would allow you to use the virtual timer as intended.
Thanks,
M.
--
Jazz is not dead. It just smells funny...
Hello Chen-Yu,
On Tue, 20 Feb 2018 11:32:03 +0800
Chen-Yu Tsai <[email protected]> wrote:
> On Mon, Feb 19, 2018 at 4:18 PM, Mylène Josserand
> <[email protected]> wrote:
> > Add the support for A83T.
> >
> > A83T SoC has an additional register than A80 to handle CPU configurations:
> > R_CPUS_CFG. Information about the register comes from Allwinner's BSP
> > driver.
> > An important difference is the Power Off Gating register for clusters
> > which is BIT(4) in case of SUN9I-A80 and BIT(0) in case of SUN8I-A83T.
> >
> > Signed-off-by: Mylène Josserand <[email protected]>
> > ---
> > arch/arm/mach-sunxi/Kconfig | 2 +-
> > arch/arm/mach-sunxi/mc_smp.c | 239 +++++++++++++++++++++++++++++++++++--------
>
> The same high-level comments as Maxime. Splitting the patch
> will make this much easier to understand.
Yep, I will do it in next version.
>
> > 2 files changed, 198 insertions(+), 43 deletions(-)
> >
> > diff --git a/arch/arm/mach-sunxi/Kconfig b/arch/arm/mach-sunxi/Kconfig
> > index ce53ceaf4cc5..a0ad35c41c02 100644
> > --- a/arch/arm/mach-sunxi/Kconfig
> > +++ b/arch/arm/mach-sunxi/Kconfig
> > @@ -51,7 +51,7 @@ config MACH_SUN9I
> > config ARCH_SUNXI_MC_SMP
> > bool
> > depends on SMP
> > - default MACH_SUN9I
> > + default y if MACH_SUN9I || MACH_SUN8I
> > select ARM_CCI400_PORT_CTRL
> > select ARM_CPU_SUSPEND
> >
> > diff --git a/arch/arm/mach-sunxi/mc_smp.c b/arch/arm/mach-sunxi/mc_smp.c
> > index 11e46c6efb90..fec592bf68b4 100644
> > --- a/arch/arm/mach-sunxi/mc_smp.c
> > +++ b/arch/arm/mach-sunxi/mc_smp.c
> > @@ -55,20 +55,29 @@
> > #define CPUCFG_CX_RST_CTRL_L2_RST BIT(8)
> > #define CPUCFG_CX_RST_CTRL_CX_RST(n) BIT(4 + (n))
> > #define CPUCFG_CX_RST_CTRL_CORE_RST(n) BIT(n)
> > +#define CPUCFG_CX_RST_CTRL_CORE_RST_ALL (0xf << 0)
> >
> > #define PRCM_CPU_PO_RST_CTRL(c) (0x4 + 0x4 * (c))
> > #define PRCM_CPU_PO_RST_CTRL_CORE(n) BIT(n)
> > #define PRCM_CPU_PO_RST_CTRL_CORE_ALL 0xf
> > #define PRCM_PWROFF_GATING_REG(c) (0x100 + 0x4 * (c))
> > -#define PRCM_PWROFF_GATING_REG_CLUSTER BIT(4)
> > +/* The power off register for clusters are different from SUN9I and SUN8I */
> > +#define PRCM_PWROFF_GATING_REG_CLUSTER_SUN8I BIT(0)
> > +#define PRCM_PWROFF_GATING_REG_CLUSTER_SUN9I BIT(4)
> > #define PRCM_PWROFF_GATING_REG_CORE(n) BIT(n)
> > #define PRCM_PWR_SWITCH_REG(c, cpu) (0x140 + 0x10 * (c) + 0x4 * (cpu))
> > #define PRCM_CPU_SOFT_ENTRY_REG 0x164
> >
> > +/* R_CPUCFG registers, specific to SUN8I */
> > +#define R_CPUCFG_CLUSTER_PO_RST_CTRL(c) (0x30 + (c) * 0x4)
> > +#define R_CPUCFG_CLUSTER_PO_RST_CTRL_CORE(n) BIT(n)
> > +#define R_CPUCFG_CPU_SOFT_ENTRY_REG 0x01a4
> > +
> > #define CPU0_SUPPORT_HOTPLUG_MAGIC0 0xFA50392F
> > #define CPU0_SUPPORT_HOTPLUG_MAGIC1 0x790DCA3A
> >
> > static void __iomem *cpucfg_base;
> > +static void __iomem *r_cpucfg_base;
> > static void __iomem *prcm_base;
> > static void __iomem *sram_b_smp_base;
> >
> > @@ -157,6 +166,16 @@ static int sunxi_cpu_powerup(unsigned int cpu, unsigned int cluster)
> > reg &= ~PRCM_CPU_PO_RST_CTRL_CORE(cpu);
> > writel(reg, prcm_base + PRCM_CPU_PO_RST_CTRL(cluster));
> >
> > + if (r_cpucfg_base) {
> > + /* assert cpu power-on reset */
> > + reg = readl(r_cpucfg_base +
> > + R_CPUCFG_CLUSTER_PO_RST_CTRL(cluster));
> > + reg &= ~(R_CPUCFG_CLUSTER_PO_RST_CTRL_CORE(cpu));
> > + writel(reg, r_cpucfg_base +
> > + R_CPUCFG_CLUSTER_PO_RST_CTRL(cluster));
> > + udelay(10);
> > + }
> > +
> > /* Cortex-A7: hold L1 reset disable signal low */
> > if (!sunxi_core_is_cortex_a15(cpu, cluster)) {
> > reg = readl(cpucfg_base + CPUCFG_CX_CTRL_REG0(cluster));
> > @@ -180,17 +199,37 @@ static int sunxi_cpu_powerup(unsigned int cpu, unsigned int cluster)
> > /* open power switch */
> > sunxi_cpu_power_switch_set(cpu, cluster, true);
> >
> > + /* Handle A83T bit swap */
> > + if (of_machine_is_compatible("allwinner,sun8i-a83t")) {
> > + if (cpu == 0)
> > + cpu = 4;
> > + }
> > +
> > /* clear processor power gate */
> > reg = readl(prcm_base + PRCM_PWROFF_GATING_REG(cluster));
> > reg &= ~PRCM_PWROFF_GATING_REG_CORE(cpu);
> > writel(reg, prcm_base + PRCM_PWROFF_GATING_REG(cluster));
> > udelay(20);
> >
> > + if (of_machine_is_compatible("allwinner,sun8i-a83t")) {
> > + if (cpu == 4)
> > + cpu = 0;
> > + }
> > +
> > /* de-assert processor power-on reset */
> > reg = readl(prcm_base + PRCM_CPU_PO_RST_CTRL(cluster));
> > reg |= PRCM_CPU_PO_RST_CTRL_CORE(cpu);
> > writel(reg, prcm_base + PRCM_CPU_PO_RST_CTRL(cluster));
> >
> > + if (r_cpucfg_base) {
> > + reg = readl(r_cpucfg_base +
> > + R_CPUCFG_CLUSTER_PO_RST_CTRL(cluster));
> > + reg |= R_CPUCFG_CLUSTER_PO_RST_CTRL_CORE(cpu);
> > + writel(reg, r_cpucfg_base +
> > + R_CPUCFG_CLUSTER_PO_RST_CTRL(cluster));
> > + udelay(10);
> > + }
> > +
> > /* de-assert all processor resets */
> > reg = readl(cpucfg_base + CPUCFG_CX_RST_CTRL(cluster));
> > reg |= CPUCFG_CX_RST_CTRL_DBG_RST(cpu);
> > @@ -212,6 +251,14 @@ static int sunxi_cluster_powerup(unsigned int cluster)
> > if (cluster >= SUNXI_NR_CLUSTERS)
> > return -EINVAL;
> >
> > + /* For A83T, assert cluster cores resets */
> > + if (of_machine_is_compatible("allwinner,sun8i-a83t")) {
> > + reg = readl(cpucfg_base + CPUCFG_CX_RST_CTRL(cluster));
> > + reg &= ~CPUCFG_CX_RST_CTRL_CORE_RST_ALL; /* Core Reset */
> > + writel(reg, cpucfg_base + CPUCFG_CX_RST_CTRL(cluster));
> > + udelay(10);
> > + }
> > +
> > /* assert ACINACTM */
> > reg = readl(cpucfg_base + CPUCFG_CX_CTRL_REG1(cluster));
> > reg |= CPUCFG_CX_CTRL_REG1_ACINACTM;
> > @@ -222,6 +269,16 @@ static int sunxi_cluster_powerup(unsigned int cluster)
> > reg &= ~PRCM_CPU_PO_RST_CTRL_CORE_ALL;
> > writel(reg, prcm_base + PRCM_CPU_PO_RST_CTRL(cluster));
> >
> > + /* assert cluster cores resets */
> > + if (r_cpucfg_base) {
> > + reg = readl(r_cpucfg_base +
> > + R_CPUCFG_CLUSTER_PO_RST_CTRL(cluster));
> > + reg &= ~CPUCFG_CX_RST_CTRL_CORE_RST_ALL;
> > + writel(reg, r_cpucfg_base +
> > + R_CPUCFG_CLUSTER_PO_RST_CTRL(cluster));
> > + udelay(10);
> > + }
> > +
> > /* assert cluster resets */
> > reg = readl(cpucfg_base + CPUCFG_CX_RST_CTRL(cluster));
> > reg &= ~CPUCFG_CX_RST_CTRL_DBG_SOC_RST;
> > @@ -252,7 +309,10 @@ static int sunxi_cluster_powerup(unsigned int cluster)
> >
> > /* clear cluster power gate */
> > reg = readl(prcm_base + PRCM_PWROFF_GATING_REG(cluster));
> > - reg &= ~PRCM_PWROFF_GATING_REG_CLUSTER;
> > + if (of_machine_is_compatible("allwinner,sun8i-a83t"))
> > + reg &= ~PRCM_PWROFF_GATING_REG_CLUSTER_SUN8I;
> > + else
> > + reg &= ~PRCM_PWROFF_GATING_REG_CLUSTER_SUN9I;
> > writel(reg, prcm_base + PRCM_PWROFF_GATING_REG(cluster));
> > udelay(20);
> >
> > @@ -516,7 +576,10 @@ static int sunxi_cluster_powerdown(unsigned int cluster)
> > /* gate cluster power */
> > pr_debug("%s: gate cluster power\n", __func__);
> > reg = readl(prcm_base + PRCM_PWROFF_GATING_REG(cluster));
> > - reg |= PRCM_PWROFF_GATING_REG_CLUSTER;
> > + if (of_machine_is_compatible("allwinner,sun8i-a83t"))
> > + reg |= PRCM_PWROFF_GATING_REG_CLUSTER_SUN8I;
> > + else
> > + reg |= PRCM_PWROFF_GATING_REG_CLUSTER_SUN9I;
> > writel(reg, prcm_base + PRCM_PWROFF_GATING_REG(cluster));
> > udelay(20);
> >
> > @@ -684,37 +747,25 @@ static int __init sunxi_mc_smp_lookback(void)
> > return ret;
> > }
> >
> > -static int __init sunxi_mc_smp_init(void)
> > +static int sun9i_dt_parsing(struct resource res)
> > {
> > - struct device_node *cpucfg_node, *sram_node, *node;
> > - struct resource res;
> > + struct device_node *prcm_node, *cpucfg_node, *sram_node;
> > int ret;
> >
> > - if (!of_machine_is_compatible("allwinner,sun9i-a80"))
> > - return -ENODEV;
> > -
> > - if (!sunxi_mc_smp_cpu_table_init())
> > - return -EINVAL;
> > -
> > - if (!cci_probed()) {
> > - pr_err("%s: CCI-400 not available\n", __func__);
> > - return -ENODEV;
> > - }
> > -
> > - node = of_find_compatible_node(NULL, NULL, "allwinner,sun9i-a80-prcm");
> > - if (!node) {
> > - pr_err("%s: PRCM not available\n", __func__);
> > + prcm_node = of_find_compatible_node(NULL, NULL,
> > + "allwinner,sun9i-a80-prcm");
> > + if (!prcm_node)
> > return -ENODEV;
> > - }
> >
> > /*
> > * Unfortunately we can not request the I/O region for the PRCM.
> > * It is shared with the PRCM clock.
> > */
> > - prcm_base = of_iomap(node, 0);
> > - of_node_put(node);
> > + prcm_base = of_iomap(prcm_node, 0);
> > + of_node_put(prcm_node);
> > if (!prcm_base) {
> > pr_err("%s: failed to map PRCM registers\n", __func__);
> > + iounmap(prcm_base);
>
> Why are you trying to unmap the pointer that you already failed to map?
Oops, indeed.
>
> > return -ENOMEM;
> > }
> >
> > @@ -748,35 +799,88 @@ static int __init sunxi_mc_smp_init(void)
> > goto err_put_sram_node;
> > }
> >
> > - /* Configure CCI-400 for boot cluster */
> > - ret = sunxi_mc_smp_lookback();
> > - if (ret) {
> > - pr_err("%s: failed to configure boot cluster: %d\n",
> > - __func__, ret);
> > - goto err_unmap_release_secure_sram;
> > - }
> > + r_cpucfg_base = NULL;
>
> This is not needed. Global static variables without initial values are
> always initialized to 0.
Okay, I will remove it.
>
> >
> > /* We don't need the CPUCFG and SRAM device nodes anymore */
> > of_node_put(cpucfg_node);
> > of_node_put(sram_node);
> >
> > - /* Set the hardware entry point address */
> > - writel(__pa_symbol(sunxi_mc_smp_secondary_startup),
> > - prcm_base + PRCM_CPU_SOFT_ENTRY_REG);
> > -
> > - /* Actually enable multi cluster SMP */
> > - smp_set_ops(&sunxi_mc_smp_smp_ops);
> > -
> > - pr_info("sunxi multi cluster SMP support installed\n");
> > -
> > return 0;
> >
> > -err_unmap_release_secure_sram:
> > - iounmap(sram_b_smp_base);
> > - of_address_to_resource(sram_node, 0, &res);
> > +err_unmap_release_cpucfg:
> > + iounmap(cpucfg_base);
> > + of_address_to_resource(cpucfg_node, 0, &res);
> > release_mem_region(res.start, resource_size(&res));
> > err_put_sram_node:
> > of_node_put(sram_node);
> > +err_put_cpucfg_node:
> > + of_node_put(cpucfg_node);
> > +err_unmap_prcm:
> > + iounmap(prcm_base);
> > +
> > + return ret;
> > +}
> > +
> > +static int sun8i_dt_parsing(struct resource res)
> > +{
> > + struct device_node *node, *cpucfg_node;
> > + int ret;
> > +
> > + node = of_find_compatible_node(NULL, NULL,
> > + "allwinner,sun8i-a83t-prcm");
> > + if (!node)
> > + return -ENODEV;
> > +
> > + /*
> > + * Unfortunately we can not request the I/O region for the PRCM.
> > + * It is shared with the PRCM clock.
> > + */
> > + prcm_base = of_iomap(node, 0);
> > + of_node_put(node);
> > + if (!prcm_base) {
> > + pr_err("%s: failed to map PRCM registers\n", __func__);
> > + iounmap(prcm_base);
> > + return -ENOMEM;
> > + }
> > +
> > + cpucfg_node = of_find_compatible_node(NULL, NULL,
> > + "allwinner,sun8i-a83t-cpucfg");
> > + if (!cpucfg_node) {
> > + ret = -ENODEV;
> > + pr_err("%s: CPUCFG not available\n", __func__);
> > + goto err_unmap_prcm;
> > + }
> > +
> > + cpucfg_base = of_io_request_and_map(cpucfg_node, 0, "sunxi-mc-smp");
> > + if (IS_ERR(cpucfg_base)) {
> > + ret = PTR_ERR(cpucfg_base);
> > + pr_err("%s: failed to map CPUCFG registers: %d\n",
> > + __func__, ret);
> > + goto err_put_cpucfg_node;
> > + }
> > +
> > + node = of_find_compatible_node(NULL, NULL,
> > + "allwinner,sun8i-a83t-r-cpucfg");
> > + if (!node) {
> > + ret = -ENODEV;
> > + goto err_unmap_release_cpucfg;
> > + }
> > +
> > + r_cpucfg_base = of_iomap(node, 0);
> > + if (!r_cpucfg_base) {
> > + pr_err("%s: failed to map R-CPUCFG registers\n",
> > + __func__);
> > + ret = -ENOMEM;
> > + goto err_put_node;
> > + }
> > +
> > + /* We don't need the CPUCFG device node anymore */
> > + of_node_put(cpucfg_node);
> > + of_node_put(node);
> > +
> > + return 0;
> > +err_put_node:
> > + of_node_put(node);
> > err_unmap_release_cpucfg:
> > iounmap(cpucfg_base);
> > of_address_to_resource(cpucfg_node, 0, &res);
> > @@ -785,6 +889,57 @@ static int __init sunxi_mc_smp_init(void)
> > of_node_put(cpucfg_node);
> > err_unmap_prcm:
> > iounmap(prcm_base);
> > +
> > + return ret;
> > +}
> > +
> > +static int __init sunxi_mc_smp_init(void)
> > +{
> > + struct resource res;
> > + int ret;
> > +
> > + if (!of_machine_is_compatible("allwinner,sun9i-a80") &&
> > + !of_machine_is_compatible("allwinner,sun8i-a83t"))
> > + return -ENODEV;
> > +
> > + if (!sunxi_mc_smp_cpu_table_init())
> > + return -EINVAL;
> > +
> > + if (!cci_probed()) {
> > + pr_err("%s: CCI-400 not available\n", __func__);
> > + return -ENODEV;
> > + }
> > +
> > + if (of_machine_is_compatible("allwinner,sun9i-a80"))
> > + ret = sun9i_dt_parsing(res);
> > + else
> > + ret = sun8i_dt_parsing(res);
> > + if (ret) {
> > + pr_err("%s: failed to parse DT: %d\n", __func__, ret);
> > + return ret;
> > + }
> > +
> > + /* Configure CCI-400 for boot cluster */
> > + ret = sunxi_mc_smp_lookback();
> > + if (ret) {
> > + pr_err("%s: failed to configure boot cluster: %d\n",
> > + __func__, ret);
> > + return ret; /* MYMY: Need to handle this error case */
>
> Please add functions to release the resources. They will need to be
> platform specific, since it requires you to find the device node to
> get the resource that you want to release (for CPUCFG / R-CPUCFG).
> For sun9i do it in the refactor patch. For A83T do it in the patch
> you add support.
It seems that I forgot to handle this error case (the comment was
a kind of "TODO" for myself). Thank you for pointing it out to me.
Thanks for the review!
Best regards,
Mylène
>
> > + }
> > +
> > + /* Set the hardware entry point address */
> > + if (of_machine_is_compatible("allwinner,sun9i-a80"))
> > + writel(__pa_symbol(sunxi_mc_smp_secondary_startup),
> > + prcm_base + PRCM_CPU_SOFT_ENTRY_REG);
> > + else
> > + writel(__pa_symbol(sunxi_mc_smp_secondary_startup),
> > + r_cpucfg_base + R_CPUCFG_CPU_SOFT_ENTRY_REG);
> > +
> > + /* Actually enable multi cluster SMP */
> > + smp_set_ops(&sunxi_mc_smp_smp_ops);
> > +
> > + pr_info("sunxi multi cluster SMP support installed\n");
> > +
> > return ret;
> > }
> >
> > --
> > 2.11.0
> >
--
Mylène Josserand, Bootlin (formerly Free Electrons)
Embedded Linux and Kernel engineering
http://bootlin.com
Hi Marc,
Thank you for the review!
On Tue, 20 Feb 2018 18:07:54 +0000
Marc Zyngier <[email protected]> wrote:
> Hi Mylène,
>
> On 19/02/18 08:18, Mylène Josserand wrote:
> > The ARM architected timers use an offset between their physical and
> > virtual counters. That offset should be configured by the bootloader
> > in CNTVOFF.
> >
> > However, the A83t bootloader fails to do so, and we end up with an
> > undefined offset (which in our case is random), meaning that each CPU
> > will have a different time, which isn't working very well.
> >
> > Fix that by setting the arm,cpu-registers-not-fw-configured that will
> > make Linux use the physical timers instead of the virtual ones. One
> > possible side effect would be that the virtualization features would
> > be disabled. However, due to the way the GIC has been integrated in
> > the system, it is already unusable so we're effectively not losing any
> > feature.
> >
> > Signed-off-by: Mylène Josserand <[email protected]>
> > ---
> > arch/arm/boot/dts/sun8i-a83t.dtsi | 1 +
> > 1 file changed, 1 insertion(+)
> >
> > diff --git a/arch/arm/boot/dts/sun8i-a83t.dtsi b/arch/arm/boot/dts/sun8i-a83t.dtsi
> > index e97a6d17b8d0..b9bdb891cf2f 100644
> > --- a/arch/arm/boot/dts/sun8i-a83t.dtsi
> > +++ b/arch/arm/boot/dts/sun8i-a83t.dtsi
> > @@ -123,6 +123,7 @@
> > <GIC_PPI 14 (GIC_CPU_MASK_SIMPLE(8) | IRQ_TYPE_LEVEL_LOW)>,
> > <GIC_PPI 11 (GIC_CPU_MASK_SIMPLE(8) | IRQ_TYPE_LEVEL_LOW)>,
> > <GIC_PPI 10 (GIC_CPU_MASK_SIMPLE(8) | IRQ_TYPE_LEVEL_LOW)>;
> > + arm,cpu-registers-not-fw-configured;
> > };
> >
> > clocks {
> >
>
> Is the firmware dropping you in the kernel in secure or non-secure mode?
For sun8i-a83t, the kernel is in secure mode.
>
> If the later, what you have is the only solution. If the former, that
> I'd suggest you adopt what we already have for the Renesas stuff (see
> arch/arm/mach-shmobile/headsmp-apmu.S and commit 3fd45a136ff6).
>
> It would allow you to use the virtual timer as intended.
Okay, thanks for the commit pointed, I will try to adopt what it is done
for Renesas.
Best regards,
--
Mylène Josserand, Bootlin (formerly Free Electrons)
Embedded Linux and Kernel engineering
http://bootlin.com