[PATCH v1 2/5] drivers: nvme: Implement NVMe flush command (0x0)

[Neil Armstrong]

On 5/29/26 05:36, dmukhin at ford.com wrote:
> On Thu, May 28, 2026 at 11:28:33AM +0200, Neil Armstrong wrote:
>> Hi,
>>
>> On 5/28/26 03:20, dmukhin at ford.com wrote:
>>> From: Denis Mukhin <dmukhin at ford.com>
>>>
>>> Add flush command implementation along with a high-level
>>> disk_blk_flush() API to be called at certain checkpoints
>>> during the boot (e.g. updating custom bootflow flags stored
>>> on NVMe).
>>
>> First general comment, please split the patch by adding the
>> flush infrastructure first and then the nvme support.
> 
> Will do, thanks.
> 
>>
>> The for NVMe, can it support write-through like SCSI FUA (Force Unit
>> Access) which bypasses the cache on writes, with slower writes
>> but safer and simpler than calling a flush before OS boot.
> 
> Yes, I've seen there was FUA patch [1], which was then reverted [2].
> 
> For our purposes, flush seems to be enough: the use case is
> updating NVMe-backed boot counter from a custom bootflow.

OK the FUA was reverted because Bin's comment was not adressed:
https://lore.kernel.org/u-boot/CAEUhbmU02GnqEEfuqNrSj8R5XEZkmkTUaLp1+BMbYQNdzTUZ7A@mail.gmail.com/

I would prefer having FUA added so all board could have this enabled
and not have to update their boot flow to call flush, this was basically
why the SCSI FUA was enabled.

As Bin's comment, perhaps a way would be to disable the Write Cache if
the cache is present as init time (WCE), and it's even simpler, or
get the enable state of the WC and use FUA if enabled.

Because the same question remains with the flush command, it's
only valid if the WC is present and is enabled.

Thanks,
Neil

> 
> [1] https://lore.kernel.org/u-boot/20211019104049.v3.1.Ic581ec99f46b6dfa2e0b1922e670a333ac859e82@changeid/
> [2] https://lore.kernel.org/u-boot/CAGi-RUL96GK5nfHx73UQQd_4sFByt3gNwqhFpHps+h58S20JwQ@mail.gmail.com/
> 
> [..]
>>>    /**
>>>     * blk_find_device() - Find a block device
>>>     *
>>> @@ -559,6 +577,16 @@ static inline ulong blk_derase(struct blk_desc *block_dev, lbaint_t start,
>>>    	return block_dev->block_erase(block_dev, start, blkcnt);
>>>    }
>>> +static inline ulong blk_dflush(struct blk_desc *block_dev)
>>> +{
>>> +	blkcache_invalidate(block_dev->uclass_id, block_dev->devnum);
>>
>> Call blkcache_invalidate only if flush cmd is supported
> 
> Ack.
> 
>>
>>> +
>>> +	if (block_dev->block_flush)
>>> +		return block_dev->block_flush(block_dev);
>>> +
>>> +	return 0;
>>
>> Why do you return success here and not from blk_flush() when flush is not supported?
> 
> Will update.
> 
> 
> Thanks,
> Denis