[U-Boot] [PATCH 1/3] ARM: rmobile: Switch CPU to non-secure HYP mode for r8a7790 based boards
Oleksandr
olekstysh at gmail.com
Fri Feb 8 11:40:42 UTC 2019
On 07.02.19 19:19, Oleksandr wrote:
>
> 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_tyshchenko at 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 at 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.
From the first look:
1. It should be different total number of CPUs:
For R8A7790 we have the following:
#define R8A7790_PSCI_NR_CPUS 8
But for R8A7791 it seems we need to use:
#define R8A7791_PSCI_NR_CPUS 2
2. It should be new pm-r8a7791.c file which will don't have any CA7
related stuff (CPU cores, SCU, etc).
Or it should be the single pm-r8a779x.c which must distinguish what SoC
is being used and do proper things.
>>
>>>>> 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
More information about the U-Boot
mailing list