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

[Andy Huang <tehuang@realtek.com>]

> [ 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 <tehuang@realtek.com> wrote:
> > > On Tue, Mar 24, 2020 at 03:52:15PM +0800, yhchuang@realtek.com
> wrote:
> > > > --- a/drivers/net/wireless/realtek/rtw88/Kconfig
> > > > +++ b/drivers/net/wireless/realtek/rtw88/Kconfig
> 
> > > 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.
> 
> > > 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.
> 
> > > > +   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.
> 
Yes, I think you make sense, we will provide v7, which will be similar to v5 with
more detailed explain in commit log.

Tzu-En

> 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.
> 
> > > 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
> 
> ------Please consider the environment before printing this e-mail.