mail archive of the barebox mailing list
 help / color / mirror / Atom feed
From: Ahmad Fatoum <a.fatoum@pengutronix.de>
To: "Lucas Stach" <l.stach@pengutronix.de>,
	"Uwe Kleine-König" <u.kleine-koenig@pengutronix.de>,
	barebox@lists.infradead.org
Cc: rcz@pengutronix.de
Subject: Re: [PATCH v2 0/3] Support usb booting on i.MX8MP
Date: Wed, 13 Oct 2021 21:33:09 +0200	[thread overview]
Message-ID: <9be876f6-3719-addd-e241-90c35dd8b9f5@pengutronix.de> (raw)
In-Reply-To: <18eb76a0689a44244b81d33d3606cb48f9f447ff.camel@pengutronix.de>

On 13.08.21 22:32, Lucas Stach wrote:
> 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.

8MM doesn't have ROM API. 8MN does.

> 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.

On 8MM, the PBL loaded in the second upload is overwritten with the running
PBL (from the first upload), so practically, the only change seems to be
whether we overwrite a PBL transferred for nought or not?

>>    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.

Sounds like a good compromise, but I don't yet understand Lucas'
point. PBL is entered twice with Uwe's changes as well.

>>  - 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.

Agreed.

Cheers,
Ahmad

> 
> 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
> 
> 
> 


-- 
Pengutronix e.K.                           |                             |
Steuerwalder Str. 21                       | http://www.pengutronix.de/  |
31137 Hildesheim, Germany                  | Phone: +49-5121-206917-0    |
Amtsgericht Hildesheim, HRA 2686           | Fax:   +49-5121-206917-5555 |

_______________________________________________
barebox mailing list
barebox@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/barebox

      reply	other threads:[~2021-10-13 19:35 UTC|newest]

Thread overview: 12+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2021-08-13 15:22 Uwe Kleine-König
2021-08-13 15:22 ` [PATCH v2 1/3] imx8mp-evk: Add support for booting via USB Uwe Kleine-König
2021-08-13 19:26   ` Ahmad Fatoum
2021-08-13 20:20     ` Lucas Stach
2021-08-13 20:48       ` Ahmad Fatoum
2021-10-26 16:30   ` Ahmad Fatoum
2021-08-13 15:22 ` [PATCH v2 2/3] imx-usb-loader: Drop nearly unused struct usb_id Uwe Kleine-König
2021-10-27  6:05   ` Ahmad Fatoum
2021-08-13 15:22 ` [PATCH v2 3/3] imx-usb-loader: Add support for i.MX8MP Uwe Kleine-König
2021-10-27  6:06   ` Ahmad Fatoum
2021-08-13 20:32 ` [PATCH v2 0/3] Support usb booting on i.MX8MP Lucas Stach
2021-10-13 19:33   ` Ahmad Fatoum [this message]

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=9be876f6-3719-addd-e241-90c35dd8b9f5@pengutronix.de \
    --to=a.fatoum@pengutronix.de \
    --cc=barebox@lists.infradead.org \
    --cc=l.stach@pengutronix.de \
    --cc=rcz@pengutronix.de \
    --cc=u.kleine-koenig@pengutronix.de \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox