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 <tr...@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 > >> <mkorpersh...@kernel.org> wrote: > >> > >> > Hi, > >> > > >> > On Tue, 06 May 2025 18:10:06 +0200, neil.armstr...@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
signature.asc
Description: PGP signature