[Patch v5 0/8] Qualcomm SCM Rework

[Patch v5 0/8] Qualcomm SCM Rework

[Andy Gross]

The following set of patches does a bit of rework on the existing
Qualcomm SCM firmware.  The first couple of patches deals with turning
the current SCM into a platform driver.  The next couple are cleanups
that make adding the 64 support a little easier.  I added in a patch to
convert the scm-32 to use DMA streaming APIs.

I took Kumar's 64 bit support patch and modified it to use the arm_smccc
calls.  This simplified things quite a bit.

Lastly, there are a few DT patches to add the firmware node for a couple of the
supported platforms.

Changes from v4:
  * Fix Documentation error
  * Fix misc review comments

Changes from v3:
  * Added compatibles for the specific platforms requiring one clock.
  * All other platforms will use the generic binding that requires 3 clocks.

Changes from v2:
  * Added use of streaming DMA APIs for scm-32.
  * Changed DT binding to simplify compats.
  * Removed Kconfig.platform change that selected QCOM_SCM for ARM64
  * Fixed misc comments
  * Changed init function to populate the device differently.
  * Fixed missing of_node_put

Changes from v1:
  * Changed binding to reflect proper firmware node usage
  * Added arch_initcall to populate the firmware device, if present
  * Fixed various review comments
  * Removed extraneous includes from SCM 64 file.


Andy Gross (7):
  dt/bindings: firmware: Add Qualcomm SCM binding
  firmware: qcom: scm: Convert SCM to platform driver
  firmware: qcom: scm: Use atomic SCM for cold boot
  firmware: qcom: scm: Generalize shared error map
  firmware: qcom: scm: Convert to streaming DMA APIS
  dts: qcom: apq8084: Add SCM firmware node
  arm64: dts: msm8916: Add SCM firmware node

Kumar Gala (1):
  firmware: qcom: scm: Add support for ARM64 SoCs

 .../devicetree/bindings/firmware/qcom,scm.txt      |  28 +++
 arch/arm/boot/dts/qcom-apq8084.dtsi                |   8 +
 arch/arm64/boot/dts/qcom/msm8916.dtsi              |   8 +
 drivers/firmware/qcom_scm-32.c                     | 271 ++++++++-------------
 drivers/firmware/qcom_scm-64.c                     | 199 ++++++++++++++-
 drivers/firmware/qcom_scm.c                        | 171 ++++++++++++-
 drivers/firmware/qcom_scm.h                        |  31 ++-
 7 files changed, 523 insertions(+), 193 deletions(-)
 create mode 100644 Documentation/devicetree/bindings/firmware/qcom,scm.txt

-- 
1.9.1

[Patch v5 1/8] dt/bindings: firmware: Add Qualcomm SCM binding

[Andy Gross]

This patch adds the device tree support for the Qualcomm SCM firmware.

Signed-off-by: Andy Gross <andy.gross@linaro.org>
---
 .../devicetree/bindings/firmware/qcom,scm.txt      | 28 ++++++++++++++++++++++
 1 file changed, 28 insertions(+)
 create mode 100644 Documentation/devicetree/bindings/firmware/qcom,scm.txt

diff --git a/Documentation/devicetree/bindings/firmware/qcom,scm.txt b/Documentation/devicetree/bindings/firmware/qcom,scm.txt
new file mode 100644
index 0000000..3b4436e
--- /dev/null
+++ b/Documentation/devicetree/bindings/firmware/qcom,scm.txt
@@ -0,0 +1,28 @@
+QCOM Secure Channel Manager (SCM)
+
+Qualcomm processors include an interface to communicate to the secure firmware.
+This interface allows for clients to request different types of actions.  These
+can include CPU power up/down, HDCP requests, loading of firmware, and other
+assorted actions.
+
+Required properties:
+- compatible: must contain one of the following:
+ * "qcom,scm-apq8064" for APQ8064 platforms
+ * "qcom,scm-msm8660" for MSM8660 platforms
+ * "qcom,scm-msm8690" for MSM8690 platforms
+ * "qcom,scm" for later processors (MSM8916, APQ8084, MSM8974, etc)
+- clocks: One to three clocks may be required based on compatible.
+ * Only core clock required for "qcom,scm-apq8064", "qcom,scm-msm8660", and "qcom,scm-msm8960"
+ * Core, iface, and bus clocks required for "qcom,scm"
+- clock-names: Must contain "core" for the core clock, "iface" for the interface
+  clock and "bus" for the bus clock per the requirements of the compatible.
+
+Example for MSM8916:
+
+	firmware {
+		scm {
+			compatible = "qcom,scm";
+			clocks = <&gcc GCC_CRYPTO_CLK> , <&gcc GCC_CRYPTO_AXI_CLK>, <&gcc GCC_CRYPTO_AHB_CLK>;
+			clock-names = "core", "bus", "iface";
+		};
+	};
-- 
1.9.1

[Patch v5 2/8] firmware: qcom: scm: Convert SCM to platform driver

[Andy Gross]

This patch converts the Qualcomm SCM firmware driver into a platform
driver.

Signed-off-by: Andy Gross <andy.gross@linaro.org>
---
 drivers/firmware/qcom_scm.c | 165 +++++++++++++++++++++++++++++++++++++++++---
 1 file changed, 156 insertions(+), 9 deletions(-)

diff --git a/drivers/firmware/qcom_scm.c b/drivers/firmware/qcom_scm.c
index 45c008d..4d6ae32 100644
--- a/drivers/firmware/qcom_scm.c
+++ b/drivers/firmware/qcom_scm.c
@@ -10,19 +10,61 @@
  * 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/platform_device.h>
+#include <linux/module.h>
 #include <linux/cpumask.h>
 #include <linux/export.h>
 #include <linux/types.h>
 #include <linux/qcom_scm.h>
+#include <linux/of.h>
+#include <linux/of_platform.h>
+#include <linux/clk.h>
 
 #include "qcom_scm.h"
 
+struct qcom_scm {
+	struct device *dev;
+	struct clk *core_clk;
+	struct clk *iface_clk;
+	struct clk *bus_clk;
+};
+
+static struct qcom_scm *__scm;
+
+static int qcom_scm_clk_enable(void)
+{
+	int ret;
+
+	ret = clk_prepare_enable(__scm->core_clk);
+	if (ret)
+		goto bail;
+
+	ret = clk_prepare_enable(__scm->iface_clk);
+	if (ret)
+		goto disable_core;
+
+	ret = clk_prepare_enable(__scm->bus_clk);
+	if (ret)
+		goto disable_iface;
+
+	return 0;
+
+disable_iface:
+	clk_disable_unprepare(__scm->iface_clk);
+disable_core:
+	clk_disable_unprepare(__scm->core_clk);
+bail:
+	return ret;
+}
+
+static void qcom_scm_clk_disable(void)
+{
+	clk_disable_unprepare(__scm->core_clk);
+	clk_disable_unprepare(__scm->iface_clk);
+	clk_disable_unprepare(__scm->bus_clk);
+}
+
 /**
  * qcom_scm_set_cold_boot_addr() - Set the cold boot address for cpus
  * @entry: Entry point function for the cpus
@@ -72,12 +114,17 @@ EXPORT_SYMBOL(qcom_scm_cpu_power_down);
  */
 bool qcom_scm_hdcp_available(void)
 {
-	int ret;
+	int ret = qcom_scm_clk_enable();
+
+	if (ret)
+		return ret;
 
 	ret = __qcom_scm_is_call_available(QCOM_SCM_SVC_HDCP,
-		QCOM_SCM_CMD_HDCP);
+						QCOM_SCM_CMD_HDCP);
+
+	qcom_scm_clk_disable();
 
-	return (ret > 0) ? true : false;
+	return ret > 0 ? true : false;
 }
 EXPORT_SYMBOL(qcom_scm_hdcp_available);
 
@@ -91,6 +138,106 @@ EXPORT_SYMBOL(qcom_scm_hdcp_available);
  */
 int qcom_scm_hdcp_req(struct qcom_scm_hdcp_req *req, u32 req_cnt, u32 *resp)
 {
-	return __qcom_scm_hdcp_req(req, req_cnt, resp);
+	int ret = qcom_scm_clk_enable();
+
+	if (ret)
+		return ret;
+
+	ret = __qcom_scm_hdcp_req(req, req_cnt, resp);
+	qcom_scm_clk_disable();
+	return ret;
 }
 EXPORT_SYMBOL(qcom_scm_hdcp_req);
