[RFC PATCH 0/5] Add smp booting support for Qualcomm ARMv8 SoCs

[RFC PATCH 0/5] Add smp booting support for Qualcomm ARMv8 SoCs

[Kumar Gala]

This patch set adds support for SMP boot on the MSM8x16 family of Qualcomm SoCs.

To support SMP on the MSM8x16 SoCs we need to add ARMv8/64-bit SCM interfaces to
setup the boot/release addresses for the secondary CPUs.  In addition we need
a uniquie set of cpu ops.  I'm aware the desired methods for booting secondary
CPUs is either via spintable or PSCI.  However, these SoCs are shipping with a
firmware that does not support those methods.

- k

-- 
Qualcomm Innovation Center, Inc.
The Qualcomm Innovation Center, Inc. is a member of the Code Aurora Forum,
a Linux Foundation Collaborative Project

[RFC PATCH 3/5] arm64: introduce CPU_OF_TABLES for cpu ops selection

[Kumar Gala]

From: Abhimanyu Kapur <abhimany@codeaurora.org>

Add support to arm64 to provide a dt-based method to allow soc-vendors to
supply cpu_ops. Also move psci and smp_spin_table ops to use CPU_OF_TABLES.

Signed-off-by: Abhimanyu Kapur <abhimany@codeaurora.org>
Signed-off-by: Kumar Gala <galak@codeaurora.org>
---
 arch/arm64/include/asm/cpu_ops.h   |  5 +++++
 arch/arm64/kernel/cpu_ops.c        | 27 +++++++++------------------
 arch/arm64/kernel/psci.c           |  1 +
 arch/arm64/kernel/smp_spin_table.c |  3 ++-
 4 files changed, 17 insertions(+), 19 deletions(-)

diff --git a/arch/arm64/include/asm/cpu_ops.h b/arch/arm64/include/asm/cpu_ops.h
index da301ee..a7efab8 100644
--- a/arch/arm64/include/asm/cpu_ops.h
+++ b/arch/arm64/include/asm/cpu_ops.h
@@ -67,4 +67,9 @@ extern const struct cpu_operations *cpu_ops[NR_CPUS];
 int __init cpu_read_ops(struct device_node *dn, int cpu);
 void __init cpu_read_bootcpu_ops(void);
 
+#define CPU_METHOD_OF_DECLARE(name, __ops)				\
+	static const struct cpu_operations *__cpu_method_table_##name	\
+	__used __section(__cpu_method_of_table)				\
+	= __ops;
+
 #endif /* ifndef __ASM_CPU_OPS_H */
diff --git a/arch/arm64/kernel/cpu_ops.c b/arch/arm64/kernel/cpu_ops.c
index cce9524..ad33f98 100644
--- a/arch/arm64/kernel/cpu_ops.c
+++ b/arch/arm64/kernel/cpu_ops.c
@@ -22,29 +22,20 @@
 #include <linux/of.h>
 #include <linux/string.h>
 
-extern const struct cpu_operations smp_spin_table_ops;
-extern const struct cpu_operations cpu_psci_ops;
-
 const struct cpu_operations *cpu_ops[NR_CPUS];
 
-static const struct cpu_operations *supported_cpu_ops[] __initconst = {
-#ifdef CONFIG_SMP
-	&smp_spin_table_ops,
-#endif
-	&cpu_psci_ops,
-	NULL,
-};
+extern struct cpu_operations __cpu_method_of_table[];
+static const struct cpu_operations *__cpu_method_of_table_sentinel
+	__used __section(__cpu_method_of_table_end);
 
-static const struct cpu_operations * __init cpu_get_ops(const char *name)
+const struct cpu_operations * __init cpu_get_ops(const char *name)
 {
-	const struct cpu_operations **ops = supported_cpu_ops;
-
-	while (*ops) {
-		if (!strcmp(name, (*ops)->name))
-			return *ops;
+	const struct cpu_operations **start = (void *)__cpu_method_of_table;
 
-		ops++;
-	}
+	for (; *start; start++) {
+		if (!strcmp((*start)->name, name))
+			return *start;
+	};
 
 	return NULL;
 }
diff --git a/arch/arm64/kernel/psci.c b/arch/arm64/kernel/psci.c
index 9b8a70a..2f255c7 100644
--- a/arch/arm64/kernel/psci.c
+++ b/arch/arm64/kernel/psci.c
@@ -523,3 +523,4 @@ const struct cpu_operations cpu_psci_ops = {
 #endif
 };
 
+CPU_METHOD_OF_DECLARE(psci, &cpu_psci_ops);
diff --git a/arch/arm64/kernel/smp_spin_table.c b/arch/arm64/kernel/smp_spin_table.c
index 14944e5..b41a8b4 100644
--- a/arch/arm64/kernel/smp_spin_table.c
+++ b/arch/arm64/kernel/smp_spin_table.c
@@ -119,9 +119,10 @@ static int smp_spin_table_cpu_boot(unsigned int cpu)
 	return 0;
 }
 
