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

[Ezequiel Garcia]

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?

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

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

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

> Also this patch is not enabling CONFIG_DISPLAY_CPUINFO which is required
> to see the same information before and after this patch for existing
> boards and this should be added too.
>

OK.

> Thanks,
> Michal
>



-- 
Ezequiel García, VanguardiaSur
www.vanguardiasur.com.ar