[U-Boot] [PATCH 0/5] Fixes to Marvell A38x platforms

[U-Boot] [PATCH 0/5] Fixes to Marvell A38x platforms

[kostap at marvell.com]

From: Konstantin Porotchkin <kostap@marvell.com>

The support for legacy Marvell SoC plaform  db-88f6820-gp is broken starting 
middle of 2017.
This patch series returns functionality to this platform by fixing
problem in environment handling load from SPI flash.
Additionally the "bubt" command is extended to support legacy image verification
and added to db-88f6820-gp default configuration.

Konstantin Porotchkin (5):
  fix: env: Fix the SPI flash device setup for DM mode
  fix: mvebu: Add SPI parameters for environment setup
  fix: cmd: mvebu: Eclude mvebu commands from SPL builds
  cmd: mvebu: bubt: Add support for legacy Marvell SoCs
  defconfig: db-88f6820-gp: Add bubt command to the build

 cmd/Makefile                    |  3 +-
 cmd/mvebu/bubt.c                | 75 +++++++++++++++++++++++++++++++++++++++--
 configs/db-88f6820-gp_defconfig |  1 +
 env/sf.c                        |  3 +-
 include/configs/db-88f6820-gp.h |  6 ++++
 5 files changed, 83 insertions(+), 5 deletions(-)

-- 
2.7.4

[U-Boot] [PATCH 1/5] fix: env: Fix the SPI flash device setup for DM mode

[kostap at marvell.com]

From: Konstantin Porotchkin <kostap@marvell.com>

For some reason the spi_flash_probe_bus_cs() is called
inside the setup_flash_device() with zero values in place
of configurated SPI flash mode and maximum flash speed.
This code causes HALT error during startup environment
relocation on some platforms - namely Armada-38x-GP board.
Fix the function call by replacing zeros with the appropriate
values - CONFIG_ENV_SPI_MAX_HZ and CONFIG_ENV_SPI_MODE.

Signed-off-by: Konstantin Porotchkin <kostap@marvell.com>
Cc: Igal Liberman <igall@marvell.com>
Cc: Stefan Roese <sr@denx.de>
---
 env/sf.c | 3 ++-
 1 file changed, 2 insertions(+), 1 deletion(-)

