>
>
> maybe handle channel 2 here as well, ie.:
> .center_freq = ((_channel) == 2) ? 5935 : 5950 + (5 * (_channel)),
>
> > + .hw_value = (_channel), \
> > + .max_antenna_gain = 0, \
> > + .max_power = 30, \
> > +}
>
> so we can drop this one below...
>
Will do.
>
>
> > + /* We ignore this BSS ID rather than try to continue on.
> > + * Otherwise we will cause an OOPs because our frequency is 0.
> > + * The main case this occurs is some new frequency band
> > + * we have not seen before, and if we return an error,
> > + * we will cause the scan to fail. It seems better to
> > + * report the error, skip this BSS, and move on.
> > + */
> > + return 0;
> > + }
> > bss_data.chan = ieee80211_get_channel(wiphy, freq);
>
> How could this fail? Our wiphy registers all possible channels so if
> ieee80211_channel_to_frequency() succeeds ieee80211_get_channel() can
> not fail.
I agree.
>
>
> > @@ -6965,6 +7066,10 @@ static int brcmf_construct_chaninfo(struct brcmf_cfg80211_info *cfg,
> > for (i = 0; i < band->n_channels; i++)
> > band->channels[i].flags = IEEE80211_CHAN_DISABLED;
> > band = wiphy->bands[NL80211_BAND_5GHZ];
> > + if (band)
>
> Eh. Why is this conditional? We are creating all bands in the wiphy
> instance so why the null check here?
I just matched what was there, I can remove all of them if we want.
(I'll take care of the rest of the comments between here)
>
> > - brcmf_dbg(INFO, "nmode=%d, vhtmode=%d, bw_cap=(%d, %d)\n",
> > + brcmf_dbg(INFO,
> > + "nmode=%d, vhtmode=%d, bw_cap=(%d, %d, %d), he_cap=(%d, %d)\n",
> > nmode, vhtmode, bw_cap[NL80211_BAND_2GHZ],
> > - bw_cap[NL80211_BAND_5GHZ]);
> > + bw_cap[NL80211_BAND_5GHZ], bw_cap[NL80211_BAND_6GHZ],
> > + he_cap[0], he_cap[1]);
>
> So are these he mac and phy capabilities? ...
No, unfortunately, it's either 1 or 0 on these chips, and all chips i tested.
This is the hardware capability iovar.
In the debug firmware i have access to (not apple's), i do see a
command that looks like it may give the he cap, but i can't find how
it would ever be triggered.
(The iovar code for the iovar above is either always just return 0 or return 1)
There are no obvious iovars that relate, and the absolute latest
bcmdhd hardcodes the he caps, as do infineon's latest ifx code.
:(
I'l hack around see if i can get the caps out of it.
I'll double check other ones.
> > @@ -8331,18 +8589,21 @@ struct brcmf_cfg80211_info *brcmf_cfg80211_attach(struct brcmf_pub *drvr,
> > if (brcmf_feat_is_enabled(ifp, BRCMF_FEAT_DUMP_OBSS))
> > ops->dump_survey = brcmf_cfg80211_dump_survey;
> >
> > - err = wiphy_register(wiphy);
> > - if (err < 0) {
> > - bphy_err(drvr, "Could not register wiphy device (%d)\n", err);
> > - goto priv_out;
> > - }
> > -
> > + /* We have to configure the bands before we register the wiphy device
> > + * because it requires that band capabilities be correct.
> > + */
>
> Is it?
If you register the 6g band without he_cap set, 80211 is unhappy.
It sanity checks the bands in wiphy_register, and we get caught in the
HE supported check for 6g.
See here:
https://elixir.bootlin.com/linux/latest/source/net/wireless/core.c#L823
In general, you can see it sanity checks the bands/etc as part of
registration. It happens that 6g triggers the he one, but it seems in
general the bands are supposed to be sane before registration.
> The order was deliberate. brcmf_setup_wiphybands() calls
> brcmf_construct_chaninfo() which disables all channels. When you do that
> before wiphy_register() the orig_flags of the channel will be DISABLED
> and can never be used.
I'll take care of this by copying the flags around.
>
> > err = brcmf_setup_wiphybands(cfg);
> > if (err) {
> > bphy_err(drvr, "Setting wiphy bands failed (%d)\n", err);
> > goto wiphy_unreg_out;
> > }
> >
> > + err = wiphy_register(wiphy);
> > + if (err < 0) {
> > + bphy_err(drvr, "Could not register wiphy device (%d)\n", err);
> > + goto priv_out;
> > + }
> > +
> > /* If cfg80211 didn't disable 40MHz HT CAP in wiphy_register(),
> > * setup 40MHz in 2GHz band and enable OBSS scanning.
> > */
>
