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

Tue Apr 24 12:53:36 UTC 2018

On 24.4.2018 14:49, Lokesh Vutla wrote:
> 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.

I know that it will work. Please send it and I will take a look. The
same is for v7m which can be also squashed with v7 + some Kconfig options.

Thanks,
Michal