On 2/7/19 4:28 PM, Oleksandr wrote:
On 05.02.19 20:48, Marek Vasut wrote:
Hi Marek
Hi,
On 1/31/19 6:38 PM, Oleksandr Tyshchenko wrote:
From: Oleksandr Tyshchenko oleksandr_tyshchenko@epam.com
Both Lager and Stout boards are based on r8a7790 SoC.
Leave platform specific functions for bringing seconadary CPUs up empty, since our target is to use PSCI for that.
Also take care of updating arch timer while we are in secure mode.
Signed-off-by: Oleksandr Tyshchenko oleksandr_tyshchenko@epam.com
arch/arm/mach-rmobile/Kconfig.32 | 4 ++++ board/renesas/lager/lager.c | 51 ++++++++++++++++++++++++++++++++++++++++ board/renesas/stout/stout.c | 51 ++++++++++++++++++++++++++++++++++++++++ include/configs/lager.h | 3 +++ include/configs/stout.h | 3 +++ 5 files changed, 112 insertions(+)
diff --git a/arch/arm/mach-rmobile/Kconfig.32 b/arch/arm/mach-rmobile/Kconfig.32 index 076a019..a2e9e3d 100644 --- a/arch/arm/mach-rmobile/Kconfig.32 +++ b/arch/arm/mach-rmobile/Kconfig.32 @@ -76,6 +76,8 @@ config TARGET_LAGER select SUPPORT_SPL select USE_TINY_PRINTF imply CMD_DM + select CPU_V7_HAS_NONSEC + select CPU_V7_HAS_VIRT
Shouldn't this be a H2 CPU property instead of a board property ?
Probably yes, sounds reasonable. I will move these options under "config R8A7790".
Does this apply to M2* too , since it has CA15 cores as well ?
I think, yes. But, without PSCI support being implemented for M2*, I think it is not worth to select these options for it as well.
It's basically the same SoC with two CPU cores less, how would that PSCI be any different on M2 ?
config TARGET_KZM9G bool "KZM9D board" @@ -115,6 +117,8 @@ config TARGET_STOUT select SUPPORT_SPL select USE_TINY_PRINTF imply CMD_DM + select CPU_V7_HAS_NONSEC + select CPU_V7_HAS_VIRT endchoice diff --git a/board/renesas/lager/lager.c b/board/renesas/lager/lager.c index 062e88c..9b96cc4 100644 --- a/board/renesas/lager/lager.c +++ b/board/renesas/lager/lager.c @@ -76,6 +76,53 @@ int board_early_init_f(void) return 0; } +#ifdef CONFIG_ARMV7_NONSEC +#define TIMER_BASE 0xE6080000 +#define TIMER_CNTCR 0x00 +#define TIMER_CNTFID0 0x20
+/*
- Taking into the account that arch timer is only configurable in
secure mode
- and we are going to enter kernel/hypervisor in a non-secure mode,
update
- arch timer right now to avoid possible issues. Make sure arch
timer is
- enabled and configured to use proper frequency.
- */
+static void rcar_gen2_timer_init(void) +{ + u32 freq = RMOBILE_XTAL_CLK / 2;
+ /* + * Update the arch timer if it is either not running, or is not at the + * right frequency. + */ + if ((readl(TIMER_BASE + TIMER_CNTCR) & 1) == 0 || + readl(TIMER_BASE + TIMER_CNTFID0) != freq) {
What is this check about ?
Actually, this code for updating arch timer was borrowed from Linux almost as is.
Code in Linux uses this check in order to update timer only if required (either timer disabled or uses wrong freq).
This check avoids abort in Linux if loader has already updated timer and switched to non-secure mode.
But here in U-Boot, while we are still in secure mode, we can safely remove this check and always update the timer. Shall I remove this check?
Shouldn't the timer be correctly configured by U-Boot in the first place? And shouldn't this be done by timer driver rather than some ad-hoc init code?
+ /* Update registers with correct frequency */ + writel(freq, TIMER_BASE + TIMER_CNTFID0); + asm volatile("mcr p15, 0, %0, c14, c0, 0" : : "r" (freq));
+ /* Make sure arch timer is started by setting bit 0 of CNTCR */ + writel(1, TIMER_BASE + TIMER_CNTCR);
Shouldn't this be in the timer driver instead ?
Which timer driver? Probably, I missed something, but I didn't find any arch timer driver being used for Gen2.
I see that TMU timer (arch/sh/lib/time.c) is used as a system timer, but it is yet another IP.
Would it be correct, if I move arch timer updating code to arch/arm/mach-rmobile directory?
Actually, at the same location the corresponding code lives in Linux:
https://elixir.bootlin.com/linux/v5.0-rc5/source/arch/arm/mach-shmobile/setu...
Presumably if this is something else than TMU, it needs proper driver that goes into drivers/ .
+ } +}
+/*
- In order not to break compilation if someone decides to build
with PSCI
- support disabled, keep these dummy functions.
- */
That's currently the only use-case.
Shall I drop this comment and dummy functions below?
Since there is no PSCI support, yes.
[...]
I'd like to have a cover letter go with V2, which describes what you're trying to achieve here.