[Patch v2 0/8] Qualcomm SCM Rework

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

Change 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: Generalize shared error map
  firmware: qcom: scm: Add memory allocation API
  firmware: qcom: scm: Use atomic SCM for cold boot
  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/Kconfig.platforms                       |   1 +
 arch/arm64/boot/dts/qcom/msm8916.dtsi              |   8 +
 drivers/firmware/qcom_scm-32.c                     |  68 +++-----
 drivers/firmware/qcom_scm-64.c                     | 187 ++++++++++++++++++++-
 drivers/firmware/qcom_scm.c                        | 181 +++++++++++++++++++-
 drivers/firmware/qcom_scm.h                        |  25 +++
 8 files changed, 448 insertions(+), 58 deletions(-)
 create mode 100644 Documentation/devicetree/bindings/firmware/qcom,scm.txt

-- 
1.9.1

[Patch v2 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..a679a87
--- /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
+ * "qcom,scm-apq8084" for APQ8084
+ * "qcom,scm-msm8916" for MSM8916
+ * "qcom,scm-msm8974" for MSM8974
+- clocks: One to three clocks may be required based on compatible.
+ * Only core clock required for "qcom,scm-apq8064"
+ * Core, iface, and bus clocks required for all other compatibles.
+- 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-msm8916";
+			clocks = <&gcc GCC_CRYPTO_CLK> , <&gcc GCC_CRYPTO_AXI_CLK>, <&gcc GCC_CRYPTO_AHB_CLK>;
+			clock-names = "core", "bus", "iface";
+		};
+	};
-- 
1.9.1

[Patch v2 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>
---
 arch/arm64/Kconfig.platforms |   1 +
 drivers/firmware/qcom_scm.c  | 163 ++++++++++++++++++++++++++++++++++++++++---
 2 files changed, 155 insertions(+), 9 deletions(-)

diff --git a/arch/arm64/Kconfig.platforms b/arch/arm64/Kconfig.platforms
index efa77c1..6f0876f 100644
--- a/arch/arm64/Kconfig.platforms
+++ b/arch/arm64/Kconfig.platforms
@@ -76,6 +76,7 @@ config ARCH_MVEBU
 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/qcom_scm.c b/drivers/firmware/qcom_scm.c
index 45c008d..d4e9145 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,104 @@ 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);
+
+/**
+ * qcom_scm_is_available() - Checks if SCM is available
+ */
+bool qcom_scm_is_available(void)
+{
+	return !!__scm;
+}
+EXPORT_SYMBOL(qcom_scm_is_available);
+
+static int qcom_scm_probe(struct platform_device *pdev)
+{
+	struct qcom_scm *scm;
+	long rate;
+	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-apq8064")) {
+		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 */
+	rate = clk_round_rate(scm->core_clk, INT_MAX);
+	ret = clk_set_rate(scm->core_clk, rate);
+	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-apq8084",},
+	{ .compatible = "qcom,scm-msm8916",},
+	{ .compatible = "qcom,scm-msm8974",},
+	{},
+};
+
+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,
+};
+
+builtin_platform_driver(qcom_scm_driver);
+
+static int __init qcom_scm_init(void)
+{
+	struct device_node *np;
+
+	np = of_find_node_by_name(NULL, "firmware");
+	if (!np)
+		return -ENODEV;
+
+	return of_platform_populate(np, qcom_scm_dt_match, NULL, NULL);
+
+}
+
+arch_initcall(qcom_scm_init);
+
+MODULE_DESCRIPTION("Qualcomm SCM driver");
+MODULE_LICENSE("GPL v2");
-- 
1.9.1

[Patch v2 3/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.

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 0883292..9e3dc2f 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 v2 4/8] firmware: qcom: scm: Add memory allocation API

[Andy Gross]

This patch adds APIs for the scm-32 and scm-64 to use for coherent memory
allocation.

Signed-off-by: Andy Gross <andy.gross@linaro.org>
---
 drivers/firmware/qcom_scm.c | 16 ++++++++++++++++
 drivers/firmware/qcom_scm.h |  4 ++++
 2 files changed, 20 insertions(+)

diff --git a/drivers/firmware/qcom_scm.c b/drivers/firmware/qcom_scm.c
index d4e9145..8f78938 100644
--- a/drivers/firmware/qcom_scm.c
+++ b/drivers/firmware/qcom_scm.c
@@ -20,6 +20,7 @@
 #include <linux/of.h>
 #include <linux/of_platform.h>
 #include <linux/clk.h>