-const struct cpu_operations smp_spin_table_ops = {
+static const struct cpu_operations smp_spin_table_ops = {
 	.name		= "spin-table",
 	.cpu_init	= smp_spin_table_cpu_init,
 	.cpu_prepare	= smp_spin_table_cpu_prepare,
 	.cpu_boot	= smp_spin_table_cpu_boot,
 };
+CPU_METHOD_OF_DECLARE(spin_table, &smp_spin_table_ops);
-- 
Qualcomm Innovation Center, Inc.
The Qualcomm Innovation Center, Inc. is a member of the Code Aurora Forum,
a Linux Foundation Collaborative Project

Re: [RFC PATCH 3/5] arm64: introduce CPU_OF_TABLES for cpu ops selection

[Arnd Bergmann]

On Thursday 09 April 2015 12:37:09 Kumar Gala wrote:
> @@ -67,4 +67,9 @@ extern const struct cpu_operations *cpu_ops[NR_CPUS];
>  int __init cpu_read_ops(struct device_node *dn, int cpu);
>  void __init cpu_read_bootcpu_ops(void);
>  
> +#define CPU_METHOD_OF_DECLARE(name, __ops)                             \
> +       static const struct cpu_operations *__cpu_method_table_##name   \
> +       __used __section(__cpu_method_of_table)                         \
> +       = __ops;
> +
>  #endif /* ifndef __ASM_CPU_OPS_H */
> 

I'd rather not add this, to avoid giving the appearance that platforms
can just add another one here.

	Arnd

Re: [RFC PATCH 3/5] arm64: introduce CPU_OF_TABLES for cpu ops selection

[Mark Rutland]

On Thu, Apr 09, 2015 at 10:17:06PM +0100, Arnd Bergmann wrote:
> On Thursday 09 April 2015 12:37:09 Kumar Gala wrote:
> > @@ -67,4 +67,9 @@ extern const struct cpu_operations *cpu_ops[NR_CPUS];
> >  int __init cpu_read_ops(struct device_node *dn, int cpu);
> >  void __init cpu_read_bootcpu_ops(void);
> >  
> > +#define CPU_METHOD_OF_DECLARE(name, __ops)                             \
> > +       static const struct cpu_operations *__cpu_method_table_##name   \
> > +       __used __section(__cpu_method_of_table)                         \
> > +       = __ops;
> > +
> >  #endif /* ifndef __ASM_CPU_OPS_H */
> > 
> 
> I'd rather not add this, to avoid giving the appearance that platforms
> can just add another one here.

Likewise.

SMP bringup is always hairy, and giving the appearance that such code
should be hidden away somewhere (and missing critical review) is a bad
idea.

While this seemed like a good idea in the past for 32-bit, we've only
seen marginal decoupling, and very little in the way of actual semantics
defined for the enable-methods. Which effectively spreads implementation
details further rather than separating them from the interface.

Mark.
--
To unsubscribe from this list: send the line "unsubscribe devicetree" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [RFC PATCH 3/5] arm64: introduce CPU_OF_TABLES for cpu ops selection

[Lorenzo Pieralisi]

On Thu, Apr 09, 2015 at 06:37:09PM +0100, Kumar Gala wrote:
> From: Abhimanyu Kapur <abhimany@codeaurora.org>
> 
> Add support to arm64 to provide a dt-based method to allow soc-vendors to
> supply cpu_ops. Also move psci and smp_spin_table ops to use CPU_OF_TABLES.

On arm64 there are two enable methods: PSCI and spin-table.

Update your firmware accordingly and drop this patch, thanks.

Lorenzo

> Signed-off-by: Abhimanyu Kapur <abhimany@codeaurora.org>
> Signed-off-by: Kumar Gala <galak@codeaurora.org>
> ---
>  arch/arm64/include/asm/cpu_ops.h   |  5 +++++
>  arch/arm64/kernel/cpu_ops.c        | 27 +++++++++------------------
>  arch/arm64/kernel/psci.c           |  1 +
>  arch/arm64/kernel/smp_spin_table.c |  3 ++-
>  4 files changed, 17 insertions(+), 19 deletions(-)
> 
> diff --git a/arch/arm64/include/asm/cpu_ops.h b/arch/arm64/include/asm/cpu_ops.h
> index da301ee..a7efab8 100644
> --- a/arch/arm64/include/asm/cpu_ops.h
> +++ b/arch/arm64/include/asm/cpu_ops.h
> @@ -67,4 +67,9 @@ extern const struct cpu_operations *cpu_ops[NR_CPUS];
>  int __init cpu_read_ops(struct device_node *dn, int cpu);
>  void __init cpu_read_bootcpu_ops(void);
>  
> +#define CPU_METHOD_OF_DECLARE(name, __ops)				\
> +	static const struct cpu_operations *__cpu_method_table_##name	\
> +	__used __section(__cpu_method_of_table)				\
> +	= __ops;
> +
>  #endif /* ifndef __ASM_CPU_OPS_H */
> diff --git a/arch/arm64/kernel/cpu_ops.c b/arch/arm64/kernel/cpu_ops.c
> index cce9524..ad33f98 100644
> --- a/arch/arm64/kernel/cpu_ops.c
> +++ b/arch/arm64/kernel/cpu_ops.c
> @@ -22,29 +22,20 @@
>  #include <linux/of.h>
>  #include <linux/string.h>
>  
> -extern const struct cpu_operations smp_spin_table_ops;
> -extern const struct cpu_operations cpu_psci_ops;
> -
>  const struct cpu_operations *cpu_ops[NR_CPUS];
>  
> -static const struct cpu_operations *supported_cpu_ops[] __initconst = {
> -#ifdef CONFIG_SMP
> -	&smp_spin_table_ops,
> -#endif
> -	&cpu_psci_ops,
> -	NULL,
> -};
> +extern struct cpu_operations __cpu_method_of_table[];
> +static const struct cpu_operations *__cpu_method_of_table_sentinel
> +	__used __section(__cpu_method_of_table_end);
>  
> -static const struct cpu_operations * __init cpu_get_ops(const char *name)
> +const struct cpu_operations * __init cpu_get_ops(const char *name)
>  {
> -	const struct cpu_operations **ops = supported_cpu_ops;
> -
> -	while (*ops) {
> -		if (!strcmp(name, (*ops)->name))
> -			return *ops;
> +	const struct cpu_operations **start = (void *)__cpu_method_of_table;
>  
> -		ops++;
> -	}
> +	for (; *start; start++) {
> +		if (!strcmp((*start)->name, name))
> +			return *start;
> +	};
>  
>  	return NULL;
>  }
> diff --git a/arch/arm64/kernel/psci.c b/arch/arm64/kernel/psci.c
> index 9b8a70a..2f255c7 100644
> --- a/arch/arm64/kernel/psci.c
> +++ b/arch/arm64/kernel/psci.c
> @@ -523,3 +523,4 @@ const struct cpu_operations cpu_psci_ops = {
>  #endif
>  };
>  
> +CPU_METHOD_OF_DECLARE(psci, &cpu_psci_ops);
> diff --git a/arch/arm64/kernel/smp_spin_table.c b/arch/arm64/kernel/smp_spin_table.c
> index 14944e5..b41a8b4 100644
> --- a/arch/arm64/kernel/smp_spin_table.c
> +++ b/arch/arm64/kernel/smp_spin_table.c
> @@ -119,9 +119,10 @@ static int smp_spin_table_cpu_boot(unsigned int cpu)
>  	return 0;
>  }
>  
> -const struct cpu_operations smp_spin_table_ops = {
> +static const struct cpu_operations smp_spin_table_ops = {
>  	.name		= "spin-table",
>  	.cpu_init	= smp_spin_table_cpu_init,
>  	.cpu_prepare	= smp_spin_table_cpu_prepare,
>  	.cpu_boot	= smp_spin_table_cpu_boot,
>  };
> +CPU_METHOD_OF_DECLARE(spin_table, &smp_spin_table_ops);
> -- 
> Qualcomm Innovation Center, Inc.
> The Qualcomm Innovation Center, Inc. is a member of the Code Aurora Forum,
> a Linux Foundation Collaborative Project
> 
> --
> To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
> the body of a message to majordomo@vger.kernel.org
> More majordomo info at  http://vger.kernel.org/majordomo-info.html
> Please read the FAQ at  http://www.tux.org/lkml/
> 

[RFC PATCH 5/5] arm64: qcom: add cpu operations

[Kumar Gala]

From: Abhimanyu Kapur <abhimany-sgV2jX0FEOL9JmXXK+q4OQ@public.gmane.org>

Add qcom cpu operations for arm-v8 cpus. Implement secondary cpu boot ops
As a part of this change update device tree documentation for:

1. Arm cortex-a ACC device which provides percpu reg
2. Armv8 cortex-a compatible string in arm/cpus.txt

Signed-off-by: Abhimanyu Kapur <abhimany-sgV2jX0FEOL9JmXXK+q4OQ@public.gmane.org>
Signed-off-by: Kumar Gala <galak-sgV2jX0FEOL9JmXXK+q4OQ@public.gmane.org>
---
 Documentation/devicetree/bindings/arm/cpus.txt    |   2 +
 Documentation/devicetree/bindings/arm/msm/acc.txt |  19 ++
 drivers/soc/qcom/Makefile                         |   1 +
 drivers/soc/qcom/cpu_ops.c                        | 343 ++++++++++++++++++++++
 4 files changed, 365 insertions(+)
 create mode 100644 Documentation/devicetree/bindings/arm/msm/acc.txt
 create mode 100644 drivers/soc/qcom/cpu_ops.c

diff --git a/Documentation/devicetree/bindings/arm/cpus.txt b/Documentation/devicetree/bindings/arm/cpus.txt
index 8b9e0a9..35cabe5 100644
--- a/Documentation/devicetree/bindings/arm/cpus.txt
+++ b/Documentation/devicetree/bindings/arm/cpus.txt
@@ -185,6 +185,8 @@ nodes to be present and contain the properties described below.
 			  be one of:
 			     "psci"
 			     "spin-table"
+			     "qcom,arm-cortex-acc"
+
 			# On ARM 32-bit systems this property is optional and
 			  can be one of:
 			    "allwinner,sun6i-a31"
diff --git a/Documentation/devicetree/bindings/arm/msm/acc.txt b/Documentation/devicetree/bindings/arm/msm/acc.txt
new file mode 100644
index 0000000..ae2d725
--- /dev/null
+++ b/Documentation/devicetree/bindings/arm/msm/acc.txt
@@ -0,0 +1,19 @@
+Application Processor Sub-system (APSS) Application Clock Controller (ACC)
+
+The ACC provides clock, power domain, and reset control to a CPU. There is one ACC
+register region per CPU within the APSS remapped region as well as an alias register
+region that remaps accesses to the ACC associated with the CPU accessing the region.
+
+Required properties:
+- compatible:		Must be "qcom,arm-cortex-acc"
+- reg:			The first element specifies the base address and size of
+			the register region. An optional second element specifies
+			the base address and size of the alias register region.
+
+Example:
+
+	clock-controller@b088000 {
+		compatible = "qcom,arm-cortex-acc";
+		reg = <0x0b088000 0x1000>,
+		      <0x0b008000 0x1000>;
+	}
diff --git a/drivers/soc/qcom/Makefile b/drivers/soc/qcom/Makefile
index 4389012..bb6030a 100644
--- a/drivers/soc/qcom/Makefile
+++ b/drivers/soc/qcom/Makefile
@@ -1 +1,2 @@
+obj-$(CONFIG_ARM64)	+=	cpu_ops.o
 obj-$(CONFIG_QCOM_GSBI)	+=	qcom_gsbi.o
diff --git a/drivers/soc/qcom/cpu_ops.c b/drivers/soc/qcom/cpu_ops.c
new file mode 100644
index 0000000..d831cb0
--- /dev/null
+++ b/drivers/soc/qcom/cpu_ops.c
@@ -0,0 +1,343 @@
+/* Copyright (c) 2014, The Linux Foundation. All rights reserved.
+ * Copyright (c) 2013 ARM Ltd.
+ *
+ * This program is free software; you can redistribute it and/or modify
+ * it under the terms of the GNU General Public License version 2 and
+ * only version 2 as published by the Free Software Foundation.
+ *
+ * This program is distributed in the hope that it will be useful,
+ * but WITHOUT ANY WARRANTY; without even the implied warranty of
+ * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
+ * GNU General Public License for more details.
+ */
+
+/* MSM ARMv8 CPU Operations
+ * Based on arch/arm64/kernel/smp_spin_table.c
+ */
+
+#include <linux/bitops.h>
+#include <linux/cpu.h>
+#include <linux/cpumask.h>
+#include <linux/delay.h>
+#include <linux/init.h>
+#include <linux/io.h>
+#include <linux/of.h>
+#include <linux/of_address.h>
+#include <linux/smp.h>
+#include <linux/qcom_scm.h>
+
+#include <asm/barrier.h>
+#include <asm/cacheflush.h>
+#include <asm/cpu_ops.h>
+#include <asm/cputype.h>
+#include <asm/smp_plat.h>
+
+static DEFINE_RAW_SPINLOCK(boot_lock);
+
+DEFINE_PER_CPU(int, cold_boot_done);
+
+#if 0
+static int cold_boot_flags[] = {
+	0,
+	QCOM_SCM_FLAG_COLDBOOT_CPU1,
+	QCOM_SCM_FLAG_COLDBOOT_CPU2,
+	QCOM_SCM_FLAG_COLDBOOT_CPU3,
+};
+#endif
+
+/* CPU power domain register offsets */
+#define CPU_PWR_CTL		0x4
+#define CPU_PWR_GATE_CTL	0x14
+#define LDO_BHS_PWR_CTL		0x28
+
+/* L2 power domain register offsets */
+#define L2_PWR_CTL_OVERRIDE	0xc
+#define L2_PWR_CTL		0x14
+#define L2_PWR_STATUS		0x18
+#define	L2_CORE_CBCR		0x58
+#define L1_RST_DIS		0x284
+
+#define L2_SPM_STS		0xc
+#define L2_VREG_CTL		0x1c
+
+#define SCM_IO_READ		1
+#define SCM_IO_WRITE		2
+
+/*
+ * struct msm_l2ccc_of_info: represents of data for l2 cache clock controller.
+ * @compat: compat string for l2 cache clock controller
+ * @l2_pon: l2 cache power on routine
+ */
+struct msm_l2ccc_of_info {
+	const char *compat;
+	int (*l2_power_on) (struct device_node *dn, u32 l2_mask, int cpu);
+	u32 l2_power_on_mask;
+};
+
+
+static int power_on_l2_msm8916(struct device_node *l2ccc_node, u32 pon_mask,
+				int cpu)
+{
+	u32 pon_status;
+	void __iomem *l2_base;
+
+	l2_base = of_iomap(l2ccc_node, 0);
+	if (!l2_base)
+		return -ENOMEM;
+
+	/* Skip power-on sequence if l2 cache is already powered up*/
+	pon_status = (__raw_readl(l2_base + L2_PWR_STATUS) & pon_mask)
+				== pon_mask;
+	if (pon_status) {
+		iounmap(l2_base);
+		return 0;
+	}
+
+	/* Close L2/SCU Logic GDHS and power up the cache */
+	writel_relaxed(0x10D700, l2_base + L2_PWR_CTL);
+
+	/* Assert PRESETDBGn */
+	writel_relaxed(0x400000, l2_base + L2_PWR_CTL_OVERRIDE);
+	mb();
+	udelay(2);
+
+	/* De-assert L2/SCU memory Clamp */
+	writel_relaxed(0x101700, l2_base + L2_PWR_CTL);
+
+	/* Wakeup L2/SCU RAMs by deasserting sleep signals */
+	writel_relaxed(0x101703, l2_base + L2_PWR_CTL);
+	mb();
+	udelay(2);
+
+	/* Enable clocks via SW_CLK_EN */
+	writel_relaxed(0x01, l2_base + L2_CORE_CBCR);
+
+	/* De-assert L2/SCU logic clamp */
+	writel_relaxed(0x101603, l2_base + L2_PWR_CTL);
+	mb();
+	udelay(2);
+
+	/* De-assert PRESSETDBg */
+	writel_relaxed(0x0, l2_base + L2_PWR_CTL_OVERRIDE);
+
+	/* De-assert L2/SCU Logic reset */
+	writel_relaxed(0x100203, l2_base + L2_PWR_CTL);
+	mb();
+	udelay(54);
+
+	/* Turn on the PMIC_APC */
+	writel_relaxed(0x10100203, l2_base + L2_PWR_CTL);
+
+	/* Set H/W clock control for the cpu CBC block */
+	writel_relaxed(0x03, l2_base + L2_CORE_CBCR);
+	mb();
+	iounmap(l2_base);
+
+	return 0;
+}
+
+static const struct msm_l2ccc_of_info l2ccc_info[] = {
+	{
+		.compat = "qcom,8916-l2ccc",
+		.l2_power_on = power_on_l2_msm8916,
+		.l2_power_on_mask = BIT(9),
+	},
+};
+
+static int power_on_l2_cache(struct device_node *l2ccc_node, int cpu)
+{
+	int ret, i;
+	const char *compat;
+
+	ret = of_property_read_string(l2ccc_node, "compatible", &compat);
+	if (ret)
+		return ret;
+
+	for (i = 0; i < ARRAY_SIZE(l2ccc_info); i++) {
+		const struct msm_l2ccc_of_info *ptr = &l2ccc_info[i];
+
+		if (!of_compat_cmp(ptr->compat, compat, strlen(compat)))
+				return ptr->l2_power_on(l2ccc_node,
+						ptr->l2_power_on_mask, cpu);
+	}
+	pr_err("Compat string not found for L2CCC node\n");
+	return -EIO;
+}
+
+static int msm_unclamp_secondary_arm_cpu(unsigned int cpu)
+{
+
+	int ret = 0;
+	struct device_node *cpu_node, *acc_node, *l2_node, *l2ccc_node;
+	void __iomem *reg;
+
+	cpu_node = of_get_cpu_node(cpu, NULL);
+	if (!cpu_node)
+		return -ENODEV;
+
+	acc_node = of_parse_phandle(cpu_node, "qcom,acc", 0);
+	if (!acc_node) {
+			ret = -ENODEV;
+			goto out_acc;
+	}
+
+	l2_node = of_parse_phandle(cpu_node, "next-level-cache", 0);
+	if (!l2_node) {
+		ret = -ENODEV;
+		goto out_l2;
+	}
+
+	l2ccc_node = of_parse_phandle(l2_node, "power-domain", 0);
+	if (!l2ccc_node) {
+		ret = -ENODEV;
+		goto out_l2;
+	}
+
+	/* Ensure L2-cache of the CPU is powered on before
+	 * unclamping cpu power rails.
+	 */
+	ret = power_on_l2_cache(l2ccc_node, cpu);
+	if (ret) {
+		pr_err("L2 cache power up failed for CPU%d\n", cpu);
+		goto out_l2ccc;
+	}
+
+	reg = of_iomap(acc_node, 0);
+	if (!reg) {
+		ret = -ENOMEM;
+		goto out_acc_reg;
+	}
+
+	/* Assert Reset on cpu-n */
+	writel_relaxed(0x00000033, reg + CPU_PWR_CTL);
+	mb();
+
+	/*Program skew to 16 X0 clock cycles*/
+	writel_relaxed(0x10000001, reg + CPU_PWR_GATE_CTL);
+	mb();
+	udelay(2);
+
+	/* De-assert coremem clamp */
+	writel_relaxed(0x00000031, reg + CPU_PWR_CTL);
+	mb();
+
+	/* Close coremem array gdhs */
+	writel_relaxed(0x00000039, reg + CPU_PWR_CTL);
+	mb();
+	udelay(2);
+
+	/* De-assert cpu-n clamp */
+	writel_relaxed(0x00020038, reg + CPU_PWR_CTL);
+	mb();
+	udelay(2);
+
+	/* De-assert cpu-n reset */
+	writel_relaxed(0x00020008, reg + CPU_PWR_CTL);
+	mb();
+
+	/* Assert PWRDUP signal on core-n */
+	writel_relaxed(0x00020088, reg + CPU_PWR_CTL);
+	mb();
+
+	/* Secondary CPU-N is now alive */
+	iounmap(reg);
+out_acc_reg:
+	of_node_put(l2ccc_node);
+out_l2ccc:
+	of_node_put(l2_node);
+out_l2:
+	of_node_put(acc_node);
+out_acc:
+	of_node_put(cpu_node);
+
+	return ret;
+}
+
+static void write_pen_release(u64 val)
+{
+	void *start = (void *)&secondary_holding_pen_release;
+	unsigned long size = sizeof(secondary_holding_pen_release);
+
+	secondary_holding_pen_release = val;
+	smp_wmb();
+	__flush_dcache_area(start, size);
+}
+
+static int secondary_pen_release(unsigned int cpu)
+{
+	unsigned long timeout;
+
+	/*
+	 * Set synchronisation state between this boot processor
+	 * and the secondary one
+	 */
+	raw_spin_lock(&boot_lock);
+	write_pen_release(cpu_logical_map(cpu));
+
+	timeout = jiffies + (1 * HZ);
+	while (time_before(jiffies, timeout)) {
+		if (secondary_holding_pen_release == INVALID_HWID)
+			break;
+		udelay(10);
+	}
+	raw_spin_unlock(&boot_lock);
+
+	return secondary_holding_pen_release != INVALID_HWID ? -ENOSYS : 0;
+}
+
+static int __init msm_cpu_init(struct device_node *dn, unsigned int cpu)
+{
+	/* Mark CPU0 cold boot flag as done */
+	if (!cpu && !per_cpu(cold_boot_done, cpu))
+		per_cpu(cold_boot_done, cpu) = true;
+
+	return 0;
+}
+
+static int __init msm_cpu_prepare(unsigned int cpu)
+{
+	const cpumask_t *mask = cpumask_of(cpu);
+
+	if (qcom_scm_set_cold_boot_addr(secondary_holding_pen, mask)) {
+		pr_warn("CPU%d:Failed to set boot address\n", cpu);
+		return -ENOSYS;
+	}
+
+	return 0;
+}
+
+static int msm_cpu_boot(unsigned int cpu)
+{
+	int ret = 0;
+
+	if (per_cpu(cold_boot_done, cpu) == false) {
+		ret = msm_unclamp_secondary_arm_cpu(cpu);
+		if (ret)
+			return ret;
+		per_cpu(cold_boot_done, cpu) = true;
+	}
+	return secondary_pen_release(cpu);
+}
+
+void msm_cpu_postboot(void)
+{
+	/*
+	 * Let the primary processor know we're out of the pen.
+	 */
+	write_pen_release(INVALID_HWID);
+
+	/*
+	 * Synchronise with the boot thread.
+	 */
+	raw_spin_lock(&boot_lock);
+	raw_spin_unlock(&boot_lock);
+}
+
+static const struct cpu_operations msm_cortex_a_ops = {
+	.name		= "qcom,arm-cortex-acc",
+	.cpu_init	= msm_cpu_init,
+	.cpu_prepare	= msm_cpu_prepare,
+	.cpu_boot	= msm_cpu_boot,
+	.cpu_postboot	= msm_cpu_postboot,
+};
+CPU_METHOD_OF_DECLARE(msm_cortex_a_ops, &msm_cortex_a_ops);
-- 
Qualcomm Innovation Center, Inc.
The Qualcomm Innovation Center, Inc. is a member of the Code Aurora Forum,
a Linux Foundation Collaborative Project

--
To unsubscribe from this list: send the line "unsubscribe devicetree" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [RFC PATCH 5/5] arm64: qcom: add cpu operations

[Arnd Bergmann]

On Thursday 09 April 2015 12:37:11 Kumar Gala wrote:
> From: Abhimanyu Kapur <abhimany@codeaurora.org>
> 
> Add qcom cpu operations for arm-v8 cpus. Implement secondary cpu boot ops
> As a part of this change update device tree documentation for:
> 
> 1. Arm cortex-a ACC device which provides percpu reg
> 2. Armv8 cortex-a compatible string in arm/cpus.txt
> 
> Signed-off-by: Abhimanyu Kapur <abhimany@codeaurora.org>
> Signed-off-by: Kumar Gala <galak@codeaurora.org>
> ---
>  Documentation/devicetree/bindings/arm/cpus.txt    |   2 +
>  Documentation/devicetree/bindings/arm/msm/acc.txt |  19 ++
>  drivers/soc/qcom/Makefile                         |   1 +
>  drivers/soc/qcom/cpu_ops.c                        | 343 ++++++++++++++++++++++
>  4 files changed, 365 insertions(+)
>  create mode 100644 Documentation/devicetree/bindings/arm/msm/acc.txt
> 

I don't want this in drivers/soc. Please find a way to integrate it into the
arch/arm64 code.

	Arnd

Re: [RFC PATCH 5/5] arm64: qcom: add cpu operations

[Catalin Marinas]

On Thu, Apr 09, 2015 at 11:19:02PM +0200, Arnd Bergmann wrote:
> On Thursday 09 April 2015 12:37:11 Kumar Gala wrote:
> > From: Abhimanyu Kapur <abhimany-sgV2jX0FEOL9JmXXK+q4OQ@public.gmane.org>
> > 
> > Add qcom cpu operations for arm-v8 cpus. Implement secondary cpu boot ops
> > As a part of this change update device tree documentation for:
> > 
> > 1. Arm cortex-a ACC device which provides percpu reg
> > 2. Armv8 cortex-a compatible string in arm/cpus.txt
> > 
> > Signed-off-by: Abhimanyu Kapur <abhimany-sgV2jX0FEOL9JmXXK+q4OQ@public.gmane.org>
> > Signed-off-by: Kumar Gala <galak-sgV2jX0FEOL9JmXXK+q4OQ@public.gmane.org>
> > ---
> >  Documentation/devicetree/bindings/arm/cpus.txt    |   2 +
> >  Documentation/devicetree/bindings/arm/msm/acc.txt |  19 ++
> >  drivers/soc/qcom/Makefile                         |   1 +
> >  drivers/soc/qcom/cpu_ops.c                        | 343 ++++++++++++++++++++++
> >  4 files changed, 365 insertions(+)
> >  create mode 100644 Documentation/devicetree/bindings/arm/msm/acc.txt
> 
> I don't want this in drivers/soc. Please find a way to integrate it into the
> arch/arm64 code.

And rename it to something like qc-special-cpu-ops-dont-copy-this.c

-- 
Catalin
--
To unsubscribe from this list: send the line "unsubscribe devicetree" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [RFC PATCH 5/5] arm64: qcom: add cpu operations

[Lorenzo Pieralisi]

On Thu, Apr 09, 2015 at 06:37:11PM +0100, Kumar Gala wrote:
> From: Abhimanyu Kapur <abhimany@codeaurora.org>
> 
> Add qcom cpu operations for arm-v8 cpus. Implement secondary cpu boot ops
> As a part of this change update device tree documentation for:
> 
> 1. Arm cortex-a ACC device which provides percpu reg
> 2. Armv8 cortex-a compatible string in arm/cpus.txt

I am pretty sure you heard about a standard FW interface called PSCI,
please implement it and drop this patch, thanks.

Lorenzo

> Signed-off-by: Abhimanyu Kapur <abhimany@codeaurora.org>
> Signed-off-by: Kumar Gala <galak@codeaurora.org>
> ---
>  Documentation/devicetree/bindings/arm/cpus.txt    |   2 +
>  Documentation/devicetree/bindings/arm/msm/acc.txt |  19 ++
>  drivers/soc/qcom/Makefile                         |   1 +
>  drivers/soc/qcom/cpu_ops.c                        | 343 ++++++++++++++++++++++
>  4 files changed, 365 insertions(+)
>  create mode 100644 Documentation/devicetree/bindings/arm/msm/acc.txt
>  create mode 100644 drivers/soc/qcom/cpu_ops.c
> 
> diff --git a/Documentation/devicetree/bindings/arm/cpus.txt b/Documentation/devicetree/bindings/arm/cpus.txt
> index 8b9e0a9..35cabe5 100644
> --- a/Documentation/devicetree/bindings/arm/cpus.txt
> +++ b/Documentation/devicetree/bindings/arm/cpus.txt
> @@ -185,6 +185,8 @@ nodes to be present and contain the properties described below.
>                           be one of:
>                              "psci"
>                              "spin-table"
> +                            "qcom,arm-cortex-acc"
> +
>                         # On ARM 32-bit systems this property is optional and
>                           can be one of:
>                             "allwinner,sun6i-a31"
> diff --git a/Documentation/devicetree/bindings/arm/msm/acc.txt b/Documentation/devicetree/bindings/arm/msm/acc.txt
> new file mode 100644
> index 0000000..ae2d725
> --- /dev/null
> +++ b/Documentation/devicetree/bindings/arm/msm/acc.txt
> @@ -0,0 +1,19 @@
> +Application Processor Sub-system (APSS) Application Clock Controller (ACC)
> +
> +The ACC provides clock, power domain, and reset control to a CPU. There is one ACC
> +register region per CPU within the APSS remapped region as well as an alias register
> +region that remaps accesses to the ACC associated with the CPU accessing the region.
> +
> +Required properties:
> +- compatible:          Must be "qcom,arm-cortex-acc"
> +- reg:                 The first element specifies the base address and size of
> +                       the register region. An optional second element specifies
> +                       the base address and size of the alias register region.
> +
> +Example:
> +
> +       clock-controller@b088000 {
> +               compatible = "qcom,arm-cortex-acc";
> +               reg = <0x0b088000 0x1000>,
> +                     <0x0b008000 0x1000>;
> +       }
> diff --git a/drivers/soc/qcom/Makefile b/drivers/soc/qcom/Makefile
> index 4389012..bb6030a 100644
> --- a/drivers/soc/qcom/Makefile
> +++ b/drivers/soc/qcom/Makefile
> @@ -1 +1,2 @@
> +obj-$(CONFIG_ARM64)    +=      cpu_ops.o
>  obj-$(CONFIG_QCOM_GSBI)        +=      qcom_gsbi.o
> diff --git a/drivers/soc/qcom/cpu_ops.c b/drivers/soc/qcom/cpu_ops.c
> new file mode 100644
> index 0000000..d831cb0
> --- /dev/null
> +++ b/drivers/soc/qcom/cpu_ops.c
> @@ -0,0 +1,343 @@
> +/* Copyright (c) 2014, The Linux Foundation. All rights reserved.
> + * Copyright (c) 2013 ARM Ltd.
> + *
> + * This program is free software; you can redistribute it and/or modify
> + * it under the terms of the GNU General Public License version 2 and
> + * only version 2 as published by the Free Software Foundation.
> + *
> + * This program is distributed in the hope that it will be useful,
> + * but WITHOUT ANY WARRANTY; without even the implied warranty of
> + * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
> + * GNU General Public License for more details.
> + */
> +
> +/* MSM ARMv8 CPU Operations
> + * Based on arch/arm64/kernel/smp_spin_table.c
> + */
> +
> +#include <linux/bitops.h>
> +#include <linux/cpu.h>
> +#include <linux/cpumask.h>
> +#include <linux/delay.h>
> +#include <linux/init.h>
> +#include <linux/io.h>
> +#include <linux/of.h>
> +#include <linux/of_address.h>
> +#include <linux/smp.h>
> +#include <linux/qcom_scm.h>
> +
> +#include <asm/barrier.h>
> +#include <asm/cacheflush.h>
> +#include <asm/cpu_ops.h>
> +#include <asm/cputype.h>
> +#include <asm/smp_plat.h>
> +
> +static DEFINE_RAW_SPINLOCK(boot_lock);
> +
> +DEFINE_PER_CPU(int, cold_boot_done);
> +
> +#if 0
> +static int cold_boot_flags[] = {
> +       0,
> +       QCOM_SCM_FLAG_COLDBOOT_CPU1,
> +       QCOM_SCM_FLAG_COLDBOOT_CPU2,
> +       QCOM_SCM_FLAG_COLDBOOT_CPU3,
> +};
> +#endif
> +
> +/* CPU power domain register offsets */
> +#define CPU_PWR_CTL            0x4
> +#define CPU_PWR_GATE_CTL       0x14
> +#define LDO_BHS_PWR_CTL                0x28
> +
> +/* L2 power domain register offsets */
> +#define L2_PWR_CTL_OVERRIDE    0xc
> +#define L2_PWR_CTL             0x14
> +#define L2_PWR_STATUS          0x18
> +#define        L2_CORE_CBCR            0x58
> +#define L1_RST_DIS             0x284
> +
> +#define L2_SPM_STS             0xc
> +#define L2_VREG_CTL            0x1c
> +
> +#define SCM_IO_READ            1
> +#define SCM_IO_WRITE           2
> +
> +/*
> + * struct msm_l2ccc_of_info: represents of data for l2 cache clock controller.
> + * @compat: compat string for l2 cache clock controller
> + * @l2_pon: l2 cache power on routine
> + */
> +struct msm_l2ccc_of_info {
> +       const char *compat;
> +       int (*l2_power_on) (struct device_node *dn, u32 l2_mask, int cpu);
> +       u32 l2_power_on_mask;
> +};
> +
> +
> +static int power_on_l2_msm8916(struct device_node *l2ccc_node, u32 pon_mask,
> +                               int cpu)
> +{
> +       u32 pon_status;
> +       void __iomem *l2_base;
> +
> +       l2_base = of_iomap(l2ccc_node, 0);
> +       if (!l2_base)
> +               return -ENOMEM;
> +
> +       /* Skip power-on sequence if l2 cache is already powered up*/
> +       pon_status = (__raw_readl(l2_base + L2_PWR_STATUS) & pon_mask)
> +                               == pon_mask;
> +       if (pon_status) {
> +               iounmap(l2_base);
> +               return 0;
> +       }
> +
> +       /* Close L2/SCU Logic GDHS and power up the cache */
> +       writel_relaxed(0x10D700, l2_base + L2_PWR_CTL);
> +
> +       /* Assert PRESETDBGn */
> +       writel_relaxed(0x400000, l2_base + L2_PWR_CTL_OVERRIDE);
> +       mb();
> +       udelay(2);
> +
> +       /* De-assert L2/SCU memory Clamp */
> +       writel_relaxed(0x101700, l2_base + L2_PWR_CTL);
> +
> +       /* Wakeup L2/SCU RAMs by deasserting sleep signals */
> +       writel_relaxed(0x101703, l2_base + L2_PWR_CTL);
> +       mb();
> +       udelay(2);
> +
> +       /* Enable clocks via SW_CLK_EN */
> +       writel_relaxed(0x01, l2_base + L2_CORE_CBCR);
> +
> +       /* De-assert L2/SCU logic clamp */
> +       writel_relaxed(0x101603, l2_base + L2_PWR_CTL);
> +       mb();
> +       udelay(2);
> +
> +       /* De-assert PRESSETDBg */
> +       writel_relaxed(0x0, l2_base + L2_PWR_CTL_OVERRIDE);
> +
> +       /* De-assert L2/SCU Logic reset */
> +       writel_relaxed(0x100203, l2_base + L2_PWR_CTL);
> +       mb();
> +       udelay(54);
> +
> +       /* Turn on the PMIC_APC */
> +       writel_relaxed(0x10100203, l2_base + L2_PWR_CTL);
> +
> +       /* Set H/W clock control for the cpu CBC block */
> +       writel_relaxed(0x03, l2_base + L2_CORE_CBCR);
> +       mb();
> +       iounmap(l2_base);
> +
> +       return 0;
> +}
> +
> +static const struct msm_l2ccc_of_info l2ccc_info[] = {
> +       {
> +               .compat = "qcom,8916-l2ccc",
> +               .l2_power_on = power_on_l2_msm8916,
> +               .l2_power_on_mask = BIT(9),
> +       },
> +};
> +
> +static int power_on_l2_cache(struct device_node *l2ccc_node, int cpu)
> +{
> +       int ret, i;
> +       const char *compat;
> +
> +       ret = of_property_read_string(l2ccc_node, "compatible", &compat);
> +       if (ret)
> +               return ret;
> +
> +       for (i = 0; i < ARRAY_SIZE(l2ccc_info); i++) {
> +               const struct msm_l2ccc_of_info *ptr = &l2ccc_info[i];
> +
> +               if (!of_compat_cmp(ptr->compat, compat, strlen(compat)))
> +                               return ptr->l2_power_on(l2ccc_node,
> +                                               ptr->l2_power_on_mask, cpu);
> +       }
> +       pr_err("Compat string not found for L2CCC node\n");
> +       return -EIO;
> +}
> +
> +static int msm_unclamp_secondary_arm_cpu(unsigned int cpu)
> +{
> +
> +       int ret = 0;
> +       struct device_node *cpu_node, *acc_node, *l2_node, *l2ccc_node;
> +       void __iomem *reg;
> +
> +       cpu_node = of_get_cpu_node(cpu, NULL);
> +       if (!cpu_node)
> +               return -ENODEV;
> +
> +       acc_node = of_parse_phandle(cpu_node, "qcom,acc", 0);
> +       if (!acc_node) {
> +                       ret = -ENODEV;
> +                       goto out_acc;
> +       }
> +
> +       l2_node = of_parse_phandle(cpu_node, "next-level-cache", 0);
> +       if (!l2_node) {
> +               ret = -ENODEV;
> +               goto out_l2;
> +       }
> +
> +       l2ccc_node = of_parse_phandle(l2_node, "power-domain", 0);
> +       if (!l2ccc_node) {
> +               ret = -ENODEV;
> +               goto out_l2;
> +       }
> +
> +       /* Ensure L2-cache of the CPU is powered on before
> +        * unclamping cpu power rails.
> +        */
> +       ret = power_on_l2_cache(l2ccc_node, cpu);
> +       if (ret) {
> +               pr_err("L2 cache power up failed for CPU%d\n", cpu);
> +               goto out_l2ccc;
> +       }
> +
> +       reg = of_iomap(acc_node, 0);
> +       if (!reg) {
> +               ret = -ENOMEM;
> +               goto out_acc_reg;
> +       }
> +
> +       /* Assert Reset on cpu-n */
> +       writel_relaxed(0x00000033, reg + CPU_PWR_CTL);
> +       mb();
> +
> +       /*Program skew to 16 X0 clock cycles*/
> +       writel_relaxed(0x10000001, reg + CPU_PWR_GATE_CTL);
> +       mb();
> +       udelay(2);
> +
> +       /* De-assert coremem clamp */
> +       writel_relaxed(0x00000031, reg + CPU_PWR_CTL);
> +       mb();
> +
> +       /* Close coremem array gdhs */
> +       writel_relaxed(0x00000039, reg + CPU_PWR_CTL);
> +       mb();
> +       udelay(2);
> +
> +       /* De-assert cpu-n clamp */
> +       writel_relaxed(0x00020038, reg + CPU_PWR_CTL);
> +       mb();
> +       udelay(2);
> +
> +       /* De-assert cpu-n reset */
> +       writel_relaxed(0x00020008, reg + CPU_PWR_CTL);
> +       mb();
> +
> +       /* Assert PWRDUP signal on core-n */
> +       writel_relaxed(0x00020088, reg + CPU_PWR_CTL);
> +       mb();
> +
> +       /* Secondary CPU-N is now alive */
> +       iounmap(reg);
> +out_acc_reg:
> +       of_node_put(l2ccc_node);
> +out_l2ccc:
> +       of_node_put(l2_node);
> +out_l2:
> +       of_node_put(acc_node);
> +out_acc:
> +       of_node_put(cpu_node);
> +
> +       return ret;
> +}
> +
> +static void write_pen_release(u64 val)
> +{
> +       void *start = (void *)&secondary_holding_pen_release;
> +       unsigned long size = sizeof(secondary_holding_pen_release);
> +
> +       secondary_holding_pen_release = val;
> +       smp_wmb();
> +       __flush_dcache_area(start, size);
> +}
> +
> +static int secondary_pen_release(unsigned int cpu)
> +{
> +       unsigned long timeout;
> +
> +       /*
> +        * Set synchronisation state between this boot processor
> +        * and the secondary one
> +        */
> +       raw_spin_lock(&boot_lock);
> +       write_pen_release(cpu_logical_map(cpu));
> +
> +       timeout = jiffies + (1 * HZ);
> +       while (time_before(jiffies, timeout)) {
> +               if (secondary_holding_pen_release == INVALID_HWID)
> +                       break;
> +               udelay(10);
> +       }
> +       raw_spin_unlock(&boot_lock);
> +
> +       return secondary_holding_pen_release != INVALID_HWID ? -ENOSYS : 0;
> +}
> +
> +static int __init msm_cpu_init(struct device_node *dn, unsigned int cpu)
> +{
> +       /* Mark CPU0 cold boot flag as done */
> +       if (!cpu && !per_cpu(cold_boot_done, cpu))
> +               per_cpu(cold_boot_done, cpu) = true;
> +
> +       return 0;
> +}
> +
> +static int __init msm_cpu_prepare(unsigned int cpu)
> +{
> +       const cpumask_t *mask = cpumask_of(cpu);
> +
> +       if (qcom_scm_set_cold_boot_addr(secondary_holding_pen, mask)) {
> +               pr_warn("CPU%d:Failed to set boot address\n", cpu);
> +               return -ENOSYS;
> +       }
> +
> +       return 0;
> +}
> +
> +static int msm_cpu_boot(unsigned int cpu)
> +{
> +       int ret = 0;
> +
> +       if (per_cpu(cold_boot_done, cpu) == false) {
> +               ret = msm_unclamp_secondary_arm_cpu(cpu);
> +               if (ret)
> +                       return ret;
> +               per_cpu(cold_boot_done, cpu) = true;
> +       }
> +       return secondary_pen_release(cpu);
> +}
> +
> +void msm_cpu_postboot(void)
> +{
> +       /*
> +        * Let the primary processor know we're out of the pen.
> +        */
> +       write_pen_release(INVALID_HWID);
> +
> +       /*
> +        * Synchronise with the boot thread.
> +        */
> +       raw_spin_lock(&boot_lock);
> +       raw_spin_unlock(&boot_lock);
> +}
> +
> +static const struct cpu_operations msm_cortex_a_ops = {
> +       .name           = "qcom,arm-cortex-acc",
> +       .cpu_init       = msm_cpu_init,
> +       .cpu_prepare    = msm_cpu_prepare,
> +       .cpu_boot       = msm_cpu_boot,
> +       .cpu_postboot   = msm_cpu_postboot,
> +};
> +CPU_METHOD_OF_DECLARE(msm_cortex_a_ops, &msm_cortex_a_ops);
> --
> Qualcomm Innovation Center, Inc.
> The Qualcomm Innovation Center, Inc. is a member of the Code Aurora Forum,
> a Linux Foundation Collaborative Project
> 
> --
> To unsubscribe from this list: send the line "unsubscribe devicetree" in
> the body of a message to majordomo@vger.kernel.org
> More majordomo info at  http://vger.kernel.org/majordomo-info.html
> 

Re: [RFC PATCH 5/5] arm64: qcom: add cpu operations

[Mark Rutland]

> diff --git a/Documentation/devicetree/bindings/arm/cpus.txt b/Documentation/devicetree/bindings/arm/cpus.txt
> index 8b9e0a9..35cabe5 100644
> --- a/Documentation/devicetree/bindings/arm/cpus.txt
> +++ b/Documentation/devicetree/bindings/arm/cpus.txt
> @@ -185,6 +185,8 @@ nodes to be present and contain the properties described below.
>                           be one of:
>                              "psci"
>                              "spin-table"

In the case of these two, there's documentation on what the OS, FW, and
HW are expected to do. There's a PSCI spec, and spin-table is documented
in booting.txt (which is admittedly not fantastic).

> +                            "qcom,arm-cortex-acc"

However, this has no semantics associated with it. Per the code below
it seems to encompass more than just poking the APSS ACC, so the name
isn't great either.

[...]

> + * Based on arch/arm64/kernel/smp_spin_table.c

This is not a phrase that will ever make me happy.

[...]

> +static DEFINE_RAW_SPINLOCK(boot_lock);

We got rid of this for spin-table. It's pointless.

> +DEFINE_PER_CPU(int, cold_boot_done);

This looks suspicious.

> +#if 0
> +static int cold_boot_flags[] = {
> +       0,
> +       QCOM_SCM_FLAG_COLDBOOT_CPU1,
> +       QCOM_SCM_FLAG_COLDBOOT_CPU2,
> +       QCOM_SCM_FLAG_COLDBOOT_CPU3,
> +};
> +#endif

I take it this shouldn't be here?

[...]

> +static int power_on_l2_msm8916(struct device_node *l2ccc_node, u32 pon_mask,
> +                               int cpu)
> +{
> +       u32 pon_status;
> +       void __iomem *l2_base;
> +
> +       l2_base = of_iomap(l2ccc_node, 0);
> +       if (!l2_base)
> +               return -ENOMEM;

I didn't see any mention of an L2 requiring power-up in the rest of the
series...

[...]

> +static void write_pen_release(u64 val)
> +{
> +       void *start = (void *)&secondary_holding_pen_release;
> +       unsigned long size = sizeof(secondary_holding_pen_release);
> +
> +       secondary_holding_pen_release = val;
> +       smp_wmb();
> +       __flush_dcache_area(start, size);
> +}
> +
> +static int secondary_pen_release(unsigned int cpu)
> +{
> +       unsigned long timeout;
> +
> +       /*
> +        * Set synchronisation state between this boot processor
> +        * and the secondary one
> +        */
> +       raw_spin_lock(&boot_lock);
> +       write_pen_release(cpu_logical_map(cpu));
> +
> +       timeout = jiffies + (1 * HZ);
> +       while (time_before(jiffies, timeout)) {
> +               if (secondary_holding_pen_release == INVALID_HWID)
> +                       break;
> +               udelay(10);
> +       }
> +       raw_spin_unlock(&boot_lock);
> +
> +       return secondary_holding_pen_release != INVALID_HWID ? -ENOSYS : 0;
> +}

So you want to share the pen, but duplicate the code for managing it?

> +static int __init msm_cpu_init(struct device_node *dn, unsigned int cpu)
> +{
> +       /* Mark CPU0 cold boot flag as done */
> +       if (!cpu && !per_cpu(cold_boot_done, cpu))
> +               per_cpu(cold_boot_done, cpu) = true;
> +
> +       return 0;
> +}

[...]

> +static int msm_cpu_boot(unsigned int cpu)
> +{
> +       int ret = 0;
> +
> +       if (per_cpu(cold_boot_done, cpu) == false) {
> +               ret = msm_unclamp_secondary_arm_cpu(cpu);
> +               if (ret)
> +                       return ret;
> +               per_cpu(cold_boot_done, cpu) = true;
> +       }
> +       return secondary_pen_release(cpu);
> +}

Ah, so cold_boot_done is for pseudo-hotplug. Absolute NAK to that.

The only thing this gives you over spin-table is one-time powering up of
the CPUs that can be performed prior to entry to Linux. If you do that,
you can trivially share the spin-table code by setting each CPU's
enable-method to "spin-table".

That won't give you cpuidle or actual hotplug. For those you'll need
PSCI.

Mark.

Re: [RFC PATCH 5/5] arm64: qcom: add cpu operations

[Arnd Bergmann]

On Tuesday 14 April 2015 17:29:53 Mark Rutland wrote:
> 
> > +static int msm_cpu_boot(unsigned int cpu)
> > +{
> > +       int ret = 0;
> > +
> > +       if (per_cpu(cold_boot_done, cpu) == false) {
> > +               ret = msm_unclamp_secondary_arm_cpu(cpu);
> > +               if (ret)
> > +                       return ret;
> > +               per_cpu(cold_boot_done, cpu) = true;
> > +       }
> > +       return secondary_pen_release(cpu);
> > +}
> 
> Ah, so cold_boot_done is for pseudo-hotplug. Absolute NAK to that.
> 
> The only thing this gives you over spin-table is one-time powering up of
> the CPUs that can be performed prior to entry to Linux. If you do that,
> you can trivially share the spin-table code by setting each CPU's
> enable-method to "spin-table".
> 
> That won't give you cpuidle or actual hotplug. For those you'll need
> PSCI.

Maybe a way out for the broken firmware is to have a custom boot wrapper
that gets distributed separately and that uses the normal spin-table
API. We've done similar things on arch/arm/mach-sunxi for boot loaders
that are just too different from what we expect.

Once someone implements a proper loader, they could skip that extra
wrapper.

	Arnd

Re: [RFC PATCH 5/5] arm64: qcom: add cpu operations

[Catalin Marinas]

On Tue, Apr 14, 2015 at 10:51:40PM +0200, Arnd Bergmann wrote:
> On Tuesday 14 April 2015 17:29:53 Mark Rutland wrote:
> > > +static int msm_cpu_boot(unsigned int cpu)
> > > +{
> > > +       int ret = 0;
> > > +
> > > +       if (per_cpu(cold_boot_done, cpu) == false) {
> > > +               ret = msm_unclamp_secondary_arm_cpu(cpu);
> > > +               if (ret)
> > > +                       return ret;
> > > +               per_cpu(cold_boot_done, cpu) = true;
> > > +       }
> > > +       return secondary_pen_release(cpu);
> > > +}
> > 
> > Ah, so cold_boot_done is for pseudo-hotplug. Absolute NAK to that.
> > 
> > The only thing this gives you over spin-table is one-time powering up of
> > the CPUs that can be performed prior to entry to Linux. If you do that,
> > you can trivially share the spin-table code by setting each CPU's
> > enable-method to "spin-table".
> > 
> > That won't give you cpuidle or actual hotplug. For those you'll need
> > PSCI.
> 
> Maybe a way out for the broken firmware is to have a custom boot wrapper
> that gets distributed separately and that uses the normal spin-table
> API. We've done similar things on arch/arm/mach-sunxi for boot loaders
> that are just too different from what we expect.

As a starting point, we actually have one that can do both spin table
and PSCI ;) (three-clause BSD license):

git://git.kernel.org/pub/scm/linux/kernel/git/mark/boot-wrapper-aarch64.git

Its primary goal is to create an ELF file that can be loaded on a
software model but there isn't anything that prevents you from
generating a kernel Image-like header.

-- 
Catalin
--
To unsubscribe from this list: send the line "unsubscribe devicetree" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [RFC PATCH 5/5] arm64: qcom: add cpu operations

[Al Stone]

On 04/14/2015 10:29 AM, Mark Rutland wrote:
>> diff --git a/Documentation/devicetree/bindings/arm/cpus.txt b/Documentation/devicetree/bindings/arm/cpus.txt
>> index 8b9e0a9..35cabe5 100644
>> --- a/Documentation/devicetree/bindings/arm/cpus.txt
>> +++ b/Documentation/devicetree/bindings/arm/cpus.txt
>> @@ -185,6 +185,8 @@ nodes to be present and contain the properties described below.
>>                           be one of:
>>                              "psci"
>>                              "spin-table"
> 
> In the case of these two, there's documentation on what the OS, FW, and
> HW are expected to do. There's a PSCI spec, and spin-table is documented
> in booting.txt (which is admittedly not fantastic).
> [snip...]

Perhaps a side topic, but I thought spin-table was being actively discouraged
for arm64.  Forgive me if I missed the memo, but is that not correct?

-- 
ciao,
al
-----------------------------------
Al Stone
Software Engineer
Red Hat, Inc.
ahs3@redhat.com
-----------------------------------

Re: [RFC PATCH 5/5] arm64: qcom: add cpu operations

[Mark Rutland]

On Tue, Apr 14, 2015 at 11:52:39PM +0100, Al Stone wrote:
> On 04/14/2015 10:29 AM, Mark Rutland wrote:
> >> diff --git a/Documentation/devicetree/bindings/arm/cpus.txt b/Documentation/devicetree/bindings/arm/cpus.txt
> >> index 8b9e0a9..35cabe5 100644
> >> --- a/Documentation/devicetree/bindings/arm/cpus.txt
> >> +++ b/Documentation/devicetree/bindings/arm/cpus.txt
> >> @@ -185,6 +185,8 @@ nodes to be present and contain the properties described below.
> >>                           be one of:
> >>                              "psci"
> >>                              "spin-table"
> > 
> > In the case of these two, there's documentation on what the OS, FW, and
> > HW are expected to do. There's a PSCI spec, and spin-table is documented
> > in booting.txt (which is admittedly not fantastic).
> > [snip...]
> 
> Perhaps a side topic, but I thought spin-table was being actively discouraged
> for arm64.  Forgive me if I missed the memo, but is that not correct?

We prefer that people implement PSCI, and if they must use spin-table,
each CPU has its own release address.

However, we don't want implementation-specific mechanisms, and
spin-table is preferable to these.

Mark.

Re: [RFC PATCH 5/5] arm64: qcom: add cpu operations

[Catalin Marinas]

On Wed, Apr 15, 2015 at 10:04:25AM +0100, Mark Rutland wrote:
> On Tue, Apr 14, 2015 at 11:52:39PM +0100, Al Stone wrote:
> > On 04/14/2015 10:29 AM, Mark Rutland wrote:
> > >> diff --git a/Documentation/devicetree/bindings/arm/cpus.txt b/Documentation/devicetree/bindings/arm/cpus.txt
> > >> index 8b9e0a9..35cabe5 100644
> > >> --- a/Documentation/devicetree/bindings/arm/cpus.txt
> > >> +++ b/Documentation/devicetree/bindings/arm/cpus.txt
> > >> @@ -185,6 +185,8 @@ nodes to be present and contain the properties described below.
> > >>                           be one of:
> > >>                              "psci"
> > >>                              "spin-table"
> > > 
> > > In the case of these two, there's documentation on what the OS, FW, and
> > > HW are expected to do. There's a PSCI spec, and spin-table is documented
> > > in booting.txt (which is admittedly not fantastic).
> > > [snip...]
> > 
> > Perhaps a side topic, but I thought spin-table was being actively discouraged
> > for arm64.  Forgive me if I missed the memo, but is that not correct?
> 
> We prefer that people implement PSCI, and if they must use spin-table,
> each CPU has its own release address.
> 
> However, we don't want implementation-specific mechanisms, and
> spin-table is preferable to these.

An important aspect is that with spin-table you don't get CPU off or
suspend and some kernel functionality will be missing (kexec being one
of them).

-- 
Catalin
--
To unsubscribe from this list: send the line "unsubscribe devicetree" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [RFC PATCH 5/5] arm64: qcom: add cpu operations

[Al Stone]

On 04/15/2015 08:53 AM, Catalin Marinas wrote:
> On Wed, Apr 15, 2015 at 10:04:25AM +0100, Mark Rutland wrote:
>> On Tue, Apr 14, 2015 at 11:52:39PM +0100, Al Stone wrote:
>>> On 04/14/2015 10:29 AM, Mark Rutland wrote:
>>>>> diff --git a/Documentation/devicetree/bindings/arm/cpus.txt b/Documentation/devicetree/bindings/arm/cpus.txt
>>>>> index 8b9e0a9..35cabe5 100644
>>>>> --- a/Documentation/devicetree/bindings/arm/cpus.txt
>>>>> +++ b/Documentation/devicetree/bindings/arm/cpus.txt
>>>>> @@ -185,6 +185,8 @@ nodes to be present and contain the properties described below.
>>>>>                           be one of:
>>>>>                              "psci"
>>>>>                              "spin-table"
>>>>
>>>> In the case of these two, there's documentation on what the OS, FW, and
>>>> HW are expected to do. There's a PSCI spec, and spin-table is documented
>>>> in booting.txt (which is admittedly not fantastic).
>>>> [snip...]
>>>
>>> Perhaps a side topic, but I thought spin-table was being actively discouraged
>>> for arm64.  Forgive me if I missed the memo, but is that not correct?
>>
>> We prefer that people implement PSCI, and if they must use spin-table,
>> each CPU has its own release address.
>>
>> However, we don't want implementation-specific mechanisms, and
>> spin-table is preferable to these.
> 
> An important aspect is that with spin-table you don't get CPU off or
> suspend and some kernel functionality will be missing (kexec being one
> of them).
> 

Thanks for the clarifications.  I misunderstood; I knew PSCI was
preferred but somehow had it in my head that spin-table was just
a non-starter.

-- 
ciao,
al
-----------------------------------
Al Stone
Software Engineer
Red Hat, Inc.
ahs3@redhat.com
-----------------------------------

Re: [RFC PATCH 0/5] Add smp booting support for Qualcomm ARMv8 SoCs

[Catalin Marinas]

On Thu, Apr 09, 2015 at 12:37:06PM -0500, Kumar Gala wrote:
> This patch set adds support for SMP boot on the MSM8x16 family of Qualcomm SoCs.
> 
> To support SMP on the MSM8x16 SoCs we need to add ARMv8/64-bit SCM interfaces to
> setup the boot/release addresses for the secondary CPUs.  In addition we need
> a uniquie set of cpu ops.  I'm aware the desired methods for booting secondary
> CPUs is either via spintable or PSCI.  However, these SoCs are shipping with a
> firmware that does not support those methods.

And the reason is? Some guesses:

a) QC doesn't think boot interface (and cpuidle) standardisation is
   worth the effort (to put it nicely)
