On 8 February 2017 at 07:30, David Lin <dlin@xxxxxxxxxxx> wrote: > steve.derosier@xxxxxxxxx [mailto:steve.derosier@xxxxxxxxx] wrote: >> On Tue, Feb 7, 2017 at 10:15 PM, David Lin <dlin@xxxxxxxxxxx> wrote: >> >> Rafał Miłecki [mailto:zajec5@xxxxxxxxx] 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ł