Re: [U-Boot] [PATCH 1/3] ARM: rmobile: Switch CPU to non-secure HYP mode for r8a7790 based boards

2019-03-18 Thread Patrick DELAUNAY
Hi,

> From: Oleksandr 
> Sent: jeudi 14 mars 2019 12:37
> 
> 
> On 14.03.19 02:16, Marek Vasut wrote:
> > On 2/12/19 8:52 PM, Oleksandr wrote:
> >
> > Hi,
> 
> Hi
> 
> >
> > [...]
> >
> > I was thinking about this whole PSCI situation and how it's all
> > implemented today. Basically what we do is generate a separate reduced
> > shred of U-Boot, which will remain resident in memory and which could
> > be called by some other software. That shred is a piece of U-Boot
> > proper, but a lot of stuff is just torn away at runtime, so it's not
> > the whole binary, just tattered remnants of it.
> >
> > I really do not like this approach to the whole U-Boot PSCI in the
> > first place, it seems really fragile and dangerous.
> >
> > But look, U-Boot already has support for U-Boot SPL/TPL, which is
> > small, can contain a full driver model, drivers and all the necessary
> > support functionality. It is built from the same sources, at the same
> > time, but it is not a shred of U-Boot proper but rather a separate entity.
> >
> > So I wonder, can't we use this U-Boot SPL/TPL as the piece of code
> > which would remain resident in memory and handle the PSCI calls
> > instead ? I think U-Boot can load such a code or it could be somehow
> > bundled with U-Boot proper (using a fitImage maybe ?). This way you
> > won't have to re-implement all the drivers in arch/arm/, the DM/DT
> > remains in place and the whole PSCI handler would be self-contained,
> > so no need to fiddle with linker sections of U-Boot itself.
> >
> > I am CCing other people who use this PSCI stuff in U-Boot, maybe they
> > have some thoughts on this approach.
> 
> Thank you for your honest answer.
> 
> Let's see what the community will say.

I will answer for STM32MP1 platform.

On STM32MP157 chip, the PSCI support is expected minimal in U-Boot for basic 
boot defconfig.
  ROM code => SPL => U-Boot (install PSCI monitor) => Kernel
  => only CPU_ON/OFF for hotplug needed by kernel but no power management.

For full PSCI support (standby modes), we are using TF-A secure monitor 
(trusted boot defconfig) with full power support or OP-TEE secure monitor
  ROM code => TF-A (BL2 install secure monitor = BL32 : SP min) => U-Boot (non 
secure world) => Kernel
  ROM code => TF-A (BL2)  =>  secure OS = OP-TEE => U-Boot (non secure world) 
=> Kernel
In these 2 cases U-Boot access secure resource with SPCI request (SMC call).


But it in the future of the basic boot, if we want add a full PSCI support in 
U-Boot for power management, we need at least access to some resources:
- I2C driver
- STPMIC1 driver
- DDR driver (to switch the DDR in self refresh mode)
- Clock /reset driver

I agree all this part are already managed by U-Boot drivers / u-class.
But we need also a way to have access to board information / DT description.

It is also why I don't implement the function psci_system_off() in 
./arch/arm/mach-stm32mp/psci.c
=> I don't want recode a I2C driver in PSCI code...

void __secure psci_system_off(u32 function_id)
{
/* System Off is not managed, waiting user power off
 * TODO: handle I2C write in PMIC Main Control register bit 0 = SWOFF
 */
while (1)
wfi();
}

So have a PSCI firmware with driver model based on the U-Boot code (as it is 
done for SPL/TPL) seems for me a good solution.

Question: what the right moment to install  the secure monitor ?

+ SPL=> U-Boot  then U-Boot can use PSCI installed firmware but no access of 
secure resource in U-Boot
+ U-Boot => Kernel  then U-Boot is executed in secure world can't use the 
PSCI firmware (as today)

PS: the first solution is used for some board when the secure monitor of TF-A = 
BL32 is installed by U-Boot (see CONFIG_SPL_ATF)

ROM code => SPL : install BL32 compiled from TF-A code => U-Boot (non 
secure world) => Kernel

> 
> >
> > And I apologize again for taking this long to reply.
> 
> No problem.
> 
> 
> >
> > [...]
> >
> --
> Regards,
> 
> Oleksandr Tyshchenko

Regards 

Patrick

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


Re: [U-Boot] [PATCH 1/3] ARM: rmobile: Switch CPU to non-secure HYP mode for r8a7790 based boards

