[PATCH RFT v3 0/3] fastboot: add support for generic block flashing

[Tom Rini]

On Wed, May 21, 2025 at 08:52:41PM +0200, Mattijs Korpershoek wrote:
> Hi Tom,
> 
> On mer., mai 21, 2025 at 09:12, Tom Rini <trini at konsulko.com> wrote:
> 
> > On Wed, May 21, 2025 at 04:49:35PM +0200, Mattijs Korpershoek wrote:
> >> Hi Neil,
> >> 
> >> On mar., mai 20, 2025 at 13:35, Mattijs Korpershoek <mkorpershoek at kernel.org> wrote:
> >> 
> >> > Hi,
> >> >
> >> > On Tue, 06 May 2025 18:10:06 +0200, neil.armstrong at linaro.org wrote:
> >> >> This serie permits using any block device as target
> >> >> for fastboot by moving the generic block logic into
> >> >> a common set of helpers and also use them as generic
> >> >> backend.
> >> >> 
> >> >> The erase logic has been extended to support software
> >> >> erase since only 2 block drivers exposes the erase
> >> >> operation.
> >> >> 
> >> >> [...]
> >> >
> >> > Thanks, Applied to https://source.denx.de/u-boot/custodians/u-boot-dfu (u-boot-dfu-next)
> >> >
> >> > [1/3] fastboot: blk: introduce fastboot block flashing support
> >> >       https://source.denx.de/u-boot/custodians/u-boot-dfu/-/commit/88239b5bb04bea2b58f7bf4c3ea72cf832de818c
> >> > [2/3] fastboot: blk: switch emmc to use the block helpers
> >> >       https://source.denx.de/u-boot/custodians/u-boot-dfu/-/commit/25ab5c32c28b9f25fb193f726f239d75af3c365a
> >> > [3/3] fastboot: integrate block flashing back-end
> >> >       https://source.denx.de/u-boot/custodians/u-boot-dfu/-/commit/a885bd8c969e25d03bf406207d89b1145c9490fb
> >> 
> >> It seems this series cause CI to fail:
> >> https://source.denx.de/u-boot/custodians/u-boot-dfu/-/pipelines/26238
> >> 
> >> Without the patches applied, it passes:
> >> https://source.denx.de/u-boot/custodians/u-boot-dfu/-/pipelines/26235
> >> 
> >> Do you have any idea what is going wrong?
> >> I could not find anything obvious by skimming through the logs.
> >
> > It's a Kconfig problem then. Some platform is prompting for a value (not
> > a y/n) and there's no default.
> 
> You are correct. Thank you for the suggestion.
> 
> I've applied the following diff to 3/3:
> diff --git a/drivers/fastboot/Kconfig b/drivers/fastboot/Kconfig
> index 68967abb751e..fdf34a6abe1e 100644
> --- a/drivers/fastboot/Kconfig
> +++ b/drivers/fastboot/Kconfig
> @@ -200,6 +200,7 @@ config FASTBOOT_MMC_USER_NAME
>  config FASTBOOT_FLASH_BLOCK_INTERFACE_NAME
>         string "Define FASTBOOT block interface name"
>         depends on FASTBOOT_FLASH_BLOCK
> +       default "none"
>         help
>           The fastboot "flash" and "erase" commands support operations
>           on any Block device, this should specify the block device name

Assuming that the code will see "none" and handle the error correctly,
OK. But we really should have a configured true value here, yes?

> @@ -212,6 +213,7 @@ config FASTBOOT_FLASH_BLOCK_INTERFACE_NAME
>  config FASTBOOT_FLASH_BLOCK_DEVICE_ID
>         int "Define FASTBOOT block device identifier"
>         depends on FASTBOOT_FLASH_BLOCK
> +       default 0
>         help
>           The fastboot "flash" and "erase" commands support operations
>           on any Block device, this should specify the block device

I do not like this at first. If FASTBOOT_FLASH_BLOCK_DEVICE_ID is set,
there should be a valid ID set too yes? Potentially worse, is 0 a valid
option here? If so, is that likely to be a real and common one? In that
case, we should also be updating the help text to make sure it's clear
what the normal range is I think.

-- 
Tom
-------------- next part --------------
A non-text attachment was scrubbed...
Name: signature.asc
Type: application/pgp-signature
Size: 659 bytes
Desc: not available
URL: <https://lists.denx.de/pipermail/u-boot/attachments/20250521/8bc1e7bc/attachment.sig>