2015-08-04 13:54:59

by Scott Shu

[permalink] [raw]
Subject: [PATCH v3 0/8] This series adds SMP support for the MediaTek MT6580.

This patchset adds support SMP on MediaTek MT6580 Cortex-A7 quad-core SoC.

This is based on v4.2-rc1 and following patch series:
(1) Yingjoe Chen's "Add SMP bringup support for mt65xx socs" [1]
(2) Mars Cheng's "Add mt6580 basic chip support" [2]
(3) Sascha Hauer's "Mediatek SCPSYS power domain support" [3]

The secondary cores are power off as default on MT6580, this change adds
a new enable-method to turn on power to the cores during booting process.

The System Power Manager (SPM) inside the SCPSYS is for the CPU MTCMOS
power domain control. Please check [3] for more information about SCPSYS.

[1] https://lkml.org/lkml/2015/5/16/33
[2] https://lkml.org/lkml/2015/6/30/119
[3] https://lkml.org/lkml/2015/6/9/172

Change in v3:
1. Add CPU power control in SCPSYS driver

Change in v2:
1. Rebase to 4.2-rc1
2. Fix using usleep() in atomic context
3. Put a timeout in cpu power on/off sequence
4. Fix some coding style

Scott Shu (8):
Document: bindings: DT: Add SMP enable method for MT6580 SoC
platform
dt-bindings: soc: Add SCPSYS compatible support for mt6580
soc: Mediatek: Add SCPSYS CPU power domain driver
ARM: multi_v7_defconfig: enable INFRACFG and SCPSYS modules
ARM: mediatek: add smp bringup code for MT6580
ARM: Mediatek: enable GPT6 on boot up to make arch timer working
for MT6580
ARM: dts: mt6580: Add device nodes to the MT6580 dtsi file
ARM: dts: mt6580: enable basic SMP bringup for MT6580

Documentation/devicetree/bindings/arm/cpus.txt | 1 +
.../devicetree/bindings/soc/mediatek/scpsys.txt | 2 +-
arch/arm/boot/dts/mt6580.dtsi | 26 ++
arch/arm/configs/multi_v7_defconfig | 2 +
arch/arm/mach-mediatek/mediatek.c | 4 +-
arch/arm/mach-mediatek/platsmp.c | 137 +++++++++++
drivers/soc/mediatek/mtk-scpsys.c | 250 ++++++++++++++++++++
include/linux/soc/mediatek/scpsys.h | 9 +
8 files changed, 429 insertions(+), 2 deletions(-)
create mode 100644 include/linux/soc/mediatek/scpsys.h

--
1.7.9.5


2015-08-04 13:55:11

by Scott Shu

[permalink] [raw]
Subject: [PATCH v3 1/8] Document: bindings: DT: Add SMP enable method for MT6580 SoC platform

For MT6580 SoC platform, the secondary cores are in powered off state
as default, so compared with MT6589, one new enable method is needed.
This method using the SPM (System Power Manager) inside the SCPSYS to
control the CPU power.

Signed-off-by: Scott Shu <[email protected]>
---
Documentation/devicetree/bindings/arm/cpus.txt | 1 +
1 file changed, 1 insertion(+)

diff --git a/Documentation/devicetree/bindings/arm/cpus.txt b/Documentation/devicetree/bindings/arm/cpus.txt
index d58eb45..66a8f03 100644
--- a/Documentation/devicetree/bindings/arm/cpus.txt
+++ b/Documentation/devicetree/bindings/arm/cpus.txt
@@ -195,6 +195,7 @@ nodes to be present and contain the properties described below.
"marvell,armada-380-smp"
"marvell,armada-390-smp"
"marvell,armada-xp-smp"
+ "mediatek,mt6580-smp"
"mediatek,mt6589-smp"
"mediatek,mt81xx-tz-smp"
"qcom,gcc-msm8660"
--
1.7.9.5

2015-08-04 13:57:55

by Scott Shu

[permalink] [raw]
Subject: [PATCH v3 2/8] dt-bindings: soc: Add SCPSYS compatible support for mt6580

This adds "mediatek,mt6580-scpsys" in the compatible properties of
SCPSYS node for MT6580 SoC.

Signed-off-by: Scott Shu <[email protected]>
---
.../devicetree/bindings/soc/mediatek/scpsys.txt | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/Documentation/devicetree/bindings/soc/mediatek/scpsys.txt b/Documentation/devicetree/bindings/soc/mediatek/scpsys.txt
index c051114..7998bc2 100644
--- a/Documentation/devicetree/bindings/soc/mediatek/scpsys.txt
+++ b/Documentation/devicetree/bindings/soc/mediatek/scpsys.txt
@@ -12,7 +12,7 @@ power/power_domain.txt. It provides the power domains defined in
include/dt-bindings/power/mt8173-power.h.

Required properties:
-- compatible: Must be "mediatek,mt8173-scpsys"
+- compatible: Must be "mediatek,mt6580-scpsys" or "mediatek,mt8173-scpsys"
- #power-domain-cells: Must be 1
- reg: Address range of the SCPSYS unit
- infracfg: must contain a phandle to the infracfg controller
--
1.7.9.5

2015-08-04 13:56:47

by Scott Shu

[permalink] [raw]
Subject: [PATCH v3 3/8] soc: Mediatek: Add SCPSYS CPU power domain driver

This adds a CPU power domain driver for the Mediatek SCPSYS unit on
MT6580.

Signed-off-by: Scott Shu <[email protected]>
---
drivers/soc/mediatek/mtk-scpsys.c | 250 +++++++++++++++++++++++++++++++++++
include/linux/soc/mediatek/scpsys.h | 9 ++
2 files changed, 259 insertions(+)
create mode 100644 include/linux/soc/mediatek/scpsys.h

diff --git a/drivers/soc/mediatek/mtk-scpsys.c b/drivers/soc/mediatek/mtk-scpsys.c
index 43a79ed..ca0f2dd 100644
--- a/drivers/soc/mediatek/mtk-scpsys.c
+++ b/drivers/soc/mediatek/mtk-scpsys.c
@@ -15,18 +15,30 @@
#include <linux/io.h>
#include <linux/kernel.h>
#include <linux/mfd/syscon.h>
+#include <linux/of.h>
#include <linux/of_device.h>
+#include <linux/of_address.h>
#include <linux/platform_device.h>
#include <linux/pm_domain.h>
#include <linux/regmap.h>
#include <linux/soc/mediatek/infracfg.h>
+#include <linux/soc/mediatek/scpsys.h>
#include <dt-bindings/power/mt8173-power.h>

+#define SPM_POWERON_CONFIG_SET 0x0000
+#define SPM_CA7_CPU0_PWR_CON 0x0200
#define SPM_VDE_PWR_CON 0x0210
#define SPM_MFG_PWR_CON 0x0214
+#define SPM_CA7_CPU1_PWR_CON 0x0218
+#define SPM_CA7_CPU2_PWR_CON 0x021c
+#define SPM_CA7_CPU3_PWR_CON 0x0220
#define SPM_VEN_PWR_CON 0x0230
#define SPM_ISP_PWR_CON 0x0238
#define SPM_DIS_PWR_CON 0x023c
+#define SPM_CA7_CPU0_L1_PDN 0x025c
+#define SPM_CA7_CPU1_L1_PDN 0x0264
+#define SPM_CA7_CPU2_L1_PDN 0x026c
+#define SPM_CA7_CPU3_L1_PDN 0x0274
#define SPM_VEN2_PWR_CON 0x0298
#define SPM_AUDIO_PWR_CON 0x029c
#define SPM_MFG_2D_PWR_CON 0x02c0
@@ -34,12 +46,20 @@
#define SPM_USB_PWR_CON 0x02cc
#define SPM_PWR_STATUS 0x060c
#define SPM_PWR_STATUS_2ND 0x0610
+#define SPM_SLEEP_TIMER_STA 0x0720

+/* bit definition in SPM_CA7_CPUx_PWR_CON */
#define PWR_RST_B_BIT BIT(0)
#define PWR_ISO_BIT BIT(1)
#define PWR_ON_BIT BIT(2)
#define PWR_ON_2ND_BIT BIT(3)
#define PWR_CLK_DIS_BIT BIT(4)
+#define SRAM_CKISO_BIT BIT(5)
+#define SRAM_ISOINT_B_BIT BIT(6)
+
+/* bit definition in SPM_CA7_CPUx_L1_PDN */
+#define L1_PDN BIT(0)
+#define L1_PDN_ACK BIT(8)

#define PWR_STATUS_DISP BIT(3)
#define PWR_STATUS_MFG BIT(4)
@@ -52,6 +72,28 @@
#define PWR_STATUS_AUDIO BIT(24)
#define PWR_STATUS_USB BIT(25)

