linux-sunxi.lists.linux.dev archive mirror
 help / color / mirror / Atom feed
From: Sean Anderson <sean.anderson@seco.com>
To: Andre Przywara <andre.przywara@arm.com>
Cc: Simon Glass <sjg@chromium.org>,
	Jagan Teki <jagan@amarulasolutions.com>,
	u-boot@lists.denx.de, linux-sunxi@lists.linux.dev,
	Patrick Delaunay <patrick.delaunay@foss.st.com>,
	Heiko Schocher <hs@denx.de>,
	Kever Yang <kever.yang@rock-chips.com>,
	Philipp Tomsich <philipp.tomsich@vrull.eu>
Subject: Re: [RFC PATCH 0/3] fastboot: sunxi: Determine MMC device at runtime
Date: Mon, 24 May 2021 11:33:29 -0400	[thread overview]
Message-ID: <b5fabc04-18c0-dc31-f1a5-93501b5d9bc5@seco.com> (raw)
In-Reply-To: <20210524161508.2129096c@slackpad.fritz.box>



On 5/24/21 11:15 AM, Andre Przywara wrote:
 > On Mon, 24 May 2021 10:37:31 -0400
 > Sean Anderson <sean.anderson@seco.com> wrote:
 >
 >> On 5/23/21 8:36 PM, Andre Przywara wrote:
 >>   > At the moment the fastboot code relies on the Kconfig variable
 >>   > CONFIG_FASTBOOT_FLASH_MMC_DEV to point to the MMC device to use for the
 >>   > flash command. This value needs to be the *U-Boot device number*, which
 >>   > is picked by the U-Boot device model at runtime. This makes it quite
 >>   > tricky and fragile to fix this variable at compile time, as other DT
 >>   > nodes and aliases influence the enumeration process.
 >>   >
 >>   > To make this process more robust, allow the device number to be picked at
 >>   > runtime, which sounds like a better fit to find this device number. Patch
 >>   > 1/3 introduces a weak function for that purpose.
 >>   > Patch 2/3 then implements this function for the Allwinner platform. The
 >>   > code follows the idea behind the existing Kconfig defaults: Use the eMMC
 >>   > device, if that exists, or the SD card otherwise. This patch is actually
 >>   > not sunxi specific, so might be adopted by other platforms as well.
 >>   > Patch 3/3 then drops the existing Kconfig defaults for sunxi, to clean
 >>   > this up and remove the implicit assumption that the eMMC device is always
 >>   > device 1 (as least for the fastboot code).
 >>   >
 >>   > I would be curious if others think this is the right way forward.
 >>   > The fact that the U-Boot device numbers are determined at runtime
 >>   > seems to conflict with the idea of a Kconfig variable in the first place,
 >>   > hence this series. This brings us one step closer to the end goal of
 >>   > removing the "eMMC is device 1" assumption.
 >>
 >> I would actually favor removing CONFIG_FASTBOOT_FLASH_MMC_DEV
 >> altogether, and just specifying the device explicitly in fastboot
 >> commands. If you need to dynamically change the device, you can create
 >> some aliases. E.g. you could have something like
 >>
 >> "fastboot_aliases=setenv fastboot_partition_alias_user ${mmcdev}.0:0"
 >>
 >> and then run this variable just before calling `fastboot 0` (or whatever
 >> your usb device is).
 >
 > Fine with me. I was actually wondering about this already, but didn't
 > want to disrupt every user, especially since I can't really test this
 > very well.

The ideal way would be to define aliases dynamically mapping old
partition names to new syntax. So if the U-Boot shell were a bit more
expressive, one could do

	for partition in $(mmc part list $mmcdev); do
		env set fastboot_partition_alias_${partition} ${mmcdev}\#${partition}
	done
	fastboot usb 0

