On 8 February 2017 at 08:28, David Lin <dlin@xxxxxxxxxxx> wrote: >> From: Rafał Miłecki [mailto:zajec5@xxxxxxxxx] worte: >> Sent: Wednesday, February 08, 2017 3:24 PM >> 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/co >> >> >> mmi >> >> >> 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/co >> >> >> mmi >> >> >> 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. >> > > All right. I will do that. > >> >> > 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. >> > > I only need simple way to disable 2g or 5g band. I will follow your suggestion to document these marvell proprietary parameters. Seriously? Refusing to use generic binding because you think marvell,5ghz; is simpler than ieee80211-freq-limit = <2402000 2482000>; (not to mention your property seems reversed!)? I don't know how else to explain this to you. We don't want duplicated properties where one can work. Just use existing one. Don't add new one even if documented. -- Rafał
