[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 1/5] firmware: qcom: scm: Split out 32-bit specific SCM code

[Kumar Gala]

Split out the 32-bit SCM implementation into its own file to prep for
supporting a 64-bit/ARM64 implementation as well.  We create a simple shim
to ensure both versions conform to the same interface.

Signed-off-by: Kumar Gala <galak@codeaurora.org>
---
 drivers/firmware/Makefile                      |   3 +-
 drivers/firmware/{qcom_scm.c => qcom_scm-32.c} |  15 +-
 drivers/firmware/qcom_scm.c                    | 442 +------------------------
 drivers/firmware/qcom_scm.h                    |  29 ++
 4 files changed, 42 insertions(+), 447 deletions(-)
 copy drivers/firmware/{qcom_scm.c => qcom_scm-32.c} (96%)
 create mode 100644 drivers/firmware/qcom_scm.h

diff --git a/drivers/firmware/Makefile b/drivers/firmware/Makefile
index 3fdd391..3001f1a 100644
--- a/drivers/firmware/Makefile
+++ b/drivers/firmware/Makefile
@@ -12,7 +12,8 @@ obj-$(CONFIG_ISCSI_IBFT_FIND)	+= iscsi_ibft_find.o
 obj-$(CONFIG_ISCSI_IBFT)	+= iscsi_ibft.o
 obj-$(CONFIG_FIRMWARE_MEMMAP)	+= memmap.o
 obj-$(CONFIG_QCOM_SCM)		+= qcom_scm.o
-CFLAGS_qcom_scm.o :=$(call as-instr,.arch_extension sec,-DREQUIRES_SEC=1)
+obj-$(CONFIG_QCOM_SCM)		+= qcom_scm-32.o
+CFLAGS_qcom_scm-32.o :=$(call as-instr,.arch_extension sec,-DREQUIRES_SEC=1)
 
 obj-$(CONFIG_GOOGLE_FIRMWARE)	+= google/
 obj-$(CONFIG_EFI)		+= efi/
diff --git a/drivers/firmware/qcom_scm.c b/drivers/firmware/qcom_scm-32.c
similarity index 96%
copy from drivers/firmware/qcom_scm.c
copy to drivers/firmware/qcom_scm-32.c
index 994b50f..89be15e 100644
--- a/drivers/firmware/qcom_scm.c
+++ b/drivers/firmware/qcom_scm-32.c
@@ -27,6 +27,7 @@
 #include <asm/outercache.h>
 #include <asm/cacheflush.h>
 
+#include "qcom_scm.h"
 
 #define QCOM_SCM_ENOMEM		-5
 #define QCOM_SCM_EOPNOTSUPP	-4
@@ -386,8 +387,6 @@ u32 qcom_scm_get_version(void)
 }
 EXPORT_SYMBOL(qcom_scm_get_version);
 
-#define QCOM_SCM_SVC_BOOT			0x1
-#define QCOM_SCM_BOOT_ADDR			0x1
 /*
  * Set the cold/warm boot address for one of the CPU cores.
  */
@@ -412,7 +411,7 @@ static int qcom_scm_set_boot_addr(u32 addr, int flags)
  * Set the cold boot address of the cpus. Any cpu outside the supported
  * range would be removed from the cpu present mask.
  */
-int qcom_scm_set_cold_boot_addr(void *entry, const cpumask_t *cpus)
+int __qcom_scm_set_cold_boot_addr(void *entry, const cpumask_t *cpus)
 {
 	int flags = 0;
 	int cpu;
@@ -435,7 +434,6 @@ int qcom_scm_set_cold_boot_addr(void *entry, const cpumask_t *cpus)
 
 	return qcom_scm_set_boot_addr(virt_to_phys(entry), flags);
 }
-EXPORT_SYMBOL(qcom_scm_set_cold_boot_addr);
 
 /**
  * qcom_scm_set_warm_boot_addr() - Set the warm boot address for cpus
@@ -445,7 +443,7 @@ EXPORT_SYMBOL(qcom_scm_set_cold_boot_addr);
  * Set the Linux entry point for the SCM to transfer control to when coming
  * out of a power down. CPU power down may be executed on cpuidle or hotplug.
  */
-int qcom_scm_set_warm_boot_addr(void *entry, const cpumask_t *cpus)
+int __qcom_scm_set_warm_boot_addr(void *entry, const cpumask_t *cpus)
 {
 	int ret;
 	int flags = 0;
@@ -473,10 +471,6 @@ int qcom_scm_set_warm_boot_addr(void *entry, const cpumask_t *cpus)
 
 	return ret;
 }
-EXPORT_SYMBOL(qcom_scm_set_warm_boot_addr);
-
-#define QCOM_SCM_CMD_TERMINATE_PC	0x2
-#define QCOM_SCM_FLUSH_FLAG_MASK	0x3
 
 /**
  * qcom_scm_cpu_power_down() - Power down the cpu
@@ -486,9 +480,8 @@ EXPORT_SYMBOL(qcom_scm_set_warm_boot_addr);
  * the control would return from this function, otherwise, the cpu jumps to the
  * warm boot entry point set for this cpu upon reset.
  */
-void qcom_scm_cpu_power_down(u32 flags)
+void __qcom_scm_cpu_power_down(u32 flags)
 {
 	qcom_scm_call_atomic1(QCOM_SCM_SVC_BOOT, QCOM_SCM_CMD_TERMINATE_PC,
 			flags & QCOM_SCM_FLUSH_FLAG_MASK);
 }
-EXPORT_SYMBOL(qcom_scm_cpu_power_down);
diff --git a/drivers/firmware/qcom_scm.c b/drivers/firmware/qcom_scm.c
index 994b50f..9989241 100644
--- a/drivers/firmware/qcom_scm.c
+++ b/drivers/firmware/qcom_scm.c
@@ -16,393 +16,12 @@
  * 02110-1301, USA.
  */
 
-#include <linux/slab.h>
-#include <linux/io.h>
-#include <linux/module.h>
-#include <linux/mutex.h>
-#include <linux/errno.h>
-#include <linux/err.h>
+#include <linux/cpumask.h>
+#include <linux/export.h>
+#include <linux/types.h>
 #include <linux/qcom_scm.h>
 
-#include <asm/outercache.h>
-#include <asm/cacheflush.h>
-
-
-#define QCOM_SCM_ENOMEM		-5
-#define QCOM_SCM_EOPNOTSUPP	-4
-#define QCOM_SCM_EINVAL_ADDR	-3
-#define QCOM_SCM_EINVAL_ARG	-2
-#define QCOM_SCM_ERROR		-1
-#define QCOM_SCM_INTERRUPTED	1
-
-#define QCOM_SCM_FLAG_COLDBOOT_CPU0	0x00
-#define QCOM_SCM_FLAG_COLDBOOT_CPU1	0x01
-#define QCOM_SCM_FLAG_COLDBOOT_CPU2	0x08
-#define QCOM_SCM_FLAG_COLDBOOT_CPU3	0x20
-
-#define QCOM_SCM_FLAG_WARMBOOT_CPU0	0x04
-#define QCOM_SCM_FLAG_WARMBOOT_CPU1	0x02
-#define QCOM_SCM_FLAG_WARMBOOT_CPU2	0x10
-#define QCOM_SCM_FLAG_WARMBOOT_CPU3	0x40
-
-struct qcom_scm_entry {
-	int flag;
-	void *entry;
-};
-
-static struct qcom_scm_entry qcom_scm_wb[] = {
-	{ .flag = QCOM_SCM_FLAG_WARMBOOT_CPU0 },
-	{ .flag = QCOM_SCM_FLAG_WARMBOOT_CPU1 },
-	{ .flag = QCOM_SCM_FLAG_WARMBOOT_CPU2 },
-	{ .flag = QCOM_SCM_FLAG_WARMBOOT_CPU3 },
-};
-
-static DEFINE_MUTEX(qcom_scm_lock);
-
-/**
- * struct qcom_scm_command - one SCM command buffer
- * @len: total available memory for command and response
- * @buf_offset: start of command buffer
- * @resp_hdr_offset: start of response buffer
- * @id: command to be executed
- * @buf: buffer returned from qcom_scm_get_command_buffer()
- *
- * An SCM command is laid out in memory as follows:
- *
- *	------------------- <--- struct qcom_scm_command
- *	| command header  |
- *	------------------- <--- qcom_scm_get_command_buffer()
- *	| command buffer  |
- *	------------------- <--- struct qcom_scm_response and
- *	| response header |      qcom_scm_command_to_response()
- *	------------------- <--- qcom_scm_get_response_buffer()
- *	| response buffer |
- *	-------------------
- *
- * There can be arbitrary padding between the headers and buffers so
- * you should always use the appropriate qcom_scm_get_*_buffer() routines
- * to access the buffers in a safe manner.
- */
-struct qcom_scm_command {
-	__le32 len;
-	__le32 buf_offset;
-	__le32 resp_hdr_offset;
-	__le32 id;
-	__le32 buf[0];
-};
-
-/**
- * struct qcom_scm_response - one SCM response buffer
- * @len: total available memory for response
- * @buf_offset: start of response data relative to start of qcom_scm_response
- * @is_complete: indicates if the command has finished processing
- */
-struct qcom_scm_response {
-	__le32 len;
-	__le32 buf_offset;
-	__le32 is_complete;
-};
-
-/**
- * alloc_qcom_scm_command() - Allocate an SCM command
- * @cmd_size: size of the command buffer
- * @resp_size: size of the response buffer
- *
- * Allocate an SCM command, including enough room for the command
- * and response headers as well as the command and response buffers.
- *
- * Returns a valid &qcom_scm_command on success or %NULL if the allocation fails.
- */
-static struct qcom_scm_command *alloc_qcom_scm_command(size_t cmd_size, size_t resp_size)
-{
-	struct qcom_scm_command *cmd;
-	size_t len = sizeof(*cmd) + sizeof(struct qcom_scm_response) + cmd_size +
-		resp_size;
-	u32 offset;
-
-	cmd = kzalloc(PAGE_ALIGN(len), GFP_KERNEL);
-	if (cmd) {
-		cmd->len = cpu_to_le32(len);
-		offset = offsetof(struct qcom_scm_command, buf);
-		cmd->buf_offset = cpu_to_le32(offset);
-		cmd->resp_hdr_offset = cpu_to_le32(offset + cmd_size);
-	}
-	return cmd;
-}
-
-/**
- * free_qcom_scm_command() - Free an SCM command
- * @cmd: command to free
- *
- * Free an SCM command.
- */
-static inline void free_qcom_scm_command(struct qcom_scm_command *cmd)
-{
-	kfree(cmd);
-}
-
-/**
- * qcom_scm_command_to_response() - Get a pointer to a qcom_scm_response
- * @cmd: command
- *
- * Returns a pointer to a response for a command.
- */
-static inline struct qcom_scm_response *qcom_scm_command_to_response(
-		const struct qcom_scm_command *cmd)
-{
-	return (void *)cmd + le32_to_cpu(cmd->resp_hdr_offset);
-}
-
-/**
- * qcom_scm_get_command_buffer() - Get a pointer to a command buffer
- * @cmd: command
- *
- * Returns a pointer to the command buffer of a command.
- */
-static inline void *qcom_scm_get_command_buffer(const struct qcom_scm_command *cmd)
-{
-	return (void *)cmd->buf;
-}
-
-/**
- * qcom_scm_get_response_buffer() - Get a pointer to a response buffer
- * @rsp: response
- *
- * Returns a pointer to a response buffer of a response.
- */
-static inline void *qcom_scm_get_response_buffer(const struct qcom_scm_response *rsp)
-{
-	return (void *)rsp + le32_to_cpu(rsp->buf_offset);
-}
-
-static int qcom_scm_remap_error(int err)
-{
-	pr_err("qcom_scm_call failed with error code %d\n", err);
-	switch (err) {
-	case QCOM_SCM_ERROR:
-		return -EIO;
-	case QCOM_SCM_EINVAL_ADDR:
-	case QCOM_SCM_EINVAL_ARG:
-		return -EINVAL;
-	case QCOM_SCM_EOPNOTSUPP:
-		return -EOPNOTSUPP;
-	case QCOM_SCM_ENOMEM:
-		return -ENOMEM;
-	}
-	return -EINVAL;
-}
-
-static u32 smc(u32 cmd_addr)
-{
-	int context_id;
-	register u32 r0 asm("r0") = 1;
-	register u32 r1 asm("r1") = (u32)&context_id;
-	register u32 r2 asm("r2") = cmd_addr;
-	do {
-		asm volatile(
-			__asmeq("%0", "r0")
-			__asmeq("%1", "r0")
-			__asmeq("%2", "r1")
-			__asmeq("%3", "r2")
-#ifdef REQUIRES_SEC
-			".arch_extension sec\n"
-#endif
-			"smc	#0	@ switch to secure world\n"
-			: "=r" (r0)
-			: "r" (r0), "r" (r1), "r" (r2)
-			: "r3");
-	} while (r0 == QCOM_SCM_INTERRUPTED);
-
-	return r0;
-}
-
-static int __qcom_scm_call(const struct qcom_scm_command *cmd)
-{
-	int ret;
-	u32 cmd_addr = virt_to_phys(cmd);
-
-	/*
-	 * Flush the command buffer so that the secure world sees
-	 * the correct data.
-	 */
-	__cpuc_flush_dcache_area((void *)cmd, cmd->len);
-	outer_flush_range(cmd_addr, cmd_addr + cmd->len);
-
-	ret = smc(cmd_addr);
-	if (ret < 0)
-		ret = qcom_scm_remap_error(ret);
-
-	return ret;
-}
-
-static void qcom_scm_inv_range(unsigned long start, unsigned long end)
-{
-	u32 cacheline_size, ctr;
-
-	asm volatile("mrc p15, 0, %0, c0, c0, 1" : "=r" (ctr));
-	cacheline_size = 4 << ((ctr >> 16) & 0xf);
-
-	start = round_down(start, cacheline_size);
-	end = round_up(end, cacheline_size);
-	outer_inv_range(start, end);
-	while (start < end) {
-		asm ("mcr p15, 0, %0, c7, c6, 1" : : "r" (start)
-		     : "memory");
-		start += cacheline_size;
-	}
-	dsb();
-	isb();
-}
-
-/**
- * qcom_scm_call() - Send an SCM command
- * @svc_id: service identifier
- * @cmd_id: command identifier
- * @cmd_buf: command buffer
- * @cmd_len: length of the command buffer
- * @resp_buf: response buffer
- * @resp_len: length of the response buffer
- *
- * Sends a command to the SCM and waits for the command to finish processing.
- *
- * A note on cache maintenance:
- * Note that any buffers that are expected to be accessed by the secure world
- * must be flushed before invoking qcom_scm_call and invalidated in the cache
- * immediately after qcom_scm_call returns. Cache maintenance on the command
- * and response buffers is taken care of by qcom_scm_call; however, callers are
- * responsible for any other cached buffers passed over to the secure world.
- */
-static int qcom_scm_call(u32 svc_id, u32 cmd_id, const void *cmd_buf,
-			size_t cmd_len, void *resp_buf, size_t resp_len)
-{
-	int ret;
-	struct qcom_scm_command *cmd;
-	struct qcom_scm_response *rsp;
-	unsigned long start, end;
-
-	cmd = alloc_qcom_scm_command(cmd_len, resp_len);
-	if (!cmd)
-		return -ENOMEM;
-
-	cmd->id = cpu_to_le32((svc_id << 10) | cmd_id);
-	if (cmd_buf)
-		memcpy(qcom_scm_get_command_buffer(cmd), cmd_buf, cmd_len);
-
-	mutex_lock(&qcom_scm_lock);
-	ret = __qcom_scm_call(cmd);
-	mutex_unlock(&qcom_scm_lock);
-	if (ret)
-		goto out;
-
-	rsp = qcom_scm_command_to_response(cmd);
-	start = (unsigned long)rsp;
-
-	do {
-		qcom_scm_inv_range(start, start + sizeof(*rsp));
-	} while (!rsp->is_complete);
-
-	end = (unsigned long)qcom_scm_get_response_buffer(rsp) + resp_len;
-	qcom_scm_inv_range(start, end);
-
-	if (resp_buf)
-		memcpy(resp_buf, qcom_scm_get_response_buffer(rsp), resp_len);
-out:
-	free_qcom_scm_command(cmd);
-	return ret;
-}
-
-#define SCM_CLASS_REGISTER	(0x2 << 8)
-#define SCM_MASK_IRQS		BIT(5)
-#define SCM_ATOMIC(svc, cmd, n) (((((svc) << 10)|((cmd) & 0x3ff)) << 12) | \
-				SCM_CLASS_REGISTER | \
-				SCM_MASK_IRQS | \
-				(n & 0xf))
-
-/**
- * qcom_scm_call_atomic1() - Send an atomic SCM command with one argument
- * @svc_id: service identifier
- * @cmd_id: command identifier
- * @arg1: first argument
- *
- * This shall only be used with commands that are guaranteed to be
- * uninterruptable, atomic and SMP safe.
- */
-static s32 qcom_scm_call_atomic1(u32 svc, u32 cmd, u32 arg1)
-{
-	int context_id;
-
-	register u32 r0 asm("r0") = SCM_ATOMIC(svc, cmd, 1);
-	register u32 r1 asm("r1") = (u32)&context_id;
-	register u32 r2 asm("r2") = arg1;
-
-	asm volatile(
-			__asmeq("%0", "r0")
-			__asmeq("%1", "r0")
-			__asmeq("%2", "r1")
-			__asmeq("%3", "r2")
-#ifdef REQUIRES_SEC
-			".arch_extension sec\n"
-#endif
-			"smc    #0      @ switch to secure world\n"
-			: "=r" (r0)
-			: "r" (r0), "r" (r1), "r" (r2)
-			: "r3");
-	return r0;
-}
-
-u32 qcom_scm_get_version(void)
-{
-	int context_id;
-	static u32 version = -1;
-	register u32 r0 asm("r0");
-	register u32 r1 asm("r1");
-
-	if (version != -1)
-		return version;
-
-	mutex_lock(&qcom_scm_lock);
-
-	r0 = 0x1 << 8;
-	r1 = (u32)&context_id;
-	do {
-		asm volatile(
-			__asmeq("%0", "r0")
-			__asmeq("%1", "r1")
-			__asmeq("%2", "r0")
-			__asmeq("%3", "r1")
-#ifdef REQUIRES_SEC
-			".arch_extension sec\n"
-#endif
-			"smc	#0	@ switch to secure world\n"
-			: "=r" (r0), "=r" (r1)
-			: "r" (r0), "r" (r1)
-			: "r2", "r3");
-	} while (r0 == QCOM_SCM_INTERRUPTED);
-
-	version = r1;
-	mutex_unlock(&qcom_scm_lock);
-
-	return version;
-}
-EXPORT_SYMBOL(qcom_scm_get_version);
-
-#define QCOM_SCM_SVC_BOOT			0x1
-#define QCOM_SCM_BOOT_ADDR			0x1
-/*
- * Set the cold/warm boot address for one of the CPU cores.
- */
-static int qcom_scm_set_boot_addr(u32 addr, int flags)
-{
-	struct {
-		__le32 flags;
-		__le32 addr;
-	} cmd;
-
-	cmd.addr = cpu_to_le32(addr);
-	cmd.flags = cpu_to_le32(flags);
-	return qcom_scm_call(QCOM_SCM_SVC_BOOT, QCOM_SCM_BOOT_ADDR,
-			&cmd, sizeof(cmd), NULL, 0);
-}
+#include "qcom_scm.h"
 
 /**
  * qcom_scm_set_cold_boot_addr() - Set the cold boot address for cpus
@@ -414,26 +33,7 @@ static int qcom_scm_set_boot_addr(u32 addr, int flags)
  */
 int qcom_scm_set_cold_boot_addr(void *entry, const cpumask_t *cpus)
 {
-	int flags = 0;
-	int cpu;
-	int scm_cb_flags[] = {
-		QCOM_SCM_FLAG_COLDBOOT_CPU0,
-		QCOM_SCM_FLAG_COLDBOOT_CPU1,
-		QCOM_SCM_FLAG_COLDBOOT_CPU2,
-		QCOM_SCM_FLAG_COLDBOOT_CPU3,
-	};
-
-	if (!cpus || (cpus && cpumask_empty(cpus)))
-		return -EINVAL;
-
-	for_each_cpu(cpu, cpus) {
-		if (cpu < ARRAY_SIZE(scm_cb_flags))
-			flags |= scm_cb_flags[cpu];
-		else
-			set_cpu_present(cpu, false);
-	}
-
-	return qcom_scm_set_boot_addr(virt_to_phys(entry), flags);
+	return __qcom_scm_set_cold_boot_addr(entry, cpus);
 }
 EXPORT_SYMBOL(qcom_scm_set_cold_boot_addr);
 