which would map old partition names to new-style syntax. Unfortunately,
we don't quite have that capability yet. I think defining a bunch of
aliases when you run fastboot could be... suprising. I'm not sure if
there is a good solution here.

 > So can you use this explicit device naming from the host side already
 > with the current code?  Can you give an example how this would look
 > like?

Flash the partition named "rootfs" on MMC 0 hardware partition 0

$ fastboot flash 0#rootfs root.simg

Flash the entirety of MMC 1 hardware partition 2

$ fastboot flash 1.2:0 boot2.img

For a more thorough treatment of this syntax, see [1]

 > The documentation I could find only speaks of the Android
 > partition names (like "system"), which requires environment variables
 > to work, IIUC?

They require that you use a GPT-formatted disk with partition labels.
Alternatively, you can use aliases. There is one sentence on the
new-style syntax here [2], but I really should expand on it...

--Sean

[1] https://u-boot.readthedocs.io/en/latest/usage/partitions.html#partitions
[2] https://u-boot.readthedocs.io/en/latest/android/fastboot.html#partition-names

 >
 > Thanks,
 > Andre
 >
 >>   >
 >>   > I am looking forward to any comments on this series!
 >>   >
 >>   > Cheers,
 >>   > Andre
 >>   >
 >>   > Andre Przywara (3):
 >>   >    fastboot: Allow runtime determination of MMC device
 >>   >    sunxi: Implement fastboot_get_mmc_device()
 >>   >    sunxi: Drop sunxi FASTBOOT_FLASH_MMC_DEV defaults
 >>   >
 >>   >   board/sunxi/board.c           | 37 +++++++++++++++++++++++++++++++++++
 >>   >   drivers/fastboot/Kconfig      |  4 +---
 >>   >   drivers/fastboot/fb_command.c |  6 +++---
 >>   >   drivers/fastboot/fb_common.c  |  3 ++-
 >>   >   drivers/fastboot/fb_mmc.c     | 12 ++++++++----
 >>   >   include/fastboot.h            |  7 +++++++
 >>   >   6 files changed, 58 insertions(+), 11 deletions(-)
 >>   >
 >

  reply	other threads:[~2021-05-24 15:33 UTC|newest]

Thread overview: 14+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2021-05-24  0:36 Andre Przywara
2021-05-24  0:36 ` [RFC PATCH 1/3] fastboot: Allow runtime determination of MMC device Andre Przywara
2021-05-27 13:44   ` Simon Glass
2021-05-24  0:36 ` [RFC PATCH 2/3] sunxi: Implement fastboot_get_mmc_device() Andre Przywara
2021-05-24 14:33   ` Sean Anderson
2021-05-24 14:57     ` Andre Przywara
2021-05-24  0:36 ` [RFC PATCH 3/3] sunxi: Drop sunxi FASTBOOT_FLASH_MMC_DEV defaults Andre Przywara
2021-05-24 14:37 ` [RFC PATCH 0/3] fastboot: sunxi: Determine MMC device at runtime Sean Anderson
2021-05-24 15:15   ` Andre Przywara
2021-05-24 15:33     ` Sean Anderson [this message]
2021-05-24 15:28   ` Maxime Ripard
2021-05-24 15:43     ` Sean Anderson
2021-06-07 15:14       ` Maxime Ripard
2021-06-07 15:21         ` Sean Anderson

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=b5fabc04-18c0-dc31-f1a5-93501b5d9bc5@seco.com \
    --to=sean.anderson@seco.com \
    --cc=andre.przywara@arm.com \
    --cc=hs@denx.de \
    --cc=jagan@amarulasolutions.com \
    --cc=kever.yang@rock-chips.com \
    --cc=linux-sunxi@lists.linux.dev \
    --cc=patrick.delaunay@foss.st.com \
    --cc=philipp.tomsich@vrull.eu \
    --cc=sjg@chromium.org \
    --cc=u-boot@lists.denx.de \
    --subject='Re: [RFC PATCH 0/3] fastboot: sunxi: Determine MMC device at runtime' \
    /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

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).