[PATCH v4 09/10] tools: spkgimage: add Renesas SPKG format

[Marek Vasut]

On 3/8/23 21:26, Ralph Siemsen wrote:

[...]

> diff --git a/tools/Makefile b/tools/Makefile
> index e13effbb66..1d7abcb916 100644
> --- a/tools/Makefile
> +++ b/tools/Makefile
> @@ -125,6 +125,7 @@ dumpimage-mkimage-objs := aisimage.o \
>   			stm32image.o \
>   			$(ROCKCHIP_OBS) \
>   			socfpgaimage.o \
> +			spkgimage.o \

Maybe just call the file renesas_spkgimage.o so its clear which 
SoC/vendor this file is associtated with.

>   			sunxi_egon.o \
>   			lib/crc16-ccitt.o \
>   			lib/hash-checksum.o \
> diff --git a/tools/spkgimage.c b/tools/spkgimage.c
> new file mode 100644
> index 0000000000..ea8036e114
> --- /dev/null
> +++ b/tools/spkgimage.c
> @@ -0,0 +1,330 @@
> +// SPDX-License-Identifier: BSD-2-Clause
> +/*
> + * Generate Renesas RZ/N1 BootROM header (SPKG)
> + * (C) Copyright 2022 Schneider Electric
> + *
> + * Based on spkg_utility.c
> + * (C) Copyright 2016 Renesas Electronics Europe Ltd
> + */
> +
> +#include "imagetool.h"
> +#include <limits.h>
> +#include <image.h>
> +#include <stdarg.h>
> +#include <stdint.h>
> +#include <u-boot/crc.h>
> +#include "spkgimage.h"
> +
> +static struct spkg_file out_buf;
> +
> +static uint32_t padding;

Is this padding here and the padding in struct config_file below 
different padding ? Can we get rid of these static global variables ?

> +/* Note: the ordering of the bitfields does not matter */
> +static struct config_file {
> +	unsigned int version:1;
> +	unsigned int ecc_block_size:2;
> +	unsigned int ecc_enable:1;
> +	unsigned int ecc_scheme:3;
> +	unsigned int ecc_bytes:8;
> +	unsigned int blp_len;
> +	unsigned int padding;
> +} conf;
> +
> +static int check_range(const char *name, int val, int min, int max)
> +{
> +	if (val < min) {
> +		fprintf(stderr, "Warning: param '%s' adjusted to min %d\n",
> +			name, min);
> +		val = min;
> +	}
> +
> +	if (val > max) {
> +		fprintf(stderr, "Warning: param '%s' adjusted to max %d\n",
> +			name, max);
> +		val = max;
> +	}

There is a macro clamp() which implements range limiting .

> +	return val;
> +}
> +
> +static int spkgimage_parse_config_line(char *line, size_t line_num)
> +{
> +	char *saveptr;
> +	char *delim = "\t ";
> +	char *name = strtok_r(line, delim, &saveptr);
> +	char *val_str = strtok_r(NULL, delim, &saveptr);
> +	int value = atoi(val_str);
> +
> +	if (!strcmp("VERSION", name)) {
> +		conf.version = check_range(name, value, 1, 15);
> +	} else if (!strcmp("NAND_ECC_ENABLE", name)) {
> +		conf.ecc_enable = check_range(name, value, 0, 1);
> +	} else if (!strcmp("NAND_ECC_BLOCK_SIZE", name)) {
> +		conf.ecc_block_size = check_range(name, value, 0, 2);
> +	} else if (!strcmp("NAND_ECC_SCHEME", name)) {
> +		conf.ecc_scheme = check_range(name, value, 0, 7);
> +	} else if (!strcmp("NAND_BYTES_PER_ECC_BLOCK", name)) {
> +		conf.ecc_bytes = check_range(name, value, 0, 255);
> +	} else if (!strcmp("ADD_DUMMY_BLP", name)) {
> +		conf.blp_len = value ? SPKG_BLP_SIZE : 0;
> +	} else if (!strcmp("PADDING", name)) {
> +		if (strrchr(val_str, 'K'))
> +			value = value * 1024;
> +		else if (strrchr(val_str, 'M'))
> +			value = value * 1024 * 1024;
> +		conf.padding = check_range(name, value, 1, INT_MAX);
> +	} else {
> +		fprintf(stderr,
> +			"config error: unknown keyword on line %ld\n",
> +			line_num);
> +		return -EINVAL;
> +	}
> +
> +	return 0;
> +}
> +
> +static int spkgimage_parse_config_file(char *filename)
> +{
> +	FILE *fcfg;
> +	char line[256];
> +	size_t len;
> +	size_t line_num = 0;
> +
> +	fcfg = fopen(filename, "r");
> +	if (!fcfg)
> +		return -EINVAL;
> +
> +	while (fgets(line, sizeof(line), fcfg)) {
> +		line_num += 1;
> +
> +		/* Skip blank lines and comments */
> +		if (line[0] == '\n' || line[0] == '#')
> +			continue;
> +
> +		/* Strip the trailing newline */
> +		len = strlen(line);
> +		if (line[len - 1] == '\n')
> +			line[--len] = 0;

Use len - 1 here too to avoid confusion ?

> +
> +		/* Parse the line */
> +		if (spkgimage_parse_config_line(line, line_num))
> +			return -EINVAL;
> +	}
> +
> +	fclose(fcfg);
> +
> +	/* Avoid divide-by-zero later on */
> +	if (!conf.padding)
> +		conf.padding = 1;
> +
> +	return 0;
> +}

[...]