+
+static int qcom_scm_probe(struct platform_device *pdev)
+{
+	struct qcom_scm *scm;
+	int ret;
+
+	scm = devm_kzalloc(&pdev->dev, sizeof(*scm), GFP_KERNEL);
+	if (!scm)
+		return -ENOMEM;
+
+	scm->core_clk = devm_clk_get(&pdev->dev, "core");
+	if (IS_ERR(scm->core_clk)) {
+		if (PTR_ERR(scm->core_clk) != -EPROBE_DEFER)
+			dev_err(&pdev->dev, "failed to acquire core clk\n");
+		return PTR_ERR(scm->core_clk);
+	}
+
+	if (of_device_is_compatible(pdev->dev.of_node, "qcom,scm")) {
+		scm->iface_clk = devm_clk_get(&pdev->dev, "iface");
+		if (IS_ERR(scm->iface_clk)) {
+			if (PTR_ERR(scm->iface_clk) != -EPROBE_DEFER)
+				dev_err(&pdev->dev, "failed to acquire iface clk\n");
+			return PTR_ERR(scm->iface_clk);
+		}
+
+		scm->bus_clk = devm_clk_get(&pdev->dev, "bus");
+		if (IS_ERR(scm->bus_clk)) {
+			if (PTR_ERR(scm->bus_clk) != -EPROBE_DEFER)
+				dev_err(&pdev->dev, "failed to acquire bus clk\n");
+			return PTR_ERR(scm->bus_clk);
+		}
+	}
+
+	/* vote for max clk rate for highest performance */
+	ret = clk_set_rate(scm->core_clk, INT_MAX);
+	if (ret)
+		return ret;
+
+	__scm = scm;
+	__scm->dev = &pdev->dev;
+
+	return 0;
+}
+
+static const struct of_device_id qcom_scm_dt_match[] = {
+	{ .compatible = "qcom,scm-apq8064",},
+	{ .compatible = "qcom,scm-msm8660",},
+	{ .compatible = "qcom,scm-msm8960",},
+	{ .compatible = "qcom,scm",},
+	{}
+};
+
+MODULE_DEVICE_TABLE(of, qcom_scm_dt_match);
+
+static struct platform_driver qcom_scm_driver = {
+	.driver = {
+		.name	= "qcom_scm",
+		.of_match_table = qcom_scm_dt_match,
+	},
+	.probe = qcom_scm_probe,
+};
+
+static int __init qcom_scm_init(void)
+{
+	struct device_node *np, *parent;
+	int ret;
+
+	np = of_find_matching_node_and_match(NULL, qcom_scm_dt_match, NULL);
+
+	if (!np)
+		return -ENODEV;
+
+	parent = of_get_next_parent(np);
+	ret = of_platform_populate(parent, qcom_scm_dt_match, NULL, NULL);
+
+	of_node_put(parent);
+
+	if (ret)
+		return ret;
+
+	return platform_driver_register(&qcom_scm_driver);
+}
+
+arch_initcall(qcom_scm_init);
+
+static void __exit qcom_scm_exit(void)
+{
+	platform_driver_unregister(&qcom_scm_driver);
+}
+module_exit(qcom_scm_exit);
+
+MODULE_DESCRIPTION("Qualcomm SCM driver");
+MODULE_LICENSE("GPL v2");
-- 
1.9.1

[Patch v5 3/8] firmware: qcom: scm: Use atomic SCM for cold boot

[Andy Gross]

This patch changes the cold_set_boot_addr function to use atomic SCM
calls.  cold_set_boot_addr required adding qcom_scm_call_atomic2 to
support the two arguments going to the smc call.  Using atomic removes
the need for memory allocation and instead places all arguments in
registers.

Signed-off-by: Andy Gross <andy.gross@linaro.org>
---
 drivers/firmware/qcom_scm-32.c | 63 ++++++++++++++++++++++++++++++------------
 1 file changed, 45 insertions(+), 18 deletions(-)

diff --git a/drivers/firmware/qcom_scm-32.c b/drivers/firmware/qcom_scm-32.c
index 0883292..5be6a12 100644
--- a/drivers/firmware/qcom_scm-32.c
+++ b/drivers/firmware/qcom_scm-32.c
@@ -342,6 +342,41 @@ static s32 qcom_scm_call_atomic1(u32 svc, u32 cmd, u32 arg1)
 	return r0;
 }
 
+/**
+ * qcom_scm_call_atomic2() - Send an atomic SCM command with two arguments
+ * @svc_id:	service identifier
+ * @cmd_id:	command identifier
+ * @arg1:	first argument
+ * @arg2:	second argument
+ *
+ * This shall only be used with commands that are guaranteed to be
+ * uninterruptable, atomic and SMP safe.
+ */
+static s32 qcom_scm_call_atomic2(u32 svc, u32 cmd, u32 arg1, u32 arg2)
+{
+	int context_id;
+
+	register u32 r0 asm("r0") = SCM_ATOMIC(svc, cmd, 2);
+	register u32 r1 asm("r1") = (u32)&context_id;
+	register u32 r2 asm("r2") = arg1;
+	register u32 r3 asm("r3") = arg2;
+
+	asm volatile(
+			__asmeq("%0", "r0")
+			__asmeq("%1", "r0")
+			__asmeq("%2", "r1")
+			__asmeq("%3", "r2")
+			__asmeq("%4", "r3")
+#ifdef REQUIRES_SEC
+			".arch_extension sec\n"
+#endif
+			"smc    #0      @ switch to secure world\n"
+			: "=r" (r0)
+			: "r" (r0), "r" (r1), "r" (r2), "r" (r3)
+			);
+	return r0;
+}
+
 u32 qcom_scm_get_version(void)
 {
 	int context_id;
@@ -378,22 +413,6 @@ u32 qcom_scm_get_version(void)
 }
 EXPORT_SYMBOL(qcom_scm_get_version);
 
-/*
- * 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);
-}
-
 /**
  * qcom_scm_set_cold_boot_addr() - Set the cold boot address for cpus
  * @entry: Entry point function for the cpus
@@ -423,7 +442,8 @@ int __qcom_scm_set_cold_boot_addr(void *entry, const cpumask_t *cpus)
 			set_cpu_present(cpu, false);
 	}
 
-	return qcom_scm_set_boot_addr(virt_to_phys(entry), flags);
+	return qcom_scm_call_atomic2(QCOM_SCM_SVC_BOOT, QCOM_SCM_BOOT_ADDR,
+				    flags, virt_to_phys(entry));
 }
 
 /**
@@ -439,6 +459,10 @@ int __qcom_scm_set_warm_boot_addr(void *entry, const cpumask_t *cpus)
 	int ret;
 	int flags = 0;
 	int cpu;
+	struct {
+		__le32 flags;
+		__le32 addr;
+	} cmd;
 
 	/*
 	 * Reassign only if we are switching from hotplug entry point
@@ -454,7 +478,10 @@ int __qcom_scm_set_warm_boot_addr(void *entry, const cpumask_t *cpus)
 	if (!flags)
 		return 0;
 
-	ret = qcom_scm_set_boot_addr(virt_to_phys(entry), flags);
+	cmd.addr = cpu_to_le32(virt_to_phys(entry));
+	cmd.flags = cpu_to_le32(flags);
+	ret = qcom_scm_call(QCOM_SCM_SVC_BOOT, QCOM_SCM_BOOT_ADDR,
+			    &cmd, sizeof(cmd), NULL, 0);
 	if (!ret) {
 		for_each_cpu(cpu, cpus)
 			qcom_scm_wb[cpu].entry = entry;
-- 
1.9.1

[Patch v5 4/8] firmware: qcom: scm: Generalize shared error map

[Andy Gross]

This patch moves the qcom_scm_remap_error function to the include file
where can be used by both the 32 and 64 bit versions of the code.

Reviewed-by: Stephen Boyd <sboyd@codeaurora.org>
Acked-by: Bjorn Andersson <bjorn.andersson@linaro.org>
Signed-off-by: Andy Gross <agross@codeaurora.org>
Signed-off-by: Andy Gross <andy.gross@linaro.org>
---
 drivers/firmware/qcom_scm-32.c | 17 -----------------
 drivers/firmware/qcom_scm.h    | 16 ++++++++++++++++
 2 files changed, 16 insertions(+), 17 deletions(-)

diff --git a/drivers/firmware/qcom_scm-32.c b/drivers/firmware/qcom_scm-32.c
index 5be6a12..4388d13 100644
--- a/drivers/firmware/qcom_scm-32.c
+++ b/drivers/firmware/qcom_scm-32.c
@@ -168,23 +168,6 @@ static inline void *qcom_scm_get_response_buffer(const struct qcom_scm_response
 	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;
diff --git a/drivers/firmware/qcom_scm.h b/drivers/firmware/qcom_scm.h
index 2cce75c..7dcc733 100644
--- a/drivers/firmware/qcom_scm.h
+++ b/drivers/firmware/qcom_scm.h
@@ -44,4 +44,20 @@ extern int __qcom_scm_hdcp_req(struct qcom_scm_hdcp_req *req, u32 req_cnt,
 #define QCOM_SCM_ERROR		-1
 #define QCOM_SCM_INTERRUPTED	1
 
+static inline 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;
+	}
+	return -EINVAL;
+}
+
 #endif
-- 
1.9.1

[Patch v5 5/8] firmware: qcom: scm: Convert to streaming DMA APIS

[Andy Gross]

This patch converts the Qualcomm SCM driver to use the streaming DMA APIs
for communication buffers.

Signed-off-by: Andy Gross <andy.gross@linaro.org>
---
 drivers/firmware/qcom_scm-32.c | 189 +++++++++++------------------------------
 drivers/firmware/qcom_scm.c    |   6 +-
 drivers/firmware/qcom_scm.h    |  10 ++-
 3 files changed, 59 insertions(+), 146 deletions(-)

diff --git a/drivers/firmware/qcom_scm-32.c b/drivers/firmware/qcom_scm-32.c
index 4388d13..e92bf7a 100644
--- a/drivers/firmware/qcom_scm-32.c
+++ b/drivers/firmware/qcom_scm-32.c
@@ -23,8 +23,7 @@
 #include <linux/errno.h>
 #include <linux/err.h>
 #include <linux/qcom_scm.h>
-
-#include <asm/cacheflush.h>
+#include <linux/dma-mapping.h>
 
 #include "qcom_scm.h"
 
@@ -64,11 +63,11 @@ static DEFINE_MUTEX(qcom_scm_lock);
  *
  *	------------------- <--- struct qcom_scm_command
  *	| command header  |
- *	------------------- <--- qcom_scm_get_command_buffer()
+ *	------------------- <--- buf[0]
  *	| command buffer  |
- *	------------------- <--- struct qcom_scm_response and
- *	| response header |      qcom_scm_command_to_response()
- *	------------------- <--- qcom_scm_get_response_buffer()
+ *	------------------- <--- struct qcom_scm_response
+ *	| response header |
+ *	-------------------
  *	| response buffer |
  *	-------------------
  *
@@ -96,79 +95,7 @@ struct qcom_scm_response {
 	__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 u32 smc(u32 cmd_addr)
+static u32 smc(dma_addr_t cmd_addr)
 {
 	int context_id;
 	register u32 r0 asm("r0") = 1;
@@ -192,45 +119,9 @@ static u32 smc(u32 cmd_addr)
 	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.
-	 */
-	secure_flush_area(cmd, 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
+ * @dev: struct device
  * @svc_id: service identifier
  * @cmd_id: command identifier
  * @cmd_buf: command buffer
@@ -247,42 +138,59 @@ static void qcom_scm_inv_range(unsigned long start, unsigned long end)
  * 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)
