Re: [RFC PATCH 0/3] fastboot: sunxi: Determine MMC device at runtime
On 6/7/21 11:14 AM, Maxime Ripard wrote:
> 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.
Well, it would only be required if you need to set the MMC device at
runtime. For existing users, there is no change required. I'm
specifically pointing out that this functionality is already achievable
without any changes to C code.
--Sean
Re: [RFC PATCH 0/3] fastboot: sunxi: Determine MMC device at runtime
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
signature.asc
Description: PGP signature
Re: [RFC PATCH 0/3] fastboot: sunxi: Determine MMC device at runtime
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
Re: [RFC PATCH 0/3] fastboot: sunxi: Determine MMC device at runtime
On 5/24/21 11:15 AM, Andre Przywara wrote:
> On Mon, 24 May 2021 10:37:31 -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).
>
> 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(-)
>> >
>
Re: [RFC PATCH 0/3] fastboot: sunxi: Determine MMC device at runtime
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 way the new requirement would be very
obvious instead of introducing a new environment variable no one really
expects?
Maxime
signature.asc
Description: PGP signature
Re: [RFC PATCH 0/3] fastboot: sunxi: Determine MMC device at runtime
On Mon, 24 May 2021 10:37:31 -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).
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.
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? The documentation I could find only speaks of the Android
partition names (like "system"), which requires environment variables
to work, IIUC?
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(-)
> >
Re: [RFC PATCH 0/3] fastboot: sunxi: Determine MMC device at runtime
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).
--Sean
>
> 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(-)
>
[RFC PATCH 0/3] fastboot: sunxi: Determine MMC device at runtime
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 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(-) -- 2.17.5
