[U-Boot] [PATCH] board: ti: am57xx: Correct the fastboot product var

"fastboot flashall" expects "fastboot getvar product" to be "beagle_x15board". Instead, "am57xx" is returned, as it's set in $board env var from SYS_BOARD in board/ti/am57xx/Kconfig file.
Override fastboot product variable and set it to correct value, to fix "fastboot flashall".
Signed-off-by: Sam Protsenko semen.protsenko@linaro.org --- board/ti/am57xx/board.c | 6 ++++++ 1 file changed, 6 insertions(+)
diff --git a/board/ti/am57xx/board.c b/board/ti/am57xx/board.c index 1a903f13a6..c8eac4edde 100644 --- a/board/ti/am57xx/board.c +++ b/board/ti/am57xx/board.c @@ -685,6 +685,11 @@ static int device_okay(const char *path) } #endif
+static void am57x_set_fastboot_vars(void) +{ + env_set("fastboot.product", "beagle_x15board"); +} + int board_late_init(void) { setup_board_eeprom_env(); @@ -717,6 +722,7 @@ int board_late_init(void)
omap_die_id_serial(); omap_set_fastboot_vars(); + am57x_set_fastboot_vars();
am57x_idk_lcd_detect();

On 7/25/19 9:22 AM, Sam Protsenko wrote:
"fastboot flashall" expects "fastboot getvar product" to be "beagle_x15board". Instead, "am57xx" is returned, as it's set in $board env var from SYS_BOARD in board/ti/am57xx/Kconfig file.
Override fastboot product variable and set it to correct value, to fix "fastboot flashall".
Signed-off-by: Sam Protsenko semen.protsenko@linaro.org
board/ti/am57xx/board.c | 6 ++++++ 1 file changed, 6 insertions(+)
diff --git a/board/ti/am57xx/board.c b/board/ti/am57xx/board.c index 1a903f13a6..c8eac4edde 100644 --- a/board/ti/am57xx/board.c +++ b/board/ti/am57xx/board.c @@ -685,6 +685,11 @@ static int device_okay(const char *path) } #endif
+static void am57x_set_fastboot_vars(void) +{
- env_set("fastboot.product", "beagle_x15board");
This doesn't seem right.. This is common source for all AM57x based boards, the only thing we can return here is "am57xx". Either fastboot needs some sort of conversion on its side, or we set the exact board name the same way we do device-tree name detection.
Andrew
+}
int board_late_init(void) { setup_board_eeprom_env(); @@ -717,6 +722,7 @@ int board_late_init(void)
omap_die_id_serial(); omap_set_fastboot_vars();
am57x_set_fastboot_vars();
am57x_idk_lcd_detect();

On Thu, Jul 25, 2019 at 5:05 PM Andrew F. Davis afd@ti.com wrote:
On 7/25/19 9:22 AM, Sam Protsenko wrote:
"fastboot flashall" expects "fastboot getvar product" to be "beagle_x15board". Instead, "am57xx" is returned, as it's set in $board env var from SYS_BOARD in board/ti/am57xx/Kconfig file.
Override fastboot product variable and set it to correct value, to fix "fastboot flashall".
Signed-off-by: Sam Protsenko semen.protsenko@linaro.org
board/ti/am57xx/board.c | 6 ++++++ 1 file changed, 6 insertions(+)
diff --git a/board/ti/am57xx/board.c b/board/ti/am57xx/board.c index 1a903f13a6..c8eac4edde 100644 --- a/board/ti/am57xx/board.c +++ b/board/ti/am57xx/board.c @@ -685,6 +685,11 @@ static int device_okay(const char *path) } #endif
+static void am57x_set_fastboot_vars(void) +{
env_set("fastboot.product", "beagle_x15board");
This doesn't seem right.. This is common source for all AM57x based boards, the only thing we can return here is "am57xx". Either fastboot needs some sort of conversion on its side, or we set the exact board name the same way we do device-tree name detection.
The thing is, we have only beagle_x15 target in AOSP right now, which we use for all AM57xx based boards (as I understand). So "fastboot flashall" expects "getvar product" to be exactly that, otherwise it fails. We can check board_is_x15() to do what you're suggesting to do, but in that case we won't be able to use "fastboot flashall" e.g. for AM57xx EVM. How do you suggest to fix that case if we don't return "beagle_x15board"?
Anyway, do you know any other usages of "getvar product" than this use-case? As fastboot is Android protocol, I think we should make it work properly with Android first, and all custom down-stream usages should respect that.
Andrew
+}
int board_late_init(void) { setup_board_eeprom_env(); @@ -717,6 +722,7 @@ int board_late_init(void)
omap_die_id_serial(); omap_set_fastboot_vars();
am57x_set_fastboot_vars(); am57x_idk_lcd_detect();

On 7/25/19 10:43 AM, Sam Protsenko wrote:
On Thu, Jul 25, 2019 at 5:05 PM Andrew F. Davis afd@ti.com wrote:
On 7/25/19 9:22 AM, Sam Protsenko wrote:
"fastboot flashall" expects "fastboot getvar product" to be "beagle_x15board". Instead, "am57xx" is returned, as it's set in $board env var from SYS_BOARD in board/ti/am57xx/Kconfig file.
Override fastboot product variable and set it to correct value, to fix "fastboot flashall".
Signed-off-by: Sam Protsenko semen.protsenko@linaro.org
board/ti/am57xx/board.c | 6 ++++++ 1 file changed, 6 insertions(+)
diff --git a/board/ti/am57xx/board.c b/board/ti/am57xx/board.c index 1a903f13a6..c8eac4edde 100644 --- a/board/ti/am57xx/board.c +++ b/board/ti/am57xx/board.c @@ -685,6 +685,11 @@ static int device_okay(const char *path) } #endif
+static void am57x_set_fastboot_vars(void) +{
env_set("fastboot.product", "beagle_x15board");
This doesn't seem right.. This is common source for all AM57x based boards, the only thing we can return here is "am57xx". Either fastboot needs some sort of conversion on its side, or we set the exact board name the same way we do device-tree name detection.
The thing is, we have only beagle_x15 target in AOSP right now, which we use for all AM57xx based boards (as I understand). So "fastboot flashall" expects "getvar product" to be exactly that, otherwise it fails. We can check board_is_x15() to do what you're suggesting to do, but in that case we won't be able to use "fastboot flashall" e.g. for AM57xx EVM. How do you suggest to fix that case if we don't return "beagle_x15board"?
If we are a AM57x-EVM board then that is what U-Boot should return, if we use "beagle_x15" as the name of the device for our Android product on both AM57x-EVM and Beagle x15 then we need some conversion in our Android fastboot to handle that.
Anyway, do you know any other usages of "getvar product" than this use-case? As fastboot is Android protocol, I think we should make it work properly with Android first, and all custom down-stream usages should respect that.
BeagleBoard x15 is not the only AM57x based Android product, at least I hope not. :) If we want to re-use one Android build for both boards we should move that logic to our Android build, not hack that choice here in U-boot. For all we know we may split those out and have a different builds for each Beagle and EVM, then we would need to revert this.
Andrew
Andrew
+}
int board_late_init(void) { setup_board_eeprom_env(); @@ -717,6 +722,7 @@ int board_late_init(void)
omap_die_id_serial(); omap_set_fastboot_vars();
am57x_set_fastboot_vars(); am57x_idk_lcd_detect();

On Thu, Jul 25, 2019 at 6:03 PM Andrew F. Davis afd@ti.com wrote:
On 7/25/19 10:43 AM, Sam Protsenko wrote:
On Thu, Jul 25, 2019 at 5:05 PM Andrew F. Davis afd@ti.com wrote:
On 7/25/19 9:22 AM, Sam Protsenko wrote:
"fastboot flashall" expects "fastboot getvar product" to be "beagle_x15board". Instead, "am57xx" is returned, as it's set in $board env var from SYS_BOARD in board/ti/am57xx/Kconfig file.
Override fastboot product variable and set it to correct value, to fix "fastboot flashall".
Signed-off-by: Sam Protsenko semen.protsenko@linaro.org
board/ti/am57xx/board.c | 6 ++++++ 1 file changed, 6 insertions(+)
diff --git a/board/ti/am57xx/board.c b/board/ti/am57xx/board.c index 1a903f13a6..c8eac4edde 100644 --- a/board/ti/am57xx/board.c +++ b/board/ti/am57xx/board.c @@ -685,6 +685,11 @@ static int device_okay(const char *path) } #endif
+static void am57x_set_fastboot_vars(void) +{
env_set("fastboot.product", "beagle_x15board");
This doesn't seem right.. This is common source for all AM57x based boards, the only thing we can return here is "am57xx". Either fastboot needs some sort of conversion on its side, or we set the exact board name the same way we do device-tree name detection.
The thing is, we have only beagle_x15 target in AOSP right now, which we use for all AM57xx based boards (as I understand). So "fastboot flashall" expects "getvar product" to be exactly that, otherwise it fails. We can check board_is_x15() to do what you're suggesting to do, but in that case we won't be able to use "fastboot flashall" e.g. for AM57xx EVM. How do you suggest to fix that case if we don't return "beagle_x15board"?
If we are a AM57x-EVM board then that is what U-Boot should return, if we use "beagle_x15" as the name of the device for our Android product on both AM57x-EVM and Beagle x15 then we need some conversion in our Android fastboot to handle that.
Seems like it's possible to do, by using TARGET_BOARD_INFO_FILE in beagle_x15 BoardConfig.mk, and providing something like:
require board=a|b|c
Also, on U-Boot side we still need to fill fastboot.product correctly, w.r.t. to each particular board (e.g. reuse $board_name for this).
Will check if it works fine and send v2 soon.
Anyway, do you know any other usages of "getvar product" than this use-case? As fastboot is Android protocol, I think we should make it work properly with Android first, and all custom down-stream usages should respect that.
BeagleBoard x15 is not the only AM57x based Android product, at least I hope not. :) If we want to re-use one Android build for both boards we should move that logic to our Android build, not hack that choice here in U-boot. For all we know we may split those out and have a different builds for each Beagle and EVM, then we would need to revert this.
Andrew
Andrew
+}
int board_late_init(void) { setup_board_eeprom_env(); @@ -717,6 +722,7 @@ int board_late_init(void)
omap_die_id_serial(); omap_set_fastboot_vars();
am57x_set_fastboot_vars(); am57x_idk_lcd_detect();

On Thu, Jul 25, 2019 at 7:28 PM Sam Protsenko semen.protsenko@linaro.org wrote:
On Thu, Jul 25, 2019 at 6:03 PM Andrew F. Davis afd@ti.com wrote:
On 7/25/19 10:43 AM, Sam Protsenko wrote:
On Thu, Jul 25, 2019 at 5:05 PM Andrew F. Davis afd@ti.com wrote:
On 7/25/19 9:22 AM, Sam Protsenko wrote:
"fastboot flashall" expects "fastboot getvar product" to be "beagle_x15board". Instead, "am57xx" is returned, as it's set in $board env var from SYS_BOARD in board/ti/am57xx/Kconfig file.
Override fastboot product variable and set it to correct value, to fix "fastboot flashall".
Signed-off-by: Sam Protsenko semen.protsenko@linaro.org
board/ti/am57xx/board.c | 6 ++++++ 1 file changed, 6 insertions(+)
diff --git a/board/ti/am57xx/board.c b/board/ti/am57xx/board.c index 1a903f13a6..c8eac4edde 100644 --- a/board/ti/am57xx/board.c +++ b/board/ti/am57xx/board.c @@ -685,6 +685,11 @@ static int device_okay(const char *path) } #endif
+static void am57x_set_fastboot_vars(void) +{
env_set("fastboot.product", "beagle_x15board");
This doesn't seem right.. This is common source for all AM57x based boards, the only thing we can return here is "am57xx". Either fastboot needs some sort of conversion on its side, or we set the exact board name the same way we do device-tree name detection.
The thing is, we have only beagle_x15 target in AOSP right now, which we use for all AM57xx based boards (as I understand). So "fastboot flashall" expects "getvar product" to be exactly that, otherwise it fails. We can check board_is_x15() to do what you're suggesting to do, but in that case we won't be able to use "fastboot flashall" e.g. for AM57xx EVM. How do you suggest to fix that case if we don't return "beagle_x15board"?
If we are a AM57x-EVM board then that is what U-Boot should return, if we use "beagle_x15" as the name of the device for our Android product on both AM57x-EVM and Beagle x15 then we need some conversion in our Android fastboot to handle that.
Seems like it's possible to do, by using TARGET_BOARD_INFO_FILE in beagle_x15 BoardConfig.mk, and providing something like:
require board=a|b|c
Also, on U-Boot side we still need to fill fastboot.product correctly, w.r.t. to each particular board (e.g. reuse $board_name for this).
Will check if it works fine and send v2 soon.
Superseded by v2: https://patchwork.ozlabs.org/patch/1137022/
Anyway, do you know any other usages of "getvar product" than this use-case? As fastboot is Android protocol, I think we should make it work properly with Android first, and all custom down-stream usages should respect that.
BeagleBoard x15 is not the only AM57x based Android product, at least I hope not. :) If we want to re-use one Android build for both boards we should move that logic to our Android build, not hack that choice here in U-boot. For all we know we may split those out and have a different builds for each Beagle and EVM, then we would need to revert this.
Andrew
Andrew
+}
int board_late_init(void) { setup_board_eeprom_env(); @@ -717,6 +722,7 @@ int board_late_init(void)
omap_die_id_serial(); omap_set_fastboot_vars();
am57x_set_fastboot_vars(); am57x_idk_lcd_detect();
participants (2)
-
Andrew F. Davis
-
Sam Protsenko