2018-04-03 06:22:31

by Mylène Josserand

[permalink] [raw]
Subject: [PATCH v5 00/13] Sunxi: Add SMP support on A83T

Hello everyone,

This is a V5 of my series that adds SMP support for Allwinner sun8i-a83t.
Based on sunxi's tree, sunxi/for-next branch.

For boot CPU, I tried to add the CNTVOFF initialization in mc_smp.c file
(in sunxi_mc_smp_init function) but it is not working.
Instead of adding it in timer's function of sunxi.c file, I create
a new machine to handle this case. Thanks to that, it is specific to
sun8i-a83t SoC. Let me know what you think.

Changes since v4:
- Rebased my series according to new Chen-Yu series:
"ARM: sunxi: Clean and improvements for multi-cluster SMP"
https://lkml.org/lkml/2018/3/8/886
- Updated my series according to Marc Zyngier's reviews to add CNTVOFF
initialization's function into ARM's common part. Thanks to that, other
platforms such as Renesa can use this function.
- For boot CPU, create a new machine to handle the CNTVOFF initialization
using "init_early" callback.
Changes since v3:
- Take into account Maxime's reviews:
- split the first patch into 4 new patches: add sun9i device tree
parsing, rename some variables, add a83t support and finally,
add hotplug support.
- Move the code of previous patch 07 (to disable CPU0 disabling)
into hotplug support patch (see patch 04)
- Remove the patch that added PRCM register because it is already
available. Because of that, update the device tree parsing to use
"sun8i-a83t-r-ccu".
- Use a variable to know which SoC we currently have
- Take into account Chen-Yu's reviews: create two iounmap functions
to release the resources of the device tree parsing.
- Take into account Marc's review: Update the code to initialize CNTVOFF
register. As there is already assembly code in the driver, I decided
to create an assembly file not to mix assembly and C code.
For that, I create 3 new patches: move the current assembly code that
handles the cluster cache enabling into a file, move the cpu_resume entry
in this file and finally, add a new assembly entry to initialize the timer
offset for boot CPU and secondary CPUs.

Changes since v2:
- Rebased my modifications according to new Chen Yu'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 for more visibility.
- 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: To be able to use macro definition (such as ARM_CPU_PART_MASK)
in an assembly file, we need to separate macro difinitions and C functions
definitions. This is what this patch is doing.
Patch 02: Move assembly code into a new assembly file
Patch 03: Move another assembly code (resuming function) into above file
Patch 04-06: Add registers nodes (cpucfg, r_cpucfg and cci-400) needed
for SMP bringup.
Patch 07: Create a new assembly file to initialize the CNTVOFF register. Can
be used on other platform so add it in "common" ARM folder.
Patch 08: Create a new machine to initialize the timer offset for boot CPU.
Add a new entry to initialize the timer offset for secondary CPUs.
Patch 09: Prepare to handle sun8i-a83t by renaming some sun9i-a80 functions.
Patch 10: Again a preparation to support sun8i-a83t by moving some structures.
Patch 11: Add a new field to know if we are on sun9i-a80 or sun8i-a83t.
Patch 12: Convert the sunxi SMP driver to add support for A83T.
This SoC has a bit flip that needs to be handled.
Patch 13: Enable the smp support on sun8i-a83t by adding enable-method.

If you have any remarks/questions, let me know.
Thank you in advance,
Mylène

Mylène Josserand (13):
ARM: move cputype definitions into another file
ARM: sunxi: smp: Move assembly code into a file
ARM: sunxi: smp: Move cpu_resume assembly entry into file
ARM: dts: sun8i: Add CPUCFG device node for A83T dtsi
ARM: dts: sun8i: Add R_CPUCFG device node for the A83T dtsi
ARM: dts: sun8i: a83t: Add CCI-400 node
ARM: smp: Add initialization of CNTVOFF
ARM: sunxi: Add initialization of CNTVOFF
ARM: sun9i: smp: Rename clusters's power-off
ARM: sun9i: smp: Move structures
ARM: sun9i: smp: Add is_sun9i field
ARM: sun8i: smp: Add support for A83T
ARM: dts: sun8i: Add enable-method for SMP support for the A83T SoC

arch/arm/boot/dts/sun8i-a83t.dtsi | 59 ++++++++
arch/arm/common/Makefile | 1 +
arch/arm/common/smp_cntvoff.S | 35 +++++
arch/arm/include/asm/cputype.h | 94 +-----------
arch/arm/include/asm/cputype_def.h | 98 +++++++++++++
arch/arm/include/asm/smp_cntvoff.h | 8 ++
arch/arm/mach-sunxi/Makefile | 4 +-
arch/arm/mach-sunxi/headsmp.S | 81 +++++++++++
arch/arm/mach-sunxi/mc_smp.c | 287 ++++++++++++++++++++++---------------
arch/arm/mach-sunxi/sunxi.c | 18 ++-
10 files changed, 472 insertions(+), 213 deletions(-)
create mode 100644 arch/arm/common/smp_cntvoff.S
create mode 100644 arch/arm/include/asm/cputype_def.h
create mode 100644 arch/arm/include/asm/smp_cntvoff.h
create mode 100644 arch/arm/mach-sunxi/headsmp.S

--
2.11.0



2018-04-03 06:22:31

by Mylène Josserand

[permalink] [raw]
Subject: [PATCH v5 12/13] ARM: sun8i: smp: Add support for A83T

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.
There is also a bit swap between sun8i-a83t and sun9i-a80 that must be
handled.

Signed-off-by: Mylène Josserand <[email protected]>
---
arch/arm/mach-sunxi/mc_smp.c | 120 +++++++++++++++++++++++++++++++++++++++++--
1 file changed, 117 insertions(+), 3 deletions(-)

diff --git a/arch/arm/mach-sunxi/mc_smp.c b/arch/arm/mach-sunxi/mc_smp.c
index 468a6c46bfc9..72e497dc43ac 100644
--- a/arch/arm/mach-sunxi/mc_smp.c
+++ b/arch/arm/mach-sunxi/mc_smp.c
@@ -55,22 +55,31 @@
#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))
+/* The power off register for clusters are different from a80 and a83t */
+#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 *prcm_base;
static void __iomem *sram_b_smp_base;
+static void __iomem *r_cpucfg_base;
static int index;

/*
@@ -81,6 +90,7 @@ struct sunxi_mc_smp_nodes {
struct device_node *prcm_node;
struct device_node *cpucfg_node;
struct device_node *sram_node;
+ struct device_node *r_cpucfg_node;
};

/* This structure holds SoC-specific bits tied to an enable-method string. */
@@ -94,6 +104,7 @@ extern void sunxi_mc_smp_secondary_startup(void);
extern void sunxi_mc_smp_resume(void);

static int __init sun9i_a80_get_smp_nodes(struct sunxi_mc_smp_nodes *nodes);
+static int __init sun8i_a83t_get_smp_nodes(struct sunxi_mc_smp_nodes *nodes);

static const struct sunxi_mc_smp_data sunxi_mc_smp_data[] __initconst = {
{
@@ -101,6 +112,11 @@ static const struct sunxi_mc_smp_data sunxi_mc_smp_data[] __initconst = {
.get_smp_nodes = sun9i_a80_get_smp_nodes,
.is_sun9i = true,
},
+ {
+ .enable_method = "allwinner,sun8i-a83t-smp",
+ .get_smp_nodes = sun8i_a83t_get_smp_nodes,
+ .is_sun9i = false,
+ },
};

static bool sunxi_core_is_cortex_a15(unsigned int core, unsigned int cluster)
@@ -188,6 +204,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));
@@ -211,17 +237,38 @@ 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 (!sunxi_mc_smp_data[index].is_sun9i) {
+ 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);

+ /* Handle A83T bit swap */
+ if (!sunxi_mc_smp_data[index].is_sun9i) {
+ 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);
@@ -243,6 +290,14 @@ static int sunxi_cluster_powerup(unsigned int cluster)
if (cluster >= SUNXI_NR_CLUSTERS)
return -EINVAL;

+ /* For A83T, assert cluster cores resets */
+ if (!sunxi_mc_smp_data[index].is_sun9i) {
+ 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;
@@ -253,6 +308,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;
@@ -285,6 +350,8 @@ static int sunxi_cluster_powerup(unsigned int cluster)
reg = readl(prcm_base + PRCM_PWROFF_GATING_REG(cluster));
if (sunxi_mc_smp_data[index].is_sun9i)
reg &= ~PRCM_PWROFF_GATING_REG_CLUSTER_SUN9I;
+ else
+ reg &= ~PRCM_PWROFF_GATING_REG_CLUSTER_SUN8I;
writel(reg, prcm_base + PRCM_PWROFF_GATING_REG(cluster));
udelay(20);

@@ -483,6 +550,8 @@ static int sunxi_cluster_powerdown(unsigned int cluster)
reg = readl(prcm_base + PRCM_PWROFF_GATING_REG(cluster));
if (sunxi_mc_smp_data[index].is_sun9i)
reg |= PRCM_PWROFF_GATING_REG_CLUSTER_SUN9I;
+ else
+ reg |= PRCM_PWROFF_GATING_REG_CLUSTER_SUN8I;
writel(reg, prcm_base + PRCM_PWROFF_GATING_REG(cluster));
udelay(20);

@@ -564,8 +633,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-a80 */
+ if (!sunxi_mc_smp_data[index].is_sun9i)
+ if (cpu == 0)
+ return false;
return true;
}
#endif
@@ -645,6 +718,7 @@ static void __init sunxi_mc_smp_put_nodes(struct sunxi_mc_smp_nodes *nodes)
of_node_put(nodes->prcm_node);
of_node_put(nodes->cpucfg_node);
of_node_put(nodes->sram_node);
+ of_node_put(nodes->r_cpucfg_node);
memset(nodes, 0, sizeof(*nodes));
}

@@ -674,6 +748,32 @@ static int __init sun9i_a80_get_smp_nodes(struct sunxi_mc_smp_nodes *nodes)
return 0;
}

+static int __init sun8i_a83t_get_smp_nodes(struct sunxi_mc_smp_nodes *nodes)
+{
+ nodes->prcm_node = of_find_compatible_node(NULL, NULL,
+ "allwinner,sun8i-a83t-r-ccu");
+ if (!nodes->prcm_node) {
+ pr_err("%s: PRCM not available\n", __func__);
+ return -ENODEV;
+ }
+
+ nodes->cpucfg_node = of_find_compatible_node(NULL, NULL,
+ "allwinner,sun8i-a83t-cpucfg");
+ if (!nodes->cpucfg_node) {
+ pr_err("%s: CPUCFG not available\n", __func__);
+ return -ENODEV;
+ }
+
+ nodes->r_cpucfg_node = of_find_compatible_node(NULL, NULL,
+ "allwinner,sun8i-a83t-r-cpucfg");
+ if (!nodes->r_cpucfg_node) {
+ pr_err("%s: RCPUCFG not available\n", __func__);
+ return -ENODEV;
+ }
+
+ return 0;
+}
+
static int __init sunxi_mc_smp_init(void)
{
struct sunxi_mc_smp_nodes nodes = { 0 };
@@ -752,6 +852,15 @@ static int __init sunxi_mc_smp_init(void)
pr_err("%s: failed to map secure SRAM\n", __func__);
goto err_unmap_release_cpucfg;
}
+ } else {
+ r_cpucfg_base = of_io_request_and_map(nodes.r_cpucfg_node,
+ 0, "sunxi-mc-smp");
+ if (IS_ERR(r_cpucfg_base)) {
+ ret = PTR_ERR(r_cpucfg_base);
+ pr_err("%s: failed to map R-CPUCFG registers\n",
+ __func__);
+ goto err_unmap_release_cpucfg;
+ }
}

/* Configure CCI-400 for boot cluster */
@@ -759,7 +868,7 @@ static int __init sunxi_mc_smp_init(void)
if (ret) {
pr_err("%s: failed to configure boot cluster: %d\n",
__func__, ret);
- goto err_unmap_release_secure_sram;
+ goto err_unmap_release_sram_rcpucfg;
}

/* We don't need the device nodes anymore */
@@ -768,6 +877,8 @@ static int __init sunxi_mc_smp_init(void)
/* Set the hardware entry point address */
if (sunxi_mc_smp_data[index].is_sun9i)
addr = prcm_base + PRCM_CPU_SOFT_ENTRY_REG;
+ else
+ addr = r_cpucfg_base + R_CPUCFG_CPU_SOFT_ENTRY_REG;
writel(__pa_symbol(sunxi_mc_smp_secondary_startup), addr);

/* Actually enable multi cluster SMP */
@@ -777,10 +888,13 @@ static int __init sunxi_mc_smp_init(void)

return 0;

-err_unmap_release_secure_sram:
+err_unmap_release_sram_rcpucfg:
if (sunxi_mc_smp_data[index].is_sun9i) {
iounmap(sram_b_smp_base);
of_address_to_resource(nodes.sram_node, 0, &res);
+ } else {
+ iounmap(r_cpucfg_base);
+ of_address_to_resource(nodes.r_cpucfg_node, 0, &res);
}
release_mem_region(res.start, resource_size(&res));
err_unmap_release_cpucfg:
--
2.11.0


2018-04-03 06:22:48

by Mylène Josserand

[permalink] [raw]
Subject: [PATCH v5 13/13] ARM: dts: sun8i: Add enable-method for SMP support for the A83T SoC

Add the use of enable-method property for SMP support which allows
to handle the SMP support for this specific SoC.

This commit adds enable-method properties to all CPU nodes.

Signed-off-by: Mylène Josserand <[email protected]>
---
arch/arm/boot/dts/sun8i-a83t.dtsi | 8 ++++++++
1 file changed, 8 insertions(+)

diff --git a/arch/arm/boot/dts/sun8i-a83t.dtsi b/arch/arm/boot/dts/sun8i-a83t.dtsi
index 9ac905884d81..208d2823fdef 100644
--- a/arch/arm/boot/dts/sun8i-a83t.dtsi
+++ b/arch/arm/boot/dts/sun8i-a83t.dtsi
@@ -67,6 +67,7 @@
device_type = "cpu";
operating-points-v2 = <&cpu0_opp_table>;
cci-control-port = <&cci_control0>;
+ enable-method = "allwinner,sun8i-a83t-smp";
reg = <0>;
};

@@ -75,6 +76,7 @@
device_type = "cpu";
operating-points-v2 = <&cpu0_opp_table>;
cci-control-port = <&cci_control0>;
+ enable-method = "allwinner,sun8i-a83t-smp";
reg = <1>;
};

@@ -83,6 +85,7 @@
device_type = "cpu";
operating-points-v2 = <&cpu0_opp_table>;
cci-control-port = <&cci_control0>;
+ enable-method = "allwinner,sun8i-a83t-smp";
reg = <2>;
};

@@ -91,6 +94,7 @@
device_type = "cpu";
operating-points-v2 = <&cpu0_opp_table>;
cci-control-port = <&cci_control0>;
+ enable-method = "allwinner,sun8i-a83t-smp";
reg = <3>;
};

@@ -101,6 +105,7 @@
device_type = "cpu";
operating-points-v2 = <&cpu1_opp_table>;
cci-control-port = <&cci_control1>;
+ enable-method = "allwinner,sun8i-a83t-smp";
reg = <0x100>;
};

@@ -109,6 +114,7 @@
device_type = "cpu";
operating-points-v2 = <&cpu1_opp_table>;
cci-control-port = <&cci_control1>;
+ enable-method = "allwinner,sun8i-a83t-smp";
reg = <0x101>;
};

@@ -117,6 +123,7 @@
device_type = "cpu";
operating-points-v2 = <&cpu1_opp_table>;
cci-control-port = <&cci_control1>;
+ enable-method = "allwinner,sun8i-a83t-smp";
reg = <0x102>;
};

@@ -125,6 +132,7 @@
device_type = "cpu";
operating-points-v2 = <&cpu1_opp_table>;
cci-control-port = <&cci_control1>;
+ enable-method = "allwinner,sun8i-a83t-smp";
reg = <0x103>;
};
};
--
2.11.0


2018-04-03 06:23:07

by Mylène Josserand

[permalink] [raw]
Subject: [PATCH v5 09/13] ARM: sun9i: smp: Rename clusters's power-off

To prepare the support for sun8i-a83t, rename the variable name
that handles the power-off of clusters because it is different from
sun9i-a80 to sun8i-a83t.

The power off register for clusters are different from a80 and a83t.

Signed-off-by: Mylène Josserand <[email protected]>
Acked-by: Maxime Ripard <[email protected]>
---
arch/arm/mach-sunxi/mc_smp.c | 6 +++---
1 file changed, 3 insertions(+), 3 deletions(-)

