From: Peter Hessler Subject: Re: iwx: avoid adding unused firmware PHY contexts To: tech@openbsd.org Date: Tue, 10 Mar 2026 11:52:23 +0100 On 2026 Mar 10 (Tue) at 09:31:01 +0100 (+0100), Stefan Sperling wrote: :The Linux iwlwifi driver no longer adds unused firmware PHY contexts. :See Linux commit f3276ff0d498a364dfdff74cc1825b5f6e27f472 from 2023. : :The reason given in the description of that change is that "newer" firmware :runs calibration when a PHY context is added. I assume that adding a PHY :context which is unused either wastes time or could lead to miscalibration. : :I am not sure which devices are covered by "newer". Both MA and BZ seem :recent enough that their firmware might have the new behaviour already. : :The change does not break AX200 and works fine on BZ, so why not. : :ok? : Tested on AX211, OK :M sys/dev/pci/if_iwx.c | 44+ 36- :M sys/dev/pci/if_iwxreg.h | 1+ 1- :M sys/dev/pci/if_iwxvar.h | 1+ 0- : :3 files changed, 46 insertions(+), 37 deletions(-) : :commit - 826ff44182208595e7c4d3a6c8d1b90f55e9b509 :commit + 7927c7445c653ee155b4f3a5542afce66f21b158 :blob - 577938554f636baf626d39330efb85c24eda13eb :blob + ea3d57e1b24f28f4bcbe740ed06949030b03aab2 :--- sys/dev/pci/if_iwx.c :+++ sys/dev/pci/if_iwx.c :@@ -8614,19 +8614,31 @@ iwx_auth(struct iwx_softc *sc) : : splassert(IPL_NET); : :- if (ic->ic_opmode == IEEE80211_M_MONITOR) { :- err = iwx_phy_ctxt_update(sc, &sc->sc_phyctxt[0], :- ic->ic_ibss_chan, 1, 1, 0, IEEE80211_HTOP0_SCO_SCN, :- IEEE80211_VHTOP0_CHAN_WIDTH_HT); :- if (err) :- return err; :- } else { :- err = iwx_phy_ctxt_update(sc, &sc->sc_phyctxt[0], :- in->in_ni.ni_chan, 1, 1, 0, IEEE80211_HTOP0_SCO_SCN, :- IEEE80211_VHTOP0_CHAN_WIDTH_HT); :- if (err) :- return err; :+ if (ic->ic_opmode == IEEE80211_M_MONITOR) :+ sc->sc_phyctxt[0].channel = ic->ic_ibss_chan; :+ else :+ sc->sc_phyctxt[0].channel = in->in_ni.ni_chan; :+ :+ err = iwx_phy_ctxt_cmd(sc, &sc->sc_phyctxt[0], 1, 1, :+ IWX_FW_CTXT_ACTION_ADD, 0, IEEE80211_HTOP0_SCO_SCN, :+ IEEE80211_VHTOP0_CHAN_WIDTH_HT); :+ if (err) { :+ printf("%s: could not add phy context (error %d)\n", :+ DEVNAME(sc), err); :+ return err; : } :+ sc->sc_flags |= IWX_FLAG_PHY_ACTIVE; :+ :+ if (iwx_lookup_cmd_ver(sc, IWX_DATA_PATH_GROUP, :+ IWX_RLC_CONFIG_CMD) == 2) { :+ err = iwx_phy_send_rlc(sc, &sc->sc_phyctxt[0], 1, 1); :+ if (err) { :+ printf("%s: could not configure RLC for PHY " :+ "(error %d)\n", DEVNAME(sc), err); :+ goto rm_phy_ctxt; :+ } :+ } :+ : in->in_phyctxt = &sc->sc_phyctxt[0]; : IEEE80211_ADDR_COPY(in->in_macaddr, in->in_ni.ni_macaddr); : :@@ -8634,7 +8646,7 @@ iwx_auth(struct iwx_softc *sc) : if (err) { : printf("%s: could not add MAC context (error %d)\n", : DEVNAME(sc), err); :- return err; :+ goto rm_phy_ctxt; : } : sc->sc_flags |= IWX_FLAG_MAC_ACTIVE; : :@@ -8698,6 +8710,13 @@ rm_mac_ctxt: : iwx_mac_ctxt_cmd(sc, in, IWX_FW_CTXT_ACTION_REMOVE, 0); : sc->sc_flags &= ~IWX_FLAG_MAC_ACTIVE; : } :+rm_phy_ctxt: :+ if (generation == sc->sc_generation) { :+ iwx_phy_ctxt_cmd(sc, &sc->sc_phyctxt[0], 1, 1, :+ IWX_FW_CTXT_ACTION_REMOVE, 0, IEEE80211_HTOP0_SCO_SCN, :+ IEEE80211_VHTOP0_CHAN_WIDTH_HT); :+ sc->sc_flags &= ~IWX_FLAG_PHY_ACTIVE; :+ } : return err; : } : :@@ -8739,12 +8758,17 @@ iwx_deauth(struct iwx_softc *sc) : sc->sc_flags &= ~IWX_FLAG_MAC_ACTIVE; : } : :- /* Move unused PHY context to a default channel. */ :- err = iwx_phy_ctxt_update(sc, &sc->sc_phyctxt[0], :- &ic->ic_channels[1], 1, 1, 0, IEEE80211_HTOP0_SCO_SCN, :- IEEE80211_VHTOP0_CHAN_WIDTH_HT); :- if (err) :- return err; :+ if (sc->sc_flags & IWX_FLAG_PHY_ACTIVE) { :+ err = iwx_phy_ctxt_cmd(sc, &sc->sc_phyctxt[0], 1, 1, :+ IWX_FW_CTXT_ACTION_REMOVE, 0, IEEE80211_HTOP0_SCO_SCN, :+ IEEE80211_VHTOP0_CHAN_WIDTH_HT); :+ if (err) { :+ printf("%s: could not remove PHY context (error %d)\n", :+ DEVNAME(sc), err); :+ return err; :+ } :+ sc->sc_flags &= ~IWX_FLAG_PHY_ACTIVE; :+ } : : return 0; : } :@@ -9751,23 +9775,6 @@ iwx_init_hw(struct iwx_softc *sc) : */ : sc->sc_phyctxt[i].id = i; : sc->sc_phyctxt[i].channel = &ic->ic_channels[1]; :- err = iwx_phy_ctxt_cmd(sc, &sc->sc_phyctxt[i], 1, 1, :- IWX_FW_CTXT_ACTION_ADD, 0, IEEE80211_HTOP0_SCO_SCN, :- IEEE80211_VHTOP0_CHAN_WIDTH_HT); :- if (err) { :- printf("%s: could not add phy context %d (error %d)\n", :- DEVNAME(sc), i, err); :- goto err; :- } :- if (iwx_lookup_cmd_ver(sc, IWX_DATA_PATH_GROUP, :- IWX_RLC_CONFIG_CMD) == 2) { :- err = iwx_phy_send_rlc(sc, &sc->sc_phyctxt[i], 1, 1); :- if (err) { :- printf("%s: could not configure RLC for PHY " :- "%d (error %d)\n", DEVNAME(sc), i, err); :- goto err; :- } :- } : } : : err = iwx_config_ltr(sc); :@@ -10079,6 +10086,7 @@ iwx_stop(struct ifnet *ifp) : sc->sc_flags &= ~IWX_FLAG_HW_ERR; : sc->sc_flags &= ~IWX_FLAG_SHUTDOWN; : sc->sc_flags &= ~IWX_FLAG_TXFLUSH; :+ sc->sc_flags &= ~IWX_FLAG_PHY_ACTIVE; : : sc->sc_rx_ba_sessions = 0; : sc->ba_rx.start_tidmask = 0; :blob - f8af0bb93308432b9aea90c88f2aacca45d6d7b4 :blob + fe32d0f9fd66320aeee03a80cc9007494680fa9e :--- sys/dev/pci/if_iwxreg.h :+++ sys/dev/pci/if_iwxreg.h :@@ -3395,7 +3395,7 @@ struct iwx_fw_channel_info { : (0x1 << IWX_PHY_RX_CHAIN_MIMO_FORCE_POS) : : /* TODO: fix the value, make it depend on firmware at runtime? */ :-#define IWX_NUM_PHY_CTX 3 :+#define IWX_NUM_PHY_CTX 1 : : /** : * struct iwl_phy_context_cmd - config of the PHY context :blob - e7fdcd85769cfce5e8b5e86eae972009626fcbaa :blob + c42e0a4a296c22a456c5ed307af4f04e1689d3f3 :--- sys/dev/pci/if_iwxvar.h :+++ sys/dev/pci/if_iwxvar.h :@@ -290,6 +290,7 @@ struct iwx_rx_ring { : #define IWX_FLAG_SHUTDOWN 0x100 /* shutting down; new tasks forbidden */ : #define IWX_FLAG_BGSCAN 0x200 /* background scan in progress */ : #define IWX_FLAG_TXFLUSH 0x400 /* Tx queue flushing in progress */ :+#define IWX_FLAG_PHY_ACTIVE 0x800 /* PHY context added to firmware */ : : struct iwx_ucode_status { : uint32_t uc_lmac_error_event_table[2]; : -- "In Christianity neither morality nor religion come into contact with reality at any point." -- Friedrich Nietzsche