Hi Denis,

On 2026-07-16T22:33:54, None <[email protected]> wrote:
> drivers: block: Introduce blk_flush()/blk_dflush()
>
> Add generic flush operations for committing dirty data to the storage
> device.
>
> This provides a common block-layer interface for flushing pending
> writes, allowing callers to ensure that data buffered by the block
> device or its backing implementation is written out to the underlying
> storage.
>
> Note, flushing a partition device via disk_blk_flush() flushes the entire
> underlying device, so callers don't expect partition-scoped semantics.
>
> Signed-off-by: Denis Mukhin <[email protected]>
>
> disk/disk-uclass.c         |  6 ++++++
>  drivers/block/blk-uclass.c | 15 +++++++++++++++
>  include/blk.h              | 26 ++++++++++++++++++++++++++
>  include/part.h             |  8 ++++++++
>  4 files changed, 55 insertions(+)

> Note, flushing a partition device via disk_blk_flush() flushes the entire
> underlying device, so callers don't expect partition-scoped semantics.

I suspect you mean 'so callers should not expect partition-scoped
semantics' - please can you reword this?

> diff --git a/include/blk.h b/include/blk.h
> @@ -239,6 +240,14 @@ struct blk_ops {
> +     /**
> +      * flush() - commit all dirty data to storage
> +      *
> +      * @dev:        Device to flush
> +      * @return 0 if OK, -ve on error
> +      */
> +     unsigned long (*flush)(struct udevice *dev);

The read/write/erase ops use ulong because they return a block count,
but flush() only ever returns 0 or an error, so please can you make it
(and blk_flush(), blk_dflush() and disk_blk_flush()) return int? That
matches select_hwpart() just below, and avoids errors from drivers
(e.g. nvme_blk_flush() in patch 3) being silently converted to large
positive values at each layer.

> diff --git a/disk/disk-uclass.c b/disk/disk-uclass.c
> @@ -122,6 +122,11 @@ unsigned long disk_blk_erase(struct udevice *dev, 
> lbaint_t start,
> +unsigned long disk_blk_flush(struct udevice *dev)
> +{
> +     return blk_flush(dev_get_parent(dev));
> +}

The other functions in this file (disk_blk_read() etc.) have a
function comment. Please can you add one here, mentioning that this
flushes the whole underlying device, not just the partition? That is
worth stating at the implementation, not just in the commit message.

> diff --git a/include/part.h b/include/part.h
> @@ -454,6 +454,14 @@ ulong disk_blk_erase(struct udevice *dev, lbaint_t 
> start, lbaint_t blkcnt);
> + * Return:   0 success, or -ve error number (see the IS_ERR_VALUE()) macro

The closing paren is misplaced - it should be '(see the IS_ERR_VALUE()
macro)'. Also '0 on success'.

Regards,
Simon

Reply via email to