Re: [PATCH v3 5/5] cmd: mvebu: bubt: show image boot device

2020-03-24 Thread Joel Johnson 

On 2020-03-24 01:28, Stefan Roese wrote:

On 23.03.20 18:43, Joel Johnson wrote:

When a mismatch is found trying to write an image for one boot method
to a different boot device, print an error message including the image
header marked target boot device type.

Signed-off-by: Joel Johnson 
Reviewed-by: Stefan Roese 

---

v2 changes:
   - newly added in v2 series
v3 changes:
   - none

---
  cmd/mvebu/bubt.c | 15 ---
  1 file changed, 12 insertions(+), 3 deletions(-)

diff --git a/cmd/mvebu/bubt.c b/cmd/mvebu/bubt.c
index fbcad37c40..f992507041 100644
--- a/cmd/mvebu/bubt.c
+++ b/cmd/mvebu/bubt.c
@@ -676,6 +676,17 @@ static int a38x_check_boot_mode(const struct 
bubt_dev *dst)

if (a38x_boot_modes[mode].id == hdr->blockid)
return 0;
  + for (int i = 0; i < A38X_BOOT_MODE_MAX; i++) {


This will fail, as A38X_BOOT_MODE_MAX is not defined any more. Please
use ARRAY_SIZE() here as well.

Thanks,
Stefan


Yeah, I caught that but missed committing it for this series - I'd sent 
out a v4 with this fixed. A v5 with your other ifdef change will include 
it.


Joel

Re: [PATCH v3 5/5] cmd: mvebu: bubt: show image boot device

2020-03-24 Thread Stefan Roese 

On 23.03.20 18:43, Joel Johnson wrote:

When a mismatch is found trying to write an image for one boot method
to a different boot device, print an error message including the image
header marked target boot device type.

Signed-off-by: Joel Johnson 
Reviewed-by: Stefan Roese 

---

v2 changes:
   - newly added in v2 series
v3 changes:
   - none

---
  cmd/mvebu/bubt.c | 15 ---
  1 file changed, 12 insertions(+), 3 deletions(-)

diff --git a/cmd/mvebu/bubt.c b/cmd/mvebu/bubt.c
index fbcad37c40..f992507041 100644
--- a/cmd/mvebu/bubt.c
+++ b/cmd/mvebu/bubt.c
@@ -676,6 +676,17 @@ static int a38x_check_boot_mode(const struct bubt_dev *dst)
if (a38x_boot_modes[mode].id == hdr->blockid)
return 0;
  
+	for (int i = 0; i < A38X_BOOT_MODE_MAX; i++) {


This will fail, as A38X_BOOT_MODE_MAX is not defined any more. Please
use ARRAY_SIZE() here as well.

Thanks,
Stefan


+   if (a38x_boot_modes[i].id == hdr->blockid) {
+   printf("Error: image meant to be booted from "
+  " \"%s\", not \"%s\"!\n",
+  a38x_boot_modes[i].name, dst->name);
+   return -ENOEXEC;
+   }
+   }
+
+   printf("Error: unknown boot device in image header: 0x%x\n",
+  hdr->blockid);
return -ENOEXEC;
  }
  
@@ -745,10 +756,8 @@ static int bubt_verify(const struct bubt_dev *dst)
  
  #if defined(CONFIG_ARMADA_38X)

err = a38x_check_boot_mode(dst);
-   if (err) {
-   puts("Error: image not built for destination device!\n");
+   if (err)
return err;
-   }
  #endif
  
  	return 0;





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: s...@denx.de

Re: [PATCH v3 5/5] cmd: mvebu: bubt: show image boot device

2020-03-23 Thread Joel Johnson 

On 2020-03-23 11:43, Joel Johnson wrote:

When a mismatch is found trying to write an image for one boot method
to a different boot device, print an error message including the image
header marked target boot device type.

Signed-off-by: Joel Johnson 
Reviewed-by: Stefan Roese 

---

v2 changes:
  - newly added in v2 series
v3 changes:
  - none

---
 cmd/mvebu/bubt.c | 15 ---
 1 file changed, 12 insertions(+), 3 deletions(-)

diff --git a/cmd/mvebu/bubt.c b/cmd/mvebu/bubt.c
index fbcad37c40..f992507041 100644
--- a/cmd/mvebu/bubt.c
+++ b/cmd/mvebu/bubt.c
@@ -676,6 +676,17 @@ static int a38x_check_boot_mode(const struct 
bubt_dev *dst)

if (a38x_boot_modes[mode].id == hdr->blockid)
return 0;

+   for (int i = 0; i < A38X_BOOT_MODE_MAX; i++) {
+   if (a38x_boot_modes[i].id == hdr->blockid) {
+   printf("Error: image meant to be booted from "
+  " \"%s\", not \"%s\"!\n",
+  a38x_boot_modes[i].name, dst->name);
+   return -ENOEXEC;
+   }
+   }
+
+   printf("Error: unknown boot device in image header: 0x%x\n",
+  hdr->blockid);
return -ENOEXEC;
 }

@@ -745,10 +756,8 @@ static int bubt_verify(const struct bubt_dev *dst)

 #if defined(CONFIG_ARMADA_38X)
err = a38x_check_boot_mode(dst);
-   if (err) {
-   puts("Error: image not built for destination device!\n");
+   if (err)
return err;
-   }
 #endif

return 0;


I sent this one prematurely before complete build testing, I'll followup 
with further testing with a change to use ARRAY_SIZE here as well.


Joel