diff mbox

[1/2] dt-bindings: document common IEEE 802.11 frequency properties

Message ID 20161228155955.25518-1-zajec5@gmail.com (mailing list archive)
State Changes Requested
Delegated to: Johannes Berg
Headers show

Commit Message

Rafał Miłecki Dec. 28, 2016, 3:59 p.m. UTC
From: Rafał Miłecki <rafal@milecki.pl>

This new file should be used for properties handled at higher level and
so usable with all drivers.

Signed-off-by: Rafał Miłecki <rafal@milecki.pl>
---
 .../devicetree/bindings/net/wireless/ieee80211.txt       | 16 ++++++++++++++++
 1 file changed, 16 insertions(+)
 create mode 100644 Documentation/devicetree/bindings/net/wireless/ieee80211.txt

Comments

Arend van Spriel Dec. 28, 2016, 8:05 p.m. UTC | #1
On 28-12-16 16:59, Rafał Miłecki wrote:
> From: Rafał Miłecki <rafal@milecki.pl>
> 
> This new file should be used for properties handled at higher level and
> so usable with all drivers.
> 
> Signed-off-by: Rafał Miłecki <rafal@milecki.pl>
> ---
>  .../devicetree/bindings/net/wireless/ieee80211.txt       | 16 ++++++++++++++++
>  1 file changed, 16 insertions(+)
>  create mode 100644 Documentation/devicetree/bindings/net/wireless/ieee80211.txt
> 
> diff --git a/Documentation/devicetree/bindings/net/wireless/ieee80211.txt b/Documentation/devicetree/bindings/net/wireless/ieee80211.txt
> new file mode 100644
> index 0000000..c762769
> --- /dev/null
> +++ b/Documentation/devicetree/bindings/net/wireless/ieee80211.txt
> @@ -0,0 +1,16 @@
> +Common IEEE 802.11 properties
> +
> +This provides documentation of common properties that are handled by a proper
> +net layer and don't require extra driver code.

Please do not make any assumptions on how DT properties are handled nor
by what. Just state that these properties apply to all wireless devices
and are applicable to device specific bindings.

> +Optional properties:
> + - ieee80211-min-center-freq : minimal supported frequency in KHz
> + - ieee80211-max-center-freq : maximal supported frequency in KHz
> +
> +Example:
> +
> +pcie@0,0 {
> +	reg = <0x0000 0 0 0 0>;
> +	ieee80211-min-center-freq = <2437000>;
> +	ieee80211-max-center-freq = <2457000>;
> +};

Is this the proper level to define it. I was expecting a child node of
the pci(e) controller. Maybe I am misreading the example.

Regards,
Rafał Miłecki Dec. 28, 2016, 8:32 p.m. UTC | #2
On 28 December 2016 at 21:05, Arend van Spriel
<arend.vanspriel@broadcom.com> wrote:
> On 28-12-16 16:59, Rafał Miłecki wrote:
>> From: Rafał Miłecki <rafal@milecki.pl>
>>
>> This new file should be used for properties handled at higher level and
>> so usable with all drivers.
>>
>> Signed-off-by: Rafał Miłecki <rafal@milecki.pl>
>> ---
>>  .../devicetree/bindings/net/wireless/ieee80211.txt       | 16 ++++++++++++++++
>>  1 file changed, 16 insertions(+)
>>  create mode 100644 Documentation/devicetree/bindings/net/wireless/ieee80211.txt
>>
>> diff --git a/Documentation/devicetree/bindings/net/wireless/ieee80211.txt b/Documentation/devicetree/bindings/net/wireless/ieee80211.txt
>> new file mode 100644
>> index 0000000..c762769
>> --- /dev/null
>> +++ b/Documentation/devicetree/bindings/net/wireless/ieee80211.txt
>> @@ -0,0 +1,16 @@
>> +Common IEEE 802.11 properties
>> +
>> +This provides documentation of common properties that are handled by a proper
>> +net layer and don't require extra driver code.
>
> Please do not make any assumptions on how DT properties are handled nor
> by what. Just state that these properties apply to all wireless devices
> and are applicable to device specific bindings.

OK, I'll try to improve this description.


>> +Optional properties:
>> + - ieee80211-min-center-freq : minimal supported frequency in KHz
>> + - ieee80211-max-center-freq : maximal supported frequency in KHz
>> +
>> +Example:
>> +
>> +pcie@0,0 {
>> +     reg = <0x0000 0 0 0 0>;
>> +     ieee80211-min-center-freq = <2437000>;
>> +     ieee80211-max-center-freq = <2457000>;
>> +};
>
> Is this the proper level to define it. I was expecting a child node of
> the pci(e) controller. Maybe I am misreading the example.

