Re: [PATCH v2 4/6] cmd: Add flush support for all blk devices

[dmukhin@ford.com]

On Fri, May 29, 2026 at 04:57:48AM -0500, Simon Glass wrote:
> Hi Denis,
> 
> On 2026-05-29T03:44:35, None <dmukhin@ford.com> wrote:
> > cmd: Add flush support for all blk devices
> >
> > Introduce flush subcommand for all blk devices to allow committing
> > dirty data explicitly to the given block device.
> >
> > Signed-off-by: Denis Mukhin <dmukhin@ford.com>
> >
> > cmd/blk_common.c | 14 ++++++++++++++
> >  cmd/ide.c        |  1 +
> >  cmd/nvme.c       |  1 +
> >  cmd/pvblock.c    |  1 +
> >  cmd/sata.c       |  1 +
> >  cmd/scsi.c       |  1 +
> >  cmd/usb.c        |  1 +
> >  cmd/virtio.c     |  1 +
> >  8 files changed, 21 insertions(+)
> 
> > diff --git a/cmd/blk_common.c b/cmd/blk_common.c
> > @@ -37,6 +37,20 @@ int blk_common_cmd(int argc, char *const argv[], enum uclass_id uclass_id,
> > +             } else if (strncmp(argv[1], 'flush', 5) == 0) {
> > +                     struct blk_desc *desc;
> > +                     int ret;
> > +
> > +                     ret = blk_get_desc(uclass_id, *cur_devnump, &desc);
> > +                     if (ret)
> > +                             return CMD_RET_FAILURE;
> > +
> > +                     if (blk_dflush(desc)) {
> > +                             printf("\nfailed to flush device %s\n", if_name);
> > +                             return CMD_RET_FAILURE;
> > +                     }
> > +
> > +                     return CMD_RET_SUCCESS;
> > +             }
> 
> Please print the return value from blk_dflush() and the device number
> in the failure message - otherwise a user can't tell -ENOSYS from a
> genuine I/O error. Compare with the erase arm just below which reports
> the block count. You can use %dE to get an error string if the board
> enables it.
> 
> Do you want to print something on success?
> 
> It is a bit misleading to suggest that all the commands support
> 'flush'. But I suppose we can worry about that later.
> 
> > diff --git a/cmd/blk_common.c b/cmd/blk_common.c
> > @@ -37,6 +37,20 @@ int blk_common_cmd(int argc, char *const argv[], enum uclass_id uclass_id,
> > +             } else if (strncmp(argv[1], 'flush', 5) == 0) {
> 
> cmd/blkmap.c also routes through blk_common_cmd(), but dispatches via
> U_BOOT_SUBCMD_MKENT() so flush never reaches this function unless you
> add a matching subcmd entry. Either add it for consistency or call out
> in the commit message that blkmap is intentionally excluded.

Thanks for the feedback!
Will update the patch.

> 
> Regards,
> Simon