+static int qcom_scm_call(struct device *dev, 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;
+	size_t alloc_len = sizeof(*cmd) + cmd_len + sizeof(*rsp) + resp_len;
+	dma_addr_t cmd_phys;
 
-	cmd = alloc_qcom_scm_command(cmd_len, resp_len);
+	cmd = kzalloc(PAGE_ALIGN(alloc_len), GFP_KERNEL);
 	if (!cmd)
 		return -ENOMEM;
 
+	cmd->len = cpu_to_le32(alloc_len);
+	cmd->buf_offset = cpu_to_le32(sizeof(*cmd));
+	cmd->resp_hdr_offset = cpu_to_le32(sizeof(*cmd) + cmd_len);
+
 	cmd->id = cpu_to_le32((svc_id << 10) | cmd_id);
 	if (cmd_buf)
-		memcpy(qcom_scm_get_command_buffer(cmd), cmd_buf, cmd_len);
+		memcpy(cmd->buf, cmd_buf, cmd_len);
+
+	rsp = (void *)cmd->buf + le32_to_cpu(cmd->resp_hdr_offset);
+
+	cmd_phys = dma_map_single(dev, cmd, alloc_len, DMA_TO_DEVICE);
+	if (dma_mapping_error(dev, cmd_phys)) {
+		kfree(cmd);
+		return -ENOMEM;
+	}
 
 	mutex_lock(&qcom_scm_lock);
-	ret = __qcom_scm_call(cmd);
+	ret = smc(cmd_phys);
+	if (ret < 0)
+		ret = qcom_scm_remap_error(ret);
 	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));
+		dma_sync_single_for_cpu(dev, cmd_phys + sizeof(*cmd) + cmd_len,
+					sizeof(*rsp), DMA_FROM_DEVICE);
 	} 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);
+	if (resp_buf) {
+		dma_sync_single_for_cpu(dev, cmd_phys + sizeof(*cmd) + cmd_len +
+					le32_to_cpu(rsp->buf_offset),
+					resp_len, DMA_FROM_DEVICE);
+		memcpy(resp_buf, (void *)rsp + le32_to_cpu(rsp->buf_offset),
+		       resp_len);
+	}
 out:
-	free_qcom_scm_command(cmd);
+	dma_unmap_single(dev, cmd_phys, alloc_len, DMA_TO_DEVICE);
+	kfree(cmd);
 	return ret;
 }
 
