Received-SPF: pass (google.com: best guess record for domain of linux-wireless-owner@vger.kernel.org designates 209.132.180.67 as permitted sender) client-ip=209.132.180.67;
Authenticated-By: 
From:   Andy Huang <tehuang@realtek.com>
To:     Brian Norris <briannorris@chromium.org>,
        Tony Chuang <yhchuang@realtek.com>
CC:     "kvalo@codeaurora.org" <kvalo@codeaurora.org>,
        "linux-wireless@vger.kernel.org" <linux-wireless@vger.kernel.org>
Subject: RE: [PATCH v5 1/2] rtw88: add regulatory process strategy for different chipset
Thread-Topic: [PATCH v5 1/2] rtw88: add regulatory process strategy for
 different chipset
Thread-Index: AQHV+OpgY5V9hxNNkkODhgd1sW65nKhRtBGAgAQw2LA=
Date:   Mon, 23 Mar 2020 08:41:05 +0000
Message-ID: <84a3e219774c48b5a976c0cb9392af51@realtek.com>
References: <20200313034918.22222-1-yhchuang@realtek.com>
 <20200313034918.22222-2-yhchuang@realtek.com>
 <20200321001645.GA16851@google.com>
In-Reply-To: <20200321001645.GA16851@google.com>
Accept-Language: zh-TW, en-US
Content-Language: zh-TW
Content-Type: text/plain; charset="us-ascii"
Content-Transfer-Encoding: 8BIT
MIME-Version: 1.0
Sender: linux-wireless-owner@vger.kernel.org
Precedence: bulk

> 
> Hi,
> 
> On Fri, Mar 13, 2020 at 11:49:17AM +0800, yhchuang@realtek.com wrote:
> > diff --git a/drivers/net/wireless/realtek/rtw88/main.c
> b/drivers/net/wireless/realtek/rtw88/main.c
> > index 2f73820cd9ba..635d9964beaa 100644
> > --- a/drivers/net/wireless/realtek/rtw88/main.c
> > +++ b/drivers/net/wireless/realtek/rtw88/main.c
> > @@ -1510,8 +1510,10 @@ int rtw_register_hw(struct rtw_dev *rtwdev,
> struct ieee80211_hw *hw)
> >  		return ret;
> >  	}
> >
> > -	if (regulatory_hint(hw->wiphy, rtwdev->regd.alpha2))
> > -		rtw_err(rtwdev, "regulatory_hint fail\n");
> > +	if (!rtwdev->efuse.country_worldwide) {
> > +		if (regulatory_hint(hw->wiphy, rtwdev->efuse.country_code))
> > +			rtw_err(rtwdev, "regulatory_hint fail\n");
>  Might as well log the error code, whlie you're at it?
> 
> > +	}
> >
> >  	rtw_debugfs_init(rtwdev);
> >
> 
> > diff --git a/drivers/net/wireless/realtek/rtw88/regd.c
> b/drivers/net/wireless/realtek/rtw88/regd.c
> > index 69744dd65968..500a02b97a9c 100644
> > --- a/drivers/net/wireless/realtek/rtw88/regd.c
> > +++ b/drivers/net/wireless/realtek/rtw88/regd.c
> > @@ -7,6 +7,18 @@
> >  #include "debug.h"
> >  #include "phy.h"
> >
> > +static const struct ieee80211_regdomain rtw88_world_regdom = {
> > +	.n_reg_rules = 5,
> > +	.alpha2 =  "99",
> > +	.reg_rules = {
> > +		REG_RULE(2412 - 10, 2462 + 10, 40, 0, 20, 0),
> > +		REG_RULE(2467 - 10, 2484 + 10, 40, 0, 20, NL80211_RRF_NO_IR),
> > +		REG_RULE(5180 - 10, 5240 + 10, 80, 0, 20, NL80211_RRF_NO_IR),
> > +		REG_RULE(5260 - 10, 5700 + 10, 80, 0, 20,
> > +			 NL80211_RRF_NO_IR | NL80211_RRF_DFS),
> > +		REG_RULE(5745 - 10, 5825 + 10, 80, 0, 20, NL80211_RRF_NO_IR),
> > +	}
> > +};
> 
> These rules look substantially identical to the default world rules
> specified in the standard regdb, except for the fact that you're missing
> the NO-ODFM part of this band:
> 
>         # Channel 14. Only JP enables this and for 802.11b only
>         (2474 - 2494 @ 20), (20), NO-IR, NO-OFDM
> 
> So, why do you need to specify a custom one?

It's because the channel plan from USER could be violated when connect
to an 802.11d AP, if we use the stack's worldwide. When the kernel scans,
some of the passive channels could become active, and then the stack will
intersect the channel plans because REGULATORY_STRICT_REG has been set.
If stack's worldwide is used, the result of intersecting will come out with a
channel plan that the passive channels became active. But if we use custom
worldwide, it won't happen, the passive channels will remain passive.

However, after our discussion, we think it's acceptable to follow 802.11d
setting and discard the USER setting, so we will send a fixed patch.

> 
> ...
> 
> >  static int rtw_regd_notifier_apply(struct rtw_dev *rtwdev,
> >  				   struct wiphy *wiphy,
> >  				   struct regulatory_request *request)
> >  {
> > -	if (request->initiator == NL80211_REGDOM_SET_BY_USER)
> > +	if (request->initiator == NL80211_REGDOM_SET_BY_DRIVER)
> > +		return -EINVAL;
> > +	if (request->initiator == NL80211_REGDOM_SET_BY_USER &&
> > +	    !IS_ENABLED(CONFIG_RTW88_REGD_USER_REG_HINTS))
> > +		return -EINVAL;
> > +	if (request->initiator == NL80211_REGDOM_SET_BY_COUNTRY_IE &&
> > +	    !rtw_regd_is_ww(&rtwdev->regd))
> > +		return -EINVAL;
> > +	if (request->initiator == NL80211_REGDOM_SET_BY_CORE &&
> > +	    !rtwdev->efuse.country_worldwide) {
> > +		rtwdev->regd =
> > +			rtw_regd_find_reg_by_name(rtwdev->efuse.country_code);
> >  		return 0;
> > +	}
> 
> None of these errors actually go anywhere; if you were planning to
> ignore these, shouldn't they be surfaced somewhere? Or can't these be
> encoded in your regulatory policy instead? Like
> REGULATORY_COUNTRY_IE_IGNORE, for one.
> 
> And as with your WOWLAN implementation: if there's no way to surface
> errors, you should at least log something.

We'll fix it.

Tzu-En

> 
> Brian
> 
> >  	rtwdev->regd = rtw_regd_find_reg_by_name(request->alpha2);
> >  	rtw_regd_apply_world_flags(wiphy, request->initiator);
> >
> 
>