On 18.04.19 10:14, Biju Das wrote:
Hi Oleksandr,
Hi Biju
Thanks for the patch.
Subject: [PATCH] ARM: mach-shmobile: Don't configure ARCH timer if PSCI is
enabled
From: Oleksandr Tyshchenko <oleksandr_tyshchenko@xxxxxxxx>
If CONFIG_PSCI is enabled then most likely we are running on PSCI-enabled
U-Boot which, we assume, has already taken care of configuring ARCH timer
stuff before switching to non-secure mode.
Signed-off-by: Oleksandr Tyshchenko <oleksandr_tyshchenko@xxxxxxxx>
---
A bit of context here...
We are highly interested in Renesas "Stout" board support (r8a7790) in Xen
hypervisor. The reason is to have fully supported HW for performing
"OSSTEST" (Xen automatic test system) on ARM32.
To reach that target we need a "generic way" for the secondary CPU cores
bring up and switching them to non-secure hyp mode.
So, the PSCI as a generic well-known way to bring up CPUs, was chosen for
that purpose.
You can find corresponding patches for U-Boot here:
http://u-boot.10912.n7.nabble.com/PATCH-0-3-PSCI-support-for-r8a7790-
SoC-Lager-Stout-boards-td357352.html
You can find corresponding patches for Xen hypervisor here:
https://www.mail-archive.com/xen-
devel@xxxxxxxxxxxxxxxxxxxx/msg43332.html
To sumarize:
Together with enabling CONFIG_PSCI in shmobile_defconfig, current patch is
a minimal required change needed to run mainline Linux on top of PSCI-
enabled U-Boot.
There is no need to modify device tree. U-Boot will take care of inserting
proper "enable-method" strings in CPU nodes.
---
arch/arm/mach-shmobile/setup-rcar-gen2.c | 9 ++++++++-
1 file changed, 8 insertions(+), 1 deletion(-)
diff --git a/arch/arm/mach-shmobile/setup-rcar-gen2.c b/arch/arm/mach-
shmobile/setup-rcar-gen2.c
index eea60b2..bac4490 100644
--- a/arch/arm/mach-shmobile/setup-rcar-gen2.c
+++ b/arch/arm/mach-shmobile/setup-rcar-gen2.c
@@ -32,7 +32,7 @@ static const struct of_device_id cpg_matches[]
__initconst = {
{ /* sentinel */ }
};
-static unsigned int __init get_extal_freq(void)
+static unsigned int __init __maybe_unused get_extal_freq(void)
{
const struct of_device_id *match;
struct device_node *cpg, *extal;
@@ -60,6 +60,12 @@ static unsigned int __init get_extal_freq(void)
void __init rcar_gen2_timer_init(void)
{
+/*
+ * If CONFIG_PSCI is enabled then most likely we are running on
+PSCI-enabled
+ * U-Boot which, we assume, has already taken care of configuring ARCH
+timer
+ * stuff before switching to non-secure mode.
+ */
+#if !defined(CONFIG_ARM_PSCI)
Is it required? If you see the below comment, it is already taken care
by the below code.
Is your code entering into this block, when booting the kernel in NS mode?
No, the code below seems to do the proper things. If arch timer is
configured in U-Boot (enabled and programmed with correct freq) then
that code doesn't try to update it.
But "#if !defined(CONFIG_ARM_PSCI)" wraps secure_cntvoff_init() call as
well, which resets CNTVOFF,
and this is something I tried to avoid the first from being executed on
a CPU, which is in SVC mode (when running on top of Xen).
I was thinking whether I should wrap arch timer updating procedure as
well and decided to do so, but probably I shouldn't have.
<snip>
/*
87 * Update the timer if it is either not running, or is not at the
88 * right frequency. The timer is only configurable in secure mode
89 * so this avoids an abort if the loader started the timer and
90 * entered the kernel in non-secure mode.
91 */
92
93 if ((ioread32(base + CNTCR) & 1) == 0 ||
94 ioread32(base + CNTFID0) != freq) {
95 /* Update registers with correct frequency */
96 iowrite32(freq, base + CNTFID0);
97 asm volatile("mcr p15, 0, %0, c14, c0, 0" : : "r" (freq));
98
99 /* make sure arch timer is started by setting bit 0 of CNTCR */
100 iowrite32(1, base + CNTCR);
101 }
Regards,
Biju
void __iomem *base;
u32 freq;
@@ -101,6 +107,7 @@ void __init rcar_gen2_timer_init(void)
}
iounmap(base);
+#endif /* #if !defined(CONFIG_ARM_PSCI) */
of_clk_init(NULL);
timer_probe();
--
2.7.4
--
Regards,
Oleksandr Tyshchenko