2019-03-14 Thread Oleksandr


On 14.03.19 02:16, Marek Vasut wrote:

On 2/12/19 8:52 PM, Oleksandr wrote:

Hi,


Hi



[...]

I was thinking about this whole PSCI situation and how it's all
implemented today. Basically what we do is generate a separate reduced
shred of U-Boot, which will remain resident in memory and which could be
called by some other software. That shred is a piece of U-Boot proper,
but a lot of stuff is just torn away at runtime, so it's not the whole
binary, just tattered remnants of it.

I really do not like this approach to the whole U-Boot PSCI in the first
place, it seems really fragile and dangerous.

But look, U-Boot already has support for U-Boot SPL/TPL, which is small,
can contain a full driver model, drivers and all the necessary support
functionality. It is built from the same sources, at the same time, but
it is not a shred of U-Boot proper but rather a separate entity.

So I wonder, can't we use this U-Boot SPL/TPL as the piece of code which
would remain resident in memory and handle the PSCI calls instead ? I
think U-Boot can load such a code or it could be somehow bundled with
U-Boot proper (using a fitImage maybe ?). This way you won't have to
re-implement all the drivers in arch/arm/, the DM/DT remains in place
and the whole PSCI handler would be self-contained, so no need to fiddle
with linker sections of U-Boot itself.

I am CCing other people who use this PSCI stuff in U-Boot, maybe they
have some thoughts on this approach.


Thank you for your honest answer.

Let's see what the community will say.




And I apologize again for taking this long to reply.


No problem.




[...]


--
Regards,

Oleksandr Tyshchenko

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


Re: [U-Boot] [PATCH 1/3] ARM: rmobile: Switch CPU to non-secure HYP mode for r8a7790 based boards

2019-03-13 Thread Ley Foon Tan
On Thu, 2019-03-14 at 01:16 +0100, Marek Vasut wrote:
> On 2/12/19 8:52 PM, Oleksandr wrote:
> 
> Hi,
> 
> [...]
> 
> I was thinking about this whole PSCI situation and how it's all
> implemented today. Basically what we do is generate a separate
> reduced
> shred of U-Boot, which will remain resident in memory and which could
> be
> called by some other software. That shred is a piece of U-Boot
> proper,
> but a lot of stuff is just torn away at runtime, so it's not the
> whole
> binary, just tattered remnants of it.
> 
> I really do not like this approach to the whole U-Boot PSCI in the
> first
> place, it seems really fragile and dangerous.
> 
> But look, U-Boot already has support for U-Boot SPL/TPL, which is
> small,
> can contain a full driver model, drivers and all the necessary
> support
> functionality. It is built from the same sources, at the same time,
> but
> it is not a shred of U-Boot proper but rather a separate entity.
> 
> So I wonder, can't we use this U-Boot SPL/TPL as the piece of code
> which
> would remain resident in memory and handle the PSCI calls instead ? I
> think U-Boot can load such a code or it could be somehow bundled with
> U-Boot proper (using a fitImage maybe ?). This way you won't have to
> re-implement all the drivers in arch/arm/, the DM/DT remains in place
> and the whole PSCI handler would be self-contained, so no need to
> fiddle
> with linker sections of U-Boot itself.
> 
> I am CCing other people who use this PSCI stuff in U-Boot, maybe they
> have some thoughts on this approach.
> 
> And I apologize again for taking this long to reply.
> 
> [...]
> 
Adding Chin Liang and Jeremy.


Regards
Ley Foon
___
U-Boot mailing list
U-Boot@lists.denx.de
https://lists.denx.de/listinfo/u-boot


Re: [U-Boot] [PATCH 1/3] ARM: rmobile: Switch CPU to non-secure HYP mode for r8a7790 based boards

2019-03-13 Thread Marek Vasut
On 2/12/19 8:52 PM, Oleksandr wrote:

Hi,

[...]

I was thinking about this whole PSCI situation and how it's all
implemented today. Basically what we do is generate a separate reduced
shred of U-Boot, which will remain resident in memory and which could be
called by some other software. That shred is a piece of U-Boot proper,
but a lot of stuff is just torn away at runtime, so it's not the whole
binary, just tattered remnants of it.

I really do not like this approach to the whole U-Boot PSCI in the first
place, it seems really fragile and dangerous.

But look, U-Boot already has support for U-Boot SPL/TPL, which is small,
can contain a full driver model, drivers and all the necessary support
functionality. It is built from the same sources, at the same time, but
it is not a shred of U-Boot proper but rather a separate entity.