@@ -447,37 +47,10 @@ EXPORT_SYMBOL(qcom_scm_set_cold_boot_addr);
  */
 int qcom_scm_set_warm_boot_addr(void *entry, const cpumask_t *cpus)
 {
-	int ret;
-	int flags = 0;
-	int cpu;
-
-	/*
-	 * Reassign only if we are switching from hotplug entry point
-	 * to cpuidle entry point or vice versa.
-	 */
-	for_each_cpu(cpu, cpus) {
-		if (entry == qcom_scm_wb[cpu].entry)
-			continue;
-		flags |= qcom_scm_wb[cpu].flag;
-	}
-
-	/* No change in entry function */
-	if (!flags)
-		return 0;
-
-	ret = qcom_scm_set_boot_addr(virt_to_phys(entry), flags);
-	if (!ret) {
-		for_each_cpu(cpu, cpus)
-			qcom_scm_wb[cpu].entry = entry;
-	}
-
-	return ret;
+	return __qcom_scm_set_warm_boot_addr(entry, cpus);
 }
 EXPORT_SYMBOL(qcom_scm_set_warm_boot_addr);
 
-#define QCOM_SCM_CMD_TERMINATE_PC	0x2
-#define QCOM_SCM_FLUSH_FLAG_MASK	0x3
-
 /**
  * qcom_scm_cpu_power_down() - Power down the cpu
  * @flags - Flags to flush cache
@@ -488,7 +61,6 @@ EXPORT_SYMBOL(qcom_scm_set_warm_boot_addr);
  */
 void qcom_scm_cpu_power_down(u32 flags)
 {
-	qcom_scm_call_atomic1(QCOM_SCM_SVC_BOOT, QCOM_SCM_CMD_TERMINATE_PC,
-			flags & QCOM_SCM_FLUSH_FLAG_MASK);
+	__qcom_scm_cpu_power_down(flags);
 }
 EXPORT_SYMBOL(qcom_scm_cpu_power_down);
