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

[Lokesh Vutla]

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?

> 
> 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 :)

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

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

[1] https://gcc.gnu.org/bugzilla/show_bug.cgi?id=57907

Thanks and regards,
Lokesh

> 
> Thanks,
> Michal
>