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