b) The hardware was available before we even mentioned PSCI
c) PSCI is not suitable for the QC's SCM interface
d) Any combination of the above

I strongly suspect it's point (a). Should we expect future QC hardware
to do the same?

You could argue the reason was (b), though we've been discussing PSCI
for at least two years and, according to QC press releases, MSM8916
started sampling in 2014.

The only valid reason is (c) and if that's the case, I would expect a
proposal for a new firmware interface protocol (it could be PSCI-based),
well documented, that can be shared with others that may encounter the
same shortcomings.

-- 
Catalin

Re: [RFC PATCH 0/5] Add smp booting support for Qualcomm ARMv8 SoCs

[Kumar Gala]

On Apr 10, 2015, at 5:05 AM, Catalin Marinas <catalin.marinas-5wv7dgnIgG8@public.gmane.org> wrote:

> On Thu, Apr 09, 2015 at 12:37:06PM -0500, Kumar Gala wrote:
>> This patch set adds support for SMP boot on the MSM8x16 family of Qualcomm SoCs.
>> 
>> To support SMP on the MSM8x16 SoCs we need to add ARMv8/64-bit SCM interfaces to
>> setup the boot/release addresses for the secondary CPUs.  In addition we need
>> a uniquie set of cpu ops.  I'm aware the desired methods for booting secondary
>> CPUs is either via spintable or PSCI.  However, these SoCs are shipping with a
>> firmware that does not support those methods.
> 
> And the reason is? Some guesses:
> 
> a) QC doesn't think boot interface (and cpuidle) standardisation is
>   worth the effort (to put it nicely)
> b) The hardware was available before we even mentioned PSCI
> c) PSCI is not suitable for the QC's SCM interface
> d) Any combination of the above
> 
> I strongly suspect it's point (a). Should we expect future QC hardware
> to do the same?
> 
> You could argue the reason was (b), though we've been discussing PSCI
> for at least two years and, according to QC press releases, MSM8916
> started sampling in 2014.
> 
> The only valid reason is (c) and if that's the case, I would expect a
> proposal for a new firmware interface protocol (it could be PSCI-based),
> well documented, that can be shared with others that may encounter the
> same shortcomings.
> 
> -- 
> Catalin

Does it matter?  I’ve always felt the kernel was a place of inclusion.  Qualcomm choose for whatever reason to not use PSCI or spin table.  You don’t like it, I might not like it, but it is what it is.

- k

-- 
Qualcomm Innovation Center, Inc.
The Qualcomm Innovation Center, Inc. is a member of the Code Aurora Forum,
a Linux Foundation Collaborative Project

--
To unsubscribe from this list: send the line "unsubscribe devicetree" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [RFC PATCH 0/5] Add smp booting support for Qualcomm ARMv8 SoCs

[Catalin Marinas]

On Fri, Apr 10, 2015 at 10:24:46AM -0500, Kumar Gala wrote:
> On Apr 10, 2015, at 5:05 AM, Catalin Marinas <catalin.marinas-5wv7dgnIgG8@public.gmane.org> wrote:
> > On Thu, Apr 09, 2015 at 12:37:06PM -0500, Kumar Gala wrote:
> >> This patch set adds support for SMP boot on the MSM8x16 family of Qualcomm SoCs.
> >> 
> >> To support SMP on the MSM8x16 SoCs we need to add ARMv8/64-bit SCM interfaces to
> >> setup the boot/release addresses for the secondary CPUs.  In addition we need
> >> a uniquie set of cpu ops.  I'm aware the desired methods for booting secondary
> >> CPUs is either via spintable or PSCI.  However, these SoCs are shipping with a
> >> firmware that does not support those methods.
> > 
> > And the reason is? Some guesses:
> > 
> > a) QC doesn't think boot interface (and cpuidle) standardisation is
> >   worth the effort (to put it nicely)
> > b) The hardware was available before we even mentioned PSCI
> > c) PSCI is not suitable for the QC's SCM interface
> > d) Any combination of the above
> > 
> > I strongly suspect it's point (a). Should we expect future QC hardware
> > to do the same?
> > 
> > You could argue the reason was (b), though we've been discussing PSCI
> > for at least two years and, according to QC press releases, MSM8916
> > started sampling in 2014.
> > 
> > The only valid reason is (c) and if that's the case, I would expect a
> > proposal for a new firmware interface protocol (it could be PSCI-based),
> > well documented, that can be shared with others that may encounter the
> > same shortcomings.
> 
> Does it matter?  I’ve always felt the kernel was a place of inclusion.
> Qualcomm choose for whatever reason to not use PSCI or spin table.
> You don’t like it, I might not like it, but it is what it is.

