пн, 19 жовт. 2026 р. о 11:33 Mattijs Korpershoek <[email protected]> пише:
>
> 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?

You are mixing up. This patch does not add any vendor commands. It
allows using native commands with vendor quirks seamlessly.

For example. Tegra devices require bootloader and bct to be encrypted
and bct to be updated every time bootloader is flashed.

$ fastboot stage bootloader.img
$ fastboot oem board:write_bootloader

Used has to stage bootloader, then write is with some custom vendor
bootloader flashing command which we have to make, document etc etc.

This series allows intercept native fastboot commands of flashing some
partitions and apply vendor specific adjustments directly. So I can
flash bootloader, boot and system with native command `fastboot flash`
but bootloader flashing will be performed with encryption and does not
require from user any additional movements.

board_fastboot_get_part_info allows filtering partitions to flash sent
by fastboot flash command and board_fastboot_flash implements vendor
hook to flash this partition.

And this is not restricted to bootloader partition only.

>
> 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

Reply via email to