Page MenuHomeFreeBSD

Updates to mpr.4 man page
ClosedPublic

Authored by slm on Apr 25 2016, 10:59 PM.

Details

Summary
  • Add 3216 and 3224 support.
  • Add SSU, chain_alloc_fail, and spinup_wait_time information.
  • Clear up some sentences.
  • Correct some typos.

Diff Detail

Lint
Lint Skipped
Unit
Unit Tests Skipped

Event Timeline

slm updated this revision to Diff 15608.Apr 25 2016, 10:59 PM
slm retitled this revision from to Updates to mpr.4 man page.
slm updated this object.
slm edited the test plan for this revision. (Show Details)
slm added reviewers: ken, scottl, ambrisko, asomers, mav, allanjude.
slm set the repository for this revision to rS FreeBSD src repository.
ken accepted this revision.Apr 26 2016, 6:15 PM
ken edited edge metadata.

Looks good.

This revision is now accepted and ready to land.Apr 26 2016, 6:15 PM
wblock added a subscriber: wblock.Apr 27 2016, 10:01 PM
wblock added inline comments.
share/man/man4/mpr.4
41 ↗(On Diff #15608)

Please bump this.

48 ↗(On Diff #15608)

This sentence mentions the kernel three times now.

"if not already in the kernel that you are using" can be assumed from context.

"the following" is almost always used incorrectly, usually "these" is better.

Avoid "you" and "your" (https://www.freebsd.org/doc/en_US.ISO8859-1/books/fdp-primer/writing-style.html#writing-style-tips)

So, I suggest:

To compile this driver into the kernel, place these lines in the kernel configuration file:
57 ↗(On Diff #15608)

Simpler:

The driver can be loaded as a module at boot time by placing this line in

("Alternatively" and "if not already loaded" are not needed, if the module is already part of the kernel there is only a non-fatal warning.)

137 ↗(On Diff #15608)

Please start new sentences on new lines.

139 ↗(On Diff #15608)

"may" is for permission, "can" is for possibility. "via" still means "a route" to me, but I know that not everyone agrees.

Also, "can be seen" seems a little weird. The command is what shows them, but the value is actually stored in the variable, not shown.

The current number of free chain frames is stored in the
144 ↗(On Diff #15608)

As above (but avoiding repeating "seen"):

The lowest number of free chain frames seen since boot is stored in the
150 ↗(On Diff #15608)

As above:

is stored in the
153 ↗(On Diff #15608)

Please start new sentences on new lines.
Also, s/if/whether/

161 ↗(On Diff #15608)

s/is shown in/is stored in/

192 ↗(On Diff #15608)

No comma needed here. It would not hurt to break that into two sentences, though:

SCSI command to SATA direct-access devices during shutdown.
This allows the
193 ↗(On Diff #15608)

I would say "before powering down."

201 ↗(On Diff #15608)

As above:
s/the following/these/

206 ↗(On Diff #15608)

Avoid semicolons, the rutabaga of the punctuation world.

Send SSU to SSDs, but not to HDDs.
This is the default value.
213 ↗(On Diff #15608)

As above:
s/the following/this/

221 ↗(On Diff #15608)

s/are valid as/are valid/

223 ↗(On Diff #15608)

This sentence is repetitive, using "failed" for several things. Also "may" versus "can". How about:

SATA disks that take several seconds to spin up and fail the SATA Identify command might not be discovered by the driver.
225 ↗(On Diff #15608)

"The following" is usually best avoided. Also, "spinup" versus "spin up".

This problem can sometimes be overcome by increasing the value of the spinup wait time in
.Xr loader.conf 5
with the
.Bd -literal -offset indent
hw.mpr.spinup_wait_time
​.Ed
​.Pp
tunable.
The value is the number of seconds to wait for SATA devices to spin up when
the device fails the initial SATA identify command.
236 ↗(On Diff #15608)

Try to avoid if/then sentences with a pause (comma). Better to have declarative sentences.
Also, no "the following".

Spinup wait times can be set for specific adapters in
.Xr loader.conf 5
with the
​.Bd -literal -offset indent
​dev.mpr.X.spinup_wait_time
​.Ed
​.Pp
tunable.
X represents the adapter number, and the tunable value is the number of seconds
to wait for SATA devices to spin up when they fail the initial SATA Identify command.
slm added a comment.Apr 28 2016, 5:20 PM

Wow. That's a lot of stuff. Who knew there could be so much wrong in such a little document :) I appreciate the feedback. I'll make the changes as suggested and put up a patch within a couple of days.

slm updated this revision to Diff 15744.Apr 29 2016, 9:32 PM
slm edited edge metadata.
slm removed rS FreeBSD src repository as the repository for this revision.

Updated with Warren's initial comments.

This revision now requires review to proceed.Apr 29 2016, 9:32 PM
ken accepted this revision.May 5 2016, 3:23 PM
ken edited edge metadata.
This revision is now accepted and ready to land.May 5 2016, 3:23 PM
wblock added a comment.May 8 2016, 4:58 AM

One small note, but otherwise looks great! Thank you for your patience!

mpr.4
185

Don't use an apostrophe for a plural, just "IDs".

slm added inline comments.May 9 2016, 3:43 PM
mpr.4
185

I'll remove before committing. Thanks Warren.

This revision was automatically updated to reflect the committed changes.