Yes, it matters, but only if Qualcomm wants the SoC support in mainline.
Just because Qualcomm Inc does not want to invest in implementing a
standard firmware interface is not a good enough reason to merge the
kernel code.

What if Qualcomm decides that it doesn't like DT, nor ACPI but comes up
with yet another way to describe hardware because that's what the
firmware provides? Should the kernel community take it? You could argue
that this is a significant change but it's about the principle. And each
SoC with its own non-standard boot protocol for no good reason is
significant.

It's not Qualcomm Inc maintaining the kernel code but individuals like
you and me who may or may not be sponsored by Qualcomm. And being
sponsored by a company to do kernel maintenance work does not mean that
said company decides what gets merged (on my side, ARM Ltd management is
aware of this, though it's not always easy for the parties involved).

We haven't really asked for anything difficult like hardware changes,
such decisions are left with Qualcomm. We asked for a standard secure
firmware interface, either an existing one or, if not suitable (with
good arguments), to come up with a proposal for an alternative
_standard_ interface. I don't even have the certitude that the firmware
interface used in these patches would be consistent across Qualcomm
SoCs, let alone being properly documented.

So please come up with proper technical arguments rather than the kernel
should take whatever SoC vendors dreamt of.

-- 
Catalin
--
To unsubscribe from this list: send the line "unsubscribe devicetree" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [RFC PATCH 0/5] Add smp booting support for Qualcomm ARMv8 SoCs

[Kumar Gala]

> On Apr 10, 2015, at 11:10 AM, Catalin Marinas <catalin.marinas@arm.com> wrote:
> 
> On Fri, Apr 10, 2015 at 10:24:46AM -0500, Kumar Gala wrote:
>> On Apr 10, 2015, at 5:05 AM, Catalin Marinas <catalin.marinas@arm.com> wrote:
>>> On Thu, Apr 09, 2015 at 12:37:06PM -0500, Kumar Gala wrote:
>>>> This patch set adds support for SMP boot on the MSM8x16 family of Qualcomm SoCs.
>>>> 
>>>> To support SMP on the MSM8x16 SoCs we need to add ARMv8/64-bit SCM interfaces to
>>>> setup the boot/release addresses for the secondary CPUs.  In addition we need
>>>> a uniquie set of cpu ops.  I'm aware the desired methods for booting secondary
>>>> CPUs is either via spintable or PSCI.  However, these SoCs are shipping with a
>>>> firmware that does not support those methods.
>>> 
>>> And the reason is? Some guesses:
>>> 
>>> a) QC doesn't think boot interface (and cpuidle) standardisation is
>>>  worth the effort (to put it nicely)
>>> b) The hardware was available before we even mentioned PSCI
>>> c) PSCI is not suitable for the QC's SCM interface
>>> d) Any combination of the above
>>> 
>>> I strongly suspect it's point (a). Should we expect future QC hardware
>>> to do the same?
>>> 
>>> You could argue the reason was (b), though we've been discussing PSCI
>>> for at least two years and, according to QC press releases, MSM8916
>>> started sampling in 2014.
>>> 
>>> The only valid reason is (c) and if that's the case, I would expect a
>>> proposal for a new firmware interface protocol (it could be PSCI-based),
>>> well documented, that can be shared with others that may encounter the
>>> same shortcomings.
>> 
>> Does it matter?  I’ve always felt the kernel was a place of inclusion.
>> Qualcomm choose for whatever reason to not use PSCI or spin table.
>> You don’t like it, I might not like it, but it is what it is.
> 
> Yes, it matters, but only if Qualcomm wants the SoC support in mainline.
> Just because Qualcomm Inc does not want to invest in implementing a
> standard firmware interface is not a good enough reason to merge the
> kernel code.

The reason to merge the code upstream it expands functionality for a platform.  There is nothing that says when someone licenses an ARM core that they must also implement this standard.  Qualcomm choose for whatever reasons to not implement it.  There are examples on other architectures supporting non-standard platforms all the time (x86 supported Voyager and SGI VIS for a long time).  As far as I can tell you are just wanting uniformity to impose this rule.

> What if Qualcomm decides that it doesn't like DT, nor ACPI but comes up
> with yet another way to describe hardware because that's what the
> firmware provides? Should the kernel community take it? You could argue
> that this is a significant change but it's about the principle. And each
> SoC with its own non-standard boot protocol for no good reason is
> significant.

I wouldn’t argue that because we are talking about something that has an extremely small impact on the maintainability or changes to how the kernel actually functions.  Also, if Qualcomm did come up with some other means to replace DT or ACPI and felt it was worth trying to get accepted upstream, I would hope the upstream would look at it before just saying it was not using some standard.

> It's not Qualcomm Inc maintaining the kernel code but individuals like
> you and me who may or may not be sponsored by Qualcomm. And being
> sponsored by a company to do kernel maintenance work does not mean that
> said company decides what gets merged (on my side, ARM Ltd management is
> aware of this, though it's not always easy for the parties involved).

Fair enough, but you’ve not given any reasons the code isn’t maintainable.  You’ve only said you don’t like it because it doesn’t use one of the defacto “standards”.

As you say, its individual doing the maintenance, so those individuals are not likely to have access to change firmware on a given device.  So saying go change firmware is pretty much saying we don’t want to support your platform or code upstream.

> We haven't really asked for anything difficult like hardware changes,
> such decisions are left with Qualcomm. We asked for a standard secure
> firmware interface, either an existing one or, if not suitable (with
> good arguments), to come up with a proposal for an alternative
> _standard_ interface. I don't even have the certitude that the firmware
> interface used in these patches would be consistent across Qualcomm
> SoCs, let alone being properly documented.

If and when those issues arise we can accept or reject that code.  Right now when I look at the impact to supporting this to generic arch/arm64 kernel it is either non-existant if we use the CPU_OF_TABLES or extremely minor if we just add an entry to the supported_cpu_ops struct.

> So please come up with proper technical arguments rather than the kernel
> should take whatever SoC vendors dreamt of.

There is no technical argument to be made.  This is about the community and you as maintainer wanting to accept code that complies to your decision or not.

- k

-- 
Qualcomm Innovation Center, Inc.
The Qualcomm Innovation Center, Inc. is a member of the Code Aurora Forum,
a Linux Foundation Collaborative Project

Re: [RFC PATCH 0/5] Add smp booting support for Qualcomm ARMv8 SoCs

[Catalin Marinas]

On Fri, Apr 10, 2015 at 02:06:33PM -0500, Kumar Gala wrote:
> On Apr 10, 2015, at 11:10 AM, Catalin Marinas <catalin.marinas@arm.com> wrote:
> > On Fri, Apr 10, 2015 at 10:24:46AM -0500, Kumar Gala wrote:
> >> On Apr 10, 2015, at 5:05 AM, Catalin Marinas <catalin.marinas@arm.com> wrote:
> >>> On Thu, Apr 09, 2015 at 12:37:06PM -0500, Kumar Gala wrote:
> >>>> This patch set adds support for SMP boot on the MSM8x16 family of Qualcomm SoCs.
> >>>> 
> >>>> To support SMP on the MSM8x16 SoCs we need to add ARMv8/64-bit SCM interfaces to
> >>>> setup the boot/release addresses for the secondary CPUs.  In addition we need
> >>>> a uniquie set of cpu ops.  I'm aware the desired methods for booting secondary
> >>>> CPUs is either via spintable or PSCI.  However, these SoCs are shipping with a
> >>>> firmware that does not support those methods.
> >>> 
> >>> And the reason is? Some guesses:
> >>> 
> >>> a) QC doesn't think boot interface (and cpuidle) standardisation is
> >>>  worth the effort (to put it nicely)
> >>> b) The hardware was available before we even mentioned PSCI
> >>> c) PSCI is not suitable for the QC's SCM interface
> >>> d) Any combination of the above
> >>> 
> >>> I strongly suspect it's point (a). Should we expect future QC hardware
> >>> to do the same?
> >>> 
> >>> You could argue the reason was (b), though we've been discussing PSCI
> >>> for at least two years and, according to QC press releases, MSM8916
> >>> started sampling in 2014.
> >>> 
> >>> The only valid reason is (c) and if that's the case, I would expect a
> >>> proposal for a new firmware interface protocol (it could be PSCI-based),
> >>> well documented, that can be shared with others that may encounter the
> >>> same shortcomings.
> >> 
> >> Does it matter?  I’ve always felt the kernel was a place of inclusion.
> >> Qualcomm choose for whatever reason to not use PSCI or spin table.
> >> You don’t like it, I might not like it, but it is what it is.
> > 
> > Yes, it matters, but only if Qualcomm wants the SoC support in mainline.
> > Just because Qualcomm Inc does not want to invest in implementing a
> > standard firmware interface is not a good enough reason to merge the
> > kernel code.
> 
> The reason to merge the code upstream it expands functionality for a
> platform.

This alone has never been a good enough reason. Code (both design and
style) needs to pass the review.

> There is nothing that says when someone licenses an ARM core that they
> must also implement this standard.

Just to be perfectly clear: this has nothing to do with ARM Ltd nor the
ARM hardware licensing terms. ARM Ltd doesn't even require you to use
Linux, that's your choice. But when you make an OS choice, you have to
abide by its rules.

> Qualcomm choose for whatever reasons to not implement it.  There are
> examples on other architectures supporting non-standard platforms all
> the time (x86 supported Voyager and SGI VIS for a long time).  As far
> as I can tell you are just wanting uniformity to impose this rule.

Don't confuse non-standard hardware (which has always been acceptable on
ARM) with non-standard ways of entering the kernel from firmware,
whether it's a primary or secondary CPU. Just look at how many smp_ops
structures are defined on x86.

> > What if Qualcomm decides that it doesn't like DT, nor ACPI but comes up
> > with yet another way to describe hardware because that's what the
> > firmware provides? Should the kernel community take it? You could argue
> > that this is a significant change but it's about the principle. And each
> > SoC with its own non-standard boot protocol for no good reason is
> > significant.
> 
> I wouldn’t argue that because we are talking about something that has
> an extremely small impact on the maintainability or changes to how the
> kernel actually functions.

It's not about one particular case but about where to draw the line.
Just multiply this change by the number of SoC variants and you'll no
longer see this as "extremely small impact". Why would we accept it for
Qualcomm and reject it for others? It's either giving up trying to
standardise this altogether or enforcing rules. Since you only care
about Qualcomm hardware, you don't care about the overall picture.

By your reasoning, Qualcomm may solely decide to change the booting for
the primary CPU as well, ignore single Image requirements (well, why
would Qualcomm care about other SoCs) and so on. The kernel community
should just accept such changes without questioning because they expand
platform functionality. I am not asking for *hardware* standardisation
here, but common software interfaces.

> Also, if Qualcomm did come up with some other means to replace DT or
> ACPI and felt it was worth trying to get accepted upstream, I would
> hope the upstream would look at it before just saying it was not using
> some standard.

We would still expect proper technical arguments. That's exactly what
I'm asking here; is PSCI not suitable for Qualcomm? If not, can it be
improved? If you have completely different needs, can you come up with a
standard firmware interface, which may only be used by Qualcomm for the
time being, but at least guarantee consistency between subsequent
Qualcomm SoC revisions? But the message you give is pretty much
"Qualcomm cannot be bothered to explain itself to the kernel community".

And what makes you think no-one looked at your code (there's an
unanswered question, asked twice, from Arnd already)? One of the first
steps in a review is looking at the overall design and asking questions
if the choices made are not clear. The review is not just about coding
style or spotting bugs.

Looking beyond this set of patches, I can foresee that you won't care
about the generic arm64 cpuidle driver either, or more precisely the
separation between cpuidle subsystem+driver and the SoC-specific
back-end (cpu_operations).

> > It's not Qualcomm Inc maintaining the kernel code but individuals like
> > you and me who may or may not be sponsored by Qualcomm. And being
> > sponsored by a company to do kernel maintenance work does not mean that
> > said company decides what gets merged (on my side, ARM Ltd management is
> > aware of this, though it's not always easy for the parties involved).
> 
> Fair enough, but you’ve not given any reasons the code isn’t
> maintainable.  You’ve only said you don’t like it because it doesn’t
> use one of the defacto “standards”.

See above about maintainability and how it no longer scales to tens of
SoCs on the long term.

> As you say, its individual doing the maintenance, so those individuals
> are not likely to have access to change firmware on a given device.
> So saying go change firmware is pretty much saying we don’t want to
> support your platform or code upstream.

I don't blame you here for the lack of standard firmware interfaces. But
it's a message you should have given back to Qualcomm 2-3 years ago.
Maybe you did and Qualcomm ignored it, which kind of makes it worse.

> > We haven't really asked for anything difficult like hardware changes,
> > such decisions are left with Qualcomm. We asked for a standard secure
> > firmware interface, either an existing one or, if not suitable (with
> > good arguments), to come up with a proposal for an alternative
> > _standard_ interface. I don't even have the certitude that the firmware
> > interface used in these patches would be consistent across Qualcomm
> > SoCs, let alone being properly documented.
> 
> If and when those issues arise we can accept or reject that code.
> Right now when I look at the impact to supporting this to generic
> arch/arm64 kernel it is either non-existant if we use the
> CPU_OF_TABLES or extremely minor if we just add an entry to the
> supported_cpu_ops struct.

Again, see above about how such change is no longer small when each SoC
does the same.

> > So please come up with proper technical arguments rather than the kernel
> > should take whatever SoC vendors dreamt of.
> 
> There is no technical argument to be made.  This is about the
> community and you as maintainer wanting to accept code that complies
> to your decision or not.

If you are not willing to make technical arguments, I don't have to
provide any further reasons in this thread. It's your choice. In the
meantime, the short answer is NAK.

-- 
Catalin

Re: [RFC PATCH 0/5] Add smp booting support for Qualcomm ARMv8 SoCs

[Kumar Gala]

> On Apr 13, 2015, at 4:41 AM, Catalin Marinas <catalin.marinas@arm.com> wrote:
> 
> On Fri, Apr 10, 2015 at 02:06:33PM -0500, Kumar Gala wrote:
>> On Apr 10, 2015, at 11:10 AM, Catalin Marinas <catalin.marinas@arm.com> wrote:
>>> On Fri, Apr 10, 2015 at 10:24:46AM -0500, Kumar Gala wrote:
>>>> On Apr 10, 2015, at 5:05 AM, Catalin Marinas <catalin.marinas@arm.com> wrote:
>>>>> On Thu, Apr 09, 2015 at 12:37:06PM -0500, Kumar Gala wrote:
>>>>>> This patch set adds support for SMP boot on the MSM8x16 family of Qualcomm SoCs.
>>>>>> 
>>>>>> To support SMP on the MSM8x16 SoCs we need to add ARMv8/64-bit SCM interfaces to
>>>>>> setup the boot/release addresses for the secondary CPUs.  In addition we need
>>>>>> a uniquie set of cpu ops.  I'm aware the desired methods for booting secondary
>>>>>> CPUs is either via spintable or PSCI.  However, these SoCs are shipping with a
>>>>>> firmware that does not support those methods.
>>>>> 
>>>>> And the reason is? Some guesses:
>>>>> 
>>>>> a) QC doesn't think boot interface (and cpuidle) standardisation is
>>>>> worth the effort (to put it nicely)
>>>>> b) The hardware was available before we even mentioned PSCI
>>>>> c) PSCI is not suitable for the QC's SCM interface
>>>>> d) Any combination of the above
>>>>> 
>>>>> I strongly suspect it's point (a). Should we expect future QC hardware
>>>>> to do the same?
>>>>> 
>>>>> You could argue the reason was (b), though we've been discussing PSCI
>>>>> for at least two years and, according to QC press releases, MSM8916
>>>>> started sampling in 2014.
>>>>> 
>>>>> The only valid reason is (c) and if that's the case, I would expect a
>>>>> proposal for a new firmware interface protocol (it could be PSCI-based),
>>>>> well documented, that can be shared with others that may encounter the
>>>>> same shortcomings.
>>>> 
>>>> Does it matter?  I’ve always felt the kernel was a place of inclusion.
>>>> Qualcomm choose for whatever reason to not use PSCI or spin table.
>>>> You don’t like it, I might not like it, but it is what it is.
>>> 
>>> Yes, it matters, but only if Qualcomm wants the SoC support in mainline.
>>> Just because Qualcomm Inc does not want to invest in implementing a
>>> standard firmware interface is not a good enough reason to merge the
>>> kernel code.
>> 
>> The reason to merge the code upstream it expands functionality for a
>> platform.
> 
> This alone has never been a good enough reason. Code (both design and
> style) needs to pass the review.