diff --git a/arch/arm/mach-sunxi/mc_smp.c b/arch/arm/mach-sunxi/mc_smp.c
index 727968d6a3e5..03f021d0c73e 100644
--- a/arch/arm/mach-sunxi/mc_smp.c
+++ b/arch/arm/mach-sunxi/mc_smp.c
@@ -60,7 +60,7 @@
#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)
+#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
@@ -255,7 +255,7 @@ 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;
+ reg &= ~PRCM_PWROFF_GATING_REG_CLUSTER_SUN9I;
writel(reg, prcm_base + PRCM_PWROFF_GATING_REG(cluster));
udelay(20);

@@ -452,7 +452,7 @@ 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;
+ reg |= PRCM_PWROFF_GATING_REG_CLUSTER_SUN9I;
writel(reg, prcm_base + PRCM_PWROFF_GATING_REG(cluster));
udelay(20);

--
2.11.0


2018-04-03 06:23:16

by Mylène Josserand

[permalink] [raw]
Subject: [PATCH v5 11/13] ARM: sun9i: smp: Add is_sun9i field

To prepare the support of sun8i-a83t, add a field in the smp_data
structure to enable the case of sun9i.

Start to handle the differences between sun9i-a80 and sun8i-a83t
by using this variable.
Add an index to retrieve which structures we are using.

Signed-off-by: Mylène Josserand <[email protected]>
---
arch/arm/mach-sunxi/mc_smp.c | 37 +++++++++++++++++++++++++------------
1 file changed, 25 insertions(+), 12 deletions(-)

diff --git a/arch/arm/mach-sunxi/mc_smp.c b/arch/arm/mach-sunxi/mc_smp.c
index 0a7252df207f..468a6c46bfc9 100644
--- a/arch/arm/mach-sunxi/mc_smp.c
+++ b/arch/arm/mach-sunxi/mc_smp.c
@@ -71,6 +71,7 @@
static void __iomem *cpucfg_base;
static void __iomem *prcm_base;
static void __iomem *sram_b_smp_base;
+static int index;

/*
* This holds any device nodes that we requested resources for,
@@ -86,6 +87,7 @@ struct sunxi_mc_smp_nodes {
struct sunxi_mc_smp_data {
const char *enable_method;
int (*get_smp_nodes)(struct sunxi_mc_smp_nodes *nodes);
+ int is_sun9i;
};

extern void sunxi_mc_smp_secondary_startup(void);
@@ -97,6 +99,7 @@ static const struct sunxi_mc_smp_data sunxi_mc_smp_data[] __initconst = {
{
.enable_method = "allwinner,sun9i-a80-smp",
.get_smp_nodes = sun9i_a80_get_smp_nodes,
+ .is_sun9i = true,
},
};

@@ -280,7 +283,8 @@ 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_SUN9I;
+ if (sunxi_mc_smp_data[index].is_sun9i)
+ reg &= ~PRCM_PWROFF_GATING_REG_CLUSTER_SUN9I;
writel(reg, prcm_base + PRCM_PWROFF_GATING_REG(cluster));
udelay(20);

@@ -477,7 +481,8 @@ 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_SUN9I;
+ if (sunxi_mc_smp_data[index].is_sun9i)
+ reg |= PRCM_PWROFF_GATING_REG_CLUSTER_SUN9I;
writel(reg, prcm_base + PRCM_PWROFF_GATING_REG(cluster));
udelay(20);

@@ -675,6 +680,7 @@ static int __init sunxi_mc_smp_init(void)
struct device_node *node;
struct resource res;
int i, ret;
+ void __iomem *addr;

/*
* Don't bother checking the "cpus" node, as an enable-method
@@ -699,6 +705,8 @@ static int __init sunxi_mc_smp_init(void)
break;
}

+ index = i;
+
of_node_put(node);
if (ret)
return -ENODEV;
@@ -736,12 +744,14 @@ static int __init sunxi_mc_smp_init(void)
goto err_unmap_prcm;
}

- sram_b_smp_base = of_io_request_and_map(nodes.sram_node, 0,
- "sunxi-mc-smp");
- if (IS_ERR(sram_b_smp_base)) {
- ret = PTR_ERR(sram_b_smp_base);
- pr_err("%s: failed to map secure SRAM\n", __func__);
- goto err_unmap_release_cpucfg;
+ if (sunxi_mc_smp_data[index].is_sun9i) {
+ sram_b_smp_base = of_io_request_and_map(nodes.sram_node, 0,
+ "sunxi-mc-smp");
+ if (IS_ERR(sram_b_smp_base)) {
+ ret = PTR_ERR(sram_b_smp_base);
+ pr_err("%s: failed to map secure SRAM\n", __func__);
+ goto err_unmap_release_cpucfg;
+ }
}

/* Configure CCI-400 for boot cluster */
@@ -756,8 +766,9 @@ static int __init sunxi_mc_smp_init(void)
sunxi_mc_smp_put_nodes(&nodes);

/* Set the hardware entry point address */
- writel(__pa_symbol(sunxi_mc_smp_secondary_startup),
- prcm_base + PRCM_CPU_SOFT_ENTRY_REG);
+ if (sunxi_mc_smp_data[index].is_sun9i)
+ addr = prcm_base + PRCM_CPU_SOFT_ENTRY_REG;
+ writel(__pa_symbol(sunxi_mc_smp_secondary_startup), addr);

/* Actually enable multi cluster SMP */
smp_set_ops(&sunxi_mc_smp_smp_ops);
@@ -767,8 +778,10 @@ static int __init sunxi_mc_smp_init(void)
return 0;

err_unmap_release_secure_sram:
- iounmap(sram_b_smp_base);
- of_address_to_resource(nodes.sram_node, 0, &res);
+ if (sunxi_mc_smp_data[index].is_sun9i) {
+ iounmap(sram_b_smp_base);
+ of_address_to_resource(nodes.sram_node, 0, &res);
+ }
release_mem_region(res.start, resource_size(&res));
err_unmap_release_cpucfg:
iounmap(cpucfg_base);
--
2.11.0


2018-04-03 06:23:43

by Mylène Josserand

[permalink] [raw]
Subject: [PATCH v5 10/13] ARM: sun9i: smp: Move structures

To prepare the support for sun8i-a83t, move some structures
at the beginning of the file.

Signed-off-by: Mylène Josserand <[email protected]>
---
arch/arm/mach-sunxi/mc_smp.c | 48 +++++++++++++++++++++++---------------------
1 file changed, 25 insertions(+), 23 deletions(-)

diff --git a/arch/arm/mach-sunxi/mc_smp.c b/arch/arm/mach-sunxi/mc_smp.c
index 03f021d0c73e..0a7252df207f 100644
--- a/arch/arm/mach-sunxi/mc_smp.c
+++ b/arch/arm/mach-sunxi/mc_smp.c
@@ -72,9 +72,34 @@ static void __iomem *cpucfg_base;
static void __iomem *prcm_base;
static void __iomem *sram_b_smp_base;

+/*
+ * This holds any device nodes that we requested resources for,
+ * so that we may easily release resources in the error path.
+ */
+struct sunxi_mc_smp_nodes {
+ struct device_node *prcm_node;
+ struct device_node *cpucfg_node;
+ struct device_node *sram_node;
+};
+
+/* This structure holds SoC-specific bits tied to an enable-method string. */
+struct sunxi_mc_smp_data {
+ const char *enable_method;
+ int (*get_smp_nodes)(struct sunxi_mc_smp_nodes *nodes);
+};
+
extern void sunxi_mc_smp_secondary_startup(void);
extern void sunxi_mc_smp_resume(void);

+static int __init sun9i_a80_get_smp_nodes(struct sunxi_mc_smp_nodes *nodes);
+
+static const struct sunxi_mc_smp_data sunxi_mc_smp_data[] __initconst = {
+ {
+ .enable_method = "allwinner,sun9i-a80-smp",
+ .get_smp_nodes = sun9i_a80_get_smp_nodes,
+ },
+};
+
static bool sunxi_core_is_cortex_a15(unsigned int core, unsigned int cluster)
{
struct device_node *node;
@@ -610,22 +635,6 @@ static int __init sunxi_mc_smp_loopback(void)
return ret;
}

-/*
- * This holds any device nodes that we requested resources for,
- * so that we may easily release resources in the error path.
- */
-struct sunxi_mc_smp_nodes {
- struct device_node *prcm_node;
- struct device_node *cpucfg_node;
- struct device_node *sram_node;
-};
-
-/* This structure holds SoC-specific bits tied to an enable-method string. */
-struct sunxi_mc_smp_data {
- const char *enable_method;
- int (*get_smp_nodes)(struct sunxi_mc_smp_nodes *nodes);
-};
-
static void __init sunxi_mc_smp_put_nodes(struct sunxi_mc_smp_nodes *nodes)
{
of_node_put(nodes->prcm_node);
@@ -660,13 +669,6 @@ static int __init sun9i_a80_get_smp_nodes(struct sunxi_mc_smp_nodes *nodes)
return 0;
}