@@ -437,7 +345,8 @@ int __qcom_scm_set_cold_boot_addr(void *entry, const cpumask_t *cpus)
  * 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(struct device *dev, void *entry,
+				  const cpumask_t *cpus)
 {
 	int ret;
 	int flags = 0;
@@ -463,7 +372,7 @@ int __qcom_scm_set_warm_boot_addr(void *entry, const cpumask_t *cpus)
 
 	cmd.addr = cpu_to_le32(virt_to_phys(entry));
 	cmd.flags = cpu_to_le32(flags);
-	ret = qcom_scm_call(QCOM_SCM_SVC_BOOT, QCOM_SCM_BOOT_ADDR,
+	ret = qcom_scm_call(dev, QCOM_SCM_SVC_BOOT, QCOM_SCM_BOOT_ADDR,
 			    &cmd, sizeof(cmd), NULL, 0);
 	if (!ret) {
 		for_each_cpu(cpu, cpus)
@@ -487,25 +396,27 @@ void __qcom_scm_cpu_power_down(u32 flags)
 			flags & QCOM_SCM_FLUSH_FLAG_MASK);
 }
 
-int __qcom_scm_is_call_available(u32 svc_id, u32 cmd_id)
+int __qcom_scm_is_call_available(struct device *dev, u32 svc_id, u32 cmd_id)
 {
 	int ret;
 	__le32 svc_cmd = cpu_to_le32((svc_id << 10) | cmd_id);
 	__le32 ret_val = 0;
 
-	ret = qcom_scm_call(QCOM_SCM_SVC_INFO, QCOM_IS_CALL_AVAIL_CMD, &svc_cmd,
-			sizeof(svc_cmd), &ret_val, sizeof(ret_val));
+	ret = qcom_scm_call(dev, QCOM_SCM_SVC_INFO, QCOM_IS_CALL_AVAIL_CMD,
+			    &svc_cmd, sizeof(svc_cmd), &ret_val,
+			    sizeof(ret_val));
 	if (ret)
 		return ret;
 
 	return le32_to_cpu(ret_val);
 }
 
-int __qcom_scm_hdcp_req(struct qcom_scm_hdcp_req *req, u32 req_cnt, u32 *resp)
+int __qcom_scm_hdcp_req(struct device *dev, struct qcom_scm_hdcp_req *req,
+			u32 req_cnt, u32 *resp)
 {
 	if (req_cnt > QCOM_SCM_HDCP_MAX_REQ_CNT)
 		return -ERANGE;
 
-	return qcom_scm_call(QCOM_SCM_SVC_HDCP, QCOM_SCM_CMD_HDCP,
+	return qcom_scm_call(dev, QCOM_SCM_SVC_HDCP, QCOM_SCM_CMD_HDCP,
 		req, req_cnt * sizeof(*req), resp, sizeof(*resp));
 }
diff --git a/drivers/firmware/qcom_scm.c b/drivers/firmware/qcom_scm.c
index 4d6ae32..23ccec6 100644
--- a/drivers/firmware/qcom_scm.c
+++ b/drivers/firmware/qcom_scm.c
@@ -89,7 +89,7 @@ EXPORT_SYMBOL(qcom_scm_set_cold_boot_addr);
  */
 int qcom_scm_set_warm_boot_addr(void *entry, const cpumask_t *cpus)
 {
-	return __qcom_scm_set_warm_boot_addr(entry, cpus);
+	return __qcom_scm_set_warm_boot_addr(__scm->dev, entry, cpus);
 }
 EXPORT_SYMBOL(qcom_scm_set_warm_boot_addr);
 
@@ -119,7 +119,7 @@ bool qcom_scm_hdcp_available(void)
 	if (ret)
 		return ret;
 
-	ret = __qcom_scm_is_call_available(QCOM_SCM_SVC_HDCP,
+	ret = __qcom_scm_is_call_available(__scm->dev, QCOM_SCM_SVC_HDCP,
 						QCOM_SCM_CMD_HDCP);
 
 	qcom_scm_clk_disable();
@@ -143,7 +143,7 @@ int qcom_scm_hdcp_req(struct qcom_scm_hdcp_req *req, u32 req_cnt, u32 *resp)
 	if (ret)
 		return ret;
 
-	ret = __qcom_scm_hdcp_req(req, req_cnt, resp);
+	ret = __qcom_scm_hdcp_req(__scm->dev, req, req_cnt, resp);
 	qcom_scm_clk_disable();
 	return ret;
 }
diff --git a/drivers/firmware/qcom_scm.h b/drivers/firmware/qcom_scm.h
index 7dcc733..afe6676 100644
--- a/drivers/firmware/qcom_scm.h
+++ b/drivers/firmware/qcom_scm.h
@@ -19,7 +19,8 @@
 #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_warm_boot_addr(struct device *dev, 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
@@ -29,12 +30,13 @@ extern void __qcom_scm_cpu_power_down(u32 flags);
 
 #define QCOM_SCM_SVC_INFO		0x6
 #define QCOM_IS_CALL_AVAIL_CMD		0x1
-extern int __qcom_scm_is_call_available(u32 svc_id, u32 cmd_id);
+extern int __qcom_scm_is_call_available(struct device *dev, u32 svc_id,
+		u32 cmd_id);
 
 #define QCOM_SCM_SVC_HDCP		0x11
 #define QCOM_SCM_CMD_HDCP		0x01
-extern int __qcom_scm_hdcp_req(struct qcom_scm_hdcp_req *req, u32 req_cnt,
-		u32 *resp);
+extern int __qcom_scm_hdcp_req(struct device *dev,
+		struct qcom_scm_hdcp_req *req, u32 req_cnt, u32 *resp);
 
 /* common error codes */
 #define QCOM_SCM_ENOMEM		-5
-- 
1.9.1

[Patch v5 6/8] firmware: qcom: scm: Add support for ARM64 SoCs

[Andy Gross]

From: Kumar Gala <galak@codeaurora.org>

Add an implementation of the SCM interface that works on ARM64 SoCs.  This
is used by things like determine if we have HDCP support or not on the
system.

Signed-off-by: Kumar Gala <galak@codeaurora.org>
Signed-off-by: Andy Gross <andy.gross@linaro.org>
---
 drivers/firmware/qcom_scm-32.c |   4 +
 drivers/firmware/qcom_scm-64.c | 199 +++++++++++++++++++++++++++++++++++++++--
 drivers/firmware/qcom_scm.c    |   2 +
 drivers/firmware/qcom_scm.h    |   5 ++
 4 files changed, 205 insertions(+), 5 deletions(-)

diff --git a/drivers/firmware/qcom_scm-32.c b/drivers/firmware/qcom_scm-32.c
index e92bf7a..4971b55 100644
--- a/drivers/firmware/qcom_scm-32.c
+++ b/drivers/firmware/qcom_scm-32.c
@@ -420,3 +420,7 @@ int __qcom_scm_hdcp_req(struct device *dev, struct qcom_scm_hdcp_req *req,
 	return qcom_scm_call(dev, QCOM_SCM_SVC_HDCP, QCOM_SCM_CMD_HDCP,
 		req, req_cnt * sizeof(*req), resp, sizeof(*resp));
 }
+
+void __qcom_scm_init(void)
+{
+}
diff --git a/drivers/firmware/qcom_scm-64.c b/drivers/firmware/qcom_scm-64.c
index bb6555f..43ed4de 100644
--- a/drivers/firmware/qcom_scm-64.c
+++ b/drivers/firmware/qcom_scm-64.c
@@ -12,7 +12,143 @@
 
 #include <linux/io.h>
 #include <linux/errno.h>
+#include <linux/delay.h>
+#include <linux/mutex.h>
+#include <linux/slab.h>
+#include <linux/types.h>
 #include <linux/qcom_scm.h>
+#include <linux/arm-smccc.h>
+#include <linux/dma-mapping.h>
+
+#include "qcom_scm.h"
+
+#define QCOM_SCM_FNID(s, c) ((((s) & 0xFF) << 8) | ((c) & 0xFF))
+
+#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) & 0x3) << 4) | \
+			   (((b) & 0x3) << 6) | \
+			   (((c) & 0x3) << 8) | \
+			   (((d) & 0x3) << 10) | \
+			   (((e) & 0x3) << 12) | \
+			   (((f) & 0x3) << 14) | \
+			   (((g) & 0x3) << 16) | \
+			   (((h) & 0x3) << 18) | \
+			   (((i) & 0x3) << 20) | \
+			   (((j) & 0x3) << 22) | \
+			   (num & 0xf))
+
+#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
+ * @res:	The values returned by the secure syscall
+ */
+struct qcom_scm_desc {
+	u32 arginfo;
+	u64 args[MAX_QCOM_SCM_ARGS];
+	struct arm_smccc_res res;
+};
+
+static u64 qcom_smccc_convention = -1;
+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 N_REGISTER_ARGS (MAX_QCOM_SCM_ARGS - N_EXT_QCOM_SCM_ARGS + 1)
+
+/**
+ * qcom_scm_call() - Invoke a syscall in the secure world
+ * @dev:	device
+ * @svc_id:	service identifier
+ * @cmd_id:	command identifier
+ * @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.
+*/
+static int qcom_scm_call(struct device *dev, u32 svc_id, u32 cmd_id,
+			 struct qcom_scm_desc *desc)
+{
+	int arglen = desc->arginfo & 0xf;
+	int retry_count = 0, i;
+	u32 fn_id = QCOM_SCM_FNID(svc_id, cmd_id);
+	u64 cmd, x5 = desc->args[FIRST_EXT_ARG_IDX];
+	dma_addr_t args_phys = 0;
+	void *args_virt = NULL;
+	size_t alloc_len;
+
+	if (unlikely(arglen > N_REGISTER_ARGS)) {
+		alloc_len = N_EXT_QCOM_SCM_ARGS * sizeof(u64);
+		args_virt = kzalloc(PAGE_ALIGN(alloc_len), GFP_KERNEL);
+
+		if (!args_virt)
+			return qcom_scm_remap_error(-ENOMEM);
+
+		if (qcom_smccc_convention == ARM_SMCCC_SMC_32) {
+			__le32 *args = args_virt;
+
+			for (i = 0; i < N_EXT_QCOM_SCM_ARGS; i++)
+				args[i] = cpu_to_le32(desc->args[i +
+						      FIRST_EXT_ARG_IDX]);
+		} else {
+			__le64 *args = args_virt;
+
+			for (i = 0; i < N_EXT_QCOM_SCM_ARGS; i++)
+				args[i] = cpu_to_le64(desc->args[i +
+						      FIRST_EXT_ARG_IDX]);
+		}
+
+		args_phys = dma_map_single(dev, args_virt, alloc_len,
+					   DMA_TO_DEVICE);
+
+		if (dma_mapping_error(dev, args_phys)) {
+			kfree(args_virt);
+			return qcom_scm_remap_error(-ENOMEM);
+		}
+
+		x5 = args_phys;
+	}
+
+	do {
+		mutex_lock(&qcom_scm_lock);
+
+		cmd = ARM_SMCCC_CALL_VAL(ARM_SMCCC_STD_CALL,
+					 qcom_smccc_convention,
+					 ARM_SMCCC_OWNER_SIP, fn_id);
+
+		do {
+			arm_smccc_smc(cmd, desc->arginfo, desc->args[0],
+				      desc->args[1], desc->args[2], x5, 0, 0,
+				      &desc->res);
+		} while (desc->res.a0 == QCOM_SCM_INTERRUPTED);
+
+		mutex_unlock(&qcom_scm_lock);
+
+		if (desc->res.a0 == QCOM_SCM_V2_EBUSY) {
+			if (retry_count++ > QCOM_SCM_EBUSY_MAX_RETRY)
+				break;
+			msleep(QCOM_SCM_EBUSY_WAIT_MS);
+		}
+	}  while (desc->res.a0 == QCOM_SCM_V2_EBUSY);
+
+	if (args_virt) {
+		dma_unmap_single(dev, args_phys, alloc_len, DMA_TO_DEVICE);
+		kfree(args_virt);
+	}
+
+	if (desc->res.a0 < 0)
+		return qcom_scm_remap_error(desc->res.a0);
+
+	return 0;
+}
 
 /**
  * qcom_scm_set_cold_boot_addr() - Set the cold boot address for cpus
@@ -29,13 +165,15 @@ int __qcom_scm_set_cold_boot_addr(void *entry, const cpumask_t *cpus)
 
 /**
  * qcom_scm_set_warm_boot_addr() - Set the warm boot address for cpus
+ * @dev: Device pointer
  * @entry: Entry point function for the cpus
  * @cpus: The cpumask of cpus that will use the entry point
  *
  * 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(struct device *dev, void *entry,
+				  const cpumask_t *cpus)
 {
 	return -ENOTSUPP;
 }
@@ -52,12 +190,63 @@ void __qcom_scm_cpu_power_down(u32 flags)
 {
 }
 
-int __qcom_scm_is_call_available(u32 svc_id, u32 cmd_id)
+int __qcom_scm_is_call_available(struct device *dev, u32 svc_id, u32 cmd_id)
 {
-	return -ENOTSUPP;
+	int ret;
+	struct qcom_scm_desc desc = {0};
+
+	desc.arginfo = QCOM_SCM_ARGS(1);
+	desc.args[0] = QCOM_SCM_FNID(svc_id, cmd_id) |
+			(ARM_SMCCC_OWNER_SIP << ARM_SMCCC_OWNER_SHIFT);
+
+	ret = qcom_scm_call(dev, QCOM_SCM_SVC_INFO, QCOM_IS_CALL_AVAIL_CMD,
+			    &desc);
+
+	return ret ? : desc.res.a1;
 }
 
-int __qcom_scm_hdcp_req(struct qcom_scm_hdcp_req *req, u32 req_cnt, u32 *resp)
+int __qcom_scm_hdcp_req(struct device *dev, struct qcom_scm_hdcp_req *req,
+			u32 req_cnt, u32 *resp)
 {
-	return -ENOTSUPP;
+	int ret;
+	struct qcom_scm_desc desc = {0};
+
+	if (req_cnt > QCOM_SCM_HDCP_MAX_REQ_CNT)
+		return -ERANGE;
+
+	desc.args[0] = req[0].addr;
+	desc.args[1] = req[0].val;
+	desc.args[2] = req[1].addr;
+	desc.args[3] = req[1].val;
+	desc.args[4] = req[2].addr;
+	desc.args[5] = req[2].val;
+	desc.args[6] = req[3].addr;
+	desc.args[7] = req[3].val;
+	desc.args[8] = req[4].addr;
+	desc.args[9] = req[4].val;
+	desc.arginfo = QCOM_SCM_ARGS(10);
+
+	ret = qcom_scm_call(dev, QCOM_SCM_SVC_HDCP, QCOM_SCM_CMD_HDCP, &desc);
+	*resp = desc.res.a1;
+
+	return ret;
+}
+
+void __qcom_scm_init(void)
+{
+	u64 cmd;
+	struct arm_smccc_res res;
+	u32 function = QCOM_SCM_FNID(QCOM_SCM_SVC_INFO, QCOM_IS_CALL_AVAIL_CMD);
+
+	/* First try a SMC64 call */
+	cmd = ARM_SMCCC_CALL_VAL(ARM_SMCCC_FAST_CALL, ARM_SMCCC_SMC_64,
+				 ARM_SMCCC_OWNER_SIP, function);
+
+	arm_smccc_smc(cmd, QCOM_SCM_ARGS(1), cmd & (~BIT(ARM_SMCCC_TYPE_SHIFT)),
+		      0, 0, 0, 0, 0, &res);
+
+	if (!res.a0 && res.a1)
+		qcom_smccc_convention = ARM_SMCCC_SMC_64;
+	else
+		qcom_smccc_convention = ARM_SMCCC_SMC_32;
 }
diff --git a/drivers/firmware/qcom_scm.c b/drivers/firmware/qcom_scm.c
index 23ccec6..475410b3e 100644
--- a/drivers/firmware/qcom_scm.c
+++ b/drivers/firmware/qcom_scm.c
@@ -189,6 +189,8 @@ static int qcom_scm_probe(struct platform_device *pdev)
 	__scm = scm;
 	__scm->dev = &pdev->dev;
 