You haven’t really made any comments about the code itself, other than just not liking the firmware interface.

> There is nothing that says when someone licenses an ARM core that they
>> must also implement this standard.
> 
> Just to be perfectly clear: this has nothing to do with ARM Ltd nor the
> ARM hardware licensing terms. ARM Ltd doesn't even require you to use
> Linux, that's your choice. But when you make an OS choice, you have to
> abide by its rules.

But Linux has tended to support hardware as broadly as it can.  Yes it tries to get firmware to improve but there are numerous devices with embedded firmware interfaces that Linux probably would love to change but deals with because they are fixed.

> Qualcomm choose for whatever reasons to not implement it.  There are
>> examples on other architectures supporting non-standard platforms all
>> the time (x86 supported Voyager and SGI VIS for a long time).  As far
>> as I can tell you are just wanting uniformity to impose this rule.
> 
> Don't confuse non-standard hardware (which has always been acceptable on
> ARM) with non-standard ways of entering the kernel from firmware,
> whether it's a primary or secondary CPU. Just look at how many smp_ops
> structures are defined on x86.

When Voyager was supported it had a unique means for SMP bring up.  In the 4.0 kernel MIPS supports 13 different means, PowerPC has 14 different means, ARM has 36 different means.

>>> What if Qualcomm decides that it doesn't like DT, nor ACPI but comes up
>>> with yet another way to describe hardware because that's what the
>>> firmware provides? Should the kernel community take it? You could argue
>>> that this is a significant change but it's about the principle. And each
>>> SoC with its own non-standard boot protocol for no good reason is
>>> significant.
>> 
>> I wouldn’t argue that because we are talking about something that has
>> an extremely small impact on the maintainability or changes to how the
>> kernel actually functions.
> 
> It's not about one particular case but about where to draw the line.
> Just multiply this change by the number of SoC variants and you'll no
> longer see this as "extremely small impact". Why would we accept it for
> Qualcomm and reject it for others? It's either giving up trying to
> standardise this altogether or enforcing rules. Since you only care
> about Qualcomm hardware, you don't care about the overall picture.
> 
> By your reasoning, Qualcomm may solely decide to change the booting for
> the primary CPU as well, ignore single Image requirements (well, why
> would Qualcomm care about other SoCs) and so on. The kernel community
> should just accept such changes without questioning because they expand
> platform functionality. I am not asking for *hardware* standardisation
> here, but common software interfaces.
> 
>> Also, if Qualcomm did come up with some other means to replace DT or
>> ACPI and felt it was worth trying to get accepted upstream, I would
>> hope the upstream would look at it before just saying it was not using
>> some standard.
> 
> We would still expect proper technical arguments. That's exactly what
> I'm asking here; is PSCI not suitable for Qualcomm? If not, can it be
> improved? If you have completely different needs, can you come up with a
> standard firmware interface, which may only be used by Qualcomm for the
> time being, but at least guarantee consistency between subsequent
> Qualcomm SoC revisions? But the message you give is pretty much
> "Qualcomm cannot be bothered to explain itself to the kernel community".
> 
> And what makes you think no-one looked at your code (there's an
> unanswered question, asked twice, from Arnd already)? One of the first
> steps in a review is looking at the overall design and asking questions
> if the choices made are not clear. The review is not just about coding
> style or spotting bugs.

Agreed, I need to address Arnd’s comment.  However, if there isn’t any likehood of getting this code accepted at all to support this device than there really isn’t any point.

> Looking beyond this set of patches, I can foresee that you won't care
> about the generic arm64 cpuidle driver either, or more precisely the
> separation between cpuidle subsystem+driver and the SoC-specific
> back-end (cpu_operations).

That’s probably true for what I guess are a number of reasons.  I’m guessing the arm64 cpuidle driver expects PSCI.

>>> It's not Qualcomm Inc maintaining the kernel code but individuals like
>>> you and me who may or may not be sponsored by Qualcomm. And being
>>> sponsored by a company to do kernel maintenance work does not mean that
>>> said company decides what gets merged (on my side, ARM Ltd management is
>>> aware of this, though it's not always easy for the parties involved).
>> 
>> Fair enough, but you’ve not given any reasons the code isn’t
>> maintainable.  You’ve only said you don’t like it because it doesn’t
>> use one of the defacto “standards”.
> 
> See above about maintainability and how it no longer scales to tens of
> SoCs on the long term.
> 
>> As you say, its individual doing the maintenance, so those individuals
>> are not likely to have access to change firmware on a given device.
>> So saying go change firmware is pretty much saying we don’t want to
>> support your platform or code upstream.
> 
> I don't blame you here for the lack of standard firmware interfaces. But
> it's a message you should have given back to Qualcomm 2-3 years ago.
> Maybe you did and Qualcomm ignored it, which kind of makes it worse.
> 
>>> We haven't really asked for anything difficult like hardware changes,
>>> such decisions are left with Qualcomm. We asked for a standard secure
>>> firmware interface, either an existing one or, if not suitable (with
>>> good arguments), to come up with a proposal for an alternative
>>> _standard_ interface. I don't even have the certitude that the firmware
>>> interface used in these patches would be consistent across Qualcomm
>>> SoCs, let alone being properly documented.
>> 
>> If and when those issues arise we can accept or reject that code.
>> Right now when I look at the impact to supporting this to generic
>> arch/arm64 kernel it is either non-existant if we use the
>> CPU_OF_TABLES or extremely minor if we just add an entry to the
>> supported_cpu_ops struct.
> 
> Again, see above about how such change is no longer small when each SoC
> does the same.

There are so many places that we already deal with per SoC uniqueness.  We setup a software interface in the kernel and people develop towards it.

Are you saying that going forward all SoCs should have the same clocking programming interface to ease portability?  Do you expect them all to have the same pin control programming interface if someone defines a firmware abstraction?

>>> So please come up with proper technical arguments rather than the kernel
>>> should take whatever SoC vendors dreamt of.
>> 
>> There is no technical argument to be made.  This is about the
>> community and you as maintainer wanting to accept code that complies
>> to your decision or not.
> 
> If you are not willing to make technical arguments, I don't have to
> provide any further reasons in this thread. It's your choice. In the
> meantime, the short answer is NAK.

From my understanding for this given device when the firmware was developed PSCI wasn’t available.  I’m not really familiar with the details.

- k

-- 
Qualcomm Innovation Center, Inc.
The Qualcomm Innovation Center, Inc. is a member of the Code Aurora Forum,
a Linux Foundation Collaborative Project

Re: [RFC PATCH 0/5] Add smp booting support for Qualcomm ARMv8 SoCs

[Kumar Gala]

> On Apr 14, 2015, at 9:21 AM, Kumar Gala <galak@codeaurora.org> wrote:
> 
>>>> 
>>>> So please come up with proper technical arguments rather than the kernel
>>>> should take whatever SoC vendors dreamt of.
>>> 
>>> There is no technical argument to be made.  This is about the
>>> community and you as maintainer wanting to accept code that complies
>>> to your decision or not.
>> 
>> If you are not willing to make technical arguments, I don't have to
>> provide any further reasons in this thread. It's your choice. In the
>> meantime, the short answer is NAK.

I assume you would than NAK someone trying to get support for their Nexus 9 Tablet using a Tegra K1.  It appears the shipping code for that device didn’t use PSCI (again guessing because it wasn’t available at the time).

https://android.googlesource.com/kernel/tegra/+/android-tegra-flounder-3.10-lollipop-release/arch/arm64/mach-tegra/platsmp.c

If so, I find this counter to the Linux kernel communities normal desire to support the most hardware platforms.

- k

-- 
Qualcomm Innovation Center, Inc.
The Qualcomm Innovation Center, Inc. is a member of the Code Aurora Forum,
a Linux Foundation Collaborative Project

Re: [RFC PATCH 0/5] Add smp booting support for Qualcomm ARMv8 SoCs

[Mark Rutland]

On Tue, Apr 14, 2015 at 03:44:11PM +0100, Kumar Gala wrote:
> 
> > On Apr 14, 2015, at 9:21 AM, Kumar Gala <galak@codeaurora.org> wrote:
> > 
> >>>> 
> >>>> So please come up with proper technical arguments rather than the kernel
> >>>> should take whatever SoC vendors dreamt of.
> >>> 
> >>> There is no technical argument to be made.  This is about the
> >>> community and you as maintainer wanting to accept code that complies
> >>> to your decision or not.
> >> 
> >> If you are not willing to make technical arguments, I don't have to
> >> provide any further reasons in this thread. It's your choice. In the
> >> meantime, the short answer is NAK.
> 
> I assume you would than NAK someone trying to get support for their
> Nexus 9 Tablet using a Tegra K1.

That would depend on how they try to boot secondary CPUs. It's
unfortunate that a product is shipping with an arbitrary implementation
specific SMP bringup mechanism, but its existence doesn't necessitate
that we support it.

People are currently working on PSCI support for 32-bit Tegra platforms
in U-Boot, and it's my understanding that a reasonable proportion of
that could should be possible to reuse for 64-bit. That may be
applicable to the Nexus 9.

Otherwise it's possible to have a shim which can place the secondaries
into a spin-table. It looks like that would be necessary anyway; there
seem to be some egregious boot protocol violations. 

> It appears the shipping code for that device didn’t use PSCI (again guessing because it wasn’t available at the time).
> 
> https://android.googlesource.com/kernel/tegra/+/android-tegra-flounder-3.10-lollipop-release/arch/arm64/mach-tegra/platsmp.c

Commit e790f1deb26a2e23 ("arm64: psci: add support for PSCI invocations
from the kernel") has been upstream since v3.9-rc1. It's even in the
(v3.10 derived) tree you link to:

https://android.googlesource.com/kernel/tegra/+/android-tegra-flounder-3.10-lollipop-release/arch/arm64/kernel/psci.c

As is spin-table:

https://android.googlesource.com/kernel/tegra/+/android-tegra-flounder-3.10-lollipop-release/arch/arm64/kernel/smp_spin_table.c

> If so, I find this counter to the Linux kernel communities normal desire to support the most hardware platforms.

Supporting hardware platforms doesn't mean that we must accept code
which we believe to be problematic.

We don't want implementation-specific SMP bringup mechanisms because
we've learnt from the lessons of the past. They're almost always
ill-defined, not reusable, and they're a maintenance burden for all
system software targeting ARM which gets worse over time.

If there are technical problems with the existing mechanisms, we're open
to solving them. Others have engaged with the kernel community and/or
ARM (in the case of PSCI) to do so, and all others upstream so far have
used common enable methods.

Mark. 

Re: [RFC PATCH 0/5] Add smp booting support for Qualcomm ARMv8 SoCs

[Lorenzo Pieralisi]

On Tue, Apr 14, 2015 at 03:21:17PM +0100, Kumar Gala wrote:

[...]

> > Looking beyond this set of patches, I can foresee that you won't care
> > about the generic arm64 cpuidle driver either, or more precisely the
> > separation between cpuidle subsystem+driver and the SoC-specific
> > back-end (cpu_operations).
> 
> That's probably true for what I guess are a number of reasons.  I'm guessing the arm64 cpuidle driver expects PSCI.

Wrap lines sensibly please.

The arm64 cpuidle driver, that is now arm generic cpuidle driver does
not expect anything apart from an enable-method (and you pulled
part of its back-end implementation for arm32 Qualcomm platforms, FYI).

It took years to consolidate it and the main reason was the lack of
standard interfaces for power down/up sequences that this patchset of
yours wants to promote in arm64 world.

The lack of standard power interfaces may not have been an issue for you,
who cares about Qualcomm code, it has been a sore issue for people
trying to generalize things across ARM platforms in the kernel, which is
the only sensible way forward.

PSCI is a standard interface (and Qualcomm are already contributing to
it, for the records) that can certainly be extended, and you are welcome
to contribute to it, but certainly not ignored.

Thanks,
Lorenzo

Re: [RFC PATCH 0/5] Add smp booting support for Qualcomm ARMv8 SoCs

[Lina Iyer]

On Tue, Apr 14 2015 at 16:32 -0600, Lorenzo Pieralisi wrote:
>On Tue, Apr 14, 2015 at 03:21:17PM +0100, Kumar Gala wrote:
>
>[...]
>
>> > Looking beyond this set of patches, I can foresee that you won't care
>> > about the generic arm64 cpuidle driver either, or more precisely the
>> > separation between cpuidle subsystem+driver and the SoC-specific
>> > back-end (cpu_operations).
>>
>> That's probably true for what I guess are a number of reasons.  I'm guessing the arm64 cpuidle driver expects PSCI.
>
>Wrap lines sensibly please.
>
>The arm64 cpuidle driver, that is now arm generic cpuidle driver does
>not expect anything apart from an enable-method (and you pulled
>part of its back-end implementation for arm32 Qualcomm platforms, FYI).
>
The backend for this SoC would leverage the same platform code as ARM32.
The cpu_operations callbacks for init and suspend will call into the the
same platform functions used by arm32 QCOM SoCs.

Thanks,
Lina

>It took years to consolidate it and the main reason was the lack of
>standard interfaces for power down/up sequences that this patchset of
>yours wants to promote in arm64 world.
>
>The lack of standard power interfaces may not have been an issue for you,
>who cares about Qualcomm code, it has been a sore issue for people
>trying to generalize things across ARM platforms in the kernel, which is
>the only sensible way forward.
>
>PSCI is a standard interface (and Qualcomm are already contributing to
>it, for the records) that can certainly be extended, and you are welcome
>to contribute to it, but certainly not ignored.
>
>Thanks,
>Lorenzo
>--
>To unsubscribe from this list: send the line "unsubscribe linux-arm-msm" in
>the body of a message to majordomo@vger.kernel.org
>More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [RFC PATCH 0/5] Add smp booting support for Qualcomm ARMv8 SoCs

[Lorenzo Pieralisi]

On Wed, Apr 15, 2015 at 05:17:59PM +0100, Lina Iyer wrote:
> On Tue, Apr 14 2015 at 16:32 -0600, Lorenzo Pieralisi wrote:
> >On Tue, Apr 14, 2015 at 03:21:17PM +0100, Kumar Gala wrote:
> >
> >[...]
> >
> >> > Looking beyond this set of patches, I can foresee that you won't care
> >> > about the generic arm64 cpuidle driver either, or more precisely the
> >> > separation between cpuidle subsystem+driver and the SoC-specific
> >> > back-end (cpu_operations).
> >>
> >> That's probably true for what I guess are a number of reasons.  I'm guessing the arm64 cpuidle driver expects PSCI.
> >
> >Wrap lines sensibly please.
> >
> >The arm64 cpuidle driver, that is now arm generic cpuidle driver does
> >not expect anything apart from an enable-method (and you pulled
> >part of its back-end implementation for arm32 Qualcomm platforms, FYI).
> >
> The backend for this SoC would leverage the same platform code as ARM32.
> The cpu_operations callbacks for init and suspend will call into the the
> same platform functions used by arm32 QCOM SoCs.

It is understood, but this does not mean we should merge this patchset,
actually it is the other way around. It was extremely complicated
to factor out some common CPUidle bits because of the prolification
of power down interfaces in arm/mach code, each with its quirks
du jour.

If we had a standard interface (that encompasses what all ARM mach code
power interfaces do, basically PSCI) when arm32 power management code
was being pushed upstream we would not have that power management arm/mach
code today.

PSCI is there to solve that problem, and it predates v8, there is no
reason to merge code that provides no added value (I am obviously
talking about the pseudo-boot protocol this patchset is enabling, not
the platforms themselves which we definitely want to support upstream,
with some preconditions that are equal for everyone) and to leverage
a legacy that does not exist for arm64.

Thanks,
Lorenzo

> 
> Thanks,
> Lina
> 
> >It took years to consolidate it and the main reason was the lack of
> >standard interfaces for power down/up sequences that this patchset of
> >yours wants to promote in arm64 world.
> >
> >The lack of standard power interfaces may not have been an issue for you,
> >who cares about Qualcomm code, it has been a sore issue for people
> >trying to generalize things across ARM platforms in the kernel, which is
> >the only sensible way forward.
> >
> >PSCI is a standard interface (and Qualcomm are already contributing to
> >it, for the records) that can certainly be extended, and you are welcome
> >to contribute to it, but certainly not ignored.
> >
> >Thanks,
> >Lorenzo
> >--
> >To unsubscribe from this list: send the line "unsubscribe linux-arm-msm" in
> >the body of a message to majordomo@vger.kernel.org
> >More majordomo info at  http://vger.kernel.org/majordomo-info.html
> 

Re: [RFC PATCH 0/5] Add smp booting support for Qualcomm ARMv8 SoCs

[Catalin Marinas]

On Tue, Apr 14, 2015 at 09:21:17AM -0500, Kumar Gala wrote:
> On Apr 13, 2015, at 4:41 AM, Catalin Marinas <catalin.marinas@arm.com> wrote:
> > On Fri, Apr 10, 2015 at 02:06:33PM -0500, Kumar Gala wrote:
> >> On Apr 10, 2015, at 11:10 AM, Catalin Marinas <catalin.marinas@arm.com> wrote:
> >> Qualcomm choose for whatever reasons to not implement it.  There are
> >> examples on other architectures supporting non-standard platforms all
> >> the time (x86 supported Voyager and SGI VIS for a long time).  As far
> >> as I can tell you are just wanting uniformity to impose this rule.
> > 
> > Don't confuse non-standard hardware (which has always been acceptable on
> > ARM) with non-standard ways of entering the kernel from firmware,
> > whether it's a primary or secondary CPU. Just look at how many smp_ops
> > structures are defined on x86.
> 
> When Voyager was supported it had a unique means for SMP bring up.

And was it fixed afterwards on newer board revisions?

> In the 4.0 kernel MIPS supports 13 different means, PowerPC has 14
> different means, ARM has 36 different means.

Obviously you don't think there is any problem with having 36 different
ways to enable secondary CPUs. I have a completely different opinion.
While it was not feasible to change the rules on arm32, it's a good
opportunity to impose some rules on arm64, given the new AArch64
exception model.

> > Looking beyond this set of patches, I can foresee that you won't care
> > about the generic arm64 cpuidle driver either, or more precisely the
> > separation between cpuidle subsystem+driver and the SoC-specific
> > back-end (cpu_operations).
> 
> That’s probably true for what I guess are a number of reasons.  I’m
> guessing the arm64 cpuidle driver expects PSCI.

Lorenzo replied already. It expects a cpu_operations back-end
implementing the cpu_suspend function. The only enable-method we have
providing cpu_suspend functionality is PSCI.

> >>> We haven't really asked for anything difficult like hardware changes,
> >>> such decisions are left with Qualcomm. We asked for a standard secure
> >>> firmware interface, either an existing one or, if not suitable (with
> >>> good arguments), to come up with a proposal for an alternative
> >>> _standard_ interface. I don't even have the certitude that the firmware
> >>> interface used in these patches would be consistent across Qualcomm
> >>> SoCs, let alone being properly documented.
> >> 
> >> If and when those issues arise we can accept or reject that code.
> >> Right now when I look at the impact to supporting this to generic
> >> arch/arm64 kernel it is either non-existant if we use the
> >> CPU_OF_TABLES or extremely minor if we just add an entry to the
> >> supported_cpu_ops struct.
> > 
> > Again, see above about how such change is no longer small when each SoC
> > does the same.
> 
> There are so many places that we already deal with per SoC uniqueness.
> We setup a software interface in the kernel and people develop towards
> it.
> 
> Are you saying that going forward all SoCs should have the same
> clocking programming interface to ease portability?  Do you expect
> them all to have the same pin control programming interface if someone
> defines a firmware abstraction?

What you said is primarily about device interfaces and given how
different they are, most of them not even requiring firmware
interaction, it doesn't make sense to come up with a firmware
abstraction (well, it may actually be impossible). But if you spot some
common pattern, it's fine by me to propose such interface.

However, what I'm not talking about devices but CPUs. The ARM CPUs are
defined by an architecture spec and they have a consistent behaviour.
Hardware for CPU powering on/off is not architected/standardised and
that's not because ARM wouldn't want to but because SoC vendors want to
be able differentiate. Given that (on ARMv8 especially) Linux is not the
first thing to run on a CPU out of reset and it may need to go back to
firmware for deeper sleep states, it makes sense to at least hide some
of the implementation specific SoC details behind a standard firmware
interface. It keeps the hardware folk happy as they can design CPU power
on/off as they like with little impact on the software world. Yes, I
said "little" impact overall, the only thing needed is a shift of such
CPU enabling code from the kernel to firmware. ARM even provides an open
source (BSD) firmware to make things easier for partners and there is
similar work going into U-Boot.

Ideally you shouldn't place all the firmware in ROM (just part of it)
but even if it is and we find bugs that require working around in the
kernel, we can apply quirks on a case by case basis. But what you are
arguing about is to plainly ignore this standardisation effort.

Long term plan is to avoid most of the SoC specific code in the kernel
before device_initcall() level (wouldn't it be nice if you could load
the SoC support from modules in initramfs?). The clocks are an exception
for the time being but it's something that could be sorted by sane prior
firmware initialisation and just frequency changes at run-time (that's
what we are actually imposing on ACPI-capable SoCs).

-- 
Catalin

Re: [RFC PATCH 0/5] Add smp booting support for Qualcomm ARMv8 SoCs

[Mark Rutland]

On Fri, Apr 10, 2015 at 11:05:29AM +0100, Catalin Marinas wrote:
> On Thu, Apr 09, 2015 at 12:37:06PM -0500, Kumar Gala wrote:
> > This patch set adds support for SMP boot on the MSM8x16 family of Qualcomm SoCs.
> > 
> > To support SMP on the MSM8x16 SoCs we need to add ARMv8/64-bit SCM interfaces to
> > setup the boot/release addresses for the secondary CPUs.  In addition we need
> > a uniquie set of cpu ops.  I'm aware the desired methods for booting secondary
> > CPUs is either via spintable or PSCI.  However, these SoCs are shipping with a
> > firmware that does not support those methods.
> 
> And the reason is? Some guesses:
> 
> a) QC doesn't think boot interface (and cpuidle) standardisation is
>    worth the effort (to put it nicely)
> b) The hardware was available before we even mentioned PSCI
> c) PSCI is not suitable for the QC's SCM interface
> d) Any combination of the above
> 
> I strongly suspect it's point (a). Should we expect future QC hardware
> to do the same?
> 
> You could argue the reason was (b), though we've been discussing PSCI
> for at least two years and, according to QC press releases, MSM8916
> started sampling in 2014.
> 
> The only valid reason is (c) and if that's the case, I would expect a
> proposal for a new firmware interface protocol (it could be PSCI-based),
> well documented, that can be shared with others that may encounter the
> same shortcomings.

