| Message ID | 20210926171143.1.Ic581ec99f46b6dfa2e0b1922e670a333ac859e82@changeid |
|---|---|
| State | Superseded |
| Delegated to: | Tom Rini |
| Headers | show |
| Series | nvme: Enable FUA | expand |
On 2021-09-26 11:12, 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. Maybe worth mentioning that FUA stands for Force Unit Access and makes sure that data has been written to non-volatile storage before command completion. It seems to me that volatile cache needs to be explicitly enabled by the Volatile Write Cache feature. But I guess it could be already enabled by earlier firmware/on reboot? Setting the FUA bit will make certain that data land on non-volatile storage in any case. So: Reviewed-by: Stefan Agner <stefan@agner.ch> Out of curiosity, has this been a problem in a real world situation? -- Stefan > > Signed-off-by: Jon Lin <jon.lin@rock-chips.com> > --- > > drivers/nvme/nvme.c | 3 +++ > 1 file changed, 3 insertions(+) > > diff --git a/drivers/nvme/nvme.c b/drivers/nvme/nvme.c > index f6465ea7f4..5d05cb6e9e 100644 > --- a/drivers/nvme/nvme.c > +++ b/drivers/nvme/nvme.c > @@ -761,6 +761,9 @@ static ulong nvme_blk_rw(struct udevice *udev, > lbaint_t blknr, > c.rw.appmask = 0; > c.rw.metadata = 0; > > + /* Always enable FUA for data integrity */ > + c.rw.control |= NVME_RW_FUA; > + > while (total_lbas) { > if (total_lbas < lbas) { > lbas = (u16)total_lbas;
On 2021/9/27 20:28, Stefan Agner wrote: > On 2021-09-26 11:12, 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. > Maybe worth mentioning that FUA stands for Force Unit Access and makes > sure that data has been written to non-volatile storage before command > completion. > > It seems to me that volatile cache needs to be explicitly enabled by the > Volatile Write Cache feature. But I guess it could be already enabled by > earlier firmware/on reboot? Setting the FUA bit will make certain that > data land on non-volatile storage in any case. So: > > Reviewed-by: Stefan Agner <stefan@agner.ch> > > Out of curiosity, has this been a problem in a real world situation? We are trying to interact with the host computer in the device u-boot stage, involving nvme upgrade and device reset. When the two progress are continuous, there are corresponding problems. > -- > Stefan > >> Signed-off-by: Jon Lin <jon.lin@rock-chips.com> >> --- >> >> drivers/nvme/nvme.c | 3 +++ >> 1 file changed, 3 insertions(+) >> >> diff --git a/drivers/nvme/nvme.c b/drivers/nvme/nvme.c >> index f6465ea7f4..5d05cb6e9e 100644 >> --- a/drivers/nvme/nvme.c >> +++ b/drivers/nvme/nvme.c >> @@ -761,6 +761,9 @@ static ulong nvme_blk_rw(struct udevice *udev, >> lbaint_t blknr, >> c.rw.appmask = 0; >> c.rw.metadata = 0; >> >> + /* Always enable FUA for data integrity */ >> + c.rw.control |= NVME_RW_FUA; >> + >> while (total_lbas) { >> if (total_lbas < lbas) { >> lbas = (u16)total_lbas; > >
diff --git a/drivers/nvme/nvme.c b/drivers/nvme/nvme.c index f6465ea7f4..5d05cb6e9e 100644 --- a/drivers/nvme/nvme.c +++ b/drivers/nvme/nvme.c @@ -761,6 +761,9 @@ static ulong nvme_blk_rw(struct udevice *udev, lbaint_t blknr, c.rw.appmask = 0; c.rw.metadata = 0; + /* Always enable FUA for data integrity */ + c.rw.control |= NVME_RW_FUA; + while (total_lbas) { if (total_lbas < lbas) { lbas = (u16)total_lbas;
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 <jon.lin@rock-chips.com> --- drivers/nvme/nvme.c | 3 +++ 1 file changed, 3 insertions(+)