So I wonder, can't we use this U-Boot SPL/TPL as the piece of code which
would remain resident in memory and handle the PSCI calls instead ? I
think U-Boot can load such a code or it could be somehow bundled with
U-Boot proper (using a fitImage maybe ?). This way you won't have to
re-implement all the drivers in arch/arm/, the DM/DT remains in place
and the whole PSCI handler would be self-contained, so no need to fiddle
with linker sections of U-Boot itself.

I am CCing other people who use this PSCI stuff in U-Boot, maybe they
have some thoughts on this approach.

And I apologize again for taking this long to reply.

[...]

-- 
Best regards,
Marek Vasut
___
U-Boot mailing list
U-Boot@lists.denx.de
https://lists.denx.de/listinfo/u-boot


Re: [U-Boot] [PATCH 1/3] ARM: rmobile: Switch CPU to non-secure HYP mode for r8a7790 based boards

2019-02-27 Thread Marek Vasut
On 2/26/19 7:37 PM, Oleksandr wrote:
> 
> Hi, Marek

Hi,

>>>
 2. It should be new pm-r8a7791.c file which will don't have any CA7
 related stuff (CPU cores, SCU, etc).
>>> I'd like to have a generic pm-gen2.c file , which parses the DT and
>>> figures the configuration out that way. We are trying to get rid of all
>>> the ad-hoc hardcoded configuration stuff in favor of DT.
>>>
 Or it should be the single pm-r8a779x.c which must distinguish what SoC
 is being used and do proper things.
>>> Right
>>
>>
>> I agree to have a single generic pm-gen2.c file which covers all Gen2
>> SoCs.
>>
>> But, unfortunately, I only have Stout boards (H2), and therefore can
>> check only on them. This is why the current patch series adds support
>> for H2 SoC only.
>>
>> Theoretically, I could add support for M2 as well, but, I wouldn't
>> have possibility to check.
>>
>>
>> I would focus on the r8a7790 for now, as the Stout board is our
>> nearest target which we want to support in Xen and the PSCI feature is
>> a mandatory option.
>>
>> What do you think?
> 
> Could you, please, answer that question...

I am sorry, I am too busy right now. I will answer that once I can
properly study the problem and give you a useful answer.

-- 
Best regards,
Marek Vasut
___
U-Boot mailing list
U-Boot@lists.denx.de
https://lists.denx.de/listinfo/u-boot


Re: [U-Boot] [PATCH 1/3] ARM: rmobile: Switch CPU to non-secure HYP mode for r8a7790 based boards

2019-02-26 Thread Oleksandr


Hi, Marek




2. It should be new pm-r8a7791.c file which will don't have any CA7
related stuff (CPU cores, SCU, etc).

I'd like to have a generic pm-gen2.c file , which parses the DT and
figures the configuration out that way. We are trying to get rid of all
the ad-hoc hardcoded configuration stuff in favor of DT.


Or it should be the single pm-r8a779x.c which must distinguish what SoC
is being used and do proper things.

Right



I agree to have a single generic pm-gen2.c file which covers all Gen2 
SoCs.


But, unfortunately, I only have Stout boards (H2), and therefore can 
check only on them. This is why the current patch series adds support 
for H2 SoC only.


Theoretically, I could add support for M2 as well, but, I wouldn't 
have possibility to check.



I would focus on the r8a7790 for now, as the Stout board is our 
nearest target which we want to support in Xen and the PSCI feature is 
a mandatory option.


What do you think?


Could you, please, answer that question...







--
Regards,

Oleksandr Tyshchenko

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


Re: [U-Boot] [PATCH 1/3] ARM: rmobile: Switch CPU to non-secure HYP mode for r8a7790 based boards

2019-02-12 Thread Oleksandr


On 09.02.19 18:37, Marek Vasut wrote:

Hi


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

This information should be in the DT for each SoC, so you should extract
it from there.


2. It should be new pm-r8a7791.c file which will don't have any CA7
related stuff (CPU cores, SCU, etc).

I'd like to have a generic pm-gen2.c file , which parses the DT and
figures the configuration out that way. We are trying to get rid of all
the ad-hoc hardcoded configuration stuff in favor of DT.


Or it should be the single pm-r8a779x.c which must distinguish what SoC
is being used and do proper things.

Right



I agree to have a single generic pm-gen2.c file which covers all Gen2 SoCs.

But, unfortunately, I only have Stout boards (H2), and therefore can 
check only on them. This is why the current patch series adds support 
for H2 SoC only.


