| Message ID | 20180115154649.9551-2-ezequiel@vanguardiasur.com.ar |
|---|---|
| State | Superseded |
| Delegated to: | Michal Simek |
| Headers | show |
| Series | [U-Boot,1/2] zynq: board: Remove checkboard() | expand |
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@vanguardiasur.com.ar> > Signed-off-by: Ezequiel Garcia <ezequiel@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. :-) > + * > + * SPDX-License-Identifier: GPL-2.0+ > + */ > + > +#include <common.h> > +#include <stdio.h> Already the part of common.h > +#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. > +#include <asm/arch/sys_proto.h> > +#include <asm/arch/ps7_init_gpl.h> This header is probably not needed. > + > +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. 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. 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. 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. Thanks, Michal
On 16 January 2018 at 09:08, Michal Simek <michal.simek@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@vanguardiasur.com.ar> >> Signed-off-by: Ezequiel Garcia <ezequiel@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@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 >
Hi, On 16.1.2018 19:38, Ezequiel Garcia wrote: > On 16 January 2018 at 09:08, Michal Simek <michal.simek@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@vanguardiasur.com.ar> >>> Signed-off-by: Ezequiel Garcia <ezequiel@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@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
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 + * + * SPDX-License-Identifier: GPL-2.0+ + */ + +#include <common.h> +#include <stdio.h> +#include <zynqpl.h> +#include <asm/arch/sys_proto.h> +#include <asm/arch/ps7_init_gpl.h> + +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"}, + { /* 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); + return 0; +}