On 8 February 2017 at 07:30, David Lin <[email protected]> wrote:
> [email protected] [mailto:[email protected]] wrote:
>> On Tue, Feb 7, 2017 at 10:15 PM, David Lin <[email protected]> wrote:
>> >> Rafał Miłecki [mailto:[email protected]] wrote:
>> >> Please use ieee80211-freq-limit:
>> >> https://git.kernel.org/cgit/linux/kernel/git/davem/net-next.git/commi
>> >> t/?id=b3
>> >> 30b25eaabda00d74e47566d9200907da381896
>> >>
>> >> Most likely with wiphy_read_of_freq_limits helper:
>> >> https://git.kernel.org/cgit/linux/kernel/git/davem/net-next.git/commi
>> >> t/?id=e6
>> >> 91ac2f75b69bee743f0370d79454ba4429b175
>> >
>> > I already replied meaning of these parameters:
>> > <marvell,2ghz> is used to disable 2g band.
>> > <marvell,5ghz> is used to disable 5g band.
>> > <marvell, chainmask> is used to specify antenna number (if default number
>> is suitable for you, there is no need to use this parameter).
>> > <marvell,powertable> should not be used for chip with device power table.
>> In fact, this parameter should not be used any more.
>> >
>>
>> David, I think you're not understanding the comment, or at least that's what 
>> it
>> looks like to me. Yes, you did reply as to the meaning.
>> And, your reply, while informative, didn't tell us you understood and were
>> willing to fix the problem. I doubt you meant it this way, but it feels 
>> defensive
>> and like a brush-off.
>>
>> First off, you will still have to document any DT bindings you're adding. 
>> Just
>> because you answer the question in the review doesn't mean you're not
>> responsible for doing so.
>>
>> And second off, I think that Rafal (and sorry about my spelling, looks like
>> there's some sort of accent on the l that I don't know how to make my
>> keyboard do) is saying: there's already some generic bindings that can be 
>> used
>> to disable the 2g or 5g bands. Granted they're even newer than your patch,
>> but I do think if said bindings exist and are appropriate, you should use 
>> them.
>>
>> - Steve
>
> These parameters are marvell proprietary parameters, I don't think it should 
> be added to DT bindings.

Steve is correct.

You have to document new properties, just because they are Marvell
specific doesn't change anything. You just document them in a proper
place.


> BTW, <marvell,2ghz> and <marvell,5ghz> are only used for mwlwifi to report 
> supported bands, it is not related to limitation of frequency.

How reporting a single band doesn't limit reported frequencies? You
can achieve exactly the same using generic property, so there is no
place for Marvell specific ones.

In fact there were drivers of 3 vendors requiring band/freq-related
description in DT: Broadcom, Marvell & Mediatek. This property was
discussed & designed to support all limitation cases we found possible
to make it usable with (hopefully) all drivers.

-- 
Rafał

Reply via email to