Return-path: Received: from mail-oi0-f68.google.com ([209.85.218.68]:36561 "EHLO mail-oi0-f68.google.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1753601AbdBHHpT (ORCPT ); Wed, 8 Feb 2017 02:45:19 -0500 Received: by mail-oi0-f68.google.com with SMTP id u143so10416840oif.3 for ; Tue, 07 Feb 2017 23:44:47 -0800 (PST) MIME-Version: 1.0 In-Reply-To: <8492870c897543a1b3c635a96f1066cb@SC-EXCH02.marvell.com> References: <260962939eeb4dbbb6e462cc010aac21@SC-EXCH02.marvell.com> <3e57f30c29254db4a906e3e71ac36da5@SC-EXCH02.marvell.com> <8492870c897543a1b3c635a96f1066cb@SC-EXCH02.marvell.com> From: =?UTF-8?B?UmFmYcWCIE1pxYJlY2tp?= Date: Wed, 8 Feb 2017 08:44:46 +0100 Message-ID: (sfid-20170208_084817_550407_7B01FD5E) Subject: Re: [PATCH v9] Add new mac80211 driver mwlwifi. To: David Lin Cc: Steve deRosier , Kalle Valo , "linux-wireless@vger.kernel.org" , Johannes Berg , Chor Teck Law , James Lin , Pete Hsieh Content-Type: text/plain; charset=UTF-8 Sender: linux-wireless-owner@vger.kernel.org List-ID: On 8 February 2017 at 08:28, David Lin wrote: >> From: Rafa=C5=82 Mi=C5=82ecki [mailto:zajec5@gmail.com] worte: >> Sent: Wednesday, February 08, 2017 3:24 PM >> On 8 February 2017 at 07:30, David Lin wrote: >> > steve.derosier@gmail.com [mailto:steve.derosier@gmail.com] wrote: >> >> On Tue, Feb 7, 2017 at 10:15 PM, David Lin wrote: >> >> >> Rafa=C5=82 Mi=C5=82ecki [mailto:zajec5@gmail.com] wrote: >> >> >> Please use ieee80211-freq-limit: >> >> >> https://git.kernel.org/cgit/linux/kernel/git/davem/net-next.git/co >> >> >> mmi >> >> >> t/?id=3Db3 >> >> >> 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=3De6 >> >> >> 91ac2f75b69bee743f0370d79454ba4429b175 >> >> > >> >> > I already replied meaning of these parameters: >> >> > is used to disable 2g band. >> >> > is used to disable 5g band. >> >> > is used to specify antenna number (if default >> >> > number >> >> is suitable for you, there is no need to use this parameter). >> >> > 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 e= xist 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 speci= fic >> doesn't change anything. You just document them in a proper place. >> > > All right. I will do that. > >> >> > BTW, and are only used for mwlwifi to re= port >> 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 fo= r >> Marvell specific ones. >> >> In fact there were drivers of 3 vendors requiring band/freq-related desc= ription >> in DT: Broadcom, Marvell & Mediatek. This property was discussed & desig= ned >> 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 sugge= stion to document these marvell proprietary parameters. Seriously? Refusing to use generic binding because you think marvell,5ghz; is simpler than ieee80211-freq-limit =3D <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. --=20 Rafa=C5=82