Re: [PATCH v6 1/2] rtw88: add regulatory process strategy for different chipset

[ I'll preface this by saying that the more I look at the regulatory
core, the more I realize I'm confused or wrong at times. So forgive me
if I've made errors along the way, and please do correct me. ]

On Tue, Mar 24, 2020 at 8:11 PM Andy Huang [off-list ref] wrote:

quoted

On Tue, Mar 24, 2020 at 03:52:15PM +0800, yhchuang@realtek.com wrote:

quoted

--- a/drivers/net/wireless/realtek/rtw88/Kconfig
+++ b/drivers/net/wireless/realtek/rtw88/Kconfig

quoted

I'm still not sure why rtw88 needs this, and nobody else does. I read

I think in Atheros driver, ATH_REG_DYNAMIC_USER_REG_HINTS config serves
the same purpose.

Ah, I forgot about that one, sorry.

quoted

your commit message, but that doesn't sound like something that belongs
in a single driver still.

As our previous commit message claims, it is due to FCC [...]

Yes, I saw that: my point was that effectively all drivers are subject
to this FCC rule, and so this could be a common CONFIG_*. But if we
already have the ATH_* one (I missed that, above), I guess we can have
an rtw88 one too. It might be less confusing (and more
straightforwardly-implemented) if we moved this stuff to the core
someday, though.

quoted

quoted

+   ret = regulatory_hint(hw->wiphy, rtwdev->efuse.country_code);
+   if (ret)
+           rtw_warn(rtwdev, "failed to hint regulatory: %d\n", ret);

I don't think this is what you want; you had it right in previous
revisions:

      if (!rtwdev->efuse.country_worldwide) {
              if (regulatory_hint(hw->wiphy, rtwdev->efuse.country_code))
                      rtw_err( ... );
      }

Without the 'country_worlwide' check, you start "hinting" (even on
worldwide chips) that you really wanted "country" 00 only, and so we
*never* adapt to more strict country settings. That's not how world-wide
settings are supposed to work.

It doesn't mean that we want country 00 only, we will get country notifies
from stack, and we will apply it if we accept it. We don't want stack to change
the channel plan for us.

I noted this to you privately, but I don't believe it's expected to
call regulatory_hint() with "00". See the kerneldoc:

 * @alpha2: the ISO/IEC 3166 alpha2 the driver claims its regulatory domain
 *      should be in. If @rd is set this should be NULL. Note that if you
 *      set this to NULL you should still set rd->alpha2 to some accepted
 *      alpha2.

Note that "00" is *not* actually an ISO 3166 alpha2 code.

The key problem I'm seeing: once you do this, you establish a
wiphy-specific regd, and this regd never updates its country code or
DFS region according to IE updates. So attributes like
NL80211_ATTR_DFS_REGION and NL80211_ATTR_REG_ALPHA2 remain unset.

Your previous revision -- which for WW settings used
wiphy_apply_custom_regulatory() and *not* regulatory_hint() -- did not
have that problem.

quoted

Why are you ignoring SET_BY_DRIVER?

Since the notification with NL80211_REGDOM_SET_BY_DRIVER flag might
comes from an another chipset's regulatory_hint().

Ack.

Brian