On 4 January 2017 at 21:07, Arend Van Spriel <arend.vanspriel@xxxxxxxxxxxx> wrote: > On 4-1-2017 18:58, Rafał Miłecki wrote: >> From: Rafał Miłecki <rafal@xxxxxxxxxx> >> >> There are some devices (e.g. Netgear R8000 home router) with one chipset >> model used for different radios, some of them limited to subbands. NVRAM >> entries don't contain any extra info on such limitations and firmware >> reports full list of channels to us. We need to store extra limitation >> info in DT to support such devices properly. >> >> Now there is a cfg80211 helper for reading such info use it in brcmfmac. >> This patch adds check for channel being disabled with orig_flags which >> is how this wiphy helper and wiphy_register work. >> >> Signed-off-by: Rafał Miłecki <rafal@xxxxxxxxxx> >> --- >> This patch should probably go through wireless-driver-next which is why >> it got weird number 4/3. I'm sending it just as a proof of concept. >> It was succesfully tested on SmartRG SR400ac with BCM43602. >> >> V4: Respect IEEE80211_CHAN_DISABLED in orig_flags >> V5: Update commit message >> V6: Call wiphy_read_of_freq_limits after brcmf_setup_wiphybands to make it work >> with helper setting "flags" instead of "orig_flags". >> --- >> drivers/net/wireless/broadcom/brcm80211/brcmfmac/cfg80211.c | 9 ++++++++- >> 1 file changed, 8 insertions(+), 1 deletion(-) >> >> diff --git a/drivers/net/wireless/broadcom/brcm80211/brcmfmac/cfg80211.c b/drivers/net/wireless/broadcom/brcm80211/brcmfmac/cfg80211.c >> index ccae3bb..a008ba5 100644 >> --- a/drivers/net/wireless/broadcom/brcm80211/brcmfmac/cfg80211.c >> +++ b/drivers/net/wireless/broadcom/brcm80211/brcmfmac/cfg80211.c >> @@ -5886,6 +5886,9 @@ static int brcmf_construct_chaninfo(struct brcmf_cfg80211_info *cfg, >> band->band); >> channel[index].hw_value = ch.control_ch_num; >> >> + if (channel->orig_flags & IEEE80211_CHAN_DISABLED) >> + continue; >> + > > So to be clear this is still needed for subsequent calls to > brcmf_setup_wiphybands(). The subsequent calls are done from the > regulatory notifier. So I think we have an issue with this approach. Say > the device comes up with US. That would set DISABLED flags for channels > 12 to 14. With a country update to PL we would want to enable channels > 12 and 13, right? The orig_flags are copied from the initial flags > during wiphy_register() so it seems we will skip enabling 12 and 13. I > think we should remove the check above and call > wiphy_read_of_freq_limits() as a last step within > brcmf_setup_wiphybands(). It means it is called every time, but it > safeguards that the limits in DT are always applied. I'm not exactly happy with channels management in brcmfmac. Before calling wiphy_register it already disables channels unavailable for current country. This results in setting IEEE80211_CHAN_DISABLED in orig_flags of channels that may become available later, after country change. Please note it happens even right now, without this patch. Maybe you can workaround this by ignoring orig_flags in custom regulatory code, but I'd just prefer to have it nicely handled in the first place. This is the next feature I'm going to work on. AFAIU this patch won't be applied for now (it's for wireless-drivers-next and we first need to get wiphy_read_of_freq_limits in that tree). By the time that happens I may have another patchset cleaning brcmfmac ready. And FWIW this patch wouldn't make things worse *at this point* as we don't really support country switching for any device yet. So I hope problem with channels in brcmfmac doesn't mean we need to postpone patches 1-3. -- Rafał
