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/commit/?id=b3 >> 30b25eaabda00d74e47566d9200907da381896 >> >> Most likely with wiphy_read_of_freq_limits helper: >> https://git.kernel.org/cgit/linux/kernel/git/davem/net-next.git/commit/?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