diff --git a/drivers/firmware/qcom_scm.h b/drivers/firmware/qcom_scm.h
new file mode 100644
index 0000000..9fbede4
--- /dev/null
+++ b/drivers/firmware/qcom_scm.h
@@ -0,0 +1,29 @@
+/* Copyright (c) 2010-2014, The Linux Foundation. All rights reserved.
+ *
+ * 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.
+ */
+#ifndef __QCOM_SCM_INT_H
+#define __QCOM_SCM_INT_H
+
+#define QCOM_SCM_SVC_BOOT		0x1
+#define QCOM_SCM_BOOT_ADDR		0x1
+#define QCOM_SCM_BOOT_ADDR_MC		0x11
+
+#define QCOM_SCM_FLAG_HLOS		0x01
+#define QCOM_SCM_FLAG_COLDBOOT_MC	0x02
+#define QCOM_SCM_FLAG_WARMBOOT_MC	0x04
+extern int __qcom_scm_set_warm_boot_addr(void *entry, const cpumask_t *cpus);
+extern int __qcom_scm_set_cold_boot_addr(void *entry, const cpumask_t *cpus);
+
+#define QCOM_SCM_CMD_TERMINATE_PC	0x2
+#define QCOM_SCM_FLUSH_FLAG_MASK	0x3
+#define QCOM_SCM_CMD_CORE_HOTPLUGGED	0x10
+extern void __qcom_scm_cpu_power_down(u32 flags);
+#endif
-- 
Qualcomm Innovation Center, Inc.
The Qualcomm Innovation Center, Inc. is a member of the Code Aurora Forum,
a Linux Foundation Collaborative Project

[RFC PATCH 2/5] firmware: qcom: scm: Add support for ARM64 SoCs

[Kumar Gala]

Add an implementation of the SCM interface that works on ARM64/64-bit SoCs

Signed-off-by: Kumar Gala <galak@codeaurora.org>
---
 arch/arm64/Kconfig             |   1 +
 drivers/firmware/Makefile      |   4 +
 drivers/firmware/qcom_scm-64.c | 466 +++++++++++++++++++++++++++++++++++++++++
 3 files changed, 471 insertions(+)
 create mode 100644 drivers/firmware/qcom_scm-64.c

diff --git a/arch/arm64/Kconfig b/arch/arm64/Kconfig
index 610965dd..11e97d8 100644
--- a/arch/arm64/Kconfig
+++ b/arch/arm64/Kconfig
@@ -180,6 +180,7 @@ config ARCH_MEDIATEK
 config ARCH_QCOM
 	bool "Qualcomm Platforms"
 	select PINCTRL
+	select QCOM_SCM
 	help
 	  This enables support for the ARMv8 based Qualcomm chipsets.
 
diff --git a/drivers/firmware/Makefile b/drivers/firmware/Makefile
index 3001f1a..c79751a 100644
--- a/drivers/firmware/Makefile
+++ b/drivers/firmware/Makefile
@@ -12,7 +12,11 @@ obj-$(CONFIG_ISCSI_IBFT_FIND)	+= iscsi_ibft_find.o
 obj-$(CONFIG_ISCSI_IBFT)	+= iscsi_ibft.o
 obj-$(CONFIG_FIRMWARE_MEMMAP)	+= memmap.o
 obj-$(CONFIG_QCOM_SCM)		+= qcom_scm.o
+ifdef CONFIG_64BIT
+obj-$(CONFIG_QCOM_SCM)		+= qcom_scm-64.o
+else
 obj-$(CONFIG_QCOM_SCM)		+= qcom_scm-32.o
+endif
 CFLAGS_qcom_scm-32.o :=$(call as-instr,.arch_extension sec,-DREQUIRES_SEC=1)
 
 obj-$(CONFIG_GOOGLE_FIRMWARE)	+= google/
