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

Tue Apr 24 12:49:06 UTC 2018

Hi Michal,

On Tuesday 24 April 2018 05:54 PM, Michal Simek wrote:
> 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.

Sorry, I did not mean this patch does not work. It definitely works. I
am trying to tell that with some kconfig changes we should be able to
re-use armv7 folder instead of creating new folder. Apologies if my
reply has communicated wrongly.

Will post an RFC series for R5 support.

Thanks and regards,
Lokesh