Message ID | 20231020114236.4129636-4-neal.frager@amd.com |
---|---|
State | Changes Requested |
Headers | show |
Series | [v6,01/10] package/binutils-bare-metal: new package | expand |
On Fri, 20 Oct 2023 12:42:30 +0100 Neal Frager <neal.frager@amd.com> wrote: > This patch adds a new virtual package for adding a bare-metal toolchain to > buildroot. By default, this package will configure a bare-metal toolchain > for the Xilinx microblaze little endian architecture. When configured for > the Xilinx microblaze architecture, this toolchain can be used to build the > microblaze firmware applications for zynqmp and versal. > > Signed-off-by: Ibai Erkiaga <ibai.erkiaga-elorza@amd.com> > Signed-off-by: Neal Frager <neal.frager@amd.com> [Tested on Kria KV260 starter kit] Tested-by: Luca Ceresoli <luca.ceresoli@bootlin.com> Reviewed-by: Luca Ceresoli <luca.ceresoli@bootlin.com>
Hello, On Fri, 20 Oct 2023 12:42:30 +0100 Neal Frager via buildroot <buildroot@buildroot.org> wrote: > diff --git a/toolchain/Config.in b/toolchain/Config.in > index d8081f1b9d..b1333f92f0 100644 > --- a/toolchain/Config.in > +++ b/toolchain/Config.in > @@ -59,6 +59,7 @@ config BR2_TOOLCHAIN_EXTERNAL > > endchoice > > +source "toolchain/toolchain-bare-metal-buildroot/Config.in" > source "toolchain/toolchain-buildroot/Config.in" > source "toolchain/toolchain-external/Config.in" I'm not really happy with where this ends up showing in menuconfig. Indeed, in menuconfig, we end up seeing something like this: Toolchain type (External toolchain) ---> [*] host toolchain-bare-metal -*- host binutils-bare-metal -*- host gcc-bare-metal -*- host newlib-bare-metal *** Toolchain External Options *** Toolchain (Bootlin toolchains) ---> So basically, right in the middle of the settings for the "normal" toolchain, we have those options about the bare-metal toolchain. Maybe something like this instead: diff --git a/toolchain/Config.in b/toolchain/Config.in index b1333f92f0..278d1de65c 100644 --- a/toolchain/Config.in +++ b/toolchain/Config.in @@ -59,7 +59,6 @@ config BR2_TOOLCHAIN_EXTERNAL endchoice -source "toolchain/toolchain-bare-metal-buildroot/Config.in" source "toolchain/toolchain-buildroot/Config.in" source "toolchain/toolchain-external/Config.in" @@ -919,4 +918,8 @@ config BR2_TOOLCHAIN_HAS_LIBQUADMATH default y if BR2_i386 || BR2_x86_64 default y if BR2_POWERPC_CPU_HAS_VSX +comment "Bare metal toolchain" + +source "toolchain/toolchain-bare-metal-buildroot/Config.in" + endmenu Basically, move it at the end of toolchain/Config.in, with a small Config.in comment separating it from the rest. > > diff --git a/toolchain/toolchain-bare-metal-buildroot/Config.in b/toolchain/toolchain-bare-metal-buildroot/Config.in > new file mode 100644 > index 0000000000..6ebf2cdf86 > --- /dev/null > +++ b/toolchain/toolchain-bare-metal-buildroot/Config.in > @@ -0,0 +1,21 @@ > +config BR2_PACKAGE_HOST_TOOLCHAIN_BARE_METAL_BUILDROOT > + bool "host toolchain-bare-metal" > + select BR2_PACKAGE_HOST_BINUTILS_BARE_METAL > + select BR2_PACKAGE_HOST_GCC_BARE_METAL > + select BR2_PACKAGE_HOST_NEWLIB_BARE_METAL Those selects are no longer needed, as all those options will be gone. > + help > + Build a bare-metal toolchain in addition to the main Linux toolchain > + > +if BR2_PACKAGE_HOST_TOOLCHAIN_BARE_METAL_BUILDROOT > + > +config BR2_PACKAGE_HOST_TOOLCHAIN_BARE_METAL_BUILDROOT_ARCH > + string This option needs to be made visible (with a prompt), otherwise nobody can chose a different architecture/tuple. > + default "microblazeel-xilinx" Is "xilinx" really relevant here? I don't think it is. I'm also wondering whether having a default value actually makes sense. It should probably just stay empty by default, and be defined by the specific defconfigs that need this? > + help > + select architecture for bare-metal toolchain > + > +source "package/binutils-bare-metal/Config.in.host" > +source "package/gcc-bare-metal/Config.in.host" > +source "package/newlib-bare-metal/Config.in.host" These file inclusions can be dropped. > +++ b/toolchain/toolchain-bare-metal-buildroot/toolchain-bare-metal-buildroot.mk > @@ -0,0 +1,7 @@ > +################################################################################ > +# > +# toolchain-bare-metal-buildroot > +# > +################################################################################ > + > +(eval $(host-virtual-package)) I'm a bit rusty with virtual packages, but I don't see where this virtual package pulls in host-newlib-bare-metal as a dependency. Don't you need: TOOLCHAIN_BARE_METAL_BUILDROOT_DEPENDENCIES = host-newlib-bare-metal to make sure you trigger the build of host-newlib-bare-metal -> host-gcc-bare-metal -> host-binutils-bare-metal ? Best regards, Thomas
Hi Thomas, > Le 31 oct. 2023 à 06:51, Thomas Petazzoni <thomas.petazzoni@bootlin.com> a écrit : > > Hello, > >> On Fri, 20 Oct 2023 12:42:30 +0100 >> Neal Frager via buildroot <buildroot@buildroot.org> wrote: >> >> >> diff --git a/toolchain/Config.in b/toolchain/Config.in >> index d8081f1b9d..b1333f92f0 100644 >> --- a/toolchain/Config.in >> +++ b/toolchain/Config.in >> @@ -59,6 +59,7 @@ config BR2_TOOLCHAIN_EXTERNAL >> >> endchoice >> >> +source "toolchain/toolchain-bare-metal-buildroot/Config.in" >> source "toolchain/toolchain-buildroot/Config.in" >> source "toolchain/toolchain-external/Config.in" > > I'm not really happy with where this ends up showing in menuconfig. > Indeed, in menuconfig, we end up seeing something like this: > > Toolchain type (External toolchain) ---> > [*] host toolchain-bare-metal > -*- host binutils-bare-metal > -*- host gcc-bare-metal > -*- host newlib-bare-metal > *** Toolchain External Options *** > Toolchain (Bootlin toolchains) ---> > > So basically, right in the middle of the settings for the "normal" > toolchain, we have those options about the bare-metal toolchain. > > Maybe something like this instead: > > diff --git a/toolchain/Config.in b/toolchain/Config.in > index b1333f92f0..278d1de65c 100644 > --- a/toolchain/Config.in > +++ b/toolchain/Config.in > @@ -59,7 +59,6 @@ config BR2_TOOLCHAIN_EXTERNAL > > endchoice > > -source "toolchain/toolchain-bare-metal-buildroot/Config.in" > source "toolchain/toolchain-buildroot/Config.in" > source "toolchain/toolchain-external/Config.in" > > @@ -919,4 +918,8 @@ config BR2_TOOLCHAIN_HAS_LIBQUADMATH > default y if BR2_i386 || BR2_x86_64 > default y if BR2_POWERPC_CPU_HAS_VSX > > +comment "Bare metal toolchain" > + > +source "toolchain/toolchain-bare-metal-buildroot/Config.in" > + > endmenu > > Basically, move it at the end of toolchain/Config.in, with a small > Config.in comment separating it from the rest. Good idea. I am implementing this for v7. > >> >> diff --git a/toolchain/toolchain-bare-metal-buildroot/Config.in b/toolchain/toolchain-bare-metal-buildroot/Config.in >> new file mode 100644 >> index 0000000000..6ebf2cdf86 >> --- /dev/null >> +++ b/toolchain/toolchain-bare-metal-buildroot/Config.in >> @@ -0,0 +1,21 @@ >> +config BR2_PACKAGE_HOST_TOOLCHAIN_BARE_METAL_BUILDROOT >> + bool "host toolchain-bare-metal" >> + select BR2_PACKAGE_HOST_BINUTILS_BARE_METAL >> + select BR2_PACKAGE_HOST_GCC_BARE_METAL >> + select BR2_PACKAGE_HOST_NEWLIB_BARE_METAL > > Those selects are no longer needed, as all those options will be gone. Yes, changes done for v7. Except for the newlib-bare-metal package which will be a target package still requiring a select. > >> + help >> + Build a bare-metal toolchain in addition to the main Linux toolchain >> + >> +if BR2_PACKAGE_HOST_TOOLCHAIN_BARE_METAL_BUILDROOT >> + >> +config BR2_PACKAGE_HOST_TOOLCHAIN_BARE_METAL_BUILDROOT_ARCH >> + string > > This option needs to be made visible (with a prompt), otherwise nobody > can chose a different architecture/tuple. Yes, you are right. I am fixing this for v7. > >> + default "microblazeel-xilinx" > > Is "xilinx" really relevant here? I don't think it is. I'm also > wondering whether having a default value actually makes sense. It > should probably just stay empty by default, and be defined by the > specific defconfigs that need this? > In my testing, binutils and gcc expect the arch name microblazeel-xilinx-elf with the xilinx included. I will change the definition to the full tuple using microblazeel-xilinx-elf in v7. As for the default, I am not sure we should get rid of having a default. If the toolchain bare metal buildroot gets selected without an architecture defined, we will have build errors with binutils, gcc and newlib. If it is ok with you, I would prefer to keep a default architecture definition, but making it visible, so users could configure a different architecture for another platform. >> + help >> + select architecture for bare-metal toolchain >> + >> +source "package/binutils-bare-metal/Config.in.host" >> +source "package/gcc-bare-metal/Config.in.host" >> +source "package/newlib-bare-metal/Config.in.host" > > These file inclusions can be dropped. Except for the newlib one, yes, the others will be dropped in v7. > >> +++ b/toolchain/toolchain-bare-metal-buildroot/toolchain-bare-metal-buildroot.mk >> @@ -0,0 +1,7 @@ >> +################################################################################ >> +# >> +# toolchain-bare-metal-buildroot >> +# >> +################################################################################ >> + >> +(eval $(host-virtual-package)) > > I'm a bit rusty with virtual packages, but I don't see where this > virtual package pulls in host-newlib-bare-metal as a dependency. Don't > you need: > > TOOLCHAIN_BARE_METAL_BUILDROOT_DEPENDENCIES = host-newlib-bare-metal > > to make sure you trigger the build of host-newlib-bare-metal -> > host-gcc-bare-metal -> host-binutils-bare-metal ? Yes, I am fixing all of the dependencies for v7, so all of the packages build in the correct order. > > Best regards, > > Thomas > -- > Thomas Petazzoni, co-owner and CEO, Bootlin > Embedded Linux and Kernel engineering and training > https://bootlin.com Thank you for your support! Best regards, Neal Frager AMD
diff --git a/DEVELOPERS b/DEVELOPERS index 677b8f09bd..849a5263b5 100644 --- a/DEVELOPERS +++ b/DEVELOPERS @@ -1370,6 +1370,7 @@ N: Ibai Erkiaga <ibai.erkiaga-elorza@amd.com> F: package/binutils-bare-metal/ F: package/gcc-bare-metal/ F: package/newlib-bare-metal/ +F: toolchain/toolchain-bare-metal-buildroot/ N: Ignacy Gawędzki <ignacy.gawedzki@green-communications.fr> F: package/angularjs/ @@ -2207,6 +2208,7 @@ F: package/bootgen/ F: package/gcc-bare-metal/ F: package/newlib-bare-metal/ F: package/versal-firmware/ +F: toolchain/toolchain-bare-metal-buildroot/ N: Nicola Di Lieto <nicola.dilieto@gmail.com> F: package/uacme/ diff --git a/toolchain/Config.in b/toolchain/Config.in index d8081f1b9d..b1333f92f0 100644 --- a/toolchain/Config.in +++ b/toolchain/Config.in @@ -59,6 +59,7 @@ config BR2_TOOLCHAIN_EXTERNAL endchoice +source "toolchain/toolchain-bare-metal-buildroot/Config.in" source "toolchain/toolchain-buildroot/Config.in" source "toolchain/toolchain-external/Config.in" diff --git a/toolchain/toolchain-bare-metal-buildroot/Config.in b/toolchain/toolchain-bare-metal-buildroot/Config.in new file mode 100644 index 0000000000..6ebf2cdf86 --- /dev/null +++ b/toolchain/toolchain-bare-metal-buildroot/Config.in @@ -0,0 +1,21 @@ +config BR2_PACKAGE_HOST_TOOLCHAIN_BARE_METAL_BUILDROOT + bool "host toolchain-bare-metal" + select BR2_PACKAGE_HOST_BINUTILS_BARE_METAL + select BR2_PACKAGE_HOST_GCC_BARE_METAL + select BR2_PACKAGE_HOST_NEWLIB_BARE_METAL + help + Build a bare-metal toolchain in addition to the main Linux toolchain + +if BR2_PACKAGE_HOST_TOOLCHAIN_BARE_METAL_BUILDROOT + +config BR2_PACKAGE_HOST_TOOLCHAIN_BARE_METAL_BUILDROOT_ARCH + string + default "microblazeel-xilinx" + help + select architecture for bare-metal toolchain + +source "package/binutils-bare-metal/Config.in.host" +source "package/gcc-bare-metal/Config.in.host" +source "package/newlib-bare-metal/Config.in.host" + +endif #BR2_PACKAGE_HOST_TOOLCHAIN_BARE_METAL_BUILDROOT diff --git a/toolchain/toolchain-bare-metal-buildroot/toolchain-bare-metal-buildroot.mk b/toolchain/toolchain-bare-metal-buildroot/toolchain-bare-metal-buildroot.mk new file mode 100644 index 0000000000..170c73aa31 --- /dev/null +++ b/toolchain/toolchain-bare-metal-buildroot/toolchain-bare-metal-buildroot.mk @@ -0,0 +1,7 @@ +################################################################################ +# +# toolchain-bare-metal-buildroot +# +################################################################################ + +(eval $(host-virtual-package))