[U-Boot] [PATCH 2/2] zynq: Support CPU info display

[Michal Simek]

Hi,

On 16.1.2018 19:38, Ezequiel Garcia wrote:
> On 16 January 2018 at 09:08, Michal Simek <michal.simek at xilinx.com> wrote:
>> On 15.1.2018 16:46, Ezequiel Garcia wrote:
>>> This commit adds CPU and silicon version information
>>> consuming the SLCR IDCODE and DEVCFG MCTRL registers,
>>> respectively.
>>>
>>> Signed-off-by: Ariel D'Alessandro <ariel at vanguardiasur.com.ar>
>>> Signed-off-by: Ezequiel Garcia <ezequiel at vanguardiasur.com.ar>
>>> ---
>>>  arch/arm/mach-zynq/Makefile   |  1 +
>>>  arch/arm/mach-zynq/cpu_info.c | 49 +++++++++++++++++++++++++++++++++++++++++++
>>>  2 files changed, 50 insertions(+)
>>>  create mode 100644 arch/arm/mach-zynq/cpu_info.c
>>>
>>> diff --git a/arch/arm/mach-zynq/Makefile b/arch/arm/mach-zynq/Makefile
>>> index e3f0117da563..31f1e0d5a8ad 100644
>>> --- a/arch/arm/mach-zynq/Makefile
>>> +++ b/arch/arm/mach-zynq/Makefile
>>> @@ -14,5 +14,6 @@ obj-y       += ddrc.o
>>>  obj-y        += slcr.o
>>>  obj-y        += clk.o
>>>  obj-y        += lowlevel_init.o
>>> +obj-$(CONFIG_DISPLAY_CPUINFO) += cpu_info.o
>>>  AFLAGS_lowlevel_init.o := -mfpu=neon
>>>  obj-$(CONFIG_SPL_BUILD)      += spl.o ps7_spl_init.o
>>> diff --git a/arch/arm/mach-zynq/cpu_info.c b/arch/arm/mach-zynq/cpu_info.c
>>> new file mode 100644
>>> index 000000000000..730ccccb73da
>>> --- /dev/null
>>> +++ b/arch/arm/mach-zynq/cpu_info.c
>>> @@ -0,0 +1,49 @@
>>> +/*
>>> + * Copyright (C) 2018 VanguardiaSur - www.vanguardiasur.com.ar
>>
>> The part of code below is copied from board.c which is interestingly
>> copyrighted just by me. But there should be also Xilinx. :-)
>>
> 
> Oh, sorry about!
> 
> /*
>  * (C) Copyright 2012 Michal Simek <monstr at monstr.eu>
>  * (C) Copyright 2018 VanguardiaSur - www.vanguardiasur.com.ar
>  *
>  * SPDX-License-Identifier:     GPL-2.0+
>  */
> 
> This OK ? How should the Xilinx copyright look like?

I have sent a patch to update years and add line to board.c. If you can
based your work on the top of that that will be great.




> 
>>> + *
>>> + * SPDX-License-Identifier:  GPL-2.0+
>>> + */
>>> +
>>> +#include <common.h>
>>> +#include <stdio.h>
>>
>> Already the part of common.h
>>
> 
> Got it.
> 
>>> +#include <zynqpl.h>
>>
>> This is not needed but both these patches are pointing to one more thing
>> which could be the part of this series which is filling FPGA structure
>> which should be removed from board.c too.
>>
> 
> OK.
> 
>>> +#include <asm/arch/sys_proto.h>
>>> +#include <asm/arch/ps7_init_gpl.h>
>>
>> This header is probably not needed.
>>
> 
> Seems needed because of PCW_SILICON_VERSION_3.

ah right.

> 
>>> +
>>> +static const struct {
>>> +     u8 idcode;
>>> +     const char *cpuinfo;
>>> +} zynq_cpu_info[] = {
>>> +     { .idcode = XILINX_ZYNQ_7007S,  .cpuinfo = "7007S" },
>>> +     { .idcode = XILINX_ZYNQ_7010,   .cpuinfo = "7010" },
>>> +     { .idcode = XILINX_ZYNQ_7012S,  .cpuinfo = "7012S" },
>>> +     { .idcode = XILINX_ZYNQ_7014S,  .cpuinfo = "7014S" },
>>> +     { .idcode = XILINX_ZYNQ_7015,   .cpuinfo = "7015" },
>>> +     { .idcode = XILINX_ZYNQ_7020,   .cpuinfo = "7020" },
>>> +     { .idcode = XILINX_ZYNQ_7030,   .cpuinfo = "7030" },
>>> +     { .idcode = XILINX_ZYNQ_7035,   .cpuinfo = "7035" },
>>> +     { .idcode = XILINX_ZYNQ_7045,   .cpuinfo = "7045" },
>>> +     { .idcode = XILINX_ZYNQ_7100,   .cpuinfo = "7100"},
>>
>> If you look at include/zynqpl.h then 7z007s name is used instead.
> 
> Oh, missed those strings. We should be able to reuse them.

I hope that filling that structure together will be great.

> 
>> Normally this was available via fpga info command but not a problem to
>> show it as the part of boot log.
>>
>>> +     { /* Sentinel */ },
>>> +};
>>> +
>>> +int print_cpuinfo(void)
>>> +{
>>> +     u32 idcode, version;
>>> +     u8 i;
>>> +
>>> +     idcode = zynq_slcr_get_idcode();
>>> +
>>> +     for (i = 0; zynq_cpu_info[i].idcode; i++) {
>>> +             if (zynq_cpu_info[i].idcode == idcode) {
>>> +                     printf("CPU:   Zynq %s\n", zynq_cpu_info[i].cpuinfo);
>>> +                     break;
>>> +             }
>>> +     }
>>> +
>>> +     version = zynq_get_silicon_version() << 1;
>>> +     if (version > (PCW_SILICON_VERSION_3 << 1))
>>> +             version += 1;
>>> +     printf("Silicon: v%d.%d\n", version >> 1, version & 1);
>>
>>
>> When this is running on QEMU only silicon version is shown and nothing else.
>>
>> U-Boot 2018.01-00101-g2cb1e21ccc8e (Jan 16 2018 - 12:08:57 +0100) Xilinx
>> Zynq ZC702
>>
>> Silicon: v0.0
>> Model: Zynq ZC702 Development Board
>> I2C:   ready
>> DRAM:  ECC disabled 1 GiB
>>
>> I think if you print cpu information you should also print silicon
>> version. If you don't print it, then just don't print silicon version too.
>>
> 
> OK, makes sense.
> 
>> Anyway just a summary. You pointed to this code that's why I think to do
>> it properly we should also remove fpga initialization from board.c and
>> move it to mach-zynq and detect idcode just once and then reuse it.
>>
> 
> Right. Perhaps we can put both fpga init and print_cpuinfo in slcr.c.


I am not sure that slcr is the right file. cpu.c seems to me better. But
not a problem if you want to create new file.

Thanks,
Michal