пт, 16 жовт. 2026 р. о 17:26 Mattijs Korpershoek <[email protected]> пише:
>
> 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.
> >
> > 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.
>
Hello Mattijs, sure sure, will do.
> > ---
> > 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.
>
Sure.
> > +__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.
>
Using -ENOTSUPP will enforce using it in board calls as well if you
want to continue normal operation, this would look even more weird. I
can document how to implement this callback and why 0 is used.
Check drivers/fastboot/fb_spi_flash.c and drivers/fastboot/fb_nand.c
Both use the similar calls just for their specific cases.
> > +}
> > +
> > +__weak int board_fastboot_erase(char *cmd_parameter, char *response)
> > +{
> > + return 0;
>
> Same.
>
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?
>
if (IS_ENABLED(CONFIG_FASTBOOT_FLASH)) {
r = board_fastboot_get_part_info(part_name, response, size);
if (r != 1)
return r;
}
getvar_get_part_info continues; Tegra callback returns 0 for
bootloader partitions, and 1 for every other for example.
> 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.
>
Here any code can be used I suppose, -ENOTSUPP is fine, then
if (IS_ENABLED(CONFIG_FASTBOOT_FLASH)) {
r = board_fastboot_get_part_info(part_name, response, size);
if (r != -ENOTSUPP)
return r;
}
> > +}
> > +
> > 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
