[PATCH v3 0/4] qcom_scm: Add support for MC boot address API

[PATCH v3 0/4] qcom_scm: Add support for MC boot address API

[Stephan Gerhold]

The "firmware: qcom: scm: Add support for MC boot address API" commit
was reverted again in 5.16 [1]. This is a new attempt to add it back
with much less potential build problems.

For that I first simplify the existing qcom_scm_set_cold/warm_boot_addr()
implementations. The idea is that cpu_logical_map(), MPIDR_AFFINITY_LEVEL()
etc are not needed if we just set the entry address for all CPUs.
Nothing in the mainline tree actually requires setting a different entry
address for one particular CPU and I cannot really think of a use case for this.

[1]: https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git/commit/?id=7db2bc925e4642df6255612e5521af0423ada18a

Stephan Gerhold (4):
  cpuidle: qcom-spm: Check if any CPU is managed by SPM
  firmware: qcom: scm: Simplify set_cold/warm_boot_addr()
  firmware: qcom: scm: Drop cpumask parameter from set_boot_addr()
  firmware: qcom: scm: Add support for MC boot address API

 arch/arm/mach-qcom/platsmp.c       |   3 +-
 drivers/cpuidle/cpuidle-qcom-spm.c |  28 +++++-
 drivers/firmware/qcom_scm.c        | 132 ++++++++++++-----------------
 drivers/firmware/qcom_scm.h        |   5 ++
 include/linux/qcom_scm.h           |   4 +-
 5 files changed, 84 insertions(+), 88 deletions(-)

-- 
2.34.1

[PATCH v3 0/4] qcom_scm: Add support for MC boot address API

[Stephan Gerhold]

The "firmware: qcom: scm: Add support for MC boot address API" commit
was reverted again in 5.16 [1]. This is a new attempt to add it back
with much less potential build problems.

For that I first simplify the existing qcom_scm_set_cold/warm_boot_addr()
implementations. The idea is that cpu_logical_map(), MPIDR_AFFINITY_LEVEL()
etc are not needed if we just set the entry address for all CPUs.
Nothing in the mainline tree actually requires setting a different entry
address for one particular CPU and I cannot really think of a use case for this.

[1]: https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git/commit/?id=7db2bc925e4642df6255612e5521af0423ada18a

Stephan Gerhold (4):
  cpuidle: qcom-spm: Check if any CPU is managed by SPM
  firmware: qcom: scm: Simplify set_cold/warm_boot_addr()
  firmware: qcom: scm: Drop cpumask parameter from set_boot_addr()
  firmware: qcom: scm: Add support for MC boot address API

 arch/arm/mach-qcom/platsmp.c       |   3 +-
 drivers/cpuidle/cpuidle-qcom-spm.c |  28 +++++-
 drivers/firmware/qcom_scm.c        | 132 ++++++++++++-----------------
 drivers/firmware/qcom_scm.h        |   5 ++
 include/linux/qcom_scm.h           |   4 +-
 5 files changed, 84 insertions(+), 88 deletions(-)

-- 
2.34.1


_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

[PATCH v3 1/4] cpuidle: qcom-spm: Check if any CPU is managed by SPM

[Stephan Gerhold]

At the moment, the "qcom-spm-cpuidle" platform device is always created,
even if none of the CPUs is actually managed by the SPM. On non-qcom
platforms this will result in infinite probe-deferral due to the
failing qcom_scm_is_available() call.

To avoid this, look through the CPU DT nodes and check if there is
actually any CPU managed by a SPM (as indicated by the qcom,saw property).
It should also be available because e.g. MSM8916 has qcom,saw defined
but it's typically not enabled with ARM64/PSCI firmwares.

This is needed in preparation of a follow-up change that calls
qcom_scm_set_warm_boot_addr() a single time before registering any
cpuidle drivers. Otherwise this call might be made even on devices
that have this driver enabled but actually make use of PSCI.

Fixes: 60f3692b5f0b ("cpuidle: qcom_spm: Detach state machine from main SPM handling")
Reported-by: Marek Szyprowski <m.szyprowski@samsung.com>
Link: https://lore.kernel.org/r/86e3e09f-a8d7-3dff-3fc6-ddd7d30c5d78@samsung.com/
Signed-off-by: Stephan Gerhold <stephan@gerhold.net>
---
Daniel, would be great if you could ack this patch and PATCH 3/4
(the cpuidle part) if they look good to you. I think it's easiest if Bjorn
takes them together with the qcom_scm changes through the qcom tree.

Marek had an alternative fix for this [1], the difference in this patch is
that it avoids creating the platform device entirely if no CPU is managed
by a SPM.

[1]: https://lore.kernel.org/r/20211020120643.28231-1-m.szyprowski@samsung.com/
---
 drivers/cpuidle/cpuidle-qcom-spm.c | 20 ++++++++++++++++++++
 1 file changed, 20 insertions(+)

diff --git a/drivers/cpuidle/cpuidle-qcom-spm.c b/drivers/cpuidle/cpuidle-qcom-spm.c
index 01e77913a414..5f27dcc6c110 100644
--- a/drivers/cpuidle/cpuidle-qcom-spm.c
+++ b/drivers/cpuidle/cpuidle-qcom-spm.c
@@ -155,6 +155,22 @@ static struct platform_driver spm_cpuidle_driver = {
 	},
 };
 
+static bool __init qcom_spm_find_any_cpu(void)
+{
+	struct device_node *cpu_node, *saw_node;
+
+	for_each_of_cpu_node(cpu_node) {
+		saw_node = of_parse_phandle(cpu_node, "qcom,saw", 0);
+		if (of_device_is_available(saw_node)) {
+			of_node_put(saw_node);
+			of_node_put(cpu_node);
+			return true;
+		}
+		of_node_put(saw_node);
+	}
+	return false;
+}
+
 static int __init qcom_spm_cpuidle_init(void)
 {
 	struct platform_device *pdev;
@@ -164,6 +180,10 @@ static int __init qcom_spm_cpuidle_init(void)
 	if (ret)
 		return ret;
 
+	/* Make sure there is actually any CPU managed by the SPM */
+	if (!qcom_spm_find_any_cpu())
+		return 0;
+
 	pdev = platform_device_register_simple("qcom-spm-cpuidle",
 					       -1, NULL, 0);
 	if (IS_ERR(pdev)) {
-- 
2.34.1

[PATCH v3 1/4] cpuidle: qcom-spm: Check if any CPU is managed by SPM

[Stephan Gerhold]

At the moment, the "qcom-spm-cpuidle" platform device is always created,
even if none of the CPUs is actually managed by the SPM. On non-qcom
platforms this will result in infinite probe-deferral due to the
failing qcom_scm_is_available() call.

To avoid this, look through the CPU DT nodes and check if there is
actually any CPU managed by a SPM (as indicated by the qcom,saw property).
It should also be available because e.g. MSM8916 has qcom,saw defined
but it's typically not enabled with ARM64/PSCI firmwares.

This is needed in preparation of a follow-up change that calls
qcom_scm_set_warm_boot_addr() a single time before registering any
cpuidle drivers. Otherwise this call might be made even on devices
that have this driver enabled but actually make use of PSCI.

Fixes: 60f3692b5f0b ("cpuidle: qcom_spm: Detach state machine from main SPM handling")
Reported-by: Marek Szyprowski <m.szyprowski@samsung.com>
Link: https://lore.kernel.org/r/86e3e09f-a8d7-3dff-3fc6-ddd7d30c5d78@samsung.com/
Signed-off-by: Stephan Gerhold <stephan@gerhold.net>
---
Daniel, would be great if you could ack this patch and PATCH 3/4
(the cpuidle part) if they look good to you. I think it's easiest if Bjorn
takes them together with the qcom_scm changes through the qcom tree.

Marek had an alternative fix for this [1], the difference in this patch is
that it avoids creating the platform device entirely if no CPU is managed
by a SPM.

[1]: https://lore.kernel.org/r/20211020120643.28231-1-m.szyprowski@samsung.com/
---
 drivers/cpuidle/cpuidle-qcom-spm.c | 20 ++++++++++++++++++++
 1 file changed, 20 insertions(+)

diff --git a/drivers/cpuidle/cpuidle-qcom-spm.c b/drivers/cpuidle/cpuidle-qcom-spm.c
index 01e77913a414..5f27dcc6c110 100644
--- a/drivers/cpuidle/cpuidle-qcom-spm.c
+++ b/drivers/cpuidle/cpuidle-qcom-spm.c
@@ -155,6 +155,22 @@ static struct platform_driver spm_cpuidle_driver = {
 	},
 };
 
+static bool __init qcom_spm_find_any_cpu(void)
+{
+	struct device_node *cpu_node, *saw_node;
+
+	for_each_of_cpu_node(cpu_node) {
+		saw_node = of_parse_phandle(cpu_node, "qcom,saw", 0);
+		if (of_device_is_available(saw_node)) {
+			of_node_put(saw_node);
+			of_node_put(cpu_node);
+			return true;
+		}
+		of_node_put(saw_node);
+	}
+	return false;
+}
+
 static int __init qcom_spm_cpuidle_init(void)
 {
 	struct platform_device *pdev;
@@ -164,6 +180,10 @@ static int __init qcom_spm_cpuidle_init(void)
 	if (ret)
 		return ret;
 
+	/* Make sure there is actually any CPU managed by the SPM */
+	if (!qcom_spm_find_any_cpu())
+		return 0;
+
 	pdev = platform_device_register_simple("qcom-spm-cpuidle",
 					       -1, NULL, 0);
 	if (IS_ERR(pdev)) {
-- 
2.34.1


_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

[PATCH v3 2/4] firmware: qcom: scm: Simplify set_cold/warm_boot_addr()

[Stephan Gerhold]

The qcom_scm_set_cold/warm_boot_addr() implementations have a lot of
functionality that is actually not used.

For example, set_warm_boot_addr() caches the last used entry address
and skips making the SCM call when the entry address is unchanged.
But there is actually just a single call of qcom_scm_set_warm_boot_addr()
in the whole kernel tree, which always configures the entry address
to cpu_resume_arm().

Simplify this by having a single qcom_scm_set_boot_addr() function
for both cold and warm boot address. This is totally sufficient for
the functionality supported in the mainline tree.

Signed-off-by: Stephan Gerhold <stephan@gerhold.net>
---
 drivers/firmware/qcom_scm.c | 105 +++++++++---------------------------
 drivers/firmware/qcom_scm.h |   1 +
 2 files changed, 27 insertions(+), 79 deletions(-)

diff --git a/drivers/firmware/qcom_scm.c b/drivers/firmware/qcom_scm.c
index 7db8066b19fd..e0fca80bf6fc 100644
--- a/drivers/firmware/qcom_scm.c
+++ b/drivers/firmware/qcom_scm.c
@@ -49,26 +49,12 @@ struct qcom_scm_mem_map_info {
 	__le64 mem_size;
 };
 
-#define QCOM_SCM_FLAG_COLDBOOT_CPU0	0x00
-#define QCOM_SCM_FLAG_COLDBOOT_CPU1	0x01
-#define QCOM_SCM_FLAG_COLDBOOT_CPU2	0x08
-#define QCOM_SCM_FLAG_COLDBOOT_CPU3	0x20
-
-#define QCOM_SCM_FLAG_WARMBOOT_CPU0	0x04
-#define QCOM_SCM_FLAG_WARMBOOT_CPU1	0x02
-#define QCOM_SCM_FLAG_WARMBOOT_CPU2	0x10
-#define QCOM_SCM_FLAG_WARMBOOT_CPU3	0x40
-
-struct qcom_scm_wb_entry {
-	int flag;
-	void *entry;
+/* Each bit configures cold/warm boot address for one of the 4 CPUs */
+static const u8 qcom_scm_cpu_cold_bits[QCOM_SCM_BOOT_MAX_CPUS] = {
+	0, BIT(0), BIT(3), BIT(5)
 };
-
-static struct qcom_scm_wb_entry qcom_scm_wb[] = {
-	{ .flag = QCOM_SCM_FLAG_WARMBOOT_CPU0 },
-	{ .flag = QCOM_SCM_FLAG_WARMBOOT_CPU1 },
-	{ .flag = QCOM_SCM_FLAG_WARMBOOT_CPU2 },
-	{ .flag = QCOM_SCM_FLAG_WARMBOOT_CPU3 },
+static const u8 qcom_scm_cpu_warm_bits[QCOM_SCM_BOOT_MAX_CPUS] = {
+	BIT(2), BIT(1), BIT(4), BIT(6)
 };
 
 static const char * const qcom_scm_convention_names[] = {
@@ -260,49 +246,41 @@ static bool __qcom_scm_is_call_available(struct device *dev, u32 svc_id,
 	return ret ? false : !!res.result[0];
 }
 
-/**
- * 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
- *
- * 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)
+static int qcom_scm_set_boot_addr(void *entry, const cpumask_t *cpus,
+				  const u8 *cpu_bits)
 {
-	int ret;
-	int flags = 0;
 	int cpu;
+	unsigned int flags = 0;
 	struct qcom_scm_desc desc = {
 		.svc = QCOM_SCM_SVC_BOOT,
 		.cmd = QCOM_SCM_BOOT_SET_ADDR,
 		.arginfo = QCOM_SCM_ARGS(2),
+		.owner = ARM_SMCCC_OWNER_SIP,
 	};
 
-	/*
-	 * Reassign only if we are switching from hotplug entry point
-	 * to cpuidle entry point or vice versa.
-	 */
 	for_each_cpu(cpu, cpus) {
-		if (entry == qcom_scm_wb[cpu].entry)
-			continue;
-		flags |= qcom_scm_wb[cpu].flag;
+		if (cpu >= QCOM_SCM_BOOT_MAX_CPUS)
+			return -EINVAL;
+		flags |= cpu_bits[cpu];
 	}
 
-	/* No change in entry function */
-	if (!flags)
-		return 0;
-
 	desc.args[0] = flags;
 	desc.args[1] = virt_to_phys(entry);
 
-	ret = qcom_scm_call(__scm->dev, &desc, NULL);
-	if (!ret) {
-		for_each_cpu(cpu, cpus)
-			qcom_scm_wb[cpu].entry = entry;
-	}
+	return qcom_scm_call_atomic(__scm ? __scm->dev : NULL, &desc, NULL);
+}
 
-	return ret;
+/**
+ * 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
+ *
+ * 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)
+{
+	return qcom_scm_set_boot_addr(entry, cpus, qcom_scm_cpu_warm_bits);
 }
 EXPORT_SYMBOL(qcom_scm_set_warm_boot_addr);
 
@@ -310,41 +288,10 @@ EXPORT_SYMBOL(qcom_scm_set_warm_boot_addr);
  * 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
- *
- * Set the cold boot address of the cpus. Any cpu outside the supported
- * range would be removed from the cpu present mask.
  */
 int qcom_scm_set_cold_boot_addr(void *entry, const cpumask_t *cpus)
 {
-	int flags = 0;
-	int cpu;
-	int scm_cb_flags[] = {
-		QCOM_SCM_FLAG_COLDBOOT_CPU0,
-		QCOM_SCM_FLAG_COLDBOOT_CPU1,
-		QCOM_SCM_FLAG_COLDBOOT_CPU2,
-		QCOM_SCM_FLAG_COLDBOOT_CPU3,
-	};
-	struct qcom_scm_desc desc = {
-		.svc = QCOM_SCM_SVC_BOOT,
-		.cmd = QCOM_SCM_BOOT_SET_ADDR,
-		.arginfo = QCOM_SCM_ARGS(2),
-		.owner = ARM_SMCCC_OWNER_SIP,
-	};
-
-	if (!cpus || cpumask_empty(cpus))
-		return -EINVAL;
-
-	for_each_cpu(cpu, cpus) {
-		if (cpu < ARRAY_SIZE(scm_cb_flags))
-			flags |= scm_cb_flags[cpu];
-		else
-			set_cpu_present(cpu, false);
-	}
-
-	desc.args[0] = flags;
-	desc.args[1] = virt_to_phys(entry);
-
-	return qcom_scm_call_atomic(__scm ? __scm->dev : NULL, &desc, NULL);
+	return qcom_scm_set_boot_addr(entry, cpus, qcom_scm_cpu_cold_bits);
 }
 EXPORT_SYMBOL(qcom_scm_set_cold_boot_addr);
 
diff --git a/drivers/firmware/qcom_scm.h b/drivers/firmware/qcom_scm.h
index d92156ceb3ac..fd7d2a5f3e70 100644
--- a/drivers/firmware/qcom_scm.h
+++ b/drivers/firmware/qcom_scm.h
@@ -80,6 +80,7 @@ extern int scm_legacy_call(struct device *dev, const struct qcom_scm_desc *desc,
 #define QCOM_SCM_BOOT_SET_DLOAD_MODE	0x10
 #define QCOM_SCM_BOOT_SET_REMOTE_STATE	0x0a
 #define QCOM_SCM_FLUSH_FLAG_MASK	0x3
+#define QCOM_SCM_BOOT_MAX_CPUS		4
 
 #define QCOM_SCM_SVC_PIL		0x02
 #define QCOM_SCM_PIL_PAS_INIT_IMAGE	0x01
-- 
2.34.1

[PATCH v3 2/4] firmware: qcom: scm: Simplify set_cold/warm_boot_addr()

[Stephan Gerhold]

The qcom_scm_set_cold/warm_boot_addr() implementations have a lot of
functionality that is actually not used.

For example, set_warm_boot_addr() caches the last used entry address
and skips making the SCM call when the entry address is unchanged.
But there is actually just a single call of qcom_scm_set_warm_boot_addr()
in the whole kernel tree, which always configures the entry address
to cpu_resume_arm().

Simplify this by having a single qcom_scm_set_boot_addr() function
for both cold and warm boot address. This is totally sufficient for
the functionality supported in the mainline tree.

Signed-off-by: Stephan Gerhold <stephan@gerhold.net>
---
 drivers/firmware/qcom_scm.c | 105 +++++++++---------------------------
 drivers/firmware/qcom_scm.h |   1 +
 2 files changed, 27 insertions(+), 79 deletions(-)

diff --git a/drivers/firmware/qcom_scm.c b/drivers/firmware/qcom_scm.c
index 7db8066b19fd..e0fca80bf6fc 100644
--- a/drivers/firmware/qcom_scm.c
+++ b/drivers/firmware/qcom_scm.c
@@ -49,26 +49,12 @@ struct qcom_scm_mem_map_info {
 	__le64 mem_size;
 };
 
-#define QCOM_SCM_FLAG_COLDBOOT_CPU0	0x00
-#define QCOM_SCM_FLAG_COLDBOOT_CPU1	0x01
-#define QCOM_SCM_FLAG_COLDBOOT_CPU2	0x08
-#define QCOM_SCM_FLAG_COLDBOOT_CPU3	0x20
-
-#define QCOM_SCM_FLAG_WARMBOOT_CPU0	0x04
-#define QCOM_SCM_FLAG_WARMBOOT_CPU1	0x02
-#define QCOM_SCM_FLAG_WARMBOOT_CPU2	0x10
-#define QCOM_SCM_FLAG_WARMBOOT_CPU3	0x40
-
-struct qcom_scm_wb_entry {
-	int flag;
-	void *entry;
+/* Each bit configures cold/warm boot address for one of the 4 CPUs */
+static const u8 qcom_scm_cpu_cold_bits[QCOM_SCM_BOOT_MAX_CPUS] = {
+	0, BIT(0), BIT(3), BIT(5)
 };
-
-static struct qcom_scm_wb_entry qcom_scm_wb[] = {
-	{ .flag = QCOM_SCM_FLAG_WARMBOOT_CPU0 },
-	{ .flag = QCOM_SCM_FLAG_WARMBOOT_CPU1 },
-	{ .flag = QCOM_SCM_FLAG_WARMBOOT_CPU2 },
-	{ .flag = QCOM_SCM_FLAG_WARMBOOT_CPU3 },
+static const u8 qcom_scm_cpu_warm_bits[QCOM_SCM_BOOT_MAX_CPUS] = {
+	BIT(2), BIT(1), BIT(4), BIT(6)
 };
 
 static const char * const qcom_scm_convention_names[] = {
@@ -260,49 +246,41 @@ static bool __qcom_scm_is_call_available(struct device *dev, u32 svc_id,
 	return ret ? false : !!res.result[0];
 }
 
-/**
- * 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
- *
- * 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)
+static int qcom_scm_set_boot_addr(void *entry, const cpumask_t *cpus,
+				  const u8 *cpu_bits)
 {
-	int ret;
-	int flags = 0;
 	int cpu;
+	unsigned int flags = 0;
 	struct qcom_scm_desc desc = {
 		.svc = QCOM_SCM_SVC_BOOT,
 		.cmd = QCOM_SCM_BOOT_SET_ADDR,
 		.arginfo = QCOM_SCM_ARGS(2),
+		.owner = ARM_SMCCC_OWNER_SIP,
 	};
 
-	/*
-	 * Reassign only if we are switching from hotplug entry point
-	 * to cpuidle entry point or vice versa.
-	 */
 	for_each_cpu(cpu, cpus) {
-		if (entry == qcom_scm_wb[cpu].entry)
-			continue;
-		flags |= qcom_scm_wb[cpu].flag;
+		if (cpu >= QCOM_SCM_BOOT_MAX_CPUS)
+			return -EINVAL;
+		flags |= cpu_bits[cpu];
 	}
 
-	/* No change in entry function */
-	if (!flags)
-		return 0;
-
 	desc.args[0] = flags;
 	desc.args[1] = virt_to_phys(entry);
 
-	ret = qcom_scm_call(__scm->dev, &desc, NULL);
-	if (!ret) {
-		for_each_cpu(cpu, cpus)
-			qcom_scm_wb[cpu].entry = entry;
-	}
+	return qcom_scm_call_atomic(__scm ? __scm->dev : NULL, &desc, NULL);
+}
 
-	return ret;
+/**
+ * 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
+ *
+ * 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)
+{
+	return qcom_scm_set_boot_addr(entry, cpus, qcom_scm_cpu_warm_bits);
 }
 EXPORT_SYMBOL(qcom_scm_set_warm_boot_addr);
 
@@ -310,41 +288,10 @@ EXPORT_SYMBOL(qcom_scm_set_warm_boot_addr);
  * 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
- *
- * Set the cold boot address of the cpus. Any cpu outside the supported
- * range would be removed from the cpu present mask.
  */
 int qcom_scm_set_cold_boot_addr(void *entry, const cpumask_t *cpus)
 {
-	int flags = 0;
-	int cpu;
-	int scm_cb_flags[] = {
-		QCOM_SCM_FLAG_COLDBOOT_CPU0,
-		QCOM_SCM_FLAG_COLDBOOT_CPU1,
-		QCOM_SCM_FLAG_COLDBOOT_CPU2,
-		QCOM_SCM_FLAG_COLDBOOT_CPU3,
-	};
-	struct qcom_scm_desc desc = {
-		.svc = QCOM_SCM_SVC_BOOT,
-		.cmd = QCOM_SCM_BOOT_SET_ADDR,
-		.arginfo = QCOM_SCM_ARGS(2),
-		.owner = ARM_SMCCC_OWNER_SIP,
-	};
-
-	if (!cpus || cpumask_empty(cpus))
-		return -EINVAL;
-
-	for_each_cpu(cpu, cpus) {
-		if (cpu < ARRAY_SIZE(scm_cb_flags))
-			flags |= scm_cb_flags[cpu];
-		else
-			set_cpu_present(cpu, false);
-	}
-
-	desc.args[0] = flags;
-	desc.args[1] = virt_to_phys(entry);
-
-	return qcom_scm_call_atomic(__scm ? __scm->dev : NULL, &desc, NULL);
+	return qcom_scm_set_boot_addr(entry, cpus, qcom_scm_cpu_cold_bits);
 }
 EXPORT_SYMBOL(qcom_scm_set_cold_boot_addr);
 
diff --git a/drivers/firmware/qcom_scm.h b/drivers/firmware/qcom_scm.h
index d92156ceb3ac..fd7d2a5f3e70 100644
--- a/drivers/firmware/qcom_scm.h
+++ b/drivers/firmware/qcom_scm.h
@@ -80,6 +80,7 @@ extern int scm_legacy_call(struct device *dev, const struct qcom_scm_desc *desc,
 #define QCOM_SCM_BOOT_SET_DLOAD_MODE	0x10
 #define QCOM_SCM_BOOT_SET_REMOTE_STATE	0x0a
 #define QCOM_SCM_FLUSH_FLAG_MASK	0x3
+#define QCOM_SCM_BOOT_MAX_CPUS		4
 
 #define QCOM_SCM_SVC_PIL		0x02
 #define QCOM_SCM_PIL_PAS_INIT_IMAGE	0x01
-- 
2.34.1


_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

[PATCH v3 3/4] firmware: qcom: scm: Drop cpumask parameter from set_boot_addr()

[Stephan Gerhold]

qcom_scm_set_cold/warm_boot_addr() currently take a cpumask parameter,
but it's not very useful because at the end we always set the same entry
address for all CPUs. This also allows speeding up probe of
cpuidle-qcom-spm a bit because only one SCM call needs to be made to
the TrustZone firmware, instead of one per CPU.

The main reason for this change is that it allows implementing the
"multi-cluster" variant of the set_boot_addr() call more easily
without having to rely on functions that break in certain build
configurations or that are not exported to modules.

Signed-off-by: Stephan Gerhold <stephan@gerhold.net>
---
 arch/arm/mach-qcom/platsmp.c       |  3 +--
 drivers/cpuidle/cpuidle-qcom-spm.c |  8 ++++----
 drivers/firmware/qcom_scm.c        | 19 ++++++++-----------
 include/linux/qcom_scm.h           |  4 ++--
 4 files changed, 15 insertions(+), 19 deletions(-)

diff --git a/arch/arm/mach-qcom/platsmp.c b/arch/arm/mach-qcom/platsmp.c
index 58a4228455ce..65a0d5ce2bb3 100644
--- a/arch/arm/mach-qcom/platsmp.c
+++ b/arch/arm/mach-qcom/platsmp.c
@@ -357,8 +357,7 @@ static void __init qcom_smp_prepare_cpus(unsigned int max_cpus)
 {
 	int cpu;
 
-	if (qcom_scm_set_cold_boot_addr(secondary_startup_arm,
-					cpu_present_mask)) {
+	if (qcom_scm_set_cold_boot_addr(secondary_startup_arm)) {
 		for_each_present_cpu(cpu) {
 			if (cpu == smp_processor_id())
 				continue;
diff --git a/drivers/cpuidle/cpuidle-qcom-spm.c b/drivers/cpuidle/cpuidle-qcom-spm.c
index 5f27dcc6c110..beedf22cbe78 100644
--- a/drivers/cpuidle/cpuidle-qcom-spm.c
+++ b/drivers/cpuidle/cpuidle-qcom-spm.c
@@ -122,10 +122,6 @@ static int spm_cpuidle_register(struct device *cpuidle_dev, int cpu)
 	if (ret <= 0)
 		return ret ? : -ENODEV;
 
-	ret = qcom_scm_set_warm_boot_addr(cpu_resume_arm, cpumask_of(cpu));
-	if (ret)
-		return ret;
-
 	return cpuidle_register(&data->cpuidle_driver, NULL);
 }
 
@@ -136,6 +132,10 @@ static int spm_cpuidle_drv_probe(struct platform_device *pdev)
 	if (!qcom_scm_is_available())
 		return -EPROBE_DEFER;
 
+	ret = qcom_scm_set_warm_boot_addr(cpu_resume_arm);
+	if (ret)
+		return dev_err_probe(&pdev->dev, ret, "set warm boot addr failed");
+
 	for_each_possible_cpu(cpu) {
 		ret = spm_cpuidle_register(&pdev->dev, cpu);
 		if (ret && ret != -ENODEV) {
diff --git a/drivers/firmware/qcom_scm.c b/drivers/firmware/qcom_scm.c
index e0fca80bf6fc..e89be2f0cdec 100644
--- a/drivers/firmware/qcom_scm.c
+++ b/drivers/firmware/qcom_scm.c
@@ -246,8 +246,7 @@ static bool __qcom_scm_is_call_available(struct device *dev, u32 svc_id,
 	return ret ? false : !!res.result[0];
 }
 
-static int qcom_scm_set_boot_addr(void *entry, const cpumask_t *cpus,
-				  const u8 *cpu_bits)
+static int qcom_scm_set_boot_addr(void *entry, const u8 *cpu_bits)
 {
 	int cpu;
 	unsigned int flags = 0;
@@ -258,7 +257,7 @@ static int qcom_scm_set_boot_addr(void *entry, const cpumask_t *cpus,
 		.owner = ARM_SMCCC_OWNER_SIP,
 	};
 
-	for_each_cpu(cpu, cpus) {
+	for_each_present_cpu(cpu) {
 		if (cpu >= QCOM_SCM_BOOT_MAX_CPUS)
 			return -EINVAL;
 		flags |= cpu_bits[cpu];
@@ -271,27 +270,25 @@ static int qcom_scm_set_boot_addr(void *entry, const cpumask_t *cpus,
 }
 
 /**
- * qcom_scm_set_warm_boot_addr() - Set the warm boot address for cpus
+ * qcom_scm_set_warm_boot_addr() - Set the warm boot address for all cpus
  * @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(void *entry)
 {
-	return qcom_scm_set_boot_addr(entry, cpus, qcom_scm_cpu_warm_bits);
+	return qcom_scm_set_boot_addr(entry, qcom_scm_cpu_warm_bits);
 }
 EXPORT_SYMBOL(qcom_scm_set_warm_boot_addr);
 
 /**
- * qcom_scm_set_cold_boot_addr() - Set the cold boot address for cpus
+ * qcom_scm_set_cold_boot_addr() - Set the cold boot address for all cpus
  * @entry: Entry point function for the cpus
- * @cpus: The cpumask of cpus that will use the entry point
  */
-int qcom_scm_set_cold_boot_addr(void *entry, const cpumask_t *cpus)
+int qcom_scm_set_cold_boot_addr(void *entry)
 {
-	return qcom_scm_set_boot_addr(entry, cpus, qcom_scm_cpu_cold_bits);
+	return qcom_scm_set_boot_addr(entry, qcom_scm_cpu_cold_bits);
 }
 EXPORT_SYMBOL(qcom_scm_set_cold_boot_addr);
 
diff --git a/include/linux/qcom_scm.h b/include/linux/qcom_scm.h
index 81cad9e1e412..048d09e1965b 100644
--- a/include/linux/qcom_scm.h
+++ b/include/linux/qcom_scm.h
@@ -63,8 +63,8 @@ enum qcom_scm_ice_cipher {
 
 extern bool qcom_scm_is_available(void);
 
-extern int qcom_scm_set_cold_boot_addr(void *entry, const cpumask_t *cpus);
-extern int qcom_scm_set_warm_boot_addr(void *entry, const cpumask_t *cpus);
+extern int qcom_scm_set_cold_boot_addr(void *entry);
+extern int qcom_scm_set_warm_boot_addr(void *entry);
 extern void qcom_scm_cpu_power_down(u32 flags);
 extern int qcom_scm_set_remote_state(u32 state, u32 id);
 
-- 
2.34.1

[PATCH v3 3/4] firmware: qcom: scm: Drop cpumask parameter from set_boot_addr()

[Stephan Gerhold]

qcom_scm_set_cold/warm_boot_addr() currently take a cpumask parameter,
but it's not very useful because at the end we always set the same entry
address for all CPUs. This also allows speeding up probe of
cpuidle-qcom-spm a bit because only one SCM call needs to be made to
the TrustZone firmware, instead of one per CPU.

The main reason for this change is that it allows implementing the
"multi-cluster" variant of the set_boot_addr() call more easily
without having to rely on functions that break in certain build
configurations or that are not exported to modules.

Signed-off-by: Stephan Gerhold <stephan@gerhold.net>
---
 arch/arm/mach-qcom/platsmp.c       |  3 +--
 drivers/cpuidle/cpuidle-qcom-spm.c |  8 ++++----
 drivers/firmware/qcom_scm.c        | 19 ++++++++-----------
 include/linux/qcom_scm.h           |  4 ++--
 4 files changed, 15 insertions(+), 19 deletions(-)

diff --git a/arch/arm/mach-qcom/platsmp.c b/arch/arm/mach-qcom/platsmp.c
index 58a4228455ce..65a0d5ce2bb3 100644
--- a/arch/arm/mach-qcom/platsmp.c
+++ b/arch/arm/mach-qcom/platsmp.c
@@ -357,8 +357,7 @@ static void __init qcom_smp_prepare_cpus(unsigned int max_cpus)
 {
 	int cpu;
 
-	if (qcom_scm_set_cold_boot_addr(secondary_startup_arm,
-					cpu_present_mask)) {
+	if (qcom_scm_set_cold_boot_addr(secondary_startup_arm)) {
 		for_each_present_cpu(cpu) {
 			if (cpu == smp_processor_id())
 				continue;
diff --git a/drivers/cpuidle/cpuidle-qcom-spm.c b/drivers/cpuidle/cpuidle-qcom-spm.c
index 5f27dcc6c110..beedf22cbe78 100644
--- a/drivers/cpuidle/cpuidle-qcom-spm.c
+++ b/drivers/cpuidle/cpuidle-qcom-spm.c
@@ -122,10 +122,6 @@ static int spm_cpuidle_register(struct device *cpuidle_dev, int cpu)
 	if (ret <= 0)
 		return ret ? : -ENODEV;
 
-	ret = qcom_scm_set_warm_boot_addr(cpu_resume_arm, cpumask_of(cpu));
-	if (ret)
-		return ret;
-
 	return cpuidle_register(&data->cpuidle_driver, NULL);
 }
 
@@ -136,6 +132,10 @@ static int spm_cpuidle_drv_probe(struct platform_device *pdev)
 	if (!qcom_scm_is_available())
 		return -EPROBE_DEFER;
 
+	ret = qcom_scm_set_warm_boot_addr(cpu_resume_arm);
+	if (ret)
+		return dev_err_probe(&pdev->dev, ret, "set warm boot addr failed");
+
 	for_each_possible_cpu(cpu) {
 		ret = spm_cpuidle_register(&pdev->dev, cpu);
 		if (ret && ret != -ENODEV) {
diff --git a/drivers/firmware/qcom_scm.c b/drivers/firmware/qcom_scm.c
index e0fca80bf6fc..e89be2f0cdec 100644
--- a/drivers/firmware/qcom_scm.c
+++ b/drivers/firmware/qcom_scm.c
@@ -246,8 +246,7 @@ static bool __qcom_scm_is_call_available(struct device *dev, u32 svc_id,
 	return ret ? false : !!res.result[0];
 }
 
-static int qcom_scm_set_boot_addr(void *entry, const cpumask_t *cpus,
-				  const u8 *cpu_bits)
+static int qcom_scm_set_boot_addr(void *entry, const u8 *cpu_bits)
 {
 	int cpu;
 	unsigned int flags = 0;
@@ -258,7 +257,7 @@ static int qcom_scm_set_boot_addr(void *entry, const cpumask_t *cpus,
 		.owner = ARM_SMCCC_OWNER_SIP,
 	};
 
-	for_each_cpu(cpu, cpus) {
+	for_each_present_cpu(cpu) {
 		if (cpu >= QCOM_SCM_BOOT_MAX_CPUS)
 			return -EINVAL;
 		flags |= cpu_bits[cpu];
@@ -271,27 +270,25 @@ static int qcom_scm_set_boot_addr(void *entry, const cpumask_t *cpus,
 }
 
 /**
- * qcom_scm_set_warm_boot_addr() - Set the warm boot address for cpus
+ * qcom_scm_set_warm_boot_addr() - Set the warm boot address for all cpus
  * @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(void *entry)
 {
-	return qcom_scm_set_boot_addr(entry, cpus, qcom_scm_cpu_warm_bits);
+	return qcom_scm_set_boot_addr(entry, qcom_scm_cpu_warm_bits);
 }
 EXPORT_SYMBOL(qcom_scm_set_warm_boot_addr);
 
 /**
- * qcom_scm_set_cold_boot_addr() - Set the cold boot address for cpus
+ * qcom_scm_set_cold_boot_addr() - Set the cold boot address for all cpus
  * @entry: Entry point function for the cpus
- * @cpus: The cpumask of cpus that will use the entry point
  */
-int qcom_scm_set_cold_boot_addr(void *entry, const cpumask_t *cpus)
+int qcom_scm_set_cold_boot_addr(void *entry)
 {
-	return qcom_scm_set_boot_addr(entry, cpus, qcom_scm_cpu_cold_bits);
+	return qcom_scm_set_boot_addr(entry, qcom_scm_cpu_cold_bits);
 }
 EXPORT_SYMBOL(qcom_scm_set_cold_boot_addr);
 
diff --git a/include/linux/qcom_scm.h b/include/linux/qcom_scm.h
index 81cad9e1e412..048d09e1965b 100644
--- a/include/linux/qcom_scm.h
+++ b/include/linux/qcom_scm.h
@@ -63,8 +63,8 @@ enum qcom_scm_ice_cipher {
 
 extern bool qcom_scm_is_available(void);
 
-extern int qcom_scm_set_cold_boot_addr(void *entry, const cpumask_t *cpus);
-extern int qcom_scm_set_warm_boot_addr(void *entry, const cpumask_t *cpus);
+extern int qcom_scm_set_cold_boot_addr(void *entry);
+extern int qcom_scm_set_warm_boot_addr(void *entry);
 extern void qcom_scm_cpu_power_down(u32 flags);
 extern int qcom_scm_set_remote_state(u32 state, u32 id);
 
-- 
2.34.1


_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

[PATCH v3 4/4] firmware: qcom: scm: Add support for MC boot address API

[Stephan Gerhold]

It looks like the old QCOM_SCM_BOOT_SET_ADDR API is broken on some
MSM8916 firmware versions that implement the newer SMC32 calling
convention. It just returns -EINVAL no matter which arguments are
being passed.

This does not cause any problems downstream because it first tries
to use the new multi-cluster API replacement which is working fine.

Implement support for the multi-cluster variant of the SCM call
by attempting it first but still fallback to the old call in case
of an error. Also, to be absolutely sure only use the multi-cluster
variant with the SMC calling convention since older platforms should
not need this.

Signed-off-by: Stephan Gerhold <stephan@gerhold.net>
---
Changes in v3:
  - Avoid all build testing problems by setting the entry address for
    all CPUs in all affinity levels (~0ULL).
---
 drivers/firmware/qcom_scm.c | 32 ++++++++++++++++++++++++++++++--
 drivers/firmware/qcom_scm.h |  4 ++++
 2 files changed, 34 insertions(+), 2 deletions(-)

diff --git a/drivers/firmware/qcom_scm.c b/drivers/firmware/qcom_scm.c
index e89be2f0cdec..7c50169f6465 100644
--- a/drivers/firmware/qcom_scm.c
+++ b/drivers/firmware/qcom_scm.c
@@ -269,6 +269,28 @@ static int qcom_scm_set_boot_addr(void *entry, const u8 *cpu_bits)
 	return qcom_scm_call_atomic(__scm ? __scm->dev : NULL, &desc, NULL);
 }
 
+static int qcom_scm_set_boot_addr_mc(void *entry, unsigned int flags)
+{
+	struct qcom_scm_desc desc = {
+		.svc = QCOM_SCM_SVC_BOOT,
+		.cmd = QCOM_SCM_BOOT_SET_ADDR_MC,
+		.owner = ARM_SMCCC_OWNER_SIP,
+		.arginfo = QCOM_SCM_ARGS(6),
+		.args = {
+			virt_to_phys(entry),
+			/* Apply to all CPUs in all affinity levels */
+			~0ULL, ~0ULL, ~0ULL, ~0ULL,
+			flags,
+		},
+	};
+
+	/* Need a device for DMA of the additional arguments */
+	if (!__scm || __get_convention() == SMC_CONVENTION_LEGACY)
+		return -EOPNOTSUPP;
+
+	return qcom_scm_call(__scm->dev, &desc, NULL);
+}
+
 /**
  * qcom_scm_set_warm_boot_addr() - Set the warm boot address for all cpus
  * @entry: Entry point function for the cpus
@@ -278,7 +300,10 @@ static int qcom_scm_set_boot_addr(void *entry, const u8 *cpu_bits)
  */
 int qcom_scm_set_warm_boot_addr(void *entry)
 {
-	return qcom_scm_set_boot_addr(entry, qcom_scm_cpu_warm_bits);
+	if (qcom_scm_set_boot_addr_mc(entry, QCOM_SCM_BOOT_MC_FLAG_WARMBOOT))
+		/* Fallback to old SCM call */
+		return qcom_scm_set_boot_addr(entry, qcom_scm_cpu_warm_bits);
+	return 0;
 }
 EXPORT_SYMBOL(qcom_scm_set_warm_boot_addr);
 
@@ -288,7 +313,10 @@ EXPORT_SYMBOL(qcom_scm_set_warm_boot_addr);
  */
 int qcom_scm_set_cold_boot_addr(void *entry)
 {
-	return qcom_scm_set_boot_addr(entry, qcom_scm_cpu_cold_bits);
+	if (qcom_scm_set_boot_addr_mc(entry, QCOM_SCM_BOOT_MC_FLAG_COLDBOOT))
+		/* Fallback to old SCM call */
+		return qcom_scm_set_boot_addr(entry, qcom_scm_cpu_cold_bits);
+	return 0;
 }
 EXPORT_SYMBOL(qcom_scm_set_cold_boot_addr);
 
diff --git a/drivers/firmware/qcom_scm.h b/drivers/firmware/qcom_scm.h
index fd7d2a5f3e70..83d6885fb71c 100644
--- a/drivers/firmware/qcom_scm.h
+++ b/drivers/firmware/qcom_scm.h
@@ -78,9 +78,13 @@ extern int scm_legacy_call(struct device *dev, const struct qcom_scm_desc *desc,
 #define QCOM_SCM_BOOT_SET_ADDR		0x01
 #define QCOM_SCM_BOOT_TERMINATE_PC	0x02
 #define QCOM_SCM_BOOT_SET_DLOAD_MODE	0x10
+#define QCOM_SCM_BOOT_SET_ADDR_MC	0x11
 #define QCOM_SCM_BOOT_SET_REMOTE_STATE	0x0a
 #define QCOM_SCM_FLUSH_FLAG_MASK	0x3
 #define QCOM_SCM_BOOT_MAX_CPUS		4
+#define QCOM_SCM_BOOT_MC_FLAG_AARCH64	BIT(0)
+#define QCOM_SCM_BOOT_MC_FLAG_COLDBOOT	BIT(1)
+#define QCOM_SCM_BOOT_MC_FLAG_WARMBOOT	BIT(2)
 
 #define QCOM_SCM_SVC_PIL		0x02
 #define QCOM_SCM_PIL_PAS_INIT_IMAGE	0x01
-- 
2.34.1

[PATCH v3 4/4] firmware: qcom: scm: Add support for MC boot address API

[Stephan Gerhold]

It looks like the old QCOM_SCM_BOOT_SET_ADDR API is broken on some
MSM8916 firmware versions that implement the newer SMC32 calling
convention. It just returns -EINVAL no matter which arguments are
being passed.

This does not cause any problems downstream because it first tries
to use the new multi-cluster API replacement which is working fine.

Implement support for the multi-cluster variant of the SCM call
by attempting it first but still fallback to the old call in case
of an error. Also, to be absolutely sure only use the multi-cluster
variant with the SMC calling convention since older platforms should
not need this.

Signed-off-by: Stephan Gerhold <stephan@gerhold.net>
---
Changes in v3:
  - Avoid all build testing problems by setting the entry address for
    all CPUs in all affinity levels (~0ULL).
---
 drivers/firmware/qcom_scm.c | 32 ++++++++++++++++++++++++++++++--
 drivers/firmware/qcom_scm.h |  4 ++++
 2 files changed, 34 insertions(+), 2 deletions(-)

diff --git a/drivers/firmware/qcom_scm.c b/drivers/firmware/qcom_scm.c
index e89be2f0cdec..7c50169f6465 100644
--- a/drivers/firmware/qcom_scm.c
+++ b/drivers/firmware/qcom_scm.c
@@ -269,6 +269,28 @@ static int qcom_scm_set_boot_addr(void *entry, const u8 *cpu_bits)
 	return qcom_scm_call_atomic(__scm ? __scm->dev : NULL, &desc, NULL);
 }
 
+static int qcom_scm_set_boot_addr_mc(void *entry, unsigned int flags)
+{
+	struct qcom_scm_desc desc = {
+		.svc = QCOM_SCM_SVC_BOOT,
+		.cmd = QCOM_SCM_BOOT_SET_ADDR_MC,
+		.owner = ARM_SMCCC_OWNER_SIP,
+		.arginfo = QCOM_SCM_ARGS(6),
+		.args = {
+			virt_to_phys(entry),
+			/* Apply to all CPUs in all affinity levels */
+			~0ULL, ~0ULL, ~0ULL, ~0ULL,
+			flags,
+		},
+	};
+
+	/* Need a device for DMA of the additional arguments */
+	if (!__scm || __get_convention() == SMC_CONVENTION_LEGACY)
+		return -EOPNOTSUPP;
+
+	return qcom_scm_call(__scm->dev, &desc, NULL);
+}
+
 /**
  * qcom_scm_set_warm_boot_addr() - Set the warm boot address for all cpus
  * @entry: Entry point function for the cpus
@@ -278,7 +300,10 @@ static int qcom_scm_set_boot_addr(void *entry, const u8 *cpu_bits)
  */
 int qcom_scm_set_warm_boot_addr(void *entry)
 {
-	return qcom_scm_set_boot_addr(entry, qcom_scm_cpu_warm_bits);
+	if (qcom_scm_set_boot_addr_mc(entry, QCOM_SCM_BOOT_MC_FLAG_WARMBOOT))
+		/* Fallback to old SCM call */
+		return qcom_scm_set_boot_addr(entry, qcom_scm_cpu_warm_bits);
+	return 0;
 }
 EXPORT_SYMBOL(qcom_scm_set_warm_boot_addr);
 
@@ -288,7 +313,10 @@ EXPORT_SYMBOL(qcom_scm_set_warm_boot_addr);
  */
 int qcom_scm_set_cold_boot_addr(void *entry)
 {
-	return qcom_scm_set_boot_addr(entry, qcom_scm_cpu_cold_bits);
+	if (qcom_scm_set_boot_addr_mc(entry, QCOM_SCM_BOOT_MC_FLAG_COLDBOOT))
+		/* Fallback to old SCM call */
+		return qcom_scm_set_boot_addr(entry, qcom_scm_cpu_cold_bits);
+	return 0;
 }
 EXPORT_SYMBOL(qcom_scm_set_cold_boot_addr);
 
diff --git a/drivers/firmware/qcom_scm.h b/drivers/firmware/qcom_scm.h
index fd7d2a5f3e70..83d6885fb71c 100644
--- a/drivers/firmware/qcom_scm.h
+++ b/drivers/firmware/qcom_scm.h
@@ -78,9 +78,13 @@ extern int scm_legacy_call(struct device *dev, const struct qcom_scm_desc *desc,
 #define QCOM_SCM_BOOT_SET_ADDR		0x01
 #define QCOM_SCM_BOOT_TERMINATE_PC	0x02
 #define QCOM_SCM_BOOT_SET_DLOAD_MODE	0x10
+#define QCOM_SCM_BOOT_SET_ADDR_MC	0x11
 #define QCOM_SCM_BOOT_SET_REMOTE_STATE	0x0a
 #define QCOM_SCM_FLUSH_FLAG_MASK	0x3
 #define QCOM_SCM_BOOT_MAX_CPUS		4
+#define QCOM_SCM_BOOT_MC_FLAG_AARCH64	BIT(0)
+#define QCOM_SCM_BOOT_MC_FLAG_COLDBOOT	BIT(1)
+#define QCOM_SCM_BOOT_MC_FLAG_WARMBOOT	BIT(2)
 
 #define QCOM_SCM_SVC_PIL		0x02
 #define QCOM_SCM_PIL_PAS_INIT_IMAGE	0x01
-- 
2.34.1


_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

Re: [PATCH v3 1/4] cpuidle: qcom-spm: Check if any CPU is managed by SPM

[Marek Szyprowski]

On 01.12.2021 14:05, Stephan Gerhold wrote:
> At the moment, the "qcom-spm-cpuidle" platform device is always created,
> even if none of the CPUs is actually managed by the SPM. On non-qcom
> platforms this will result in infinite probe-deferral due to the
> failing qcom_scm_is_available() call.
>
> To avoid this, look through the CPU DT nodes and check if there is
> actually any CPU managed by a SPM (as indicated by the qcom,saw property).
> It should also be available because e.g. MSM8916 has qcom,saw defined
> but it's typically not enabled with ARM64/PSCI firmwares.
>
> This is needed in preparation of a follow-up change that calls
> qcom_scm_set_warm_boot_addr() a single time before registering any
> cpuidle drivers. Otherwise this call might be made even on devices
> that have this driver enabled but actually make use of PSCI.
>
> Fixes: 60f3692b5f0b ("cpuidle: qcom_spm: Detach state machine from main SPM handling")
> Reported-by: Marek Szyprowski <m.szyprowski@samsung.com>
> Link: https://lore.kernel.org/r/86e3e09f-a8d7-3dff-3fc6-ddd7d30c5d78@samsung.com/
> Signed-off-by: Stephan Gerhold <stephan@gerhold.net>

I'm fine with this fix.

Tested-by: Marek Szyprowski <m.szyprowski@samsung.com>

> ---
> Daniel, would be great if you could ack this patch and PATCH 3/4
> (the cpuidle part) if they look good to you. I think it's easiest if Bjorn
> takes them together with the qcom_scm changes through the qcom tree.
>
> Marek had an alternative fix for this [1], the difference in this patch is
> that it avoids creating the platform device entirely if no CPU is managed
> by a SPM.
>
> [1]: https://lore.kernel.org/r/20211020120643.28231-1-m.szyprowski@samsung.com/
> ---
>   drivers/cpuidle/cpuidle-qcom-spm.c | 20 ++++++++++++++++++++
>   1 file changed, 20 insertions(+)
>
> diff --git a/drivers/cpuidle/cpuidle-qcom-spm.c b/drivers/cpuidle/cpuidle-qcom-spm.c
> index 01e77913a414..5f27dcc6c110 100644
> --- a/drivers/cpuidle/cpuidle-qcom-spm.c
> +++ b/drivers/cpuidle/cpuidle-qcom-spm.c
> @@ -155,6 +155,22 @@ static struct platform_driver spm_cpuidle_driver = {
>   	},
>   };
>   
> +static bool __init qcom_spm_find_any_cpu(void)
> +{
> +	struct device_node *cpu_node, *saw_node;
> +
> +	for_each_of_cpu_node(cpu_node) {
> +		saw_node = of_parse_phandle(cpu_node, "qcom,saw", 0);
> +		if (of_device_is_available(saw_node)) {
> +			of_node_put(saw_node);
> +			of_node_put(cpu_node);
> +			return true;
> +		}
> +		of_node_put(saw_node);
> +	}
> +	return false;
> +}
> +
>   static int __init qcom_spm_cpuidle_init(void)
>   {
>   	struct platform_device *pdev;
> @@ -164,6 +180,10 @@ static int __init qcom_spm_cpuidle_init(void)
>   	if (ret)
>   		return ret;
>   
> +	/* Make sure there is actually any CPU managed by the SPM */
> +	if (!qcom_spm_find_any_cpu())
> +		return 0;
> +
>   	pdev = platform_device_register_simple("qcom-spm-cpuidle",
>   					       -1, NULL, 0);
>   	if (IS_ERR(pdev)) {

Best regards
-- 
Marek Szyprowski, PhD
Samsung R&D Institute Poland

Re: [PATCH v3 1/4] cpuidle: qcom-spm: Check if any CPU is managed by SPM

[Marek Szyprowski]

On 01.12.2021 14:05, Stephan Gerhold wrote:
> At the moment, the "qcom-spm-cpuidle" platform device is always created,
> even if none of the CPUs is actually managed by the SPM. On non-qcom
> platforms this will result in infinite probe-deferral due to the
> failing qcom_scm_is_available() call.
>
> To avoid this, look through the CPU DT nodes and check if there is
> actually any CPU managed by a SPM (as indicated by the qcom,saw property).
> It should also be available because e.g. MSM8916 has qcom,saw defined
> but it's typically not enabled with ARM64/PSCI firmwares.
>
> This is needed in preparation of a follow-up change that calls
> qcom_scm_set_warm_boot_addr() a single time before registering any
> cpuidle drivers. Otherwise this call might be made even on devices
> that have this driver enabled but actually make use of PSCI.
>
> Fixes: 60f3692b5f0b ("cpuidle: qcom_spm: Detach state machine from main SPM handling")
> Reported-by: Marek Szyprowski <m.szyprowski@samsung.com>
> Link: https://lore.kernel.org/r/86e3e09f-a8d7-3dff-3fc6-ddd7d30c5d78@samsung.com/
> Signed-off-by: Stephan Gerhold <stephan@gerhold.net>

I'm fine with this fix.

Tested-by: Marek Szyprowski <m.szyprowski@samsung.com>

> ---
> Daniel, would be great if you could ack this patch and PATCH 3/4
> (the cpuidle part) if they look good to you. I think it's easiest if Bjorn
> takes them together with the qcom_scm changes through the qcom tree.
>
> Marek had an alternative fix for this [1], the difference in this patch is
> that it avoids creating the platform device entirely if no CPU is managed
> by a SPM.
>
> [1]: https://lore.kernel.org/r/20211020120643.28231-1-m.szyprowski@samsung.com/
> ---
>   drivers/cpuidle/cpuidle-qcom-spm.c | 20 ++++++++++++++++++++
>   1 file changed, 20 insertions(+)
>
> diff --git a/drivers/cpuidle/cpuidle-qcom-spm.c b/drivers/cpuidle/cpuidle-qcom-spm.c
> index 01e77913a414..5f27dcc6c110 100644
> --- a/drivers/cpuidle/cpuidle-qcom-spm.c
> +++ b/drivers/cpuidle/cpuidle-qcom-spm.c
> @@ -155,6 +155,22 @@ static struct platform_driver spm_cpuidle_driver = {
>   	},
>   };
>   
> +static bool __init qcom_spm_find_any_cpu(void)
> +{
> +	struct device_node *cpu_node, *saw_node;
> +
> +	for_each_of_cpu_node(cpu_node) {
> +		saw_node = of_parse_phandle(cpu_node, "qcom,saw", 0);
> +		if (of_device_is_available(saw_node)) {
> +			of_node_put(saw_node);
> +			of_node_put(cpu_node);
> +			return true;
> +		}
> +		of_node_put(saw_node);
> +	}
> +	return false;
> +}
> +
>   static int __init qcom_spm_cpuidle_init(void)
>   {
>   	struct platform_device *pdev;
> @@ -164,6 +180,10 @@ static int __init qcom_spm_cpuidle_init(void)
>   	if (ret)
>   		return ret;
>   
> +	/* Make sure there is actually any CPU managed by the SPM */
> +	if (!qcom_spm_find_any_cpu())
> +		return 0;
> +
>   	pdev = platform_device_register_simple("qcom-spm-cpuidle",
>   					       -1, NULL, 0);
>   	if (IS_ERR(pdev)) {

Best regards
-- 
Marek Szyprowski, PhD
Samsung R&D Institute Poland


_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

Re: [PATCH v3 1/4] cpuidle: qcom-spm: Check if any CPU is managed by SPM

[Daniel Lezcano]

On 01/12/2021 14:05, Stephan Gerhold wrote:
> At the moment, the "qcom-spm-cpuidle" platform device is always created,
> even if none of the CPUs is actually managed by the SPM. On non-qcom
> platforms this will result in infinite probe-deferral due to the
> failing qcom_scm_is_available() call.
> 
> To avoid this, look through the CPU DT nodes and check if there is
> actually any CPU managed by a SPM (as indicated by the qcom,saw property).
> It should also be available because e.g. MSM8916 has qcom,saw defined
> but it's typically not enabled with ARM64/PSCI firmwares.
> 
> This is needed in preparation of a follow-up change that calls
> qcom_scm_set_warm_boot_addr() a single time before registering any
> cpuidle drivers. Otherwise this call might be made even on devices
> that have this driver enabled but actually make use of PSCI.
> 
> Fixes: 60f3692b5f0b ("cpuidle: qcom_spm: Detach state machine from main SPM handling")
> Reported-by: Marek Szyprowski <m.szyprowski@samsung.com>
> Link: https://lore.kernel.org/r/86e3e09f-a8d7-3dff-3fc6-ddd7d30c5d78@samsung.com/
> Signed-off-by: Stephan Gerhold <stephan@gerhold.net>
> ---
> Daniel, would be great if you could ack this patch and PATCH 3/4
> (the cpuidle part) if they look good to you. I think it's easiest if Bjorn
> takes them together with the qcom_scm changes through the qcom tree.

Acked-by: Daniel Lezcano <daniel.lezcano@linaro.org>


> Marek had an alternative fix for this [1], the difference in this patch is
> that it avoids creating the platform device entirely if no CPU is managed
> by a SPM.
> 
> [1]: https://lore.kernel.org/r/20211020120643.28231-1-m.szyprowski@samsung.com/
> ---
>  drivers/cpuidle/cpuidle-qcom-spm.c | 20 ++++++++++++++++++++
>  1 file changed, 20 insertions(+)
> 
> diff --git a/drivers/cpuidle/cpuidle-qcom-spm.c b/drivers/cpuidle/cpuidle-qcom-spm.c
> index 01e77913a414..5f27dcc6c110 100644
> --- a/drivers/cpuidle/cpuidle-qcom-spm.c
> +++ b/drivers/cpuidle/cpuidle-qcom-spm.c
> @@ -155,6 +155,22 @@ static struct platform_driver spm_cpuidle_driver = {
>  	},
>  };
>  
> +static bool __init qcom_spm_find_any_cpu(void)
> +{
> +	struct device_node *cpu_node, *saw_node;
> +
> +	for_each_of_cpu_node(cpu_node) {
> +		saw_node = of_parse_phandle(cpu_node, "qcom,saw", 0);
> +		if (of_device_is_available(saw_node)) {
> +			of_node_put(saw_node);
> +			of_node_put(cpu_node);
> +			return true;
> +		}
> +		of_node_put(saw_node);
> +	}
> +	return false;
> +}
> +
>  static int __init qcom_spm_cpuidle_init(void)
>  {
>  	struct platform_device *pdev;
> @@ -164,6 +180,10 @@ static int __init qcom_spm_cpuidle_init(void)
>  	if (ret)
>  		return ret;
>  
> +	/* Make sure there is actually any CPU managed by the SPM */
> +	if (!qcom_spm_find_any_cpu())
> +		return 0;
> +
>  	pdev = platform_device_register_simple("qcom-spm-cpuidle",
>  					       -1, NULL, 0);
>  	if (IS_ERR(pdev)) {
> 


-- 
<http://www.linaro.org/> Linaro.org │ Open source software for ARM SoCs

Follow Linaro:  <http://www.facebook.com/pages/Linaro> Facebook |
<http://twitter.com/#!/linaroorg> Twitter |
<http://www.linaro.org/linaro-blog/> Blog

Re: [PATCH v3 1/4] cpuidle: qcom-spm: Check if any CPU is managed by SPM

[Daniel Lezcano]

On 01/12/2021 14:05, Stephan Gerhold wrote:
> At the moment, the "qcom-spm-cpuidle" platform device is always created,
> even if none of the CPUs is actually managed by the SPM. On non-qcom
> platforms this will result in infinite probe-deferral due to the
> failing qcom_scm_is_available() call.
> 
> To avoid this, look through the CPU DT nodes and check if there is
> actually any CPU managed by a SPM (as indicated by the qcom,saw property).
> It should also be available because e.g. MSM8916 has qcom,saw defined
> but it's typically not enabled with ARM64/PSCI firmwares.
> 
> This is needed in preparation of a follow-up change that calls
> qcom_scm_set_warm_boot_addr() a single time before registering any
> cpuidle drivers. Otherwise this call might be made even on devices
> that have this driver enabled but actually make use of PSCI.
> 
> Fixes: 60f3692b5f0b ("cpuidle: qcom_spm: Detach state machine from main SPM handling")
> Reported-by: Marek Szyprowski <m.szyprowski@samsung.com>
> Link: https://lore.kernel.org/r/86e3e09f-a8d7-3dff-3fc6-ddd7d30c5d78@samsung.com/
> Signed-off-by: Stephan Gerhold <stephan@gerhold.net>
> ---
> Daniel, would be great if you could ack this patch and PATCH 3/4
> (the cpuidle part) if they look good to you. I think it's easiest if Bjorn
> takes them together with the qcom_scm changes through the qcom tree.

Acked-by: Daniel Lezcano <daniel.lezcano@linaro.org>


> Marek had an alternative fix for this [1], the difference in this patch is
> that it avoids creating the platform device entirely if no CPU is managed
> by a SPM.
> 
> [1]: https://lore.kernel.org/r/20211020120643.28231-1-m.szyprowski@samsung.com/
> ---
>  drivers/cpuidle/cpuidle-qcom-spm.c | 20 ++++++++++++++++++++
>  1 file changed, 20 insertions(+)
> 
> diff --git a/drivers/cpuidle/cpuidle-qcom-spm.c b/drivers/cpuidle/cpuidle-qcom-spm.c
> index 01e77913a414..5f27dcc6c110 100644
> --- a/drivers/cpuidle/cpuidle-qcom-spm.c
> +++ b/drivers/cpuidle/cpuidle-qcom-spm.c
> @@ -155,6 +155,22 @@ static struct platform_driver spm_cpuidle_driver = {
>  	},
>  };
>  
> +static bool __init qcom_spm_find_any_cpu(void)
> +{
> +	struct device_node *cpu_node, *saw_node;
> +
> +	for_each_of_cpu_node(cpu_node) {
> +		saw_node = of_parse_phandle(cpu_node, "qcom,saw", 0);
> +		if (of_device_is_available(saw_node)) {
> +			of_node_put(saw_node);
> +			of_node_put(cpu_node);
> +			return true;
> +		}
> +		of_node_put(saw_node);
> +	}
> +	return false;
> +}
> +
>  static int __init qcom_spm_cpuidle_init(void)
>  {
>  	struct platform_device *pdev;
> @@ -164,6 +180,10 @@ static int __init qcom_spm_cpuidle_init(void)
>  	if (ret)
>  		return ret;
>  
> +	/* Make sure there is actually any CPU managed by the SPM */
> +	if (!qcom_spm_find_any_cpu())
> +		return 0;
> +
>  	pdev = platform_device_register_simple("qcom-spm-cpuidle",
>  					       -1, NULL, 0);
>  	if (IS_ERR(pdev)) {
> 


-- 
<http://www.linaro.org/> Linaro.org │ Open source software for ARM SoCs

Follow Linaro:  <http://www.facebook.com/pages/Linaro> Facebook |
<http://twitter.com/#!/linaroorg> Twitter |
<http://www.linaro.org/linaro-blog/> Blog

_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

Re: [PATCH v3 3/4] firmware: qcom: scm: Drop cpumask parameter from set_boot_addr()

[Daniel Lezcano]

On 01/12/2021 14:05, Stephan Gerhold wrote:
> qcom_scm_set_cold/warm_boot_addr() currently take a cpumask parameter,
> but it's not very useful because at the end we always set the same entry
> address for all CPUs. This also allows speeding up probe of
> cpuidle-qcom-spm a bit because only one SCM call needs to be made to
> the TrustZone firmware, instead of one per CPU.
> 
> The main reason for this change is that it allows implementing the
> "multi-cluster" variant of the set_boot_addr() call more easily
> without having to rely on functions that break in certain build
> configurations or that are not exported to modules.
> 
> Signed-off-by: Stephan Gerhold <stephan@gerhold.net>

Acked-by: Daniel Lezcano <daniel.lezcano@linaro.org>

> ---
>  arch/arm/mach-qcom/platsmp.c       |  3 +--
>  drivers/cpuidle/cpuidle-qcom-spm.c |  8 ++++----
>  drivers/firmware/qcom_scm.c        | 19 ++++++++-----------
>  include/linux/qcom_scm.h           |  4 ++--
>  4 files changed, 15 insertions(+), 19 deletions(-)
> 
> diff --git a/arch/arm/mach-qcom/platsmp.c b/arch/arm/mach-qcom/platsmp.c
> index 58a4228455ce..65a0d5ce2bb3 100644
> --- a/arch/arm/mach-qcom/platsmp.c
> +++ b/arch/arm/mach-qcom/platsmp.c
> @@ -357,8 +357,7 @@ static void __init qcom_smp_prepare_cpus(unsigned int max_cpus)
>  {
>  	int cpu;
>  
> -	if (qcom_scm_set_cold_boot_addr(secondary_startup_arm,
> -					cpu_present_mask)) {
> +	if (qcom_scm_set_cold_boot_addr(secondary_startup_arm)) {
>  		for_each_present_cpu(cpu) {
>  			if (cpu == smp_processor_id())
>  				continue;
> diff --git a/drivers/cpuidle/cpuidle-qcom-spm.c b/drivers/cpuidle/cpuidle-qcom-spm.c
> index 5f27dcc6c110..beedf22cbe78 100644
> --- a/drivers/cpuidle/cpuidle-qcom-spm.c
> +++ b/drivers/cpuidle/cpuidle-qcom-spm.c
> @@ -122,10 +122,6 @@ static int spm_cpuidle_register(struct device *cpuidle_dev, int cpu)
>  	if (ret <= 0)
>  		return ret ? : -ENODEV;
>  
> -	ret = qcom_scm_set_warm_boot_addr(cpu_resume_arm, cpumask_of(cpu));
> -	if (ret)
> -		return ret;
> -
>  	return cpuidle_register(&data->cpuidle_driver, NULL);
>  }
>  
> @@ -136,6 +132,10 @@ static int spm_cpuidle_drv_probe(struct platform_device *pdev)
>  	if (!qcom_scm_is_available())
>  		return -EPROBE_DEFER;
>  
> +	ret = qcom_scm_set_warm_boot_addr(cpu_resume_arm);
> +	if (ret)
> +		return dev_err_probe(&pdev->dev, ret, "set warm boot addr failed");
> +
>  	for_each_possible_cpu(cpu) {
>  		ret = spm_cpuidle_register(&pdev->dev, cpu);
>  		if (ret && ret != -ENODEV) {
> diff --git a/drivers/firmware/qcom_scm.c b/drivers/firmware/qcom_scm.c
> index e0fca80bf6fc..e89be2f0cdec 100644
> --- a/drivers/firmware/qcom_scm.c
> +++ b/drivers/firmware/qcom_scm.c
> @@ -246,8 +246,7 @@ static bool __qcom_scm_is_call_available(struct device *dev, u32 svc_id,
>  	return ret ? false : !!res.result[0];
>  }
>  
> -static int qcom_scm_set_boot_addr(void *entry, const cpumask_t *cpus,
> -				  const u8 *cpu_bits)
> +static int qcom_scm_set_boot_addr(void *entry, const u8 *cpu_bits)
>  {
>  	int cpu;
>  	unsigned int flags = 0;
> @@ -258,7 +257,7 @@ static int qcom_scm_set_boot_addr(void *entry, const cpumask_t *cpus,
>  		.owner = ARM_SMCCC_OWNER_SIP,
>  	};
>  
> -	for_each_cpu(cpu, cpus) {
> +	for_each_present_cpu(cpu) {
>  		if (cpu >= QCOM_SCM_BOOT_MAX_CPUS)
>  			return -EINVAL;
>  		flags |= cpu_bits[cpu];
> @@ -271,27 +270,25 @@ static int qcom_scm_set_boot_addr(void *entry, const cpumask_t *cpus,
>  }
>  
>  /**
> - * qcom_scm_set_warm_boot_addr() - Set the warm boot address for cpus
> + * qcom_scm_set_warm_boot_addr() - Set the warm boot address for all cpus
>   * @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(void *entry)
>  {
> -	return qcom_scm_set_boot_addr(entry, cpus, qcom_scm_cpu_warm_bits);
> +	return qcom_scm_set_boot_addr(entry, qcom_scm_cpu_warm_bits);
>  }
>  EXPORT_SYMBOL(qcom_scm_set_warm_boot_addr);
>  
>  /**
> - * qcom_scm_set_cold_boot_addr() - Set the cold boot address for cpus
> + * qcom_scm_set_cold_boot_addr() - Set the cold boot address for all cpus
>   * @entry: Entry point function for the cpus
> - * @cpus: The cpumask of cpus that will use the entry point
>   */
> -int qcom_scm_set_cold_boot_addr(void *entry, const cpumask_t *cpus)
> +int qcom_scm_set_cold_boot_addr(void *entry)
>  {
> -	return qcom_scm_set_boot_addr(entry, cpus, qcom_scm_cpu_cold_bits);
> +	return qcom_scm_set_boot_addr(entry, qcom_scm_cpu_cold_bits);
>  }
>  EXPORT_SYMBOL(qcom_scm_set_cold_boot_addr);
>  
> diff --git a/include/linux/qcom_scm.h b/include/linux/qcom_scm.h
> index 81cad9e1e412..048d09e1965b 100644
> --- a/include/linux/qcom_scm.h
> +++ b/include/linux/qcom_scm.h
> @@ -63,8 +63,8 @@ enum qcom_scm_ice_cipher {
>  
>  extern bool qcom_scm_is_available(void);
>  
> -extern int qcom_scm_set_cold_boot_addr(void *entry, const cpumask_t *cpus);
> -extern int qcom_scm_set_warm_boot_addr(void *entry, const cpumask_t *cpus);
> +extern int qcom_scm_set_cold_boot_addr(void *entry);
> +extern int qcom_scm_set_warm_boot_addr(void *entry);
>  extern void qcom_scm_cpu_power_down(u32 flags);
>  extern int qcom_scm_set_remote_state(u32 state, u32 id);
>  
> 


-- 
<http://www.linaro.org/> Linaro.org │ Open source software for ARM SoCs

Follow Linaro:  <http://www.facebook.com/pages/Linaro> Facebook |
<http://twitter.com/#!/linaroorg> Twitter |
<http://www.linaro.org/linaro-blog/> Blog

Re: [PATCH v3 3/4] firmware: qcom: scm: Drop cpumask parameter from set_boot_addr()

[Daniel Lezcano]

On 01/12/2021 14:05, Stephan Gerhold wrote:
> qcom_scm_set_cold/warm_boot_addr() currently take a cpumask parameter,
> but it's not very useful because at the end we always set the same entry
> address for all CPUs. This also allows speeding up probe of
> cpuidle-qcom-spm a bit because only one SCM call needs to be made to
> the TrustZone firmware, instead of one per CPU.
> 
> The main reason for this change is that it allows implementing the
> "multi-cluster" variant of the set_boot_addr() call more easily
> without having to rely on functions that break in certain build
> configurations or that are not exported to modules.
> 
> Signed-off-by: Stephan Gerhold <stephan@gerhold.net>

Acked-by: Daniel Lezcano <daniel.lezcano@linaro.org>

> ---
>  arch/arm/mach-qcom/platsmp.c       |  3 +--
>  drivers/cpuidle/cpuidle-qcom-spm.c |  8 ++++----
>  drivers/firmware/qcom_scm.c        | 19 ++++++++-----------
>  include/linux/qcom_scm.h           |  4 ++--
>  4 files changed, 15 insertions(+), 19 deletions(-)
> 
> diff --git a/arch/arm/mach-qcom/platsmp.c b/arch/arm/mach-qcom/platsmp.c
> index 58a4228455ce..65a0d5ce2bb3 100644
> --- a/arch/arm/mach-qcom/platsmp.c
> +++ b/arch/arm/mach-qcom/platsmp.c
> @@ -357,8 +357,7 @@ static void __init qcom_smp_prepare_cpus(unsigned int max_cpus)
>  {
>  	int cpu;
>  
> -	if (qcom_scm_set_cold_boot_addr(secondary_startup_arm,
> -					cpu_present_mask)) {
> +	if (qcom_scm_set_cold_boot_addr(secondary_startup_arm)) {
>  		for_each_present_cpu(cpu) {
>  			if (cpu == smp_processor_id())
>  				continue;
> diff --git a/drivers/cpuidle/cpuidle-qcom-spm.c b/drivers/cpuidle/cpuidle-qcom-spm.c
> index 5f27dcc6c110..beedf22cbe78 100644
> --- a/drivers/cpuidle/cpuidle-qcom-spm.c
> +++ b/drivers/cpuidle/cpuidle-qcom-spm.c
> @@ -122,10 +122,6 @@ static int spm_cpuidle_register(struct device *cpuidle_dev, int cpu)
>  	if (ret <= 0)
>  		return ret ? : -ENODEV;
>  
> -	ret = qcom_scm_set_warm_boot_addr(cpu_resume_arm, cpumask_of(cpu));
> -	if (ret)
> -		return ret;
> -
>  	return cpuidle_register(&data->cpuidle_driver, NULL);
>  }
>  
> @@ -136,6 +132,10 @@ static int spm_cpuidle_drv_probe(struct platform_device *pdev)
>  	if (!qcom_scm_is_available())
>  		return -EPROBE_DEFER;
>  
> +	ret = qcom_scm_set_warm_boot_addr(cpu_resume_arm);
> +	if (ret)
> +		return dev_err_probe(&pdev->dev, ret, "set warm boot addr failed");
> +
>  	for_each_possible_cpu(cpu) {
>  		ret = spm_cpuidle_register(&pdev->dev, cpu);
>  		if (ret && ret != -ENODEV) {
> diff --git a/drivers/firmware/qcom_scm.c b/drivers/firmware/qcom_scm.c
> index e0fca80bf6fc..e89be2f0cdec 100644
> --- a/drivers/firmware/qcom_scm.c
> +++ b/drivers/firmware/qcom_scm.c
> @@ -246,8 +246,7 @@ static bool __qcom_scm_is_call_available(struct device *dev, u32 svc_id,
>  	return ret ? false : !!res.result[0];
>  }
>  
> -static int qcom_scm_set_boot_addr(void *entry, const cpumask_t *cpus,
> -				  const u8 *cpu_bits)
> +static int qcom_scm_set_boot_addr(void *entry, const u8 *cpu_bits)
>  {
>  	int cpu;
>  	unsigned int flags = 0;
> @@ -258,7 +257,7 @@ static int qcom_scm_set_boot_addr(void *entry, const cpumask_t *cpus,
>  		.owner = ARM_SMCCC_OWNER_SIP,
>  	};
>  
> -	for_each_cpu(cpu, cpus) {
> +	for_each_present_cpu(cpu) {
>  		if (cpu >= QCOM_SCM_BOOT_MAX_CPUS)
>  			return -EINVAL;
>  		flags |= cpu_bits[cpu];
> @@ -271,27 +270,25 @@ static int qcom_scm_set_boot_addr(void *entry, const cpumask_t *cpus,
>  }
>  
>  /**
> - * qcom_scm_set_warm_boot_addr() - Set the warm boot address for cpus
> + * qcom_scm_set_warm_boot_addr() - Set the warm boot address for all cpus
>   * @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(void *entry)
>  {
> -	return qcom_scm_set_boot_addr(entry, cpus, qcom_scm_cpu_warm_bits);
> +	return qcom_scm_set_boot_addr(entry, qcom_scm_cpu_warm_bits);
>  }
>  EXPORT_SYMBOL(qcom_scm_set_warm_boot_addr);
>  
>  /**
> - * qcom_scm_set_cold_boot_addr() - Set the cold boot address for cpus
> + * qcom_scm_set_cold_boot_addr() - Set the cold boot address for all cpus
>   * @entry: Entry point function for the cpus
> - * @cpus: The cpumask of cpus that will use the entry point
>   */
> -int qcom_scm_set_cold_boot_addr(void *entry, const cpumask_t *cpus)
> +int qcom_scm_set_cold_boot_addr(void *entry)
>  {
> -	return qcom_scm_set_boot_addr(entry, cpus, qcom_scm_cpu_cold_bits);
> +	return qcom_scm_set_boot_addr(entry, qcom_scm_cpu_cold_bits);
>  }
>  EXPORT_SYMBOL(qcom_scm_set_cold_boot_addr);
>  
> diff --git a/include/linux/qcom_scm.h b/include/linux/qcom_scm.h
> index 81cad9e1e412..048d09e1965b 100644
> --- a/include/linux/qcom_scm.h
> +++ b/include/linux/qcom_scm.h
> @@ -63,8 +63,8 @@ enum qcom_scm_ice_cipher {
>  
>  extern bool qcom_scm_is_available(void);
>  
> -extern int qcom_scm_set_cold_boot_addr(void *entry, const cpumask_t *cpus);
> -extern int qcom_scm_set_warm_boot_addr(void *entry, const cpumask_t *cpus);
> +extern int qcom_scm_set_cold_boot_addr(void *entry);
> +extern int qcom_scm_set_warm_boot_addr(void *entry);
>  extern void qcom_scm_cpu_power_down(u32 flags);
>  extern int qcom_scm_set_remote_state(u32 state, u32 id);
>  
> 


-- 
<http://www.linaro.org/> Linaro.org │ Open source software for ARM SoCs

Follow Linaro:  <http://www.facebook.com/pages/Linaro> Facebook |
<http://twitter.com/#!/linaroorg> Twitter |
<http://www.linaro.org/linaro-blog/> Blog

_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

Re: [PATCH v3 0/4] qcom_scm: Add support for MC boot address API

[Bjorn Andersson]

On Wed, 1 Dec 2021 14:05:01 +0100, Stephan Gerhold wrote:
> The "firmware: qcom: scm: Add support for MC boot address API" commit
> was reverted again in 5.16 [1]. This is a new attempt to add it back
> with much less potential build problems.
> 
> For that I first simplify the existing qcom_scm_set_cold/warm_boot_addr()
> implementations. The idea is that cpu_logical_map(), MPIDR_AFFINITY_LEVEL()
> etc are not needed if we just set the entry address for all CPUs.
> Nothing in the mainline tree actually requires setting a different entry
> address for one particular CPU and I cannot really think of a use case for this.
> 
> [...]

Applied, thanks!

[1/4] cpuidle: qcom-spm: Check if any CPU is managed by SPM
      commit: 0ee30ace67e425ab83a1673bf51f50b577328cf9
[2/4] firmware: qcom: scm: Simplify set_cold/warm_boot_addr()
      commit: 7734c4b507cefbcf2f7a2a806e79c43e52528c5f
[3/4] firmware: qcom: scm: Drop cpumask parameter from set_boot_addr()
      commit: 52beb1fc237d67cdc64277dc90047767a6fc52d7
[4/4] firmware: qcom: scm: Add support for MC boot address API
      commit: f60a317bcbea5c5b8923d6de6c7288850fdd83fb

Best regards,
-- 
Bjorn Andersson <bjorn.andersson@linaro.org>

Re: [PATCH v3 0/4] qcom_scm: Add support for MC boot address API

[Bjorn Andersson]

On Wed, 1 Dec 2021 14:05:01 +0100, Stephan Gerhold wrote:
> The "firmware: qcom: scm: Add support for MC boot address API" commit
> was reverted again in 5.16 [1]. This is a new attempt to add it back
> with much less potential build problems.
> 
> For that I first simplify the existing qcom_scm_set_cold/warm_boot_addr()
> implementations. The idea is that cpu_logical_map(), MPIDR_AFFINITY_LEVEL()
> etc are not needed if we just set the entry address for all CPUs.
> Nothing in the mainline tree actually requires setting a different entry
> address for one particular CPU and I cannot really think of a use case for this.
> 
> [...]

Applied, thanks!

[1/4] cpuidle: qcom-spm: Check if any CPU is managed by SPM
      commit: 0ee30ace67e425ab83a1673bf51f50b577328cf9
[2/4] firmware: qcom: scm: Simplify set_cold/warm_boot_addr()
      commit: 7734c4b507cefbcf2f7a2a806e79c43e52528c5f
[3/4] firmware: qcom: scm: Drop cpumask parameter from set_boot_addr()
      commit: 52beb1fc237d67cdc64277dc90047767a6fc52d7
[4/4] firmware: qcom: scm: Add support for MC boot address API
      commit: f60a317bcbea5c5b8923d6de6c7288850fdd83fb

Best regards,
-- 
Bjorn Andersson <bjorn.andersson@linaro.org>

_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

Re: [PATCH v3 0/4] qcom_scm: Add support for MC boot address API

[patchwork-bot+linux-arm-msm]

Hello:

This series was applied to qcom/linux.git (for-next)
by Bjorn Andersson <bjorn.andersson@linaro.org>:

On Wed,  1 Dec 2021 14:05:01 +0100 you wrote:
> The "firmware: qcom: scm: Add support for MC boot address API" commit
> was reverted again in 5.16 [1]. This is a new attempt to add it back
> with much less potential build problems.
> 
> For that I first simplify the existing qcom_scm_set_cold/warm_boot_addr()
> implementations. The idea is that cpu_logical_map(), MPIDR_AFFINITY_LEVEL()
> etc are not needed if we just set the entry address for all CPUs.
> Nothing in the mainline tree actually requires setting a different entry
> address for one particular CPU and I cannot really think of a use case for this.
> 
> [...]

Here is the summary with links:
  - [v3,1/4] cpuidle: qcom-spm: Check if any CPU is managed by SPM
    (no matching commit)
  - [v3,2/4] firmware: qcom: scm: Simplify set_cold/warm_boot_addr()
    https://git.kernel.org/qcom/c/7734c4b507ce
  - [v3,3/4] firmware: qcom: scm: Drop cpumask parameter from set_boot_addr()
    https://git.kernel.org/qcom/c/52beb1fc237d
  - [v3,4/4] firmware: qcom: scm: Add support for MC boot address API
    https://git.kernel.org/qcom/c/f60a317bcbea

You are awesome, thank you!
-- 
Deet-doot-dot, I am a bot.
https://korg.docs.kernel.org/patchwork/pwbot.html