Re: [PATCH v2 4/6] rtw88: update regulatory settings implementaion

2019-10-19 Thread Kalle Valo
Brian Norris  writes:

> On Wed, Oct 16, 2019 at 7:55 PM Tony Chuang  wrote:
>>
>> From: Brian Norris
>> >
>> > On Wed, Oct 16, 2019 at 5:33 AM  wrote:
>> > > This also supports user regulatory hints, and it should only be
>> > > enabled for specific distributions that need this to correct
>> > > the cards reglutory.
> ...
>> > There should be a pretty high bar for introducing either new CONFIG_*
>> > options or module parameters, in my opinion, and I'm not sure you
>> > really satisfied it. Why "should only be enabled" by certain
>> > distributions? Your opinion? If it's the technical limitation you
>> > refer to ("efuse settings"), then just detect the efuse and prevent
>> > user hints only on those modules.
>> >
>>
>> Because the efuse/module does not contain the information if the
>> user's hint is allowed. But sometimes distributions require to set the
>> regulatory via "NL80211_CMD_SET_REG".
>> So we are leaving the CONFIG_* here for some reason that needs it.
>
> Is there ever a case where user hint is not allowed? For what reason?
> If not efuse, then what?
>
> Or alternatively: if someone sets CONFIG_RTW88_REGD_USER_REG_HINTS=y,
> then what problems will they have? Technical problems (e.g., firmware
> will crash on certain modules) or  problems?

I'm not convinced either that a Kconfig option is the correct thing to
do here. We need to understand more about the background first.

This patch needs a lot more discussion, please move it out from this
patchset and handle it separately. That way it won't block other
patches.

-- 
https://wireless.wiki.kernel.org/en/developers/documentation/submittingpatches


Re: [PATCH v2 4/6] rtw88: update regulatory settings implementaion

2019-10-16 Thread Brian Norris
On Wed, Oct 16, 2019 at 7:55 PM Tony Chuang  wrote:
>
> From: Brian Norris
> >
> > On Wed, Oct 16, 2019 at 5:33 AM  wrote:
> > > This also supports user regulatory hints, and it should only be
> > > enabled for specific distributions that need this to correct
> > > the cards reglutory.
...
> > There should be a pretty high bar for introducing either new CONFIG_*
> > options or module parameters, in my opinion, and I'm not sure you
> > really satisfied it. Why "should only be enabled" by certain
> > distributions? Your opinion? If it's the technical limitation you
> > refer to ("efuse settings"), then just detect the efuse and prevent
> > user hints only on those modules.
> >
>
> Because the efuse/module does not contain the information if the
> user's hint is allowed. But sometimes distributions require to set the
> regulatory via "NL80211_CMD_SET_REG".
> So we are leaving the CONFIG_* here for some reason that needs it.

Is there ever a case where user hint is not allowed? For what reason?
If not efuse, then what?

Or alternatively: if someone sets CONFIG_RTW88_REGD_USER_REG_HINTS=y,
then what problems will they have? Technical problems (e.g., firmware
will crash on certain modules) or  problems?

(If "" means "legal", then just blink twice to acknowledge. Or
just don't answer.)

Brian


RE: [PATCH v2 4/6] rtw88: update regulatory settings implementaion

2019-10-16 Thread Tony Chuang
From: Brian Norris
> 
> On Wed, Oct 16, 2019 at 5:33 AM  wrote:
> > This also supports user regulatory hints, and it should only be
> > enabled for specific distributions that need this to correct
> > the cards reglutory.
> 
> s/cards/card's/
> s/reglutory/regulatory/

Typo should be fixed in v3 :)

> 
> There should be a pretty high bar for introducing either new CONFIG_*
> options or module parameters, in my opinion, and I'm not sure you
> really satisfied it. Why "should only be enabled" by certain
> distributions? Your opinion? If it's the technical limitation you
> refer to ("efuse settings"), then just detect the efuse and prevent
> user hints only on those modules.
> 

Because the efuse/module does not contain the information if the
user's hint is allowed. But sometimes distributions require to set the
regulatory via "NL80211_CMD_SET_REG".
So we are leaving the CONFIG_* here for some reason that needs it.

Yan-Hsuan


Re: [PATCH v2 4/6] rtw88: update regulatory settings implementaion

2019-10-16 Thread Brian Norris
On Wed, Oct 16, 2019 at 5:33 AM  wrote:
> This also supports user regulatory hints, and it should only be
> enabled for specific distributions that need this to correct
> the cards reglutory.

s/cards/card's/
s/reglutory/regulatory/

There should be a pretty high bar for introducing either new CONFIG_*
options or module parameters, in my opinion, and I'm not sure you
really satisfied it. Why "should only be enabled" by certain
distributions? Your opinion? If it's the technical limitation you
refer to ("efuse settings"), then just detect the efuse and prevent
user hints only on those modules.

Brian