diff mbox series

[v5,1/2] rtw88: add regulatory process strategy for different chipset

Message ID 20200313034918.22222-2-yhchuang@realtek.com (mailing list archive)
State Changes Requested
Delegated to: Kalle Valo
Headers show
Series rtw88: update regulatory settings | expand

Commit Message

Tony Chuang March 13, 2020, 3:49 a.m. UTC 
From: Tzu-En Huang <tehuang@realtek.com>

If the country code in efuse is not set to a specific regulatory,
then the efuse setting is WW (world-wide).

For the country codes that are set to a specific country, the driver
is not going to apply any new setting notified from the stack.

For the country codes that are not set, which is treated as WW, the
driver provides a customized WW regulatory through
wiphy_apply_custom_regulatory(). But when there's any other
regulatory found in IE, notified by NL80211_REGDOM_SET_BY_COUNTRY_IE,
the driver will apply the setting found in the IE for 802.11d.

We want all of the regulatory rules being set strictly, not to loose
the restrictions of the country setting, so apply REGULATORY_STRICT_REG
to all of the regulatory settings.

For user notification (NL80211_REGDOM_SET_BY_USER), we'd like to
default disable it because as FCC publication 594280 states:
"In particular, users must not be relied on to set a country code or
location code to ensure compliance". If any distributions or OSes want
to set regulatory through user space tool (ex, iw reg set), then they
should honor the compile flag option RTW88_REGD_USER_REG_HINTS.

Signed-off-by: Tzu-En Huang <tehuang@realtek.com>
Signed-off-by: Yan-Hsuan Chuang <yhchuang@realtek.com>
---

v3 -> v4
  * squashed with "rtw88: support dynamic user regulatory setting"
  * modify the commit log

v4 -> v5
  * no change

 drivers/net/wireless/realtek/rtw88/Kconfig | 10 ++++
 drivers/net/wireless/realtek/rtw88/main.c  |  6 ++-
 drivers/net/wireless/realtek/rtw88/main.h  |  1 +
 drivers/net/wireless/realtek/rtw88/regd.c  | 61 ++++++++++++++++++----
 4 files changed, 65 insertions(+), 13 deletions(-)

Comments

Brian Norris March 21, 2020, 12:16 a.m. UTC | #1 
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?

...

>  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.

Brian

>  	rtwdev->regd = rtw_regd_find_reg_by_name(request->alpha2);
>  	rtw_regd_apply_world_flags(wiphy, request->initiator);
>
Andy Huang March 23, 2020, 8:41 a.m. UTC | #2 
> 
> 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);
> >
> 
>
diff mbox series

Patch

diff --git a/drivers/net/wireless/realtek/rtw88/Kconfig b/drivers/net/wireless/realtek/rtw88/Kconfig
index 33bd7ed797ff..04b84ec1dfc1 100644
--- a/drivers/net/wireless/realtek/rtw88/Kconfig
+++ b/drivers/net/wireless/realtek/rtw88/Kconfig
@@ -52,4 +52,14 @@  config RTW88_DEBUGFS
 
 	  If unsure, say Y to simplify debug problems
 
+config RTW88_REGD_USER_REG_HINTS
+	bool "Realtek rtw88 user regulatory hints"
+	depends on RTW88_CORE
+	default n
+	help
+	  Enable regulatoy user hints
+
+	  If unsure, say N. This should only be allowed on distributions
+	  that need this to correct the regulatory.
+
 endif
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");
+	}
 
 	rtw_debugfs_init(rtwdev);
 