-static const struct sunxi_mc_smp_data sunxi_mc_smp_data[] __initconst = {
- {
- .enable_method = "allwinner,sun9i-a80-smp",
- .get_smp_nodes = sun9i_a80_get_smp_nodes,
- },
-};
-
static int __init sunxi_mc_smp_init(void)
{
struct sunxi_mc_smp_nodes nodes = { 0 };
--
2.11.0


2018-04-03 06:24:34

by Mylène Josserand

[permalink] [raw]
Subject: [PATCH v5 01/13] ARM: move cputype definitions into another file

To add the support for SMP on sun8i-a83t, we will use some
definitions in an assembly file so move definitions into
another file to separate C functions and macro defintions.

Signed-off-by: Mylène Josserand <[email protected]>
---
arch/arm/include/asm/cputype.h | 94 +-----------------------------------
arch/arm/include/asm/cputype_def.h | 98 ++++++++++++++++++++++++++++++++++++++
2 files changed, 99 insertions(+), 93 deletions(-)
create mode 100644 arch/arm/include/asm/cputype_def.h

diff --git a/arch/arm/include/asm/cputype.h b/arch/arm/include/asm/cputype.h
index cb546425da8a..4cb26e840a58 100644
--- a/arch/arm/include/asm/cputype.h
+++ b/arch/arm/include/asm/cputype.h
@@ -4,99 +4,7 @@

#include <linux/stringify.h>
#include <linux/kernel.h>
-
-#define CPUID_ID 0
-#define CPUID_CACHETYPE 1
-#define CPUID_TCM 2
-#define CPUID_TLBTYPE 3
-#define CPUID_MPUIR 4
-#define CPUID_MPIDR 5
-#define CPUID_REVIDR 6
-
-#ifdef CONFIG_CPU_V7M
-#define CPUID_EXT_PFR0 0x40
-#define CPUID_EXT_PFR1 0x44
-#define CPUID_EXT_DFR0 0x48
-#define CPUID_EXT_AFR0 0x4c
-#define CPUID_EXT_MMFR0 0x50
-#define CPUID_EXT_MMFR1 0x54
-#define CPUID_EXT_MMFR2 0x58
-#define CPUID_EXT_MMFR3 0x5c
-#define CPUID_EXT_ISAR0 0x60
-#define CPUID_EXT_ISAR1 0x64
-#define CPUID_EXT_ISAR2 0x68
-#define CPUID_EXT_ISAR3 0x6c
-#define CPUID_EXT_ISAR4 0x70
-#define CPUID_EXT_ISAR5 0x74
-#else
-#define CPUID_EXT_PFR0 "c1, 0"
-#define CPUID_EXT_PFR1 "c1, 1"
-#define CPUID_EXT_DFR0 "c1, 2"
-#define CPUID_EXT_AFR0 "c1, 3"
-#define CPUID_EXT_MMFR0 "c1, 4"
-#define CPUID_EXT_MMFR1 "c1, 5"
-#define CPUID_EXT_MMFR2 "c1, 6"
-#define CPUID_EXT_MMFR3 "c1, 7"
-#define CPUID_EXT_ISAR0 "c2, 0"
-#define CPUID_EXT_ISAR1 "c2, 1"
-#define CPUID_EXT_ISAR2 "c2, 2"
-#define CPUID_EXT_ISAR3 "c2, 3"
-#define CPUID_EXT_ISAR4 "c2, 4"
-#define CPUID_EXT_ISAR5 "c2, 5"
-#endif
-
-#define MPIDR_SMP_BITMASK (0x3 << 30)
-#define MPIDR_SMP_VALUE (0x2 << 30)
-
-#define MPIDR_MT_BITMASK (0x1 << 24)
-
-#define MPIDR_HWID_BITMASK 0xFFFFFF
-
-#define MPIDR_INVALID (~MPIDR_HWID_BITMASK)
-
-#define MPIDR_LEVEL_BITS 8
-#define MPIDR_LEVEL_MASK ((1 << MPIDR_LEVEL_BITS) - 1)
-#define MPIDR_LEVEL_SHIFT(level) (MPIDR_LEVEL_BITS * level)
-
-#define MPIDR_AFFINITY_LEVEL(mpidr, level) \
- ((mpidr >> (MPIDR_LEVEL_BITS * level)) & MPIDR_LEVEL_MASK)
-
-#define ARM_CPU_IMP_ARM 0x41
-#define ARM_CPU_IMP_DEC 0x44
-#define ARM_CPU_IMP_INTEL 0x69
-
-/* ARM implemented processors */
-#define ARM_CPU_PART_ARM1136 0x4100b360
-#define ARM_CPU_PART_ARM1156 0x4100b560
-#define ARM_CPU_PART_ARM1176 0x4100b760
-#define ARM_CPU_PART_ARM11MPCORE 0x4100b020
-#define ARM_CPU_PART_CORTEX_A8 0x4100c080
-#define ARM_CPU_PART_CORTEX_A9 0x4100c090
-#define ARM_CPU_PART_CORTEX_A5 0x4100c050
-#define ARM_CPU_PART_CORTEX_A7 0x4100c070
-#define ARM_CPU_PART_CORTEX_A12 0x4100c0d0
-#define ARM_CPU_PART_CORTEX_A17 0x4100c0e0
-#define ARM_CPU_PART_CORTEX_A15 0x4100c0f0
-#define ARM_CPU_PART_MASK 0xff00fff0
-
-/* DEC implemented cores */
-#define ARM_CPU_PART_SA1100 0x4400a110
-
-/* Intel implemented cores */
-#define ARM_CPU_PART_SA1110 0x6900b110
-#define ARM_CPU_REV_SA1110_A0 0
-#define ARM_CPU_REV_SA1110_B0 4
-#define ARM_CPU_REV_SA1110_B1 5
-#define ARM_CPU_REV_SA1110_B2 6
-#define ARM_CPU_REV_SA1110_B4 8
-
-#define ARM_CPU_XSCALE_ARCH_MASK 0xe000
-#define ARM_CPU_XSCALE_ARCH_V1 0x2000
-#define ARM_CPU_XSCALE_ARCH_V2 0x4000
-#define ARM_CPU_XSCALE_ARCH_V3 0x6000
-
-/* Qualcomm implemented cores */
-#define ARM_CPU_PART_SCORPION 0x510002d0
+#include <asm/cputype_def.h>

extern unsigned int processor_id;

diff --git a/arch/arm/include/asm/cputype_def.h b/arch/arm/include/asm/cputype_def.h
new file mode 100644
index 000000000000..3a62ea13dc35
--- /dev/null
+++ b/arch/arm/include/asm/cputype_def.h
@@ -0,0 +1,98 @@
+/* SPDX-License-Identifier: GPL-2.0 */
+#ifndef __ASM_ARM_CPUTYPEDEF_H
+#define __ASM_ARM_CPUTYPEDEF_H
+
+#define CPUID_ID 0
+#define CPUID_CACHETYPE 1
+#define CPUID_TCM 2
+#define CPUID_TLBTYPE 3
+#define CPUID_MPUIR 4
+#define CPUID_MPIDR 5
+#define CPUID_REVIDR 6
+
+#ifdef CONFIG_CPU_V7M
+#define CPUID_EXT_PFR0 0x40
+#define CPUID_EXT_PFR1 0x44
+#define CPUID_EXT_DFR0 0x48
+#define CPUID_EXT_AFR0 0x4c
+#define CPUID_EXT_MMFR0 0x50
+#define CPUID_EXT_MMFR1 0x54
+#define CPUID_EXT_MMFR2 0x58
+#define CPUID_EXT_MMFR3 0x5c
+#define CPUID_EXT_ISAR0 0x60
+#define CPUID_EXT_ISAR1 0x64
+#define CPUID_EXT_ISAR2 0x68
+#define CPUID_EXT_ISAR3 0x6c
+#define CPUID_EXT_ISAR4 0x70
+#define CPUID_EXT_ISAR5 0x74
+#else
+#define CPUID_EXT_PFR0 "c1, 0"
+#define CPUID_EXT_PFR1 "c1, 1"
+#define CPUID_EXT_DFR0 "c1, 2"
+#define CPUID_EXT_AFR0 "c1, 3"
+#define CPUID_EXT_MMFR0 "c1, 4"
+#define CPUID_EXT_MMFR1 "c1, 5"
+#define CPUID_EXT_MMFR2 "c1, 6"
+#define CPUID_EXT_MMFR3 "c1, 7"
+#define CPUID_EXT_ISAR0 "c2, 0"
+#define CPUID_EXT_ISAR1 "c2, 1"
+#define CPUID_EXT_ISAR2 "c2, 2"
+#define CPUID_EXT_ISAR3 "c2, 3"
+#define CPUID_EXT_ISAR4 "c2, 4"
+#define CPUID_EXT_ISAR5 "c2, 5"
+#endif
+
+#define MPIDR_SMP_BITMASK (0x3 << 30)
+#define MPIDR_SMP_VALUE (0x2 << 30)
+
+#define MPIDR_MT_BITMASK (0x1 << 24)
+
+#define MPIDR_HWID_BITMASK 0xFFFFFF
+
+#define MPIDR_INVALID (~MPIDR_HWID_BITMASK)
+
+#define MPIDR_LEVEL_BITS 8
+#define MPIDR_LEVEL_MASK ((1 << MPIDR_LEVEL_BITS) - 1)
+#define MPIDR_LEVEL_SHIFT(level) (MPIDR_LEVEL_BITS * level)
+
+#define MPIDR_AFFINITY_LEVEL(mpidr, level) \
+ ((mpidr >> (MPIDR_LEVEL_BITS * level)) & MPIDR_LEVEL_MASK)
+
+#define ARM_CPU_IMP_ARM 0x41
+#define ARM_CPU_IMP_DEC 0x44
+#define ARM_CPU_IMP_INTEL 0x69
+
+/* ARM implemented processors */
+#define ARM_CPU_PART_ARM1136 0x4100b360
+#define ARM_CPU_PART_ARM1156 0x4100b560
+#define ARM_CPU_PART_ARM1176 0x4100b760
+#define ARM_CPU_PART_ARM11MPCORE 0x4100b020
+#define ARM_CPU_PART_CORTEX_A8 0x4100c080
+#define ARM_CPU_PART_CORTEX_A9 0x4100c090
+#define ARM_CPU_PART_CORTEX_A5 0x4100c050
+#define ARM_CPU_PART_CORTEX_A7 0x4100c070
+#define ARM_CPU_PART_CORTEX_A12 0x4100c0d0
+#define ARM_CPU_PART_CORTEX_A17 0x4100c0e0
+#define ARM_CPU_PART_CORTEX_A15 0x4100c0f0
+#define ARM_CPU_PART_MASK 0xff00fff0
+
+/* DEC implemented cores */
+#define ARM_CPU_PART_SA1100 0x4400a110
+
+/* Intel implemented cores */
+#define ARM_CPU_PART_SA1110 0x6900b110
+#define ARM_CPU_REV_SA1110_A0 0
+#define ARM_CPU_REV_SA1110_B0 4
+#define ARM_CPU_REV_SA1110_B1 5
+#define ARM_CPU_REV_SA1110_B2 6
+#define ARM_CPU_REV_SA1110_B4 8
+
+#define ARM_CPU_XSCALE_ARCH_MASK 0xe000
+#define ARM_CPU_XSCALE_ARCH_V1 0x2000
+#define ARM_CPU_XSCALE_ARCH_V2 0x4000
+#define ARM_CPU_XSCALE_ARCH_V3 0x6000
+
+/* Qualcomm implemented cores */
+#define ARM_CPU_PART_SCORPION 0x510002d0
+
+#endif
--
2.11.0


2018-04-03 06:24:43

by Mylène Josserand

[permalink] [raw]
Subject: [PATCH v5 03/13] ARM: sunxi: smp: Move cpu_resume assembly entry into file

Move the CPU resume assembly function into the assembly file
to remove all assembly code from C code.

Signed-off-by: Mylène Josserand <[email protected]>
---
arch/arm/mach-sunxi/headsmp.S | 5 +++++
arch/arm/mach-sunxi/mc_smp.c | 11 +----------
2 files changed, 6 insertions(+), 10 deletions(-)

diff --git a/arch/arm/mach-sunxi/headsmp.S b/arch/arm/mach-sunxi/headsmp.S
index 119005b5dff9..79890fbe5613 100644
--- a/arch/arm/mach-sunxi/headsmp.S
+++ b/arch/arm/mach-sunxi/headsmp.S
@@ -73,3 +73,8 @@ ENTRY(sunxi_mc_smp_secondary_startup)
bl sunxi_mc_smp_cluster_cache_enable
b secondary_startup
ENDPROC(sunxi_mc_smp_secondary_startup)
+
+ENTRY(sunxi_mc_smp_resume)
+ bl sunxi_mc_smp_cluster_cache_enable
+ b cpu_resume
+ENDPROC(sunxi_mc_smp_resume)
diff --git a/arch/arm/mach-sunxi/mc_smp.c b/arch/arm/mach-sunxi/mc_smp.c
index 9cc6c8fc6db7..727968d6a3e5 100644
--- a/arch/arm/mach-sunxi/mc_smp.c
+++ b/arch/arm/mach-sunxi/mc_smp.c
@@ -73,6 +73,7 @@ static void __iomem *prcm_base;
static void __iomem *sram_b_smp_base;

extern void sunxi_mc_smp_secondary_startup(void);
+extern void sunxi_mc_smp_resume(void);

static bool sunxi_core_is_cortex_a15(unsigned int core, unsigned int cluster)
{
@@ -572,16 +573,6 @@ static bool __init sunxi_mc_smp_cpu_table_init(void)
*/
typedef typeof(cpu_reset) phys_reset_t;

-static void __init __naked sunxi_mc_smp_resume(void)
-{
- asm volatile(
- "bl sunxi_mc_smp_cluster_cache_enable\n"
- "b cpu_resume"
- /* Let compiler know about sunxi_mc_smp_cluster_cache_enable */
- :: "i" (sunxi_mc_smp_cluster_cache_enable)
- );
-}
-
static int __init nocache_trampoline(unsigned long __unused)
{
phys_reset_t phys_reset;
--
2.11.0


2018-04-03 06:24:48

by Mylène Josserand

[permalink] [raw]
Subject: [PATCH v5 07/13] ARM: smp: Add initialization of CNTVOFF

The CNTVOFF register from arch timer is uninitialized.
It should be done by the bootloader but it is currently not the case,
even for boot CPU because this SoC is booting in secure mode.
It leads to an random offset value meaning that each CPU will have a
different time, which isn't working very well.

Add assembly code used for boot CPU and secondary CPU cores to make
sure that the CNTVOFF register is initialized. Because this code can
be used by different platforms, add this assembly file in ARM's common
folder.

Signed-off-by: Mylène Josserand <[email protected]>
---
arch/arm/common/Makefile | 1 +
arch/arm/common/smp_cntvoff.S | 35 +++++++++++++++++++++++++++++++++++
arch/arm/include/asm/smp_cntvoff.h | 8 ++++++++
3 files changed, 44 insertions(+)
create mode 100644 arch/arm/common/smp_cntvoff.S
create mode 100644 arch/arm/include/asm/smp_cntvoff.h

diff --git a/arch/arm/common/Makefile b/arch/arm/common/Makefile
index 70b4a14ed993..83117deb86c8 100644
--- a/arch/arm/common/Makefile
+++ b/arch/arm/common/Makefile
@@ -10,6 +10,7 @@ obj-$(CONFIG_DMABOUNCE) += dmabounce.o
obj-$(CONFIG_SHARP_LOCOMO) += locomo.o
obj-$(CONFIG_SHARP_PARAM) += sharpsl_param.o
obj-$(CONFIG_SHARP_SCOOP) += scoop.o
+obj-$(CONFIG_SMP) += smp_cntvoff.o
obj-$(CONFIG_PCI_HOST_ITE8152) += it8152.o
obj-$(CONFIG_MCPM) += mcpm_head.o mcpm_entry.o mcpm_platsmp.o vlock.o
CFLAGS_REMOVE_mcpm_entry.o = -pg
diff --git a/arch/arm/common/smp_cntvoff.S b/arch/arm/common/smp_cntvoff.S
new file mode 100644
index 000000000000..65ed199a50fe
--- /dev/null
+++ b/arch/arm/common/smp_cntvoff.S
@@ -0,0 +1,35 @@
+// SPDX-License-Identifier: GPL-2.0
+/*
+ * Copyright (c) 2018 Chen-Yu Tsai
+ * Copyright (c) 2018 Bootlin
+ *
+ * Chen-Yu Tsai <[email protected]>
+ * Mylène Josserand <[email protected]>
+ *
+ * SMP support for sunxi based systems with Cortex A7/A15
+ *
+ */
+
+#include <linux/linkage.h>
+#include <asm/assembler.h>
+
+ENTRY(smp_init_cntvoff)
+ .arch armv7-a
+ /*
+ * CNTVOFF has to be initialized either from non-secure Hypervisor
+ * mode or secure Monitor mode with SCR.NS==1. If TrustZone is enabled
+ * then it should be handled by the secure code
+ */
+ cps #MON_MODE
+ mrc p15, 0, r1, c1, c1, 0 /* Get Secure Config */
+ orr r0, r1, #1
+ mcr p15, 0, r0, c1, c1, 0 /* Set Non Secure bit */
+ isb
+ mov r0, #0
+ mcrr p15, 4, r0, r0, c14 /* CNTVOFF = 0 */
+ isb
+ mcr p15, 0, r1, c1, c1, 0 /* Set Secure bit */
+ isb
+ cps #SVC_MODE
+ ret lr
+ENDPROC(smp_init_cntvoff)
diff --git a/arch/arm/include/asm/smp_cntvoff.h b/arch/arm/include/asm/smp_cntvoff.h
new file mode 100644
index 000000000000..59a95f7604ee
--- /dev/null
+++ b/arch/arm/include/asm/smp_cntvoff.h
@@ -0,0 +1,8 @@
+// SPDX-License-Identifier: GPL-2.0
+
+#ifndef __ASMARM_ARCH_CNTVOFF_H
+#define __ASMARM_ARCH_CNTVOFF_H
+
+extern void smp_init_cntvoff(void);
+
+#endif
--
2.11.0


2018-04-03 06:24:59

by Mylène Josserand

[permalink] [raw]
Subject: [PATCH v5 06/13] ARM: dts: sun8i: a83t: Add CCI-400 node

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 85f14f4ebeed..9ac905884d81 100644
--- a/arch/arm/boot/dts/sun8i-a83t.dtsi
+++ b/arch/arm/boot/dts/sun8i-a83t.dtsi
@@ -66,6 +66,7 @@
compatible = "arm,cortex-a7";
device_type = "cpu";
operating-points-v2 = <&cpu0_opp_table>;
+ cci-control-port = <&cci_control0>;
reg = <0>;
};

@@ -73,6 +74,7 @@
compatible = "arm,cortex-a7";
device_type = "cpu";
operating-points-v2 = <&cpu0_opp_table>;
+ cci-control-port = <&cci_control0>;
reg = <1>;
};

@@ -80,6 +82,7 @@
compatible = "arm,cortex-a7";
device_type = "cpu";
operating-points-v2 = <&cpu0_opp_table>;
+ cci-control-port = <&cci_control0>;
reg = <2>;
};

@@ -87,6 +90,7 @@
compatible = "arm,cortex-a7";
device_type = "cpu";
operating-points-v2 = <&cpu0_opp_table>;
+ cci-control-port = <&cci_control0>;
reg = <3>;
};

@@ -96,6 +100,7 @@
compatible = "arm,cortex-a7";
device_type = "cpu";
operating-points-v2 = <&cpu1_opp_table>;
+ cci-control-port = <&cci_control1>;
reg = <0x100>;
};

@@ -103,6 +108,7 @@
compatible = "arm,cortex-a7";
device_type = "cpu";
operating-points-v2 = <&cpu1_opp_table>;
+ cci-control-port = <&cci_control1>;
reg = <0x101>;
};

@@ -110,6 +116,7 @@
compatible = "arm,cortex-a7";
device_type = "cpu";
operating-points-v2 = <&cpu1_opp_table>;
+ cci-control-port = <&cci_control1>;
reg = <0x102>;
};

@@ -117,6 +124,7 @@
compatible = "arm,cortex-a7";
device_type = "cpu";
operating-points-v2 = <&cpu1_opp_table>;
+ cci-control-port = <&cci_control1>;
reg = <0x103>;
};
};
@@ -354,6 +362,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


2018-04-03 06:24:59

by Mylène Josserand

[permalink] [raw]
Subject: [PATCH v5 02/13] ARM: sunxi: smp: Move assembly code into a file

Move the assembly code for cluster cache enabling
into an assembly file instead of having it directly in C code.

Remove the CFLAGS because we are using the ARM directive "arch"
instead.

Signed-off-by: Mylène Josserand <[email protected]>
---
arch/arm/mach-sunxi/Makefile | 4 +--
arch/arm/mach-sunxi/headsmp.S | 75 +++++++++++++++++++++++++++++++++++++++++++
arch/arm/mach-sunxi/mc_smp.c | 71 ++--------------------------------------
3 files changed, 79 insertions(+), 71 deletions(-)
create mode 100644 arch/arm/mach-sunxi/headsmp.S

diff --git a/arch/arm/mach-sunxi/Makefile b/arch/arm/mach-sunxi/Makefile
index 7de9cc286d53..7f45071ae74a 100644
--- a/arch/arm/mach-sunxi/Makefile
+++ b/arch/arm/mach-sunxi/Makefile
@@ -1,5 +1,3 @@
-CFLAGS_mc_smp.o += -march=armv7-a
-
obj-$(CONFIG_ARCH_SUNXI) += sunxi.o
-obj-$(CONFIG_ARCH_SUNXI_MC_SMP) += mc_smp.o
+obj-$(CONFIG_ARCH_SUNXI_MC_SMP) += mc_smp.o headsmp.o
obj-$(CONFIG_SMP) += platsmp.o
diff --git a/arch/arm/mach-sunxi/headsmp.S b/arch/arm/mach-sunxi/headsmp.S
new file mode 100644
index 000000000000..119005b5dff9
--- /dev/null
+++ b/arch/arm/mach-sunxi/headsmp.S
@@ -0,0 +1,75 @@
+// SPDX-License-Identifier: GPL-2.0
+/*
+ * Copyright (c) 2018 Chen-Yu Tsai
+ * Copyright (c) 2018 Bootlin
+ *
+ * Chen-Yu Tsai <[email protected]>
+ * Mylène Josserand <[email protected]>
+ *
+ * SMP support for sunxi based systems with Cortex A7/A15
+ *
+ */
+
+#include <linux/linkage.h>
+#include <asm/assembler.h>
+#include <asm/cputype_def.h>
+
+ENTRY(sunxi_mc_smp_cluster_cache_enable)
+ .arch armv7-a
+ /*
+ * Enable cluster-level coherency, in preparation for turning on the MMU.
+ *
+ * Also enable regional clock gating and L2 data latency settings for
+ * Cortex-A15. These settings are from the vendor kernel.
+ */
+ mrc p15, 0, r1, c0, c0, 0
+ movw r2, #(ARM_CPU_PART_MASK & 0xffff)
+ movt r2, #(ARM_CPU_PART_MASK >> 16)
+ and r1, r1, r2
+ movw r2, #(ARM_CPU_PART_CORTEX_A15 & 0xffff)
+ movt r2, #(ARM_CPU_PART_CORTEX_A15 >> 16)
+ cmp r1, r2
+ bne not_a15
+
+ /* The following is Cortex-A15 specific */
+
+ /* ACTLR2: Enable CPU regional clock gates */
+ mrc p15, 1, r1, c15, c0, 4
+ orr r1, r1, #(0x1 << 31)
+ mcr p15, 1, r1, c15, c0, 4
+
+ /* L2ACTLR */
+ mrc p15, 1, r1, c15, c0, 0
+ /* Enable L2, GIC, and Timer regional clock gates */
+ orr r1, r1, #(0x1 << 26)
+ /* Disable clean/evict from being pushed to external */
+ orr r1, r1, #(0x1<<3)
+ mcr p15, 1, r1, c15, c0, 0
+
+ /* L2CTRL: L2 data RAM latency */
+ mrc p15, 1, r1, c9, c0, 2
+ bic r1, r1, #(0x7 << 0)
+ orr r1, r1, #(0x3 << 0)
+ mcr p15, 1, r1, c9, c0, 2
+
+ /* End of Cortex-A15 specific setup */
+ not_a15:
+
+ /* Get value of sunxi_mc_smp_first_comer */
+ adr r1, first
+ ldr r0, [r1]
+ ldr r0, [r1, r0]
+
+ /* Skip cci_enable_port_for_self if not first comer */
+ cmp r0, #0
+ bxeq lr
+ b cci_enable_port_for_self
+
+ .align 2
+ first: .word sunxi_mc_smp_first_comer - .
+ENDPROC(sunxi_mc_smp_cluster_cache_enable)
+
+ENTRY(sunxi_mc_smp_secondary_startup)
+ bl sunxi_mc_smp_cluster_cache_enable
+ b secondary_startup
+ENDPROC(sunxi_mc_smp_secondary_startup)
diff --git a/arch/arm/mach-sunxi/mc_smp.c b/arch/arm/mach-sunxi/mc_smp.c
index c0246ec54a0a..9cc6c8fc6db7 100644
--- a/arch/arm/mach-sunxi/mc_smp.c
+++ b/arch/arm/mach-sunxi/mc_smp.c
@@ -72,6 +72,8 @@ static void __iomem *cpucfg_base;
static void __iomem *prcm_base;
static void __iomem *sram_b_smp_base;

