[PATCH v2 0/3] Support usb booting on i.MX8MP

[Lucas Stach]

Hi Uwe,

Am Freitag, dem 13.08.2021 um 17:22 +0200 schrieb Uwe Kleine-König:
> Hello,
> 
> compared to (implicit) v1, the following changed here:
> 
>  - be a bit more conservative in "imx-usb-loader: Add support for
>    i.MX8MP" regarding last_transfer; feedback by Sascha Hauer
> 
>  - Add support in PBL to be actually booted using imx-usb-loader (only
>    tested the vendor U-Boot for v1), also skip the barebox header (i.e.
>    the first (usually) 0x8000 bytes) before the IVT header on uploadeing
> 
> There are two things that resulted in discussions between Lucas, Ahmad
> and me during development of this series:
> 
>  - On i.MX8MM and i.MX8MQ after the PBL is uploaded it's followed up by
>    the complete image. So the PBL is uploaded twice. The motivation for
>    this behaviour was that when booting from MMC loading from offset 0
>    is easier because depending on the mode the hardware is in the offset
>    has to be specified in different ways. So to make the two modes more
>    similar the PBL is loaded twice for both modes.
> 
>    Here however after the PBL only the piggy data is sent because it
>    feels wrong for me to spend some extra effort in imx-usb-loader to do
>    a kludge for USB just because there are some difficulties with MMC.
> 
>    Arguments for this choice (apart from that already being completed
>    and tested :-) are:
> 
>    - No duplication of data that is overwritten anyhow
>    - Better interoperability with U-Boot/mfg-tools
>      While this is usually not our focus I consider it quite annoying
>      that there are at least three implementations of imx-usb-loader and
>      depending on which machine and bootloader you use you have to pick
>      the right implementation. With the longterm goal to have
>      imx-usb-loader and barebox available in Debian some
>      interoperability would be nice.
> 
>    I quickly tried using the boot rom load image support when booting
>    from SD, but it didn't work out of the box and I didn't debug that.
>    However I imagine that in this mode it would also be more natural to
>    not expect the PBL twice.
> 
>    Switching to the duplicated operation mode would make imx-usb-loader
>    a bit more complicated (because in the MXS code path the duplication
>    isn't available yet), in return we'd save an offset calculation in
>    the PBL.

I can follow those arguments, but for the sake of the sanity of
everyone involved I would vote for changing the i.MX8MM USB loading to
work in the same way, maybe even using the ROM API if it works there
too. i.MX8MM isn't too widely used yet, so I guess we can still change
it without inflicting too much incompatibility pain.

The i.MX8M* Barebox lowlevel flow is already quite delicate, so I would
rather avoid having to deal with subtle differences between the various
family members.

> 
>    I should be possible to look at the first post-PBL data chunk coming
>    in via USB and judge which mode of operation is used and behave
>    accordingly. If this is considered a good idea I can create a patch
>    for that.
> 
>  - When booting via USB the boot source is identified as "serial". I
>    would expect that the semantic of that is rs232 and not USB. The
>    source of this confusion is probably that the Reference Manual calls
>    the USB  boot mode "USB Serial Download boot". I stuck to
>    BOOTSOURCE_SERIAL here for consistency. If it's not only me who would
>    consider using BOOTSOURCE_USB instead a fix this should be changed
>    consistently for all i.MX platforms.
>    Ahmad pointed out that this might break barebox shell scripts.

This however sounds like breaking backwards compat for no real gain. As
even the reference manual calls this boot mode "serial" I always found
this quite clear. My vote is on keeping things as they are.

Regards,
Lucas

> 
> Uwe Kleine-König (3):
>   imx8mp-evk: Add support for booting via USB
>   imx-usb-loader: Drop nearly unused struct usb_id
>   imx-usb-loader: Add support for i.MX8MP
> 
>  arch/arm/boards/nxp-imx8mp-evk/lowlevel.c |  27 +++++
>  arch/arm/mach-imx/boot.c                  |   4 +-
>  include/asm-generic/sections.h            |   1 +
>  scripts/imx/imx-usb-loader.c              | 131 ++++++++++++----------
>  4 files changed, 103 insertions(+), 60 deletions(-)
> 
> 
> base-commit: 72424fd057d135ec0e41139fe4cb5740471d33a5