diff --git a/drivers/firmware/qcom_scm-64.c b/drivers/firmware/qcom_scm-64.c
new file mode 100644
index 0000000..72b75d6
--- /dev/null
+++ b/drivers/firmware/qcom_scm-64.c
@@ -0,0 +1,466 @@
+/* Copyright (c) 2014-2015, The Linux Foundation. All rights reserved.
+ *
+ * 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.
+ *
+ * You should have received a copy of the GNU General Public License
+ * along with this program; if not, write to the Free Software
+ * Foundation, Inc., 51 Franklin Street, Fifth Floor, Boston, MA
+ * 02110-1301, USA.
+ */
+
+#include <linux/cpumask.h>
+#include <linux/delay.h>
+#include <linux/mutex.h>
+#include <linux/slab.h>
+#include <linux/types.h>
+#include <linux/qcom_scm.h>
+
+#include <asm/cacheflush.h>
+#include <asm/compiler.h>
+#include <asm/smp_plat.h>
+
+#include "qcom_scm.h"
+
+#define QCOM_SCM_SIP_FNID(s, c) (((((s) & 0xFF) << 8) | ((c) & 0xFF)) | 0x02000000)
+
+#define MAX_QCOM_SCM_ARGS 10
+#define MAX_QCOM_SCM_RETS 3
+
+#define QCOM_SCM_ARGS_IMPL(num, a, b, c, d, e, f, g, h, i, j, ...) (\
+			(((a) & 0xff) << 4) | \
+			(((b) & 0xff) << 6) | \
+			(((c) & 0xff) << 8) | \
+			(((d) & 0xff) << 10) | \
+			(((e) & 0xff) << 12) | \
+			(((f) & 0xff) << 14) | \
+			(((g) & 0xff) << 16) | \
+			(((h) & 0xff) << 18) | \
+			(((i) & 0xff) << 20) | \
+			(((j) & 0xff) << 22) | \
+			(num & 0xffff))
+
+#define QCOM_SCM_ARGS(...) QCOM_SCM_ARGS_IMPL(__VA_ARGS__, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0)
+
+/**
+ * struct qcom_scm_desc
+ * @arginfo: Metadata describing the arguments in args[]
+ * @args: The array of arguments for the secure syscall
+ * @ret: The values returned by the secure syscall
+ * @extra_arg_buf: The buffer containing extra arguments
+		   (that don't fit in available registers)
+ * @x5: The 4rd argument to the secure syscall or physical address of
+	extra_arg_buf
+ */
+struct qcom_scm_desc {
+	u32 arginfo;
+	u64 args[MAX_QCOM_SCM_ARGS];
+	u64 ret[MAX_QCOM_SCM_RETS];
+
+	/* private */
+	void *extra_arg_buf;
+	u64 x5;
+};
+
+
+#define QCOM_SCM_ENOMEM		-5
+#define QCOM_SCM_EOPNOTSUPP	-4
+#define QCOM_SCM_EINVAL_ADDR	-3
+#define QCOM_SCM_EINVAL_ARG	-2
+#define QCOM_SCM_ERROR		-1
+#define QCOM_SCM_INTERRUPTED	1
+#define QCOM_SCM_EBUSY		-55
+#define QCOM_SCM_V2_EBUSY	-12
+
+static DEFINE_MUTEX(qcom_scm_lock);
+
+#define QCOM_SCM_EBUSY_WAIT_MS 30
+#define QCOM_SCM_EBUSY_MAX_RETRY 20
+
+#define N_EXT_QCOM_SCM_ARGS 7
+#define FIRST_EXT_ARG_IDX 3
+#define SMC_ATOMIC_SYSCALL 31
+#define N_REGISTER_ARGS (MAX_QCOM_SCM_ARGS - N_EXT_QCOM_SCM_ARGS + 1)
+#define SMC64_MASK 0x40000000
+#define SMC_ATOMIC_MASK 0x80000000
+#define IS_CALL_AVAIL_CMD 1
+
+#define R0_STR "x0"
+#define R1_STR "x1"
+#define R2_STR "x2"
+#define R3_STR "x3"
+#define R4_STR "x4"
+#define R5_STR "x5"
+
+static int qcom_scm_remap_error(int err)
+{
+	switch (err) {
+	case QCOM_SCM_ERROR:
+		return -EIO;
+	case QCOM_SCM_EINVAL_ADDR:
+	case QCOM_SCM_EINVAL_ARG:
+		return -EINVAL;
+	case QCOM_SCM_EOPNOTSUPP:
+		return -EOPNOTSUPP;
+	case QCOM_SCM_ENOMEM:
+		return -ENOMEM;
+	case QCOM_SCM_EBUSY:
+		return QCOM_SCM_EBUSY;
+	case QCOM_SCM_V2_EBUSY:
+		return QCOM_SCM_V2_EBUSY;
+	}
+	return -EINVAL;
+}
+
+int __qcom_scm_call_armv8_64(u64 x0, u64 x1, u64 x2, u64 x3, u64 x4, u64 x5,
+				u64 *ret1, u64 *ret2, u64 *ret3)
+{
+	register u64 r0 asm("r0") = x0;
+	register u64 r1 asm("r1") = x1;
+	register u64 r2 asm("r2") = x2;
+	register u64 r3 asm("r3") = x3;
+	register u64 r4 asm("r4") = x4;
+	register u64 r5 asm("r5") = x5;
+
+	do {
+		asm volatile(
+			__asmeq("%0", R0_STR)
+			__asmeq("%1", R1_STR)
+			__asmeq("%2", R2_STR)
+			__asmeq("%3", R3_STR)
+			__asmeq("%4", R0_STR)
+			__asmeq("%5", R1_STR)
+			__asmeq("%6", R2_STR)
+			__asmeq("%7", R3_STR)
+			__asmeq("%8", R4_STR)
+			__asmeq("%9", R5_STR)
+#ifdef REQUIRES_SEC
+			".arch_extension sec\n"
+#endif
+			"smc	#0\n"
+			: "=r" (r0), "=r" (r1), "=r" (r2), "=r" (r3)
+			: "r" (r0), "r" (r1), "r" (r2), "r" (r3), "r" (r4),
+			  "r" (r5)
+			: "x6", "x7", "x8", "x9", "x10", "x11", "x12", "x13",
+			  "x14", "x15", "x16", "x17");
+	} while (r0 == QCOM_SCM_INTERRUPTED);
+
+	if (ret1)
+		*ret1 = r1;
+	if (ret2)
+		*ret2 = r2;
+	if (ret3)
+		*ret3 = r3;
+
+	return r0;
+}
+
+int __qcom_scm_call_armv8_32(u32 w0, u32 w1, u32 w2, u32 w3, u32 w4, u32 w5,
+				u64 *ret1, u64 *ret2, u64 *ret3)
+{
+	register u32 r0 asm("r0") = w0;
+	register u32 r1 asm("r1") = w1;
+	register u32 r2 asm("r2") = w2;
+	register u32 r3 asm("r3") = w3;
+	register u32 r4 asm("r4") = w4;
+	register u32 r5 asm("r5") = w5;
+
+	do {
+		asm volatile(
+			__asmeq("%0", R0_STR)
+			__asmeq("%1", R1_STR)
+			__asmeq("%2", R2_STR)
+			__asmeq("%3", R3_STR)
+			__asmeq("%4", R0_STR)
+			__asmeq("%5", R1_STR)
+			__asmeq("%6", R2_STR)
+			__asmeq("%7", R3_STR)
+			__asmeq("%8", R4_STR)
+			__asmeq("%9", R5_STR)
+#ifdef REQUIRES_SEC
+			".arch_extension sec\n"
+#endif
+			"smc	#0\n"
+			: "=r" (r0), "=r" (r1), "=r" (r2), "=r" (r3)
+			: "r" (r0), "r" (r1), "r" (r2), "r" (r3), "r" (r4),
+			  "r" (r5)
+			: "x6", "x7", "x8", "x9", "x10", "x11", "x12", "x13",
+			"x14", "x15", "x16", "x17");
+
+	} while (r0 == QCOM_SCM_INTERRUPTED);
+
+	if (ret1)
+		*ret1 = r1;
+	if (ret2)
+		*ret2 = r2;
+	if (ret3)
+		*ret3 = r3;
+
+	return r0;
+}
+
+struct qcom_scm_extra_arg {
+	union {
+		u32 args32[N_EXT_QCOM_SCM_ARGS];
+		u64 args64[N_EXT_QCOM_SCM_ARGS];
+	};
+};
+
+static enum qcom_scm_interface_version {
+	QCOM_SCM_UNKNOWN,
+	QCOM_SCM_LEGACY,
+	QCOM_SCM_ARMV8_32,
+	QCOM_SCM_ARMV8_64,
+} qcom_scm_version = QCOM_SCM_UNKNOWN;
+
+/* This will be set to specify SMC32 or SMC64 */
+static u32 qcom_scm_version_mask;
+
+/*
+ * If there are more than N_REGISTER_ARGS, allocate a buffer and place
+ * the additional arguments in it. The extra argument buffer will be
+ * pointed to by X5.
+ */
+static int allocate_extra_arg_buffer(struct qcom_scm_desc *desc, gfp_t flags)
+{
+	int i, j;
+	struct qcom_scm_extra_arg *argbuf;
+	int arglen = desc->arginfo & 0xf;
+	size_t argbuflen = PAGE_ALIGN(sizeof(struct qcom_scm_extra_arg));
+
+	desc->x5 = desc->args[FIRST_EXT_ARG_IDX];
+
+	if (likely(arglen <= N_REGISTER_ARGS)) {
+		desc->extra_arg_buf = NULL;
+		return 0;
+	}
+
+	argbuf = kzalloc(argbuflen, flags);
+	if (!argbuf) {
+		pr_err("qcom_scm_call: failed to alloc mem for extended argument buffer\n");
+		return -ENOMEM;
+	}
+
+	desc->extra_arg_buf = argbuf;
+
+	j = FIRST_EXT_ARG_IDX;
+	if (qcom_scm_version == QCOM_SCM_ARMV8_64)
+		for (i = 0; i < N_EXT_QCOM_SCM_ARGS; i++)
+			argbuf->args64[i] = desc->args[j++];
+	else
+		for (i = 0; i < N_EXT_QCOM_SCM_ARGS; i++)
+			argbuf->args32[i] = desc->args[j++];
+	desc->x5 = virt_to_phys(argbuf);
+	__flush_dcache_area(argbuf, argbuflen);
+
+	return 0;
+}
+
+/**
+ * qcom_scm_call() - Invoke a syscall in the secure world
+ * @svc_id: service identifier
+ * @cmd_id: command identifier
+ * @fn_id: The function ID for this syscall
+ * @desc: Descriptor structure containing arguments and return values
+ *
+ * Sends a command to the SCM and waits for the command to finish processing.
+ * This should *only* be called in pre-emptible context.
+ *
+ * A note on cache maintenance:
+ * Note that any buffers that are expected to be accessed by the secure world
+ * must be flushed before invoking qcom_scm_call and invalidated in the cache
+ * immediately after qcom_scm_call returns. An important point that must be noted
+ * is that on ARMV8 architectures, invalidation actually also causes a dirty
+ * cache line to be cleaned (flushed + unset-dirty-bit). Therefore it is of
+ * paramount importance that the buffer be flushed before invoking qcom_scm_call,
+ * even if you don't care about the contents of that buffer.
+ *
+ * Note that cache maintenance on the argument buffer (desc->args) is taken care
+ * of by qcom_scm_call; however, callers are responsible for any other cached
+ * buffers passed over to the secure world.
+*/
+static int qcom_scm_call(u32 svc_id, u32 cmd_id, struct qcom_scm_desc *desc)
+{
+	int arglen = desc->arginfo & 0xf;
+	int ret, retry_count = 0;
+	u32 fn_id = QCOM_SCM_SIP_FNID(svc_id, cmd_id);
+	u64 x0;
+
+	ret = allocate_extra_arg_buffer(desc, GFP_KERNEL);
+	if (ret)
+		return ret;
+
+	x0 = fn_id | qcom_scm_version_mask;
+
+	do {
+		mutex_lock(&qcom_scm_lock);
+
+		desc->ret[0] = desc->ret[1] = desc->ret[2] = 0;
+
+		pr_debug("qcom_scm_call: func id %#llx, args: %#x, %#llx, %#llx, %#llx, %#llx\n",
+			x0, desc->arginfo, desc->args[0], desc->args[1],
+			desc->args[2], desc->x5);
+
+		if (qcom_scm_version == QCOM_SCM_ARMV8_64)
+			ret = __qcom_scm_call_armv8_64(x0, desc->arginfo,
+						  desc->args[0], desc->args[1],
+						  desc->args[2], desc->x5,
+						  &desc->ret[0], &desc->ret[1],
+						  &desc->ret[2]);
+		else
+			ret = __qcom_scm_call_armv8_32(x0, desc->arginfo,
+						  desc->args[0], desc->args[1],
+						  desc->args[2], desc->x5,
+						  &desc->ret[0], &desc->ret[1],
+						  &desc->ret[2]);
+		mutex_unlock(&qcom_scm_lock);
+
+		if (ret == QCOM_SCM_V2_EBUSY)
+			msleep(QCOM_SCM_EBUSY_WAIT_MS);
+	}  while (ret == QCOM_SCM_V2_EBUSY && (retry_count++ < QCOM_SCM_EBUSY_MAX_RETRY));
+
+	if (ret < 0)
+		pr_err("qcom_scm_call failed: func id %#llx, arginfo: %#x, args: %#llx, %#llx, %#llx, %#llx, ret: %d, syscall returns: %#llx, %#llx, %#llx\n",
+			x0, desc->arginfo, desc->args[0], desc->args[1],
+			desc->args[2], desc->x5, ret, desc->ret[0],
+			desc->ret[1], desc->ret[2]);
+
+	if (arglen > N_REGISTER_ARGS)
+		kfree(desc->extra_arg_buf);
+	if (ret < 0)
+		return qcom_scm_remap_error(ret);
+	return 0;
+}
+
+/**
+ * qcom_scm_call_atomic() - Invoke a syscall in the secure world
+ *
+ * Similar to qcom_scm_call except that this can be invoked in atomic context.
+ * There is also no retry mechanism implemented. Please ensure that the
+ * secure world syscall can be executed in such a context and can complete
+ * in a timely manner.
+ */
+static int qcom_scm_call_atomic(u32 s, u32 c, struct qcom_scm_desc *desc)
+{
+	int arglen = desc->arginfo & 0xf;
+	int ret;
+	u32 fn_id = QCOM_SCM_SIP_FNID(s, c);
+	u64 x0;
+
+	ret = allocate_extra_arg_buffer(desc, GFP_ATOMIC);
+	if (ret)
+		return ret;
+
+	x0 = fn_id | BIT(SMC_ATOMIC_SYSCALL) | qcom_scm_version_mask;
+
+	pr_debug("qcom_scm_call: func id %#llx, args: %#x, %#llx, %#llx, %#llx, %#llx\n",
+		x0, desc->arginfo, desc->args[0], desc->args[1],
+		desc->args[2], desc->x5);
+
+	if (qcom_scm_version == QCOM_SCM_ARMV8_64)
+		ret = __qcom_scm_call_armv8_64(x0, desc->arginfo, desc->args[0],
+					  desc->args[1], desc->args[2],
+					  desc->x5, &desc->ret[0],
+					  &desc->ret[1], &desc->ret[2]);
+	else
+		ret = __qcom_scm_call_armv8_32(x0, desc->arginfo, desc->args[0],
+					  desc->args[1], desc->args[2],
+					  desc->x5, &desc->ret[0],
+					  &desc->ret[1], &desc->ret[2]);
+	if (ret < 0)
+		pr_err("qcom_scm_call failed: func id %#llx, arginfo: %#x, args: %#llx, %#llx, %#llx, %#llx, ret: %d, syscall returns: %#llx, %#llx, %#llx\n",
+			x0, desc->arginfo, desc->args[0], desc->args[1],
+			desc->args[2], desc->x5, ret, desc->ret[0],
+			desc->ret[1], desc->ret[2]);
+
+	if (arglen > N_REGISTER_ARGS)
+		kfree(desc->extra_arg_buf);
+	if (ret < 0)
+		return qcom_scm_remap_error(ret);
+	return ret;
+}
+
+static int qcom_scm_set_boot_addr(void *entry, const cpumask_t *cpus, int flags)
+{
+	struct qcom_scm_desc desc = {0};
+	unsigned int cpu = cpumask_first(cpus);
+	u64 mpidr_el1 = cpu_logical_map(cpu);
+
+	/* For now we assume only a single cpu is set in the mask */
+	WARN_ON(cpumask_weight(cpus) != 1);
+
+	if (mpidr_el1 & ~MPIDR_HWID_BITMASK) {
+		pr_err("CPU%d:Failed to set boot address\n", cpu);
+		return -ENOSYS;
+	}
+
+	desc.args[0] = virt_to_phys(entry);
+	desc.args[1] = BIT(MPIDR_AFFINITY_LEVEL(mpidr_el1, 0));
+	desc.args[2] = BIT(MPIDR_AFFINITY_LEVEL(mpidr_el1, 1));
+	desc.args[3] = BIT(MPIDR_AFFINITY_LEVEL(mpidr_el1, 2));
+	desc.args[4] = ~0ULL;
+	desc.args[5] = QCOM_SCM_FLAG_HLOS | flags;
+	desc.arginfo = QCOM_SCM_ARGS(6);
+
+	return qcom_scm_call(QCOM_SCM_SVC_BOOT, QCOM_SCM_BOOT_ADDR_MC, &desc);
+}
+
+int __qcom_scm_set_cold_boot_addr(void *entry, const cpumask_t *cpus)
+{
+	int flags = QCOM_SCM_FLAG_COLDBOOT_MC;
+
+	return qcom_scm_set_boot_addr(entry, cpus, flags);
+}
+
+int __qcom_scm_set_warm_boot_addr(void *entry, const cpumask_t *cpus)
+{
+	int flags = QCOM_SCM_FLAG_WARMBOOT_MC;
+
+	return qcom_scm_set_boot_addr(entry, cpus, flags);
+}
+
+void __qcom_scm_cpu_power_down(u32 flags)
+{
+	struct qcom_scm_desc desc = {0};
+	desc.args[0] = QCOM_SCM_CMD_CORE_HOTPLUGGED |
+		       (flags & QCOM_SCM_FLUSH_FLAG_MASK);
+	desc.arginfo = QCOM_SCM_ARGS(1);
+
+	qcom_scm_call_atomic(QCOM_SCM_SVC_BOOT, QCOM_SCM_CMD_TERMINATE_PC, &desc);
+}
+
+#define QCOM_SCM_SVC_INFO              0x6
+static int __init qcom_scm_init(void)
+{
+	int ret;
+	u64 ret1 = 0, x0;
+
+	/* First try a SMC64 call */
+	qcom_scm_version = QCOM_SCM_ARMV8_64;
+	x0 = QCOM_SCM_SIP_FNID(QCOM_SCM_SVC_INFO, IS_CALL_AVAIL_CMD) | SMC_ATOMIC_MASK;
+	ret = __qcom_scm_call_armv8_64(x0 | SMC64_MASK, QCOM_SCM_ARGS(1), x0, 0, 0, 0,
+				  &ret1, NULL, NULL);
+	if (ret || !ret1) {
+		/* Try SMC32 call */
+		ret1 = 0;
+		ret = __qcom_scm_call_armv8_32(x0, QCOM_SCM_ARGS(1), x0, 0, 0,
+						0, &ret1, NULL, NULL);
+		if (ret || !ret1)
+			qcom_scm_version = QCOM_SCM_LEGACY;
+		else
+			qcom_scm_version = QCOM_SCM_ARMV8_32;
+	} else
+		qcom_scm_version_mask = SMC64_MASK;
+
+	pr_debug("qcom_scm_call: qcom_scm version is %x, mask is %x\n",
+		 qcom_scm_version, qcom_scm_version_mask);
+
+	return 0;
+}
+early_initcall(qcom_scm_init);
-- 
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

