Re: [PATCH v3 1/3] Documentation: dt: net: add mt76 wireless device binding

[Martin Blumenstingl <martin.blumenstingl-gM/Ye1E23mwN+BqQ9rBEUg@public.gmane.org>]

On Wed, Dec 28, 2016 at 11:08 AM, Rafał Miłecki <zajec5-Re5JQEeQqe8AvxtiuMwx3w@public.gmane.org> wrote:
> On 3 October 2016 at 15:29, Kalle Valo <kvalo-sgV2jX0FEOL9JmXXK+q4OQ@public.gmane.org> wrote:
>> Arnd Bergmann <arnd-r2nGTMty4D4@public.gmane.org> writes:
>>
>>> On Friday 30 September 2016, Felix Fietkau wrote:
>>>> >> >> >> +                 device_type = "pci";
>>>> >> >> >> +                 mediatek,mtd-eeprom = <&factory 0x8000>;
>>>> >> >> >> +                 mediatek,2ghz = <0>;
>>>> >> >
>>>> >> > It's not clear what the possible values for the 2ghz property are,
>>>> >> > can you be more verbose in the description? How is <0> different
>>>> >> > from no property?
>>>> >> 0 means disabled, no property means unchanged (compared to EEPROM).
>>>> >
>>>> > Maybe have a boolean property instead then to say "mediatek,2ghz-disabled" ?
>>>> >
>>>> > If zero is the only possible value, there is no need to put a number in there.
>>>> 1 is also possible, which will force-enable the capability.
>>>
>>> Ok, then both those values should be documented in the binding.
>>
>> Related to this, Martin sent patches which add generic bindings for
>> enabling 2 Ghz and 5 Ghz bands.
>>
>> [RFC,1/3] Documentation: dt-bindings: add IEEE 802.11 binding documentation
>> https://patchwork.kernel.org/patch/9359833/
>>
>> [RFC,2/3] of: add IEEE 802.11 device configuration support code
>> https://patchwork.kernel.org/patch/9359837/
>
> I would prefer something more generic. Many tri-band routers split 5
> GHz band into 2 sets of channels and they have separated radios for
> them.
>
> E.g. Netgear R8000 has phy0 which should be used for higher part of 5
> GHz band (channels 149+) and phy2 which should be used for lower part
> of 5 GHz band (channels from 36 to 48 or 64).
>
> What do you think about some more flexible properties like:
> ieee80211-min-center-freq
> ieee80211-max-center-freq
what would happen if only one of these properties was given or would
we forbid that (because the .dts should always describe the hardware,
and if we describe a lower bound then we should also describe the
upper bound)?
the benefits of your solution are:
- this would allow *enabling* bands as well (my proposal allows this
as well, but the .dts is indeed a bit hard to read - unlike your
solution which looks nice to me)
- we could handle this within generic cfg80211/mac80211 code instead
of "duplicating" it per driver

should we describe the center freq in Hz or MHz (cfg80211's
ieee80211_channel uses the latter)?

@Arnd: what do you think from devicetree perspective?


Regards,
Martin

[0] http://lxr.free-electrons.com/source/include/net/cfg80211.h#L130
--
To unsubscribe from this list: send the line "unsubscribe devicetree" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html