Page MenuHomeFreeBSD

nvme_ctrlr_enable: Remove delays
ClosedPublic

Authored by imp on Oct 1 2021, 3:27 AM.
Tags
None
Referenced Files
Unknown Object (File)
Thu, Apr 11, 11:09 AM
Unknown Object (File)
Mar 7 2024, 4:20 PM
Unknown Object (File)
Jan 27 2024, 7:18 AM
Unknown Object (File)
Dec 25 2023, 4:53 PM
Unknown Object (File)
Dec 25 2023, 2:51 PM
Unknown Object (File)
Dec 20 2023, 9:58 AM
Unknown Object (File)
Dec 20 2023, 2:18 AM
Unknown Object (File)
Dec 5 2023, 6:30 PM
Subscribers

Details

Summary

Remove delays after writing the administrative queue registers. These
delays are from the very earliest days of the driver (they are in the
first commit) and were most likely vestiges of the Chatham NVMe
prototype card that was used to create this driver. Many of the
workarounds necessary for it aren't necessary for standards compliant
cards. The original driver had other areas marked for Chatham, but these
were not. They are unneeded. There's three lines of supporting evidence.

First, the NVMe standards make no mention of a delay time after these
registers are written. Second, the Linux driver doesn't have them, even
as an option. Third, all my nvme cards work w/o them.

To be safe, add a write barrier between setting up the admin queue and
enabling the controller.

Diff Detail

Repository
rG FreeBSD src repository
Lint
Lint Not Applicable
Unit
Tests Not Applicable

Event Timeline

imp requested review of this revision.Oct 1 2021, 3:27 AM

I also don't see a point in DELAY()'s. Just thinking would it be right to use bus_space_barrier() before cc write for some non-x86, since it depends on the other registers state?

This revision is now accepted and ready to land.Oct 1 2021, 2:19 PM

Doesn't build for me.

sys/dev/nvme/nvme_ctrlr.c
58
--- nvme_ctrlr.o ---
/usr/src/sys/dev/nvme/nvme_ctrlr.c:60:11: error: too few arguments to function call, expected 5, have 4
            flags);
                 ^
./x86/bus.h:1002:1: note: 'bus_space_barrier' declared here
bus_space_barrier(bus_space_tag_t tag __unused, bus_space_handle_t bsh __unused,
^
sys/dev/nvme/nvme_ctrlr.c
58

I'm an idiot... I got this too, fix it, but somehow uploaded the wrong one :(

this time with right barrier

This revision now requires review to proceed.Oct 1 2021, 4:38 PM
imp marked an inline comment as done.
This revision was not accepted when it landed; it landed in state Needs Review.Oct 1 2021, 4:56 PM
This revision was automatically updated to reflect the committed changes.