diff mbox series

[v2,3/5] cmd: mvebu: bubt: verify A38x target device type

Message ID 20200129145013.125237-3-mrjoel@lixil.net
State Superseded
Headers show
Series [v2,1/5] cmd: mvebu: bubt: add A38x support | expand

Commit Message

Joel Johnson Jan. 29, 2020, 2:50 p.m. UTC
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 branch.

Signed-off-by: Joel Johnson <mrjoel at lixil.net>

---

v2 changes:
  - none

---
 cmd/mvebu/bubt.c | 49 ++++++++++++++++++++++++++++++++++++++++++++++--
 1 file changed, 47 insertions(+), 2 deletions(-)

Comments

Stefan Roese March 23, 2020, 10:06 a.m. UTC | #1
On 29.01.20 15:50, 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 branch.
> 
> Signed-off-by: Joel Johnson <mrjoel at lixil.net>
> 
> ---
> 
> v2 changes:
>    - none
> 
> ---
>   cmd/mvebu/bubt.c | 49 ++++++++++++++++++++++++++++++++++++++++++++++--
>   1 file changed, 47 insertions(+), 2 deletions(-)
> 
> diff --git a/cmd/mvebu/bubt.c b/cmd/mvebu/bubt.c
> index b80b81c82a..78061a6a2d 100644
> --- a/cmd/mvebu/bubt.c
> +++ b/cmd/mvebu/bubt.c
> @@ -107,6 +107,26 @@ struct a38x_main_hdr_v1 {
>   	u8  ext;                   /* 0x1E      */
>   	u8  checksum;              /* 0x1F      */
>   };
> +
> +#define A38X_BOOT_MODE_MAX	8

Can't you drop this define here...

> +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[A38X_BOOT_MODE_MAX] = {

... and use [] here...

> +	{ 0x4D, "i2c"  },
> +	{ 0x5A, "spi"  },
> +	{ 0x69, "uart" },
> +	{ 0x78, "sata" },
> +	{ 0x8B, "nand" },
> +	{ 0x9C, "pex"  },
> +	{ 0xAE, "mmc"  },
> +	{},
> +};
> +
>   #endif
>   
>   struct bubt_dev {
> @@ -644,6 +664,23 @@ static int check_image_header(void)
>   	return 0;
>   }
>   #elif defined(CONFIG_ARMADA_38X)
> +static int a38x_check_boot_mode(const struct bubt_dev *dst)
> +{
> +	int mode;
> +	const struct a38x_main_hdr_v1 *hdr =
> +		(struct a38x_main_hdr_v1 *)get_load_addr();
> +
> +	for (mode = 0; mode < A38X_BOOT_MODE_MAX; mode++) {

... and use ARRAY_SIZE() here instead?

Thanks,
Stefan
Joel Johnson March 23, 2020, 3:35 p.m. UTC | #2
On 2020-03-23 04:06, Stefan Roese wrote:
> On 29.01.20 15:50, 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 branch.
>> 
>> Signed-off-by: Joel Johnson <mrjoel at lixil.net>
>> 
>> ---
>> 
>> v2 changes:
>>    - none
>> 
>> ---
>>   cmd/mvebu/bubt.c | 49 
>> ++++++++++++++++++++++++++++++++++++++++++++++--
>>   1 file changed, 47 insertions(+), 2 deletions(-)
>> 
>> diff --git a/cmd/mvebu/bubt.c b/cmd/mvebu/bubt.c
>> index b80b81c82a..78061a6a2d 100644
>> --- a/cmd/mvebu/bubt.c
>> +++ b/cmd/mvebu/bubt.c
>> @@ -107,6 +107,26 @@ struct a38x_main_hdr_v1 {
>>   	u8  ext;                   /* 0x1E      */
>>   	u8  checksum;              /* 0x1F      */
>>   };
>> +
>> +#define A38X_BOOT_MODE_MAX	8
> 
> Can't you drop this define here...
> 
>> +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[A38X_BOOT_MODE_MAX] = {
> 
> ... and use [] here...
> 
>> +	{ 0x4D, "i2c"  },
>> +	{ 0x5A, "spi"  },
>> +	{ 0x69, "uart" },
>> +	{ 0x78, "sata" },
>> +	{ 0x8B, "nand" },
>> +	{ 0x9C, "pex"  },
>> +	{ 0xAE, "mmc"  },
>> +	{},
>> +};
>> +
>>   #endif
>>     struct bubt_dev {
>> @@ -644,6 +664,23 @@ static int check_image_header(void)
>>   	return 0;
>>   }
>>   #elif defined(CONFIG_ARMADA_38X)
>> +static int a38x_check_boot_mode(const struct bubt_dev *dst)
>> +{
>> +	int mode;
>> +	const struct a38x_main_hdr_v1 *hdr =
>> +		(struct a38x_main_hdr_v1 *)get_load_addr();
>> +
>> +	for (mode = 0; mode < A38X_BOOT_MODE_MAX; mode++) {
> 
> ... and use ARRAY_SIZE() here instead?
> 
> Thanks,
> Stefan

Yes, that'd be better for maintenance for sure, I'll incorporate the 
change.

Joel
diff mbox series

Patch

diff --git a/cmd/mvebu/bubt.c b/cmd/mvebu/bubt.c
index b80b81c82a..78061a6a2d 100644
--- a/cmd/mvebu/bubt.c
+++ b/cmd/mvebu/bubt.c
@@ -107,6 +107,26 @@  struct a38x_main_hdr_v1 {
 	u8  ext;                   /* 0x1E      */
 	u8  checksum;              /* 0x1F      */
 };
+
+#define A38X_BOOT_MODE_MAX	8
+
+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[A38X_BOOT_MODE_MAX] = {
+	{ 0x4D, "i2c"  },
+	{ 0x5A, "spi"  },
+	{ 0x69, "uart" },
+	{ 0x78, "sata" },
+	{ 0x8B, "nand" },
+	{ 0x9C, "pex"  },
+	{ 0xAE, "mmc"  },
+	{},
+};
+
 #endif
 
 struct bubt_dev {
@@ -644,6 +664,23 @@  static int check_image_header(void)
 	return 0;
 }
 #elif defined(CONFIG_ARMADA_38X)
+static int a38x_check_boot_mode(const struct bubt_dev *dst)
+{
+	int mode;
+	const struct a38x_main_hdr_v1 *hdr =
+		(struct a38x_main_hdr_v1 *)get_load_addr();
+
+	for (mode = 0; mode < A38X_BOOT_MODE_MAX; mode++) {
+		if (strcmp(a38x_boot_modes[mode].name, dst->name) == 0)
+			break;
+	}
+
+	if (a38x_boot_modes[mode].id == hdr->blockid)
+		return 0;
+
+	return -ENOEXEC;
+}
+
 static size_t a38x_header_size(const struct a38x_main_hdr_v1 *h)
 {
 	if (h->version == 1)
@@ -697,7 +734,7 @@  static int check_image_header(void)
 }
 #endif
 
-static int bubt_verify(size_t image_size)
+static int bubt_verify(const struct bubt_dev *dst)
 {
 	int err;
 
@@ -708,6 +745,14 @@  static int bubt_verify(size_t image_size)
 		return err;
 	}
 
+#if defined(CONFIG_ARMADA_38X)
+	err = a38x_check_boot_mode(dst);
+	if (err) {
+		puts("Error: image not built for destination device!\n");
+		return err;
+	}
+#endif
+
 	return 0;
 }
 
@@ -829,7 +874,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;