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

[Neil Armstrong]

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 at google.com>
>>
>> 1. Get partition info/size
>> 2. Erase partition
>> 3. Flash partition
>> 4. BCB
>>
>> Signed-off-by: Dmitrii Merkurev <dimorinny at google.com>
>> Reviewed-by: Mattijs Korpershoek <mkorpershoek at baylibre.com>
>> Tested-by: Mattijs Korpershoek <mkorpershoek at kernel.org>
>> Signed-off-by: Neil Armstrong <neil.armstrong at 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.

> 
>> +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
>> +	  identifier on the system, as a number.
>> +          The device identifier should be 0 for first device on the
>> +	  interface type specified in FLASH_BLOCK_INTERFACE_NAME config,
>> +	  1 the second, etc...
> 
> This help should be one paragraph and note something along the lines of:
> - Device identifiers are numbered starting from 0.
> - The most common case is to use the first controller.
> 
> And then yes, "default 0" is fine here because it is a reasonable
> default when configuring the system to use the functionality.

Right, I'll update with that.

Thanks,
Neil

>