Re: [PATCH 4/4] fastboot: integrate block flashing back-end

[Mattijs Korpershoek <mkorpershoek@baylibre.com>]

Hi Dmitrii,

Thank you for the patch and sorry for the review delay.

On mer., mars 06, 2024 at 18:59, Dmitrii Merkurev <dimorinny@google.com> wrote:

> 1. Get partition info/size
> 2. Erase partition
> 3. Flash partition
> 4. BCB
>
> Signed-off-by: Dmitrii Merkurev <dimorinny@google.com>
> Cc: Alex Kiernan <alex.kiernan@gmail.com>
> Cc: Patrick Delaunay <patrick.delaunay@foss.st.com>
> Cc: Simon Glass <sjg@chromium.org>
> Cc: Mattijs Korpershoek <mkorpershoek@baylibre.com>
> Cc: Ying-Chun Liu (PaulLiu) <paul.liu@linaro.org>
> ---
>  drivers/fastboot/fb_command.c |  8 ++++++++
>  drivers/fastboot/fb_common.c  | 15 +++++++++++----
>  drivers/fastboot/fb_getvar.c  |  8 +++++++-
>  3 files changed, 26 insertions(+), 5 deletions(-)
>
> diff --git a/drivers/fastboot/fb_command.c b/drivers/fastboot/fb_command.c
> index f95f4e4ae1..67ebe02efa 100644
> --- a/drivers/fastboot/fb_command.c
> +++ b/drivers/fastboot/fb_command.c
> @@ -9,6 +9,7 @@
>  #include <env.h>
>  #include <fastboot.h>
>  #include <fastboot-internal.h>
> +#include <fb_block.h>
>  #include <fb_mmc.h>
>  #include <fb_nand.h>
>  #include <part.h>
> @@ -331,6 +332,10 @@ void fastboot_data_complete(char *response)
>   */
>  static void __maybe_unused flash(char *cmd_parameter, char *response)
>  {
> +	if (IS_ENABLED(CONFIG_FASTBOOT_FLASH_BLOCK))
> +		fastboot_block_flash_write(cmd_parameter, fastboot_buf_addr,
> +					   image_size, response);
> +
>  	if (IS_ENABLED(CONFIG_FASTBOOT_FLASH_MMC))
>  		fastboot_mmc_flash_write(cmd_parameter, fastboot_buf_addr,
>  					 image_size, response);
> @@ -351,6 +356,9 @@ static void __maybe_unused flash(char *cmd_parameter, char *response)
>   */
>  static void __maybe_unused erase(char *cmd_parameter, char *response)
>  {
> +	if (IS_ENABLED(CONFIG_FASTBOOT_FLASH_BLOCK))
> +		fastboot_block_erase(cmd_parameter, response);
> +
>  	if (IS_ENABLED(CONFIG_FASTBOOT_FLASH_MMC))
>  		fastboot_mmc_erase(cmd_parameter, response);
>  
> diff --git a/drivers/fastboot/fb_common.c b/drivers/fastboot/fb_common.c
> index 3576b06772..7602653c0b 100644
> --- a/drivers/fastboot/fb_common.c
> +++ b/drivers/fastboot/fb_common.c
> @@ -97,16 +97,23 @@ int __weak fastboot_set_reboot_flag(enum fastboot_reboot_reason reason)
>  		[FASTBOOT_REBOOT_REASON_FASTBOOTD] = "boot-fastboot",
>  		[FASTBOOT_REBOOT_REASON_RECOVERY] = "boot-recovery"
>  	};
> -	const int mmc_dev = config_opt_enabled(CONFIG_FASTBOOT_FLASH_MMC,
> -					       CONFIG_FASTBOOT_FLASH_MMC_DEV, -1);
>  
> -	if (!IS_ENABLED(CONFIG_FASTBOOT_FLASH_MMC))
> +	int device = config_opt_enabled(CONFIG_FASTBOOT_FLASH_BLOCK,
> +					CONFIG_FASTBOOT_FLASH_BLOCK_DEVICE_ID, -1);
> +	if (device == -1) {
> +		device = config_opt_enabled(CONFIG_FASTBOOT_FLASH_MMC,
> +					    CONFIG_FASTBOOT_FLASH_MMC_DEV, -1);
> +	}
> +	char *bcb_iface = config_opt_enabled(CONFIG_FASTBOOT_FLASH_BLOCK,

Can this be made const?

With the above addressed:

Reviewed-by: Mattijs Korpershoek <mkorpershoek@baylibre.com>

> +					     CONFIG_FASTBOOT_FLASH_BLOCK_INTERFACE_NAME, "mmc");
> +
> +	if (device == -1)
>  		return -EINVAL;
>  
>  	if (reason >= FASTBOOT_REBOOT_REASONS_COUNT)
>  		return -EINVAL;
>  
> -	ret = bcb_find_partition_and_load("mmc", mmc_dev, "misc");
> +	ret = bcb_find_partition_and_load(bcb_iface, device, "misc");
>  	if (ret)
>  		goto out;
>  
> diff --git a/drivers/fastboot/fb_getvar.c b/drivers/fastboot/fb_getvar.c
> index f65519c57b..71507009ab 100644
> --- a/drivers/fastboot/fb_getvar.c
> +++ b/drivers/fastboot/fb_getvar.c
> @@ -8,6 +8,7 @@
>  #include <fastboot.h>
>  #include <fastboot-internal.h>
>  #include <fb_mmc.h>
> +#include <fb_block.h>
>  #include <fb_nand.h>
>  #include <fs.h>
>  #include <part.h>
> @@ -114,7 +115,12 @@ static int getvar_get_part_info(const char *part_name, char *response,
>  	struct disk_partition disk_part;
>  	struct part_info *part_info;
>  
> -	if (IS_ENABLED(CONFIG_FASTBOOT_FLASH_MMC)) {
> +	if (IS_ENABLED(CONFIG_FASTBOOT_FLASH_BLOCK)) {
> +		r = fastboot_block_get_part_info(part_name, &dev_desc, &disk_part,
> +						 response);
> +		if (r >= 0 && size)
> +			*size = disk_part.size * disk_part.blksz;
> +	} else if (IS_ENABLED(CONFIG_FASTBOOT_FLASH_MMC)) {
>  		r = fastboot_mmc_get_part_info(part_name, &dev_desc, &disk_part,
>  					       response);
>  		if (r >= 0 && size)
> -- 
> 2.44.0.278.ge034bb2e1d-goog