Page MenuHomeFreeBSD

mfi(4)/mrsas(4): Print driver versions consistently
Needs ReviewPublic

Authored by michaelo on Mon, Jan 5, 11:58 AM.
Tags
None
Referenced Files
F141876182: D54519.diff
Sun, Jan 11, 8:45 PM
Unknown Object (File)
Fri, Jan 9, 8:32 PM
Unknown Object (File)
Fri, Jan 9, 5:24 AM
Unknown Object (File)
Fri, Jan 9, 2:38 AM
Unknown Object (File)
Wed, Jan 7, 1:20 AM
Unknown Object (File)
Tue, Jan 6, 8:08 AM
F141413616: Unbenannt.jpg
Mon, Jan 5, 11:59 AM
Subscribers

Details

Reviewers
ziaee
emaste
imp
Summary

MFC after: 3 days

Diff Detail

Repository
rG FreeBSD src repository
Lint
Lint Skipped
Unit
Tests Skipped
Build Status
Buildable 69777
Build 66660: arc lint + arc unit

Event Timeline

Screenshot from the machine with both drivers now side by side:

Unbenannt.jpg (960×1 px, 262 KB)

Thanks for tagging me!

I try to be careful with "FreeBSD" as a search term, I don't think it's appropriate in these driver strings. I also don't know if it matters that it's broadcom megaraid now.

Maybe imp should review this. I'm going to add emaste because he's one marked up the broadcom.

Thanks for tagging me!

I try to be careful with "FreeBSD" as a search term, I don't think it's appropriate in these driver strings. I also don't know if it matters that it's broadcom megaraid now.

Maybe imp should review this. I'm going to add emaste because he's one marked up the broadcom.

I *intentionally* did not write Broadcom because the driver was developed when the company still was LSI, then bought by AVAGO which is now named Broadcom:

MFI(4)                 FreeBSD Kernel Interfaces Manual                 MFI(4)

NAME
     mfi – LSI MegaRAID SAS driver

This is fully in line with the driver intro.

@imp Any objections or opinions here?

device_printf will include the device prefix mfi so I'm not sure why it needs to be repeated, and I'm also not sure what value including FreeBSD in the message brings.
Including LSI, correctly capitalizing MegaRAID, and expanding version all sound good to me.

LSI hasn't owned or marked this for a decade.

correctly capitalizing MegaRAID, and expanding version all sound good to me.

I also agree with this, unsure/no-opinion on LSI.

I had the following objective:

  • Make the string compatible with mrsas(4) since both drivers overlap
  • Reflect the author of the driver back then, not now
  • Keep mfi in the script because it makes the string consistent with the first point

What is your proposition?

LSI MegaRAID FreeBSD SAS driver version: %s

That would be OK because of the prefix. Unfortunately, mrsas(4) does not print the device prefix and stops at the first device which is inconsistent with mfi(4).

I think if you want to make them the same you should remove FreeBSD from both. If people search logs for FreeBSD they shouldn't get these drivers, I think.

I think if you want to make them the same you should remove FreeBSD from both. If people search logs for FreeBSD they shouldn't get these drivers, I think.

That would be fine for me as long as we (all) agree upon.

I think if you want to make them the same you should remove FreeBSD from both. If people search logs for FreeBSD they shouldn't get these drivers, I think.

Done in https://reviews.freebsd.org/D54630

michaelo edited the summary of this revision. (Show Details)

Drop 'FreeBSD' in version banner

Unify both changes, use device_printf() in mrsas(4) as well

Will test this also on the server next week

michaelo retitled this revision from mfi(4): Print version consistent with mrsas(4) to mfi(4)/mrsas(4): Print driver versions consistently.

Looking at booth drivers, mfi(4) uses mfi_attach() to print the message while mrsas(4) uses probe. I could move both to *_attach(), but cannot tell which of both is correct. Logically attach() seems right.

print driver version in *_attach() function