+#include <linux/dma-mapping.h>
 
 #include "qcom_scm.h"
 
@@ -158,6 +159,21 @@ bool qcom_scm_is_available(void)
 }
 EXPORT_SYMBOL(qcom_scm_is_available);
 
+void *qcom_scm_alloc_buffer(size_t size, dma_addr_t *dma_addr,
+				      gfp_t gfp)
+{
+	if (__scm)
+		return dma_alloc_writecombine(__scm->dev, size, dma_addr, gfp);
+	else
+		return ERR_PTR(-ENODEV);
+}
+
+void qcom_scm_free_buffer(size_t size, void *cpu_addr,
+				 dma_addr_t dma_addr)
+{
+	dma_free_writecombine(__scm->dev, size, cpu_addr, dma_addr);
+}
+
 static int qcom_scm_probe(struct platform_device *pdev)
 {
 	struct qcom_scm *scm;
diff --git a/drivers/firmware/qcom_scm.h b/drivers/firmware/qcom_scm.h
index 7dcc733..848c7de 100644
--- a/drivers/firmware/qcom_scm.h
+++ b/drivers/firmware/qcom_scm.h
@@ -36,6 +36,10 @@ extern int __qcom_scm_is_call_available(u32 svc_id, u32 cmd_id);
 extern int __qcom_scm_hdcp_req(struct qcom_scm_hdcp_req *req, u32 req_cnt,
 		u32 *resp);
 
+extern void *qcom_scm_alloc_buffer(size_t size, dma_addr_t *dma_addr,
+				    gfp_t gfp);
+extern void qcom_scm_free_buffer(size_t size, void *virt_addr,
+				 dma_addr_t dma_addr);
 /* common error codes */
 #define QCOM_SCM_ENOMEM		-5
 #define QCOM_SCM_EOPNOTSUPP	-4
-- 
1.9.1

[Patch v2 5/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 | 187 +++++++++++++++++++++++++++++++++++++++--
 drivers/firmware/qcom_scm.c    |   2 +
 drivers/firmware/qcom_scm.h    |   5 ++
 4 files changed, 191 insertions(+), 7 deletions(-)

diff --git a/drivers/firmware/qcom_scm-32.c b/drivers/firmware/qcom_scm-32.c
index 9e3dc2f..0d2a3f8 100644
--- a/drivers/firmware/qcom_scm-32.c
+++ b/drivers/firmware/qcom_scm-32.c
@@ -482,3 +482,7 @@ int __qcom_scm_hdcp_req(struct qcom_scm_hdcp_req *req, u32 req_cnt, u32 *resp)
 	return qcom_scm_call(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..90e6f19 100644
--- a/drivers/firmware/qcom_scm-64.c
+++ b/drivers/firmware/qcom_scm-64.c
@@ -12,12 +12,135 @@
 
 #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 "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) & 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
+ * @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
+ * @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.
+*/
+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, 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;
+	void *args_virt = NULL;
+	size_t alloc_size;
+
+	if (unlikely(arglen > N_REGISTER_ARGS)) {
+		alloc_size = N_EXT_QCOM_SCM_ARGS * sizeof(u64);
+		args_virt = qcom_scm_alloc_buffer(alloc_size, &args_phys,
+						  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]);
+		}
+
+		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, arglen, 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)
+		qcom_scm_free_buffer(alloc_size, args_virt, args_phys);
+
+	if (desc->res.a0 < 0)
+		return qcom_scm_remap_error(ret);
+
+	return 0;
+}
 
 /**
  * qcom_scm_set_cold_boot_addr() - Set the cold boot address for cpus
- * @entry: Entry point function for the cpus
- * @cpus: The cpumask of cpus that will use the entry point
+ * @entry:	Entry point function for the cpus
+ * @cpus:	The cpumask of cpus that will use the entry point
  *
  * Set the cold boot address of the cpus. Any cpu outside the supported
  * range would be removed from the cpu present mask.
@@ -29,8 +152,8 @@ 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
- * @entry: Entry point function for the cpus
- * @cpus: The cpumask of cpus that will use the entry point
+ * @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.
@@ -42,7 +165,7 @@ int __qcom_scm_set_warm_boot_addr(void *entry, const cpumask_t *cpus)
 
 /**
  * qcom_scm_cpu_power_down() - Power down the cpu
- * @flags - Flags to flush cache
+ * @flags:	Flags to flush cache
  *
  * This is an end point to power down cpu. If there was a pending interrupt,
  * the control would return from this function, otherwise, the cpu jumps to the
@@ -54,10 +177,60 @@ void __qcom_scm_cpu_power_down(u32 flags)
 
 int __qcom_scm_is_call_available(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(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)
 {
-	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(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 8f78938..03c9d05 100644
--- a/drivers/firmware/qcom_scm.c
+++ b/drivers/firmware/qcom_scm.c
@@ -216,6 +216,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 848c7de..58fd454 100644
--- a/drivers/firmware/qcom_scm.h
+++ b/drivers/firmware/qcom_scm.h
@@ -40,7 +40,10 @@ extern void *qcom_scm_alloc_buffer(size_t size, dma_addr_t *dma_addr,
 				    gfp_t gfp);
 extern void qcom_scm_free_buffer(size_t size, void *virt_addr,
 				 dma_addr_t dma_addr);
+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
@@ -60,6 +63,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 v2 6/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.  This 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 | 47 ++++++++++++++++++++----------------------
 1 file changed, 22 insertions(+), 25 deletions(-)

diff --git a/drivers/firmware/qcom_scm-32.c b/drivers/firmware/qcom_scm-32.c
index 0d2a3f8..419df4d 100644
--- a/drivers/firmware/qcom_scm-32.c
+++ b/drivers/firmware/qcom_scm-32.c
@@ -294,34 +294,39 @@ out:
 				(n & 0xf))
 
 /**
- * qcom_scm_call_atomic1() - Send an atomic SCM command with one argument
+ * qcom_scm_call_atomic() - Send an atomic SCM command with one argument
  * @svc_id: service identifier
  * @cmd_id: command identifier
+ * @arglen: number of arguments
  * @arg1: first argument
+ * @arg2: second argument (optional - fill with 0 if unused)
  *
  * 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)
+static s32 qcom_scm_call_atomic(u32 svc, u32 cmd, u32 arglen, u32 arg1,
+				u32 arg2)
 {
 	int context_id;
 
-	register u32 r0 asm("r0") = SCM_ATOMIC(svc, cmd, 1);
+	register u32 r0 asm("r0") = SCM_ATOMIC(svc, cmd, arglen);
 	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)
-			: "r3");
+			: "r" (r0), "r" (r1), "r" (r2), "r" (r3)
+			);
 	return r0;
 }
 
@@ -361,22 +366,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
@@ -406,7 +395,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_atomic(QCOM_SCM_SVC_BOOT, QCOM_SCM_BOOT_ADDR, 2,
+				    flags, virt_to_phys(entry));
 }
 
 /**
@@ -422,6 +412,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
@@ -437,7 +431,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;
@@ -456,8 +453,8 @@ int __qcom_scm_set_warm_boot_addr(void *entry, const cpumask_t *cpus)
  */
 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_call_atomic(QCOM_SCM_SVC_BOOT, QCOM_SCM_CMD_TERMINATE_PC, 1,
+			flags & QCOM_SCM_FLUSH_FLAG_MASK, 0);
 }
 
 int __qcom_scm_is_call_available(u32 svc_id, u32 cmd_id)
