Re: [PATCH v3 1/2] nvme: Enable FUA
On 2021/11/19 9:14, Tom Rini wrote: On Fri, Nov 19, 2021 at 08:56:08AM +0800, Bin Meng wrote: Hi Tom, On Fri, Nov 19, 2021 at 3:14 AM Tom Rini wrote: On Tue, Oct 19, 2021 at 10:40:53AM +0800, Jon Lin wrote: Most NVME devcies maintain data in internal cache for an uncertain times, and u-boot has no method to force NVME to flush cache. So this patch adds FUA to avoid data loss caused by power off after data programming. Signed-off-by: Jon Lin Reviewed-by: Stefan Agner Applied to u-boot/next, thanks! I don't see my review comment being addressed. Please drop the patch until all things are clear. Missed your comments, sorry. Thanks to bin Meng and Tom. I'm not familiar with nvme protocol. I expect people with nvme experience to take over the patch and do the most reasonable treatment. Of course, I'll take some time to seriously ponder the nvme process to see if it can meet your requirements
Re: [PATCH v3 1/2] nvme: Enable FUA
On Fri, Nov 19, 2021 at 08:56:08AM +0800, Bin Meng wrote: > Hi Tom, > > On Fri, Nov 19, 2021 at 3:14 AM Tom Rini wrote: > > > > On Tue, Oct 19, 2021 at 10:40:53AM +0800, Jon Lin wrote: > > > > > Most NVME devcies maintain data in internal cache for an uncertain > > > times, and u-boot has no method to force NVME to flush cache. > > > So this patch adds FUA to avoid data loss caused by power off after data > > > programming. > > > > > > Signed-off-by: Jon Lin > > > Reviewed-by: Stefan Agner > > > > Applied to u-boot/next, thanks! > > I don't see my review comment being addressed. Please drop the patch > until all things are clear. Missed your comments, sorry. -- Tom signature.asc Description: PGP signature
Re: [PATCH v3 1/2] nvme: Enable FUA
Hi Tom, On Fri, Nov 19, 2021 at 3:14 AM Tom Rini wrote: > > On Tue, Oct 19, 2021 at 10:40:53AM +0800, Jon Lin wrote: > > > Most NVME devcies maintain data in internal cache for an uncertain > > times, and u-boot has no method to force NVME to flush cache. > > So this patch adds FUA to avoid data loss caused by power off after data > > programming. > > > > Signed-off-by: Jon Lin > > Reviewed-by: Stefan Agner > > Applied to u-boot/next, thanks! I don't see my review comment being addressed. Please drop the patch until all things are clear. Regards, Bin
Re: [PATCH v3 1/2] nvme: Enable FUA
On Tue, Oct 19, 2021 at 10:40:53AM +0800, Jon Lin wrote: > Most NVME devcies maintain data in internal cache for an uncertain > times, and u-boot has no method to force NVME to flush cache. > So this patch adds FUA to avoid data loss caused by power off after data > programming. > > Signed-off-by: Jon Lin > Reviewed-by: Stefan Agner Applied to u-boot/next, thanks! -- Tom signature.asc Description: PGP signature
Re: [PATCH v3 1/2] nvme: Enable FUA
Hi Jon,
On Tue, Oct 19, 2021 at 10:41 AM Jon Lin wrote:
>
> Most NVME devcies maintain data in internal cache for an uncertain
> times, and u-boot has no method to force NVME to flush cache.
> So this patch adds FUA to avoid data loss caused by power off after data
> programming.
>
> Signed-off-by: Jon Lin
> Reviewed-by: Stefan Agner
> ---
>
> Changes in v3:
> Only enable FUA when vwc is enabled
>
> drivers/nvme/nvme.c | 4
> 1 file changed, 4 insertions(+)
>
> diff --git a/drivers/nvme/nvme.c b/drivers/nvme/nvme.c
> index 3c529a2fce..9623c896a1 100644
> --- a/drivers/nvme/nvme.c
> +++ b/drivers/nvme/nvme.c
> @@ -762,6 +762,10 @@ static ulong nvme_blk_rw(struct udevice *udev, lbaint_t
> blknr,
> c.rw.appmask = 0;
> c.rw.metadata = 0;
>
> + /* Enable FUA for data integrity if vwc is enabled */
> + if (dev->vwc)
Still this does not look correct to me.
The dev->vwc field only indicates the presence of the Volatile Write
Cache, but not the enable status of it.
> + c.rw.control |= NVME_RW_FUA;
> +
> while (total_lbas) {
> if (total_lbas < lbas) {
> lbas = (u16)total_lbas;
Regards,
Bin
[PATCH v3 1/2] nvme: Enable FUA
Most NVME devcies maintain data in internal cache for an uncertain
times, and u-boot has no method to force NVME to flush cache.
So this patch adds FUA to avoid data loss caused by power off after data
programming.
Signed-off-by: Jon Lin
Reviewed-by: Stefan Agner
---
Changes in v3:
Only enable FUA when vwc is enabled
drivers/nvme/nvme.c | 4
1 file changed, 4 insertions(+)
diff --git a/drivers/nvme/nvme.c b/drivers/nvme/nvme.c
index 3c529a2fce..9623c896a1 100644
--- a/drivers/nvme/nvme.c
+++ b/drivers/nvme/nvme.c
@@ -762,6 +762,10 @@ static ulong nvme_blk_rw(struct udevice *udev, lbaint_t
blknr,
c.rw.appmask = 0;
c.rw.metadata = 0;
+ /* Enable FUA for data integrity if vwc is enabled */
+ if (dev->vwc)
+ c.rw.control |= NVME_RW_FUA;
+
while (total_lbas) {
if (total_lbas < lbas) {
lbas = (u16)total_lbas;
--
2.17.1
