[Buildroot] [PATCH] configs/digilent_genesys_zu_defconfig: add Digilent Genesys ZU board (ZynqMP SoC)
Alvaro Gamez
alvaro.gamez at hazent.com
Tue Aug 3 08:19:48 UTC 2021
Hi Luca
El vie, 30 jul 2021 a las 0:21, Luca Ceresoli
(<luca at lucaceresoli.net>) escribió:
>
> Hi Alvaro,
>
> thanks for your patch.
>
> And apologies for the late reply -- vacation time.
Don't worry, we're all busy! Thanks for your review.
> On 15/07/21 08:28, Alvaro Gamez Machado wrote:
> > This adds support the Digilent Genesys ZU development board.
> >
> > Signed-off-by: Alvaro Gamez Machado <alvaro.gamez at hazent.com>
> > ---
> > ...ve-redundant-YYLOC-global-declaratio.patch | 52 +
> > .../0001-uboot-add-genesys-zu.patch | 10 +
> > board/digilent/genesys-zu/GenesysZU.dts | 1655 +
> > board/digilent/genesys-zu/README.txt | 92 +
> > board/digilent/genesys-zu/genimage.cfg | 28 +
> > board/digilent/genesys-zu/image.its | 59 +
> > board/digilent/genesys-zu/kernel_defconfig | 414 +
> > board/digilent/genesys-zu/pm_cfg_obj.c | 630 +
> > board/digilent/genesys-zu/post-image.sh | 10 +
> > board/digilent/genesys-zu/psu_init_gpl.c | 21818 +++++++++
> > board/digilent/genesys-zu/psu_init_gpl.h | 37545 ++++++++++++++++
>
> These files are huge! You should use the
> tools/zynqmp_psu_init_minimize.sh tool in the U-Boot sources and submit
> for Buildroot the reduced files.
I didn't even know that thing existed, I will surely use it.
My last patch got rejected by the mailing list due to the big size.
> > diff --git a/board/digilent/genesys-zu/0001-scripts-dtc-Remove-redundant-YYLOC-global-declaratio.patch b/board/digilent/genesys-zu/0001-scripts-dtc-Remove-redundant-YYLOC-global-declaratio.patch
> > new file mode 100644
> > index 0000000000..48f683afbe
> > --- /dev/null
> > +++ b/board/digilent/genesys-zu/0001-scripts-dtc-Remove-redundant-YYLOC-global-declaratio.patch
> > @@ -0,0 +1,52 @@
> > +From e33a814e772cdc36436c8c188d8c42d019fda639 Mon Sep 17 00:00:00 2001
> > +From: Dirk Mueller <dmueller at suse.com>
> > +Date: Tue, 14 Jan 2020 18:53:41 +0100
> > +Subject: [PATCH] scripts/dtc: Remove redundant YYLOC global declaration
> > +
> > +gcc 10 will default to -fno-common, which causes this error at link
> > +time:
> > +
> > + (.text+0x0): multiple definition of `yylloc'; dtc-lexer.lex.o (symbol from plugin):(.text+0x0): first defined here
> > +
> > +This is because both dtc-lexer as well as dtc-parser define the same
> > +global symbol yyloc. Before with -fcommon those were merged into one
> > +defintion. The proper solution would be to to mark this as "extern",
> > +however that leads to:
> > +
> > + dtc-lexer.l:26:16: error: redundant redeclaration of 'yylloc' [-Werror=redundant-decls]
> > + 26 | extern YYLTYPE yylloc;
> > + | ^~~~~~
> > +In file included from dtc-lexer.l:24:
> > +dtc-parser.tab.h:127:16: note: previous declaration of 'yylloc' was here
> > + 127 | extern YYLTYPE yylloc;
> > + | ^~~~~~
> > +cc1: all warnings being treated as errors
> > +
> > +which means the declaration is completely redundant and can just be
> > +dropped.
> > +
> > +Signed-off-by: Dirk Mueller <dmueller at suse.com>
> > +Signed-off-by: David Gibson <david at gibson.dropbear.id.au>
> > +[robh: cherry-pick from upstream]
> > +Cc: stable at vger.kernel.org
> > +Signed-off-by: Rob Herring <robh at kernel.org>
> > +Signed-off-by: Alvaro Gamez Machado <alvaro.gamez at hazent.com>
>
> If this patch comes from upstream it would be nice to add an URL of
> where it can be found.
It's just a linux kernel patch, the one referenced on the first line:
>From e33a814e772cdc36436c8c188d8c42d019fda639 Mon Sep 17 00:00:00 2001
I can clarify it somewhere else, but where? Just below the --- commit
line is ok?
>
> > diff --git a/board/digilent/genesys-zu/GenesysZU.dts b/board/digilent/genesys-zu/GenesysZU.dts
> > new file mode 100644
> > index 0000000000..ddeba4b715
> > --- /dev/null
> > +++ b/board/digilent/genesys-zu/GenesysZU.dts
>
> Do you plan to submit this DTS to mainline Linux? It would be nice to do
> it and, in that process, have it properly reviewed. After that it can be
> removed from Buildroot.
I really should, but this DTS won't be accepted as is. The proper way
would be removing
most of it and just use #include "zynqmp.dtsi". If I have enough time
this century I really will.
> > diff --git a/board/digilent/genesys-zu/README.txt b/board/digilent/genesys-zu/README.txt
> > new file mode 100644
> > index 0000000000..2a7a86a3ab
> > --- /dev/null
> > +++ b/board/digilent/genesys-zu/README.txt
> > @@ -0,0 +1,92 @@
> > +********************************
> > +Digilent Genesys ZU - ZynqMP SoC
> > +********************************
> > +
> > +This document describes the Buildroot support for the Genesys ZU board
> > +by Digilent, based on the Zynq UltraScale+ MPSoC (aka ZynqMP).
> > +
> > +How to build it
> > +===============
> > +
> > +Configure Buildroot:
> > +
> > + $ make digilent_genesys_zu_defconfig
> > +
> > +Compile everything and build the rootfs image:
> > +
> > + $ make
> > +
> > +Result of the build
> > +-------------------
> > +
> > +After building, you should get a tree like this:
> > +
> > + output/images/
> > + +-- bl31.bin
> > + +-- boot.bin
> > + +-- u-boot.itb
> > + +-- uboot-env.bin
> > + +-- GenesysZU.dtb
> > + +-- Image
> > + +-- image.ub
> > + +-- rootfs.ext2
> > + +-- rootfs.ext4 -> rootfs.ext2
> > + +-- boot.vfat
> > + `-- sdcard.img
> > +
> > + * bl31.bin: Compiled xilinx' ARM Trusted Firmware, version xilinx-2020.1.
> > + * boot.bin: U-boot SPL binary image. This is the file the zynqmp boots first.
> > + * u-boot.itb: FIT Image containing both u-boot proper and ATF bl31.bin
> > + SPL image will load these and will run BOTH.
> > + * uboot-env.bin: U-boot environment
> > + * GenesysZU.dtb: Device tree blob, compiled from source GenesysZU.dts
> > + * Image: Kernel image
> > + * image.ub: FIT Image containing kernel image, device tree blob and, optionally,
> > + FPGA bitfile image
> > + * rootfs.ext4: filesystem image
> > + * boot.vfat: FAT partition to be written to SD.
> > + Contains boot.bin, u-boot.itb, uboot-env.bin and image.ub
> > + * sdcard.img: SD image with two partitions: boot.vfat and rootfs.ext4
>
> These descriptions are useful!
Thanks! This thing generates a ton of files that are useful by
themselves, but quite difficult
to know sometimes what should be flashed at any given time.
>
> > +
> > +ZynqMP "community workflow" boot summary
> > +========================================
> > +
> > +No Xilinx utils are used to generate the resulting binaries, but several Xilinx
> > +source code modules are used indeed.
> > +
> > +The SoC will look for boot.bin image. In this case, this is U-boot SPL image.
> > +This will load u-boot proper and ARM Trusted Firmware binary from a FIT image,
> > +but also loads PMU firmware to the PMU processor, which is a Microblaze.
> > +
> > +U-boot proper can then run linux or anything else, but before any of this, it
> > +configures the SoC using provided pm_cfg_obj.c and psu_init_gpl.c
>
> Both psu_init_gpl.c and pm_cfg_obj are part of SPL, not U-Boot proper.
Oh, right. It was part of u-boot proper for zynq devices. I'll change it.
>
> > +If a BIT file is provided in the FIT image, it will also configure the PL section
> > +of the SoC.
> > +
> > +Ideally all firmware should be taken from a single Xilinx release, i.e,
> > +2019.1, 2019.2, 2020.1... etc. However, this configuration uses modules from
> > +different release versions. Kernel is Xilinx version 2019.2, while u-boot is
> > +mainstream 2021.04 and ATF is version 2020.1. The reason for this is that
> > +kernel version 2019.2 implements correctly the tested firmware, while
>
> What do you mean here? Which firmware?
I... I don't know what I mean there? I shall correct that. It's just
an explanation of why I
used such a variety of versions.
For some reason that I didn't have the time to debug, kernel versions
different than 2019.2
didn't run with this configuration, each one failing for different
reasons, so even though
I would have liked to use something more recent, I just set on 2019.2,
which is the one that
Digilent used on their image.
>
> > +mainstream u-boot is the only one that supports full "community workflow"
> > +mode. ATF version has been chosen to be 2020.1 because this version compiles
> > +using gcc-11 without any additional patches.
> > +
> > +This set of different versions is the most stable and easy to compile on
> > +buildroot that the author has found, but there may be some incompatibilities
> > +on subsystems that have not been tested.
> > +
> > +How to write the SD card
> > +========================
> > +
> > +WARNING! This will destroy all the card content. Use with care!
> > +
> > +The sdcard.img file is a complete bootable image ready to be written
> > +on the boot medium. To install it, simply copy the image to an SD
> > +card:
> > +
> > + # dd if=output/images/sdcard.img of=/dev/sdX
> > +
> > +Where 'sdX' is the device node of the SD.
> > +
> > +Eject the SD card, insert it in the board, and power it up.
>
> ...
>
> > diff --git a/board/digilent/genesys-zu/image.its b/board/digilent/genesys-zu/image.its
> > new file mode 100644
> > index 0000000000..2ec472553b
> > --- /dev/null
> > +++ b/board/digilent/genesys-zu/image.its
> > @@ -0,0 +1,59 @@
> > +/dts-v1/;
> > +
> > +/ {
> > + description = "U-Boot fitImage";
> > + #address-cells = <1>;
> > +
> > + images {
> > + kernel {
> > + description = "Linux Kernel";
> > + data = /incbin/("Image");
> > + type = "kernel";
> > + arch = "arm64";
> > + os = "linux";
> > + compression = "none";
> > + load = <0x00080000>;
> > + entry = <0x00080000>;
> > + hash-1 {
> > + algo = "sha1";
> > + };
> > + };
> > +/*
> > + fpga {
> > + description = "FPGA bitfile";
> > + data = /incbin/("system.bit");
> > + type = "fpga";
> > + arch = "arm64";
> > + compression = "none";
> > + load = <0x4000000>;
> > + hash-1 {
> > + algo = "sha1";
> > + };
> > + };
> > +*/
>
> Unmotivated commented code is not nice. I suggest explaining what
> it is used for:
>
> /* Enable this section to load a bitstream during boot
>
Sure, good idea.
> > + fdt {
> > + description = "DTB";
> > + data = /incbin/("GenesysZU.dtb");
> > + type = "flat_dt";
> > + arch = "arm64";
> > + compression = "none";
> > + hash-1 {
> > + algo = "sha1";
> > + };
> > + };
> > + };
> > + configurations {
> > + default = "conf";
> > + conf {
> > + description = "Boot Linux kernel with FDT blob";
> > + kernel = "kernel";
> > + fdt = "fdt";
> > +/*
> > + fpga = "fpga";
> > +*/
>
> Ditto.
>
> > + hash-1 {
> > + algo = "sha1";
> > + };
> > + };
> > + };
> > +};
> > diff --git a/board/digilent/genesys-zu/kernel_defconfig b/board/digilent/genesys-zu/kernel_defconfig
> > new file mode 100644
> > index 0000000000..5778417e7e
> > --- /dev/null
> > +++ b/board/digilent/genesys-zu/kernel_defconfig
>
> As above: are you submitting this file for mainline Linux?
Again, I really should, but it won't get approved unless I prepare
first the DTS file to be acceptable.
I don't know if I'll have the time to do this, though.
>
> > diff --git a/board/digilent/genesys-zu/pm_cfg_obj.c b/board/digilent/genesys-zu/pm_cfg_obj.c
> > new file mode 100644
> > index 0000000000..a537a05493
> > --- /dev/null
> > +++ b/board/digilent/genesys-zu/pm_cfg_obj.c
> > @@ -0,0 +1,630 @@
>
> Uhm, maybe we need a minimizer tool for pm_cfg_obj too... But for now
> it's OK to commit this file.
Yep, it's quite big too. Maybe the same minimizer works,
is it just removing comments or does it something more complex?
>
> > diff --git a/board/digilent/genesys-zu/uboot-env.txt b/board/digilent/genesys-zu/uboot-env.txt
> > new file mode 100644
> > index 0000000000..7eb312f4a1
> > --- /dev/null
> > +++ b/board/digilent/genesys-zu/uboot-env.txt
>
> No way to use the standard U-Boot boot process? Why?
The main purpose of such boot method is reading MAC address from on
board SPI flash memory.
It's the same set I gathered from the image distributed by Xilinx, and
decided it best to keep it
as similar as possible. Maybe I should have explained that on the
README, so I will.
>
> > diff --git a/board/digilent/genesys-zu/uboot_defconfig b/board/digilent/genesys-zu/uboot_defconfig
> > new file mode 100644
> > index 0000000000..f22856e184
> > --- /dev/null
> > +++ b/board/digilent/genesys-zu/uboot_defconfig
>
> As above: any plan to submit this to mainline U-Boot?
And the answer is yet the same... This defconfig is also not
acceptable as is for mainline, so
it would require more work.
Nontheless, I will send a new version of the patch fixing those things
you've suggested.
Thanks!
>
> --
> Luca
--
Álvaro Gámez Machado
More information about the buildroot
mailing list