+	__qcom_scm_init();
+
 	return 0;
 }
 
diff --git a/drivers/firmware/qcom_scm.h b/drivers/firmware/qcom_scm.h
index afe6676..0ea55d7 100644
--- a/drivers/firmware/qcom_scm.h
+++ b/drivers/firmware/qcom_scm.h
@@ -38,7 +38,10 @@ extern int __qcom_scm_is_call_available(struct device *dev, u32 svc_id,
 extern int __qcom_scm_hdcp_req(struct device *dev,
 		struct qcom_scm_hdcp_req *req, u32 req_cnt, u32 *resp);
 
+extern void __qcom_scm_init(void);
+
 /* common error codes */
+#define QCOM_SCM_V2_EBUSY	-12
 #define QCOM_SCM_ENOMEM		-5
 #define QCOM_SCM_EOPNOTSUPP	-4
 #define QCOM_SCM_EINVAL_ADDR	-3
@@ -58,6 +61,8 @@ static inline int qcom_scm_remap_error(int err)
 		return -EOPNOTSUPP;
 	case QCOM_SCM_ENOMEM:
 		return -ENOMEM;
+	case QCOM_SCM_V2_EBUSY:
+		return -EBUSY;
 	}
 	return -EINVAL;
 }
-- 
1.9.1

[Patch v5 7/8] dts: qcom: apq8084: Add SCM firmware node

[Andy Gross]

This patch adds the firmware node for the SCM

Acked-by: Bjorn Andersson <bjorn.andersson@linaro.org>
Signed-off-by: Andy Gross <andy.gross@linaro.org>
---
 arch/arm/boot/dts/qcom-apq8084.dtsi | 8 ++++++++
 1 file changed, 8 insertions(+)

diff --git a/arch/arm/boot/dts/qcom-apq8084.dtsi b/arch/arm/boot/dts/qcom-apq8084.dtsi
index a33a09f..7c2df06 100644
--- a/arch/arm/boot/dts/qcom-apq8084.dtsi
+++ b/arch/arm/boot/dts/qcom-apq8084.dtsi
@@ -86,6 +86,14 @@
 		};
 	};
 
+	firmware {
+		scm {
+			compatible = "qcom,scm";
+			clocks = <&gcc GCC_CE1_CLK> , <&gcc GCC_CE1_AXI_CLK>, <&gcc GCC_CE1_AHB_CLK>;
+			clock-names = "core", "bus", "iface";
+		};
+	};
+
 	cpu-pmu {
 		compatible = "qcom,krait-pmu";
 		interrupts = <1 7 0xf04>;
-- 
1.9.1

[Patch v5 8/8] arm64: dts: msm8916: Add SCM firmware node

[Andy Gross]

This adds the devicetree node for the SCM firmware.

Acked-by: Bjorn Andersson <bjorn.andersson@linaro.org>
Signed-off-by: Andy Gross <andy.gross@linaro.org>
---
 arch/arm64/boot/dts/qcom/msm8916.dtsi | 8 ++++++++
 1 file changed, 8 insertions(+)

diff --git a/arch/arm64/boot/dts/qcom/msm8916.dtsi b/arch/arm64/boot/dts/qcom/msm8916.dtsi
index 9681200..01c7b0c 100644
--- a/arch/arm64/boot/dts/qcom/msm8916.dtsi
+++ b/arch/arm64/boot/dts/qcom/msm8916.dtsi
@@ -122,6 +122,14 @@
 		hwlocks = <&tcsr_mutex 3>;
 	};
 
