[Buildroot] [PATCH v6 1/2] package/versal-firmware: new package

[Frager, Neal]

Hi Thomas,

> This patch adds support for downloading versal microblaze firmware binaries.
> These are necessary for booting Xilinx versal devices.
> 
> The location of these binaries is temporary, and will soon be added to 
> the Xilinx firmware repository.  The temporary location is using the 
> same free distribution license as the Xilinx firmware repository.
> 
> Once these files are available on the Xilinx repository, this package 
> will be updated to the new location.
> 
> Signed-off-by: Neal Frager <neal.frager at amd.com>

> I wanted to apply this... but there is a serious licensing issue I believe, see below.

Thank you for your review and feedback.  Licensing is still something that I do not understand as well as I probably should.

> diff --git a/package/versal-firmware/Config.in 
> b/package/versal-firmware/Config.in
> new file mode 100644
> index 0000000000..122ef02175
> --- /dev/null
> +++ b/package/versal-firmware/Config.in
> @@ -0,0 +1,24 @@
> +config BR2_PACKAGE_VERSAL_FIRMWARE
> +	bool "versal-firmware"
> +	depends on BR2_aarch64
> +	help
> +	  Pre-built firmware files for Xilinx Versal boards.
> +
> +	  https://github.com/nealfrager/buildroot-firmware
> +
> +if BR2_PACKAGE_VERSAL_FIRMWARE
> +
> +config BR2_PACKAGE_VERSAL_FIRMWARE_VERSION
> +	string "versal firmware version"

> Drop "versal" here: this option appears indented under "versal-firmware", so repeating "versal" is not needed.

Ok, no problem.

> +	default v2022.2

> Quotes around v2022.2

Ok.  I will fix this.

> +config BR2_PACKAGE_VERSAL_FIRMWARE_BOARD
> +	string "versal board name"
> +	default vck190

> Same comments here.

Understood.  I will fix this too.

> +	help
> +	  Name of Versal target board.
> +	  Used for installing the appropriate firmware boot.bin.
> +
> +endif # BR2_PACKAGE_VERSAL_FIRMWARE
> diff --git a/package/versal-firmware/versal-firmware.mk 
> b/package/versal-firmware/versal-firmware.mk
> new file mode 100644
> index 0000000000..cb36d1e045
> --- /dev/null
> +++ b/package/versal-firmware/versal-firmware.mk
> @@ -0,0 +1,23 @@
> +#####################################################################
> +###########
> +#
> +# versal-firmware
> +#
> +#####################################################################
> +###########
> +
> +VERSAL_FIRMWARE_VERSION = $(call 
> +qstrip,$(BR2_PACKAGE_VERSAL_FIRMWARE_VERSION))
> +VERSAL_FIRMWARE_SITE = $(call 
> +github,nealfrager,buildroot-firmware,$(BR2_PACKAGE_VERSAL_FIRMWARE_VE
> +RSION))

> Use $(VERSAL_FIRMWARE_VERSION) here

No problem.  Thanks for catching this.

> +VERSAL_FIRMWARE_LICENSE = "Xilinx-Binary-Only or GPL-2.0-or-later"
> +
> +VERSAL_FIRMWARE_LICENSE_FILES = LICENSE

> I really don't understand how this repository code can be under GPL-2.0-or-later. The GPL implies that source code is available as the GPL explicitly states that the software should be provided under its "preferred form of modification", which a closed source binary definitely is not.

> The first part somewhat makes sense, but the second part certainly does not make sense. If those binary files contain GPLv2 code, providing only the binaries is a violation of the GPL.

> So I'm sorry, but this needs to be fixed up before we can consider applying this package.

I copied the LICENSE file from the Xilinx/ubuntu-firmware repo below.  Looking more closely at it, I believe you are right that the license should just be Xilinx-Binary-Only.

I will update the LICENSE file to only include the binary portion as described in the below license:
https://github.com/Xilinx/ubuntu-firmware/blob/main/LICENSE

The source code for the versal_plm and versal_psmfw applications comes from the same Xilinx/embeddedsw repository as the zynqmp_pmufw, so the source and binary licenses for all of these files should be the same.
https://github.com/Xilinx/embeddedsw/tree/master/lib/sw_apps

If I modify the LICENSE file and remove the commentary about GPL, do you think this could be acceptable for inclusion in buildroot?  If there is anything else I need to add, please let me know.

Thanks again for your help.

Best regards,
Neal Frager
AMD
-------------- next part --------------
A non-text attachment was scrubbed...
Name: winmail.dat
Type: application/ms-tnef
Size: 15860 bytes
Desc: not available
URL: <http://lists.buildroot.org/pipermail/buildroot/attachments/20221125/916ecff1/attachment-0001.bin>