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: > >> On 7 February 2017 at 20:12, Steve deRosier <derosier@xxxxxxxxx> wrote: > >> >> + /* look for all matching property names */ > >> >> + for_each_property_of_node(priv->dt_node, prop) { if > >> >> + (strcmp(prop->name, "marvell,2ghz") == 0) > >> >> + priv->disable_2g = true; > >> >> + if (strcmp(prop->name, "marvell,5ghz") == 0) > >> >> + priv->disable_5g = true; > >> >> + if (strcmp(prop->name, "marvell,chainmask") == 0) { prop_value = > >> >> + be32_to_cpu(*((__be32 *)prop->value)); if (prop_value == 2) > >> >> + priv->antenna_tx = ANTENNA_TX_2; > >> >> + > >> >> + prop_value = be32_to_cpu(*((__be32 *) (prop->value + 4))); if > >> >> + (prop_value == 2) > >> >> + priv->antenna_rx = ANTENNA_RX_2; > >> >> + } > >> >> + } > >> >> + > >> >> + priv->pwr_node = of_find_node_by_name(priv->dt_node, > >> >> + "marvell,powertable"); > >> >> +#endif > >> >> +} > >> > > >> > AFAICT, there's no documentation for these DT bindings. The mwlwifi > >> > node and the marvell,powertable both need full documentation in > >> > Documentation/devicetree/bindings/... . > >> > > >> > Frankly I have a feeling I'm going to need these DT nodes and I'd > >> > like to not have to guess-and-check based on the code. > >> > >> 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. BTW, <marvell,2ghz> and <marvell,5ghz> are only used for mwlwifi to report supported bands, it is not related to limitation of frequency.