+	firmware {
+		scm {
+			compatible = "qcom,scm";
+			clocks = <&gcc GCC_CRYPTO_CLK>, <&gcc GCC_CRYPTO_AXI_CLK>, <&gcc GCC_CRYPTO_AHB_CLK>;
+			clock-names = "core", "bus", "iface";
+		};
+	};
+
 	soc: soc {
 		#address-cells = <1>;
 		#size-cells = <1>;
-- 
1.9.1

Re: [Patch v5 1/8] dt/bindings: firmware: Add Qualcomm SCM binding

[Bjorn Andersson]

On Thu 12 May 20:46 PDT 2016, Andy Gross wrote:

> This patch adds the device tree support for the Qualcomm SCM firmware.
> 
> Signed-off-by: Andy Gross <andy.gross@linaro.org>

Acked-by: Bjorn Andersson <bjorn.andersson@linaro.org>

Regards,
Bjorn

Re: [Patch v5 2/8] firmware: qcom: scm: Convert SCM to platform driver

[Bjorn Andersson]

On Thu 12 May 20:46 PDT 2016, Andy Gross wrote:

> This patch converts the Qualcomm SCM firmware driver into a platform
> driver.
> 
> Signed-off-by: Andy Gross <andy.gross@linaro.org>

Acked-by: Bjorn Andersson <bjorn.andersson@linaro.org>

Regards,
Bjorn

Re: [Patch v5 3/8] firmware: qcom: scm: Use atomic SCM for cold boot

[Bjorn Andersson]

On Thu 12 May 20:46 PDT 2016, Andy Gross wrote:

> This patch changes the cold_set_boot_addr function to use atomic SCM
> calls.  cold_set_boot_addr required adding qcom_scm_call_atomic2 to
> support the two arguments going to the smc call.  Using atomic removes
> the need for memory allocation and instead places all arguments in
> registers.
> 
> Signed-off-by: Andy Gross <andy.gross@linaro.org>

Acked-by: Bjorn Andersson <bjorn.andersson@linaro.org>

Regards,
Bjorn

Re: [Patch v5 5/8] firmware: qcom: scm: Convert to streaming DMA APIS

[Bjorn Andersson]

On Thu 12 May 20:46 PDT 2016, Andy Gross wrote:

> This patch converts the Qualcomm SCM driver to use the streaming DMA APIs
> for communication buffers.
> 
> Signed-off-by: Andy Gross <andy.gross@linaro.org>
> ---
>  drivers/firmware/qcom_scm-32.c | 189 +++++++++++------------------------------
>  drivers/firmware/qcom_scm.c    |   6 +-
>  drivers/firmware/qcom_scm.h    |  10 ++-
>  3 files changed, 59 insertions(+), 146 deletions(-)
> 
> diff --git a/drivers/firmware/qcom_scm-32.c b/drivers/firmware/qcom_scm-32.c
[..]
> +static int qcom_scm_call(struct device *dev, 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;
> +	size_t alloc_len = sizeof(*cmd) + cmd_len + sizeof(*rsp) + resp_len;
> +	dma_addr_t cmd_phys;
>  
> -	cmd = alloc_qcom_scm_command(cmd_len, resp_len);
> +	cmd = kzalloc(PAGE_ALIGN(alloc_len), GFP_KERNEL);
>  	if (!cmd)
>  		return -ENOMEM;
>  
> +	cmd->len = cpu_to_le32(alloc_len);
> +	cmd->buf_offset = cpu_to_le32(sizeof(*cmd));
> +	cmd->resp_hdr_offset = cpu_to_le32(sizeof(*cmd) + cmd_len);
> +
>  	cmd->id = cpu_to_le32((svc_id << 10) | cmd_id);
>  	if (cmd_buf)
> -		memcpy(qcom_scm_get_command_buffer(cmd), cmd_buf, cmd_len);
> +		memcpy(cmd->buf, cmd_buf, cmd_len);
> +
> +	rsp = (void *)cmd->buf + le32_to_cpu(cmd->resp_hdr_offset);

I believe resp_hdr_offset counts from the beginning of the buffer and
that this therefor is supposed to be:

	rsp = (void *)cmd + le32_to_cpu(cmd->resp_hdr_offset);

With that corrected, feel free to add:

Reviewed-by: Bjorn Andersson <bjorn.andersson@linaro.org>

Regards,
Bjorn

Re: [Patch v5 6/8] firmware: qcom: scm: Add support for ARM64 SoCs

[Bjorn Andersson]

On Thu 12 May 20:46 PDT 2016, Andy Gross wrote:

> From: Kumar Gala <galak@codeaurora.org>
> 
> Add an implementation of the SCM interface that works on ARM64 SoCs.  This
> is used by things like determine if we have HDCP support or not on the
> system.
> 
> Signed-off-by: Kumar Gala <galak@codeaurora.org>
> Signed-off-by: Andy Gross <andy.gross@linaro.org>

Reviewed-by: Bjorn Andersson <bjorn.andersson@linaro.org>

Regards,
Bjorn

Re: [Patch v5 5/8] firmware: qcom: scm: Convert to streaming DMA APIS

[Andy Gross]

On Fri, May 13, 2016 at 04:48:52PM -0700, Bjorn Andersson wrote:
> > +	cmd->len = cpu_to_le32(alloc_len);
> > +	cmd->buf_offset = cpu_to_le32(sizeof(*cmd));
> > +	cmd->resp_hdr_offset = cpu_to_le32(sizeof(*cmd) + cmd_len);
> > +
> >  	cmd->id = cpu_to_le32((svc_id << 10) | cmd_id);
> >  	if (cmd_buf)
> > -		memcpy(qcom_scm_get_command_buffer(cmd), cmd_buf, cmd_len);
> > +		memcpy(cmd->buf, cmd_buf, cmd_len);
> > +
> > +	rsp = (void *)cmd->buf + le32_to_cpu(cmd->resp_hdr_offset);
> 
> I believe resp_hdr_offset counts from the beginning of the buffer and
> that this therefor is supposed to be:
> 
> 	rsp = (void *)cmd + le32_to_cpu(cmd->resp_hdr_offset);
> 
> With that corrected, feel free to add:
> 
> Reviewed-by: Bjorn Andersson <bjorn.andersson@linaro.org>

I'll fix that up.  Thanks for the review.


Andy

Re: [Patch v5 1/8] dt/bindings: firmware: Add Qualcomm SCM binding

[Rob Herring]

On Thu, May 12, 2016 at 10:46:54PM -0500, Andy Gross wrote:
> This patch adds the device tree support for the Qualcomm SCM firmware.
> 
> Signed-off-by: Andy Gross <andy.gross@linaro.org>
> ---
>  .../devicetree/bindings/firmware/qcom,scm.txt      | 28 ++++++++++++++++++++++
>  1 file changed, 28 insertions(+)
>  create mode 100644 Documentation/devicetree/bindings/firmware/qcom,scm.txt

Acked-by: Rob Herring <robh@kernel.org>

Re: [Patch v5 5/8] firmware: qcom: scm: Convert to streaming DMA APIS

[Kevin Hilman]

Hi Andy,

On Thu, May 12, 2016 at 8:46 PM, Andy Gross <andy.gross@linaro.org> wrote:
> This patch converts the Qualcomm SCM driver to use the streaming DMA APIs
> for communication buffers.
>
> Signed-off-by: Andy Gross <andy.gross@linaro.org>

This patch has landed in linux-next in the form of commit a551c3dbd689
(firmware: qcom: scm: Convert to streaming DMA APIS), and kernelci.org
found some boot breakage in next-20160523 on apq8064[1] which was
bisected down to this commit.

I reverted this commit on top of next-20160523 and it no longer
builds, so I didn't validate if things boot again with this patch
reverted.

Kevin

[1] https://kernelci.org/boot/all/job/next/kernel/next-20160523/

Re: [Patch v5 5/8] firmware: qcom: scm: Convert to streaming DMA APIS

[Andy Gross]

On 23 May 2016 at 14:26, Kevin Hilman <khilman@kernel.org> wrote:
> Hi Andy,
>
> On Thu, May 12, 2016 at 8:46 PM, Andy Gross <andy.gross@linaro.org> wrote:
>> This patch converts the Qualcomm SCM driver to use the streaming DMA APIs
>> for communication buffers.
>>
>> Signed-off-by: Andy Gross <andy.gross@linaro.org>
>
> This patch has landed in linux-next in the form of commit a551c3dbd689
> (firmware: qcom: scm: Convert to streaming DMA APIS), and kernelci.org
> found some boot breakage in next-20160523 on apq8064[1] which was
> bisected down to this commit.
>
> I reverted this commit on top of next-20160523 and it no longer
> builds, so I didn't validate if things boot again with this patch
> reverted.

Ouch I missed this failure.  I'll investigate and get it fixed.

Thanks!

Andy

Re: [Patch v5 5/8] firmware: qcom: scm: Convert to streaming DMA APIS

[Andy Gross]

On Mon, May 23, 2016 at 04:02:06PM -0500, Andy Gross wrote:
> On 23 May 2016 at 14:26, Kevin Hilman <khilman@kernel.org> wrote:
> > Hi Andy,
> >
> > On Thu, May 12, 2016 at 8:46 PM, Andy Gross <andy.gross@linaro.org> wrote:
> >> This patch converts the Qualcomm SCM driver to use the streaming DMA APIs
> >> for communication buffers.
> >>
> >> Signed-off-by: Andy Gross <andy.gross@linaro.org>
> >
> > This patch has landed in linux-next in the form of commit a551c3dbd689
> > (firmware: qcom: scm: Convert to streaming DMA APIS), and kernelci.org
> > found some boot breakage in next-20160523 on apq8064[1] which was
> > bisected down to this commit.
> >
> > I reverted this commit on top of next-20160523 and it no longer
> > builds, so I didn't validate if things boot again with this patch
> > reverted.
> 
> Ouch I missed this failure.  I'll investigate and get it fixed.

So the root cause was the fact that the DFAB clock required by the SCM is an RPM
clock.  That support isn't present yet in the kernel, so SCM probe fails.

The core clock is really only accessed so that we can bump the clock on it up to
the max for performance.  As such, I'll make it optional in the platform code.

This does bring up the issue of probe defer causing issues with the spm driver,
as it calls set_warm_boot_addr.  That may have to be addressed, but is probably
a problem best fixed in the spm.

Regards,

Andy

Re: [Patch v5 5/8] firmware: qcom: scm: Convert to streaming DMA APIS

[Kevin Hilman]

Andy Gross <andy.gross@linaro.org> writes:

> On Mon, May 23, 2016 at 04:02:06PM -0500, Andy Gross wrote:
>> On 23 May 2016 at 14:26, Kevin Hilman <khilman@kernel.org> wrote:
>> > Hi Andy,
>> >
>> > On Thu, May 12, 2016 at 8:46 PM, Andy Gross <andy.gross@linaro.org> wrote:
>> >> This patch converts the Qualcomm SCM driver to use the streaming DMA APIs
>> >> for communication buffers.
>> >>
>> >> Signed-off-by: Andy Gross <andy.gross@linaro.org>
>> >
>> > This patch has landed in linux-next in the form of commit a551c3dbd689
>> > (firmware: qcom: scm: Convert to streaming DMA APIS), and kernelci.org
>> > found some boot breakage in next-20160523 on apq8064[1] which was
>> > bisected down to this commit.
>> >
>> > I reverted this commit on top of next-20160523 and it no longer
>> > builds, so I didn't validate if things boot again with this patch
>> > reverted.
>> 
>> Ouch I missed this failure.  I'll investigate and get it fixed.
>
> So the root cause was the fact that the DFAB clock required by the SCM is an RPM
> clock.  That support isn't present yet in the kernel, so SCM probe fails.
>
> The core clock is really only accessed so that we can bump the clock on it up to
> the max for performance.  As such, I'll make it optional in the platform code.
>
> This does bring up the issue of probe defer causing issues with the spm driver,
> as it calls set_warm_boot_addr.  That may have to be addressed, but is probably
> a problem best fixed in the spm.

Nice, thanks for the explanation.

Is there a patch somewhere you'd like me to test? or were you able to
dust off an 8064 platform for testing?

Kevin

Re: [Patch v5 5/8] firmware: qcom: scm: Convert to streaming DMA APIS

[Andy Gross]

On 25 May 2016 at 15:50, Kevin Hilman <khilman@baylibre.com> wrote:
> Andy Gross <andy.gross@linaro.org> writes:
>
>> On Mon, May 23, 2016 at 04:02:06PM -0500, Andy Gross wrote:
>>> On 23 May 2016 at 14:26, Kevin Hilman <khilman@kernel.org> wrote:
>>> > Hi Andy,
>>> >
>>> > On Thu, May 12, 2016 at 8:46 PM, Andy Gross <andy.gross@linaro.org> wrote:
>>> >> This patch converts the Qualcomm SCM driver to use the streaming DMA APIs
>>> >> for communication buffers.
>>> >>
>>> >> Signed-off-by: Andy Gross <andy.gross@linaro.org>
>>> >
>>> > This patch has landed in linux-next in the form of commit a551c3dbd689
>>> > (firmware: qcom: scm: Convert to streaming DMA APIS), and kernelci.org
>>> > found some boot breakage in next-20160523 on apq8064[1] which was
>>> > bisected down to this commit.
>>> >
>>> > I reverted this commit on top of next-20160523 and it no longer
>>> > builds, so I didn't validate if things boot again with this patch
>>> > reverted.
>>>
>>> Ouch I missed this failure.  I'll investigate and get it fixed.
>>
>> So the root cause was the fact that the DFAB clock required by the SCM is an RPM
>> clock.  That support isn't present yet in the kernel, so SCM probe fails.
>>
>> The core clock is really only accessed so that we can bump the clock on it up to
>> the max for performance.  As such, I'll make it optional in the platform code.
>>
>> This does bring up the issue of probe defer causing issues with the spm driver,
>> as it calls set_warm_boot_addr.  That may have to be addressed, but is probably
>> a problem best fixed in the spm.
>
> Nice, thanks for the explanation.
>
> Is there a patch somewhere you'd like me to test? or were you able to
> dust off an 8064 platform for testing?

I had the ifc6410, but it took me a while to find the darn 5V supply.
The docs lie when they say 5V-12V.  In any case, I threw a patch on
top of next to fix this.  I need to amend my set of patches to include
these changes and add the DT information for the 8064.

Regards,

Andy

Re: [Patch v5 2/8] firmware: qcom: scm: Convert SCM to platform driver

[Stephen Boyd]

On 05/12, Andy Gross wrote:
> This patch converts the Qualcomm SCM firmware driver into a platform
> driver.

And introduces clk enable/disable + rate setting logic because
the firmware uses crypto clks during some scm calls.

> 
> +
> +static struct platform_driver qcom_scm_driver = {
> +	.driver = {
> +		.name	= "qcom_scm",
> +		.of_match_table = qcom_scm_dt_match,
> +	},
> +	.probe = qcom_scm_probe,
> +};
> +
> +static int __init qcom_scm_init(void)
> +{
> +	struct device_node *np, *parent;
> +	int ret;
> +
> +	np = of_find_matching_node_and_match(NULL, qcom_scm_dt_match, NULL);

Why can't we find a node called "firmware" in the root of the
tree? That would be clearer what's going on here, and avoid the
need to do the of_get_next_parent() dance, which just feels
awkward.

> +
> +	if (!np)
> +		return -ENODEV;
> +
> +	parent = of_get_next_parent(np);
> +	ret = of_platform_populate(parent, qcom_scm_dt_match, NULL, NULL);
> +
> +	of_node_put(parent);
> +
> +	if (ret)
> +		return ret;
> +
> +	return platform_driver_register(&qcom_scm_driver);
> +}

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

Re: [Patch v5 1/8] dt/bindings: firmware: Add Qualcomm SCM binding

[Stephen Boyd]

On 05/12, Andy Gross wrote:
> This patch adds the device tree support for the Qualcomm SCM firmware.
> 
> Signed-off-by: Andy Gross <andy.gross@linaro.org>
> ---

Reviewed-by: Stephen Boyd <sboyd@codeaurora.org>

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

Re: [Patch v5 3/8] firmware: qcom: scm: Use atomic SCM for cold boot

[Stephen Boyd]

On 05/12, Andy Gross wrote:
> This patch changes the cold_set_boot_addr function to use atomic SCM
> calls.  cold_set_boot_addr required adding qcom_scm_call_atomic2 to
> support the two arguments going to the smc call.  Using atomic removes
> the need for memory allocation and instead places all arguments in
> registers.
> 
> Signed-off-by: Andy Gross <andy.gross@linaro.org>
> ---

Reviewed-by: Stephen Boyd <sboyd@codeaurora.org>

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

Re: [Patch v5 6/8] firmware: qcom: scm: Add support for ARM64 SoCs

[Stephen Boyd]

On 05/12, Andy Gross wrote:
> +
> +#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) & 0x3) << 4) | \
> +			   (((b) & 0x3) << 6) | \
> +			   (((c) & 0x3) << 8) | \
> +			   (((d) & 0x3) << 10) | \
> +			   (((e) & 0x3) << 12) | \
> +			   (((f) & 0x3) << 14) | \
> +			   (((g) & 0x3) << 16) | \
> +			   (((h) & 0x3) << 18) | \
> +			   (((i) & 0x3) << 20) | \
> +			   (((j) & 0x3) << 22) | \
> +			   (num & 0xf))

Parenthesis around num?

> +
> +#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
> + * @res:	The values returned by the secure syscall
> + */
> +struct qcom_scm_desc {
> +	u32 arginfo;
> +	u64 args[MAX_QCOM_SCM_ARGS];
> +	struct arm_smccc_res res;
> +};

If we split the res from the descriptor structure we could make
the qcom_scm_desc const in qcom_scm_call(). We would have to add
another argument to the function though, not sure if this is a
big win idea, but just an idea to keep things "safer".

> +
> +static u64 qcom_smccc_convention = -1;
> +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 N_REGISTER_ARGS (MAX_QCOM_SCM_ARGS - N_EXT_QCOM_SCM_ARGS + 1)
> +
> +/**
> + * qcom_scm_call() - Invoke a syscall in the secure world
> + * @dev:	device
> + * @svc_id:	service identifier
> + * @cmd_id:	command identifier
> + * @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.
> +*/
> +static int qcom_scm_call(struct device *dev, u32 svc_id, u32 cmd_id,
> +			 struct qcom_scm_desc *desc)
> +{
> +	int arglen = desc->arginfo & 0xf;
> +	int retry_count = 0, i;
> +	u32 fn_id = QCOM_SCM_FNID(svc_id, cmd_id);
> +	u64 cmd, x5 = desc->args[FIRST_EXT_ARG_IDX];
> +	dma_addr_t args_phys = 0;
> +	void *args_virt = NULL;
> +	size_t alloc_len;
> +
> +	if (unlikely(arglen > N_REGISTER_ARGS)) {
> +		alloc_len = N_EXT_QCOM_SCM_ARGS * sizeof(u64);
> +		args_virt = kzalloc(PAGE_ALIGN(alloc_len), GFP_KERNEL);
> +
> +		if (!args_virt)
> +			return qcom_scm_remap_error(-ENOMEM);

Just return -ENOMEM here?

> +
> +		if (qcom_smccc_convention == ARM_SMCCC_SMC_32) {
> +			__le32 *args = args_virt;
> +
> +			for (i = 0; i < N_EXT_QCOM_SCM_ARGS; i++)
> +				args[i] = cpu_to_le32(desc->args[i +
> +						      FIRST_EXT_ARG_IDX]);
> +		} else {
> +			__le64 *args = args_virt;
> +
> +			for (i = 0; i < N_EXT_QCOM_SCM_ARGS; i++)
> +				args[i] = cpu_to_le64(desc->args[i +
> +						      FIRST_EXT_ARG_IDX]);
> +		}
> +
> +		args_phys = dma_map_single(dev, args_virt, alloc_len,
> +					   DMA_TO_DEVICE);
> +
> +		if (dma_mapping_error(dev, args_phys)) {
> +			kfree(args_virt);
> +			return qcom_scm_remap_error(-ENOMEM);

Just return -ENOMEM here?

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

Re: [Patch v5 7/8] dts: qcom: apq8084: Add SCM firmware node

[Stephen Boyd]

On 05/12, Andy Gross wrote:
> This patch adds the firmware node for the SCM
> 
> Acked-by: Bjorn Andersson <bjorn.andersson@linaro.org>
> Signed-off-by: Andy Gross <andy.gross@linaro.org>
> ---

Reviewed-by: Stephen Boyd <sboyd@codeaurora.org>

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

Re: [Patch v5 8/8] arm64: dts: msm8916: Add SCM firmware node

[Stephen Boyd]

On 05/12, Andy Gross wrote:
> This adds the devicetree node for the SCM firmware.
> 
> Acked-by: Bjorn Andersson <bjorn.andersson@linaro.org>
> Signed-off-by: Andy Gross <andy.gross@linaro.org>
> ---

Reviewed-by: Stephen Boyd <sboyd@codeaurora.org>

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

Re: [Patch v5 5/8] firmware: qcom: scm: Convert to streaming DMA APIS

[Stephen Boyd]

On 05/12, Andy Gross wrote:
> This patch converts the Qualcomm SCM driver to use the streaming DMA APIs
> for communication buffers.

Yes, but why?

> 
> Signed-off-by: Andy Gross <andy.gross@linaro.org>
> ---
>  drivers/firmware/qcom_scm-32.c | 189 +++++++++++------------------------------
>  drivers/firmware/qcom_scm.c    |   6 +-
>  drivers/firmware/qcom_scm.h    |  10 ++-
>  3 files changed, 59 insertions(+), 146 deletions(-)
> 
> diff --git a/drivers/firmware/qcom_scm-32.c b/drivers/firmware/qcom_scm-32.c
> index 4388d13..e92bf7a 100644
> --- a/drivers/firmware/qcom_scm-32.c
> +++ b/drivers/firmware/qcom_scm-32.c
> @@ -64,11 +63,11 @@ static DEFINE_MUTEX(qcom_scm_lock);
>   *
>   *	------------------- <--- struct qcom_scm_command
>   *	| command header  |
> - *	------------------- <--- qcom_scm_get_command_buffer()
> + *	------------------- <--- buf[0]
>   *	| command buffer  |
> - *	------------------- <--- struct qcom_scm_response and
> - *	| response header |      qcom_scm_command_to_response()
> - *	------------------- <--- qcom_scm_get_response_buffer()
> + *	------------------- <--- struct qcom_scm_response
> + *	| response header |
> + *	-------------------

You don't like my convenience functions? :) I always thought
qcom_scm_get_response_buffer() read better than

(void *)cmd->buf + le32_to_cpu(cmd->resp_hdr_offset);

>   *	| response buffer |
>   *	-------------------
>   *
> @@ -96,79 +95,7 @@ struct qcom_scm_response {
>  	__le32 is_complete;
>  };
>  
> -
> -/**
> - * 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);
> -}

At the least the diff would be more concentrated on what really
is happening in this patch if we left these functions as is.

> -
> -static u32 smc(u32 cmd_addr)
> +static u32 smc(dma_addr_t cmd_addr)

Please leave this as u32, the interface doesn't support anything
wider here so we shouldn't let this change on LPAE.

>  {
>  	int context_id;
>  	register u32 r0 asm("r0") = 1;
> @@ -192,45 +119,9 @@ static u32 smc(u32 cmd_addr)
>  	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.
> -	 */
> -	secure_flush_area(cmd, cmd->len);

Yay we should delete secure_flush_area() too.

> -
> -	ret = smc(cmd_addr);
> -	if (ret < 0)
> -		ret = qcom_scm_remap_error(ret);
> -
> -	return ret;
> -}
> -
> -
> @@ -143,7 +143,7 @@ int qcom_scm_hdcp_req(struct qcom_scm_hdcp_req *req, u32 req_cnt, u32 *resp)
>  	if (ret)
>  		return ret;
>  
> -	ret = __qcom_scm_hdcp_req(req, req_cnt, resp);
> +	ret = __qcom_scm_hdcp_req(__scm->dev, req, req_cnt, resp);

Hmm maybe we should pass __scm instead of dev? Is there any
advantage to that?

>  	qcom_scm_clk_disable();
>  	return ret;
>  }

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

Re: [Patch v5 2/8] firmware: qcom: scm: Convert SCM to platform driver

[Andy Gross]

On Thu, Jun 02, 2016 at 03:14:24PM -0700, Stephen Boyd wrote:
> On 05/12, Andy Gross wrote:
> > This patch converts the Qualcomm SCM firmware driver into a platform
> > driver.
> 
> And introduces clk enable/disable + rate setting logic because
> the firmware uses crypto clks during some scm calls.

I'll make the commit message more descriptive of the change.

> > 
> > +
> > +static struct platform_driver qcom_scm_driver = {
> > +	.driver = {
> > +		.name	= "qcom_scm",
> > +		.of_match_table = qcom_scm_dt_match,
> > +	},
> > +	.probe = qcom_scm_probe,
> > +};
> > +
> > +static int __init qcom_scm_init(void)
> > +{
> > +	struct device_node *np, *parent;
> > +	int ret;
> > +
> > +	np = of_find_matching_node_and_match(NULL, qcom_scm_dt_match, NULL);
> 
> Why can't we find a node called "firmware" in the root of the
> tree? That would be clearer what's going on here, and avoid the
> need to do the of_get_next_parent() dance, which just feels
> awkward.

Well I had reworked it previously to avoid calling of_platform_populate on
anyone with a firmware node.  I guess what I can do is find the firmware node
and then only populate if I get a match starting from the firmware node itself.
That satisfies both comments.

> > +
> > +	if (!np)
> > +		return -ENODEV;
> > +
> > +	parent = of_get_next_parent(np);
> > +	ret = of_platform_populate(parent, qcom_scm_dt_match, NULL, NULL);
> > +
> > +	of_node_put(parent);
> > +
> > +	if (ret)
> > +		return ret;
> > +
> > +	return platform_driver_register(&qcom_scm_driver);
> > +}
> 
> -- 
> Qualcomm Innovation Center, Inc. is a member of Code Aurora Forum,
> a Linux Foundation Collaborative Project
> --
> To unsubscribe from this list: send the line "unsubscribe linux-arm-msm" in
> the body of a message to majordomo@vger.kernel.org
> More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [Patch v5 6/8] firmware: qcom: scm: Add support for ARM64 SoCs

[Andy Gross]

On Thu, Jun 02, 2016 at 03:28:40PM -0700, Stephen Boyd wrote:
> On 05/12, Andy Gross wrote:
> > +
> > +#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) & 0x3) << 4) | \
> > +			   (((b) & 0x3) << 6) | \
> > +			   (((c) & 0x3) << 8) | \
> > +			   (((d) & 0x3) << 10) | \
> > +			   (((e) & 0x3) << 12) | \
> > +			   (((f) & 0x3) << 14) | \
> > +			   (((g) & 0x3) << 16) | \
> > +			   (((h) & 0x3) << 18) | \
> > +			   (((i) & 0x3) << 20) | \
> > +			   (((j) & 0x3) << 22) | \
> > +			   (num & 0xf))
> 
> Parenthesis around num?

To be more precise, yes.


> > +
> > +#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
> > + * @res:	The values returned by the secure syscall
> > + */
> > +struct qcom_scm_desc {
> > +	u32 arginfo;
> > +	u64 args[MAX_QCOM_SCM_ARGS];
> > +	struct arm_smccc_res res;
> > +};
> 
> If we split the res from the descriptor structure we could make
> the qcom_scm_desc const in qcom_scm_call(). We would have to add
> another argument to the function though, not sure if this is a
> big win idea, but just an idea to keep things "safer".

I like that idea.  The arginfo and args are immutable so this really locks down
the interface.  I'll try it out and see how it turns out.

> > +
> > +static u64 qcom_smccc_convention = -1;
> > +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 N_REGISTER_ARGS (MAX_QCOM_SCM_ARGS - N_EXT_QCOM_SCM_ARGS + 1)
> > +
> > +/**
> > + * qcom_scm_call() - Invoke a syscall in the secure world
> > + * @dev:	device
> > + * @svc_id:	service identifier
> > + * @cmd_id:	command identifier
> > + * @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.
> > +*/
> > +static int qcom_scm_call(struct device *dev, u32 svc_id, u32 cmd_id,
> > +			 struct qcom_scm_desc *desc)
> > +{
> > +	int arglen = desc->arginfo & 0xf;
> > +	int retry_count = 0, i;
> > +	u32 fn_id = QCOM_SCM_FNID(svc_id, cmd_id);
> > +	u64 cmd, x5 = desc->args[FIRST_EXT_ARG_IDX];
> > +	dma_addr_t args_phys = 0;
> > +	void *args_virt = NULL;
> > +	size_t alloc_len;
> > +
> > +	if (unlikely(arglen > N_REGISTER_ARGS)) {
> > +		alloc_len = N_EXT_QCOM_SCM_ARGS * sizeof(u64);
> > +		args_virt = kzalloc(PAGE_ALIGN(alloc_len), GFP_KERNEL);
> > +
> > +		if (!args_virt)
> > +			return qcom_scm_remap_error(-ENOMEM);
> 
> Just return -ENOMEM here?

Ok.  That would make it easier to follow.

> > +
> > +		if (qcom_smccc_convention == ARM_SMCCC_SMC_32) {
> > +			__le32 *args = args_virt;
> > +
> > +			for (i = 0; i < N_EXT_QCOM_SCM_ARGS; i++)
> > +				args[i] = cpu_to_le32(desc->args[i +
> > +						      FIRST_EXT_ARG_IDX]);
> > +		} else {
> > +			__le64 *args = args_virt;
> > +
> > +			for (i = 0; i < N_EXT_QCOM_SCM_ARGS; i++)
> > +				args[i] = cpu_to_le64(desc->args[i +
> > +						      FIRST_EXT_ARG_IDX]);
> > +		}
> > +
> > +		args_phys = dma_map_single(dev, args_virt, alloc_len,
> > +					   DMA_TO_DEVICE);
> > +
> > +		if (dma_mapping_error(dev, args_phys)) {
> > +			kfree(args_virt);
> > +			return qcom_scm_remap_error(-ENOMEM);
> 
> Just return -ENOMEM here?

Ok.  That would make it easier to follow.

Re: [Patch v5 5/8] firmware: qcom: scm: Convert to streaming DMA APIS

[Andy Gross]

On Thu, Jun 02, 2016 at 04:26:02PM -0700, Stephen Boyd wrote:
> On 05/12, Andy Gross wrote:
> > This patch converts the Qualcomm SCM driver to use the streaming DMA APIs
> > for communication buffers.
> 
> Yes, but why?

This was done so that we can remove the secure_flush mechanism, as we are the
only consumer (that I could find).  Using the DMA APIs would match the scm-64 so
it would be symmetric.  I'll add this to the commit message to give more reason
why the changes are being made.

> > 
> > Signed-off-by: Andy Gross <andy.gross@linaro.org>
> > ---
> >  drivers/firmware/qcom_scm-32.c | 189 +++++++++++------------------------------
> >  drivers/firmware/qcom_scm.c    |   6 +-
> >  drivers/firmware/qcom_scm.h    |  10 ++-
> >  3 files changed, 59 insertions(+), 146 deletions(-)
> > 
> > diff --git a/drivers/firmware/qcom_scm-32.c b/drivers/firmware/qcom_scm-32.c
> > index 4388d13..e92bf7a 100644
> > --- a/drivers/firmware/qcom_scm-32.c
> > +++ b/drivers/firmware/qcom_scm-32.c
> > @@ -64,11 +63,11 @@ static DEFINE_MUTEX(qcom_scm_lock);
> >   *
> >   *	------------------- <--- struct qcom_scm_command
> >   *	| command header  |
> > - *	------------------- <--- qcom_scm_get_command_buffer()
> > + *	------------------- <--- buf[0]
> >   *	| command buffer  |
> > - *	------------------- <--- struct qcom_scm_response and
> > - *	| response header |      qcom_scm_command_to_response()
> > - *	------------------- <--- qcom_scm_get_response_buffer()
> > + *	------------------- <--- struct qcom_scm_response
> > + *	| response header |
> > + *	-------------------
> 
> You don't like my convenience functions? :) I always thought
> qcom_scm_get_response_buffer() read better than
> 
> (void *)cmd->buf + le32_to_cpu(cmd->resp_hdr_offset);

If it was used in more than one function, yes.  But I felt that single use
functions were kind of a waste.  But this is in the eye of the beholder.

> 
> >   *	| response buffer |
> >   *	-------------------
> >   *
> > @@ -96,79 +95,7 @@ struct qcom_scm_response {
> >  	__le32 is_complete;
> >  };
> >  
> > -
> > -/**
> > - * 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);
> > -}
> 
> At the least the diff would be more concentrated on what really
> is happening in this patch if we left these functions as is.

Ok.  I'll run that and see how it turns out.  

> > -
> > -static u32 smc(u32 cmd_addr)
> > +static u32 smc(dma_addr_t cmd_addr)
> 
> Please leave this as u32, the interface doesn't support anything
> wider here so we shouldn't let this change on LPAE.

Fair point.  I hadn't considered that.  Thanks for catching that.

> >  {
> >  	int context_id;
> >  	register u32 r0 asm("r0") = 1;
> > @@ -192,45 +119,9 @@ static u32 smc(u32 cmd_addr)
> >  	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.
> > -	 */
> > -	secure_flush_area(cmd, cmd->len);
> 
> Yay we should delete secure_flush_area() too.

Yes I was going to do a follow on to remove that.  I opted to not add this to
the set of patches.

> > -
> > -	ret = smc(cmd_addr);
> > -	if (ret < 0)
> > -		ret = qcom_scm_remap_error(ret);
> > -
> > -	return ret;
> > -}
> > -
> > -
> > @@ -143,7 +143,7 @@ int qcom_scm_hdcp_req(struct qcom_scm_hdcp_req *req, u32 req_cnt, u32 *resp)
> >  	if (ret)
> >  		return ret;
> >  
> > -	ret = __qcom_scm_hdcp_req(req, req_cnt, resp);
> > +	ret = __qcom_scm_hdcp_req(__scm->dev, req, req_cnt, resp);
> 
> Hmm maybe we should pass __scm instead of dev? Is there any
> advantage to that?

If we needed access to something in the scm other than the device, yes.  If we
passed scm instead, we might be able to remove the singleton usage.  I'll look
and see if it makes sense.  Otherwise i'd leave the passing of the device.