diff --git a/drivers/net/wireless/realtek/rtw88/main.h b/drivers/net/wireless/realtek/rtw88/main.h
index c074cef22120..5e0ee86048bd 100644
--- a/drivers/net/wireless/realtek/rtw88/main.h
+++ b/drivers/net/wireless/realtek/rtw88/main.h
@@ -1424,6 +1424,7 @@  struct rtw_efuse {
 	u8 addr[ETH_ALEN];
 	u8 channel_plan;
 	u8 country_code[2];
+	bool country_worldwide;
 	u8 rf_board_option;
 	u8 rfe_option;
 	u8 power_track_type;
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),
+	}
+};
 #define COUNTRY_CHPLAN_ENT(_alpha2, _chplan, _txpwr_regd) \
 	{.alpha2 = (_alpha2), \
 	 .chplan = (_chplan), \
@@ -339,12 +351,31 @@  static struct rtw_regulatory rtw_regd_find_reg_by_name(char *alpha2)
 	return rtw_defined_chplan;
 }
 
+static bool rtw_regd_is_ww(struct rtw_regulatory *reg)
+{
+	if (reg->txpwr_regd == RTW_REGD_WW)
+		return true;
+	return false;
+}
+
 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;
+	}
 	rtwdev->regd = rtw_regd_find_reg_by_name(request->alpha2);
 	rtw_regd_apply_world_flags(wiphy, request->initiator);
 
@@ -352,15 +383,22 @@  static int rtw_regd_notifier_apply(struct rtw_dev *rtwdev,
 }
 
 static int
-rtw_regd_init_wiphy(struct rtw_regulatory *reg, struct wiphy *wiphy,
+rtw_regd_init_wiphy(struct rtw_dev *rtwdev, struct wiphy *wiphy,
 		    void (*reg_notifier)(struct wiphy *wiphy,
 					 struct regulatory_request *request))
 {
+	struct rtw_regulatory *reg = &rtwdev->regd;
+
 	wiphy->reg_notifier = reg_notifier;
 
-	wiphy->regulatory_flags &= ~REGULATORY_CUSTOM_REG;
-	wiphy->regulatory_flags &= ~REGULATORY_STRICT_REG;
-	wiphy->regulatory_flags &= ~REGULATORY_DISABLE_BEACON_HINTS;
+	if (rtw_regd_is_ww(reg)) {
+		rtwdev->efuse.country_worldwide = true;
+		wiphy->regulatory_flags |= REGULATORY_CUSTOM_REG;
+		wiphy_apply_custom_regulatory(wiphy, &rtw88_world_regdom);
+	} else {
+		rtwdev->efuse.country_worldwide = false;
+	}
+	wiphy->regulatory_flags |= REGULATORY_STRICT_REG;
 
 	rtw_regd_apply_hw_cap_flags(wiphy);
 
@@ -377,7 +415,7 @@  int rtw_regd_init(struct rtw_dev *rtwdev,
 		return -EINVAL;
 
 	rtwdev->regd = rtw_regd_find_reg_by_name(rtwdev->efuse.country_code);
-	rtw_regd_init_wiphy(&rtwdev->regd, wiphy, reg_notifier);
+	rtw_regd_init_wiphy(rtwdev, wiphy, reg_notifier);
 
 	return 0;
 }
@@ -388,11 +426,12 @@  void rtw_regd_notifier(struct wiphy *wiphy, struct regulatory_request *request)
 	struct rtw_dev *rtwdev = hw->priv;
 	struct rtw_hal *hal = &rtwdev->hal;
 
-	rtw_regd_notifier_apply(rtwdev, wiphy, request);
-	rtw_dbg(rtwdev, RTW_DBG_REGD,
-		"get alpha2 %c%c from initiator %d, mapping to chplan 0x%x, txregd %d\n",
-		request->alpha2[0], request->alpha2[1], request->initiator,
-		rtwdev->regd.chplan, rtwdev->regd.txpwr_regd);
+	if (!rtw_regd_notifier_apply(rtwdev, wiphy, request))
+		rtw_dbg(rtwdev, RTW_DBG_REGD,
+			"get alpha2 %c%c from initiator %d, mapping to chplan 0x%x, txregd %d\n",
+			request->alpha2[0], request->alpha2[1],
+			request->initiator, rtwdev->regd.chplan,
+			rtwdev->regd.txpwr_regd);
 
 	rtw_phy_set_tx_power_level(rtwdev, hal->current_channel);
 }