[PATCH v3 0/7] Add SMP support on sun8i-a83t

[PATCH v3 0/7] Add SMP support on sun8i-a83t

[Mylène Josserand]

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


_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

[PATCH v3 1/7] ARM: sun8i: smp: Add support for A83T

[Mylène Josserand]

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 <mylene.josserand@bootlin.com>
---
 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


_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

Re: [PATCH v3 1/7] ARM: sun8i: smp: Add support for A83T

[Maxime Ripard]

[-- Attachment #1.1: Type: text/plain, Size: 1140 bytes --]

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 <mylene.josserand@bootlin.com>

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

[-- Attachment #1.2: signature.asc --]
[-- Type: application/pgp-signature, Size: 833 bytes --]

[-- Attachment #2: Type: text/plain, Size: 176 bytes --]

_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

Re: [PATCH v3 1/7] ARM: sun8i: smp: Add support for A83T

[Mylène Josserand]

Hello Maxime,

On Mon, 19 Feb 2018 10:04:01 +0100
Maxime Ripard <maxime.ripard@bootlin.com> 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 <mylene.josserand@bootlin.com>  
> 
> 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

_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

Re: [PATCH v3 1/7] ARM: sun8i: smp: Add support for A83T

[Chen-Yu Tsai]

On Mon, Feb 19, 2018 at 4:18 PM, Mylène Josserand
<mylene.josserand@bootlin.com> 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 <mylene.josserand@bootlin.com>
> ---
>  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
>

_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

Re: [PATCH v3 1/7] ARM: sun8i: smp: Add support for A83T

[Mylène Josserand]

Hello Chen-Yu,

On Tue, 20 Feb 2018 11:32:03 +0800
Chen-Yu Tsai <wens@csie.org> wrote:

> On Mon, Feb 19, 2018 at 4:18 PM, Mylène Josserand
> <mylene.josserand@bootlin.com> 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 <mylene.josserand@bootlin.com>
> > ---
> >  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

_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

[PATCH v3 2/7] ARM: dts: sun8i: Add CPUCFG device node for A83T dtsi

[Mylène Josserand]

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 <mylene.josserand@bootlin.com>
---
 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


_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

[PATCH v3 3/7] ARM: dts: sun8i: Add PRCM device node for the A83T dtsi

[Mylène Josserand]

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 <mylene.josserand@bootlin.com>
---
 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


_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

Re: [PATCH v3 3/7] ARM: dts: sun8i: Add PRCM device node for the A83T dtsi

[Maxime Ripard]

[-- Attachment #1.1: Type: text/plain, Size: 1135 bytes --]

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 <mylene.josserand@bootlin.com>
> ---
>  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

[-- Attachment #1.2: signature.asc --]
[-- Type: application/pgp-signature, Size: 833 bytes --]

[-- Attachment #2: Type: text/plain, Size: 176 bytes --]

_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

Re: [PATCH v3 3/7] ARM: dts: sun8i: Add PRCM device node for the A83T dtsi

[Mylène Josserand]

Hi,

On Mon, 19 Feb 2018 09:48:22 +0100
Maxime Ripard <maxime.ripard@bootlin.com> 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 <mylene.josserand@bootlin.com>
> > ---
> >  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

_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

[PATCH v3 4/7] ARM: dts: sun8i: Add R_CPUCFG device node for the A83T dtsi

[Mylène Josserand]

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 <mylene.josserand@bootlin.com>
---
 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


_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

[PATCH v3 5/7] arm: dts: sun8i: a83t: Add CCI-400 node

[Mylène Josserand]

Add CCI-400 node and control-port on CPUs needed by SMP bringup.

Signed-off-by: Mylène Josserand <mylene.josserand@bootlin.com>
---
 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


_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

Re: [PATCH v3 5/7] arm: dts: sun8i: a83t: Add CCI-400 node

[Maxime Ripard]

[-- Attachment #1.1: Type: text/plain, Size: 630 bytes --]

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

[-- Attachment #1.2: signature.asc --]
[-- Type: application/pgp-signature, Size: 833 bytes --]

[-- Attachment #2: Type: text/plain, Size: 176 bytes --]

_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

Re: [PATCH v3 5/7] arm: dts: sun8i: a83t: Add CCI-400 node

[Mylène Josserand]

Hi,

On Mon, 19 Feb 2018 09:49:44 +0100
Maxime Ripard <maxime.ripard@bootlin.com> 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

_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

[PATCH v3 6/7] arm: dts: sun8i: a83t: Fix undefined offset with virtual timer

[Mylène Josserand]

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 <mylene.josserand@bootlin.com>
---
 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


_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

Re: [PATCH v3 6/7] arm: dts: sun8i: a83t: Fix undefined offset with virtual timer

[Maxime Ripard]

[-- Attachment #1.1: Type: text/plain, Size: 1128 bytes --]

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 <mylene.josserand@bootlin.com>

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

[-- Attachment #1.2: signature.asc --]
[-- Type: application/pgp-signature, Size: 833 bytes --]

[-- Attachment #2: Type: text/plain, Size: 176 bytes --]

_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

Re: [PATCH v3 6/7] arm: dts: sun8i: a83t: Fix undefined offset with virtual timer

[Mylène Josserand]

Hi,

On Mon, 19 Feb 2018 09:52:05 +0100
Maxime Ripard <maxime.ripard@bootlin.com> 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 <mylene.josserand@bootlin.com>  
> 
> 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

_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

Re: [PATCH v3 6/7] arm: dts: sun8i: a83t: Fix undefined offset with virtual timer

[Marc Zyngier]

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 <mylene.josserand@bootlin.com>
> ---
>  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...

_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

Re: [PATCH v3 6/7] arm: dts: sun8i: a83t: Fix undefined offset with virtual timer

[Mylène Josserand]

Hi Marc,

Thank you for the review!

On Tue, 20 Feb 2018 18:07:54 +0000
Marc Zyngier <marc.zyngier@arm.com> 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 <mylene.josserand@bootlin.com>
> > ---
> >  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

_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

[PATCH v3 7/7] ARM: sun8i: smp: Remove the disabling of CPU0

[Mylène Josserand]

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 <mylene.josserand@bootlin.com>
---
 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


_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

Re: [PATCH v3 7/7] ARM: sun8i: smp: Remove the disabling of CPU0

[Maxime Ripard]

[-- Attachment #1.1: Type: text/plain, Size: 1222 bytes --]

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 <mylene.josserand@bootlin.com>
> ---
>  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

[-- Attachment #1.2: signature.asc --]
[-- Type: application/pgp-signature, Size: 833 bytes --]

[-- Attachment #2: Type: text/plain, Size: 176 bytes --]

_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

Re: [PATCH v3 7/7] ARM: sun8i: smp: Remove the disabling of CPU0

[Mylène Josserand]

Hi,

On Mon, 19 Feb 2018 09:54:08 +0100
Maxime Ripard <maxime.ripard@bootlin.com> 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 <mylene.josserand@bootlin.com>
> > ---
> >  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

_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel