On Fri, Nov 14 2014 at 01:30 -0700, Daniel Lezcano wrote:
On 10/25/2014 01:40 AM, Lina Iyer wrote:
Set the warmboot address using an SCM call, only if the new address is
different than the old one.
Please could you elaborate why a new address can be changed ?
Hotplug could have a different warmboot address.
Signed-off-by: Lina Iyer <lina.iyer@xxxxxxxxxx>
---
drivers/soc/qcom/scm-boot.c | 22 ++++++++++++++++++++++
include/soc/qcom/scm-boot.h | 1 +
2 files changed, 23 insertions(+)
diff --git a/drivers/soc/qcom/scm-boot.c b/drivers/soc/qcom/scm-boot.c
index 60ff7b4..5710967 100644
--- a/drivers/soc/qcom/scm-boot.c
+++ b/drivers/soc/qcom/scm-boot.c
@@ -37,3 +37,25 @@ int scm_set_boot_addr(phys_addr_t addr, int flags)
&cmd, sizeof(cmd), NULL, 0);
}
EXPORT_SYMBOL(scm_set_boot_addr);
+
+
extra line.
+int scm_set_warm_boot_addr(void *entry, int cpu)
+{
+ static int flags[NR_CPUS] = {
+ SCM_FLAG_WARMBOOT_CPU0,
+ SCM_FLAG_WARMBOOT_CPU1,
+ SCM_FLAG_WARMBOOT_CPU2,
+ SCM_FLAG_WARMBOOT_CPU3,
+ };
Please do not do that, you don't know what NR_CPUS value could be in
the future with the single kernel image and that could lead to a bug
very hard to find. The kernel stack is 4096.
Move this out of the function:
static int scm_flags[] = {
SCM_FLAG_WARMBOOT_CPU0,
SCM_FLAG_WARMBOOT_CPU1,
SCM_FLAG_WARMBOOT_CPU2,
SCM_FLAG_WARMBOOT_CPU3,
};
Sure.
+ static DEFINE_PER_CPU(void *, last_known_entry);
It sounds odd to add those static declaration here even if I
understand that is to encapsulate them.
Sure.
+ int ret;
+
+ if (entry == per_cpu(last_known_entry, cpu))
+ return 0;
My question is: why scm_set_warm_boot_addr could be called with
different addresses ?
If this is really needed, please replace the per_cpu variable by:
struct scm_boot_addr {
int flag;
phys_addr_t entry;
};
static struct scm_boot_addr scm_flags[] = {
{ SCM_FLAG_WARMBOOT_CPU0, },
{ SCM_FLAG_WARMBOOT_CPU1, },
{ SCM_FLAG_WARMBOOT_CPU2, },
{ SCM_FLAG_WARMBOOT_CPU3, },
};
Hmm.. OK.
+ ret = scm_set_boot_addr(virt_to_phys(entry), flags[cpu]);
+ if (!ret)
+ per_cpu(last_known_entry, cpu) = entry;
+
+ return ret;
+}
diff --git a/include/soc/qcom/scm-boot.h b/include/soc/qcom/scm-boot.h
index 02b445c..100938b 100644
--- a/include/soc/qcom/scm-boot.h
+++ b/include/soc/qcom/scm-boot.h
@@ -22,5 +22,6 @@
#define SCM_FLAG_WARMBOOT_CPU3 0x40
By the way, if you look for encapsulation, perhaps these macros could
be moved into scm-boot.c, no ?
Dont think anybody outside the file needs it.
int scm_set_boot_addr(phys_addr_t addr, int flags);
+int scm_set_warm_boot_addr(void *entry, int cpu);
#endif
--
<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
--
To unsubscribe from this list: send the line "unsubscribe devicetree" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at http://vger.kernel.org/majordomo-info.html