+extern void sunxi_mc_smp_secondary_startup(void);
+
static bool sunxi_core_is_cortex_a15(unsigned int core, unsigned int cluster)
{
struct device_node *node;
@@ -300,74 +302,7 @@ static void sunxi_cluster_cache_disable_without_axi(void)
}

static int sunxi_mc_smp_cpu_table[SUNXI_NR_CLUSTERS][SUNXI_CPUS_PER_CLUSTER];
-static int sunxi_mc_smp_first_comer;
-
-/*
- * Enable cluster-level coherency, in preparation for turning on the MMU.
- *
- * Also enable regional clock gating and L2 data latency settings for
- * Cortex-A15. These settings are from the vendor kernel.
- */
-static void __naked sunxi_mc_smp_cluster_cache_enable(void)
-{
- asm volatile (
- "mrc p15, 0, r1, c0, c0, 0\n"
- "movw r2, #" __stringify(ARM_CPU_PART_MASK & 0xffff) "\n"
- "movt r2, #" __stringify(ARM_CPU_PART_MASK >> 16) "\n"
- "and r1, r1, r2\n"
- "movw r2, #" __stringify(ARM_CPU_PART_CORTEX_A15 & 0xffff) "\n"
- "movt r2, #" __stringify(ARM_CPU_PART_CORTEX_A15 >> 16) "\n"
- "cmp r1, r2\n"
- "bne not_a15\n"
-
- /* The following is Cortex-A15 specific */
-
- /* ACTLR2: Enable CPU regional clock gates */
- "mrc p15, 1, r1, c15, c0, 4\n"
- "orr r1, r1, #(0x1<<31)\n"
- "mcr p15, 1, r1, c15, c0, 4\n"
-
- /* L2ACTLR */
- "mrc p15, 1, r1, c15, c0, 0\n"
- /* Enable L2, GIC, and Timer regional clock gates */
- "orr r1, r1, #(0x1<<26)\n"
- /* Disable clean/evict from being pushed to external */
- "orr r1, r1, #(0x1<<3)\n"
- "mcr p15, 1, r1, c15, c0, 0\n"
-
- /* L2CTRL: L2 data RAM latency */
- "mrc p15, 1, r1, c9, c0, 2\n"
- "bic r1, r1, #(0x7<<0)\n"
- "orr r1, r1, #(0x3<<0)\n"
- "mcr p15, 1, r1, c9, c0, 2\n"
-
- /* End of Cortex-A15 specific setup */
- "not_a15:\n"
-
- /* Get value of sunxi_mc_smp_first_comer */
- "adr r1, first\n"
- "ldr r0, [r1]\n"
- "ldr r0, [r1, r0]\n"
-
- /* Skip cci_enable_port_for_self if not first comer */
- "cmp r0, #0\n"
- "bxeq lr\n"
- "b cci_enable_port_for_self\n"
-
- ".align 2\n"
- "first: .word sunxi_mc_smp_first_comer - .\n"
- );
-}
-
-static void __naked sunxi_mc_smp_secondary_startup(void)
-{
- asm volatile(
- "bl sunxi_mc_smp_cluster_cache_enable\n"
- "b secondary_startup"
- /* Let compiler know about sunxi_mc_smp_cluster_cache_enable */
- :: "i" (sunxi_mc_smp_cluster_cache_enable)
- );
-}
+int sunxi_mc_smp_first_comer;

static DEFINE_SPINLOCK(boot_lock);

--
2.11.0


2018-04-03 06:25:25

by Mylène Josserand

[permalink] [raw]
Subject: [PATCH v5 04/13] ARM: dts: sun8i: Add CPUCFG device node for A83T dtsi

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 568307639be8..32992afa0b12 100644
--- a/arch/arm/boot/dts/sun8i-a83t.dtsi
+++ b/arch/arm/boot/dts/sun8i-a83t.dtsi
@@ -349,6 +349,11 @@
};
};

