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

[Sam Protsenko]

On Thu, Jul 25, 2019 at 7:28 PM Sam Protsenko
<semen.protsenko at linaro.org> wrote:
>
> On Thu, Jul 25, 2019 at 6:03 PM Andrew F. Davis <afd at 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 at 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 at 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();
> > >>>
> > >>>