MFC after: 3 days
Diff Detail
- Repository
- rG FreeBSD src repository
- Lint
Lint Skipped - Unit
Tests Skipped - Build Status
Buildable 69621 Build 66504: arc lint + arc unit
Event Timeline
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 driverThis is fully in line with the driver intro.
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.
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.
Unify both changes, use device_printf() in mrsas(4) as well
Will test this also on the server next week
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.
I ran this and it looks good now:
root@deblndw014x:~ # dmesg | grep mrsas0 mrsas0: <AVAGO Thunderbolt SAS Controller> port 0x8000-0x80ff mem 0xdfa60000-0xdfa63fff,0xdfa00000-0xdfa3ffff irq 26 at device 0.0 on pci1 mrsas0: FW now in Ready state mrsas0: Using MSI-X with 16 number of vectors mrsas0: FW supports <16> MSIX vector,Online CPU 32 Current MSIX <16> mrsas0: mrsas_init_adapter: sc->reply_q_depth 0x7e0,sc->request_alloc_sz 0x1f78, sc->reply_alloc_sz 0x3f00,sc->io_frames_alloc_sz 0x3f100 mrsas0: max sge: 0x46, max chain frame size: 0x400, max fw cmd: 0x3ef sc->chain_frames_alloc_sz: 0xfbc00 mrsas0: Issuing IOC INIT command to FW. mrsas0: IOC INIT response received from FW.
The conceptional change (attach instead of probe) is that it is only displayed when the driver is used and the device is attached. Before, mrsas was just probed and how a lower pref than mfi. At the end, there is no behavior change in the driver itself, but only a change of the banner.