Theoretically, I could add support for M2 as well, but, I wouldn't have 
possibility to check.



I would focus on the r8a7790 for now, as the Stout board is our nearest 
target which we want to support in Xen and the PSCI feature is a 
mandatory option.


What do you think?



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


Re: [U-Boot] [PATCH 1/3] ARM: rmobile: Switch CPU to non-secure HYP mode for r8a7790 based boards

2019-02-12 Thread Oleksandr


On 09.02.19 18:35, Marek Vasut wrote:

Hi


On 2/7/19 6:19 PM, Oleksandr wrote:

[...]


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

You would need a DT entry and a bit of code to start the timer in case
the PSCI is enabled, yes. This would then fit into the DM/DT paradigm.


I understand your point, thank you.

Will create a simple driver for arch timer in V2.





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

Thanks


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


Re: [U-Boot] [PATCH 1/3] ARM: rmobile: Switch CPU to non-secure HYP mode for r8a7790 based boards

2019-02-09 Thread Marek Vasut
On 2/7/19 6:19 PM, Oleksandr wrote:

[...]

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

You would need a DT entry and a bit of code to start the timer in case
the PSCI is enabled, yes. This would then fit into the DM/DT paradigm.

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

Thanks

-- 
Best regards,
Marek Vasut
___
U-Boot mailing list
U-Boot@lists.denx.de
https://lists.denx.de/listinfo/u-boot


Re: [U-Boot] [PATCH 1/3] ARM: rmobile: Switch CPU to non-secure HYP mode for r8a7790 based boards

2019-02-09 Thread Marek Vasut
On 2/8/19 12:40 PM, Oleksandr wrote:
> 
> 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 
>>
>> 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 
>> ---
>>    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

This information should be in the DT for each SoC, so you should extract
it from there.

> 2. It should be new pm-r8a7791.c file which will don't have any CA7
> related stuff (CPU cores, SCU, etc).

I'd like to have a generic pm-gen2.c file , which parses the DT and
figures the configuration out that way. We are trying to get rid of all
the ad-hoc hardcoded configuration stuff in favor of DT.

> Or it should be the single pm-r8a779x.c which must distinguish what SoC
> is being used and do proper things.

Right

[...]

-- 
Best regards,
Marek Vasut
___
U-Boot mailing list
U-Boot@lists.denx.de
https://lists.denx.de/listinfo/u-boot


Re: [U-Boot] [PATCH 1/3] ARM: rmobile: Switch CPU to non-secure HYP mode for r8a7790 based boards

2019-02-08 Thread Oleksandr


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 

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 
---
   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    0xE608
+#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:


Re: [U-Boot] [PATCH 1/3] ARM: rmobile: Switch CPU to non-secure HYP mode for r8a7790 based boards

2019-02-07 Thread Oleksandr


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 

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 
---
   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    0xE608
+#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.



Re: [U-Boot] [PATCH 1/3] ARM: rmobile: Switch CPU to non-secure HYP mode for r8a7790 based boards

2019-02-07 Thread Marek Vasut
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 
>>>
>>> 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 
>>> ---
>>>   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    0xE608
>>> +#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/setup-rcar-gen2.c#L61

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 

Re: [U-Boot] [PATCH 1/3] ARM: rmobile: Switch CPU to non-secure HYP mode for r8a7790 based boards

2019-02-07 Thread Oleksandr


On 05.02.19 20:48, Marek Vasut wrote:

Hi Marek


On 1/31/19 6:38 PM, Oleksandr Tyshchenko wrote:

From: Oleksandr Tyshchenko 

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 
---
  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.





  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 0xE608
+#define TIMER_CNTCR0x00
+#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?





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




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




+void smp_set_core_boot_addr(unsigned long addr, int corenr)
+{
+}
+
+void smp_kick_all_cpus(void)
+{
+}
+
+void smp_waitloop(unsigned int previous_address)
+{
+}
+#endif
+
  #define ETHERNET_PHY_RESET185 /* GPIO 5 31 */
  
  int board_init(void)

@@ -89,6 +136,10 @@ int board_init(void)
mdelay(10);
gpio_direction_output(ETHERNET_PHY_RESET, 1);
  
+#ifdef CONFIG_ARMV7_NONSEC

Define empty function in case the macro is not set instead of ifdefing
every place that calls the rcar_gen2_timer_init() function.


Agree, will do




+   rcar_gen2_timer_init();
+#endif
+
return 0;
  }
  
