On Fri, Oct 16, 2026 at 16:26, Mattijs Korpershoek <[email protected]> wrote:
> Hi Svyatoslav, > > Thank you for the patch. > > On Wed, Sep 30, 2026 at 10:55, Svyatoslav Ryhel <[email protected]> wrote: > >> From: Ion Agorria <[email protected]> >> >> This allows boards to intercept fastboot flash/erase and modify/change >> behavior according to board specifics like AES encryption or RSA signing. >> Add board_fastboot_get_part_info to provide info as fastboot tool queries >> during flash/erase. I've given this some more thought over the weekend. We already have an 'oem board' command in fastboot. See: https://docs.u-boot-project.org/en/latest/android/fastboot.html#overview Have we considered using the 'oem board' command instead of adding another board specific command? The documentation also gives an example on custom flashing, see: https://docs.u-boot-project.org/en/latest/android/fastboot.html#running-custom-vendor-code >> >> Signed-off-by: Ion Agorria <[email protected]> >> Signed-off-by: Svyatoslav Ryhel <[email protected]> > > This patch does not apply on main > commit fed92546960a ("Gitlab CI: sage: Update for labgrid 26.0 behavior") > > I'll try to review it anyways, but please consider resubmitting it after > rebasing. > >> --- >> drivers/fastboot/fb_command.c | 20 ++++++++++++++++++++ >> drivers/fastboot/fb_getvar.c | 12 ++++++++++++ >> 2 files changed, 32 insertions(+) >> >> diff --git a/drivers/fastboot/fb_command.c b/drivers/fastboot/fb_command.c >> index 18d86988f4c..284d771fe31 100644 >> --- a/drivers/fastboot/fb_command.c >> +++ b/drivers/fastboot/fb_command.c >> @@ -17,6 +17,17 @@ >> #include <vsprintf.h> >> #include <linux/printk.h> >> > > Please document the function. Especially the return code so that > board-specific implementations know what to do. > >> +__weak int board_fastboot_flash(char *cmd_parameter, void *download_buffer, >> + u32 download_bytes, char *response) >> +{ >> + return 0; > > Why 0? > > Can we please use a more meaningful error code here? -ENOTSUPP for example. > >> +} >> + >> +__weak int board_fastboot_erase(char *cmd_parameter, char *response) >> +{ >> + return 0; > > Same. > >> +} >> + >> /** >> * image_size - final fastboot image size >> */ >> @@ -339,6 +350,11 @@ void fastboot_data_complete(char *response) >> */ >> static void __maybe_unused flash(char *cmd_parameter, char *response) >> { >> + if (IS_ENABLED(CONFIG_FASTBOOT_FLASH) && >> + board_fastboot_flash(cmd_parameter, fastboot_buf_addr, >> + image_size, response)) >> + return; >> + >> if (IS_ENABLED(CONFIG_FASTBOOT_FLASH_BLOCK)) >> fastboot_block_flash_write(cmd_parameter, fastboot_buf_addr, >> image_size, response); >> @@ -367,6 +383,10 @@ 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) && >> + board_fastboot_erase(cmd_parameter, response)) >> + return; >> + >> if (IS_ENABLED(CONFIG_FASTBOOT_FLASH_BLOCK)) >> fastboot_block_erase(cmd_parameter, response); >> >> diff --git a/drivers/fastboot/fb_getvar.c b/drivers/fastboot/fb_getvar.c >> index e8aa0e09aa6..af65fca548f 100644 >> --- a/drivers/fastboot/fb_getvar.c >> +++ b/drivers/fastboot/fb_getvar.c >> @@ -29,6 +29,12 @@ static void getvar_partition_type(char *part_name, char >> *response); >> static void getvar_partition_size(char *part_name, char *response); >> static void getvar_is_userspace(char *var_parameter, char *response); >> >> +__weak int board_fastboot_get_part_info(const char *part_name, char >> *response, >> + size_t *size) >> +{ >> + return 1; > > Why 1? > > What happens if a board_fastboot_get_part_info() returns 1 here? > > Also if we look at the documentation of part_get_info_by_name(): > """ > * part_get_info_by_name() - Search for a partition by name > * among all available registered partitions > * > * @desc: block device descriptor > * @name: the specified table entry name > * @info: returns the disk partition info > * > * Return: the partition number on match (starting on 1), -1 on no match, > * otherwise error > */ > int part_get_info_by_name(struct blk_desc *desc, const char *name, > struct disk_partition *info); > """ > > So 1 can be confusing. > > Can't we return something like -ENOTSUPP instead? > That makes it more clear that this is a stub operation that can be > re-implemented by each board. > >> +} >> + >> static const struct { >> const char *variable; >> bool list; >> @@ -116,6 +122,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)) { >> + r = board_fastboot_get_part_info(part_name, response, size); >> + if (r != 1) >> + return r; >> + } >> + >> if (IS_ENABLED(CONFIG_FASTBOOT_FLASH_BLOCK)) { >> r = fastboot_block_get_part_info(part_name, &dev_desc, >> &disk_part, >> response); >> -- >> 2.53.0