+ cpucfg@1700000 {
+ compatible = "allwinner,sun8i-a83t-cpucfg";
+ reg = <0x01700000 0x400>;
+ };
+
syscon: syscon@1c00000 {
compatible = "allwinner,sun8i-a83t-system-controller",
"syscon";
--
2.11.0


2018-04-03 06:25:32

by Mylène Josserand

[permalink] [raw]
Subject: [PATCH v5 08/13] ARM: sunxi: Add initialization of CNTVOFF

Add the initialization of CNTVOFF for sun8i-a83t.

For boot CPU, Create a new machine that handles this
function's call in an "init_early" callback.
For secondary CPUs, add this function into secondary_startup
assembly entry.

Signed-off-by: Mylène Josserand <[email protected]>
---
arch/arm/mach-sunxi/headsmp.S | 1 +
arch/arm/mach-sunxi/sunxi.c | 18 +++++++++++++++++-
2 files changed, 18 insertions(+), 1 deletion(-)

diff --git a/arch/arm/mach-sunxi/headsmp.S b/arch/arm/mach-sunxi/headsmp.S
index 79890fbe5613..b586b7cf803a 100644
--- a/arch/arm/mach-sunxi/headsmp.S
+++ b/arch/arm/mach-sunxi/headsmp.S
@@ -71,6 +71,7 @@ ENDPROC(sunxi_mc_smp_cluster_cache_enable)

ENTRY(sunxi_mc_smp_secondary_startup)
bl sunxi_mc_smp_cluster_cache_enable
+ bl smp_init_cntvoff
b secondary_startup
ENDPROC(sunxi_mc_smp_secondary_startup)

diff --git a/arch/arm/mach-sunxi/sunxi.c b/arch/arm/mach-sunxi/sunxi.c
index 5e9602ce1573..090784108c0a 100644
--- a/arch/arm/mach-sunxi/sunxi.c
+++ b/arch/arm/mach-sunxi/sunxi.c
@@ -16,6 +16,7 @@
#include <linux/platform_device.h>

#include <asm/mach/arch.h>
+#include <asm/smp_cntvoff.h>

static const char * const sunxi_board_dt_compat[] = {
"allwinner,sun4i-a10",
@@ -62,7 +63,6 @@ MACHINE_END
static const char * const sun8i_board_dt_compat[] = {
"allwinner,sun8i-a23",
"allwinner,sun8i-a33",
- "allwinner,sun8i-a83t",
"allwinner,sun8i-h2-plus",
"allwinner,sun8i-h3",
"allwinner,sun8i-r40",
@@ -75,6 +75,22 @@ DT_MACHINE_START(SUN8I_DT, "Allwinner sun8i Family")
.dt_compat = sun8i_board_dt_compat,
MACHINE_END

+void __init sun8i_cntvoff_init(void)
+{
+ smp_init_cntvoff();
+}
+
+static const char * const sun8i_cntvoff_board_dt_compat[] = {
+ "allwinner,sun8i-a83t",
+ NULL,
+};
+
+DT_MACHINE_START(SUN8I_CNTVOFF_DT, "Allwinner sun8i boards needing cntvoff")
+ .init_early = sun8i_cntvoff_init,
+ .init_time = sun6i_timer_init,
+ .dt_compat = sun8i_cntvoff_board_dt_compat,
+MACHINE_END
+
static const char * const sun9i_board_dt_compat[] = {
"allwinner,sun9i-a80",
NULL,
--
2.11.0


2018-04-03 06:27:04

by Mylène Josserand

[permalink] [raw]
Subject: [PATCH v5 05/13] ARM: dts: sun8i: Add R_CPUCFG device node for the A83T dtsi

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 32992afa0b12..85f14f4ebeed 100644
--- a/arch/arm/boot/dts/sun8i-a83t.dtsi
+++ b/arch/arm/boot/dts/sun8i-a83t.dtsi
@@ -933,6 +933,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


2018-04-03 06:48:02

by Chen-Yu Tsai

[permalink] [raw]
Subject: Re: [PATCH v5 04/13] ARM: dts: sun8i: Add CPUCFG device node for A83T dtsi

On Tue, Apr 3, 2018 at 2:18 PM, Mylène Josserand
<[email protected]> wrote:
> 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]>

Reviewed-by: Chen-Yu Tsai <[email protected]>

2018-04-03 06:48:02

by Chen-Yu Tsai

[permalink] [raw]
Subject: Re: [PATCH v5 06/13] ARM: dts: sun8i: a83t: Add CCI-400 node

On Tue, Apr 3, 2018 at 2:18 PM, Mylène Josserand
<[email protected]> wrote:
> Add CCI-400 node and control-port on CPUs needed by SMP bringup.
>
> Signed-off-by: Mylène Josserand <[email protected]>

Reviewed-by: Chen-Yu Tsai <[email protected]>

2018-04-03 06:54:52

by Chen-Yu Tsai

[permalink] [raw]
Subject: Re: [PATCH v5 01/13] ARM: move cputype definitions into another file

On Tue, Apr 3, 2018 at 2:18 PM, Mylène Josserand
<[email protected]> wrote:
> To add the support for SMP on sun8i-a83t, we will use some
> definitions in an assembly file so move definitions into
> another file to separate C functions and macro defintions.

Instead of moving the definitions, you could guard all the C
stuff in "#ifndef __ASSEMBLY__". AFAIK a few header files do that.

ChenYu

> Signed-off-by: Mylène Josserand <[email protected]>
> ---
> arch/arm/include/asm/cputype.h | 94 +-----------------------------------
> arch/arm/include/asm/cputype_def.h | 98 ++++++++++++++++++++++++++++++++++++++

2018-04-03 07:28:55

by Mylène Josserand

[permalink] [raw]
Subject: Re: [PATCH v5 01/13] ARM: move cputype definitions into another file

Hi Chen-Yu,

Thank you for your review!

On Tue, 3 Apr 2018 14:52:45 +0800
Chen-Yu Tsai <[email protected]> wrote:

> On Tue, Apr 3, 2018 at 2:18 PM, Mylène Josserand
> <[email protected]> wrote:
> > To add the support for SMP on sun8i-a83t, we will use some
> > definitions in an assembly file so move definitions into
> > another file to separate C functions and macro defintions.
>
> Instead of moving the definitions, you could guard all the C
> stuff in "#ifndef __ASSEMBLY__". AFAIK a few header files do that.

Oh, right. It is better with this check so I will use that in the next
iteration of my series.

Best regards,

Mylène

--
Mylène Josserand, Bootlin (formerly Free Electrons)
Embedded Linux and Kernel engineering
http://bootlin.com


>
> ChenYu
>
> > Signed-off-by: Mylène Josserand <[email protected]>
> > ---
> > arch/arm/include/asm/cputype.h | 94 +-----------------------------------
> > arch/arm/include/asm/cputype_def.h | 98 ++++++++++++++++++++++++++++++++++++++

2018-04-03 07:36:09

by Chen-Yu Tsai

[permalink] [raw]
Subject: Re: [PATCH v5 01/13] ARM: move cputype definitions into another file

On Tue, Apr 3, 2018 at 3:27 PM, Mylène Josserand
<[email protected]> wrote:
> Hi Chen-Yu,
>
> Thank you for your review!
>
> On Tue, 3 Apr 2018 14:52:45 +0800
> Chen-Yu Tsai <[email protected]> wrote:
>
>> On Tue, Apr 3, 2018 at 2:18 PM, Mylène Josserand
>> <[email protected]> wrote:
>> > To add the support for SMP on sun8i-a83t, we will use some
>> > definitions in an assembly file so move definitions into
>> > another file to separate C functions and macro defintions.
>>
>> Instead of moving the definitions, you could guard all the C
>> stuff in "#ifndef __ASSEMBLY__". AFAIK a few header files do that.
>
> Oh, right. It is better with this check so I will use that in the next
> iteration of my series.

I'm not sure about the policy, but I kind of assume any header under
"asm" would be usable in assembly files, so the guard would be better.

ChenYu

2018-04-03 08:48:17

by Maxime Ripard

[permalink] [raw]
Subject: Re: [PATCH v5 11/13] ARM: sun9i: smp: Add is_sun9i field

On Tue, Apr 03, 2018 at 08:18:34AM +0200, Myl?ne Josserand wrote:
> To prepare the support of sun8i-a83t, add a field in the smp_data
> structure to enable the case of sun9i.
>
> Start to handle the differences between sun9i-a80 and sun8i-a83t
> by using this variable.
>
> Add an index to retrieve which structures we are using.

This should have been in a separate commit, but maybe we can store a
pointer to the array cell we're using instead of always using the
index?

Maxime

--
Maxime Ripard, Bootlin (formerly Free Electrons)
Embedded Linux and Kernel engineering
https://bootlin.com


Attachments:
(No filename) (613.00 B)
signature.asc (849.00 B)
Download all attachments

2018-04-03 08:49:40

by Maxime Ripard

[permalink] [raw]
Subject: Re: [PATCH v5 10/13] ARM: sun9i: smp: Move structures

On Tue, Apr 03, 2018 at 08:18:33AM +0200, Myl?ne Josserand wrote:
> To prepare the support for sun8i-a83t, move some structures
> at the beginning of the file.
>
> Signed-off-by: Myl?ne Josserand <[email protected]>

I'm not quite sure what would be the benefit from that, if it's was
working before, then it would probably work in your case as welle, right?

Maxime

--
Maxime Ripard, Bootlin (formerly Free Electrons)
Embedded Linux and Kernel engineering
https://bootlin.com


Attachments:
(No filename) (506.00 B)
signature.asc (849.00 B)
Download all attachments

2018-04-03 08:50:17

by Chen-Yu Tsai

[permalink] [raw]
Subject: Re: [PATCH v5 12/13] ARM: sun8i: smp: Add support for A83T

On Tue, Apr 3, 2018 at 2: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.
> There is also a bit swap between sun8i-a83t and sun9i-a80 that must be
> handled.
>
> Signed-off-by: Mylène Josserand <[email protected]>
> ---
> arch/arm/mach-sunxi/mc_smp.c | 120 +++++++++++++++++++++++++++++++++++++++++--
> 1 file changed, 117 insertions(+), 3 deletions(-)
>
> diff --git a/arch/arm/mach-sunxi/mc_smp.c b/arch/arm/mach-sunxi/mc_smp.c
> index 468a6c46bfc9..72e497dc43ac 100644
> --- a/arch/arm/mach-sunxi/mc_smp.c
> +++ b/arch/arm/mach-sunxi/mc_smp.c
> @@ -55,22 +55,31 @@
> #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))
> +/* The power off register for clusters are different from a80 and a83t */
> +#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 */

You should mention it as A83T specific, since sun8i covers the
entire Cortex-A7-based SoC family.

> +#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 *prcm_base;
> static void __iomem *sram_b_smp_base;
> +static void __iomem *r_cpucfg_base;
> static int index;
>
> /*
> @@ -81,6 +90,7 @@ struct sunxi_mc_smp_nodes {
> struct device_node *prcm_node;
> struct device_node *cpucfg_node;
> struct device_node *sram_node;
> + struct device_node *r_cpucfg_node;
> };
>
> /* This structure holds SoC-specific bits tied to an enable-method string. */
> @@ -94,6 +104,7 @@ extern void sunxi_mc_smp_secondary_startup(void);
> extern void sunxi_mc_smp_resume(void);
>
> static int __init sun9i_a80_get_smp_nodes(struct sunxi_mc_smp_nodes *nodes);
> +static int __init sun8i_a83t_get_smp_nodes(struct sunxi_mc_smp_nodes *nodes);
>
> static const struct sunxi_mc_smp_data sunxi_mc_smp_data[] __initconst = {
> {
> @@ -101,6 +112,11 @@ static const struct sunxi_mc_smp_data sunxi_mc_smp_data[] __initconst = {
> .get_smp_nodes = sun9i_a80_get_smp_nodes,
> .is_sun9i = true,
> },
> + {
> + .enable_method = "allwinner,sun8i-a83t-smp",
> + .get_smp_nodes = sun8i_a83t_get_smp_nodes,
> + .is_sun9i = false,
> + },
> };
>
> static bool sunxi_core_is_cortex_a15(unsigned int core, unsigned int cluster)
> @@ -188,6 +204,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) {

Please check against is_sun9i, since this is A83T specific. My point
is we should be able to see clearly what parts of the code are shared,
and what parts are SoC specific.

> + /* 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));
> @@ -211,17 +237,38 @@ 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 (!sunxi_mc_smp_data[index].is_sun9i) {
> + 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);
>
> + /* Handle A83T bit swap */
> + if (!sunxi_mc_smp_data[index].is_sun9i) {
> + 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) {

Same here.

> + 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);
> @@ -243,6 +290,14 @@ static int sunxi_cluster_powerup(unsigned int cluster)
> if (cluster >= SUNXI_NR_CLUSTERS)
> return -EINVAL;
>
> + /* For A83T, assert cluster cores resets */
> + if (!sunxi_mc_smp_data[index].is_sun9i) {

Here it is correct.

> + 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;
> @@ -253,6 +308,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) {

Same here.

> + 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;
> @@ -285,6 +350,8 @@ static int sunxi_cluster_powerup(unsigned int cluster)
> reg = readl(prcm_base + PRCM_PWROFF_GATING_REG(cluster));
> if (sunxi_mc_smp_data[index].is_sun9i)
> reg &= ~PRCM_PWROFF_GATING_REG_CLUSTER_SUN9I;
> + else
> + reg &= ~PRCM_PWROFF_GATING_REG_CLUSTER_SUN8I;
> writel(reg, prcm_base + PRCM_PWROFF_GATING_REG(cluster));
> udelay(20);
>
> @@ -483,6 +550,8 @@ static int sunxi_cluster_powerdown(unsigned int cluster)
> reg = readl(prcm_base + PRCM_PWROFF_GATING_REG(cluster));
> if (sunxi_mc_smp_data[index].is_sun9i)
> reg |= PRCM_PWROFF_GATING_REG_CLUSTER_SUN9I;
> + else
> + reg |= PRCM_PWROFF_GATING_REG_CLUSTER_SUN8I;
> writel(reg, prcm_base + PRCM_PWROFF_GATING_REG(cluster));
> udelay(20);
>
> @@ -564,8 +633,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-a80 */
> + if (!sunxi_mc_smp_data[index].is_sun9i)
> + if (cpu == 0)
> + return false;

Once the above is fixed, this patch is

Reviewed-by: Chen-Yu Tsai <[email protected]>


Would it be possible for you to implement CPU0 hotplug? You needn't
do so as part of this patch or even this series. I disassembled the
BROM just now, and it is much simpler compared to the A80:

On boot, if running on cpu0, check magic register at 0x01f01dac
for magic value 0xfa50392f. If found, follow secondary startup
like other cores, otherwise continue normal boot procedure.
As you can see they use a register in CPUS-CFG instead of
secure SRAM this time.

> return true;
> }
> #endif
> @@ -645,6 +718,7 @@ static void __init sunxi_mc_smp_put_nodes(struct sunxi_mc_smp_nodes *nodes)
> of_node_put(nodes->prcm_node);
> of_node_put(nodes->cpucfg_node);
> of_node_put(nodes->sram_node);
> + of_node_put(nodes->r_cpucfg_node);
> memset(nodes, 0, sizeof(*nodes));
> }
>
> @@ -674,6 +748,32 @@ static int __init sun9i_a80_get_smp_nodes(struct sunxi_mc_smp_nodes *nodes)
> return 0;
> }
>
> +static int __init sun8i_a83t_get_smp_nodes(struct sunxi_mc_smp_nodes *nodes)
> +{
> + nodes->prcm_node = of_find_compatible_node(NULL, NULL,
> + "allwinner,sun8i-a83t-r-ccu");
> + if (!nodes->prcm_node) {
> + pr_err("%s: PRCM not available\n", __func__);
> + return -ENODEV;
> + }
> +
> + nodes->cpucfg_node = of_find_compatible_node(NULL, NULL,
> + "allwinner,sun8i-a83t-cpucfg");
> + if (!nodes->cpucfg_node) {
> + pr_err("%s: CPUCFG not available\n", __func__);
> + return -ENODEV;
> + }
> +
> + nodes->r_cpucfg_node = of_find_compatible_node(NULL, NULL,
> + "allwinner,sun8i-a83t-r-cpucfg");
> + if (!nodes->r_cpucfg_node) {
> + pr_err("%s: RCPUCFG not available\n", __func__);
> + return -ENODEV;
> + }
> +
> + return 0;
> +}
> +
> static int __init sunxi_mc_smp_init(void)
> {
> struct sunxi_mc_smp_nodes nodes = { 0 };
> @@ -752,6 +852,15 @@ static int __init sunxi_mc_smp_init(void)
> pr_err("%s: failed to map secure SRAM\n", __func__);
> goto err_unmap_release_cpucfg;
> }
> + } else {
> + r_cpucfg_base = of_io_request_and_map(nodes.r_cpucfg_node,
> + 0, "sunxi-mc-smp");
> + if (IS_ERR(r_cpucfg_base)) {
> + ret = PTR_ERR(r_cpucfg_base);
> + pr_err("%s: failed to map R-CPUCFG registers\n",
> + __func__);
> + goto err_unmap_release_cpucfg;
> + }
> }
>
> /* Configure CCI-400 for boot cluster */
> @@ -759,7 +868,7 @@ static int __init sunxi_mc_smp_init(void)
> if (ret) {
> pr_err("%s: failed to configure boot cluster: %d\n",
> __func__, ret);
> - goto err_unmap_release_secure_sram;
> + goto err_unmap_release_sram_rcpucfg;
> }
>
> /* We don't need the device nodes anymore */
> @@ -768,6 +877,8 @@ static int __init sunxi_mc_smp_init(void)
> /* Set the hardware entry point address */
> if (sunxi_mc_smp_data[index].is_sun9i)
> addr = prcm_base + PRCM_CPU_SOFT_ENTRY_REG;
> + else
> + addr = r_cpucfg_base + R_CPUCFG_CPU_SOFT_ENTRY_REG;
> writel(__pa_symbol(sunxi_mc_smp_secondary_startup), addr);
>
> /* Actually enable multi cluster SMP */
> @@ -777,10 +888,13 @@ static int __init sunxi_mc_smp_init(void)
>
> return 0;
>
> -err_unmap_release_secure_sram:
> +err_unmap_release_sram_rcpucfg:
> if (sunxi_mc_smp_data[index].is_sun9i) {
> iounmap(sram_b_smp_base);
> of_address_to_resource(nodes.sram_node, 0, &res);
> + } else {
> + iounmap(r_cpucfg_base);
> + of_address_to_resource(nodes.r_cpucfg_node, 0, &res);
> }
> release_mem_region(res.start, resource_size(&res));
> err_unmap_release_cpucfg:
> --
> 2.11.0
>

2018-04-03 08:50:28

by Chen-Yu Tsai

[permalink] [raw]
Subject: Re: [PATCH v5 11/13] ARM: sun9i: smp: Add is_sun9i field

On Tue, Apr 3, 2018 at 4:46 PM, Maxime Ripard <[email protected]> wrote:
> On Tue, Apr 03, 2018 at 08:18:34AM +0200, Mylène Josserand wrote:
>> To prepare the support of sun8i-a83t, add a field in the smp_data
>> structure to enable the case of sun9i.
>>
>> Start to handle the differences between sun9i-a80 and sun8i-a83t
>> by using this variable.
>>
>> Add an index to retrieve which structures we are using.
>
> This should have been in a separate commit, but maybe we can store a
> pointer to the array cell we're using instead of always using the
> index?

Using a pointer would also avoid some of the code movement from the
previous patch.

ChenYu

2018-04-03 08:53:34

by Chen-Yu Tsai

[permalink] [raw]
Subject: Re: [PATCH v5 10/13] ARM: sun9i: smp: Move structures

On Tue, Apr 3, 2018 at 4:47 PM, Maxime Ripard <[email protected]> wrote:
> On Tue, Apr 03, 2018 at 08:18:33AM +0200, Mylène Josserand wrote:
>> To prepare the support for sun8i-a83t, move some structures
>> at the beginning of the file.
>>
>> Signed-off-by: Mylène Josserand <[email protected]>
>
> I'm not quite sure what would be the benefit from that, if it's was
> working before, then it would probably work in your case as welle, right?

You should only need to move struct sunxi_mc_smp_data, since you are reading
the is_sun9i field. I suppose you could also get away with adding a global
variable just for that, and not have to store an index or pointer. Then none
of this code movement would be necessary. It would be slightly harder to
understand, but it's still contained within this file, and it has a clear
purpose, so I would call that acceptable.

ChenYu

2018-04-03 09:08:13

by Chen-Yu Tsai

[permalink] [raw]
Subject: Re: [PATCH v5 09/13] ARM: sun9i: smp: Rename clusters's power-off

On Tue, Apr 3, 2018 at 2:18 PM, Mylène Josserand
<[email protected]> wrote:
> To prepare the support for sun8i-a83t, rename the variable name
> that handles the power-off of clusters because it is different from
> sun9i-a80 to sun8i-a83t.
>
> The power off register for clusters are different from a80 and a83t.
>
> Signed-off-by: Mylène Josserand <[email protected]>
> Acked-by: Maxime Ripard <[email protected]>

Reviewed-by: Chen-Yu Tsai <[email protected]>

2018-04-03 09:09:07

by Chen-Yu Tsai

[permalink] [raw]
Subject: Re: [PATCH v5 05/13] ARM: dts: sun8i: Add R_CPUCFG device node for the A83T dtsi

On Tue, Apr 3, 2018 at 2:18 PM, Mylène Josserand
<[email protected]> wrote:
> 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 32992afa0b12..85f14f4ebeed 100644
> --- a/arch/arm/boot/dts/sun8i-a83t.dtsi
> +++ b/arch/arm/boot/dts/sun8i-a83t.dtsi
> @@ -933,6 +933,11 @@
> #reset-cells = <1>;
> };
>
> + r_cpucfg@1f01c00 {
> + compatible = "allwinner,sun8i-a83t-r-cpucfg";
> + reg = <0x1f01c00 0x100>;

The memory map says that its size is 0x400.

ChenYu

2018-04-03 09:13:44

by Maxime Ripard

[permalink] [raw]
Subject: Re: [PATCH v5 08/13] ARM: sunxi: Add initialization of CNTVOFF

On Tue, Apr 03, 2018 at 08:18:31AM +0200, Myl?ne Josserand wrote:
> Add the initialization of CNTVOFF for sun8i-a83t.
>
> For boot CPU, Create a new machine that handles this
> function's call in an "init_early" callback.
> For secondary CPUs, add this function into secondary_startup
> assembly entry.
>
> Signed-off-by: Myl?ne Josserand <[email protected]>
> ---
> arch/arm/mach-sunxi/headsmp.S | 1 +
> arch/arm/mach-sunxi/sunxi.c | 18 +++++++++++++++++-
> 2 files changed, 18 insertions(+), 1 deletion(-)
>
> diff --git a/arch/arm/mach-sunxi/headsmp.S b/arch/arm/mach-sunxi/headsmp.S
> index 79890fbe5613..b586b7cf803a 100644
> --- a/arch/arm/mach-sunxi/headsmp.S
> +++ b/arch/arm/mach-sunxi/headsmp.S
> @@ -71,6 +71,7 @@ ENDPROC(sunxi_mc_smp_cluster_cache_enable)
>
> ENTRY(sunxi_mc_smp_secondary_startup)
> bl sunxi_mc_smp_cluster_cache_enable
> + bl smp_init_cntvoff
> b secondary_startup
> ENDPROC(sunxi_mc_smp_secondary_startup)
>
> diff --git a/arch/arm/mach-sunxi/sunxi.c b/arch/arm/mach-sunxi/sunxi.c
> index 5e9602ce1573..090784108c0a 100644
> --- a/arch/arm/mach-sunxi/sunxi.c
> +++ b/arch/arm/mach-sunxi/sunxi.c
> @@ -16,6 +16,7 @@
> #include <linux/platform_device.h>
>
> #include <asm/mach/arch.h>
> +#include <asm/smp_cntvoff.h>
>
> static const char * const sunxi_board_dt_compat[] = {
> "allwinner,sun4i-a10",
> @@ -62,7 +63,6 @@ MACHINE_END
> static const char * const sun8i_board_dt_compat[] = {
> "allwinner,sun8i-a23",
> "allwinner,sun8i-a33",
> - "allwinner,sun8i-a83t",
> "allwinner,sun8i-h2-plus",
> "allwinner,sun8i-h3",
> "allwinner,sun8i-r40",
> @@ -75,6 +75,22 @@ DT_MACHINE_START(SUN8I_DT, "Allwinner sun8i Family")
> .dt_compat = sun8i_board_dt_compat,
> MACHINE_END
>
> +void __init sun8i_cntvoff_init(void)
> +{
> + smp_init_cntvoff();

Can't this be moved to the SMP setup code?

> +}
> +
> +static const char * const sun8i_cntvoff_board_dt_compat[] = {
> + "allwinner,sun8i-a83t",
> + NULL,
> +};
> +
> +DT_MACHINE_START(SUN8I_CNTVOFF_DT, "Allwinner sun8i boards needing cntvoff")

All of the SoCs need CNTVOFF, so that doesn't really make sense. Why
not just calling it for what it is: an A83t?

Maxime

--
Maxime Ripard, Bootlin (formerly Free Electrons)
Embedded Linux and Kernel engineering
https://bootlin.com


Attachments:
(No filename) (2.33 kB)
signature.asc (849.00 B)
Download all attachments

2018-04-03 11:16:04

by kernel test robot

[permalink] [raw]
Subject: Re: [PATCH v5 08/13] ARM: sunxi: Add initialization of CNTVOFF

Hi Myl?ne,

Thank you for the patch! Yet something to improve:

[auto build test ERROR on arm-soc/for-next]
[also build test ERROR on next-20180403]
[cannot apply to v4.16]
[if your patch is applied to the wrong git tree, please drop us a note to help improve the system]

url: https://github.com/0day-ci/linux/commits/Myl-ne-Josserand/Sunxi-Add-SMP-support-on-A83T/20180403-143751
base: https://git.kernel.org/pub/scm/linux/kernel/git/arm/arm-soc.git for-next
config: arm-arm67 (attached as .config)
compiler: arm-linux-gnueabi-gcc (Debian 7.2.0-11) 7.2.0
reproduce:
wget https://raw.githubusercontent.com/intel/lkp-tests/master/sbin/make.cross -O ~/bin/make.cross
chmod +x ~/bin/make.cross
# save the attached .config to linux build tree
make.cross ARCH=arm

All errors (new ones prefixed by >>):

arch/arm/mach-sunxi/sunxi.o: In function `sun8i_cntvoff_init':
>> sunxi.c:(.init.text+0x3c): undefined reference to `smp_init_cntvoff'

---
0-DAY kernel test infrastructure Open Source Technology Center
https://lists.01.org/pipermail/kbuild-all Intel Corporation


Attachments:
(No filename) (1.14 kB)
.config.gz (31.02 kB)
Download all attachments

2018-04-03 19:53:31

by Mylène Josserand

[permalink] [raw]
Subject: Re: [PATCH v5 05/13] ARM: dts: sun8i: Add R_CPUCFG device node for the A83T dtsi

Hello,

On Tue, 3 Apr 2018 17:07:06 +0800
Chen-Yu Tsai <[email protected]> wrote:

> On Tue, Apr 3, 2018 at 2:18 PM, Mylène Josserand
> <[email protected]> wrote:
> > 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 32992afa0b12..85f14f4ebeed 100644
> > --- a/arch/arm/boot/dts/sun8i-a83t.dtsi
> > +++ b/arch/arm/boot/dts/sun8i-a83t.dtsi
> > @@ -933,6 +933,11 @@
> > #reset-cells = <1>;
> > };
> >
> > + r_cpucfg@1f01c00 {
> > + compatible = "allwinner,sun8i-a83t-r-cpucfg";
> > + reg = <0x1f01c00 0x100>;
>
> The memory map says that its size is 0x400.
>
> ChenYu

True, thank you for the correction.

Best regards,

--
Mylène Josserand, Bootlin (formerly Free Electrons)
Embedded Linux and Kernel engineering
http://bootlin.com

2018-04-03 19:59:21

by Florian Fainelli

[permalink] [raw]
Subject: Re: [PATCH v5 01/13] ARM: move cputype definitions into another file

On 04/02/2018 11:52 PM, Chen-Yu Tsai wrote:
> On Tue, Apr 3, 2018 at 2:18 PM, Mylène Josserand
> <[email protected]> wrote:
>> To add the support for SMP on sun8i-a83t, we will use some
>> definitions in an assembly file so move definitions into
>> another file to separate C functions and macro defintions.
>
> Instead of moving the definitions, you could guard all the C
> stuff in "#ifndef __ASSEMBLY__". AFAIK a few header files do that.

Which is effectively what this patch does (still waiting for an ACK):

https://patchwork.kernel.org/patch/10239855/

>
> ChenYu
>
>> Signed-off-by: Mylène Josserand <[email protected]>
>> ---
>> arch/arm/include/asm/cputype.h | 94 +-----------------------------------
>> arch/arm/include/asm/cputype_def.h | 98 ++++++++++++++++++++++++++++++++++++++
> --
> To unsubscribe from this list: send the line "unsubscribe devicetree" in
> the body of a message to [email protected]
> More majordomo info at http://vger.kernel.org/majordomo-info.html
>


--
Florian

2018-04-03 20:08:03

by Mylène Josserand

[permalink] [raw]
Subject: Re: [PATCH v5 08/13] ARM: sunxi: Add initialization of CNTVOFF

Hello,

Thank you for the review.

On Tue, 3 Apr 2018 11:12:18 +0200
Maxime Ripard <[email protected]> wrote:

> On Tue, Apr 03, 2018 at 08:18:31AM +0200, Mylène Josserand wrote:
> > Add the initialization of CNTVOFF for sun8i-a83t.
> >
> > For boot CPU, Create a new machine that handles this
> > function's call in an "init_early" callback.
> > For secondary CPUs, add this function into secondary_startup
> > assembly entry.
> >
> > Signed-off-by: Mylène Josserand <[email protected]>
> > ---
> > arch/arm/mach-sunxi/headsmp.S | 1 +
> > arch/arm/mach-sunxi/sunxi.c | 18 +++++++++++++++++-
> > 2 files changed, 18 insertions(+), 1 deletion(-)
> >
> > diff --git a/arch/arm/mach-sunxi/headsmp.S b/arch/arm/mach-sunxi/headsmp.S
> > index 79890fbe5613..b586b7cf803a 100644
> > --- a/arch/arm/mach-sunxi/headsmp.S
> > +++ b/arch/arm/mach-sunxi/headsmp.S
> > @@ -71,6 +71,7 @@ ENDPROC(sunxi_mc_smp_cluster_cache_enable)
> >
> > ENTRY(sunxi_mc_smp_secondary_startup)
> > bl sunxi_mc_smp_cluster_cache_enable
> > + bl smp_init_cntvoff
> > b secondary_startup
> > ENDPROC(sunxi_mc_smp_secondary_startup)
> >
> > diff --git a/arch/arm/mach-sunxi/sunxi.c b/arch/arm/mach-sunxi/sunxi.c
> > index 5e9602ce1573..090784108c0a 100644
> > --- a/arch/arm/mach-sunxi/sunxi.c
> > +++ b/arch/arm/mach-sunxi/sunxi.c
> > @@ -16,6 +16,7 @@
> > #include <linux/platform_device.h>
> >
> > #include <asm/mach/arch.h>
> > +#include <asm/smp_cntvoff.h>
> >
> > static const char * const sunxi_board_dt_compat[] = {
> > "allwinner,sun4i-a10",
> > @@ -62,7 +63,6 @@ MACHINE_END
> > static const char * const sun8i_board_dt_compat[] = {
> > "allwinner,sun8i-a23",
> > "allwinner,sun8i-a33",
> > - "allwinner,sun8i-a83t",
> > "allwinner,sun8i-h2-plus",
> > "allwinner,sun8i-h3",
> > "allwinner,sun8i-r40",
> > @@ -75,6 +75,22 @@ DT_MACHINE_START(SUN8I_DT, "Allwinner sun8i Family")
> > .dt_compat = sun8i_board_dt_compat,
> > MACHINE_END
> >
> > +void __init sun8i_cntvoff_init(void)
> > +{
> > + smp_init_cntvoff();
>
> Can't this be moved to the SMP setup code?

I tried to put it in the first lines of "sunxi_mc_smp_init" function
but it did not work. I tried to find some callbacks to have an
early "init" and I only found the "init_early"'s one. There is probably
another way to handle that so do not hesitate to tell me any ideas.

>
> > +}
> > +
> > +static const char * const sun8i_cntvoff_board_dt_compat[] = {
> > + "allwinner,sun8i-a83t",
> > + NULL,
> > +};
> > +
> > +DT_MACHINE_START(SUN8I_CNTVOFF_DT, "Allwinner sun8i boards needing cntvoff")
>
> All of the SoCs need CNTVOFF, so that doesn't really make sense. Why
> not just calling it for what it is: an A83t?

Sure, I will update it.

Best regards,

--
Mylène Josserand, Bootlin (formerly Free Electrons)
Embedded Linux and Kernel engineering
http://bootlin.com

2018-04-03 20:10:27

by Mylène Josserand

[permalink] [raw]
Subject: Re: [PATCH v5 11/13] ARM: sun9i: smp: Add is_sun9i field

Hi,

On Tue, 3 Apr 2018 16:48:41 +0800
Chen-Yu Tsai <[email protected]> wrote:

> On Tue, Apr 3, 2018 at 4:46 PM, Maxime Ripard <[email protected]> wrote:
> > On Tue, Apr 03, 2018 at 08:18:34AM +0200, Mylène Josserand wrote:
> >> To prepare the support of sun8i-a83t, add a field in the smp_data
> >> structure to enable the case of sun9i.
> >>
> >> Start to handle the differences between sun9i-a80 and sun8i-a83t
> >> by using this variable.
> >>
> >> Add an index to retrieve which structures we are using.
> >
> > This should have been in a separate commit, but maybe we can store a
> > pointer to the array cell we're using instead of always using the
> > index?
>
> Using a pointer would also avoid some of the code movement from the
> previous patch.
>
> ChenYu

Yep, I will use a pointer instead. Thanks for the correction.

Best regards,

--
Mylène Josserand, Bootlin (formerly Free Electrons)
Embedded Linux and Kernel engineering
http://bootlin.com

2018-04-03 20:22:45

by Mylène Josserand

[permalink] [raw]
Subject: Re: [PATCH v5 12/13] ARM: sun8i: smp: Add support for A83T

Hello Chen-Yu,

Thanks for your review.

On Tue, 3 Apr 2018 16:47:53 +0800
Chen-Yu Tsai <[email protected]> wrote:

> On Tue, Apr 3, 2018 at 2: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.
> > There is also a bit swap between sun8i-a83t and sun9i-a80 that must be
> > handled.
> >
> > Signed-off-by: Mylène Josserand <[email protected]>
> > ---
> > arch/arm/mach-sunxi/mc_smp.c | 120 +++++++++++++++++++++++++++++++++++++++++--
> > 1 file changed, 117 insertions(+), 3 deletions(-)
> >
> > diff --git a/arch/arm/mach-sunxi/mc_smp.c b/arch/arm/mach-sunxi/mc_smp.c
> > index 468a6c46bfc9..72e497dc43ac 100644
> > --- a/arch/arm/mach-sunxi/mc_smp.c
> > +++ b/arch/arm/mach-sunxi/mc_smp.c
> > @@ -55,22 +55,31 @@
> > #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))
> > +/* The power off register for clusters are different from a80 and a83t */
> > +#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 */
>
> You should mention it as A83T specific, since sun8i covers the
> entire Cortex-A7-based SoC family.

Thanks, Maxime already told me that, I tried to pay attention to change
every "sun8i" into "sun8i-a83t" but I missed that one.

>
> > +#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 *prcm_base;
> > static void __iomem *sram_b_smp_base;
> > +static void __iomem *r_cpucfg_base;
> > static int index;
> >
> > /*
> > @@ -81,6 +90,7 @@ struct sunxi_mc_smp_nodes {
> > struct device_node *prcm_node;
> > struct device_node *cpucfg_node;
> > struct device_node *sram_node;
> > + struct device_node *r_cpucfg_node;
> > };
> >
> > /* This structure holds SoC-specific bits tied to an enable-method string. */
> > @@ -94,6 +104,7 @@ extern void sunxi_mc_smp_secondary_startup(void);
> > extern void sunxi_mc_smp_resume(void);
> >
> > static int __init sun9i_a80_get_smp_nodes(struct sunxi_mc_smp_nodes *nodes);
> > +static int __init sun8i_a83t_get_smp_nodes(struct sunxi_mc_smp_nodes *nodes);
> >
> > static const struct sunxi_mc_smp_data sunxi_mc_smp_data[] __initconst = {
> > {
> > @@ -101,6 +112,11 @@ static const struct sunxi_mc_smp_data sunxi_mc_smp_data[] __initconst = {
> > .get_smp_nodes = sun9i_a80_get_smp_nodes,
> > .is_sun9i = true,
> > },
> > + {
> > + .enable_method = "allwinner,sun8i-a83t-smp",
> > + .get_smp_nodes = sun8i_a83t_get_smp_nodes,
> > + .is_sun9i = false,
> > + },
> > };
> >
> > static bool sunxi_core_is_cortex_a15(unsigned int core, unsigned int cluster)
> > @@ -188,6 +204,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) {
>
> Please check against is_sun9i, since this is A83T specific. My point
> is we should be able to see clearly what parts of the code are shared,
> and what parts are SoC specific.

Okay, it is fine for me, I will update that and I will use a "is_sun8i"
as Maxime mentioned in previous series.

>
> > + /* 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));
> > @@ -211,17 +237,38 @@ 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 (!sunxi_mc_smp_data[index].is_sun9i) {
> > + 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);
> >
> > + /* Handle A83T bit swap */
> > + if (!sunxi_mc_smp_data[index].is_sun9i) {
> > + 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) {
>
> Same here.

ack

>
> > + 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);
> > @@ -243,6 +290,14 @@ static int sunxi_cluster_powerup(unsigned int cluster)
> > if (cluster >= SUNXI_NR_CLUSTERS)
> > return -EINVAL;
> >
> > + /* For A83T, assert cluster cores resets */
> > + if (!sunxi_mc_smp_data[index].is_sun9i) {
>
> Here it is correct.

ack

>
> > + 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;
> > @@ -253,6 +308,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) {
>
> Same here.

ack

>
> > + 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;
> > @@ -285,6 +350,8 @@ static int sunxi_cluster_powerup(unsigned int cluster)
> > reg = readl(prcm_base + PRCM_PWROFF_GATING_REG(cluster));
> > if (sunxi_mc_smp_data[index].is_sun9i)
> > reg &= ~PRCM_PWROFF_GATING_REG_CLUSTER_SUN9I;
> > + else
> > + reg &= ~PRCM_PWROFF_GATING_REG_CLUSTER_SUN8I;
> > writel(reg, prcm_base + PRCM_PWROFF_GATING_REG(cluster));
> > udelay(20);
> >
> > @@ -483,6 +550,8 @@ static int sunxi_cluster_powerdown(unsigned int cluster)
> > reg = readl(prcm_base + PRCM_PWROFF_GATING_REG(cluster));
> > if (sunxi_mc_smp_data[index].is_sun9i)
> > reg |= PRCM_PWROFF_GATING_REG_CLUSTER_SUN9I;
> > + else
> > + reg |= PRCM_PWROFF_GATING_REG_CLUSTER_SUN8I;
> > writel(reg, prcm_base + PRCM_PWROFF_GATING_REG(cluster));
> > udelay(20);
> >
> > @@ -564,8 +633,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-a80 */
> > + if (!sunxi_mc_smp_data[index].is_sun9i)
> > + if (cpu == 0)
> > + return false;
>
> Once the above is fixed, this patch is
>
> Reviewed-by: Chen-Yu Tsai <[email protected]>

Thanks!

>
>
> Would it be possible for you to implement CPU0 hotplug? You needn't
> do so as part of this patch or even this series. I disassembled the
> BROM just now, and it is much simpler compared to the A80:
>
> On boot, if running on cpu0, check magic register at 0x01f01dac
> for magic value 0xfa50392f. If found, follow secondary startup
> like other cores, otherwise continue normal boot procedure.
> As you can see they use a register in CPUS-CFG instead of
> secure SRAM this time.

Sure, I will do it with pleasure. Thank you for the explanation, I
will try that and I will let you know if it is working with this setup.

Best regards,

Mylène

--
Mylène Josserand, Bootlin (formerly Free Electrons)
Embedded Linux and Kernel engineering
http://bootlin.com

>
> > return true;
> > }
> > #endif
> > @@ -645,6 +718,7 @@ static void __init sunxi_mc_smp_put_nodes(struct sunxi_mc_smp_nodes *nodes)
> > of_node_put(nodes->prcm_node);
> > of_node_put(nodes->cpucfg_node);
> > of_node_put(nodes->sram_node);
> > + of_node_put(nodes->r_cpucfg_node);
> > memset(nodes, 0, sizeof(*nodes));
> > }
> >
> > @@ -674,6 +748,32 @@ static int __init sun9i_a80_get_smp_nodes(struct sunxi_mc_smp_nodes *nodes)
> > return 0;
> > }
> >
> > +static int __init sun8i_a83t_get_smp_nodes(struct sunxi_mc_smp_nodes *nodes)
> > +{
> > + nodes->prcm_node = of_find_compatible_node(NULL, NULL,
> > + "allwinner,sun8i-a83t-r-ccu");
> > + if (!nodes->prcm_node) {
> > + pr_err("%s: PRCM not available\n", __func__);
> > + return -ENODEV;
> > + }
> > +
> > + nodes->cpucfg_node = of_find_compatible_node(NULL, NULL,
> > + "allwinner,sun8i-a83t-cpucfg");
> > + if (!nodes->cpucfg_node) {
> > + pr_err("%s: CPUCFG not available\n", __func__);
> > + return -ENODEV;
> > + }
> > +
> > + nodes->r_cpucfg_node = of_find_compatible_node(NULL, NULL,
> > + "allwinner,sun8i-a83t-r-cpucfg");
> > + if (!nodes->r_cpucfg_node) {
> > + pr_err("%s: RCPUCFG not available\n", __func__);
> > + return -ENODEV;
> > + }
> > +
> > + return 0;
> > +}
> > +
> > static int __init sunxi_mc_smp_init(void)
> > {
> > struct sunxi_mc_smp_nodes nodes = { 0 };
> > @@ -752,6 +852,15 @@ static int __init sunxi_mc_smp_init(void)
> > pr_err("%s: failed to map secure SRAM\n", __func__);
> > goto err_unmap_release_cpucfg;
> > }
> > + } else {
> > + r_cpucfg_base = of_io_request_and_map(nodes.r_cpucfg_node,
> > + 0, "sunxi-mc-smp");
> > + if (IS_ERR(r_cpucfg_base)) {
> > + ret = PTR_ERR(r_cpucfg_base);
> > + pr_err("%s: failed to map R-CPUCFG registers\n",
> > + __func__);
> > + goto err_unmap_release_cpucfg;
> > + }
> > }
> >
> > /* Configure CCI-400 for boot cluster */
> > @@ -759,7 +868,7 @@ static int __init sunxi_mc_smp_init(void)
> > if (ret) {
> > pr_err("%s: failed to configure boot cluster: %d\n",
> > __func__, ret);
> > - goto err_unmap_release_secure_sram;
> > + goto err_unmap_release_sram_rcpucfg;
> > }
> >
> > /* We don't need the device nodes anymore */
> > @@ -768,6 +877,8 @@ static int __init sunxi_mc_smp_init(void)
> > /* Set the hardware entry point address */
> > if (sunxi_mc_smp_data[index].is_sun9i)
> > addr = prcm_base + PRCM_CPU_SOFT_ENTRY_REG;
> > + else
> > + addr = r_cpucfg_base + R_CPUCFG_CPU_SOFT_ENTRY_REG;
> > writel(__pa_symbol(sunxi_mc_smp_secondary_startup), addr);
> >
> > /* Actually enable multi cluster SMP */
> > @@ -777,10 +888,13 @@ static int __init sunxi_mc_smp_init(void)
> >
> > return 0;
> >
> > -err_unmap_release_secure_sram:
> > +err_unmap_release_sram_rcpucfg:
> > if (sunxi_mc_smp_data[index].is_sun9i) {
> > iounmap(sram_b_smp_base);
> > of_address_to_resource(nodes.sram_node, 0, &res);
> > + } else {
> > + iounmap(r_cpucfg_base);
> > + of_address_to_resource(nodes.r_cpucfg_node, 0, &res);
> > }
> > release_mem_region(res.start, resource_size(&res));
> > err_unmap_release_cpucfg:
> > --
> > 2.11.0
> >

2018-04-04 07:46:47

by Maxime Ripard

[permalink] [raw]
Subject: Re: [PATCH v5 08/13] ARM: sunxi: Add initialization of CNTVOFF

On Tue, Apr 03, 2018 at 10:06:28PM +0200, Myl?ne Josserand wrote:
> Hello,
>
> Thank you for the review.
>
> On Tue, 3 Apr 2018 11:12:18 +0200
> Maxime Ripard <[email protected]> wrote:
>
> > On Tue, Apr 03, 2018 at 08:18:31AM +0200, Myl?ne Josserand wrote:
> > > Add the initialization of CNTVOFF for sun8i-a83t.
> > >
> > > For boot CPU, Create a new machine that handles this
> > > function's call in an "init_early" callback.
> > > For secondary CPUs, add this function into secondary_startup
> > > assembly entry.
> > >
> > > Signed-off-by: Myl?ne Josserand <[email protected]>
> > > ---
> > > arch/arm/mach-sunxi/headsmp.S | 1 +
> > > arch/arm/mach-sunxi/sunxi.c | 18 +++++++++++++++++-
> > > 2 files changed, 18 insertions(+), 1 deletion(-)
> > >
> > > diff --git a/arch/arm/mach-sunxi/headsmp.S b/arch/arm/mach-sunxi/headsmp.S
> > > index 79890fbe5613..b586b7cf803a 100644
> > > --- a/arch/arm/mach-sunxi/headsmp.S
> > > +++ b/arch/arm/mach-sunxi/headsmp.S
> > > @@ -71,6 +71,7 @@ ENDPROC(sunxi_mc_smp_cluster_cache_enable)
> > >
> > > ENTRY(sunxi_mc_smp_secondary_startup)
> > > bl sunxi_mc_smp_cluster_cache_enable
> > > + bl smp_init_cntvoff
> > > b secondary_startup
> > > ENDPROC(sunxi_mc_smp_secondary_startup)
> > >
> > > diff --git a/arch/arm/mach-sunxi/sunxi.c b/arch/arm/mach-sunxi/sunxi.c
> > > index 5e9602ce1573..090784108c0a 100644
> > > --- a/arch/arm/mach-sunxi/sunxi.c
> > > +++ b/arch/arm/mach-sunxi/sunxi.c
> > > @@ -16,6 +16,7 @@
> > > #include <linux/platform_device.h>
> > >
> > > #include <asm/mach/arch.h>
> > > +#include <asm/smp_cntvoff.h>
> > >
> > > static const char * const sunxi_board_dt_compat[] = {
> > > "allwinner,sun4i-a10",
> > > @@ -62,7 +63,6 @@ MACHINE_END
> > > static const char * const sun8i_board_dt_compat[] = {
> > > "allwinner,sun8i-a23",
> > > "allwinner,sun8i-a33",
> > > - "allwinner,sun8i-a83t",
> > > "allwinner,sun8i-h2-plus",
> > > "allwinner,sun8i-h3",
> > > "allwinner,sun8i-r40",
> > > @@ -75,6 +75,22 @@ DT_MACHINE_START(SUN8I_DT, "Allwinner sun8i Family")
> > > .dt_compat = sun8i_board_dt_compat,
> > > MACHINE_END
> > >
> > > +void __init sun8i_cntvoff_init(void)
> > > +{
> > > + smp_init_cntvoff();
> >
> > Can't this be moved to the SMP setup code?
>
> I tried to put it in the first lines of "sunxi_mc_smp_init" function
> but it did not work. I tried to find some callbacks to have an
> early "init" and I only found the "init_early"'s one. There is probably
> another way to handle that so do not hesitate to tell me any ideas.

It's hard to say without more context about why it doesn't work. Have
you checked the order between early_initcall, the timer initialization
function and init_early?

> > > +}
> > > +
> > > +static const char * const sun8i_cntvoff_board_dt_compat[] = {
> > > + "allwinner,sun8i-a83t",
> > > + NULL,
> > > +};
> > > +
> > > +DT_MACHINE_START(SUN8I_CNTVOFF_DT, "Allwinner sun8i boards needing cntvoff")
> >
> > All of the SoCs need CNTVOFF, so that doesn't really make sense. Why
> > not just calling it for what it is: an A83t?
>
> Sure, I will update it.

Looking back at that code, I guess you want to change also the
smp_init_cntvoff function. It's not really related to SMP either.

Maxime

--
Maxime Ripard, Bootlin (formerly Free Electrons)
Embedded Linux and Kernel engineering
https://bootlin.com


Attachments:
(No filename) (3.40 kB)
signature.asc (849.00 B)
Download all attachments

2018-04-04 13:03:45

by Marc Zyngier

[permalink] [raw]
Subject: Re: [PATCH v5 07/13] ARM: smp: Add initialization of CNTVOFF

Hi Mylène,

On 03/04/18 07:18, Mylène Josserand wrote:
> The CNTVOFF register from arch timer is uninitialized.
> It should be done by the bootloader but it is currently not the case,
> even for boot CPU because this SoC is booting in secure mode.
> It leads to an random offset value meaning that each CPU will have a
> different time, which isn't working very well.
>
> Add assembly code used for boot CPU and secondary CPU cores to make
> sure that the CNTVOFF register is initialized. Because this code can
> be used by different platforms, add this assembly file in ARM's common
> folder.
>
> Signed-off-by: Mylène Josserand <[email protected]>
> ---
> arch/arm/common/Makefile | 1 +
> arch/arm/common/smp_cntvoff.S | 35 +++++++++++++++++++++++++++++++++++
> arch/arm/include/asm/smp_cntvoff.h | 8 ++++++++
> 3 files changed, 44 insertions(+)
> create mode 100644 arch/arm/common/smp_cntvoff.S
> create mode 100644 arch/arm/include/asm/smp_cntvoff.h
>
> diff --git a/arch/arm/common/Makefile b/arch/arm/common/Makefile
> index 70b4a14ed993..83117deb86c8 100644
> --- a/arch/arm/common/Makefile
> +++ b/arch/arm/common/Makefile
> @@ -10,6 +10,7 @@ obj-$(CONFIG_DMABOUNCE) += dmabounce.o
> obj-$(CONFIG_SHARP_LOCOMO) += locomo.o
> obj-$(CONFIG_SHARP_PARAM) += sharpsl_param.o
> obj-$(CONFIG_SHARP_SCOOP) += scoop.o
> +obj-$(CONFIG_SMP) += smp_cntvoff.o
> obj-$(CONFIG_PCI_HOST_ITE8152) += it8152.o
> obj-$(CONFIG_MCPM) += mcpm_head.o mcpm_entry.o mcpm_platsmp.o vlock.o
> CFLAGS_REMOVE_mcpm_entry.o = -pg
> diff --git a/arch/arm/common/smp_cntvoff.S b/arch/arm/common/smp_cntvoff.S
> new file mode 100644
> index 000000000000..65ed199a50fe
> --- /dev/null
> +++ b/arch/arm/common/smp_cntvoff.S
> @@ -0,0 +1,35 @@
> +// SPDX-License-Identifier: GPL-2.0
> +/*
> + * Copyright (c) 2018 Chen-Yu Tsai
> + * Copyright (c) 2018 Bootlin
> + *
> + * Chen-Yu Tsai <[email protected]>
> + * Mylène Josserand <[email protected]>

Given that this is literally lifted from shmobile_init_cntvoff, the
whole attribution is a bit dubious.

> + *
> + * SMP support for sunxi based systems with Cortex A7/A15

That's not restricted to sunxi, is it?

> + *
> + */
> +
> +#include <linux/linkage.h>
> +#include <asm/assembler.h>
> +
> +ENTRY(smp_init_cntvoff)

The name doesn't quite reflect the usage constraints. This will only
work if used from secure, and is UNPREDICTABLE otherwise (see the CPS
instruction). Also, the "smp" prefix is quite misleading, as it only
affects the current CPU, and not any other.

How about secure_cntvoff_init instead? Same thing for the file name.

> + .arch armv7-a
> + /*
> + * CNTVOFF has to be initialized either from non-secure Hypervisor
> + * mode or secure Monitor mode with SCR.NS==1. If TrustZone is enabled
> + * then it should be handled by the secure code
> + */
> + cps #MON_MODE
> + mrc p15, 0, r1, c1, c1, 0 /* Get Secure Config */
> + orr r0, r1, #1
> + mcr p15, 0, r0, c1, c1, 0 /* Set Non Secure bit */
> + isb
> + mov r0, #0
> + mcrr p15, 4, r0, r0, c14 /* CNTVOFF = 0 */
> + isb
> + mcr p15, 0, r1, c1, c1, 0 /* Set Secure bit */
> + isb
> + cps #SVC_MODE
> + ret lr
> +ENDPROC(smp_init_cntvoff)
> diff --git a/arch/arm/include/asm/smp_cntvoff.h b/arch/arm/include/asm/smp_cntvoff.h
> new file mode 100644
> index 000000000000..59a95f7604ee
> --- /dev/null
> +++ b/arch/arm/include/asm/smp_cntvoff.h
> @@ -0,0 +1,8 @@
> +// SPDX-License-Identifier: GPL-2.0
> +
> +#ifndef __ASMARM_ARCH_CNTVOFF_H
> +#define __ASMARM_ARCH_CNTVOFF_H
> +
> +extern void smp_init_cntvoff(void);
> +
> +#endif
>

It'd be good to take this opportunity to refactor the shmobile code.

Thanks,

M.
--
Jazz is not dead. It just smells funny...

2018-04-04 13:50:53

by Mylène Josserand

[permalink] [raw]
Subject: Re: [PATCH v5 01/13] ARM: move cputype definitions into another file

Hello Florian,

On Tue, 3 Apr 2018 12:56:30 -0700
Florian Fainelli <[email protected]> wrote:

> On 04/02/2018 11:52 PM, Chen-Yu Tsai wrote:
> > On Tue, Apr 3, 2018 at 2:18 PM, Mylène Josserand
> > <[email protected]> wrote:
> >> To add the support for SMP on sun8i-a83t, we will use some
> >> definitions in an assembly file so move definitions into
> >> another file to separate C functions and macro defintions.
> >
> > Instead of moving the definitions, you could guard all the C
> > stuff in "#ifndef __ASSEMBLY__". AFAIK a few header files do that.
>
> Which is effectively what this patch does (still waiting for an ACK):
>
> https://patchwork.kernel.org/patch/10239855/

Thanks for pointing out this existing patch.
I will use it for my next series and add my Tested-by (once tested, of
course).

Best regards,

--
Mylène Josserand, Bootlin (formerly Free Electrons)
Embedded Linux and Kernel engineering
http://bootlin.com

>
> >
> > ChenYu
> >
> >> Signed-off-by: Mylène Josserand <[email protected]>
> >> ---
> >> arch/arm/include/asm/cputype.h | 94 +-----------------------------------
> >> arch/arm/include/asm/cputype_def.h | 98 ++++++++++++++++++++++++++++++++++++++
> > --
> > To unsubscribe from this list: send the line "unsubscribe devicetree" in
> > the body of a message to [email protected]
> > More majordomo info at http://vger.kernel.org/majordomo-info.html
> >
>
>

2018-04-04 14:01:59

by Mylène Josserand

[permalink] [raw]
Subject: Re: [PATCH v5 07/13] ARM: smp: Add initialization of CNTVOFF

Hi Marc,

Thank you for the review.

On Wed, 4 Apr 2018 14:01:48 +0100
Marc Zyngier <[email protected]> wrote:

> Hi Mylène,
>
> On 03/04/18 07:18, Mylène Josserand wrote:
> > The CNTVOFF register from arch timer is uninitialized.
> > It should be done by the bootloader but it is currently not the case,
> > even for boot CPU because this SoC is booting in secure mode.
> > It leads to an random offset value meaning that each CPU will have a
> > different time, which isn't working very well.
> >
> > Add assembly code used for boot CPU and secondary CPU cores to make
> > sure that the CNTVOFF register is initialized. Because this code can
> > be used by different platforms, add this assembly file in ARM's common
> > folder.
> >
> > Signed-off-by: Mylène Josserand <[email protected]>
> > ---
> > arch/arm/common/Makefile | 1 +
> > arch/arm/common/smp_cntvoff.S | 35 +++++++++++++++++++++++++++++++++++
> > arch/arm/include/asm/smp_cntvoff.h | 8 ++++++++
> > 3 files changed, 44 insertions(+)
> > create mode 100644 arch/arm/common/smp_cntvoff.S
> > create mode 100644 arch/arm/include/asm/smp_cntvoff.h
> >
> > diff --git a/arch/arm/common/Makefile b/arch/arm/common/Makefile
> > index 70b4a14ed993..83117deb86c8 100644
> > --- a/arch/arm/common/Makefile
> > +++ b/arch/arm/common/Makefile
> > @@ -10,6 +10,7 @@ obj-$(CONFIG_DMABOUNCE) += dmabounce.o
> > obj-$(CONFIG_SHARP_LOCOMO) += locomo.o
> > obj-$(CONFIG_SHARP_PARAM) += sharpsl_param.o
> > obj-$(CONFIG_SHARP_SCOOP) += scoop.o
> > +obj-$(CONFIG_SMP) += smp_cntvoff.o
> > obj-$(CONFIG_PCI_HOST_ITE8152) += it8152.o
> > obj-$(CONFIG_MCPM) += mcpm_head.o mcpm_entry.o mcpm_platsmp.o vlock.o
> > CFLAGS_REMOVE_mcpm_entry.o = -pg
> > diff --git a/arch/arm/common/smp_cntvoff.S b/arch/arm/common/smp_cntvoff.S
> > new file mode 100644
> > index 000000000000..65ed199a50fe
> > --- /dev/null
> > +++ b/arch/arm/common/smp_cntvoff.S
> > @@ -0,0 +1,35 @@
> > +// SPDX-License-Identifier: GPL-2.0
> > +/*
> > + * Copyright (c) 2018 Chen-Yu Tsai
> > + * Copyright (c) 2018 Bootlin
> > + *
> > + * Chen-Yu Tsai <[email protected]>
> > + * Mylène Josserand <[email protected]>
>
> Given that this is literally lifted from shmobile_init_cntvoff, the
> whole attribution is a bit dubious.

Yes, sorry, I will fix that.

>
> > + *
> > + * SMP support for sunxi based systems with Cortex A7/A15
>
> That's not restricted to sunxi, is it?

Nope, I will update this line too (bad copy-paste from
sunxi/headsmp.S...)

>
> > + *
> > + */
> > +
> > +#include <linux/linkage.h>
> > +#include <asm/assembler.h>
> > +
> > +ENTRY(smp_init_cntvoff)
>
> The name doesn't quite reflect the usage constraints. This will only
> work if used from secure, and is UNPREDICTABLE otherwise (see the CPS
> instruction). Also, the "smp" prefix is quite misleading, as it only
> affects the current CPU, and not any other.
>
> How about secure_cntvoff_init instead? Same thing for the file name.

Sure, this name is fine for me.

>
> > + .arch armv7-a
> > + /*
> > + * CNTVOFF has to be initialized either from non-secure Hypervisor
> > + * mode or secure Monitor mode with SCR.NS==1. If TrustZone is enabled
> > + * then it should be handled by the secure code
> > + */
> > + cps #MON_MODE
> > + mrc p15, 0, r1, c1, c1, 0 /* Get Secure Config */
> > + orr r0, r1, #1
> > + mcr p15, 0, r0, c1, c1, 0 /* Set Non Secure bit */
> > + isb
> > + mov r0, #0
> > + mcrr p15, 4, r0, r0, c14 /* CNTVOFF = 0 */
> > + isb
> > + mcr p15, 0, r1, c1, c1, 0 /* Set Secure bit */
> > + isb
> > + cps #SVC_MODE
> > + ret lr
> > +ENDPROC(smp_init_cntvoff)
> > diff --git a/arch/arm/include/asm/smp_cntvoff.h b/arch/arm/include/asm/smp_cntvoff.h
> > new file mode 100644
> > index 000000000000..59a95f7604ee
> > --- /dev/null
> > +++ b/arch/arm/include/asm/smp_cntvoff.h
> > @@ -0,0 +1,8 @@
> > +// SPDX-License-Identifier: GPL-2.0
> > +
> > +#ifndef __ASMARM_ARCH_CNTVOFF_H
> > +#define __ASMARM_ARCH_CNTVOFF_H
> > +
> > +extern void smp_init_cntvoff(void);
> > +
> > +#endif
> >
>
> It'd be good to take this opportunity to refactor the shmobile code.

I can do it in this series but I do not have any shmobile platforms so
I will not be able to test my modifications (only compilation).

If someone can test it for me (who?), it is okay for me to refactor this
code :)

Best regards,

--
Mylène Josserand, Bootlin (formerly Free Electrons)
Embedded Linux and Kernel engineering
http://bootlin.com

2018-04-04 14:32:10

by Marc Zyngier

[permalink] [raw]
Subject: Re: [PATCH v5 07/13] ARM: smp: Add initialization of CNTVOFF

On Wed, 04 Apr 2018 14:59:09 +0100,
Myl?ne Josserand wrote:

> > It'd be good to take this opportunity to refactor the shmobile code.
>
> I can do it in this series but I do not have any shmobile platforms so
> I will not be able to test my modifications (only compilation).
>
> If someone can test it for me (who?), it is okay for me to refactor this
> code :)

I guess you could Cc the shmobile folks (Geert Uytterhoeven, Simon
Horman), and get them to review/test the changes.

Thanks,

M.

--
Jazz is not dead, it just smell funny.

2018-04-08 09:47:13

by Mylène Josserand

[permalink] [raw]
Subject: Re: [PATCH v5 08/13] ARM: sunxi: Add initialization of CNTVOFF

Hello Maxime,

On Wed, 4 Apr 2018 09:45:15 +0200
Maxime Ripard <[email protected]> wrote:

> On Tue, Apr 03, 2018 at 10:06:28PM +0200, Mylène Josserand wrote:
> > Hello,
> >
> > Thank you for the review.
> >
> > On Tue, 3 Apr 2018 11:12:18 +0200
> > Maxime Ripard <[email protected]> wrote:
> >
> > > On Tue, Apr 03, 2018 at 08:18:31AM +0200, Mylène Josserand wrote:
> > > > Add the initialization of CNTVOFF for sun8i-a83t.
> > > >
> > > > For boot CPU, Create a new machine that handles this
> > > > function's call in an "init_early" callback.
> > > > For secondary CPUs, add this function into secondary_startup
> > > > assembly entry.
> > > >
> > > > Signed-off-by: Mylène Josserand <[email protected]>
> > > > ---
> > > > arch/arm/mach-sunxi/headsmp.S | 1 +
> > > > arch/arm/mach-sunxi/sunxi.c | 18 +++++++++++++++++-
> > > > 2 files changed, 18 insertions(+), 1 deletion(-)
> > > >
> > > > diff --git a/arch/arm/mach-sunxi/headsmp.S b/arch/arm/mach-sunxi/headsmp.S
> > > > index 79890fbe5613..b586b7cf803a 100644
> > > > --- a/arch/arm/mach-sunxi/headsmp.S
> > > > +++ b/arch/arm/mach-sunxi/headsmp.S
> > > > @@ -71,6 +71,7 @@ ENDPROC(sunxi_mc_smp_cluster_cache_enable)
> > > >
> > > > ENTRY(sunxi_mc_smp_secondary_startup)
> > > > bl sunxi_mc_smp_cluster_cache_enable
> > > > + bl smp_init_cntvoff
> > > > b secondary_startup
> > > > ENDPROC(sunxi_mc_smp_secondary_startup)
> > > >
> > > > diff --git a/arch/arm/mach-sunxi/sunxi.c b/arch/arm/mach-sunxi/sunxi.c
> > > > index 5e9602ce1573..090784108c0a 100644
> > > > --- a/arch/arm/mach-sunxi/sunxi.c
> > > > +++ b/arch/arm/mach-sunxi/sunxi.c
> > > > @@ -16,6 +16,7 @@
> > > > #include <linux/platform_device.h>
> > > >
> > > > #include <asm/mach/arch.h>
> > > > +#include <asm/smp_cntvoff.h>
> > > >
> > > > static const char * const sunxi_board_dt_compat[] = {
> > > > "allwinner,sun4i-a10",
> > > > @@ -62,7 +63,6 @@ MACHINE_END
> > > > static const char * const sun8i_board_dt_compat[] = {
> > > > "allwinner,sun8i-a23",
> > > > "allwinner,sun8i-a33",
> > > > - "allwinner,sun8i-a83t",
> > > > "allwinner,sun8i-h2-plus",
> > > > "allwinner,sun8i-h3",
> > > > "allwinner,sun8i-r40",
> > > > @@ -75,6 +75,22 @@ DT_MACHINE_START(SUN8I_DT, "Allwinner sun8i Family")
> > > > .dt_compat = sun8i_board_dt_compat,
> > > > MACHINE_END
> > > >
> > > > +void __init sun8i_cntvoff_init(void)
> > > > +{
> > > > + smp_init_cntvoff();
> > >
> > > Can't this be moved to the SMP setup code?
> >
> > I tried to put it in the first lines of "sunxi_mc_smp_init" function
> > but it did not work. I tried to find some callbacks to have an
> > early "init" and I only found the "init_early"'s one. There is probably
> > another way to handle that so do not hesitate to tell me any ideas.
>
> It's hard to say without more context about why it doesn't work. Have
> you checked the order between early_initcall, the timer initialization
> function and init_early?
>

Yes, I tested it. I wanted to test it again to give more context. Here
is the boot log: http://code.bulix.org/n1x864-315948?raw
I added printk in the 3 functions (with "===========>").

The "init_early" and "sun6i_timer" are executed before
"arch_timer_of_init" (which is the function that parses the DT property
that we used previously "arm,cpu-registers-not-fw-configured").
The "sunxi_mc_smp_init" function is called later so I guess that is why
it is not working in that case.

> > > > +}
> > > > +
> > > > +static const char * const sun8i_cntvoff_board_dt_compat[] = {
> > > > + "allwinner,sun8i-a83t",
> > > > + NULL,
> > > > +};
> > > > +
> > > > +DT_MACHINE_START(SUN8I_CNTVOFF_DT, "Allwinner sun8i boards needing cntvoff")
> > >
> > > All of the SoCs need CNTVOFF, so that doesn't really make sense. Why
> > > not just calling it for what it is: an A83t?
> >
> > Sure, I will update it.
>
> Looking back at that code, I guess you want to change also the
> smp_init_cntvoff function. It's not really related to SMP either.

Yep, I will update it in next version.

Thanks,

Best regards,

--
Mylène Josserand, Bootlin (formerly Free Electrons)
Embedded Linux and Kernel engineering
http://bootlin.com

2018-04-09 08:32:01

by Geert Uytterhoeven

[permalink] [raw]
Subject: Re: [PATCH v5 07/13] ARM: smp: Add initialization of CNTVOFF

On Wed, Apr 4, 2018 at 4:30 PM, Marc Zyngier <[email protected]> wrote:
> On Wed, 04 Apr 2018 14:59:09 +0100,
> Mylčne Josserand wrote:

[Marc: stuck in ISO-8859-1? ;-]

>> > It'd be good to take this opportunity to refactor the shmobile code.
>>
>> I can do it in this series but I do not have any shmobile platforms so
>> I will not be able to test my modifications (only compilation).
>>
>> If someone can test it for me (who?), it is okay for me to refactor this
>> code :)
>
> I guess you could Cc the shmobile folks (Geert Uytterhoeven, Simon
> Horman), and get them to review/test the changes.

Correct. I can test on a remote R-Car E2 ALT board that needs it.

P.S. Interestingly, none of the Renesas CA15 SoCs seem to suffer from it,
only CA7.

Gr{oetje,eeting}s,

Geert

--
Geert Uytterhoeven -- There's lots of Linux beyond ia32 -- [email protected]

In personal conversations with technical people, I call myself a hacker. But
when I'm talking to journalists I just say "programmer" or something like that.
-- Linus Torvalds

2018-04-09 09:09:23

by Marc Zyngier

[permalink] [raw]
Subject: Re: [PATCH v5 07/13] ARM: smp: Add initialization of CNTVOFF

On 09/04/18 09:24, Geert Uytterhoeven wrote:
> On Wed, Apr 4, 2018 at 4:30 PM, Marc Zyngier <[email protected]> wrote:
>> On Wed, 04 Apr 2018 14:59:09 +0100,
>> Mylčne Josserand wrote:
>
> [Marc: stuck in ISO-8859-1? ;-]

I have no idea what Wanderlust does (that's what I use on my laptop).
But Thunderbird definitely interprets the original posting as 'è', which
should work with a 8859-1. I need to have a look at how to get UTF-8 to
be the default... (I hate email clients).

>
>>>> It'd be good to take this opportunity to refactor the shmobile code.
>>>
>>> I can do it in this series but I do not have any shmobile platforms so
>>> I will not be able to test my modifications (only compilation).
>>>
>>> If someone can test it for me (who?), it is okay for me to refactor this
>>> code :)
>>
>> I guess you could Cc the shmobile folks (Geert Uytterhoeven, Simon
>> Horman), and get them to review/test the changes.
>
> Correct. I can test on a remote R-Car E2 ALT board that needs it.
>
> P.S. Interestingly, none of the Renesas CA15 SoCs seem to suffer from it,
> only CA7.

I suspect A15 has the courtesy of resetting CNTVOFF to zero, and A7
doesn't. But the letter of the architecture is that it has an "UNKNOWN
reset value".

Thanks,

M.
--
Jazz is not dead. It just smells funny...

2018-04-09 09:28:19

by Maxime Ripard

[permalink] [raw]
Subject: Re: [PATCH v5 08/13] ARM: sunxi: Add initialization of CNTVOFF

On Sun, Apr 08, 2018 at 11:09:32AM +0200, Myl?ne Josserand wrote:
> Hello Maxime,
>
> On Wed, 4 Apr 2018 09:45:15 +0200
> Maxime Ripard <[email protected]> wrote:
>
> > On Tue, Apr 03, 2018 at 10:06:28PM +0200, Myl?ne Josserand wrote:
> > > Hello,
> > >
> > > Thank you for the review.
> > >
> > > On Tue, 3 Apr 2018 11:12:18 +0200
> > > Maxime Ripard <[email protected]> wrote:
> > >
> > > > On Tue, Apr 03, 2018 at 08:18:31AM +0200, Myl?ne Josserand wrote:
> > > > > Add the initialization of CNTVOFF for sun8i-a83t.
> > > > >
> > > > > For boot CPU, Create a new machine that handles this
> > > > > function's call in an "init_early" callback.
> > > > > For secondary CPUs, add this function into secondary_startup
> > > > > assembly entry.
> > > > >
> > > > > Signed-off-by: Myl?ne Josserand <[email protected]>
> > > > > ---
> > > > > arch/arm/mach-sunxi/headsmp.S | 1 +
> > > > > arch/arm/mach-sunxi/sunxi.c | 18 +++++++++++++++++-
> > > > > 2 files changed, 18 insertions(+), 1 deletion(-)
> > > > >
> > > > > diff --git a/arch/arm/mach-sunxi/headsmp.S b/arch/arm/mach-sunxi/headsmp.S
> > > > > index 79890fbe5613..b586b7cf803a 100644
> > > > > --- a/arch/arm/mach-sunxi/headsmp.S
> > > > > +++ b/arch/arm/mach-sunxi/headsmp.S
> > > > > @@ -71,6 +71,7 @@ ENDPROC(sunxi_mc_smp_cluster_cache_enable)
> > > > >
> > > > > ENTRY(sunxi_mc_smp_secondary_startup)
> > > > > bl sunxi_mc_smp_cluster_cache_enable
> > > > > + bl smp_init_cntvoff
> > > > > b secondary_startup
> > > > > ENDPROC(sunxi_mc_smp_secondary_startup)
> > > > >
> > > > > diff --git a/arch/arm/mach-sunxi/sunxi.c b/arch/arm/mach-sunxi/sunxi.c
> > > > > index 5e9602ce1573..090784108c0a 100644
> > > > > --- a/arch/arm/mach-sunxi/sunxi.c
> > > > > +++ b/arch/arm/mach-sunxi/sunxi.c
> > > > > @@ -16,6 +16,7 @@
> > > > > #include <linux/platform_device.h>
> > > > >
> > > > > #include <asm/mach/arch.h>
> > > > > +#include <asm/smp_cntvoff.h>
> > > > >
> > > > > static const char * const sunxi_board_dt_compat[] = {
> > > > > "allwinner,sun4i-a10",
> > > > > @@ -62,7 +63,6 @@ MACHINE_END
> > > > > static const char * const sun8i_board_dt_compat[] = {
> > > > > "allwinner,sun8i-a23",
> > > > > "allwinner,sun8i-a33",
> > > > > - "allwinner,sun8i-a83t",
> > > > > "allwinner,sun8i-h2-plus",
> > > > > "allwinner,sun8i-h3",
> > > > > "allwinner,sun8i-r40",
> > > > > @@ -75,6 +75,22 @@ DT_MACHINE_START(SUN8I_DT, "Allwinner sun8i Family")
> > > > > .dt_compat = sun8i_board_dt_compat,
> > > > > MACHINE_END
> > > > >
> > > > > +void __init sun8i_cntvoff_init(void)
> > > > > +{
> > > > > + smp_init_cntvoff();
> > > >
> > > > Can't this be moved to the SMP setup code?
> > >
> > > I tried to put it in the first lines of "sunxi_mc_smp_init" function
> > > but it did not work. I tried to find some callbacks to have an
> > > early "init" and I only found the "init_early"'s one. There is probably
> > > another way to handle that so do not hesitate to tell me any ideas.
> >
> > It's hard to say without more context about why it doesn't work. Have
> > you checked the order between early_initcall, the timer initialization
> > function and init_early?
> >
>
> Yes, I tested it. I wanted to test it again to give more context. Here
> is the boot log: http://code.bulix.org/n1x864-315948?raw
> I added printk in the 3 functions (with "===========>").
>
> The "init_early" and "sun6i_timer" are executed before
> "arch_timer_of_init" (which is the function that parses the DT property
> that we used previously "arm,cpu-registers-not-fw-configured").
> The "sunxi_mc_smp_init" function is called later so I guess that is why
> it is not working in that case.

Ok. Make sure to mention that in your commit log.

Maxime

--
Maxime Ripard, Bootlin (formerly Free Electrons)
Embedded Linux and Kernel engineering
https://bootlin.com


Attachments:
(No filename) (3.90 kB)
signature.asc (849.00 B)
Download all attachments

2018-04-11 07:48:30

by Mylène Josserand

[permalink] [raw]
Subject: Re: [PATCH v5 07/13] ARM: smp: Add initialization of CNTVOFF

Hello,

On Mon, 9 Apr 2018 10:24:41 +0200
Geert Uytterhoeven <[email protected]> wrote:

> On Wed, Apr 4, 2018 at 4:30 PM, Marc Zyngier <[email protected]> wrote:
> > On Wed, 04 Apr 2018 14:59:09 +0100,
> > Mylčne Josserand wrote:
>
> [Marc: stuck in ISO-8859-1? ;-]
>
> >> > It'd be good to take this opportunity to refactor the shmobile code.
> >>
> >> I can do it in this series but I do not have any shmobile platforms so
> >> I will not be able to test my modifications (only compilation).
> >>
> >> If someone can test it for me (who?), it is okay for me to refactor this
> >> code :)
> >
> > I guess you could Cc the shmobile folks (Geert Uytterhoeven, Simon
> > Horman), and get them to review/test the changes.
>
> Correct. I can test on a remote R-Car E2 ALT board that needs it.

Great, thank you!

I will add you in CC in my next version, then.

Best regards,

Mylène

--
Mylène Josserand, Bootlin (formerly Free Electrons)
Embedded Linux and Kernel engineering
http://bootlin.com