+#define MT6580_MAX_CPUS 4
+
+#ifdef CONFIG_SMP
+static DEFINE_SPINLOCK(spm_cpu_lock);
+
+static void __iomem *spm_cpu_base;
+
+static const u32 spm_cpu_pwr_con[MT6580_MAX_CPUS] = {
+ SPM_CA7_CPU0_PWR_CON,
+ SPM_CA7_CPU1_PWR_CON,
+ SPM_CA7_CPU2_PWR_CON,
+ SPM_CA7_CPU3_PWR_CON,
+};
+
+static const u32 spm_cpu_l1_pdn[MT6580_MAX_CPUS] = {
+ SPM_CA7_CPU0_L1_PDN,
+ SPM_CA7_CPU1_L1_PDN,
+ SPM_CA7_CPU2_L1_PDN,
+ SPM_CA7_CPU3_L1_PDN,
+};
+#endif
+
enum clk_id {
MT8173_CLK_MM,
MT8173_CLK_MFG,
@@ -485,3 +527,211 @@ static struct platform_driver scpsys_drv = {
};

module_platform_driver_probe(scpsys_drv, scpsys_probe);
+
+#define SPM_REGWR_EN BIT(0)
+#define SPM_PROJECT_CODE 0x0B16
+
+#ifdef CONFIG_SMP
+int spm_cpu_mtcmos_on(int cpu)
+{
+ unsigned long flags;
+ static u32 spmcpu_pwr_con, spmcpu_l1_pdn;
+ unsigned int cbit, temp;
+ int timeout = 10;
+ int ret = 0;
+
+ temp = (SPM_PROJECT_CODE << 16) | SPM_REGWR_EN;
+ writel_relaxed(temp, spm_cpu_base + SPM_POWERON_CONFIG_SET);
+
+ spmcpu_pwr_con = spm_cpu_pwr_con[cpu];
+ spmcpu_l1_pdn = spm_cpu_l1_pdn[cpu];
+
+ spin_lock_irqsave(&spm_cpu_lock, flags);
+
+ /* Set PWR_ON */
+ temp = readl_relaxed(spm_cpu_base + spmcpu_pwr_con);
+ temp |= PWR_ON_BIT;
+ writel_relaxed(temp, spm_cpu_base + spmcpu_pwr_con);
+
+ /* Wait for charging core power */
+ udelay(1);
+
+ /* Set PWR_ON_2ND */
+ temp = readl_relaxed(spm_cpu_base + spmcpu_pwr_con);
+ temp |= PWR_ON_2ND_BIT;
+ writel_relaxed(temp, spm_cpu_base + spmcpu_pwr_con);
+
+ /* Wait for the power-ack */
+ cbit = BIT(13 - cpu);
+ while (((readl_relaxed(spm_cpu_base + SPM_PWR_STATUS) &
+ cbit) != cbit) ||
+ (readl_relaxed(spm_cpu_base + SPM_PWR_STATUS_2ND) &
+ cbit) != cbit) {
+ if (--timeout == 0) {
+ ret = -ENXIO;
+ goto fail;
+ }
+ udelay(1);
+ }
+
+ temp = readl_relaxed(spm_cpu_base + spmcpu_pwr_con);
+ temp &= ~PWR_ISO_BIT;
+ writel_relaxed(temp, spm_cpu_base + spmcpu_pwr_con);
+
+ /* L1 power on */
+ temp = readl_relaxed(spm_cpu_base + spmcpu_l1_pdn);
+ temp &= ~L1_PDN;
+ writel_relaxed(temp, spm_cpu_base + spmcpu_l1_pdn);
+ timeout = 10;
+ while ((readl_relaxed(spm_cpu_base + spmcpu_l1_pdn) &
+ L1_PDN_ACK) != 0) {
+ if (--timeout == 0) {
+ ret = -ENXIO;
+ goto fail;
+ }
+ udelay(1);
+ }
+
+ /* Wait for memory power ready */
+ udelay(1);
+
+ /* Set SRAM_ISOINT_B */
+ temp = readl_relaxed(spm_cpu_base + spmcpu_pwr_con);
+ temp |= SRAM_ISOINT_B_BIT;
+ writel_relaxed(temp, spm_cpu_base + spmcpu_pwr_con);
+
+ /* Clear SRAM_CKISO */
+ temp = readl_relaxed(spm_cpu_base + spmcpu_pwr_con);
+ temp &= ~SRAM_CKISO_BIT;
+ writel_relaxed(temp, spm_cpu_base + spmcpu_pwr_con);
+
+ /* Clear PWR_CLK_DIS */
+ temp = readl_relaxed(spm_cpu_base + spmcpu_pwr_con);
+ temp &= ~PWR_CLK_DIS_BIT;
+ writel_relaxed(temp, spm_cpu_base + spmcpu_pwr_con);
+
+ /* Set PWR_RST_B to finish power on and reset sequences */
+ temp = readl_relaxed(spm_cpu_base + spmcpu_pwr_con);
+ temp |= PWR_RST_B_BIT;
+ writel_relaxed(temp, spm_cpu_base + spmcpu_pwr_con);
+fail:
+ spin_unlock_irqrestore(&spm_cpu_lock, flags);
+
+ return ret;
+}
+
+int spm_cpu_mtcmos_off(int cpu, bool wfi)
+{
+ unsigned long flags;
+ static u32 spmcpu_pwr_con, spmcpu_l1_pdn;
+ unsigned int cbit, temp;
+ int timeout = 10;
+ int ret = 0;
+
+ temp = (SPM_PROJECT_CODE << 16) | SPM_REGWR_EN;
+ writel_relaxed(temp, spm_cpu_base + SPM_POWERON_CONFIG_SET);
+
+ spmcpu_pwr_con = spm_cpu_pwr_con[cpu];
+ spmcpu_l1_pdn = spm_cpu_l1_pdn[cpu];
+
+ if (wfi) {
+ while ((readl_relaxed(spm_cpu_base + SPM_SLEEP_TIMER_STA) &
+ (1U << (16 + cpu))) == 0) {
+ if (--timeout == 0)
+ return -ENXIO;
+ udelay(1);
+ }
+ }
+
+ spin_lock_irqsave(&spm_cpu_lock, flags);
+
+ /* Set PWR_ISO */
+ temp = readl_relaxed(spm_cpu_base + spmcpu_pwr_con);
+ temp |= PWR_ISO_BIT;
+ writel_relaxed(temp, spm_cpu_base + spmcpu_pwr_con);
+
+ /* Set SRAM_CKISO */
+ temp = readl_relaxed(spm_cpu_base + spmcpu_pwr_con);
+ temp |= SRAM_CKISO_BIT;
+ writel_relaxed(temp, spm_cpu_base + spmcpu_pwr_con);
+
+ /* Clear SRAM_ISOINT_B */
+ temp = readl_relaxed(spm_cpu_base + spmcpu_pwr_con);
+ temp &= ~SRAM_ISOINT_B_BIT;
+ writel_relaxed(temp, spm_cpu_base + spmcpu_pwr_con);
+
+ /* L1 power off */
+ temp = readl_relaxed(spm_cpu_base + spmcpu_l1_pdn);
+ temp |= L1_PDN;
+ writel_relaxed(temp, spm_cpu_base + spmcpu_l1_pdn);
+ timeout = 10;
+ while ((readl_relaxed(spm_cpu_base + spmcpu_l1_pdn) &
+ L1_PDN_ACK) != L1_PDN_ACK) {
+ if (--timeout == 0) {
+ ret = -ENXIO;
+ goto fail;
+ }
+ udelay(1);
+ }
+
+ /* Clear PWR_RST_B */
+ temp = readl_relaxed(spm_cpu_base + spmcpu_pwr_con);
+ temp &= ~PWR_RST_B_BIT;
+ writel_relaxed(temp, spm_cpu_base + spmcpu_pwr_con);
+
+ /* Set PWR_CLK_DIS */
+ temp = readl_relaxed(spm_cpu_base + spmcpu_pwr_con);
+ temp |= PWR_CLK_DIS_BIT;
+ writel_relaxed(temp, spm_cpu_base + spmcpu_pwr_con);
+
+ /* Clear PWR_ON */
+ temp = readl_relaxed(spm_cpu_base + spmcpu_pwr_con);
+ temp &= ~PWR_ON_BIT;
+ writel_relaxed(temp, spm_cpu_base + spmcpu_pwr_con);
+
+ /* Clear PWR_ON_2ND */
+ temp = readl_relaxed(spm_cpu_base + spmcpu_pwr_con);
+ temp &= ~PWR_ON_2ND_BIT;
+ writel_relaxed(temp, spm_cpu_base + spmcpu_pwr_con);
+
+ timeout = 10;
+ cbit = BIT(13 - cpu);
+ while (((readl_relaxed(spm_cpu_base + SPM_PWR_STATUS) & cbit) != 0) ||
+ ((readl_relaxed(spm_cpu_base + SPM_PWR_STATUS_2ND) &
+ cbit) != 0)) {
+ if (--timeout == 0) {
+ ret = -ENXIO;
+ goto fail;
+ }
+ udelay(1);
+ }
+fail:
+ spin_unlock_irqrestore(&spm_cpu_lock, flags);
+
+ return ret;
+}
+#endif /* CONFIG_SMP */
+
+static int __init mtk_scpsys_early_init(void)
+{
+ struct device_node *node;
+ struct resource regs;
+
+ node = of_find_compatible_node(NULL, NULL, "mediatek,mt6580-scpsys");
+ if (node) {
+ if (of_address_to_resource(node, 0, &regs) < 0) {
+ pr_err("Failed to get SCPSYS registers\n");
+ return -ENXIO;
+ }
+
+ spm_cpu_base = ioremap_nocache(regs.start,
+ resource_size(&regs));
+ if (!spm_cpu_base) {
+ pr_err("%s: Unable to map I/O memory\n", __func__);
+ return -ENODEV;
+ }
+ }
+
+ return 0;
+}
+early_initcall(mtk_scpsys_early_init);
diff --git a/include/linux/soc/mediatek/scpsys.h b/include/linux/soc/mediatek/scpsys.h
new file mode 100644
index 0000000..a14b7a3
--- /dev/null
+++ b/include/linux/soc/mediatek/scpsys.h
@@ -0,0 +1,9 @@
+#ifndef __SOC_MEDIATEK_SCPSYS_H
+#define __SOC_MEDIATEK_SCPSYS_H
+
+#include <linux/kernel.h>
+
+int spm_cpu_mtcmos_on(int cpu);
+int spm_cpu_mtcmos_off(int cpu, bool wfi);
+
+#endif
--
1.7.9.5

2015-08-04 13:56:46

by Scott Shu

[permalink] [raw]
Subject: [PATCH v3 4/8] ARM: multi_v7_defconfig: enable INFRACFG and SCPSYS modules

The INFRACFG contains various infrastructure registers and the SCPSYS handles
several power management related tasks. Both are needed for SMP and CPU
hotplug on MT6580.

Signed-off-by: Scott Shu <[email protected]>
---
arch/arm/configs/multi_v7_defconfig | 2 ++
1 file changed, 2 insertions(+)

diff --git a/arch/arm/configs/multi_v7_defconfig b/arch/arm/configs/multi_v7_defconfig
index 6d83a1b..c272edd 100644
--- a/arch/arm/configs/multi_v7_defconfig
+++ b/arch/arm/configs/multi_v7_defconfig
@@ -596,6 +596,8 @@ CONFIG_KEYBOARD_NVEC=y
CONFIG_SERIO_NVEC_PS2=y
CONFIG_NVEC_POWER=y
CONFIG_NVEC_PAZ00=y
+CONFIG_MTK_INFRACFG=y
+CONFIG_MTK_SCPSYS=y
CONFIG_QCOM_GSBI=y
CONFIG_QCOM_PM=y
CONFIG_COMMON_CLK_QCOM=y
--
1.7.9.5

2015-08-04 13:56:29

by Scott Shu

[permalink] [raw]
Subject: [PATCH v3 5/8] ARM: mediatek: add smp bringup code for MT6580

Add support for cpu enable-method "mediatek,mt6580-smp" for booting
secondary CPUs on MT6580.

Signed-off-by: Scott Shu <[email protected]>
---
arch/arm/mach-mediatek/platsmp.c | 137 ++++++++++++++++++++++++++++++++++++++
1 file changed, 137 insertions(+)

diff --git a/arch/arm/mach-mediatek/platsmp.c b/arch/arm/mach-mediatek/platsmp.c
index a5bc108..94f7865 100644
--- a/arch/arm/mach-mediatek/platsmp.c
+++ b/arch/arm/mach-mediatek/platsmp.c
@@ -21,10 +21,16 @@
#include <linux/of_address.h>
#include <linux/string.h>
#include <linux/threads.h>
+#include <linux/delay.h>
+#include <asm/cacheflush.h>
+
+#include <linux/soc/mediatek/scpsys.h>

#define MTK_MAX_CPU 8
#define MTK_SMP_REG_SIZE 0x1000

+static DEFINE_SPINLOCK(boot_lock);
+
struct mtk_smp_boot_info {
unsigned long smp_base;
unsigned int jump_reg;
@@ -56,6 +62,126 @@ static const struct of_device_id mtk_smp_boot_infos[] __initconst = {
static void __iomem *mtk_smp_base;
static const struct mtk_smp_boot_info *mtk_smp_info;

+#ifdef CONFIG_HOTPLUG_CPU
+static int mt6580_cpu_kill(unsigned cpu)
+{
+ int ret;
+
+ ret = spm_cpu_mtcmos_off(cpu, 1);
+ if (ret < 0)
+ return 0;
+
+ return 1;
+}
+
+static void mt6580_cpu_die(unsigned int cpu)
+{
+ for (;;)
+ cpu_do_idle();
+}
+#endif
+
+static void write_pen_release(int val)
+{
+ pen_release = val;
+ /* Make sure this is visible to other CPUs */
+ smp_wmb();
+ sync_cache_w(&pen_release);
+}
+
+/*
+ * Refer common "pen" secondary release method
+ */
+static int mt6580_boot_secondary(unsigned int cpu, struct task_struct *idle)
+{
+ unsigned long timeout;
+ int ret;
+
+ /*
+ * Set synchronisation state between this boot processor
+ * and the secondary one
+ */
+ spin_lock(&boot_lock);
+
+ /*
+ * The secondary processor is waiting to be released from
+ * the holding pen - release it, then wait for it to flag
+ * that it has been released by resetting pen_release.
+ *
+ * Note that "pen_release" is the hardware CPU ID, whereas
+ * "cpu" is Linux's internal ID.
+ */
+ write_pen_release(cpu);
+
+ /*
+ * CPU power on control by SPM
+ */
+ ret = spm_cpu_mtcmos_on(cpu);
+ if (ret < 0) {
+ spin_unlock(&boot_lock);
+ return -ENXIO;
+ }
+
+ timeout = jiffies + (1 * HZ);
+ while (time_before(jiffies, timeout)) {
+ /* Read barrier */
+ smp_rmb();
+
+ if (pen_release == -1)
+ break;
+
+ udelay(10);
+ }
+
+ /*
+ * Now the secondary core is starting up let it run its
+ * calibrations, then wait for it to finish
+ */
+ spin_unlock(&boot_lock);
+
+ return (pen_release != -1 ? -ENXIO : 0);
+}
+
+static void mt6580_secondary_init(unsigned int cpu)
+{
+ /*
+ * Let the primary processor know we're out of the
+ * pen, then head off into the C entry point
+ */
+ write_pen_release(-1);
+
+ /*
+ * Synchronise with the boot thread.
+ */
+ spin_lock(&boot_lock);
+ spin_unlock(&boot_lock);
+}
+
+#define MT6580_INFRACFG_AO 0x10001000
+#define SW_ROM_PD BIT(31)
+
+static void __init mt6580_smp_prepare_cpus(unsigned int max_cpus)
+{
+ static void __iomem *infracfg_ao_base;
+
+ infracfg_ao_base = ioremap(MT6580_INFRACFG_AO, 0x1000);
+ if (!infracfg_ao_base) {
+ pr_err("%s: Unable to map I/O memory\n", __func__);
+ return;
+ }
+
+ /* Enable bootrom power down mode */
+ writel_relaxed(readl(infracfg_ao_base + 0x804) | SW_ROM_PD,
+ infracfg_ao_base + 0x804);
+
+ /* Write the address of slave startup into boot address
+ register for bootrom power down mode */
+ writel_relaxed(virt_to_phys(secondary_startup_arm),
+ infracfg_ao_base + 0x800);
+
+ iounmap(infracfg_ao_base);
+}
+
static int mtk_boot_secondary(unsigned int cpu, struct task_struct *idle)
{
if (!mtk_smp_base)
@@ -142,3 +268,14 @@ static struct smp_operations mt6589_smp_ops __initdata = {
.smp_boot_secondary = mtk_boot_secondary,
};
CPU_METHOD_OF_DECLARE(mt6589_smp, "mediatek,mt6589-smp", &mt6589_smp_ops);
+
+static struct smp_operations mt6580_smp_ops __initdata = {
+ .smp_prepare_cpus = mt6580_smp_prepare_cpus,
+ .smp_secondary_init = mt6580_secondary_init,
+ .smp_boot_secondary = mt6580_boot_secondary,
+#ifdef CONFIG_HOTPLUG_CPU
+ .cpu_kill = mt6580_cpu_kill,
+ .cpu_die = mt6580_cpu_die,
+#endif
+};
+CPU_METHOD_OF_DECLARE(mt6580_smp, "mediatek,mt6580-smp", &mt6580_smp_ops);
--
1.7.9.5

2015-08-04 13:55:47

by Scott Shu

[permalink] [raw]
Subject: [PATCH v3 6/8] ARM: Mediatek: enable GPT6 on boot up to make arch timer working for MT6580

We enable GTP6 which ungates the arch timer clock.

Signed-off-by: Scott Shu <[email protected]>
---
arch/arm/mach-mediatek/mediatek.c | 4 +++-
1 file changed, 3 insertions(+), 1 deletion(-)

diff --git a/arch/arm/mach-mediatek/mediatek.c b/arch/arm/mach-mediatek/mediatek.c
index 6b38d67..469d332 100644
--- a/arch/arm/mach-mediatek/mediatek.c
+++ b/arch/arm/mach-mediatek/mediatek.c
@@ -28,7 +28,8 @@ static void __init mediatek_timer_init(void)
{
void __iomem *gpt_base = 0;

- if (of_machine_is_compatible("mediatek,mt6589") ||
+ if (of_machine_is_compatible("mediatek,mt6580") ||
+ of_machine_is_compatible("mediatek,mt6589") ||
of_machine_is_compatible("mediatek,mt8135") ||
of_machine_is_compatible("mediatek,mt8127")) {
/* turn on GPT6 which ungates arch timer clocks */
@@ -46,6 +47,7 @@ static void __init mediatek_timer_init(void)
};

static const char * const mediatek_board_dt_compat[] = {
+ "mediatek,mt6580",
"mediatek,mt6589",
"mediatek,mt6592",
"mediatek,mt8127",
--
1.7.9.5

2015-08-04 13:55:46

by Scott Shu

[permalink] [raw]
Subject: [PATCH v3 7/8] ARM: dts: mt6580: Add device nodes to the MT6580 dtsi file

This adds the SCPSYS device node to the MT6580 dtsi file.

Signed-off-by: Scott Shu <[email protected]>
---
arch/arm/boot/dts/mt6580.dtsi | 6 ++++++
1 file changed, 6 insertions(+)

diff --git a/arch/arm/boot/dts/mt6580.dtsi b/arch/arm/boot/dts/mt6580.dtsi
index 06fdf6c..40957d3 100644
--- a/arch/arm/boot/dts/mt6580.dtsi
+++ b/arch/arm/boot/dts/mt6580.dtsi
@@ -67,6 +67,12 @@
#clock-cells = <0>;
};

+ scpsys: scpsys@10006000 {
+ #power-domain-cells = <1>;
+ compatible = "mediatek,mt6580-scpsys";
+ reg = <0x10006000 0x1000>;
+ };
+
timer: timer@10008000 {
compatible = "mediatek,mt6580-timer",
"mediatek,mt6577-timer";
--
1.7.9.5

2015-08-04 13:54:53

by Scott Shu

[permalink] [raw]
Subject: [PATCH v3 8/8] ARM: dts: mt6580: enable basic SMP bringup for MT6580

Add arch timer node to enable arch-timer support. MT6580 firmware
doesn't correctly setup arch-timer frequency and CNTVOFF, add
properties to workaround this.

This set cpu enable-method to enable SMP.

Signed-off-by: Scott Shu <[email protected]>
---
arch/arm/boot/dts/mt6580.dtsi | 20 ++++++++++++++++++++
1 file changed, 20 insertions(+)

diff --git a/arch/arm/boot/dts/mt6580.dtsi b/arch/arm/boot/dts/mt6580.dtsi
index 40957d3..70531b2 100644
--- a/arch/arm/boot/dts/mt6580.dtsi
+++ b/arch/arm/boot/dts/mt6580.dtsi
@@ -25,26 +25,31 @@
cpus {
#address-cells = <1>;
#size-cells = <0>;
+ enable-method = "mediatek,mt6580-smp";

cpu@0 {
device_type = "cpu";
compatible = "arm,cortex-a7";
reg = <0x0>;
+ clock-frequency = <1700000000>;
};
cpu@1 {
device_type = "cpu";
compatible = "arm,cortex-a7";
reg = <0x1>;
+ clock-frequency = <1700000000>;
};
cpu@2 {
device_type = "cpu";
compatible = "arm,cortex-a7";
reg = <0x2>;
+ clock-frequency = <1700000000>;
};
cpu@3 {
device_type = "cpu";
compatible = "arm,cortex-a7";
reg = <0x3>;
+ clock-frequency = <1700000000>;
};

};
@@ -67,6 +72,21 @@
#clock-cells = <0>;
};

+ timer {
+ compatible = "arm,armv7-timer";
+ interrupt-parent = <&gic>;
+ interrupts = <GIC_PPI 13 (GIC_CPU_MASK_SIMPLE(4) |
+ IRQ_TYPE_LEVEL_LOW)>,
+ <GIC_PPI 14 (GIC_CPU_MASK_SIMPLE(4) |
+ IRQ_TYPE_LEVEL_LOW)>,
+ <GIC_PPI 11 (GIC_CPU_MASK_SIMPLE(4) |
+ IRQ_TYPE_LEVEL_LOW)>,
+ <GIC_PPI 10 (GIC_CPU_MASK_SIMPLE(4) |
+ IRQ_TYPE_LEVEL_LOW)>;
+ clock-frequency = <13000000>;
+ arm,cpu-registers-not-fw-configured;
+ };
+
scpsys: scpsys@10006000 {
#power-domain-cells = <1>;
compatible = "mediatek,mt6580-scpsys";
--
1.7.9.5

2015-08-05 08:50:44

by Sascha Hauer

[permalink] [raw]
Subject: Re: [PATCH v3 3/8] soc: Mediatek: Add SCPSYS CPU power domain driver

On Tue, Aug 04, 2015 at 09:54:19PM +0800, Scott Shu wrote:
> This adds a CPU power domain driver for the Mediatek SCPSYS unit on
> MT6580.
>
> Signed-off-by: Scott Shu <[email protected]>
> ---
> drivers/soc/mediatek/mtk-scpsys.c | 250 +++++++++++++++++++++++++++++++++++
> include/linux/soc/mediatek/scpsys.h | 9 ++
> 2 files changed, 259 insertions(+)
> create mode 100644 include/linux/soc/mediatek/scpsys.h
>
> diff --git a/drivers/soc/mediatek/mtk-scpsys.c b/drivers/soc/mediatek/mtk-scpsys.c
> index 43a79ed..ca0f2dd 100644
> --- a/drivers/soc/mediatek/mtk-scpsys.c
> +++ b/drivers/soc/mediatek/mtk-scpsys.c
> @@ -15,18 +15,30 @@
> #include <linux/io.h>
> #include <linux/kernel.h>
> #include <linux/mfd/syscon.h>
> +#include <linux/of.h>
> #include <linux/of_device.h>
> +#include <linux/of_address.h>
> #include <linux/platform_device.h>
> #include <linux/pm_domain.h>
> #include <linux/regmap.h>
> #include <linux/soc/mediatek/infracfg.h>
> +#include <linux/soc/mediatek/scpsys.h>
> #include <dt-bindings/power/mt8173-power.h>
>
> +#define SPM_POWERON_CONFIG_SET 0x0000
> +#define SPM_CA7_CPU0_PWR_CON 0x0200
> #define SPM_VDE_PWR_CON 0x0210
> #define SPM_MFG_PWR_CON 0x0214
> +#define SPM_CA7_CPU1_PWR_CON 0x0218
> +#define SPM_CA7_CPU2_PWR_CON 0x021c
> +#define SPM_CA7_CPU3_PWR_CON 0x0220
> #define SPM_VEN_PWR_CON 0x0230
> #define SPM_ISP_PWR_CON 0x0238
> #define SPM_DIS_PWR_CON 0x023c
> +#define SPM_CA7_CPU0_L1_PDN 0x025c
> +#define SPM_CA7_CPU1_L1_PDN 0x0264
> +#define SPM_CA7_CPU2_L1_PDN 0x026c
> +#define SPM_CA7_CPU3_L1_PDN 0x0274
> #define SPM_VEN2_PWR_CON 0x0298
> #define SPM_AUDIO_PWR_CON 0x029c
> #define SPM_MFG_2D_PWR_CON 0x02c0
> @@ -34,12 +46,20 @@
> #define SPM_USB_PWR_CON 0x02cc
> #define SPM_PWR_STATUS 0x060c
> #define SPM_PWR_STATUS_2ND 0x0610
> +#define SPM_SLEEP_TIMER_STA 0x0720
>
> +/* bit definition in SPM_CA7_CPUx_PWR_CON */
> #define PWR_RST_B_BIT BIT(0)
> #define PWR_ISO_BIT BIT(1)
> #define PWR_ON_BIT BIT(2)
> #define PWR_ON_2ND_BIT BIT(3)
> #define PWR_CLK_DIS_BIT BIT(4)
> +#define SRAM_CKISO_BIT BIT(5)
> +#define SRAM_ISOINT_B_BIT BIT(6)
> +
> +/* bit definition in SPM_CA7_CPUx_L1_PDN */
> +#define L1_PDN BIT(0)
> +#define L1_PDN_ACK BIT(8)
>
> #define PWR_STATUS_DISP BIT(3)
> #define PWR_STATUS_MFG BIT(4)
> @@ -52,6 +72,28 @@
> #define PWR_STATUS_AUDIO BIT(24)
> #define PWR_STATUS_USB BIT(25)
>
> +#define MT6580_MAX_CPUS 4
> +
> +#ifdef CONFIG_SMP
> +static DEFINE_SPINLOCK(spm_cpu_lock);
> +
> +static void __iomem *spm_cpu_base;
> +
> +static const u32 spm_cpu_pwr_con[MT6580_MAX_CPUS] = {
> + SPM_CA7_CPU0_PWR_CON,
> + SPM_CA7_CPU1_PWR_CON,
> + SPM_CA7_CPU2_PWR_CON,
> + SPM_CA7_CPU3_PWR_CON,
> +};
> +
> +static const u32 spm_cpu_l1_pdn[MT6580_MAX_CPUS] = {
> + SPM_CA7_CPU0_L1_PDN,
> + SPM_CA7_CPU1_L1_PDN,
> + SPM_CA7_CPU2_L1_PDN,
> + SPM_CA7_CPU3_L1_PDN,
> +};
> +#endif
> +
> enum clk_id {
> MT8173_CLK_MM,
> MT8173_CLK_MFG,
> @@ -485,3 +527,211 @@ static struct platform_driver scpsys_drv = {
> };
>
> module_platform_driver_probe(scpsys_drv, scpsys_probe);
> +
> +#define SPM_REGWR_EN BIT(0)
> +#define SPM_PROJECT_CODE 0x0B16
> +
> +#ifdef CONFIG_SMP
> +int spm_cpu_mtcmos_on(int cpu)

The idea was that the code that the current driver has in
scpsys_power_on/scpsys_power_off is shared with this function. If we
don't do this then it indeed doesn't make much sense to put it into the
same file.

>From what I see we would need to change the prototype to something like

static int __scpsys_power_on(struct scp_domain_data *)

(maybe with some additional base addresses and stuff)

struct scp_domain_data would additionally need sram_isoint_b and sram_ckiso
members.

Sascha

--
Pengutronix e.K. | |
Industrial Linux Solutions | http://www.pengutronix.de/ |
Peiner Str. 6-8, 31137 Hildesheim, Germany | Phone: +49-5121-206917-0 |
Amtsgericht Hildesheim, HRA 2686 | Fax: +49-5121-206917-5555 |

2015-08-05 09:44:49

by Russell King - ARM Linux

[permalink] [raw]
Subject: Re: [PATCH v3 5/8] ARM: mediatek: add smp bringup code for MT6580

On Tue, Aug 04, 2015 at 09:54:21PM +0800, Scott Shu wrote:
> Add support for cpu enable-method "mediatek,mt6580-smp" for booting
> secondary CPUs on MT6580.

If you have CPU power domain support, and you power up and power down the
CPUs, why do you need the boot_lock and pen_release stuff? Isn't keeping
the power off on a CPU sufficient to prevent it entering the kernel?

--
FTTC broadband for 0.8mile line: currently at 10.5Mbps down 400kbps up
according to speedtest.net.

2015-08-05 16:47:11

by Matthias Brugger

[permalink] [raw]
Subject: Re: [PATCH v3 5/8] ARM: mediatek: add smp bringup code for MT6580

On Tuesday, August 04, 2015 09:54:21 PM Scott Shu wrote:
> Add support for cpu enable-method "mediatek,mt6580-smp" for booting
> secondary CPUs on MT6580.
>
> Signed-off-by: Scott Shu <[email protected]>
> ---
> arch/arm/mach-mediatek/platsmp.c | 137
> ++++++++++++++++++++++++++++++++++++++ 1 file changed, 137 insertions(+)
>
> diff --git a/arch/arm/mach-mediatek/platsmp.c
> b/arch/arm/mach-mediatek/platsmp.c index a5bc108..94f7865 100644
> --- a/arch/arm/mach-mediatek/platsmp.c
> +++ b/arch/arm/mach-mediatek/platsmp.c
> @@ -21,10 +21,16 @@
> #include <linux/of_address.h>
> #include <linux/string.h>
> #include <linux/threads.h>
> +#include <linux/delay.h>
> +#include <asm/cacheflush.h>
> +
> +#include <linux/soc/mediatek/scpsys.h>
>
> #define MTK_MAX_CPU 8
> #define MTK_SMP_REG_SIZE 0x1000
>
> +static DEFINE_SPINLOCK(boot_lock);
> +
> struct mtk_smp_boot_info {
> unsigned long smp_base;
> unsigned int jump_reg;
> @@ -56,6 +62,126 @@ static const struct of_device_id mtk_smp_boot_infos[]
> __initconst = { static void __iomem *mtk_smp_base;
> static const struct mtk_smp_boot_info *mtk_smp_info;
>
> +#ifdef CONFIG_HOTPLUG_CPU
> +static int mt6580_cpu_kill(unsigned cpu)
> +{
> + int ret;
> +
> + ret = spm_cpu_mtcmos_off(cpu, 1);

is this function ever called with wfi == 0?

> + if (ret < 0)
> + return 0;
> +
> + return 1;
> +}
> +
> +static void mt6580_cpu_die(unsigned int cpu)
> +{
> + for (;;)
> + cpu_do_idle();
> +}
> +#endif
> +
> +static void write_pen_release(int val)
> +{
> + pen_release = val;
> + /* Make sure this is visible to other CPUs */
> + smp_wmb();
> + sync_cache_w(&pen_release);
> +}
> +
> +/*
> + * Refer common "pen" secondary release method
> + */
> +static int mt6580_boot_secondary(unsigned int cpu, struct task_struct
> *idle) +{
> + unsigned long timeout;
> + int ret;
> +
> + /*
> + * Set synchronisation state between this boot processor
> + * and the secondary one
> + */
> + spin_lock(&boot_lock);
> +
> + /*
> + * The secondary processor is waiting to be released from
> + * the holding pen - release it, then wait for it to flag
> + * that it has been released by resetting pen_release.
> + *
> + * Note that "pen_release" is the hardware CPU ID, whereas
> + * "cpu" is Linux's internal ID.
> + */
> + write_pen_release(cpu);
> +
> + /*
> + * CPU power on control by SPM
> + */
> + ret = spm_cpu_mtcmos_on(cpu);
> + if (ret < 0) {
> + spin_unlock(&boot_lock);
> + return -ENXIO;
> + }
> +
> + timeout = jiffies + (1 * HZ);
> + while (time_before(jiffies, timeout)) {
> + /* Read barrier */
> + smp_rmb();
> +
> + if (pen_release == -1)
> + break;
> +
> + udelay(10);
> + }
> +
> + /*
> + * Now the secondary core is starting up let it run its
> + * calibrations, then wait for it to finish
> + */
> + spin_unlock(&boot_lock);
> +
> + return (pen_release != -1 ? -ENXIO : 0);
> +}
> +
> +static void mt6580_secondary_init(unsigned int cpu)
> +{
> + /*
> + * Let the primary processor know we're out of the
> + * pen, then head off into the C entry point
> + */
> + write_pen_release(-1);
> +
> + /*
> + * Synchronise with the boot thread.
> + */
> + spin_lock(&boot_lock);
> + spin_unlock(&boot_lock);
> +}
> +
> +#define MT6580_INFRACFG_AO 0x10001000
> +#define SW_ROM_PD BIT(31)

Please put the defines on the beginning of the file.
SW_ROM_PD bit is the same on other SoCs? I wasn't able to reveal that from the
data sheets. If this is MT6580 only, the define should be renamed.

> +
> +static void __init mt6580_smp_prepare_cpus(unsigned int max_cpus)
> +{
> + static void __iomem *infracfg_ao_base;
> +
> + infracfg_ao_base = ioremap(MT6580_INFRACFG_AO, 0x1000);
> + if (!infracfg_ao_base) {
> + pr_err("%s: Unable to map I/O memory\n", __func__);
> + return;
> + }
> +
> + /* Enable bootrom power down mode */
> + writel_relaxed(readl(infracfg_ao_base + 0x804) | SW_ROM_PD,
> + infracfg_ao_base + 0x804);

Please use a define for the offset.
I prefer to not cascade reads and writes but to use a temporary variable. But
if this is fine with the kernel coding style (I doubt, but I'm not sure) then
this is fine.

> +
> + /* Write the address of slave startup into boot address
> + register for bootrom power down mode */
> + writel_relaxed(virt_to_phys(secondary_startup_arm),
> + infracfg_ao_base + 0x800);
> +
> + iounmap(infracfg_ao_base);
> +}
> +
> static int mtk_boot_secondary(unsigned int cpu, struct task_struct *idle)
> {
> if (!mtk_smp_base)
> @@ -142,3 +268,14 @@ static struct smp_operations mt6589_smp_ops __initdata
> = { .smp_boot_secondary = mtk_boot_secondary,
> };
> CPU_METHOD_OF_DECLARE(mt6589_smp, "mediatek,mt6589-smp", &mt6589_smp_ops);
> +
> +static struct smp_operations mt6580_smp_ops __initdata = {
> + .smp_prepare_cpus = mt6580_smp_prepare_cpus,
> + .smp_secondary_init = mt6580_secondary_init,
> + .smp_boot_secondary = mt6580_boot_secondary,
> +#ifdef CONFIG_HOTPLUG_CPU
> + .cpu_kill = mt6580_cpu_kill,
> + .cpu_die = mt6580_cpu_die,
> +#endif
> +};
> +CPU_METHOD_OF_DECLARE(mt6580_smp, "mediatek,mt6580-smp", &mt6580_smp_ops);

2015-08-06 02:59:20

by Scott Shu

[permalink] [raw]
Subject: Re: [PATCH v3 3/8] soc: Mediatek: Add SCPSYS CPU power domain driver

On Wed, 2015-08-05 at 10:50 +0200, Sascha Hauer wrote:
> On Tue, Aug 04, 2015 at 09:54:19PM +0800, Scott Shu wrote:
> > This adds a CPU power domain driver for the Mediatek SCPSYS unit on
> > MT6580.
> >
> > Signed-off-by: Scott Shu <[email protected]>
> > ---
> > drivers/soc/mediatek/mtk-scpsys.c | 250 +++++++++++++++++++++++++++++++++++
> > include/linux/soc/mediatek/scpsys.h | 9 ++
> > 2 files changed, 259 insertions(+)
> > create mode 100644 include/linux/soc/mediatek/scpsys.h
> >
> > diff --git a/drivers/soc/mediatek/mtk-scpsys.c b/drivers/soc/mediatek/mtk-scpsys.c
> > index 43a79ed..ca0f2dd 100644
> > --- a/drivers/soc/mediatek/mtk-scpsys.c
> > +++ b/drivers/soc/mediatek/mtk-scpsys.c
> > @@ -15,18 +15,30 @@
> > #include <linux/io.h>
> > #include <linux/kernel.h>
> > #include <linux/mfd/syscon.h>
> > +#include <linux/of.h>
> > #include <linux/of_device.h>
> > +#include <linux/of_address.h>
> > #include <linux/platform_device.h>
> > #include <linux/pm_domain.h>
> > #include <linux/regmap.h>
> > #include <linux/soc/mediatek/infracfg.h>
> > +#include <linux/soc/mediatek/scpsys.h>
> > #include <dt-bindings/power/mt8173-power.h>
> >
> > +#define SPM_POWERON_CONFIG_SET 0x0000
> > +#define SPM_CA7_CPU0_PWR_CON 0x0200
> > #define SPM_VDE_PWR_CON 0x0210
> > #define SPM_MFG_PWR_CON 0x0214
> > +#define SPM_CA7_CPU1_PWR_CON 0x0218
> > +#define SPM_CA7_CPU2_PWR_CON 0x021c
> > +#define SPM_CA7_CPU3_PWR_CON 0x0220
> > #define SPM_VEN_PWR_CON 0x0230
> > #define SPM_ISP_PWR_CON 0x0238
> > #define SPM_DIS_PWR_CON 0x023c
> > +#define SPM_CA7_CPU0_L1_PDN 0x025c
> > +#define SPM_CA7_CPU1_L1_PDN 0x0264
> > +#define SPM_CA7_CPU2_L1_PDN 0x026c
> > +#define SPM_CA7_CPU3_L1_PDN 0x0274
> > #define SPM_VEN2_PWR_CON 0x0298
> > #define SPM_AUDIO_PWR_CON 0x029c
> > #define SPM_MFG_2D_PWR_CON 0x02c0
> > @@ -34,12 +46,20 @@
> > #define SPM_USB_PWR_CON 0x02cc
> > #define SPM_PWR_STATUS 0x060c
> > #define SPM_PWR_STATUS_2ND 0x0610
> > +#define SPM_SLEEP_TIMER_STA 0x0720
> >
> > +/* bit definition in SPM_CA7_CPUx_PWR_CON */
> > #define PWR_RST_B_BIT BIT(0)
> > #define PWR_ISO_BIT BIT(1)
> > #define PWR_ON_BIT BIT(2)
> > #define PWR_ON_2ND_BIT BIT(3)
> > #define PWR_CLK_DIS_BIT BIT(4)
> > +#define SRAM_CKISO_BIT BIT(5)
> > +#define SRAM_ISOINT_B_BIT BIT(6)
> > +
> > +/* bit definition in SPM_CA7_CPUx_L1_PDN */
> > +#define L1_PDN BIT(0)
> > +#define L1_PDN_ACK BIT(8)
> >
> > #define PWR_STATUS_DISP BIT(3)
> > #define PWR_STATUS_MFG BIT(4)
> > @@ -52,6 +72,28 @@
> > #define PWR_STATUS_AUDIO BIT(24)
> > #define PWR_STATUS_USB BIT(25)
> >
> > +#define MT6580_MAX_CPUS 4
> > +
> > +#ifdef CONFIG_SMP
> > +static DEFINE_SPINLOCK(spm_cpu_lock);
> > +
> > +static void __iomem *spm_cpu_base;
> > +
> > +static const u32 spm_cpu_pwr_con[MT6580_MAX_CPUS] = {
> > + SPM_CA7_CPU0_PWR_CON,
> > + SPM_CA7_CPU1_PWR_CON,
> > + SPM_CA7_CPU2_PWR_CON,
> > + SPM_CA7_CPU3_PWR_CON,
> > +};
> > +
> > +static const u32 spm_cpu_l1_pdn[MT6580_MAX_CPUS] = {
> > + SPM_CA7_CPU0_L1_PDN,
> > + SPM_CA7_CPU1_L1_PDN,
> > + SPM_CA7_CPU2_L1_PDN,
> > + SPM_CA7_CPU3_L1_PDN,
> > +};
> > +#endif
> > +
> > enum clk_id {
> > MT8173_CLK_MM,
> > MT8173_CLK_MFG,
> > @@ -485,3 +527,211 @@ static struct platform_driver scpsys_drv = {
> > };
> >
> > module_platform_driver_probe(scpsys_drv, scpsys_probe);
> > +
> > +#define SPM_REGWR_EN BIT(0)
> > +#define SPM_PROJECT_CODE 0x0B16
> > +
> > +#ifdef CONFIG_SMP
> > +int spm_cpu_mtcmos_on(int cpu)
>
> The idea was that the code that the current driver has in
> scpsys_power_on/scpsys_power_off is shared with this function. If we
> don't do this then it indeed doesn't make much sense to put it into the
> same file.
>
> From what I see we would need to change the prototype to something like
>
> static int __scpsys_power_on(struct scp_domain_data *)
>
> (maybe with some additional base addresses and stuff)
>
> struct scp_domain_data would additionally need sram_isoint_b and sram_ckiso
> members.
>
> Sascha
>
Hi Sascha,
The CPU power sequence is quite different with the others, as
described below.

* Non-CPU
1) Set PWR_ON_BIT, PWR_ON_2ND_BIT
2) Wait PWR_ACK
3) Clear PWR_CLK_DIS_BIT
4) Clear PWR_ISO_BIT
5) Set PWR_RST_B_BIT
6) Clear SRAM_PDN
7) Wait SRAM_PDN_ACK
* CPU
1) Set PWR_ON_BIT, PWR_ON_2ND_BIT
2) Wait PWR_ACK
3) Clear PWR_ISO_BIT
4) Clear L1_PDN to power on L1
5) Wait L1_PDN_ACK
6) Set SRAM_ISOINT_B
7) Clear SRAM_CKISO
8) Clear PWR_CLK_DIS
9) Set PWR_RST_B
For multi-cluster SoC, the cluster power sequence is also different.

