On Thu, May 22, 2025 at 09:52:30AM +0200, neil.armstr...@linaro.org wrote: > On 22/05/2025 08:58, Mattijs Korpershoek wrote: > > 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; > > } > > It could be "" aswell ?
I really do not like putting in invalid defaults in order to allow "make oldconfig" and so forth to not get stuck. If a board needs to set a value to compile, it needs to put in a valid value. Putting in unusable defaults to Kconfig leads to runtime problems later (this is more clear with "" and *much* less clear with a maybe-valid 0). My preference here would be to update whatever the stuck platform is to either disable the feature or set correct values. -- Tom
signature.asc
Description: PGP signature