There's no need to even fork PSCI. The PSCI specification will evolve
over time as vendors request changes and we try to accomodate them.

If there's something that PSCI doesn't do that you need it to, contact
ARM. Other vendors already have.

Mark.

Re: [RFC PATCH 0/5] Add smp booting support for Qualcomm ARMv8 SoCs

[Kumar Gala]

> On Apr 14, 2015, at 11:36 AM, Mark Rutland <mark.rutland@arm.com> wrote:
> 
> On Fri, Apr 10, 2015 at 11:05:29AM +0100, Catalin Marinas wrote:
>> On Thu, Apr 09, 2015 at 12:37:06PM -0500, Kumar Gala wrote:
>>> This patch set adds support for SMP boot on the MSM8x16 family of Qualcomm SoCs.
>>> 
>>> To support SMP on the MSM8x16 SoCs we need to add ARMv8/64-bit SCM interfaces to
>>> setup the boot/release addresses for the secondary CPUs.  In addition we need
>>> a uniquie set of cpu ops.  I'm aware the desired methods for booting secondary
>>> CPUs is either via spintable or PSCI.  However, these SoCs are shipping with a
>>> firmware that does not support those methods.
>> 
>> And the reason is? Some guesses:
>> 
>> a) QC doesn't think boot interface (and cpuidle) standardisation is
>>   worth the effort (to put it nicely)
>> b) The hardware was available before we even mentioned PSCI
>> c) PSCI is not suitable for the QC's SCM interface
>> d) Any combination of the above
>> 
>> I strongly suspect it's point (a). Should we expect future QC hardware
>> to do the same?
>> 
>> You could argue the reason was (b), though we've been discussing PSCI
>> for at least two years and, according to QC press releases, MSM8916
>> started sampling in 2014.
>> 
>> The only valid reason is (c) and if that's the case, I would expect a
>> proposal for a new firmware interface protocol (it could be PSCI-based),
>> well documented, that can be shared with others that may encounter the
>> same shortcomings.
> 
> There's no need to even fork PSCI. The PSCI specification will evolve
> over time as vendors request changes and we try to accomodate them.
> 
> If there's something that PSCI doesn't do that you need it to, contact
> ARM. Other vendors already have.

But what is someone to do between the period of getting PSCI spec updated and needing to ship a product with firmware?

The take still sounds like if you don’t implement an exact version of PSCI you are screwed from being supported in the upstream ARM64 kernel.

- k

-- 
Qualcomm Innovation Center, Inc.
The Qualcomm Innovation Center, Inc. is a member of the Code Aurora Forum,
a Linux Foundation Collaborative Project

Re: [RFC PATCH 0/5] Add smp booting support for Qualcomm ARMv8 SoCs

[Catalin Marinas]

On Tue, Apr 14, 2015 at 02:49:04PM -0500, Kumar Gala wrote:
> On Apr 14, 2015, at 11:36 AM, Mark Rutland <mark.rutland@arm.com> wrote:
> > On Fri, Apr 10, 2015 at 11:05:29AM +0100, Catalin Marinas wrote:
> >> On Thu, Apr 09, 2015 at 12:37:06PM -0500, Kumar Gala wrote:
> >>> This patch set adds support for SMP boot on the MSM8x16 family of Qualcomm SoCs.
> >>> 
> >>> To support SMP on the MSM8x16 SoCs we need to add ARMv8/64-bit SCM interfaces to
> >>> setup the boot/release addresses for the secondary CPUs.  In addition we need
> >>> a uniquie set of cpu ops.  I'm aware the desired methods for booting secondary
> >>> CPUs is either via spintable or PSCI.  However, these SoCs are shipping with a
> >>> firmware that does not support those methods.
> >> 
> >> And the reason is? Some guesses:
> >> 
> >> a) QC doesn't think boot interface (and cpuidle) standardisation is
> >>   worth the effort (to put it nicely)
> >> b) The hardware was available before we even mentioned PSCI
> >> c) PSCI is not suitable for the QC's SCM interface
> >> d) Any combination of the above
> >> 
> >> I strongly suspect it's point (a). Should we expect future QC hardware
> >> to do the same?
> >> 
> >> You could argue the reason was (b), though we've been discussing PSCI
> >> for at least two years and, according to QC press releases, MSM8916
> >> started sampling in 2014.
> >> 
> >> The only valid reason is (c) and if that's the case, I would expect a
> >> proposal for a new firmware interface protocol (it could be PSCI-based),
> >> well documented, that can be shared with others that may encounter the
> >> same shortcomings.
> > 
> > There's no need to even fork PSCI. The PSCI specification will evolve
> > over time as vendors request changes and we try to accomodate them.
> > 
> > If there's something that PSCI doesn't do that you need it to, contact
> > ARM. Other vendors already have.

Mostly yes but there may be valid reasons for not being able to use
PSCI. The spin-table method is still a firmware interface, though not
necessarily secure (a.k.a. SMC-based). The ACPI parking protocol is
another and, who knows, maybe we define a way to park CPUs back to
firmware without SMC calls (when EL3 is not available).

> But what is someone to do between the period of getting PSCI spec
> updated and needing to ship a product with firmware?
> 
> The take still sounds like if you don’t implement an exact version of
> PSCI you are screwed from being supported in the upstream ARM64
> kernel.

These are silly arguments. There is a big difference between "we
couldn't get the firmware implementing the standard for the early
silicon but we are working on fixing it for future revisions" vs. "we
don't give a s**t about these standards, the kernel must be inclusive".
So please make up your mind on which direction you want to pursue.

-- 
Catalin

Re: [RFC PATCH 0/5] Add smp booting support for Qualcomm ARMv8 SoCs

[Rob Clark]

On Tue, Apr 14, 2015 at 5:17 PM, Catalin Marinas
<catalin.marinas-5wv7dgnIgG8@public.gmane.org> wrote:
> On Tue, Apr 14, 2015 at 02:49:04PM -0500, Kumar Gala wrote:
>> On Apr 14, 2015, at 11:36 AM, Mark Rutland <mark.rutland-5wv7dgnIgG8@public.gmane.org> wrote:
>> > On Fri, Apr 10, 2015 at 11:05:29AM +0100, Catalin Marinas wrote:
>> >> On Thu, Apr 09, 2015 at 12:37:06PM -0500, Kumar Gala wrote:
>> >>> This patch set adds support for SMP boot on the MSM8x16 family of Qualcomm SoCs.
>> >>>
>> >>> To support SMP on the MSM8x16 SoCs we need to add ARMv8/64-bit SCM interfaces to
>> >>> setup the boot/release addresses for the secondary CPUs.  In addition we need
>> >>> a uniquie set of cpu ops.  I'm aware the desired methods for booting secondary
>> >>> CPUs is either via spintable or PSCI.  However, these SoCs are shipping with a
>> >>> firmware that does not support those methods.
>> >>
>> >> And the reason is? Some guesses:
>> >>
>> >> a) QC doesn't think boot interface (and cpuidle) standardisation is
>> >>   worth the effort (to put it nicely)
>> >> b) The hardware was available before we even mentioned PSCI
>> >> c) PSCI is not suitable for the QC's SCM interface
>> >> d) Any combination of the above
>> >>
>> >> I strongly suspect it's point (a). Should we expect future QC hardware
>> >> to do the same?
>> >>
>> >> You could argue the reason was (b), though we've been discussing PSCI
>> >> for at least two years and, according to QC press releases, MSM8916
>> >> started sampling in 2014.
>> >>
>> >> The only valid reason is (c) and if that's the case, I would expect a
>> >> proposal for a new firmware interface protocol (it could be PSCI-based),
>> >> well documented, that can be shared with others that may encounter the
>> >> same shortcomings.
>> >
>> > There's no need to even fork PSCI. The PSCI specification will evolve
>> > over time as vendors request changes and we try to accomodate them.
>> >
>> > If there's something that PSCI doesn't do that you need it to, contact
>> > ARM. Other vendors already have.
>
> Mostly yes but there may be valid reasons for not being able to use
> PSCI. The spin-table method is still a firmware interface, though not
> necessarily secure (a.k.a. SMC-based). The ACPI parking protocol is
> another and, who knows, maybe we define a way to park CPUs back to
> firmware without SMC calls (when EL3 is not available).
>
>> But what is someone to do between the period of getting PSCI spec
>> updated and needing to ship a product with firmware?
>>
>> The take still sounds like if you don’t implement an exact version of
>> PSCI you are screwed from being supported in the upstream ARM64
>> kernel.
>
> These are silly arguments. There is a big difference between "we
> couldn't get the firmware implementing the standard for the early
> silicon but we are working on fixing it for future revisions" vs. "we
> don't give a s**t about these standards, the kernel must be inclusive".
> So please make up your mind on which direction you want to pursue.
>

Just speaking as an outsider to this topic, but seems like most/all
tablets/phones/etc ship with signed firmware.  Which means for most of
the population, upgrading the firmware to a new version which did
support the standard (assuming it existed), isn't really an option on
our devices, any more than fixing buggy acpi tables is on our
laptops..

BR,
-R


> --
> Catalin
>
> _______________________________________________
> linux-arm-kernel mailing list
> linux-arm-kernel-IAPFreCvJWM7uuMidbF8XUB+6BGkLq7r@public.gmane.org
> http://lists.infradead.org/mailman/listinfo/linux-arm-kernel
--
To unsubscribe from this list: send the line "unsubscribe devicetree" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [RFC PATCH 0/5] Add smp booting support for Qualcomm ARMv8 SoCs

[Catalin Marinas]

On Tue, Apr 14, 2015 at 05:48:48PM -0400, Rob Clark wrote:
> Just speaking as an outsider to this topic, but seems like most/all
> tablets/phones/etc ship with signed firmware.  Which means for most of
> the population, upgrading the firmware to a new version which did
> support the standard (assuming it existed), isn't really an option on
> our devices, any more than fixing buggy acpi tables is on our
> laptops..

I wouldn't expect most population to build their own kernels on
tablets/phones. And even if you could install a custom kernel, mainline
rarely runs on such devices because of tons of out of tree patches (just
look at the Nexus 9 patches that Kumar pointed at; even ignoring the
booting protocol they are extremely far from an upstreamable form).

-- 
Catalin
--
To unsubscribe from this list: send the line "unsubscribe devicetree" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [RFC PATCH 0/5] Add smp booting support for Qualcomm ARMv8 SoCs

[Rob Clark]

