On 07.02.19 17:49, Marek Vasut wrote:
On 2/7/19 4:28 PM, Oleksandr wrote:
On 05.02.19 20:48, Marek Vasut wrote:

Hi Marek
Hi,

Hi,


On 1/31/19 6:38 PM, Oleksandr Tyshchenko wrote:
From: Oleksandr Tyshchenko <oleksandr_tyshche...@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_tyshche...@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 ?
I need some time to investigate. I will come up with an answer later.

   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?

Yes, this timer should be correctly configured by U-Boot. And yes, the timer

configuration code should be in a proper place.


+        /* 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/setup-rcar-gen2.c#L61
Presumably if this is something else than TMU, it needs proper driver
that goes into drivers/ .

Did I get your point correctly that new driver (specially for Gen2 arch timer) should be introduced in U-Boot and

the only one thing it is intended to do is to configure that timer for the future use by Linux/Hypervisor?

If yes, the only question I have is how that new driver is going to be populated? There is no corresponding node for arch timer in the device tree.

https://elixir.bootlin.com/linux/v5.0-rc5/source/arch/arm/boot/dts/r8a7790.dtsi

So, do I need specially for this case add arch timer node with reg and compatible properties?

Sorry if I ask obvious questions, not familiar enough with U-Boot.


+    }
+}
+
+/*
+ * 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.

Understand.


[...]

I'd like to have a cover letter go with V2, which describes what you're
trying to achieve here.

Yes, sure. Cover letter is present for the current V1 as well.

I thought that I had CC-ed all.

This is a link to it:

http://u-boot.10912.n7.nabble.com/PATCH-0-3-PSCI-support-for-r8a7790-SoC-Lager-Stout-boards-td357352.html


--
Regards,

Oleksandr Tyshchenko

_______________________________________________
U-Boot mailing list
U-Boot@lists.denx.de
https://lists.denx.de/listinfo/u-boot

Reply via email to