Please think if this is a good idea if we integrate the CPU support into
the scpsys_power_on()? Based on the readability and compatible
considerations, we provide this patch.
Thanks
Scott

2015-08-06 06:19:26

by Sascha Hauer

[permalink] [raw]
Subject: Re: [PATCH v3 5/8] ARM: mediatek: add smp bringup code for MT6580

On Wed, Aug 05, 2015 at 06:47:03PM +0200, Matthias Brugger wrote:
> On Tuesday, August 04, 2015 09:54:21 PM Scott Shu wrote:
> > Add support for cpu enable-method "mediatek,mt6580-smp" for booting
> > secondary CPUs on MT6580.
> >
> > Signed-off-by: Scott Shu <[email protected]>
> > ---
> > arch/arm/mach-mediatek/platsmp.c | 137
> > ++++++++++++++++++++++++++++++++++++++ 1 file changed, 137 insertions(+)
> > +
> > +static void __init mt6580_smp_prepare_cpus(unsigned int max_cpus)
> > +{
> > + static void __iomem *infracfg_ao_base;
> > +
> > + infracfg_ao_base = ioremap(MT6580_INFRACFG_AO, 0x1000);
> > + if (!infracfg_ao_base) {
> > + pr_err("%s: Unable to map I/O memory\n", __func__);
> > + return;
> > + }
> > +
> > + /* Enable bootrom power down mode */
> > + writel_relaxed(readl(infracfg_ao_base + 0x804) | SW_ROM_PD,
> > + infracfg_ao_base + 0x804);
>
> Please use a define for the offset.
> I prefer to not cascade reads and writes but to use a temporary variable. But
> if this is fine with the kernel coding style (I doubt, but I'm not sure) then
> this is fine.

For what it's worth I also prefer

val = readl(infracfg_ao_base + 0x804);
val |= SW_ROM_PD:
writel_relaxed(val, infracfg_ao_base + 0x804);

I find this better readable. I don't think though that there's common
agreement that this style is better.

Sascha

--
Pengutronix e.K. | |
Industrial Linux Solutions | http://www.pengutronix.de/ |
Peiner Str. 6-8, 31137 Hildesheim, Germany | Phone: +49-5121-206917-0 |
Amtsgericht Hildesheim, HRA 2686 | Fax: +49-5121-206917-5555 |

2015-08-06 10:03:20

by Sascha Hauer

[permalink] [raw]
Subject: Re: [PATCH v3 3/8] soc: Mediatek: Add SCPSYS CPU power domain driver

On Thu, Aug 06, 2015 at 10:59:02AM +0800, Scott Shu wrote:
> On Wed, 2015-08-05 at 10:50 +0200, Sascha Hauer wrote:
> > don't do this then it indeed doesn't make much sense to put it into the
> > same file.
> >
> > From what I see we would need to change the prototype to something like
> >
> > static int __scpsys_power_on(struct scp_domain_data *)
> >
> > (maybe with some additional base addresses and stuff)
> >
> > struct scp_domain_data would additionally need sram_isoint_b and sram_ckiso
> > members.
> >
> > Sascha
> >
> Hi Sascha,
> The CPU power sequence is quite different with the others, as
> described below.
>
> * Non-CPU
> 1) Set PWR_ON_BIT, PWR_ON_2ND_BIT
> 2) Wait PWR_ACK
> 3) Clear PWR_CLK_DIS_BIT
> 4) Clear PWR_ISO_BIT
> 5) Set PWR_RST_B_BIT
> 6) Clear SRAM_PDN
> 7) Wait SRAM_PDN_ACK
> * CPU
> 1) Set PWR_ON_BIT, PWR_ON_2ND_BIT
> 2) Wait PWR_ACK
> 3) Clear PWR_ISO_BIT
> 4) Clear L1_PDN to power on L1
> 5) Wait L1_PDN_ACK
> 6) Set SRAM_ISOINT_B
> 7) Clear SRAM_CKISO
> 8) Clear PWR_CLK_DIS
> 9) Set PWR_RST_B
> For multi-cluster SoC, the cluster power sequence is also different.
>
> Please think if this is a good idea if we integrate the CPU support into
> the scpsys_power_on()? Based on the readability and compatible
> considerations, we provide this patch.

