* [U-Boot] [PATCH] board: ti: am57xx: Correct the fastboot product var
@ 2019-07-25 13:22 Sam Protsenko
2019-07-25 14:05 ` Andrew F. Davis
0 siblings, 1 reply; 6+ messages in thread
From: Sam Protsenko @ 2019-07-25 13:22 UTC (permalink / raw)
To: u-boot
"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();
--
2.20.1
^ permalink raw reply related [flat|nested] 6+ messages in thread
* [U-Boot] [PATCH] board: ti: am57xx: Correct the fastboot product var
2019-07-25 13:22 [U-Boot] [PATCH] board: ti: am57xx: Correct the fastboot product var Sam Protsenko
@ 2019-07-25 14:05 ` Andrew F. Davis
2019-07-25 14:43 ` Sam Protsenko
0 siblings, 1 reply; 6+ messages in thread
From: Andrew F. Davis @ 2019-07-25 14:05 UTC (permalink / raw)
To: u-boot
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();
>
>
^ permalink raw reply [flat|nested] 6+ messages in thread
* [U-Boot] [PATCH] board: ti: am57xx: Correct the fastboot product var
2019-07-25 14:05 ` Andrew F. Davis
@ 2019-07-25 14:43 ` Sam Protsenko
2019-07-25 15:03 ` Andrew F. Davis
0 siblings, 1 reply; 6+ messages in thread
From: Sam Protsenko @ 2019-07-25 14:43 UTC (permalink / raw)
To: u-boot
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();
> >
> >
^ permalink raw reply [flat|nested] 6+ messages in thread
* [U-Boot] [PATCH] board: ti: am57xx: Correct the fastboot product var
2019-07-25 14:43 ` Sam Protsenko
@ 2019-07-25 15:03 ` Andrew F. Davis
2019-07-25 16:28 ` Sam Protsenko
0 siblings, 1 reply; 6+ messages in thread
From: Andrew F. Davis @ 2019-07-25 15:03 UTC (permalink / raw)
To: u-boot
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();
>>>
>>>
^ permalink raw reply [flat|nested] 6+ messages in thread
* [U-Boot] [PATCH] board: ti: am57xx: Correct the fastboot product var
2019-07-25 15:03 ` Andrew F. Davis
@ 2019-07-25 16:28 ` Sam Protsenko
2019-07-25 17:13 ` Sam Protsenko
0 siblings, 1 reply; 6+ messages in thread
From: Sam Protsenko @ 2019-07-25 16:28 UTC (permalink / raw)
To: u-boot
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();
> >>>
> >>>
^ permalink raw reply [flat|nested] 6+ messages in thread
* [U-Boot] [PATCH] board: ti: am57xx: Correct the fastboot product var
2019-07-25 16:28 ` Sam Protsenko
@ 2019-07-25 17:13 ` Sam Protsenko
0 siblings, 0 replies; 6+ messages in thread
From: Sam Protsenko @ 2019-07-25 17:13 UTC (permalink / raw)
To: u-boot
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();
> > >>>
> > >>>
^ permalink raw reply [flat|nested] 6+ messages in thread
end of thread, other threads:[~2019-07-25 17:13 UTC | newest]
Thread overview: 6+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2019-07-25 13:22 [U-Boot] [PATCH] board: ti: am57xx: Correct the fastboot product var Sam Protsenko
2019-07-25 14:05 ` Andrew F. Davis
2019-07-25 14:43 ` Sam Protsenko
2019-07-25 15:03 ` Andrew F. Davis
2019-07-25 16:28 ` Sam Protsenko
2019-07-25 17:13 ` Sam Protsenko
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.