linux-sunxi.lists.linux.dev archive mirror
 help / color / mirror / Atom feed
From: Maxime Ripard <maxime@cerno.tech>
To: Sean Anderson <sean.anderson@seco.com>
Cc: Andre Przywara <andre.przywara@arm.com>,
	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, 7 Jun 2021 17:14:07 +0200	[thread overview]
Message-ID: <20210607151407.wjktjyvd37d27nmy@gilmour> (raw)
In-Reply-To: <9f77283d-109a-25ad-d4be-4867ee47d0db@seco.com>

[-- Attachment #1: Type: text/plain, Size: 3350 bytes --]

On Mon, May 24, 2021 at 11:43:43AM -0400, Sean Anderson wrote:
> 
> 
> On 5/24/21 11:28 AM, Maxime Ripard wrote:
> > On Mon, May 24, 2021 at 10:37:31AM -0400, Sean Anderson 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).
> >
> > If we're going that way, maybe we can just pass the interface on the
> > command line like dfu does?
> 
> That could work, but I don't think it's necessary. At the moment there
> are many different ways to specify partitions (KConfig, Aliases, "U-boot
> syntax", GPT partition labels). I would rather pare things down to the
> minimum necessary ways than add yet another bit of state to specify
> partitions.
> 
> > That way the new requirement would be very obvious instead of
> > introducing a new environment variable no one really expects?
> 
> I'm not sure what you mean here. This alias system has been in place for
> a while, and it's very convenient for mapping a stable name to some
> arbitrary device and partition.

I don't deny the fact that it can be convenient. What I was pointing out
is that it's been optional the whole time, that I've been using fastboot
for like 5 years and didn't realize that this feature was there, and
it's only implemented for mmc.

To make it required, without any warning, would be fairly confusing.

Maxime

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 228 bytes --]

  reply	other threads:[~2021-06-07 15:14 UTC|newest]

Thread overview: 14+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2021-05-24  0:36 [RFC PATCH 0/3] fastboot: sunxi: Determine MMC device at runtime 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
2021-05-24 15:28   ` Maxime Ripard
2021-05-24 15:43     ` Sean Anderson
2021-06-07 15:14       ` Maxime Ripard [this message]
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=20210607151407.wjktjyvd37d27nmy@gilmour \
    --to=maxime@cerno.tech \
    --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=sean.anderson@seco.com \
    --cc=sjg@chromium.org \
    --cc=u-boot@lists.denx.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;
as well as URLs for NNTP newsgroup(s).