Hi,
On Tue, Mar 24, 2020 at 03:52:15PM +0800, yhchuang@xxxxxxxxxxx wrote:
> --- 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.
> endif
> diff --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.
>
> 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);
>