diff --git a/board/renesas/stout/stout.c 

Re: [U-Boot] [PATCH 1/3] ARM: rmobile: Switch CPU to non-secure HYP mode for r8a7790 based boards

2019-02-05 Thread Marek Vasut
On 1/31/19 6:38 PM, Oleksandr Tyshchenko wrote:
> From: Oleksandr Tyshchenko 
> 
> 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 
> ---
>  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 ?
Does this apply to M2* too , since it has CA15 cores as well ?

>  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   0xE608
> +#define TIMER_CNTCR  0x00
> +#define TIMER_CNTFID00x20
> +
> +/*
> + * 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 ?

> + /* 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 ?

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

> +void smp_set_core_boot_addr(unsigned long addr, int corenr)
> +{
> +}
> +
> +void smp_kick_all_cpus(void)
> +{
> +}
> +
> +void smp_waitloop(unsigned int previous_address)
> +{
> +}
> +#endif
> +
>  #define ETHERNET_PHY_RESET   185 /* GPIO 5 31 */
>  
>  int board_init(void)
> @@ -89,6 +136,10 @@ int board_init(void)
>   mdelay(10);
>   gpio_direction_output(ETHERNET_PHY_RESET, 1);
>  
> +#ifdef CONFIG_ARMV7_NONSEC

Define empty function in case the macro is not set instead of ifdefing
every place that calls the rcar_gen2_timer_init() function.

> + rcar_gen2_timer_init();
> +#endif
> +
>   return 0;
>  }
>  
> diff --git a/board/renesas/stout/stout.c b/board/renesas/stout/stout.c
> index 85e30db..8d034a8 100644
> --- a/board/renesas/stout/stout.c
> +++ b/board/renesas/stout/stout.c
> @@ -77,6 +77,53 @@ int board_early_init_f(void)
>   return 0;
>  }
>  
> +#ifdef CONFIG_ARMV7_NONSEC
> +#define TIMER_BASE   0xE608
> +#define TIMER_CNTCR  0x00
> +#define TIMER_CNTFID00x20
> +
> +/*
> + * 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)
> +{

Why is this stuff in board code ? It should be in driver code / core
code. And there should be only one copy of it.

> + 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) {
> + /* Update 

[U-Boot] [PATCH 1/3] ARM: rmobile: Switch CPU to non-secure HYP mode for r8a7790 based boards

2019-01-31 Thread Oleksandr Tyshchenko
From: Oleksandr Tyshchenko 

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 
---
 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
 
 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 0xE608
+#define TIMER_CNTCR0x00
+#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) {
+   /* 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);
+   }
+}
+
+/*
+ * In order not to break compilation if someone decides to build with PSCI
+ * support disabled, keep these dummy functions.
+ */
+void smp_set_core_boot_addr(unsigned long addr, int corenr)
+{
+}
+
+void smp_kick_all_cpus(void)
+{
+}
+
+void smp_waitloop(unsigned int previous_address)
+{
+}
+#endif
+
 #define ETHERNET_PHY_RESET 185 /* GPIO 5 31 */
 
 int board_init(void)
@@ -89,6 +136,10 @@ int board_init(void)
mdelay(10);
gpio_direction_output(ETHERNET_PHY_RESET, 1);
 
+#ifdef CONFIG_ARMV7_NONSEC
+   rcar_gen2_timer_init();
+#endif
+
return 0;
 }
 
diff --git a/board/renesas/stout/stout.c b/board/renesas/stout/stout.c
index 85e30db..8d034a8 100644
--- a/board/renesas/stout/stout.c
+++ b/board/renesas/stout/stout.c
@@ -77,6 +77,53 @@ int board_early_init_f(void)
return 0;
 }
 
+#ifdef CONFIG_ARMV7_NONSEC
+#define TIMER_BASE 0xE608
+#define TIMER_CNTCR0x00
+#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) {
+   /* 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);
+   }
+}
+
+/*
+ * In order not to break compilation if someone decides to build with PSCI
+ * support disabled, keep these dummy functions.
+ */
+void smp_set_core_boot_addr(unsigned long addr, int corenr)
+{
+}
+
+void smp_kick_all_cpus(void)
+{
+}
+
+void smp_waitloop(unsigned int previous_address)
+{
+}
+#endif
+
 #define ETHERNET_PHY_RESET 123 /* GPIO 3 31 */
 
 int board_init(void)
@@ -92,6 +139,10 @@ int board_init(void)
mdelay(20);