* Re: [Buildroot] [PATCH] configs/digilent_genesys_zu_defconfig: add Digilent Genesys ZU board (ZynqMP SoC) [not found] <20210715062814.8076-1-alvaro.gamez@hazent.com> @ 2021-07-21 6:37 ` Alvaro Gamez Machado 2021-07-29 22:21 ` Luca Ceresoli 1 sibling, 0 replies; 4+ messages in thread From: Alvaro Gamez Machado @ 2021-07-21 6:37 UTC (permalink / raw) To: buildroot, Luca Ceresoli On Thu, Jul 15, 2021 at 08:28:14AM +0200, Alvaro Gamez Machado wrote: > This adds support the Digilent Genesys ZU development board. > > Signed-off-by: Alvaro Gamez Machado <alvaro.gamez@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 ++++++++++++++++ > board/digilent/genesys-zu/uboot-env.txt | 83 + > board/digilent/genesys-zu/uboot_defconfig | 106 + > configs/digilent_genesys_zu_defconfig | 49 + > 14 files changed, 62551 insertions(+) > create mode 100644 board/digilent/genesys-zu/0001-scripts-dtc-Remove-redundant-YYLOC-global-declaratio.patch > create mode 100644 board/digilent/genesys-zu/0001-uboot-add-genesys-zu.patch > create mode 100644 board/digilent/genesys-zu/GenesysZU.dts > create mode 100644 board/digilent/genesys-zu/README.txt > create mode 100644 board/digilent/genesys-zu/genimage.cfg > create mode 100644 board/digilent/genesys-zu/image.its > create mode 100644 board/digilent/genesys-zu/kernel_defconfig > create mode 100644 board/digilent/genesys-zu/pm_cfg_obj.c > create mode 100755 board/digilent/genesys-zu/post-image.sh > create mode 100644 board/digilent/genesys-zu/psu_init_gpl.c > create mode 100644 board/digilent/genesys-zu/psu_init_gpl.h > create mode 100644 board/digilent/genesys-zu/uboot-env.txt > create mode 100644 board/digilent/genesys-zu/uboot_defconfig > create mode 100644 configs/digilent_genesys_zu_defconfig > Hi I sent this patch to the list, but it being too large got it stuck somewhere and didn't reach the list. The cause are files psu_init_gpl.{c,h} that are autogenerated by Xilinx utils, but that are needed to run the system. Can this message be moderated back into the list or shall I do something different? Thanks -- Alvaro G. M. _______________________________________________ buildroot mailing list buildroot@busybox.net http://lists.busybox.net/mailman/listinfo/buildroot ^ permalink raw reply [flat|nested] 4+ messages in thread
* Re: [Buildroot] [PATCH] configs/digilent_genesys_zu_defconfig: add Digilent Genesys ZU board (ZynqMP SoC) [not found] <20210715062814.8076-1-alvaro.gamez@hazent.com> 2021-07-21 6:37 ` [Buildroot] [PATCH] configs/digilent_genesys_zu_defconfig: add Digilent Genesys ZU board (ZynqMP SoC) Alvaro Gamez Machado @ 2021-07-29 22:21 ` Luca Ceresoli 2021-08-03 8:19 ` Alvaro Gamez 1 sibling, 1 reply; 4+ messages in thread From: Luca Ceresoli @ 2021-07-29 22:21 UTC (permalink / raw) To: Alvaro Gamez Machado, buildroot Hi Alvaro, thanks for your patch. And apologies for the late reply -- vacation time. 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@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. > 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@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@suse.com> > +Signed-off-by: David Gibson <david@gibson.dropbear.id.au> > +[robh: cherry-pick from upstream] > +Cc: stable@vger.kernel.org > +Signed-off-by: Rob Herring <robh@kernel.org> > +Signed-off-by: Alvaro Gamez Machado <alvaro.gamez@hazent.com> If this patch comes from upstream it would be nice to add an URL of where it can be found. > 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. > 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! > + > +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. > +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? > +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 > + 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? > 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. > 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? > 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? -- Luca _______________________________________________ buildroot mailing list buildroot@busybox.net http://lists.busybox.net/mailman/listinfo/buildroot ^ permalink raw reply [flat|nested] 4+ messages in thread
* Re: [Buildroot] [PATCH] configs/digilent_genesys_zu_defconfig: add Digilent Genesys ZU board (ZynqMP SoC) 2021-07-29 22:21 ` Luca Ceresoli @ 2021-08-03 8:19 ` Alvaro Gamez 2021-08-11 17:29 ` Luca Ceresoli 0 siblings, 1 reply; 4+ messages in thread From: Alvaro Gamez @ 2021-08-03 8:19 UTC (permalink / raw) To: Luca Ceresoli; +Cc: buildroot Hi Luca El vie, 30 jul 2021 a las 0:21, Luca Ceresoli (<luca@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@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@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@suse.com> > > +Signed-off-by: David Gibson <david@gibson.dropbear.id.au> > > +[robh: cherry-pick from upstream] > > +Cc: stable@vger.kernel.org > > +Signed-off-by: Rob Herring <robh@kernel.org> > > +Signed-off-by: Alvaro Gamez Machado <alvaro.gamez@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 _______________________________________________ buildroot mailing list buildroot@busybox.net http://lists.busybox.net/mailman/listinfo/buildroot ^ permalink raw reply [flat|nested] 4+ messages in thread
* Re: [Buildroot] [PATCH] configs/digilent_genesys_zu_defconfig: add Digilent Genesys ZU board (ZynqMP SoC) 2021-08-03 8:19 ` Alvaro Gamez @ 2021-08-11 17:29 ` Luca Ceresoli 0 siblings, 0 replies; 4+ messages in thread From: Luca Ceresoli @ 2021-08-11 17:29 UTC (permalink / raw) To: Alvaro Gamez; +Cc: buildroot Hi Alvaro, I'm replying here to topics that apply also for v2. I'm sending a separate review for the rest. On 03/08/21 10:19, Alvaro Gamez wrote: > Hi Luca > > El vie, 30 jul 2021 a las 0:21, Luca Ceresoli > (<luca@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@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@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@suse.com> >>> +Signed-off-by: David Gibson <david@gibson.dropbear.id.au> >>> +[robh: cherry-pick from upstream] >>> +Cc: stable@vger.kernel.org >>> +Signed-off-by: Rob Herring <robh@kernel.org> >>> +Signed-off-by: Alvaro Gamez Machado <alvaro.gamez@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. You should _really_ do it in v3. Come on, it's not going to take that much time! ;) >>> 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. Another question is: does any of the defconfigs provided in U-Boot work for your board? Why not? -- Luca _______________________________________________ buildroot mailing list buildroot@busybox.net http://lists.busybox.net/mailman/listinfo/buildroot ^ permalink raw reply [flat|nested] 4+ messages in thread
end of thread, other threads:[~2021-08-11 17:30 UTC | newest]
Thread overview: 4+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
[not found] <20210715062814.8076-1-alvaro.gamez@hazent.com>
2021-07-21 6:37 ` [Buildroot] [PATCH] configs/digilent_genesys_zu_defconfig: add Digilent Genesys ZU board (ZynqMP SoC) Alvaro Gamez Machado
2021-07-29 22:21 ` Luca Ceresoli
2021-08-03 8:19 ` Alvaro Gamez
2021-08-11 17:29 ` Luca Ceresoli
This is an external index of several public inboxes, see mirroring instructions on how to clone and mirror all data and code used by this external index.