| Message ID | 20200417153808.2468508-3-mrjoel@lixil.net |
|---|---|
| State | Accepted |
| Commit | 658854a626b56d3ff417453ac842e4486ba5139b |
| Delegated to: | Stefan Roese |
| Headers | show |
| Series | [v6,1/5] cmd: mvebu: bubt: add A38x support | expand |
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@lixil.net> Reviewed-by: Stefan Roese <sr@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
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;
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@lixil.net> --- 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(-)