Re: [PATCH v6 1/2] rtw88: add regulatory process strategy for different chipset
From: Brian Norris <briannorris@chromium.org>
Date: 2020-03-24 16:51:11
Hi, On Tue, Mar 24, 2020 at 03:52:15PM +0800, yhchuang@realtek.com wrote:
quoted hunk ↗ jump to hunk
--- 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. +
I'm still not sure why rtw88 needs this, and nobody else does. I read your commit message, but that doesn't sound like something that belongs in a single driver still.
quoted hunk ↗ jump to hunk
endifdiff --git a/drivers/net/wireless/realtek/rtw88/main.c b/drivers/net/wireless/realtek/rtw88/main.c index 7640e97706f5..5d43bef91a3c 100644 --- a/drivers/net/wireless/realtek/rtw88/main.c +++ b/drivers/net/wireless/realtek/rtw88/main.c@@ -1501,8 +1501,9 @@ 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"); + 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.
quoted hunk ↗ jump to hunk
rtw_debugfs_init(rtwdev);diff --git a/drivers/net/wireless/realtek/rtw88/regd.c b/drivers/net/wireless/realtek/rtw88/regd.c index 69744dd65968..4cc1234bfb9a 100644 --- a/drivers/net/wireless/realtek/rtw88/regd.c +++ b/drivers/net/wireless/realtek/rtw88/regd.c 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)
Why are you ignoring SET_BY_DRIVER? That's what happens when (a few lines up) you call regulatory_hint(). At a minimum, that doesn't deserve a loud error print when we "fail" this function -- you should handle it properly. Brian
+ return -ENOTSUPP;
+
+ if (request->initiator == NL80211_REGDOM_SET_BY_USER &&
+ !IS_ENABLED(CONFIG_RTW88_REGD_USER_REG_HINTS))
+ return -EPERM;
+
+ if (request->initiator == NL80211_REGDOM_SET_BY_CORE) {
+ char *country_code;
+
+ /* return to the efuse setting */
+ country_code = rtwdev->efuse.country_code;
+ rtwdev->regd = rtw_regd_find_reg_by_name(country_code);
return 0;
+ }
+
rtwdev->regd = rtw_regd_find_reg_by_name(request->alpha2);
rtw_regd_apply_world_flags(wiphy, request->initiator);