Maybe it's best if you go back to the v1 layout and put your scpsys code
to arch/arm/mach-mediatek/. While I think it's possible to share some
more code I am not sure anymore if this buys us something. We'll know in
the future.

Sascha


--
Pengutronix e.K. | |
Industrial Linux Solutions | http://www.pengutronix.de/ |
Peiner Str. 6-8, 31137 Hildesheim, Germany | Phone: +49-5121-206917-0 |
Amtsgericht Hildesheim, HRA 2686 | Fax: +49-5121-206917-5555 |

2015-08-06 14:00:50

by Scott Shu

[permalink] [raw]
Subject: Re: [PATCH v3 5/8] ARM: mediatek: add smp bringup code for MT6580

On Wed, 2015-08-05 at 18:47 +0200, Matthias Brugger wrote:
> On Tuesday, August 04, 2015 09:54:21 PM Scott Shu wrote:
> > Add support for cpu enable-method "mediatek,mt6580-smp" for booting
> > secondary CPUs on MT6580.
> >
> > Signed-off-by: Scott Shu <[email protected]>
> > ---
> > arch/arm/mach-mediatek/platsmp.c | 137
> > ++++++++++++++++++++++++++++++++++++++ 1 file changed, 137 insertions(+)
> >
> > diff --git a/arch/arm/mach-mediatek/platsmp.c
> > b/arch/arm/mach-mediatek/platsmp.c index a5bc108..94f7865 100644
> > --- a/arch/arm/mach-mediatek/platsmp.c
> > +++ b/arch/arm/mach-mediatek/platsmp.c
> > @@ -21,10 +21,16 @@
> > #include <linux/of_address.h>
> > #include <linux/string.h>
> > #include <linux/threads.h>
> > +#include <linux/delay.h>
> > +#include <asm/cacheflush.h>
> > +
> > +#include <linux/soc/mediatek/scpsys.h>
> >
> > #define MTK_MAX_CPU 8
> > #define MTK_SMP_REG_SIZE 0x1000
> >
> > +static DEFINE_SPINLOCK(boot_lock);
> > +
> > struct mtk_smp_boot_info {
> > unsigned long smp_base;
> > unsigned int jump_reg;
> > @@ -56,6 +62,126 @@ static const struct of_device_id mtk_smp_boot_infos[]
> > __initconst = { static void __iomem *mtk_smp_base;
> > static const struct mtk_smp_boot_info *mtk_smp_info;
> >
> > +#ifdef CONFIG_HOTPLUG_CPU
> > +static int mt6580_cpu_kill(unsigned cpu)
> > +{
> > + int ret;
> > +
> > + ret = spm_cpu_mtcmos_off(cpu, 1);
>
> is this function ever called with wfi == 0?

Yes, wfi == 0 at MT6582 "special" dual-core platform, but for MT6580,
wfi is always 1.

> > + if (ret < 0)
> > + return 0;
> > +
> > + return 1;
> > +}
> > +
> > +static void mt6580_cpu_die(unsigned int cpu)
> > +{
> > + for (;;)
> > + cpu_do_idle();
> > +}
> > +#endif
> > +
> > +static void write_pen_release(int val)
> > +{
> > + pen_release = val;
> > + /* Make sure this is visible to other CPUs */
> > + smp_wmb();
> > + sync_cache_w(&pen_release);
> > +}
> > +
> > +/*
> > + * Refer common "pen" secondary release method
> > + */
> > +static int mt6580_boot_secondary(unsigned int cpu, struct task_struct
> > *idle) +{
> > + unsigned long timeout;
> > + int ret;
> > +
> > + /*
> > + * Set synchronisation state between this boot processor
> > + * and the secondary one
> > + */
> > + spin_lock(&boot_lock);
> > +
> > + /*
> > + * The secondary processor is waiting to be released from
> > + * the holding pen - release it, then wait for it to flag
> > + * that it has been released by resetting pen_release.
> > + *
> > + * Note that "pen_release" is the hardware CPU ID, whereas
> > + * "cpu" is Linux's internal ID.
> > + */
> > + write_pen_release(cpu);
> > +
> > + /*
> > + * CPU power on control by SPM
> > + */
> > + ret = spm_cpu_mtcmos_on(cpu);
> > + if (ret < 0) {
> > + spin_unlock(&boot_lock);
> > + return -ENXIO;
> > + }
> > +
> > + timeout = jiffies + (1 * HZ);
> > + while (time_before(jiffies, timeout)) {
> > + /* Read barrier */
> > + smp_rmb();
> > +
> > + if (pen_release == -1)
> > + break;
> > +
> > + udelay(10);
> > + }
> > +
> > + /*
> > + * Now the secondary core is starting up let it run its
> > + * calibrations, then wait for it to finish
> > + */
> > + spin_unlock(&boot_lock);
> > +
> > + return (pen_release != -1 ? -ENXIO : 0);
> > +}
> > +
> > +static void mt6580_secondary_init(unsigned int cpu)
> > +{
> > + /*
> > + * Let the primary processor know we're out of the
> > + * pen, then head off into the C entry point
> > + */
> > + write_pen_release(-1);
> > +
> > + /*
> > + * Synchronise with the boot thread.
> > + */
> > + spin_lock(&boot_lock);
> > + spin_unlock(&boot_lock);
> > +}
> > +
> > +#define MT6580_INFRACFG_AO 0x10001000
> > +#define SW_ROM_PD BIT(31)
>
> Please put the defines on the beginning of the file.
> SW_ROM_PD bit is the same on other SoCs? I wasn't able to reveal that from the
> data sheets. If this is MT6580 only, the define should be renamed.
>

OK, fixed in next version.

> > +
> > +static void __init mt6580_smp_prepare_cpus(unsigned int max_cpus)
> > +{
> > + static void __iomem *infracfg_ao_base;
> > +
> > + infracfg_ao_base = ioremap(MT6580_INFRACFG_AO, 0x1000);
> > + if (!infracfg_ao_base) {
> > + pr_err("%s: Unable to map I/O memory\n", __func__);
> > + return;
> > + }
> > +
> > + /* Enable bootrom power down mode */
> > + writel_relaxed(readl(infracfg_ao_base + 0x804) | SW_ROM_PD,
> > + infracfg_ao_base + 0x804);
>
> Please use a define for the offset.
> I prefer to not cascade reads and writes but to use a temporary variable. But
> if this is fine with the kernel coding style (I doubt, but I'm not sure) then
> this is fine.
>

OK, fixed in next version.

Thanks,
Scott
> > +
> > + /* Write the address of slave startup into boot address
> > + register for bootrom power down mode */
> > + writel_relaxed(virt_to_phys(secondary_startup_arm),
> > + infracfg_ao_base + 0x800);
> > +
> > + iounmap(infracfg_ao_base);
> > +}
> > +
> > static int mtk_boot_secondary(unsigned int cpu, struct task_struct *idle)
> > {
> > if (!mtk_smp_base)
> > @@ -142,3 +268,14 @@ static struct smp_operations mt6589_smp_ops __initdata
> > = { .smp_boot_secondary = mtk_boot_secondary,
> > };
> > CPU_METHOD_OF_DECLARE(mt6589_smp, "mediatek,mt6589-smp", &mt6589_smp_ops);
> > +
> > +static struct smp_operations mt6580_smp_ops __initdata = {
> > + .smp_prepare_cpus = mt6580_smp_prepare_cpus,
> > + .smp_secondary_init = mt6580_secondary_init,
> > + .smp_boot_secondary = mt6580_boot_secondary,
> > +#ifdef CONFIG_HOTPLUG_CPU
> > + .cpu_kill = mt6580_cpu_kill,
> > + .cpu_die = mt6580_cpu_die,
> > +#endif
> > +};
> > +CPU_METHOD_OF_DECLARE(mt6580_smp, "mediatek,mt6580-smp", &mt6580_smp_ops);
>

2015-08-07 01:57:05

by Scott Shu

[permalink] [raw]
Subject: Re: [PATCH v3 3/8] soc: Mediatek: Add SCPSYS CPU power domain driver

On Thu, 2015-08-06 at 12:03 +0200, Sascha Hauer wrote:
> On Thu, Aug 06, 2015 at 10:59:02AM +0800, Scott Shu wrote:
> > On Wed, 2015-08-05 at 10:50 +0200, Sascha Hauer wrote:
> > > don't do this then it indeed doesn't make much sense to put it into the
> > > same file.
> > >
> > > From what I see we would need to change the prototype to something like
> > >
> > > static int __scpsys_power_on(struct scp_domain_data *)
> > >
> > > (maybe with some additional base addresses and stuff)
> > >
> > > struct scp_domain_data would additionally need sram_isoint_b and sram_ckiso
> > > members.
> > >
> > > Sascha
> > >
> > Hi Sascha,
> > The CPU power sequence is quite different with the others, as
> > described below.
> >
> > * Non-CPU
> > 1) Set PWR_ON_BIT, PWR_ON_2ND_BIT
> > 2) Wait PWR_ACK
> > 3) Clear PWR_CLK_DIS_BIT
> > 4) Clear PWR_ISO_BIT
> > 5) Set PWR_RST_B_BIT
> > 6) Clear SRAM_PDN
> > 7) Wait SRAM_PDN_ACK
> > * CPU
> > 1) Set PWR_ON_BIT, PWR_ON_2ND_BIT
> > 2) Wait PWR_ACK
> > 3) Clear PWR_ISO_BIT
> > 4) Clear L1_PDN to power on L1
> > 5) Wait L1_PDN_ACK
> > 6) Set SRAM_ISOINT_B
> > 7) Clear SRAM_CKISO
> > 8) Clear PWR_CLK_DIS
> > 9) Set PWR_RST_B
> > For multi-cluster SoC, the cluster power sequence is also different.
> >
> > Please think if this is a good idea if we integrate the CPU support into
> > the scpsys_power_on()? Based on the readability and compatible
> > considerations, we provide this patch.
>
> Maybe it's best if you go back to the v1 layout and put your scpsys code
> to arch/arm/mach-mediatek/. While I think it's possible to share some
> more code I am not sure anymore if this buys us something. We'll know in
> the future.
>
> Sascha
>
>
Thanks Sascha. If you agree, we prefer to keep v3 layout. It's good to
put all scpsys related code and definitions in the same place (your
driver) and limit the CPU MTCMOS code are all located in button half of
the file. We may also change the function name, for example, rename
spm_cpu_mtcmos_on/off to scpsys_cpu_power_on/off, to make a better
overall sense.