diff --git a/env/sf.c b/env/sf.c
index 4945105..2e3c600 100644
--- a/env/sf.c
+++ b/env/sf.c
@@ -58,7 +58,8 @@ static int setup_flash_device(void)
 
 	/* speed and mode will be read from DT */
 	ret = spi_flash_probe_bus_cs(CONFIG_ENV_SPI_BUS, CONFIG_ENV_SPI_CS,
-				     0, 0, &new);
+				     CONFIG_ENV_SPI_MAX_HZ, CONFIG_ENV_SPI_MODE,
+				     &new);
 	if (ret) {
 		set_default_env("spi_flash_probe_bus_cs() failed", 0);
 		return ret;
-- 
2.7.4

[U-Boot] [PATCH 1/5] fix: env: Fix the SPI flash device setup for DM mode

[Stefan Roese]

On 29.08.2018 15:34, kostap at marvell.com wrote:
> From: Konstantin Porotchkin <kostap@marvell.com>
> 
> For some reason the spi_flash_probe_bus_cs() is called
> inside the setup_flash_device() with zero values in place
> of configurated SPI flash mode and maximum flash speed.
> This code causes HALT error during startup environment
> relocation on some platforms - namely Armada-38x-GP board.
> Fix the function call by replacing zeros with the appropriate
> values - CONFIG_ENV_SPI_MAX_HZ and CONFIG_ENV_SPI_MODE.
> 
> Signed-off-by: Konstantin Porotchkin <kostap@marvell.com>
> Cc: Igal Liberman <igall@marvell.com>
> Cc: Stefan Roese <sr@denx.de>
> ---
>   env/sf.c | 3 ++-
>   1 file changed, 2 insertions(+), 1 deletion(-)
> 
> diff --git a/env/sf.c b/env/sf.c
> index 4945105..2e3c600 100644
> --- a/env/sf.c
> +++ b/env/sf.c
> @@ -58,7 +58,8 @@ static int setup_flash_device(void)
>   
>   	/* speed and mode will be read from DT */
>   	ret = spi_flash_probe_bus_cs(CONFIG_ENV_SPI_BUS, CONFIG_ENV_SPI_CS,
> -				     0, 0, &new);
> +				     CONFIG_ENV_SPI_MAX_HZ, CONFIG_ENV_SPI_MODE,
> +				     &new);
>   	if (ret) {
>   		set_default_env("spi_flash_probe_bus_cs() failed", 0);
>   		return ret;
> 

Applied to u-boot-marvell/master

Thanks,
Stefan

[U-Boot] [PATCH 1/5] fix: env: Fix the SPI flash device setup for DM mode

[Adam Ford]

On Wed, Aug 29, 2018 at 8:37 AM <kostap@marvell.com> wrote:
>
> From: Konstantin Porotchkin <kostap@marvell.com>
>
> For some reason the spi_flash_probe_bus_cs() is called
> inside the setup_flash_device() with zero values in place
> of configurated SPI flash mode and maximum flash speed.
> This code causes HALT error during startup environment
> relocation on some platforms - namely Armada-38x-GP board.
> Fix the function call by replacing zeros with the appropriate
> values - CONFIG_ENV_SPI_MAX_HZ and CONFIG_ENV_SPI_MODE.
>

This patch appears to be causing some issues with DA850 EVM in that
make it fail the CRC check and the environmental variables go to
defaults.

Reading the comment, "speed and mode will be read from DT", however
this patch explicitly uses CONFIG_ENV_SPI_MAX_HZ, CONFIG_ENV_SPI_MODE
which are not defined in the DT, but setup by either Kconfig or
include/configs.

I can create a patch that fixes the da850 by hard coding these two
values to 0 to make them match what was originally happening, but it
seems like we have the DT case and the non-DT case, but we're treating
them the same, so I thought I'd point that out here.

adam

> Signed-off-by: Konstantin Porotchkin <kostap@marvell.com>
> Cc: Igal Liberman <igall@marvell.com>
> Cc: Stefan Roese <sr@denx.de>
> ---
>  env/sf.c | 3 ++-
>  1 file changed, 2 insertions(+), 1 deletion(-)
>
> diff --git a/env/sf.c b/env/sf.c
> index 4945105..2e3c600 100644
> --- a/env/sf.c
> +++ b/env/sf.c
> @@ -58,7 +58,8 @@ static int setup_flash_device(void)
>
>         /* speed and mode will be read from DT */
>         ret = spi_flash_probe_bus_cs(CONFIG_ENV_SPI_BUS, CONFIG_ENV_SPI_CS,
> -                                    0, 0, &new);
> +                                    CONFIG_ENV_SPI_MAX_HZ, CONFIG_ENV_SPI_MODE,
> +                                    &new);
>         if (ret) {
>                 set_default_env("spi_flash_probe_bus_cs() failed", 0);
>                 return ret;
> --
> 2.7.4
>
> _______________________________________________
> U-Boot mailing list
> U-Boot at lists.denx.de
> https://lists.denx.de/listinfo/u-boot

[U-Boot] [PATCH 2/5] fix: mvebu: Add SPI parameters for environment setup

[kostap at marvell.com]

From: Konstantin Porotchkin <kostap@marvell.com>

Add definitions for CONFIG_ENV_SPI_BUS and CONFIG_ENV_SPI_CS
to Armada-388-GP board configuration

Signed-off-by: Konstantin Porotchkin <kostap@marvell.com>
Cc: Igal Liberman <igall@marvell.com>
Cc: Stefan Roese <sr@denx.de>
---
 include/configs/db-88f6820-gp.h | 6 ++++++
 1 file changed, 6 insertions(+)

diff --git a/include/configs/db-88f6820-gp.h b/include/configs/db-88f6820-gp.h
index ac810b0..f2aa21a 100644
--- a/include/configs/db-88f6820-gp.h
+++ b/include/configs/db-88f6820-gp.h
@@ -28,6 +28,12 @@
 #define CONFIG_SYS_I2C_SLAVE		0x0
 #define CONFIG_SYS_I2C_SPEED		100000
 
+/*
+ * SPI Flash configuration for the environemnt access
+ */
+#define CONFIG_ENV_SPI_BUS		0
+#define CONFIG_ENV_SPI_CS		0
+
 /* SPI NOR flash default params, used by sf commands */
 #define CONFIG_SF_DEFAULT_SPEED		1000000
 #define CONFIG_SF_DEFAULT_MODE		SPI_MODE_3
-- 
2.7.4

[U-Boot] [PATCH 2/5] fix: mvebu: Add SPI parameters for environment setup

[Stefan Roese]

On 29.08.2018 15:34, kostap at marvell.com wrote:
> From: Konstantin Porotchkin <kostap@marvell.com>
> 
> Add definitions for CONFIG_ENV_SPI_BUS and CONFIG_ENV_SPI_CS
> to Armada-388-GP board configuration
> 
> Signed-off-by: Konstantin Porotchkin <kostap@marvell.com>
> Cc: Igal Liberman <igall@marvell.com>
> Cc: Stefan Roese <sr@denx.de>
> ---
>   include/configs/db-88f6820-gp.h | 6 ++++++
>   1 file changed, 6 insertions(+)
> 
> diff --git a/include/configs/db-88f6820-gp.h b/include/configs/db-88f6820-gp.h
> index ac810b0..f2aa21a 100644
> --- a/include/configs/db-88f6820-gp.h
> +++ b/include/configs/db-88f6820-gp.h
> @@ -28,6 +28,12 @@
>   #define CONFIG_SYS_I2C_SLAVE		0x0
>   #define CONFIG_SYS_I2C_SPEED		100000
>   
> +/*
> + * SPI Flash configuration for the environemnt access
> + */
> +#define CONFIG_ENV_SPI_BUS		0
> +#define CONFIG_ENV_SPI_CS		0
> +
>   /* SPI NOR flash default params, used by sf commands */
>   #define CONFIG_SF_DEFAULT_SPEED		1000000
>   #define CONFIG_SF_DEFAULT_MODE		SPI_MODE_3
> 

Applied to u-boot-marvell/master

Thanks,
Stefan

[U-Boot] [PATCH 3/5] fix: cmd: mvebu: Eclude mvebu commands from SPL builds

[kostap at marvell.com]

From: Konstantin Porotchkin <kostap@marvell.com>

Exclude mvebu commands from SPL builds

Signed-off-by: Konstantin Porotchkin <kostap@marvell.com>
Cc: Igal Liberman <igall@marvell.com>
Cc: Stefan Roese <sr@denx.de>
---
 cmd/Makefile | 3 ++-
 1 file changed, 2 insertions(+), 1 deletion(-)

diff --git a/cmd/Makefile b/cmd/Makefile
index 3487c80..a61fab6 100644
--- a/cmd/Makefile
+++ b/cmd/Makefile
@@ -163,12 +163,13 @@ obj-$(CONFIG_CMD_BLOB) += blob.o
 obj-$(CONFIG_CMD_AVB) += avb.o
 
 obj-$(CONFIG_X86) += x86/
+
+obj-$(CONFIG_ARCH_MVEBU) += mvebu/
 endif # !CONFIG_SPL_BUILD
 
 # core command
 obj-y += nvedit.o
 
-obj-$(CONFIG_ARCH_MVEBU) += mvebu/
 obj-$(CONFIG_TI_COMMON_CMD_OPTIONS) += ti/
 
 filechk_data_gz = (echo "static const char data_gz[] ="; cat $< | scripts/bin2c; echo ";")
-- 
2.7.4

[U-Boot] [PATCH 3/5] fix: cmd: mvebu: Eclude mvebu commands from SPL builds

[Stefan Roese]

On 29.08.2018 15:34, kostap at marvell.com wrote:
> From: Konstantin Porotchkin <kostap@marvell.com>
> 
> Exclude mvebu commands from SPL builds
> 
> Signed-off-by: Konstantin Porotchkin <kostap@marvell.com>
> Cc: Igal Liberman <igall@marvell.com>
> Cc: Stefan Roese <sr@denx.de>
> ---
>   cmd/Makefile | 3 ++-
>   1 file changed, 2 insertions(+), 1 deletion(-)
> 
> diff --git a/cmd/Makefile b/cmd/Makefile
> index 3487c80..a61fab6 100644
> --- a/cmd/Makefile
> +++ b/cmd/Makefile
> @@ -163,12 +163,13 @@ obj-$(CONFIG_CMD_BLOB) += blob.o
>   obj-$(CONFIG_CMD_AVB) += avb.o
>   
>   obj-$(CONFIG_X86) += x86/
> +
> +obj-$(CONFIG_ARCH_MVEBU) += mvebu/
>   endif # !CONFIG_SPL_BUILD
>   
>   # core command
>   obj-y += nvedit.o
>   
> -obj-$(CONFIG_ARCH_MVEBU) += mvebu/
>   obj-$(CONFIG_TI_COMMON_CMD_OPTIONS) += ti/
>   
>   filechk_data_gz = (echo "static const char data_gz[] ="; cat $< | scripts/bin2c; echo ";")
> 

Applied to u-boot-marvell/master

Thanks,
Stefan

[U-Boot] [PATCH 4/5] cmd: mvebu: bubt: Add support for legacy Marvell SoCs

[kostap at marvell.com]

From: Konstantin Porotchkin <kostap@marvell.com>

Add support for image load and basic verification in bubt
for legacy Marvell SoCs (A38x, A39x, ...)

Signed-off-by: Konstantin Porotchkin <kostap@marvell.com>
Cc: Igal Liberman <igall@marvell.com>
Cc: Stefan Roese <sr@denx.de>
---
 cmd/mvebu/bubt.c | 75 +++++++++++++++++++++++++++++++++++++++++++++++++++++---
 1 file changed, 72 insertions(+), 3 deletions(-)

diff --git a/cmd/mvebu/bubt.c b/cmd/mvebu/bubt.c
index b4d371f..e10d079 100644
--- a/cmd/mvebu/bubt.c
+++ b/cmd/mvebu/bubt.c
@@ -83,6 +83,33 @@ struct mvebu_image_info {
 	u32	encrypt_start_offset;
 	u32	encrypt_size;
 };
+
+#else /* Older Armada SoCs - A38x, A39x, ... */
+
+#define	LEGACY_HDR_VERSION	1
+
+struct legacy_image_header {
+/*	type	name			byte order */
+	u8	block_id;		/*   0   */
+	u8	flags;			/*   1   */
+	u16	nand_pge_size;		/*  2-3  */
+	u32	block_size;		/*  4-7  */
+	u8	version;		/*   8   */
+	u8	hdr_size_msb;		/*   9   */
+	u16	hdr_size_lsb;		/* 10-11 */
+	u32	source_addr;		/* 12-15 */
+	u32	destination_addr;	/* 16-19 */
+	u32	execution_addr;		/* 20-23 */
+	u8	options;		/*  24   */
+	u8	nand_block_size;	/*  25   */
+	u8	nand_technology;	/*  26   */
+	u8	rsvd4;			/*  27   */
+	u16	rsvd2;			/* 28-29 */
+	u8	ext;			/*  30   */
+	u8	checksum;		/*  31   */
+
+};
+
 #endif /* CONFIG_ARMADA_XXX */
 
 struct bubt_dev {
@@ -618,11 +645,53 @@ static int check_image_header(void)
 	return 0;
 }
 
-#else /* Not ARMADA? */
+#else /* Legacy SoCs */
+u8 do_checksum8(u8 *start, u32 len)
+{
+	u8 sum = 0;
+	u8 *startp = start;
+
+	do {
+		sum += *startp;
+		startp++;
+	} while (--len);
+
+	return sum;
+}
+
 static int check_image_header(void)
 {
-	printf("bubt cmd does not support this SoC device or family!\n");
-	return -ENOEXEC;
+	struct legacy_image_header *hdr =
+			(struct legacy_image_header *)get_load_addr();
+	u32 header_len = hdr->hdr_size_lsb + (hdr->hdr_size_msb << 16);
+	u8 checksum;
+	u8 checksum_ref = hdr->checksum;
+
+	/*
+	 * For now compare checksum, and header version. Later we can
+	 * verify more stuff on the header like interface type, etc
+	 */
+	if (hdr->version != LEGACY_HDR_VERSION) {
+		printf("ERROR: Bad HDR Version 0x%x != 0x%x\n",
+		       hdr->version, LEGACY_HDR_VERSION);
+		return -ENOEXEC;
+	}
+
+	/* The checksum value is discarded from checksum calculation */
+	hdr->checksum = 0;
+
+	checksum = do_checksum8((u8 *)hdr, header_len);
+	if (checksum != checksum_ref) {
+		printf("Error: Bad Image checksum. 0x%x != 0x%x\n",
+		       checksum, checksum_ref);
+		return -ENOEXEC;
+	}
+
+	/* Restore the checksum before writing */
+	hdr->checksum = checksum_ref;
+	printf("Image checksum...OK!\n");
+
+	return 0;
 }
 #endif
 
-- 
2.7.4

[U-Boot] [PATCH 4/5] cmd: mvebu: bubt: Add support for legacy Marvell SoCs

[Jon Nettleton]

On Wed, Aug 29, 2018 at 3:36 PM <kostap@marvell.com> wrote:
>
> From: Konstantin Porotchkin <kostap@marvell.com>
>
> Add support for image load and basic verification in bubt
> for legacy Marvell SoCs (A38x, A39x, ...)
>
> Signed-off-by: Konstantin Porotchkin <kostap@marvell.com>
> Cc: Igal Liberman <igall@marvell.com>
> Cc: Stefan Roese <sr@denx.de>
> ---
>  cmd/mvebu/bubt.c | 75 +++++++++++++++++++++++++++++++++++++++++++++++++++++---
>  1 file changed, 72 insertions(+), 3 deletions(-)
>
> diff --git a/cmd/mvebu/bubt.c b/cmd/mvebu/bubt.c
> index b4d371f..e10d079 100644
> --- a/cmd/mvebu/bubt.c
> +++ b/cmd/mvebu/bubt.c
> @@ -83,6 +83,33 @@ struct mvebu_image_info {
>         u32     encrypt_start_offset;
>         u32     encrypt_size;
>  };
> +
> +#else /* Older Armada SoCs - A38x, A39x, ... */
> +
> +#define        LEGACY_HDR_VERSION      1
> +
> +struct legacy_image_header {
> +/*     type    name                    byte order */
> +       u8      block_id;               /*   0   */
> +       u8      flags;                  /*   1   */
> +       u16     nand_pge_size;          /*  2-3  */
> +       u32     block_size;             /*  4-7  */
> +       u8      version;                /*   8   */
> +       u8      hdr_size_msb;           /*   9   */
> +       u16     hdr_size_lsb;           /* 10-11 */
> +       u32     source_addr;            /* 12-15 */
> +       u32     destination_addr;       /* 16-19 */
> +       u32     execution_addr;         /* 20-23 */
> +       u8      options;                /*  24   */
> +       u8      nand_block_size;        /*  25   */
> +       u8      nand_technology;        /*  26   */
> +       u8      rsvd4;                  /*  27   */
> +       u16     rsvd2;                  /* 28-29 */
> +       u8      ext;                    /*  30   */
> +       u8      checksum;               /*  31   */
> +
> +};
> +
>  #endif /* CONFIG_ARMADA_XXX */
>
>  struct bubt_dev {
> @@ -618,11 +645,53 @@ static int check_image_header(void)
>         return 0;
>  }
>
> -#else /* Not ARMADA? */
> +#else /* Legacy SoCs */
> +u8 do_checksum8(u8 *start, u32 len)
> +{
> +       u8 sum = 0;
> +       u8 *startp = start;
> +
> +       do {
> +               sum += *startp;
> +               startp++;
> +       } while (--len);
> +
> +       return sum;
> +}
> +
>  static int check_image_header(void)
>  {
> -       printf("bubt cmd does not support this SoC device or family!\n");
> -       return -ENOEXEC;
> +       struct legacy_image_header *hdr =
> +                       (struct legacy_image_header *)get_load_addr();
> +       u32 header_len = hdr->hdr_size_lsb + (hdr->hdr_size_msb << 16);
> +       u8 checksum;
> +       u8 checksum_ref = hdr->checksum;
> +
> +       /*
> +        * For now compare checksum, and header version. Later we can
> +        * verify more stuff on the header like interface type, etc
> +        */
> +       if (hdr->version != LEGACY_HDR_VERSION) {
> +               printf("ERROR: Bad HDR Version 0x%x != 0x%x\n",
> +                      hdr->version, LEGACY_HDR_VERSION);
> +               return -ENOEXEC;
> +       }
> +
> +       /* The checksum value is discarded from checksum calculation */
> +       hdr->checksum = 0;
> +
> +       checksum = do_checksum8((u8 *)hdr, header_len);
> +       if (checksum != checksum_ref) {
> +               printf("Error: Bad Image checksum. 0x%x != 0x%x\n",
> +                      checksum, checksum_ref);
> +               return -ENOEXEC;
> +       }
> +
> +       /* Restore the checksum before writing */
> +       hdr->checksum = checksum_ref;
> +       printf("Image checksum...OK!\n");
> +
> +       return 0;
>  }
>  #endif
>

Konstantin,

I already have a more complete integration for the legacy SOCs into
the bubt command.  It incorporates much of the same code that is used
to generate the images in kwimage.c.  We haven't submitted it to
mainline yet as we were waiting for a few more patchsets to land.

The features I have incorporated are supporting all possible boot
devices, image flashing from USB devices, and most importantly image
verification before flashing, so you can't brick your device by
flashing an image for eMMC onto the SPI device.

If you could take a look and comment I would be happy to work with you
to get this support into mainline.

Thanks,
Jon

[U-Boot] [EXT] Re: [PATCH 4/5] cmd: mvebu: bubt: Add support for legacy Marvell SoCs

[Kostya Porotchkin]

Hi, Jon,

> -----Original Message-----
> From: Jon Nettleton <jon@solid-run.com>
> Sent: Wednesday, August 29, 2018 17:28
> To: Kostya Porotchkin <kostap@marvell.com>
> Cc: U-Boot Mailing List <u-boot@lists.denx.de>; Stefan Roese <sr@denx.de>
> Subject: [EXT] Re: [U-Boot] [PATCH 4/5] cmd: mvebu: bubt: Add support for
> legacy Marvell SoCs
> 
> External Email
> 
> ----------------------------------------------------------------------
> On Wed, Aug 29, 2018 at 3:36 PM <kostap@marvell.com> wrote:
> >
> > From: Konstantin Porotchkin <kostap@marvell.com>
> >
> > Add support for image load and basic verification in bubt for legacy
> > Marvell SoCs (A38x, A39x, ...)
> >
> > Signed-off-by: Konstantin Porotchkin <kostap@marvell.com>
> > Cc: Igal Liberman <igall@marvell.com>
> > Cc: Stefan Roese <sr@denx.de>
> > ---
> >  cmd/mvebu/bubt.c | 75
> > +++++++++++++++++++++++++++++++++++++++++++++++++++++---
> >  1 file changed, 72 insertions(+), 3 deletions(-)
> >
> > diff --git a/cmd/mvebu/bubt.c b/cmd/mvebu/bubt.c index
> > b4d371f..e10d079 100644
> > --- a/cmd/mvebu/bubt.c
> > +++ b/cmd/mvebu/bubt.c
> > @@ -83,6 +83,33 @@ struct mvebu_image_info {
> >         u32     encrypt_start_offset;
> >         u32     encrypt_size;
> >  };
> > +
> > +#else /* Older Armada SoCs - A38x, A39x, ... */
> > +
> > +#define        LEGACY_HDR_VERSION      1
> > +
> > +struct legacy_image_header {
> > +/*     type    name                    byte order */
> > +       u8      block_id;               /*   0   */
> > +       u8      flags;                  /*   1   */
> > +       u16     nand_pge_size;          /*  2-3  */
> > +       u32     block_size;             /*  4-7  */
> > +       u8      version;                /*   8   */
> > +       u8      hdr_size_msb;           /*   9   */
> > +       u16     hdr_size_lsb;           /* 10-11 */
> > +       u32     source_addr;            /* 12-15 */
> > +       u32     destination_addr;       /* 16-19 */
> > +       u32     execution_addr;         /* 20-23 */
> > +       u8      options;                /*  24   */
> > +       u8      nand_block_size;        /*  25   */
> > +       u8      nand_technology;        /*  26   */
> > +       u8      rsvd4;                  /*  27   */
> > +       u16     rsvd2;                  /* 28-29 */
> > +       u8      ext;                    /*  30   */
> > +       u8      checksum;               /*  31   */
> > +
> > +};
> > +
> >  #endif /* CONFIG_ARMADA_XXX */
> >
> >  struct bubt_dev {
> > @@ -618,11 +645,53 @@ static int check_image_header(void)
> >         return 0;
> >  }
> >
> > -#else /* Not ARMADA? */
> > +#else /* Legacy SoCs */
> > +u8 do_checksum8(u8 *start, u32 len)
> > +{
> > +       u8 sum = 0;
> > +       u8 *startp = start;
> > +
> > +       do {
> > +               sum += *startp;
> > +               startp++;
> > +       } while (--len);
> > +
> > +       return sum;
> > +}
> > +
> >  static int check_image_header(void)
> >  {
> > -       printf("bubt cmd does not support this SoC device or family!\n");
> > -       return -ENOEXEC;
> > +       struct legacy_image_header *hdr =
> > +                       (struct legacy_image_header *)get_load_addr();
> > +       u32 header_len = hdr->hdr_size_lsb + (hdr->hdr_size_msb << 16);
> > +       u8 checksum;
> > +       u8 checksum_ref = hdr->checksum;
> > +
> > +       /*
> > +        * For now compare checksum, and header version. Later we can
> > +        * verify more stuff on the header like interface type, etc
> > +        */
> > +       if (hdr->version != LEGACY_HDR_VERSION) {
> > +               printf("ERROR: Bad HDR Version 0x%x != 0x%x\n",
> > +                      hdr->version, LEGACY_HDR_VERSION);
> > +               return -ENOEXEC;
> > +       }
> > +
> > +       /* The checksum value is discarded from checksum calculation */
> > +       hdr->checksum = 0;
> > +
> > +       checksum = do_checksum8((u8 *)hdr, header_len);
> > +       if (checksum != checksum_ref) {
> > +               printf("Error: Bad Image checksum. 0x%x != 0x%x\n",
> > +                      checksum, checksum_ref);
> > +               return -ENOEXEC;
> > +       }
> > +
> > +       /* Restore the checksum before writing */
> > +       hdr->checksum = checksum_ref;
> > +       printf("Image checksum...OK!\n");
> > +
> > +       return 0;
> >  }
> >  #endif
> >
> 
> Konstantin,
> 
> I already have a more complete integration for the legacy SOCs into the bubt
> command.  It incorporates much of the same code that is used to generate the
> images in kwimage.c.  We haven't submitted it to mainline yet as we were
> waiting for a few more patchsets to land.
[KP] No problem, we will ask Stefan to keep this patch aside and take yours.
I made this change for handling my own tests - it was just handy to use single command for image burning.
> 
> The features I have incorporated are supporting all possible boot devices, image
> flashing from USB devices, and most importantly image verification before
> flashing, so you can't brick your device by flashing an image for eMMC onto the
> SPI device.
> 
> If you could take a look and comment I would be happy to work with you to get
> this support into mainline.
[KP] Sure, no problem. The main reason for this patch series is a fix to broken A38x-GP platform.
If you have better "bubt", let's use it!
> 
> Thanks,
> Jon

[U-Boot] [PATCH 4/5] cmd: mvebu: bubt: Add support for legacy Marvell SoCs

[Stefan Roese]

On 29.08.2018 15:34, kostap at marvell.com wrote:
> From: Konstantin Porotchkin <kostap@marvell.com>
> 
> Add support for image load and basic verification in bubt
> for legacy Marvell SoCs (A38x, A39x, ...)
> 
> Signed-off-by: Konstantin Porotchkin <kostap@marvell.com>
> Cc: Igal Liberman <igall@marvell.com>
> Cc: Stefan Roese <sr@denx.de>
> ---
>   cmd/mvebu/bubt.c | 75 +++++++++++++++++++++++++++++++++++++++++++++++++++++---
>   1 file changed, 72 insertions(+), 3 deletions(-)
> 
> diff --git a/cmd/mvebu/bubt.c b/cmd/mvebu/bubt.c
> index b4d371f..e10d079 100644
> --- a/cmd/mvebu/bubt.c
> +++ b/cmd/mvebu/bubt.c
> @@ -83,6 +83,33 @@ struct mvebu_image_info {
>   	u32	encrypt_start_offset;
>   	u32	encrypt_size;
>   };
> +
> +#else /* Older Armada SoCs - A38x, A39x, ... */
> +
> +#define	LEGACY_HDR_VERSION	1
> +
> +struct legacy_image_header {
> +/*	type	name			byte order */
> +	u8	block_id;		/*   0   */
> +	u8	flags;			/*   1   */
> +	u16	nand_pge_size;		/*  2-3  */
> +	u32	block_size;		/*  4-7  */
> +	u8	version;		/*   8   */
> +	u8	hdr_size_msb;		/*   9   */
> +	u16	hdr_size_lsb;		/* 10-11 */
> +	u32	source_addr;		/* 12-15 */
> +	u32	destination_addr;	/* 16-19 */
> +	u32	execution_addr;		/* 20-23 */
> +	u8	options;		/*  24   */
> +	u8	nand_block_size;	/*  25   */
> +	u8	nand_technology;	/*  26   */
> +	u8	rsvd4;			/*  27   */
> +	u16	rsvd2;			/* 28-29 */
> +	u8	ext;			/*  30   */
> +	u8	checksum;		/*  31   */
> +
> +};
> +
>   #endif /* CONFIG_ARMADA_XXX */
>   
>   struct bubt_dev {
> @@ -618,11 +645,53 @@ static int check_image_header(void)
>   	return 0;
>   }
>   
> -#else /* Not ARMADA? */
> +#else /* Legacy SoCs */
> +u8 do_checksum8(u8 *start, u32 len)
> +{
> +	u8 sum = 0;
> +	u8 *startp = start;
> +
> +	do {
> +		sum += *startp;
> +		startp++;
> +	} while (--len);
> +
> +	return sum;
> +}
> +
>   static int check_image_header(void)
>   {
> -	printf("bubt cmd does not support this SoC device or family!\n");
> -	return -ENOEXEC;
> +	struct legacy_image_header *hdr =
> +			(struct legacy_image_header *)get_load_addr();
> +	u32 header_len = hdr->hdr_size_lsb + (hdr->hdr_size_msb << 16);
> +	u8 checksum;
> +	u8 checksum_ref = hdr->checksum;
> +
> +	/*
> +	 * For now compare checksum, and header version. Later we can
> +	 * verify more stuff on the header like interface type, etc
> +	 */
> +	if (hdr->version != LEGACY_HDR_VERSION) {
> +		printf("ERROR: Bad HDR Version 0x%x != 0x%x\n",
> +		       hdr->version, LEGACY_HDR_VERSION);
> +		return -ENOEXEC;
> +	}
> +
> +	/* The checksum value is discarded from checksum calculation */
> +	hdr->checksum = 0;
> +
> +	checksum = do_checksum8((u8 *)hdr, header_len);
> +	if (checksum != checksum_ref) {
> +		printf("Error: Bad Image checksum. 0x%x != 0x%x\n",
> +		       checksum, checksum_ref);
> +		return -ENOEXEC;
> +	}
> +
> +	/* Restore the checksum before writing */
> +	hdr->checksum = checksum_ref;
> +	printf("Image checksum...OK!\n");
> +
> +	return 0;
>   }
>   #endif
>   
> 

Dropped for now, as a result of your discussion in this thread.

Thanks,
Stefan

[U-Boot] [PATCH 5/5] defconfig: db-88f6820-gp: Add bubt command to the build

[kostap at marvell.com]

From: Konstantin Porotchkin <kostap@marvell.com>

Add Marvell bubt command to db-88f6820-gp defconfig

Signed-off-by: Konstantin Porotchkin <kostap@marvell.com>
Cc: Igal Liberman <igall@marvell.com>
Cc: Stefan Roese <sr@denx.de>
---
 configs/db-88f6820-gp_defconfig | 1 +
 1 file changed, 1 insertion(+)

diff --git a/configs/db-88f6820-gp_defconfig b/configs/db-88f6820-gp_defconfig
index 131262d..4a8fa2e 100644
--- a/configs/db-88f6820-gp_defconfig
+++ b/configs/db-88f6820-gp_defconfig
@@ -39,6 +39,7 @@ CONFIG_CMD_EXT2=y
 CONFIG_CMD_EXT4=y
 CONFIG_CMD_FAT=y
 CONFIG_CMD_FS_GENERIC=y
+CONFIG_CMD_MVEBU_BUBT=y
 CONFIG_EFI_PARTITION=y
 # CONFIG_PARTITION_UUIDS is not set
 # CONFIG_SPL_PARTITION_UUIDS is not set
-- 
2.7.4

[U-Boot] [PATCH 5/5] defconfig: db-88f6820-gp: Add bubt command to the build

[Stefan Roese]

On 29.08.2018 15:34, kostap at marvell.com wrote:
> From: Konstantin Porotchkin <kostap@marvell.com>
> 
> Add Marvell bubt command to db-88f6820-gp defconfig
> 
> Signed-off-by: Konstantin Porotchkin <kostap@marvell.com>
> Cc: Igal Liberman <igall@marvell.com>
> Cc: Stefan Roese <sr@denx.de>
> ---
>   configs/db-88f6820-gp_defconfig | 1 +
>   1 file changed, 1 insertion(+)
> 
> diff --git a/configs/db-88f6820-gp_defconfig b/configs/db-88f6820-gp_defconfig
> index 131262d..4a8fa2e 100644
> --- a/configs/db-88f6820-gp_defconfig
> +++ b/configs/db-88f6820-gp_defconfig
> @@ -39,6 +39,7 @@ CONFIG_CMD_EXT2=y
>   CONFIG_CMD_EXT4=y
>   CONFIG_CMD_FAT=y
>   CONFIG_CMD_FS_GENERIC=y
> +CONFIG_CMD_MVEBU_BUBT=y
>   CONFIG_EFI_PARTITION=y
>   # CONFIG_PARTITION_UUIDS is not set
>   # CONFIG_SPL_PARTITION_UUIDS is not set
> 

Dropped for now, as a result of your discussion in this thread.

Thanks,
Stefan