-- 
1.9.1

[Patch v2 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..1974412 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-apq8084";
+			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 v2 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..3728e76 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-msm8916";
+			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 v2 2/8] firmware: qcom: scm: Convert SCM to platform driver

[Stephen Boyd]

On 04/25, Andy Gross wrote:
> This patch converts the Qualcomm SCM firmware driver into a platform
> driver.
> 
> Signed-off-by: Andy Gross <andy.gross@linaro.org>
> ---
>  arch/arm64/Kconfig.platforms |   1 +
>  drivers/firmware/qcom_scm.c  | 163 ++++++++++++++++++++++++++++++++++++++++---
>  2 files changed, 155 insertions(+), 9 deletions(-)
> 
> diff --git a/arch/arm64/Kconfig.platforms b/arch/arm64/Kconfig.platforms
> index efa77c1..6f0876f 100644
> --- a/arch/arm64/Kconfig.platforms
> +++ b/arch/arm64/Kconfig.platforms
> @@ -76,6 +76,7 @@ config ARCH_MVEBU
>  config ARCH_QCOM
>  	bool "Qualcomm Platforms"
>  	select PINCTRL
> +	select QCOM_SCM

So far we've left this selection up to the consumer drivers of
the qcom_scm_*() APIs. Any reason why that's changing here? I
don't see mention in the commit text.

>  	help
>  	  This enables support for the ARMv8 based Qualcomm chipsets.
>  
> +
> +/**
> + * qcom_scm_is_available() - Checks if SCM is available
> + */
> +bool qcom_scm_is_available(void)
> +{
> +	return !!__scm;
> +}
> +EXPORT_SYMBOL(qcom_scm_is_available);

What's the planned user of this? If we need it can we bury it
inside the qcom_scm_*() functions?

> +
> +static int qcom_scm_probe(struct platform_device *pdev)
> +{
[...]
> +
> +	/* vote for max clk rate for highest performance */
> +	rate = clk_round_rate(scm->core_clk, INT_MAX);
> +	ret = clk_set_rate(scm->core_clk, rate);

You can just do clk_set_rate(scm->core_clk, INT_MAX) and it will
round internally for you and do the right thing. 

> +	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-apq8084",},
> +	{ .compatible = "qcom,scm-msm8916",},
> +	{ .compatible = "qcom,scm-msm8974",},
> +	{},

Nitpick: drop , here because it's always going to be the last
entry.

> +};
> +
> +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,
> +};
> +
> +builtin_platform_driver(qcom_scm_driver);
> +
> +static int __init qcom_scm_init(void)
> +{
> +	struct device_node *np;
> +
> +	np = of_find_node_by_name(NULL, "firmware");
> +	if (!np)
> +		return -ENODEV;
> +
> +	return of_platform_populate(np, qcom_scm_dt_match, NULL, NULL);
> +

Weird newline and also we need an of_node_put() on the firmware
node at the end of this function.

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

Re: [Patch v2 3/8] firmware: qcom: scm: Generalize shared error map

[Stephen Boyd]

On 04/25, Andy Gross wrote:
> 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.
> 
> 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>
> ---

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 v2 6/8] firmware: qcom: scm: Use atomic SCM for cold boot

[Stephen Boyd]

On 04/25, Andy Gross wrote:
> This patch changes the cold_set_boot_addr function to use atomic SCM
> calls.  This 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 | 47 ++++++++++++++++++++----------------------
>  1 file changed, 22 insertions(+), 25 deletions(-)
> 
> diff --git a/drivers/firmware/qcom_scm-32.c b/drivers/firmware/qcom_scm-32.c
> index 0d2a3f8..419df4d 100644
> --- a/drivers/firmware/qcom_scm-32.c
> +++ b/drivers/firmware/qcom_scm-32.c
> @@ -294,34 +294,39 @@ out:
>  				(n & 0xf))
>  
>  /**
> - * qcom_scm_call_atomic1() - Send an atomic SCM command with one argument
> + * qcom_scm_call_atomic() - Send an atomic SCM command with one argument

Update doc to say 1 or 2 arguments now? I guess we get slightly
worse code because we force a load into the second argument
register even if it isn't used.

Also, is there a patch to change scm-32 to use dma allocations
for the buffer, or did I miss it?

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

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

[Stephen Boyd]

On 04/25, 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
> 
> diff --git a/Documentation/devicetree/bindings/firmware/qcom,scm.txt b/Documentation/devicetree/bindings/firmware/qcom,scm.txt
> new file mode 100644
> index 0000000..a679a87
> --- /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
> + * "qcom,scm-apq8084" for APQ8084
> + * "qcom,scm-msm8916" for MSM8916
> + * "qcom,scm-msm8974" for MSM8974

Do we need to keep adding these into the driver for every SoC
that we support? My understanding is apq8064 can be the one that
requires one clk, and msm8974 can be the one that requires three.
The driver can just have those two compatibles for now, and we
can keep adding compatibles here for the different SoCs, but
really we don't care, that's just to save ourselves if something
pops up and needs a workaround.

It will certainly look weird if it's firmware that's compatible
with qcom,scm-msm8974 but on an apq8084, so perhaps something
more generic like, qcom-scm-v1 and qcom,scm-v2 can be used as the
generic compatible in the driver:

	compatible = "qcom,scm-apq8064", "qcom,scm-v1";

	vs.

	compatible = "qcom,scm-apq8084", "qcom,scm-v2";

?

I just want to avoid the constant SoC churn update here if we can.

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

Re: [Patch v2 4/8] firmware: qcom: scm: Add memory allocation API

[Stephen Boyd]

On 04/25, Andy Gross wrote:
> This patch adds APIs for the scm-32 and scm-64 to use for coherent memory
> allocation.
> 
> Signed-off-by: Andy Gross <andy.gross@linaro.org>
> ---
>  drivers/firmware/qcom_scm.c | 16 ++++++++++++++++
>  drivers/firmware/qcom_scm.h |  4 ++++
>  2 files changed, 20 insertions(+)
> 
> diff --git a/drivers/firmware/qcom_scm.c b/drivers/firmware/qcom_scm.c
> index d4e9145..8f78938 100644
> --- a/drivers/firmware/qcom_scm.c
> +++ b/drivers/firmware/qcom_scm.c
> @@ -20,6 +20,7 @@
>  #include <linux/of.h>
>  #include <linux/of_platform.h>
>  #include <linux/clk.h>
> +#include <linux/dma-mapping.h>
>  
>  #include "qcom_scm.h"
>  
> @@ -158,6 +159,21 @@ bool qcom_scm_is_available(void)
>  }
>  EXPORT_SYMBOL(qcom_scm_is_available);
>  
> +void *qcom_scm_alloc_buffer(size_t size, dma_addr_t *dma_addr,
> +				      gfp_t gfp)
> +{
> +	if (__scm)
> +		return dma_alloc_writecombine(__scm->dev, size, dma_addr, gfp);
> +	else
> +		return ERR_PTR(-ENODEV);

Any reason we can't use the streaming APIs? That's pretty much
how we were doing things before. This changes it to be a
different memory type buffer which may be slower to read/write to.

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

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

[Andy Gross]

On 25 April 2016 at 20:49, Stephen Boyd <sboyd@codeaurora.org> wrote:
> On 04/25, 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
>>
>> diff --git a/Documentation/devicetree/bindings/firmware/qcom,scm.txt b/Documentation/devicetree/bindings/firmware/qcom,scm.txt
>> new file mode 100644
>> index 0000000..a679a87
>> --- /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
>> + * "qcom,scm-apq8084" for APQ8084
>> + * "qcom,scm-msm8916" for MSM8916
>> + * "qcom,scm-msm8974" for MSM8974
>
> Do we need to keep adding these into the driver for every SoC
> that we support? My understanding is apq8064 can be the one that
> requires one clk, and msm8974 can be the one that requires three.
> The driver can just have those two compatibles for now, and we
> can keep adding compatibles here for the different SoCs, but
> really we don't care, that's just to save ourselves if something
> pops up and needs a workaround.
>
> It will certainly look weird if it's firmware that's compatible
> with qcom,scm-msm8974 but on an apq8084, so perhaps something
> more generic like, qcom-scm-v1 and qcom,scm-v2 can be used as the
> generic compatible in the driver:
>
>         compatible = "qcom,scm-apq8064", "qcom,scm-v1";
>
>         vs.
>
>         compatible = "qcom,scm-apq8084", "qcom,scm-v2";
>
> ?
>
> I just want to avoid the constant SoC churn update here if we can.

Right.  We can certainly do it that way.  Its just that the v1/v2
aren't the real versions.  What if we did qcom,scm vs
qcom,scm-apq8064?

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

[Rob Herring]

On Mon, Apr 25, 2016 at 09:14:59PM -0500, Andy Gross wrote:
> On 25 April 2016 at 20:49, Stephen Boyd <sboyd@codeaurora.org> wrote:
> > On 04/25, 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
> >>
> >> diff --git a/Documentation/devicetree/bindings/firmware/qcom,scm.txt b/Documentation/devicetree/bindings/firmware/qcom,scm.txt
> >> new file mode 100644
> >> index 0000000..a679a87
> >> --- /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
> >> + * "qcom,scm-apq8084" for APQ8084
> >> + * "qcom,scm-msm8916" for MSM8916
> >> + * "qcom,scm-msm8974" for MSM8974
> >
> > Do we need to keep adding these into the driver for every SoC
> > that we support? My understanding is apq8064 can be the one that
> > requires one clk, and msm8974 can be the one that requires three.
> > The driver can just have those two compatibles for now, and we
> > can keep adding compatibles here for the different SoCs, but
> > really we don't care, that's just to save ourselves if something
> > pops up and needs a workaround.
> >
> > It will certainly look weird if it's firmware that's compatible
> > with qcom,scm-msm8974 but on an apq8084, so perhaps something

Not if the first string has apq8084.

> > more generic like, qcom-scm-v1 and qcom,scm-v2 can be used as the
> > generic compatible in the driver:
> >
> >         compatible = "qcom,scm-apq8064", "qcom,scm-v1";
> >
> >         vs.
> >
> >         compatible = "qcom,scm-apq8084", "qcom,scm-v2";
> >
> > ?
> >
> > I just want to avoid the constant SoC churn update here if we can.
> 
> Right.  We can certainly do it that way.  Its just that the v1/v2
> aren't the real versions.  What if we did qcom,scm vs
> qcom,scm-apq8064?

I'd prefer this over fake version numbers.

Is there any sane versioning of the firmware itself that could be used?

Rob

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

[Andy Gross]

On Mon, Apr 25, 2016 at 06:29:35PM -0700, Stephen Boyd wrote:
> On 04/25, Andy Gross wrote:
> > This patch converts the Qualcomm SCM firmware driver into a platform
> > driver.
> > 
> > Signed-off-by: Andy Gross <andy.gross@linaro.org>
> > ---
> >  arch/arm64/Kconfig.platforms |   1 +
> >  drivers/firmware/qcom_scm.c  | 163 ++++++++++++++++++++++++++++++++++++++++---
> >  2 files changed, 155 insertions(+), 9 deletions(-)
> > 
> > diff --git a/arch/arm64/Kconfig.platforms b/arch/arm64/Kconfig.platforms
> > index efa77c1..6f0876f 100644
> > --- a/arch/arm64/Kconfig.platforms
> > +++ b/arch/arm64/Kconfig.platforms
> > @@ -76,6 +76,7 @@ config ARCH_MVEBU
> >  config ARCH_QCOM
> >  	bool "Qualcomm Platforms"
> >  	select PINCTRL
> > +	select QCOM_SCM
> 
> So far we've left this selection up to the consumer drivers of
> the qcom_scm_*() APIs. Any reason why that's changing here? I
> don't see mention in the commit text.

We can leave it that way.
> 
> >  	help
> >  	  This enables support for the ARMv8 based Qualcomm chipsets.
> >  
> > +
> > +/**
> > + * qcom_scm_is_available() - Checks if SCM is available
> > + */
> > +bool qcom_scm_is_available(void)
> > +{
> > +	return !!__scm;
> > +}
> > +EXPORT_SYMBOL(qcom_scm_is_available);
> 
> What's the planned user of this? If we need it can we bury it
> inside the qcom_scm_*() functions?

Hmmmm doing a little searching, I don't see this being used anymore.  I'll drop
it for now.

> > +
> > +static int qcom_scm_probe(struct platform_device *pdev)
> > +{
> [...]
> > +
> > +	/* vote for max clk rate for highest performance */
> > +	rate = clk_round_rate(scm->core_clk, INT_MAX);
> > +	ret = clk_set_rate(scm->core_clk, rate);
> 
> You can just do clk_set_rate(scm->core_clk, INT_MAX) and it will
> round internally for you and do the right thing. 

I'll change this to do that.

> > +	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-apq8084",},
> > +	{ .compatible = "qcom,scm-msm8916",},
> > +	{ .compatible = "qcom,scm-msm8974",},
> > +	{},
> 
> Nitpick: drop , here because it's always going to be the last
> entry.

will fix.

> > +};
> > +
> > +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,
> > +};
> > +
> > +builtin_platform_driver(qcom_scm_driver);
> > +
> > +static int __init qcom_scm_init(void)
> > +{
> > +	struct device_node *np;
> > +
> > +	np = of_find_node_by_name(NULL, "firmware");
> > +	if (!np)
> > +		return -ENODEV;
> > +
> > +	return of_platform_populate(np, qcom_scm_dt_match, NULL, NULL);
> > +
> 
> Weird newline and also we need an of_node_put() on the firmware
> node at the end of this function.

Ah thanks for catching that.


Regards,

Andy