[PATCH v6 3/5] cmd: mvebu: bubt: verify A38x target device type

[Stefan Roese]

On 17.04.20 17:38, Joel Johnson wrote:
> Ensure that the device to which an image is being written includes
> header information indicating boot support for the destination
> device.
> 
> This is derived from the support in the SolidRun master-a38x vendor
> fork.
> 
> Signed-off-by: Joel Johnson <mrjoel at lixil.net>

Reviewed-by: Stefan Roese <sr at denx.de>

Thanks,
Stefan

> ---
> 
> v2 changes:
>    - none
> v3 changes:
>    - use ARRAY_SIZE instead of #define size
> v4 changes:
>    - none
> v5 changes:
>    - use if(IS_ENABLED()) for inline check instead of ifdef
> v6 changes:
>    - reduce #ifdef usages at the tradeoff of otherwise unneccessarily
>      exposing A38x internal structs to more than A38x builds.
>    - since state is exposed anyway, rename a38x_check_boot_mode to
>      bubt_check_boot_mode with provision for per-target customization
> 
>      This fixes building across all mvebu targets. Exposing the needed
>      a38x_* structs isn't something I'm hugely excited about, but it does
>      get the build working and avoids inline #ifdefs as requested. In
>      cases where the target being built (all but A38x) doesn't support
>      checking the boot mode, rely on the compiler to [mostly] optimize
>      the function usage away. Local tests show an increase of only 4
>      bytes of rodata in such cases.
> 
> ---
>   cmd/mvebu/bubt.c | 52 ++++++++++++++++++++++++++++++++++++++++++++----
>   1 file changed, 48 insertions(+), 4 deletions(-)
> 
> diff --git a/cmd/mvebu/bubt.c b/cmd/mvebu/bubt.c
> index b80b81c82a..be6c5869a9 100644
> --- a/cmd/mvebu/bubt.c
> +++ b/cmd/mvebu/bubt.c
> @@ -85,7 +85,7 @@ struct mvebu_image_info {
>   	u32	encrypt_start_offset;
>   	u32	encrypt_size;
>   };
> -#elif defined(CONFIG_ARMADA_38X)	/* A38X */
> +#endif
>   
>   /* Structure of the main header, version 1 (Armada 370/38x/XP) */
>   struct a38x_main_hdr_v1 {
> @@ -107,7 +107,23 @@ struct a38x_main_hdr_v1 {
>   	u8  ext;                   /* 0x1E      */
>   	u8  checksum;              /* 0x1F      */
>   };
> -#endif
> +
> +struct a38x_boot_mode {
> +	unsigned int id;
> +	const char *name;
> +};
> +
> +/* The blockid header field values used to indicate boot device of image */
> +struct a38x_boot_mode a38x_boot_modes[] = {
> +	{ 0x4D, "i2c"  },
> +	{ 0x5A, "spi"  },
> +	{ 0x69, "uart" },
> +	{ 0x78, "sata" },
> +	{ 0x8B, "nand" },
> +	{ 0x9C, "pex"  },
> +	{ 0xAE, "mmc"  },
> +	{},
> +};
>   
>   struct bubt_dev {
>   	char name[8];
> @@ -697,7 +713,29 @@ static int check_image_header(void)
>   }
>   #endif
>   
> -static int bubt_verify(size_t image_size)
> +static int bubt_check_boot_mode(const struct bubt_dev *dst)
> +{
> +	if (IS_ENABLED(CONFIG_ARMADA_38X)) {
> +		int mode;
> +		const struct a38x_main_hdr_v1 *hdr =
> +			(struct a38x_main_hdr_v1 *)get_load_addr();
> +
> +		for (mode = 0; mode < ARRAY_SIZE(a38x_boot_modes); mode++) {
> +			if (strcmp(a38x_boot_modes[mode].name, dst->name) == 0)
> +				break;
> +		}
> +
> +		if (a38x_boot_modes[mode].id == hdr->blockid)
> +			return 0;
> +
> +		puts("Error: A38x image not built for destination device!\n");
> +		return -ENOEXEC;
> +	} else {
> +		return 0;
> +	}
> +}
> +
> +static int bubt_verify(const struct bubt_dev *dst)
>   {
>   	int err;
>   
> @@ -708,6 +746,12 @@ static int bubt_verify(size_t image_size)
>   		return err;
>   	}
>   
> +	err = bubt_check_boot_mode(dst);
> +	if (err) {
> +		printf("Error: Image boot mode verification failed\n");
> +		return err;
> +	}
> +
>   	return 0;
>   }
>   
> @@ -829,7 +873,7 @@ int do_bubt_cmd(cmd_tbl_t *cmdtp, int flag, int argc, char * const argv[])
>   	if (!image_size)
>   		return -EIO;
>   
> -	err = bubt_verify(image_size);
> +	err = bubt_verify(dst);
>   	if (err)
>   		return err;
>   
> 


Viele Grüße,
Stefan

-- 
DENX Software Engineering GmbH,      Managing Director: Wolfgang Denk
HRB 165235 Munich, Office: Kirchenstr.5, D-82194 Groebenzell, Germany
Phone: (+49)-8142-66989-51 Fax: (+49)-8142-66989-80 Email: sr at denx.de