[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

[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.

[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 at 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 4/5] arm64: smp: move the pen to a header file

[Kumar Gala]

From: Abhimanyu Kapur <abhimany@codeaurora.org>

Move the secondary_pen_release variable and the secondary_holding_pen
entry function to asm/smp_plat.h so that the other cpu ops implementations
can share them.

Signed-off-by: Abhimanyu Kapur <abhimany@codeaurora.org>
Signed-off-by: Kumar Gala <galak@codeaurora.org>
---
 arch/arm64/include/asm/smp_plat.h  | 2 ++
 arch/arm64/kernel/smp.c            | 1 +
 arch/arm64/kernel/smp_spin_table.c | 3 ---
 3 files changed, 3 insertions(+), 3 deletions(-)

diff --git a/arch/arm64/include/asm/smp_plat.h b/arch/arm64/include/asm/smp_plat.h
index 59e2823..235ff04 100644
--- a/arch/arm64/include/asm/smp_plat.h
+++ b/arch/arm64/include/asm/smp_plat.h
@@ -34,10 +34,12 @@ static inline u32 mpidr_hash_size(void)
 	return 1 << mpidr_hash.bits;
 }
 
+extern void secondary_holding_pen(void);
 /*
  * Logical CPU mapping.
  */
 extern u64 __cpu_logical_map[NR_CPUS];
 #define cpu_logical_map(cpu)    __cpu_logical_map[cpu]
+extern volatile unsigned long secondary_holding_pen_release;
 
 #endif /* __ASM_SMP_PLAT_H */
diff --git a/arch/arm64/kernel/smp.c b/arch/arm64/kernel/smp.c
index 328b8ce..4ce1f23 100644
--- a/arch/arm64/kernel/smp.c
+++ b/arch/arm64/kernel/smp.c
@@ -61,6 +61,7 @@
  * where to place its SVC stack
  */
 struct secondary_data secondary_data;
+volatile unsigned long secondary_holding_pen_release = INVALID_HWID;
 
 enum ipi_msg_type {
 	IPI_RESCHEDULE,
diff --git a/arch/arm64/kernel/smp_spin_table.c b/arch/arm64/kernel/smp_spin_table.c
index b41a8b4..be833b9 100644
--- a/arch/arm64/kernel/smp_spin_table.c
+++ b/arch/arm64/kernel/smp_spin_table.c
@@ -28,9 +28,6 @@
 #include <asm/io.h>
 #include <asm/smp_plat.h>
 
-extern void secondary_holding_pen(void);
-volatile unsigned long secondary_holding_pen_release = INVALID_HWID;
-
 static phys_addr_t cpu_release_addr[NR_CPUS];
 
 /*
-- 
Qualcomm Innovation Center, Inc.
The Qualcomm Innovation Center, Inc. is a member of the Code Aurora Forum,
a Linux Foundation Collaborative Project

[RFC PATCH 4/5] arm64: smp: move the pen to a header file

[Arnd Bergmann]

On Thursday 09 April 2015 12:37:10 Kumar Gala wrote:
> From: Abhimanyu Kapur <abhimany@codeaurora.org>
> 
> Move the secondary_pen_release variable and the secondary_holding_pen
> entry function to asm/smp_plat.h so that the other cpu ops implementations
> can share them.
> 
> Signed-off-by: Abhimanyu Kapur <abhimany@codeaurora.org>
> Signed-off-by: Kumar Gala <galak@codeaurora.org>
> 

I don't believe your SMP implementation can be so broken to require this.
Please fix the code instead to not use a holding pen.

	Arnd

[RFC PATCH 4/5] arm64: smp: move the pen to a header file

[Kumar Gala]

> On Apr 9, 2015, at 4:17 PM, Arnd Bergmann <arnd@arndb.de> wrote:
> 
> On Thursday 09 April 2015 12:37:10 Kumar Gala wrote:
>> From: Abhimanyu Kapur <abhimany@codeaurora.org>
>> 
>> Move the secondary_pen_release variable and the secondary_holding_pen
>> entry function to asm/smp_plat.h so that the other cpu ops implementations
>> can share them.
>> 
>> Signed-off-by: Abhimanyu Kapur <abhimany@codeaurora.org>
>> Signed-off-by: Kumar Gala <galak@codeaurora.org>
>> 
> 
> I don't believe your SMP implementation can be so broken to require this.
> Please fix the code instead to not use a holding pen.
> 
> 	Arnd

Yeah, I can drop this.

- 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 4/5] arm64: smp: move the pen to a header file

[Mark Rutland]

On Thu, Apr 09, 2015 at 06:37:10PM +0100, Kumar Gala wrote:
> From: Abhimanyu Kapur <abhimany@codeaurora.org>
> 
> Move the secondary_pen_release variable and the secondary_holding_pen
> entry function to asm/smp_plat.h so that the other cpu ops implementations
> can share them.

If anything, this should all be moved into smp_spin_table.c, and made
static.

We made a mistake with the pen (and allowing multiple CPUs to enter the
kernel at once). That mistake shouldn't be spread further.

Mark.

> 
> Signed-off-by: Abhimanyu Kapur <abhimany@codeaurora.org>
> Signed-off-by: Kumar Gala <galak@codeaurora.org>
> ---
>  arch/arm64/include/asm/smp_plat.h  | 2 ++
>  arch/arm64/kernel/smp.c            | 1 +
>  arch/arm64/kernel/smp_spin_table.c | 3 ---
>  3 files changed, 3 insertions(+), 3 deletions(-)
> 
> diff --git a/arch/arm64/include/asm/smp_plat.h b/arch/arm64/include/asm/smp_plat.h
> index 59e2823..235ff04 100644
> --- a/arch/arm64/include/asm/smp_plat.h
> +++ b/arch/arm64/include/asm/smp_plat.h
> @@ -34,10 +34,12 @@ static inline u32 mpidr_hash_size(void)
>  	return 1 << mpidr_hash.bits;
>  }
>  
> +extern void secondary_holding_pen(void);
>  /*
>   * Logical CPU mapping.
>   */
>  extern u64 __cpu_logical_map[NR_CPUS];
>  #define cpu_logical_map(cpu)    __cpu_logical_map[cpu]
> +extern volatile unsigned long secondary_holding_pen_release;
>  
>  #endif /* __ASM_SMP_PLAT_H */
> diff --git a/arch/arm64/kernel/smp.c b/arch/arm64/kernel/smp.c
> index 328b8ce..4ce1f23 100644
> --- a/arch/arm64/kernel/smp.c
> +++ b/arch/arm64/kernel/smp.c
> @@ -61,6 +61,7 @@
>   * where to place its SVC stack
>   */
>  struct secondary_data secondary_data;
> +volatile unsigned long secondary_holding_pen_release = INVALID_HWID;
>  
>  enum ipi_msg_type {
>  	IPI_RESCHEDULE,
> diff --git a/arch/arm64/kernel/smp_spin_table.c b/arch/arm64/kernel/smp_spin_table.c
> index b41a8b4..be833b9 100644
> --- a/arch/arm64/kernel/smp_spin_table.c
> +++ b/arch/arm64/kernel/smp_spin_table.c
> @@ -28,9 +28,6 @@
>  #include <asm/io.h>
>  #include <asm/smp_plat.h>
>  
> -extern void secondary_holding_pen(void);
> -volatile unsigned long secondary_holding_pen_release = INVALID_HWID;
> -
>  static phys_addr_t cpu_release_addr[NR_CPUS];
>  
>  /*
> -- 
> 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 at lists.infradead.org
> http://lists.infradead.org/mailman/listinfo/linux-arm-kernel
> 

[RFC PATCH 4/5] arm64: smp: move the pen to a header file

[Kumar Gala]

> On Apr 14, 2015, at 10:59 AM, Mark Rutland <mark.rutland@arm.com> wrote:
> 
> On Thu, Apr 09, 2015 at 06:37:10PM +0100, Kumar Gala wrote:
>> From: Abhimanyu Kapur <abhimany@codeaurora.org>
>> 
>> Move the secondary_pen_release variable and the secondary_holding_pen
>> entry function to asm/smp_plat.h so that the other cpu ops implementations
>> can share them.
> 
> If anything, this should all be moved into smp_spin_table.c, and made
> static.
> 
> We made a mistake with the pen (and allowing multiple CPUs to enter the
> kernel at once). That mistake shouldn't be spread further.
> 
> Mark.

Yeah, it appears I can drop this and just set the secondary cores to enter secondary_entry.

- 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 5/5] arm64: qcom: add cpu operations

[Kumar Gala]

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
 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 at 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

[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

[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@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.

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

-- 
Catalin

[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 at 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 at vger.kernel.org
> More majordomo info at  http://vger.kernel.org/majordomo-info.html
> 

[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.

[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

[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

[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 at redhat.com
-----------------------------------

[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.

[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

[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 at redhat.com
-----------------------------------

[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

[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@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.
> 
> -- 
> 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

[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@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.

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

[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

[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

[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

[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

[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. 

[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

[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 at vger.kernel.org
>More majordomo info at  http://vger.kernel.org/majordomo-info.html

[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 at vger.kernel.org
> >More majordomo info at  http://vger.kernel.org/majordomo-info.html
> 

[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

[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.

[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

[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

[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@arm.com> 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@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.
>

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 at lists.infradead.org
> http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

[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

[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

[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

[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@arm.com> 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@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).

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

[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@arm.com> 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

[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>

[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

[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

[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