Re: [PATCH RFT v4 3/3] fastboot: integrate block flashing back-end

[Mattijs Korpershoek <mkorpershoek@redhat.com>]

On jeu., juin 05, 2025 at 19:48, Neil Armstrong <neil.armstrong@linaro.org> wrote:

> On 05/06/2025 16:21, Tom Rini wrote:
>> On Thu, Jun 05, 2025 at 10:16:54AM +0200, Neil Armstrong wrote:
>>> On 22/05/2025 16:39, Tom Rini wrote:
>>>> On Thu, May 22, 2025 at 02:37:07PM +0200, Neil Armstrong wrote:
>>>>
>>>>> From: Dmitrii Merkurev <dimorinny@google.com>
>>>>>
>>>>> 1. Get partition info/size
>>>>> 2. Erase partition
>>>>> 3. Flash partition
>>>>> 4. BCB
>>>>>
>>>>> Signed-off-by: Dmitrii Merkurev <dimorinny@google.com>
>>>>> Reviewed-by: Mattijs Korpershoek <mkorpershoek@baylibre.com>
>>>>> Tested-by: Mattijs Korpershoek <mkorpershoek@kernel.org>
>>>>> Signed-off-by: Neil Armstrong <neil.armstrong@linaro.org>
>>>>> ---
>>>>>    drivers/fastboot/Kconfig      | 29 +++++++++++++++++++++++++++++
>>>>>    drivers/fastboot/Makefile     |  1 +
>>>>>    drivers/fastboot/fb_command.c |  8 ++++++++
>>>>>    drivers/fastboot/fb_common.c  | 22 ++++++++++++++++++----
>>>>>    drivers/fastboot/fb_getvar.c  |  8 +++++++-
>>>>>    5 files changed, 63 insertions(+), 5 deletions(-)
>>>>
>>>> I know this was posted before I replied with more feedback moments ago.
>>>>
>>>> [snip]
>>>>> @@ -193,6 +197,31 @@ config FASTBOOT_MMC_USER_NAME
>>>>>    	  defined here.
>>>>>    	  The default target name for erasing EMMC_USER is "mmc0".
>>>>> +config FASTBOOT_FLASH_BLOCK_INTERFACE_NAME
>>>>> +	string "Define FASTBOOT block interface name"
>>>>> +	depends on FASTBOOT_FLASH_BLOCK
>>>>> +	default ""
>>>>> +	help
>>>>> +	  The fastboot "flash" and "erase" commands support operations
>>>>> +	  on any Block device, this should specify the block device name
>>>>> +	  like ide, scsi, usb, sata, nvme, virtio, blkmap, mtd...
>>>>> +	  The mmc block device type can be used but most of the features
>>>>> +	  available in the FASTBOOT_MMC will be missing.
>>>>> +	  Consider using FASTBOOT_MMC on a MMC block device until all
>>>>> +	  features are migrated.
>>>>
>>>> A default like "" in order to un-stick configs that are now here and
>>>> enabling the option is wrong. If we're enabling new functionality for
>>>> platforms, it needs to be configured correctly. This leads to building
>>>> code on platforms that won't be used on the platform so we've likely
>>>> added run-time bloat for no benefit.
>>>
>>> I agree but what's the solution ? I'll prefer having no default as it was initially.
>> 
>> No defaults is correct here, yes. It's just that the primary
>> dependencies need to be fixed so that platforms don't get stuck on the
>> prompt on features they won't actually use either.
>> 
>> Seeing what boards get stuck, and then having an idea on what
>> dependencies trip them up is tricky. What I usually do in this
>> situation, to see what platform is stuck on the prompt is:
>> - In one terminal, fire off tools/qconfig -sC. Then wait for it to
>>    seemingly be stuck with just one or two platforms left to finish
>>    syncing.
>> - In another terminal, 'ps uxwwww | grep make' to see what the build
>>    directory of one of those stuck platforms is, then manually save off
>>    the .config file, begin investigation.
>> 
>> That should provide what platform is asking this question and not having
>> a reasonable answer. Then it's a matter of seeing if it makes sense for
>> this platform to be here and so just needs to be migrated to this
>> functionality or if it's here because of some dependency problem, for
>> example what I was talking about in the previous part of this series.
>> 
>
> Ok I can't reproduce the crash with the last version, somehow v4 fixed it,
> and the changes I did still work:
> https://source.denx.de/u-boot/custodians/u-boot-ufs/-/pipelines/26523

Hmm, maybe v4 "fixed" it because we have:

default 0 for FASTBOOT_FLASH_BLOCK_DEVICE_ID

If you drop that, do you still not reproduce? (note that we don't care
as much since we agreed upon using "default 0" for device id, but it's
odd that the build issue is no longer there.

>
> Neil