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 ?
@@ -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.
Yes 0 is a sentible value
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?
Sure
Thanks,
Neil
--
Tom
