Re: [Buildroot] [PATCH v4 01/10] package/binutils-bare-metal: new package

[Luca Ceresoli via buildroot <buildroot@buildroot.org>]

Hi Neal, Ibai,

On Sun, 24 Sep 2023 16:11:56 +0100
Neal Frager <neal.frager@amd.com> wrote:

> +# if toolchain is for microblazeel-xilinx, apply Xilinx patch set
> +ifeq ($(BR2_PACKAGE_HOST_TOOLCHAIN_BARE_METAL_ARCH),"microblazeel-xilinx")
> +HOST_BINUTILS_BARE_METAL_EXTRA_DOWNLOADS = https://github.com/Xilinx/meta-xilinx/archive/refs/tags/xlnx-rel-v2023.1.tar.gz
> +
> +define HOST_BINUTILS_BARE_METAL_EXTRACT_PATCHES
> +  mkdir -p $(@D)/patches
> +  tar -xf $(HOST_BINUTILS_BARE_METAL_DL_DIR)/xlnx-rel-v2023.1.tar.gz \
> +      --strip-components=5 \
> +      -C $(@D)/patches \
> +      meta-xilinx-xlnx-rel-v2023.1/meta-microblaze/recipes-devtools/binutils/binutils
> +endef
> +HOST_BINUTILS_BARE_METAL_POST_EXTRACT_HOOKS += HOST_BINUTILS_BARE_METAL_EXTRACT_PATCHES

I think all the patches needed to build zynqmp firmwares are now in
mainline, right? (which is a great thing!!) If it is the case, then you
should probably remove all this tarball mangling and just add the
relevant patches as regular package/*/*.patch files, both for binutils
and for gcc.

> +
> +define HOST_BINUTILS_BARE_METAL_APPLY_LOCAL_PATCHES
> +	$(APPLY_PATCHES) $(@D) $(@D)/patches *.patch;
> +endef
> +HOST_BINUTILS_BARE_METAL_POST_PATCH_HOOKS += HOST_BINUTILS_BARE_METAL_APPLY_LOCAL_PATCHES
> +endif #ifeq ($(BR2_PACKAGE_HOST_TOOLCHAIN_BARE_METAL_ARCH),"microblazeel-xilinx")
> +
> +# Don't build documentation. It takes up extra space / build time,
> +# and sometimes needs specific makeinfo versions to work
> +HOST_BINUTILS_BARE_METAL_CONF_ENV += MAKEINFO=true
> +HOST_BINUTILS_BARE_METAL_MAKE_OPTS += MAKEINFO=true
> +HOST_BINUTILS_BARE_METAL_INSTALL_OPTS += MAKEINFO=true install
> +
> +HOST_BINUTILS_BARE_METAL_CONF_OPTS = \
> +	--target=$(BR2_PACKAGE_HOST_TOOLCHAIN_BARE_METAL_ARCH)-elf \

As pointed out in v3, this patch series is not bisectable due to the
chicken-egg loop on BR2_PACKAGE_HOST_TOOLCHAIN_BARE_METAL_ARCH, which
is added in patch 4 and used in patches 1-3. I don't know how this
could be better handled, thus if the maintainers have no objections I
also won't have any.

Otherwise LGTM.

Luca

-- 
Luca Ceresoli, Bootlin
Embedded Linux and Kernel engineering
https://bootlin.com
_______________________________________________
buildroot mailing list
buildroot@buildroot.org
https://lists.buildroot.org/mailman/listinfo/buildroot