This is device node, not a controller node (and yes, it's complete
node). You just need to add such a node inside the controller one.

It doesn't seem to be clearly documented, but you can see it in examples in:
Documentation/devicetree/bindings/pci/mvebu-pci.txt
Documentation/devicetree/bindings/pci/nvidia,tegra20-pcie.txt
Documentation/devicetree/bindings/pci/pci-rcar-gen2.txt

The assignment is done in
pci_scan_device -> pci_set_of_node -> of_pci_find_child_device
(so this isn't controller specific thing).
Martin Blumenstingl Dec. 28, 2016, 8:39 p.m. UTC | #3
On Wed, Dec 28, 2016 at 9:32 PM, Rafał Miłecki <zajec5@gmail.com> wrote:
> On 28 December 2016 at 21:05, Arend van Spriel
> <arend.vanspriel@broadcom.com> wrote:
>> On 28-12-16 16:59, Rafał Miłecki wrote:
>>> From: Rafał Miłecki <rafal@milecki.pl>
>>>
>>> This new file should be used for properties handled at higher level and
>>> so usable with all drivers.
>>>
>>> Signed-off-by: Rafał Miłecki <rafal@milecki.pl>
>>> ---
>>>  .../devicetree/bindings/net/wireless/ieee80211.txt       | 16 ++++++++++++++++
>>>  1 file changed, 16 insertions(+)
>>>  create mode 100644 Documentation/devicetree/bindings/net/wireless/ieee80211.txt
>>>
>>> diff --git a/Documentation/devicetree/bindings/net/wireless/ieee80211.txt b/Documentation/devicetree/bindings/net/wireless/ieee80211.txt
>>> new file mode 100644
>>> index 0000000..c762769
>>> --- /dev/null
>>> +++ b/Documentation/devicetree/bindings/net/wireless/ieee80211.txt
>>> @@ -0,0 +1,16 @@
>>> +Common IEEE 802.11 properties
>>> +
>>> +This provides documentation of common properties that are handled by a proper
>>> +net layer and don't require extra driver code.
>>
>> Please do not make any assumptions on how DT properties are handled nor
>> by what. Just state that these properties apply to all wireless devices
>> and are applicable to device specific bindings.
>
> OK, I'll try to improve this description.
>
>
>>> +Optional properties:
>>> + - ieee80211-min-center-freq : minimal supported frequency in KHz
>>> + - ieee80211-max-center-freq : maximal supported frequency in KHz
>>> +
>>> +Example:
>>> +
>>> +pcie@0,0 {
>>> +     reg = <0x0000 0 0 0 0>;
>>> +     ieee80211-min-center-freq = <2437000>;
>>> +     ieee80211-max-center-freq = <2457000>;
>>> +};
>>
>> Is this the proper level to define it. I was expecting a child node of
>> the pci(e) controller. Maybe I am misreading the example.
>
> This is device node, not a controller node (and yes, it's complete
> node). You just need to add such a node inside the controller one.
you should name the node wifi@0,0 instead. I revised my ath9k OF
binding documentation due to similar concerns (and after seeing the
result I must admit that they were right). you can have a look at the
result here: [0]

apart from that: thanks for the patch, I will try this as soon as possible!


Regards,
Martin


[0] https://github.com/torvalds/linux/blob/master/Documentation/devicetree/bindings/net/wireless/qca%2Cath9k.txt
Rafał Miłecki Dec. 28, 2016, 8:43 p.m. UTC | #4
On 28 December 2016 at 21:39, Martin Blumenstingl
<martin.blumenstingl@googlemail.com> wrote:
> On Wed, Dec 28, 2016 at 9:32 PM, Rafał Miłecki <zajec5@gmail.com> wrote:
>> On 28 December 2016 at 21:05, Arend van Spriel
>> <arend.vanspriel@broadcom.com> wrote:
>>> On 28-12-16 16:59, Rafał Miłecki wrote:
>>>> From: Rafał Miłecki <rafal@milecki.pl>
>>>>
>>>> This new file should be used for properties handled at higher level and
>>>> so usable with all drivers.
>>>>
>>>> Signed-off-by: Rafał Miłecki <rafal@milecki.pl>
>>>> ---
>>>>  .../devicetree/bindings/net/wireless/ieee80211.txt       | 16 ++++++++++++++++
>>>>  1 file changed, 16 insertions(+)
>>>>  create mode 100644 Documentation/devicetree/bindings/net/wireless/ieee80211.txt
>>>>
>>>> diff --git a/Documentation/devicetree/bindings/net/wireless/ieee80211.txt b/Documentation/devicetree/bindings/net/wireless/ieee80211.txt
>>>> new file mode 100644
>>>> index 0000000..c762769
>>>> --- /dev/null
>>>> +++ b/Documentation/devicetree/bindings/net/wireless/ieee80211.txt
>>>> @@ -0,0 +1,16 @@
>>>> +Common IEEE 802.11 properties
>>>> +
>>>> +This provides documentation of common properties that are handled by a proper
>>>> +net layer and don't require extra driver code.
>>>
>>> Please do not make any assumptions on how DT properties are handled nor
>>> by what. Just state that these properties apply to all wireless devices
>>> and are applicable to device specific bindings.
>>
>> OK, I'll try to improve this description.
>>
>>
>>>> +Optional properties:
>>>> + - ieee80211-min-center-freq : minimal supported frequency in KHz
>>>> + - ieee80211-max-center-freq : maximal supported frequency in KHz
>>>> +
>>>> +Example:
>>>> +
>>>> +pcie@0,0 {
>>>> +     reg = <0x0000 0 0 0 0>;
>>>> +     ieee80211-min-center-freq = <2437000>;
>>>> +     ieee80211-max-center-freq = <2457000>;
>>>> +};
>>>
>>> Is this the proper level to define it. I was expecting a child node of
>>> the pci(e) controller. Maybe I am misreading the example.
>>
>> This is device node, not a controller node (and yes, it's complete
>> node). You just need to add such a node inside the controller one.
> you should name the node wifi@0,0 instead. I revised my ath9k OF
> binding documentation due to similar concerns (and after seeing the
> result I must admit that they were right). you can have a look at the
> result here: [0]

Thanks for your comment, I'm far from considering myself DT expert, so
I often need help with such things, I'll change this in V2.
Felix Fietkau Dec. 28, 2016, 9:35 p.m. UTC | #5
On 2016-12-28 16:59, Rafał Miłecki wrote:
> From: Rafał Miłecki <rafal@milecki.pl>
> 
> This new file should be used for properties handled at higher level and
> so usable with all drivers.
> 
> Signed-off-by: Rafał Miłecki <rafal@milecki.pl>
> ---
>  .../devicetree/bindings/net/wireless/ieee80211.txt       | 16 ++++++++++++++++
>  1 file changed, 16 insertions(+)
>  create mode 100644 Documentation/devicetree/bindings/net/wireless/ieee80211.txt
> 
> diff --git a/Documentation/devicetree/bindings/net/wireless/ieee80211.txt b/Documentation/devicetree/bindings/net/wireless/ieee80211.txt
> new file mode 100644
> index 0000000..c762769
> --- /dev/null
> +++ b/Documentation/devicetree/bindings/net/wireless/ieee80211.txt
> @@ -0,0 +1,16 @@
> +Common IEEE 802.11 properties
> +
> +This provides documentation of common properties that are handled by a proper
> +net layer and don't require extra driver code.
> +
> +Optional properties:
> + - ieee80211-min-center-freq : minimal supported frequency in KHz
> + - ieee80211-max-center-freq : maximal supported frequency in KHz
> +
> +Example:
> +
> +pcie@0,0 {
> +	reg = <0x0000 0 0 0 0>;
> +	ieee80211-min-center-freq = <2437000>;
> +	ieee80211-max-center-freq = <2457000>;
I'm not sure that's the best way to deal with enabling/disabling bands.
If we get more bands in the future, there might be unsupported ones in
the middle, which min/max won't cover.

- Felix
Rafał Miłecki Dec. 28, 2016, 10:22 p.m. UTC | #6
On 28 December 2016 at 22:35, Felix Fietkau <nbd@nbd.name> wrote:
> On 2016-12-28 16:59, Rafał Miłecki wrote:
>> From: Rafał Miłecki <rafal@milecki.pl>
>>
>> This new file should be used for properties handled at higher level and
>> so usable with all drivers.
>>
>> Signed-off-by: Rafał Miłecki <rafal@milecki.pl>
>> ---
>>  .../devicetree/bindings/net/wireless/ieee80211.txt       | 16 ++++++++++++++++
>>  1 file changed, 16 insertions(+)
>>  create mode 100644 Documentation/devicetree/bindings/net/wireless/ieee80211.txt
>>
>> diff --git a/Documentation/devicetree/bindings/net/wireless/ieee80211.txt b/Documentation/devicetree/bindings/net/wireless/ieee80211.txt
>> new file mode 100644
>> index 0000000..c762769
>> --- /dev/null
>> +++ b/Documentation/devicetree/bindings/net/wireless/ieee80211.txt
>> @@ -0,0 +1,16 @@
>> +Common IEEE 802.11 properties
>> +
>> +This provides documentation of common properties that are handled by a proper
>> +net layer and don't require extra driver code.
>> +
>> +Optional properties:
>> + - ieee80211-min-center-freq : minimal supported frequency in KHz
>> + - ieee80211-max-center-freq : maximal supported frequency in KHz
>> +
>> +Example:
>> +
>> +pcie@0,0 {
>> +     reg = <0x0000 0 0 0 0>;
>> +     ieee80211-min-center-freq = <2437000>;
>> +     ieee80211-max-center-freq = <2457000>;
> I'm not sure that's the best way to deal with enabling/disabling bands.
> If we get more bands in the future, there might be unsupported ones in
> the middle, which min/max won't cover.

Maybe we could try specifying list of ranges? E.g.
ieee80211-center-frequencies = <2412000 2422000
    2442000 2452>;
or
ieee80211-center-frequencies = <2412000 2422000>,
    <2442000 2452>;
for device supporting channels 1, 2, 3, 7, 8, 9?
Rob Herring (Arm) Jan. 3, 2017, 7:55 p.m. UTC | #7
On Wed, Dec 28, 2016 at 04:59:54PM +0100, Rafał Miłecki wrote:
> From: Rafał Miłecki <rafal@milecki.pl>
> 
> This new file should be used for properties handled at higher level and
> so usable with all drivers.

Why is this needed? Where would this data normally come from?

> Signed-off-by: Rafał Miłecki <rafal@milecki.pl>
> ---
>  .../devicetree/bindings/net/wireless/ieee80211.txt       | 16 ++++++++++++++++
>  1 file changed, 16 insertions(+)
>  create mode 100644 Documentation/devicetree/bindings/net/wireless/ieee80211.txt
> 
> diff --git a/Documentation/devicetree/bindings/net/wireless/ieee80211.txt b/Documentation/devicetree/bindings/net/wireless/ieee80211.txt
> new file mode 100644
> index 0000000..c762769
> --- /dev/null
> +++ b/Documentation/devicetree/bindings/net/wireless/ieee80211.txt
> @@ -0,0 +1,16 @@
> +Common IEEE 802.11 properties
> +
> +This provides documentation of common properties that are handled by a proper
> +net layer and don't require extra driver code.

Not relavent to a binding. Bindings describe h/w.

> +
> +Optional properties:
> + - ieee80211-min-center-freq : minimal supported frequency in KHz
> + - ieee80211-max-center-freq : maximal supported frequency in KHz
> +
> +Example:
> +
> +pcie@0,0 {
> +	reg = <0x0000 0 0 0 0>;
> +	ieee80211-min-center-freq = <2437000>;
> +	ieee80211-max-center-freq = <2457000>;
> +};
> -- 
> 2.10.1
> 
> --
> To unsubscribe from this list: send the line "unsubscribe devicetree" in
> the body of a message to majordomo@vger.kernel.org
> More majordomo info at  http://vger.kernel.org/majordomo-info.html
Rafał Miłecki Jan. 3, 2017, 8:20 p.m. UTC | #8
On 3 January 2017 at 20:55, Rob Herring <robh@kernel.org> wrote:
> On Wed, Dec 28, 2016 at 04:59:54PM +0100, Rafał Miłecki wrote:
>> From: Rafał Miłecki <rafal@milecki.pl>
>>
>> This new file should be used for properties handled at higher level and
>> so usable with all drivers.
>
> Why is this needed? Where would this data normally come from?

Vendors limit choice of channels at their web user interface level. I
want to do better and report proper channels directly at kernel level
instead of masking them in every possible configuration tool.


>> Signed-off-by: Rafał Miłecki <rafal@milecki.pl>
>> ---
>>  .../devicetree/bindings/net/wireless/ieee80211.txt       | 16 ++++++++++++++++
>>  1 file changed, 16 insertions(+)
>>  create mode 100644 Documentation/devicetree/bindings/net/wireless/ieee80211.txt
>>
>> diff --git a/Documentation/devicetree/bindings/net/wireless/ieee80211.txt b/Documentation/devicetree/bindings/net/wireless/ieee80211.txt
>> new file mode 100644
>> index 0000000..c762769
>> --- /dev/null
>> +++ b/Documentation/devicetree/bindings/net/wireless/ieee80211.txt
>> @@ -0,0 +1,16 @@
>> +Common IEEE 802.11 properties
>> +
>> +This provides documentation of common properties that are handled by a proper
>> +net layer and don't require extra driver code.
>
> Not relavent to a binding. Bindings describe h/w.

Yes, Arend pointed it to me and I improved this Documentation file in
further versions. Could you review V4, please?
https://patchwork.kernel.org/patch/9494713/
Kalle Valo Jan. 4, 2017, 2:32 p.m. UTC | #9
Rafał Miłecki <zajec5@gmail.com> writes:

> On 3 January 2017 at 20:55, Rob Herring <robh@kernel.org> wrote:
>> On Wed, Dec 28, 2016 at 04:59:54PM +0100, Rafał Miłecki wrote:
>>> From: Rafał Miłecki <rafal@milecki.pl>
>>>
>>> This new file should be used for properties handled at higher level and
>>> so usable with all drivers.
>>
>> Why is this needed? Where would this data normally come from?
>
> Vendors limit choice of channels at their web user interface level. I
> want to do better and report proper channels directly at kernel level
> instead of masking them in every possible configuration tool.

Why do vendors limit the channels? Is it because of a hardware
limitation (antenna does not support, or not calibrated, for a certain
band etc) or something else?
Rafał Miłecki Jan. 4, 2017, 2:53 p.m. UTC | #10
On 4 January 2017 at 15:32, Kalle Valo <kvalo@codeaurora.org> wrote:
> Rafał Miłecki <zajec5@gmail.com> writes:
>
>> On 3 January 2017 at 20:55, Rob Herring <robh@kernel.org> wrote:
>>> On Wed, Dec 28, 2016 at 04:59:54PM +0100, Rafał Miłecki wrote:
>>>> From: Rafał Miłecki <rafal@milecki.pl>
>>>>
>>>> This new file should be used for properties handled at higher level and
>>>> so usable with all drivers.
>>>
>>> Why is this needed? Where would this data normally come from?
>>
>> Vendors limit choice of channels at their web user interface level. I
>> want to do better and report proper channels directly at kernel level
>> instead of masking them in every possible configuration tool.
>
> Why do vendors limit the channels? Is it because of a hardware
> limitation (antenna does not support, or not calibrated, for a certain
> band etc) or something else?

Please review & comment on the latest version, currently V5:
https://patchwork.kernel.org/patch/9495795/
"This can be used to specify extra hardware limitations caused by e.g.
used antennas or power amplifiers."
Arend van Spriel Jan. 4, 2017, 7:53 p.m. UTC | #11
On 4-1-2017 15:53, Rafał Miłecki wrote:
> On 4 January 2017 at 15:32, Kalle Valo <kvalo@codeaurora.org> wrote:
>> Rafał Miłecki <zajec5@gmail.com> writes:
>>
>>> On 3 January 2017 at 20:55, Rob Herring <robh@kernel.org> wrote:
>>>> On Wed, Dec 28, 2016 at 04:59:54PM +0100, Rafał Miłecki wrote:
>>>>> From: Rafał Miłecki <rafal@milecki.pl>
>>>>>
>>>>> This new file should be used for properties handled at higher level and
>>>>> so usable with all drivers.
>>>>
>>>> Why is this needed? Where would this data normally come from?
>>>
>>> Vendors limit choice of channels at their web user interface level. I
>>> want to do better and report proper channels directly at kernel level
>>> instead of masking them in every possible configuration tool.
>>
>> Why do vendors limit the channels? Is it because of a hardware
>> limitation (antenna does not support, or not calibrated, for a certain
>> band etc) or something else?
> 
> Please review & comment on the latest version, currently V5:
> https://patchwork.kernel.org/patch/9495795/
> "This can be used to specify extra hardware limitations caused by e.g.
> used antennas or power amplifiers."

Just to be clear, it is does not need to be a hardware limitation, but
simply a way to assure that multiple wlan radios are not interfering
with each other and crank up the total bandwidth that vendors can put on
their box.

Regards,
Arend
diff mbox

Patch

diff --git a/Documentation/devicetree/bindings/net/wireless/ieee80211.txt b/Documentation/devicetree/bindings/net/wireless/ieee80211.txt
new file mode 100644
index 0000000..c762769
--- /dev/null
+++ b/Documentation/devicetree/bindings/net/wireless/ieee80211.txt
@@ -0,0 +1,16 @@ 
+Common IEEE 802.11 properties
+
+This provides documentation of common properties that are handled by a proper
+net layer and don't require extra driver code.
+
+Optional properties:
+ - ieee80211-min-center-freq : minimal supported frequency in KHz
+ - ieee80211-max-center-freq : maximal supported frequency in KHz
+
+Example:
+
+pcie@0,0 {
+	reg = <0x0000 0 0 0 0>;
+	ieee80211-min-center-freq = <2437000>;
+	ieee80211-max-center-freq = <2457000>;
+};