Re: [Buildroot] [PATCH v2 1/4] package: andes-spi-burn: new package

[Thomas Petazzoni via buildroot <buildroot@buildroot.org>]

Hello Peter,

Thanks for this patch! See some comments below.

The commit title should be:

	package/andes-spi-burn: new package

On Wed, 4 Sep 2024 13:16:06 +0800
Yu Chien Peter Lin via buildroot <buildroot@buildroot.org> wrote:


> diff --git a/package/andes-spi-burn/Config.in.host b/package/andes-spi-burn/Config.in.host
> new file mode 100644
> index 0000000000..fed4f49f5d
> --- /dev/null
> +++ b/package/andes-spi-burn/Config.in.host
> @@ -0,0 +1,8 @@
> +config BR2_PACKAGE_HOST_ANDES_SPI_BURN
> +	bool "host andes-spi-burn"
> +	depends on BR2_riscv
> +	help
> +		Andes Technology SPI_burn tool to program bootloader and
> +		device-tree blob onto flash memory of AE350 platform.
> +
> +		https://github.com/andestech/Andes-Development-Kit

Indentation of the help text is one tab + 2 spaces. You can run "make
check-package" to get some coding style check.

> diff --git a/package/andes-spi-burn/andes-spi-burn.hash b/package/andes-spi-burn/andes-spi-burn.hash
> new file mode 100644
> index 0000000000..170f362646
> --- /dev/null
> +++ b/package/andes-spi-burn/andes-spi-burn.hash
> @@ -0,0 +1,2 @@
> +# Locally computed
> +sha256  6632bc0ddd0b6d07bd328758cd9132f48429d144079eb8011af60d95b0bdfd6b  flash.zip

No license file?

> diff --git a/package/andes-spi-burn/andes-spi-burn.mk b/package/andes-spi-burn/andes-spi-burn.mk
> new file mode 100644
> index 0000000000..2f7387ca2f
> --- /dev/null
> +++ b/package/andes-spi-burn/andes-spi-burn.mk
> @@ -0,0 +1,25 @@
> +################################################################################
> +#
> +# andes-spi-burn
> +#
> +################################################################################
> +
> +ANDES_SPI_BURN_VERSION = 5.3.0
> +ANDES_SPI_BURN_SOURCE = flash.zip

This will unfortunately not work, as the file name doesn't include a
version. So when you will update the package, there will already be a
flash.zip in $DL_DIR/andes-spi-burn/ and the new one will not be
re-downloaded. Therefore, you need to find a way to make sure that the
filename contains the version.

Alternatively, perhaps you could simply use a git clone instead, with
ANDES_SPI_BURN_SITE_METHOD = git.

Except I see your Git repo doesn't actually contain anything... This is
really not great.

> +ANDES_SPI_BURN_SITE = https://github.com/andestech/Andes-Development-Kit/releases/download/ast-v5_3_0-release-windows
> +ANDES_SPI_BURN_LICENSE = Apache-2.0

No license file?

> +
> +define HOST_ANDES_SPI_BURN_EXTRACT_CMDS
> +	$(UNZIP) -q $(HOST_ANDES_SPI_BURN_DL_DIR)/$(ANDES_SPI_BURN_SOURCE) -d $(@D)
> +endef
> +
> +define HOST_ANDES_SPI_BURN_BUILD_CMDS
> +	$(HOST_MAKE_ENV) $(MAKE) -C $(@D)/flash/src-SPI_burn clean

No need to do the "clean" first. We never do it in Buildroot packages.

> +	$(HOST_MAKE_ENV) $(MAKE) -C $(@D)/flash/src-SPI_burn --makefile=Makefile_SPIburn_linux
> +endef
> +
> +define HOST_ANDES_SPI_BURN_INSTALL_CMDS
> +	$(INSTALL) -D -m 0755 $(@D)/flash/src-SPI_burn/SPI_burn $(HOST_DIR)/bin/SPI_burn
> +endef
> +
> +$(eval $(host-generic-package))

The rest seems OK to me!

Thomas
-- 
Thomas Petazzoni, co-owner and CEO, Bootlin
Embedded Linux and Kernel engineering and training
https://bootlin.com
_______________________________________________
buildroot mailing list
buildroot@buildroot.org
https://lists.buildroot.org/mailman/listinfo/buildroot