[U-Boot] [PATCH 1/2] arm: Add minimal support for Cortex-R5

Tue Apr 24 12:24:47 UTC 2018

Hi,

On 24.4.2018 06:45, Lokesh Vutla wrote:
> Hi Michal,
> 
> On Monday 23 April 2018 11:56 AM, Michal Simek wrote:
>> On 23.4.2018 05:53, Lokesh Vutla wrote:
>>>
>>>
>>> On Friday 20 April 2018 07:21 PM, Michal Simek wrote:
>>>> This minimal support will be used by Xilinx ZynqMP R5 cpu.
>>>>
>>>> Signed-off-by: Michal Simek <michal.simek at xilinx.com>
>>>> ---
>>>>
>>>>  arch/arm/Kconfig              |  6 ++++++
>>>>  arch/arm/cpu/armv7r/Makefile  |  4 ++++
>>>>  arch/arm/cpu/armv7r/config.mk |  3 +++
>>>>  arch/arm/cpu/armv7r/cpu.c     | 24 ++++++++++++++++++++++++
>>>>  arch/arm/cpu/armv7r/start.S   | 17 +++++++++++++++++
>>>>  5 files changed, 54 insertions(+)
>>>>  create mode 100644 arch/arm/cpu/armv7r/Makefile
>>>>  create mode 100644 arch/arm/cpu/armv7r/config.mk
>>>>  create mode 100644 arch/arm/cpu/armv7r/cpu.c
>>>>  create mode 100644 arch/arm/cpu/armv7r/start.S
>>>>
>>>> diff --git a/arch/arm/Kconfig b/arch/arm/Kconfig
>>>> index b5fbce03667d..b10804f55224 100644
>>>> --- a/arch/arm/Kconfig
>>>> +++ b/arch/arm/Kconfig
>>>> @@ -192,6 +192,10 @@ config CPU_V7M
>>>>  	select THUMB2_KERNEL
>>>>  	select SYS_CACHE_SHIFT_5
>>>>  
>>>> +config CPU_V7R
>>>> +	bool
>>>> +	select SYS_CACHE_SHIFT_6
>>>
>>> select HAS_THUMB2 might be a good option?
>>
>> I didn't enable it because I didn't test it.
>> It can be added when someone tests this.
>>
>>>
>>>> +
>>>>  config CPU_PXA
>>>>  	bool
>>>>  	select SYS_CACHE_SHIFT_5
>>>> @@ -209,6 +213,7 @@ config SYS_CPU
>>>>  	default "arm1176" if CPU_ARM1176
>>>>  	default "armv7" if CPU_V7
>>>>  	default "armv7m" if CPU_V7M
>>>> +	default "armv7r" if CPU_V7R
>>>>  	default "pxa" if CPU_PXA
>>>>  	default "sa1100" if CPU_SA1100
>>>>  	default "armv8" if ARM64
>>>> @@ -223,6 +228,7 @@ config SYS_ARM_ARCH
>>>>  	default 6 if CPU_ARM1176
>>>>  	default 7 if CPU_V7
>>>>  	default 7 if CPU_V7M
>>>> +	default 7 if CPU_V7R
>>>>  	default 5 if CPU_PXA
>>>>  	default 4 if CPU_SA1100
>>>>  	default 8 if ARM64
>>>
>>> I did a grep of CPU_V7, and you might want to update for CPU_V7R in the
>>> following places:
>>>
>>> diff --git a/arch/arm/Makefile b/arch/arm/Makefile
>>> index 4fa8b38397..f4bc1f250d 100644
>>> --- a/arch/arm/Makefile
>>> +++ b/arch/arm/Makefile
>>> @@ -18,6 +18,7 @@ arch-$(CONFIG_CPU_ARM1136)	=-march=armv5
>>>  arch-$(CONFIG_CPU_ARM1176)	=-march=armv5t
>>>  arch-$(CONFIG_CPU_V7)		=$(call cc-option, -march=armv7-a, \
>>>  				 $(call cc-option, -march=armv7, -march=armv5))
>>> +arch-$(CONFIG_CPU_V7R)		=-march=armv7-r
>>
>> I have setup PLATFORM_CPPFLAGS via config.mk
>>
>> If both options are selected I am getting this compilation warning.
>> cc1: warning: switch -mcpu=cortex-r5 conflicts with -march=armv7-r switch
> 
> hmm..that's strange. I guess it should be reported to gcc? Something
> similar has been reported for a15 as well[1].
> 
> But looking at the implementation of armv7 we just included march. may
> be we should stick to it?

As I said I have not a problem to add there -march=armv7-r instead of
-mcpu=cortex-r5.

>> Not sure which one is better or if it is better to have this arch flag
>> via this makefile or as platform cppflags via config.mk.
>> I choose the second option because it seems to me better if this is in
>> subfolder. But not a problem to use different flag and put it to this
>> Makefile.
>>
>> The same style is used for cpu_v7m.
> 
> v7-r is very much close to v7. I would prefer to compare with v7
> implementation than v7-m :)

I started with symlink to v7. And I didn't play with MPU. Anyway both
ways works for me.

>>
>>>  arch-$(CONFIG_ARM64)		=-march=armv8-a
>>>
>>>  # On Tegra systems we must build SPL for the armv4 core on the device
>>> @@ -41,6 +42,7 @@ tune-$(CONFIG_CPU_PXA)		=-mcpu=xscale
>>>  tune-$(CONFIG_CPU_ARM1136)	=
>>>  tune-$(CONFIG_CPU_ARM1176)	=
>>>  tune-$(CONFIG_CPU_V7)		=
>>> +tune-$(CONFIG_CPU_V7R)		=
>>
>> Again as above. I used v7m as pattern and there is also empty tune
>> parameter. Is there any good reason to have it empty?
>>
>>>  tune-$(CONFIG_ARM64)		=
>>>
>>>  # Evaluate tune cc-option calls now
>>>
>>>
>>>> diff --git a/arch/arm/cpu/armv7r/Makefile b/arch/arm/cpu/armv7r/Makefile
>>>> new file mode 100644
>>>> index 000000000000..3c66976dfa62
>>>> --- /dev/null
>>>> +++ b/arch/arm/cpu/armv7r/Makefile
>>>
>>> hmm..do we really need to create a separate folder? IIUC, the main
>>> difference between V7 and V7R is MMU vs MPU. IMHO, we should be able to
>>> get it using Kconfigs.
>>
>> I have used V7 for the initial port and then was checking V7M which is
>> already there and used it as pattern for writing this patch.
>>
>> I have debugged V7 and found that I need to disable CONFIG_HAS_VBAR
>> that's why I have created new symbol as for V7M.
> 
> We should be able to create a kconfig symbol for that and select
> accordingly. Kernel does it the same way.

What kernel Kconfig option are you talking about?  (I didn't look at
Linux running on R5 but it is reasonable step).

It is really a question if adding new Kconfig symbol for VBAR is the
right thing to do. I would need to add VBAR to every platform which has
CPU_V7.

>>
>> Definitely I am open to hear your suggestion
> I just did a couple of quick experiments and able to get v7-r support
> along with v7 support. let me know if you would like to take a look at
> it. Then Ill pick your patch 1/2 and post the series.

I am definitely confident that this will work because I used that in
past. Definitely feel free to send it and I will take a look at retest
on R5.

Thanks,
Michal