Page MenuHomeFreeBSD

iwm: lower probe priority
Needs ReviewPublic

Authored by emaste on Feb 20 2024, 8:20 PM.

Details

Reviewers
bz
cc
imp
Summary
iwlwifi(4) supports a superset of devices compared to iwm(4), and (in
the absense of bugs or missing features) we'd prefer to use a single
driver.  Lower iwm's priority so that iwlwifi will be used by default.

Diff Detail

Lint
Lint Skipped
Unit
Tests Skipped

Event Timeline

emaste created this revision.

I would like to add some language in both iwm.4 and iwlwifi.4 about this, as well as a note on how to choose one or the other (i.e., via module_blacklist) although I'm not sure exactly what to say.

Have you tested on a pre-22000 card 8xxx/9xxx recently?
I know we have to 2 PRs (well one now, two people) on those and given they use different interfaces within iwlwifi it's possible they do behave different.

iwlwifi.4 has some comment at the end about iwm(4).

The other suggestion I have is to leave this at DEFAULT and adjust iwlwifi to +1 or otherwise remove the special case from iwlwifi which effectively you want to reverse.

sys/contrib/dev/iwlwifi/pcie/drv.c: .bsd_probe_return = (BUS_PROBE_DEFAULT - 1),

I'm now running with this change on my daily driver, which has:

Feb 20 15:53:33 nazar kernel: iwlwifi0: Detected Intel(R) Wireless-AC 9560 160MHz, REV=0x312

The other suggestion I have is to leave this at DEFAULT and adjust iwlwifi to +1 or otherwise remove the special case from iwlwifi

I wasn't quite sure what the desired approach is but spotted this in bus.h:

                                  BUS_PROBE_LOW_PRIORITY are for drivers that
* have special requirements like when there are two drivers that support
* overlapping series of hardware devices.  In this case the one that supports
* the older part of the line would return this value, while the one that
* supports the newer ones would return BUS_PROBE_DEFAULT.

which sounds like exactly the case here I think. Thanks for pointing this out though, assuming this change is correct I'll need to remove the -1 from BUS_PROBE_DEFAULT - 1

I'm now running with this change on my daily driver, which has:

Feb 20 15:53:33 nazar kernel: iwlwifi0: Detected Intel(R) Wireless-AC 9560 160MHz, REV=0x312

Cool. Thanks. That's valuable feedback given https://bugs.freebsd.org/bugzilla/show_bug.cgi?id=275255 .

The other suggestion I have is to leave this at DEFAULT and adjust iwlwifi to +1 or otherwise remove the special case from iwlwifi

I wasn't quite sure what the desired approach is but spotted this in bus.h:

                                  BUS_PROBE_LOW_PRIORITY are for drivers that
* have special requirements like when there are two drivers that support
* overlapping series of hardware devices.  In this case the one that supports
* the older part of the line would return this value, while the one that
* supports the newer ones would return BUS_PROBE_DEFAULT.

which sounds like exactly the case here I think. Thanks for pointing this out though, assuming this change is correct I'll need to remove the -1 from BUS_PROBE_DEFAULT - 1

I think in that case you remove the three lines from iwlwifi so we are on default there too in addition to the change here.
It was a (long discussed) change back then as we couldn't simply change the default in `linux_pci_probe()` but allows us to be flexible per-driver with the "FreeBSD extension" to `struct pci_driver```

diff --git sys/contrib/dev/iwlwifi/pcie/drv.c sys/contrib/dev/iwlwifi/pcie/drv.c
index 0aab7a1fdc8e..d266e0b8afac 100644
--- sys/contrib/dev/iwlwifi/pcie/drv.c
+++ sys/contrib/dev/iwlwifi/pcie/drv.c
@@ -1587,10 +1587,6 @@ static struct pci_driver iwl_pci_driver = {
        .probe = iwl_pci_probe,
        .remove = iwl_pci_remove,
        .driver.pm = IWL_PM_OPS,
-#if defined(__FreeBSD__)
-       /* Allow iwm(4) to attach for conflicting IDs for now. */
-       .bsd_probe_return = (BUS_PROBE_DEFAULT - 1),
-#endif
 };
 
 int __must_check iwl_pci_register_driver(void)

I also have a preferred driver overload in newbus for this...
But we likely need to gerneralize it...
Specifying the values in code can't easily be overridden... unless the drivers are both modules and you blocklist one of them...

Another thing (apart from man pages) we should probably make sure works out of the box is suspend/resume as that's the other missing bit (and ideally without workarounds). I need to go back and see if @hrs has some USB-DEBUG serial console code (I think he gave a presentation about it a while ago at Euro? but I lost track) and assume he doesn't have any time currently.

Here's a second one showing the NONEXIST problem with older firmware https://bugs.freebsd.org/bugzilla/show_bug.cgi?id=277432
I think it would be good to sort that before moving people from iwm to iwlwifi by default -- or at least being prepared to handle the fallout.