On Wed, Apr 15, 2015 at 9:34 AM, Catalin Marinas
<catalin.marinas@arm.com> wrote:
> On Tue, Apr 14, 2015 at 05:48:48PM -0400, Rob Clark wrote:
>> Just speaking as an outsider to this topic, but seems like most/all
>> tablets/phones/etc ship with signed firmware.  Which means for most of
>> the population, upgrading the firmware to a new version which did
>> support the standard (assuming it existed), isn't really an option on
>> our devices, any more than fixing buggy acpi tables is on our
>> laptops..
>
> I wouldn't expect most population to build their own kernels on
> tablets/phones. And even if you could install a custom kernel, mainline
> rarely runs on such devices because of tons of out of tree patches (just
> look at the Nexus 9 patches that Kumar pointed at; even ignoring the
> booting protocol they are extremely far from an upstreamable form).
>

my point being, that it happens some times.. for example John Stultz's
work on nexus7:

https://plus.google.com/111524780435806926688/posts/DzvpMLmzQiQ

If this had been a year or two in the future and on some 64b
snapdragon, and support for devices with non-PSCI fw is rejected, then
he'd be stuck.

There are folks who are working to get saner, more-upstream kernels
working on devices.. and improving kernel infrastructure for
device-needs (well, in my neck of the woods, there is drm/kms atomic
and dsi/panel framework stuff.. I'm sure other similar things in other
kernel domains).  And it seems like that is a good thing to encourage,
rather than stymie.

BR,
-R

> --
> Catalin

Re: [RFC PATCH 0/5] Add smp booting support for Qualcomm ARMv8 SoCs

[Catalin Marinas]

On Wed, Apr 15, 2015 at 11:01:17AM -0400, Rob Clark wrote:
> On Wed, Apr 15, 2015 at 9:34 AM, Catalin Marinas
> <catalin.marinas@arm.com> wrote:
> > On Tue, Apr 14, 2015 at 05:48:48PM -0400, Rob Clark wrote:
> >> Just speaking as an outsider to this topic, but seems like most/all
> >> tablets/phones/etc ship with signed firmware.  Which means for most of
> >> the population, upgrading the firmware to a new version which did
> >> support the standard (assuming it existed), isn't really an option on
> >> our devices, any more than fixing buggy acpi tables is on our
> >> laptops..
> >
> > I wouldn't expect most population to build their own kernels on
> > tablets/phones. And even if you could install a custom kernel, mainline
> > rarely runs on such devices because of tons of out of tree patches (just
> > look at the Nexus 9 patches that Kumar pointed at; even ignoring the
> > booting protocol they are extremely far from an upstreamable form).
> 
> my point being, that it happens some times.. for example John Stultz's
> work on nexus7:
> 
> https://plus.google.com/111524780435806926688/posts/DzvpMLmzQiQ
> 
> If this had been a year or two in the future and on some 64b
> snapdragon, and support for devices with non-PSCI fw is rejected, then
> he'd be stuck.
> 
> There are folks who are working to get saner, more-upstream kernels
> working on devices.. and improving kernel infrastructure for
> device-needs (well, in my neck of the woods, there is drm/kms atomic
> and dsi/panel framework stuff.. I'm sure other similar things in other
> kernel domains).  And it seems like that is a good thing to encourage,
> rather than stymie.

I'm not looking to discourage individuals trying to get upstream support
for older boards. Should the need arise, we'll look at options which may
or may not include kernel changes (e.g. wrap the kernel in a shim).

But I'm definitely going to discourage companies like Qualcomm
deliberately ignoring the existing booting protocols while trying to get
their code upstream. This patch series is posted by Qualcomm without
providing any technical reason on why they don't want to/couldn't use
PSCI (well, I guess there is no technical reason but they may not care
much about mainline either).

-- 
Catalin

Re: [RFC PATCH 0/5] Add smp booting support for Qualcomm ARMv8 SoCs

[Rob Clark]

On Thu, Apr 16, 2015 at 11:21 AM, Catalin Marinas
<catalin.marinas-5wv7dgnIgG8@public.gmane.org> wrote:
> On Wed, Apr 15, 2015 at 11:01:17AM -0400, Rob Clark wrote:
>> On Wed, Apr 15, 2015 at 9:34 AM, Catalin Marinas
>> <catalin.marinas-5wv7dgnIgG8@public.gmane.org> wrote:
>> > On Tue, Apr 14, 2015 at 05:48:48PM -0400, Rob Clark wrote:
>> >> Just speaking as an outsider to this topic, but seems like most/all
>> >> tablets/phones/etc ship with signed firmware.  Which means for most of
>> >> the population, upgrading the firmware to a new version which did
>> >> support the standard (assuming it existed), isn't really an option on
>> >> our devices, any more than fixing buggy acpi tables is on our
>> >> laptops..
>> >
>> > I wouldn't expect most population to build their own kernels on
>> > tablets/phones. And even if you could install a custom kernel, mainline
>> > rarely runs on such devices because of tons of out of tree patches (just
>> > look at the Nexus 9 patches that Kumar pointed at; even ignoring the
>> > booting protocol they are extremely far from an upstreamable form).
>>
>> my point being, that it happens some times.. for example John Stultz's
>> work on nexus7:
>>
>> https://plus.google.com/111524780435806926688/posts/DzvpMLmzQiQ
>>
>> If this had been a year or two in the future and on some 64b
>> snapdragon, and support for devices with non-PSCI fw is rejected, then
>> he'd be stuck.
>>
>> There are folks who are working to get saner, more-upstream kernels
>> working on devices.. and improving kernel infrastructure for
>> device-needs (well, in my neck of the woods, there is drm/kms atomic
>> and dsi/panel framework stuff.. I'm sure other similar things in other
>> kernel domains).  And it seems like that is a good thing to encourage,
>> rather than stymie.
>
> I'm not looking to discourage individuals trying to get upstream support
> for older boards. Should the need arise, we'll look at options which may
> or may not include kernel changes (e.g. wrap the kernel in a shim).

I had wondered about that, but afaiu psci is a smc interface, and I
would assume bootloader drops you out of secure mode before entering
the kernel or whatever comes after first stage bootloader if you are
chain-loading?  So wasn't sure even how that could work..   (ofc, well
outside of my areas so I could be quite off base)

If there is a workaround, then I am much less concerned, and would
rather talk about that :-)

> But I'm definitely going to discourage companies like Qualcomm
> deliberately ignoring the existing booting protocols while trying to get
> their code upstream. This patch series is posted by Qualcomm without
> providing any technical reason on why they don't want to/couldn't use
> PSCI (well, I guess there is no technical reason but they may not care
> much about mainline either).

Sure.. just trying to make sure the wrong people don't end up being
the ones that suffer.  I would assume/expect that it is at least
possible for qcom to change firmware/bootloader for their dev boards
and future devices and whatnot, whether they grumble about it or not.
But I guess most of what the general public has are devices w/ signed
fw, which is why "go fix your firmware" is an option that sets off
alarm bells for me.

I guess the first device where this will matter to me and a large
group of community folks would be the dragonboard 410c..  *hopefully*
it does not require signed firmware or at least qcom could make
available signed firmware which supports psci..

BR,
-R

> --
> Catalin
--
To unsubscribe from this list: send the line "unsubscribe devicetree" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [RFC PATCH 0/5] Add smp booting support for Qualcomm ARMv8 SoCs

[Catalin Marinas]

On Thu, Apr 16, 2015 at 01:17:32PM -0400, Rob Clark wrote:
> On Thu, Apr 16, 2015 at 11:21 AM, Catalin Marinas
> <catalin.marinas-5wv7dgnIgG8@public.gmane.org> wrote:
> > On Wed, Apr 15, 2015 at 11:01:17AM -0400, Rob Clark wrote:
> >> There are folks who are working to get saner, more-upstream kernels
> >> working on devices.. and improving kernel infrastructure for
> >> device-needs (well, in my neck of the woods, there is drm/kms atomic
> >> and dsi/panel framework stuff.. I'm sure other similar things in other
> >> kernel domains).  And it seems like that is a good thing to encourage,
> >> rather than stymie.
> >
> > I'm not looking to discourage individuals trying to get upstream support
> > for older boards. Should the need arise, we'll look at options which may
> > or may not include kernel changes (e.g. wrap the kernel in a shim).
> 
> I had wondered about that, but afaiu psci is a smc interface, and I
> would assume bootloader drops you out of secure mode before entering
> the kernel or whatever comes after first stage bootloader if you are
> chain-loading?

One option is to run the shim at EL2 (hyp mode) which can intercept the
SMC calls from EL1. Another option is to release all the CPUs from
firmware into the shim and use a spin-table enable method.

-- 
Catalin
--
To unsubscribe from this list: send the line "unsubscribe devicetree" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [RFC PATCH 0/5] Add smp booting support for Qualcomm ARMv8 SoCs

[Matt Sealey]

Hi Rob,

On Thu, Apr 16, 2015 at 12:17 PM, Rob Clark <robdclark@gmail.com> wrote:
> On Thu, Apr 16, 2015 at 11:21 AM, Catalin Marinas
> <catalin.marinas@arm.com> wrote:
>
>> But I'm definitely going to discourage companies like Qualcomm
>> deliberately ignoring the existing booting protocols while trying to get
>> their code upstream. This patch series is posted by Qualcomm without
>> providing any technical reason on why they don't want to/couldn't use
>> PSCI (well, I guess there is no technical reason but they may not care
>> much about mainline either).
>
> Sure.. just trying to make sure the wrong people don't end up being
> the ones that suffer.  I would assume/expect that it is at least
> possible for qcom to change firmware/bootloader for their dev boards
> and future devices and whatnot, whether they grumble about it or not.
> But I guess most of what the general public has are devices w/ signed
> fw, which is why "go fix your firmware" is an option that sets off
> alarm bells for me.
>
> I guess the first device where this will matter to me and a large
> group of community folks would be the dragonboard 410c..  *hopefully*
> it does not require signed firmware or at least qcom could make
> available signed firmware which supports psci..

For development boards, one would hope there is a way to sign your own firmware.
You can't expect - even for a phone SoC - that the development boards require
entering some kind of Faustian contract for development of low-level
software. What if
someone wants to develop a platform that doesn't require signing?

That said most of these dev boards have completely mangled JTAG anyway, and
I know Inforce (and Hardkernel, and so on) love their barely-ever-updated custom
firmware binaries, so..

The best thing would be to pick up one of those boards and port a PSCI firmware
to it (ATF or your own..) and just embarrass the SoC vendor by having better
mainline power management support (implemented by 10 lines in a device tree)
with the complicated code hidden away behind the scenes there, like it
should have
been done in the first place..

Ta.
Matt Sealey <neko@bakuhatsu.net>

Re: [RFC PATCH 0/5] Add smp booting support for Qualcomm ARMv8 SoCs

[Lorenzo Pieralisi]

On Thu, Apr 09, 2015 at 06:37:06PM +0100, Kumar Gala wrote:
> This patch set adds support for SMP boot on the MSM8x16 family of Qualcomm SoCs.
> 
> To support SMP on the MSM8x16 SoCs we need to add ARMv8/64-bit SCM interfaces to
> setup the boot/release addresses for the secondary CPUs.  In addition we need
> a uniquie set of cpu ops.  I'm aware the desired methods for booting secondary
> CPUs is either via spintable or PSCI.  However, these SoCs are shipping with a
> firmware that does not support those methods.

Why ? Do not tell me you were not aware of those standard methods,
because I can't believe you.

If there were additional features to add to spin-table and PSCI,
you were, you are and you will always be welcome to debate them.

There is no justification for this patchset, honestly.

Lorenzo

Re: [RFC PATCH 0/5] Add smp booting support for Qualcomm ARMv8 SoCs

[Kumar Gala]

On Apr 10, 2015, at 6:03 AM, Lorenzo Pieralisi <lorenzo.pieralisi@arm.com> wrote:

> On Thu, Apr 09, 2015 at 06:37:06PM +0100, Kumar Gala wrote:
>> This patch set adds support for SMP boot on the MSM8x16 family of Qualcomm SoCs.
>> 
>> To support SMP on the MSM8x16 SoCs we need to add ARMv8/64-bit SCM interfaces to
>> setup the boot/release addresses for the secondary CPUs.  In addition we need
>> a uniquie set of cpu ops.  I'm aware the desired methods for booting secondary
>> CPUs is either via spintable or PSCI.  However, these SoCs are shipping with a
>> firmware that does not support those methods.
> 
> Why ? Do not tell me you were not aware of those standard methods,
> because I can't believe you.
> 
> If there were additional features to add to spin-table and PSCI,
> you were, you are and you will always be welcome to debate them.
> 
> There is no justification for this patchset, honestly.
> 
> Lorenzo

The justification for this patchset is support for a hardware platform that exists in the world.  The kernel usual is willing to accept such things as long as the code is reasonable.

- k

-- 
Qualcomm Innovation Center, Inc.
The Qualcomm Innovation Center, Inc. is a member of the Code Aurora Forum,
a Linux Foundation Collaborative Project

Re: [RFC PATCH 0/5] Add smp booting support for Qualcomm ARMv8 SoCs

[Lorenzo Pieralisi]

On Fri, Apr 10, 2015 at 04:25:54PM +0100, Kumar Gala wrote:
> 
> On Apr 10, 2015, at 6:03 AM, Lorenzo Pieralisi <lorenzo.pieralisi@arm.com> wrote:
> 
> > On Thu, Apr 09, 2015 at 06:37:06PM +0100, Kumar Gala wrote:
> >> This patch set adds support for SMP boot on the MSM8x16 family of Qualcomm SoCs.
> >> 
> >> To support SMP on the MSM8x16 SoCs we need to add ARMv8/64-bit SCM interfaces to
> >> setup the boot/release addresses for the secondary CPUs.  In addition we need
> >> a uniquie set of cpu ops.  I'm aware the desired methods for booting secondary
> >> CPUs is either via spintable or PSCI.  However, these SoCs are shipping with a
> >> firmware that does not support those methods.
> > 
> > Why ? Do not tell me you were not aware of those standard methods,
> > because I can't believe you.
> > 
> > If there were additional features to add to spin-table and PSCI,
> > you were, you are and you will always be welcome to debate them.
> > 
> > There is no justification for this patchset, honestly.
> > 
> > Lorenzo
> 
> The justification for this patchset is support for a hardware platform that exists in the world.  The kernel usual is willing to accept such things as long as the code is reasonable.

You are telling me you add a linker section for cpu_ops just for
"a platform that exists", sorry I do not believe that (and that's *not*
reasonable).

I do not like this line of reasoning, at all, because you were aware of
PSCI and ignored it, deliberately. Start by pushing code for platforms
upstream that support PSCI, "they are coming up next" does not cut it.

I can see power management mach code coming next, just no way.

Implement PSCI and the kernel will be willing to accept it, as it stands
as far as I am concerned that's a NAK on the series.

Lorenzo

Re: [RFC PATCH 0/5] Add smp booting support for Qualcomm ARMv8 SoCs

[Rob Herring]

On Fri, Apr 10, 2015 at 10:25 AM, Kumar Gala <galak@codeaurora.org> wrote:
>
> On Apr 10, 2015, at 6:03 AM, Lorenzo Pieralisi <lorenzo.pieralisi@arm.com> wrote:
>
>> On Thu, Apr 09, 2015 at 06:37:06PM +0100, Kumar Gala wrote:
>>> This patch set adds support for SMP boot on the MSM8x16 family of Qualcomm SoCs.
>>>
>>> To support SMP on the MSM8x16 SoCs we need to add ARMv8/64-bit SCM interfaces to
>>> setup the boot/release addresses for the secondary CPUs.  In addition we need
>>> a uniquie set of cpu ops.  I'm aware the desired methods for booting secondary
>>> CPUs is either via spintable or PSCI.  However, these SoCs are shipping with a
>>> firmware that does not support those methods.
>>
>> Why ? Do not tell me you were not aware of those standard methods,
>> because I can't believe you.
>>
>> If there were additional features to add to spin-table and PSCI,
>> you were, you are and you will always be welcome to debate them.
>>
>> There is no justification for this patchset, honestly.
>>
>> Lorenzo
>
> The justification for this patchset is support for a hardware platform that exists in the world.  The kernel usual is willing to accept such things as long as the code is reasonable.


>
> - k
>
> --
> Qualcomm Innovation Center, Inc.
> The Qualcomm Innovation Center, Inc. is a member of the Code Aurora Forum,
> a Linux Foundation Collaborative Project
>
>
> _______________________________________________
> linux-arm-kernel mailing list
> linux-arm-kernel@lists.infradead.org
> http://lists.infradead.org/mailman/listinfo/linux-arm-kernel