Scott

2015-08-07 01:59:51

by Scott Shu

[permalink] [raw]
Subject: Re: [PATCH v3 5/8] ARM: mediatek: add smp bringup code for MT6580

On Thu, 2015-08-06 at 08:19 +0200, Sascha Hauer wrote:
> On Wed, Aug 05, 2015 at 06:47:03PM +0200, Matthias Brugger wrote:
> > On Tuesday, August 04, 2015 09:54:21 PM Scott Shu wrote:
> > > Add support for cpu enable-method "mediatek,mt6580-smp" for booting
> > > secondary CPUs on MT6580.
> > >
> > > Signed-off-by: Scott Shu <[email protected]>
> > > ---
> > > arch/arm/mach-mediatek/platsmp.c | 137
> > > ++++++++++++++++++++++++++++++++++++++ 1 file changed, 137 insertions(+)
> > > +
> > > +static void __init mt6580_smp_prepare_cpus(unsigned int max_cpus)
> > > +{
> > > + static void __iomem *infracfg_ao_base;
> > > +
> > > + infracfg_ao_base = ioremap(MT6580_INFRACFG_AO, 0x1000);
> > > + if (!infracfg_ao_base) {
> > > + pr_err("%s: Unable to map I/O memory\n", __func__);
> > > + return;
> > > + }
> > > +
> > > + /* Enable bootrom power down mode */
> > > + writel_relaxed(readl(infracfg_ao_base + 0x804) | SW_ROM_PD,
> > > + infracfg_ao_base + 0x804);
> >
> > Please use a define for the offset.
> > I prefer to not cascade reads and writes but to use a temporary variable. But
> > if this is fine with the kernel coding style (I doubt, but I'm not sure) then
> > this is fine.
>
> For what it's worth I also prefer
>
> val = readl(infracfg_ao_base + 0x804);
> val |= SW_ROM_PD:
> writel_relaxed(val, infracfg_ao_base + 0x804);
>
> I find this better readable. I don't think though that there's common
> agreement that this style is better.
>
> Sascha
>
Fixed in next version. Thanks.

Scott

2015-08-07 02:28:20

by Scott Shu

[permalink] [raw]
Subject: Re: [PATCH v3 5/8] ARM: mediatek: add smp bringup code for MT6580

On Wed, 2015-08-05 at 10:44 +0100, Russell King - ARM Linux wrote:
> On Tue, Aug 04, 2015 at 09:54:21PM +0800, Scott Shu wrote:
> > Add support for cpu enable-method "mediatek,mt6580-smp" for booting
> > secondary CPUs on MT6580.
>
> If you have CPU power domain support, and you power up and power down the
> CPUs, why do you need the boot_lock and pen_release stuff? Isn't keeping
> the power off on a CPU sufficient to prevent it entering the kernel?
>
Thanks Russell. Yes, the system works well after removing boot_lock and
pen_release. Will be fixed in next version.

Scott