Hi Tom, On mer., mai 21, 2025 at 13:03, Tom Rini <tr...@konsulko.com> wrote:
> 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? The code does not handle any special cases. If FASTBOOT_FLASH_BLOCK_INTERFACE_NAME is "none", we will call: blk_get_dev("none", 0); Which will then be handled via: if (!dev_desc) { fastboot_fail("no such device", response); return -ENODEV; } > >> @@ -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 Yes, 0 is a valid option here. Think of this symbol as a similar one to FASTBOOT_FLASH_MMC_DEV however it's for generic block device type (virtio, scsi, ...) I'd think that 0 is typically the most common value, since it's the first block controller of a specific type. > case, we should also be updating the help text to make sure it's clear > what the normal range is I think. Ok. That's a bit too much for me to do in a fixup. Neil, can you send a v4? > > -- > Tom
