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

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

[Rafał Miłecki]

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>;
+};
-- 
2.10.1

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

[Rafał Miłecki]

From: Rafał Miłecki <rafal-g1n6cQUeyibVItvQsEIGlw@public.gmane.org>

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-g1n6cQUeyibVItvQsEIGlw@public.gmane.org>
---
 .../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>;
+};
-- 
2.10.1

--
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

[PATCH 2/2] cfg80211: reg: support ieee80211-(min|max)-center-freq DT properties

[Rafał Miłecki]

From: Rafał Miłecki <rafal@milecki.pl>

They allow specifying hardware limitations of supported channels. This
may be useful for specifying single band devices or devices that support
only some part of the whole band.
E.g. some tri-band routers have separated radios for lower and higher
part of 5 GHz band.

Signed-off-by: Rafał Miłecki <rafal@milecki.pl>
---
 net/wireless/reg.c | 34 ++++++++++++++++++++++++++++++++++
 1 file changed, 34 insertions(+)

diff --git a/net/wireless/reg.c b/net/wireless/reg.c
index 5dbac37..35ba5c7 100644
--- a/net/wireless/reg.c
+++ b/net/wireless/reg.c
@@ -1123,6 +1123,26 @@ const char *reg_initiator_name(enum nl80211_reg_initiator initiator)
 }
 EXPORT_SYMBOL(reg_initiator_name);
 
+static bool reg_center_freq_of_valid(struct wiphy *wiphy,
+				     struct ieee80211_channel *chan)
+{
+	struct device_node *np = wiphy_dev(wiphy)->of_node;
+	u32 val;
+
+	if (!np)
+		return true;
+
+	if (!of_property_read_u32(np, "ieee80211-min-center-freq", &val) &&
+	    chan->center_freq < KHZ_TO_MHZ(val))
+		return false;
+
+	if (!of_property_read_u32(np, "ieee80211-max-center-freq", &val) &&
+	    chan->center_freq > KHZ_TO_MHZ(val))
+		return false;
+
+	return true;
+}
+
 static uint32_t reg_rule_to_chan_bw_flags(const struct ieee80211_regdomain *regd,
 					  const struct ieee80211_reg_rule *reg_rule,
 					  const struct ieee80211_channel *chan)
@@ -1209,6 +1229,13 @@ static void handle_channel(struct wiphy *wiphy,
 		return;
 	}
 
+	if (!reg_center_freq_of_valid(wiphy, chan)) {
+		pr_debug("Disabling freq %d MHz as it's out of OF limits\n",
+			 chan->center_freq);
+		chan->flags |= IEEE80211_CHAN_DISABLED;
+		return;
+	}
+
 	regd = reg_get_regdomain(wiphy);
 
 	power_rule = &reg_rule->power_rule;
@@ -1741,6 +1768,13 @@ static void handle_channel_custom(struct wiphy *wiphy,
 		return;
 	}
 
+	if (!reg_center_freq_of_valid(wiphy, chan)) {
+		pr_debug("Disabling freq %d MHz as it's out of OF limits\n",
+			 chan->center_freq);
+		chan->flags |= IEEE80211_CHAN_DISABLED;
+		return;
+	}
+
 	power_rule = &reg_rule->power_rule;
 	bw_flags = reg_rule_to_chan_bw_flags(regd, reg_rule, chan);
 
-- 
2.10.1

[PATCH 2/2] cfg80211: reg: support ieee80211-(min|max)-center-freq DT properties

[Rafał Miłecki]

From: Rafał Miłecki <rafal-g1n6cQUeyibVItvQsEIGlw@public.gmane.org>

They allow specifying hardware limitations of supported channels. This
may be useful for specifying single band devices or devices that support
only some part of the whole band.
E.g. some tri-band routers have separated radios for lower and higher
part of 5 GHz band.

Signed-off-by: Rafał Miłecki <rafal-g1n6cQUeyibVItvQsEIGlw@public.gmane.org>
---
 net/wireless/reg.c | 34 ++++++++++++++++++++++++++++++++++
 1 file changed, 34 insertions(+)

diff --git a/net/wireless/reg.c b/net/wireless/reg.c
index 5dbac37..35ba5c7 100644
--- a/net/wireless/reg.c
+++ b/net/wireless/reg.c
@@ -1123,6 +1123,26 @@ const char *reg_initiator_name(enum nl80211_reg_initiator initiator)
 }
 EXPORT_SYMBOL(reg_initiator_name);
 
+static bool reg_center_freq_of_valid(struct wiphy *wiphy,
+				     struct ieee80211_channel *chan)
+{
+	struct device_node *np = wiphy_dev(wiphy)->of_node;
+	u32 val;
+
+	if (!np)
+		return true;
+
+	if (!of_property_read_u32(np, "ieee80211-min-center-freq", &val) &&
+	    chan->center_freq < KHZ_TO_MHZ(val))
+		return false;
+
+	if (!of_property_read_u32(np, "ieee80211-max-center-freq", &val) &&
+	    chan->center_freq > KHZ_TO_MHZ(val))
+		return false;
+
+	return true;
+}
+
 static uint32_t reg_rule_to_chan_bw_flags(const struct ieee80211_regdomain *regd,
 					  const struct ieee80211_reg_rule *reg_rule,
 					  const struct ieee80211_channel *chan)
@@ -1209,6 +1229,13 @@ static void handle_channel(struct wiphy *wiphy,
 		return;
 	}
 
+	if (!reg_center_freq_of_valid(wiphy, chan)) {
+		pr_debug("Disabling freq %d MHz as it's out of OF limits\n",
+			 chan->center_freq);
+		chan->flags |= IEEE80211_CHAN_DISABLED;
+		return;
+	}
+
 	regd = reg_get_regdomain(wiphy);
 
 	power_rule = &reg_rule->power_rule;
@@ -1741,6 +1768,13 @@ static void handle_channel_custom(struct wiphy *wiphy,
 		return;
 	}
 
+	if (!reg_center_freq_of_valid(wiphy, chan)) {
+		pr_debug("Disabling freq %d MHz as it's out of OF limits\n",
+			 chan->center_freq);
+		chan->flags |= IEEE80211_CHAN_DISABLED;
+		return;
+	}
+
 	power_rule = &reg_rule->power_rule;
 	bw_flags = reg_rule_to_chan_bw_flags(regd, reg_rule, chan);
 
-- 
2.10.1

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

[Arend van Spriel]

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,

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

[Arend van Spriel]

On 28-12-16 16:59, Rafał Miłecki wrote:
> From: Rafał Miłecki <rafal-g1n6cQUeyibVItvQsEIGlw@public.gmane.org>
> 
> 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-g1n6cQUeyibVItvQsEIGlw@public.gmane.org>
> ---
>  .../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,

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

[Rafał Miłecki]

On 28 December 2016 at 21:05, Arend van Spriel
<arend.vanspriel@broadcom.com> wrote:
> On 28-12-16 16:59, Rafa=C5=82 Mi=C5=82ecki wrote:
>> From: Rafa=C5=82 Mi=C5=82ecki <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=C5=82 Mi=C5=82ecki <rafal@milecki.pl>
>> ---
>>  .../devicetree/bindings/net/wireless/ieee80211.txt       | 16 +++++++++=
+++++++
>>  1 file changed, 16 insertions(+)
>>  create mode 100644 Documentation/devicetree/bindings/net/wireless/ieee8=
0211.txt
>>
>> diff --git a/Documentation/devicetree/bindings/net/wireless/ieee80211.tx=
t 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 =3D <0x0000 0 0 0 0>;
>> +     ieee80211-min-center-freq =3D <2437000>;
>> +     ieee80211-max-center-freq =3D <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).


--=20
Rafa=C5=82

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

[Rafał Miłecki]

On 28 December 2016 at 21:05, Arend van Spriel
<arend.vanspriel-dY08KVG/lbpWk0Htik3J/w@public.gmane.org> wrote:
> On 28-12-16 16:59, Rafał Miłecki wrote:
>> From: Rafał Miłecki <rafal-g1n6cQUeyibVItvQsEIGlw@public.gmane.org>
>>
>> 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-g1n6cQUeyibVItvQsEIGlw@public.gmane.org>
>> ---
>>  .../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).


-- 
Rafał
--
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

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

[Martin Blumenstingl]

On Wed, Dec 28, 2016 at 9:32 PM, Rafa=C5=82 Mi=C5=82ecki <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=C5=82 Mi=C5=82ecki wrote:
>>> From: Rafa=C5=82 Mi=C5=82ecki <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=C5=82 Mi=C5=82ecki <rafal@milecki.pl>
>>> ---
>>>  .../devicetree/bindings/net/wireless/ieee80211.txt       | 16 ++++++++=
++++++++
>>>  1 file changed, 16 insertions(+)
>>>  create mode 100644 Documentation/devicetree/bindings/net/wireless/ieee=
80211.txt
>>>
>>> diff --git a/Documentation/devicetree/bindings/net/wireless/ieee80211.t=
xt 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 =3D <0x0000 0 0 0 0>;
>>> +     ieee80211-min-center-freq =3D <2437000>;
>>> +     ieee80211-max-center-freq =3D <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

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

[Martin Blumenstingl]

On Wed, Dec 28, 2016 at 9:32 PM, Rafał Miłecki <zajec5-Re5JQEeQqe8AvxtiuMwx3w@public.gmane.org> wrote:
> On 28 December 2016 at 21:05, Arend van Spriel
> <arend.vanspriel-dY08KVG/lbpWk0Htik3J/w@public.gmane.org> wrote:
>> On 28-12-16 16:59, Rafał Miłecki wrote:
>>> From: Rafał Miłecki <rafal-g1n6cQUeyibVItvQsEIGlw@public.gmane.org>
>>>
>>> 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-g1n6cQUeyibVItvQsEIGlw@public.gmane.org>
>>> ---
>>>  .../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

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

[Rafał Miłecki]

On 28 December 2016 at 21:39, Martin Blumenstingl
<martin.blumenstingl@googlemail.com> wrote:
> On Wed, Dec 28, 2016 at 9:32 PM, Rafa=C5=82 Mi=C5=82ecki <zajec5@gmail.co=
m> wrote:
>> On 28 December 2016 at 21:05, Arend van Spriel
>> <arend.vanspriel@broadcom.com> wrote:
>>> On 28-12-16 16:59, Rafa=C5=82 Mi=C5=82ecki wrote:
>>>> From: Rafa=C5=82 Mi=C5=82ecki <rafal@milecki.pl>
>>>>
>>>> This new file should be used for properties handled at higher level an=
d
>>>> so usable with all drivers.
>>>>
>>>> Signed-off-by: Rafa=C5=82 Mi=C5=82ecki <rafal@milecki.pl>
>>>> ---
>>>>  .../devicetree/bindings/net/wireless/ieee80211.txt       | 16 +++++++=
+++++++++
>>>>  1 file changed, 16 insertions(+)
>>>>  create mode 100644 Documentation/devicetree/bindings/net/wireless/iee=
e80211.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 =3D <0x0000 0 0 0 0>;
>>>> +     ieee80211-min-center-freq =3D <2437000>;
>>>> +     ieee80211-max-center-freq =3D <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.

--=20
Rafa=C5=82

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

[Rafał Miłecki]

On 28 December 2016 at 21:39, Martin Blumenstingl
<martin.blumenstingl-gM/Ye1E23mwN+BqQ9rBEUg@public.gmane.org> wrote:
> On Wed, Dec 28, 2016 at 9:32 PM, Rafał Miłecki <zajec5-Re5JQEeQqe8@public.gmane.orgm> wrote:
>> On 28 December 2016 at 21:05, Arend van Spriel
>> <arend.vanspriel-dY08KVG/lbpWk0Htik3J/w@public.gmane.org> wrote:
>>> On 28-12-16 16:59, Rafał Miłecki wrote:
>>>> From: Rafał Miłecki <rafal-g1n6cQUeyibVItvQsEIGlw@public.gmane.org>
>>>>
>>>> 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-g1n6cQUeyibVItvQsEIGlw@public.gmane.org>
>>>> ---
>>>>  .../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.

-- 
Rafał

Re: [PATCH 2/2] cfg80211: reg: support ieee80211-(min|max)-center-freq DT properties

[Arend van Spriel]

On 28-12-16 16:59, Rafał Miłecki wrote:
> From: Rafał Miłecki <rafal@milecki.pl>
> 
> They allow specifying hardware limitations of supported channels. This
> may be useful for specifying single band devices or devices that support
> only some part of the whole band.
> E.g. some tri-band routers have separated radios for lower and higher
> part of 5 GHz band.
> 
> Signed-off-by: Rafał Miłecki <rafal@milecki.pl>
> ---
>  net/wireless/reg.c | 34 ++++++++++++++++++++++++++++++++++
>  1 file changed, 34 insertions(+)
> 
> diff --git a/net/wireless/reg.c b/net/wireless/reg.c
> index 5dbac37..35ba5c7 100644
> --- a/net/wireless/reg.c
> +++ b/net/wireless/reg.c
> @@ -1123,6 +1123,26 @@ const char *reg_initiator_name(enum nl80211_reg_initiator initiator)
>  }
>  EXPORT_SYMBOL(reg_initiator_name);
>  
> +static bool reg_center_freq_of_valid(struct wiphy *wiphy,
> +				     struct ieee80211_channel *chan)
> +{
> +	struct device_node *np = wiphy_dev(wiphy)->of_node;
> +	u32 val;
> +
> +	if (!np)
> +		return true;
> +
> +	if (!of_property_read_u32(np, "ieee80211-min-center-freq", &val) &&
> +	    chan->center_freq < KHZ_TO_MHZ(val))
> +		return false;
> +
> +	if (!of_property_read_u32(np, "ieee80211-max-center-freq", &val) &&
> +	    chan->center_freq > KHZ_TO_MHZ(val))
> +		return false;

I suspect these functions rely on CONFIG_OF. So might not build for
!CONFIG_OF.

Regards,
Arend

Re: [PATCH 2/2] cfg80211: reg: support ieee80211-(min|max)-center-freq DT properties

[Arend van Spriel]

On 28-12-16 16:59, Rafał Miłecki wrote:
> From: Rafał Miłecki <rafal-g1n6cQUeyibVItvQsEIGlw@public.gmane.org>
> 
> They allow specifying hardware limitations of supported channels. This
> may be useful for specifying single band devices or devices that support
> only some part of the whole band.
> E.g. some tri-band routers have separated radios for lower and higher
> part of 5 GHz band.
> 
> Signed-off-by: Rafał Miłecki <rafal-g1n6cQUeyibVItvQsEIGlw@public.gmane.org>
> ---
>  net/wireless/reg.c | 34 ++++++++++++++++++++++++++++++++++
>  1 file changed, 34 insertions(+)
> 
> diff --git a/net/wireless/reg.c b/net/wireless/reg.c
> index 5dbac37..35ba5c7 100644
> --- a/net/wireless/reg.c
> +++ b/net/wireless/reg.c
> @@ -1123,6 +1123,26 @@ const char *reg_initiator_name(enum nl80211_reg_initiator initiator)
>  }
>  EXPORT_SYMBOL(reg_initiator_name);
>  
> +static bool reg_center_freq_of_valid(struct wiphy *wiphy,
> +				     struct ieee80211_channel *chan)
> +{
> +	struct device_node *np = wiphy_dev(wiphy)->of_node;
> +	u32 val;
> +
> +	if (!np)
> +		return true;
> +
> +	if (!of_property_read_u32(np, "ieee80211-min-center-freq", &val) &&
> +	    chan->center_freq < KHZ_TO_MHZ(val))
> +		return false;
> +
> +	if (!of_property_read_u32(np, "ieee80211-max-center-freq", &val) &&
> +	    chan->center_freq > KHZ_TO_MHZ(val))
> +		return false;

I suspect these functions rely on CONFIG_OF. So might not build for
!CONFIG_OF.

Regards,
Arend

Re: [PATCH 2/2] cfg80211: reg: support ieee80211-(min|max)-center-freq DT properties

[Rafał Miłecki]

On 28 December 2016 at 22:07, Arend van Spriel
<arend.vanspriel@broadcom.com> wrote:
> On 28-12-16 16:59, Rafa=C5=82 Mi=C5=82ecki wrote:
>> From: Rafa=C5=82 Mi=C5=82ecki <rafal@milecki.pl>
>>
>> They allow specifying hardware limitations of supported channels. This
>> may be useful for specifying single band devices or devices that support
>> only some part of the whole band.
>> E.g. some tri-band routers have separated radios for lower and higher
>> part of 5 GHz band.
>>
>> Signed-off-by: Rafa=C5=82 Mi=C5=82ecki <rafal@milecki.pl>
>> ---
>>  net/wireless/reg.c | 34 ++++++++++++++++++++++++++++++++++
>>  1 file changed, 34 insertions(+)
>>
>> diff --git a/net/wireless/reg.c b/net/wireless/reg.c
>> index 5dbac37..35ba5c7 100644
>> --- a/net/wireless/reg.c
>> +++ b/net/wireless/reg.c
>> @@ -1123,6 +1123,26 @@ const char *reg_initiator_name(enum nl80211_reg_i=
nitiator initiator)
>>  }
>>  EXPORT_SYMBOL(reg_initiator_name);
>>
>> +static bool reg_center_freq_of_valid(struct wiphy *wiphy,
>> +                                  struct ieee80211_channel *chan)
>> +{
>> +     struct device_node *np =3D wiphy_dev(wiphy)->of_node;
>> +     u32 val;
>> +
>> +     if (!np)
>> +             return true;
>> +
>> +     if (!of_property_read_u32(np, "ieee80211-min-center-freq", &val) &=
&
>> +         chan->center_freq < KHZ_TO_MHZ(val))
>> +             return false;
>> +
>> +     if (!of_property_read_u32(np, "ieee80211-max-center-freq", &val) &=
&
>> +         chan->center_freq > KHZ_TO_MHZ(val))
>> +             return false;
>
> I suspect these functions rely on CONFIG_OF. So might not build for
> !CONFIG_OF.

I compiled it with
# CONFIG_OF is not set

Can you test it and provide a non-working config if you see a
compilation error, please?

--=20
Rafa=C5=82

Re: [PATCH 2/2] cfg80211: reg: support ieee80211-(min|max)-center-freq DT properties

[Rafał Miłecki]

On 28 December 2016 at 22:07, Arend van Spriel
<arend.vanspriel-dY08KVG/lbpWk0Htik3J/w@public.gmane.org> wrote:
> On 28-12-16 16:59, Rafał Miłecki wrote:
>> From: Rafał Miłecki <rafal-g1n6cQUeyibVItvQsEIGlw@public.gmane.org>
>>
>> They allow specifying hardware limitations of supported channels. This
>> may be useful for specifying single band devices or devices that support
>> only some part of the whole band.
>> E.g. some tri-band routers have separated radios for lower and higher
>> part of 5 GHz band.
>>
>> Signed-off-by: Rafał Miłecki <rafal-g1n6cQUeyibVItvQsEIGlw@public.gmane.org>
>> ---
>>  net/wireless/reg.c | 34 ++++++++++++++++++++++++++++++++++
>>  1 file changed, 34 insertions(+)
>>
>> diff --git a/net/wireless/reg.c b/net/wireless/reg.c
>> index 5dbac37..35ba5c7 100644
>> --- a/net/wireless/reg.c
>> +++ b/net/wireless/reg.c
>> @@ -1123,6 +1123,26 @@ const char *reg_initiator_name(enum nl80211_reg_initiator initiator)
>>  }
>>  EXPORT_SYMBOL(reg_initiator_name);
>>
>> +static bool reg_center_freq_of_valid(struct wiphy *wiphy,
>> +                                  struct ieee80211_channel *chan)
>> +{
>> +     struct device_node *np = wiphy_dev(wiphy)->of_node;
>> +     u32 val;
>> +
>> +     if (!np)
>> +             return true;
>> +
>> +     if (!of_property_read_u32(np, "ieee80211-min-center-freq", &val) &&
>> +         chan->center_freq < KHZ_TO_MHZ(val))
>> +             return false;
>> +
>> +     if (!of_property_read_u32(np, "ieee80211-max-center-freq", &val) &&
>> +         chan->center_freq > KHZ_TO_MHZ(val))
>> +             return false;
>
> I suspect these functions rely on CONFIG_OF. So might not build for
> !CONFIG_OF.

I compiled it with
# CONFIG_OF is not set

Can you test it and provide a non-working config if you see a
compilation error, please?

-- 
Rafał

Re: [PATCH 2/2] cfg80211: reg: support ieee80211-(min|max)-center-freq DT properties

[Rafał Miłecki]

On 28 December 2016 at 22:28, Rafa=C5=82 Mi=C5=82ecki <zajec5@gmail.com> wr=
ote:
> On 28 December 2016 at 22:07, Arend van Spriel
> <arend.vanspriel@broadcom.com> wrote:
>> On 28-12-16 16:59, Rafa=C5=82 Mi=C5=82ecki wrote:
>>> From: Rafa=C5=82 Mi=C5=82ecki <rafal@milecki.pl>
>>>
>>> They allow specifying hardware limitations of supported channels. This
>>> may be useful for specifying single band devices or devices that suppor=
t
>>> only some part of the whole band.
>>> E.g. some tri-band routers have separated radios for lower and higher
>>> part of 5 GHz band.
>>>
>>> Signed-off-by: Rafa=C5=82 Mi=C5=82ecki <rafal@milecki.pl>
>>> ---
>>>  net/wireless/reg.c | 34 ++++++++++++++++++++++++++++++++++
>>>  1 file changed, 34 insertions(+)
>>>
>>> diff --git a/net/wireless/reg.c b/net/wireless/reg.c
>>> index 5dbac37..35ba5c7 100644
>>> --- a/net/wireless/reg.c
>>> +++ b/net/wireless/reg.c
>>> @@ -1123,6 +1123,26 @@ const char *reg_initiator_name(enum nl80211_reg_=
initiator initiator)
>>>  }
>>>  EXPORT_SYMBOL(reg_initiator_name);
>>>
>>> +static bool reg_center_freq_of_valid(struct wiphy *wiphy,
>>> +                                  struct ieee80211_channel *chan)
>>> +{
>>> +     struct device_node *np =3D wiphy_dev(wiphy)->of_node;
>>> +     u32 val;
>>> +
>>> +     if (!np)
>>> +             return true;
>>> +
>>> +     if (!of_property_read_u32(np, "ieee80211-min-center-freq", &val) =
&&
>>> +         chan->center_freq < KHZ_TO_MHZ(val))
>>> +             return false;
>>> +
>>> +     if (!of_property_read_u32(np, "ieee80211-max-center-freq", &val) =
&&
>>> +         chan->center_freq > KHZ_TO_MHZ(val))
>>> +             return false;
>>
>> I suspect these functions rely on CONFIG_OF. So might not build for
>> !CONFIG_OF.
>
> I compiled it with
> # CONFIG_OF is not set
>
> Can you test it and provide a non-working config if you see a
> compilation error, please?

include/linux/of.h provides a lot of dummy static inline functions if
CONFIG_OF is not used (they also allow compiler to drop most of the
code).

--=20
Rafa=C5=82

Re: [PATCH 2/2] cfg80211: reg: support ieee80211-(min|max)-center-freq DT properties

[Rafał Miłecki]

On 28 December 2016 at 22:28, Rafał Miłecki <zajec5-Re5JQEeQqe8AvxtiuMwx3w@public.gmane.org> wrote:
> On 28 December 2016 at 22:07, Arend van Spriel
> <arend.vanspriel-dY08KVG/lbpWk0Htik3J/w@public.gmane.org> wrote:
>> On 28-12-16 16:59, Rafał Miłecki wrote:
>>> From: Rafał Miłecki <rafal-g1n6cQUeyibVItvQsEIGlw@public.gmane.org>
>>>
>>> They allow specifying hardware limitations of supported channels. This
>>> may be useful for specifying single band devices or devices that support
>>> only some part of the whole band.
>>> E.g. some tri-band routers have separated radios for lower and higher
>>> part of 5 GHz band.
>>>
>>> Signed-off-by: Rafał Miłecki <rafal-g1n6cQUeyibVItvQsEIGlw@public.gmane.org>
>>> ---
>>>  net/wireless/reg.c | 34 ++++++++++++++++++++++++++++++++++
>>>  1 file changed, 34 insertions(+)
>>>
>>> diff --git a/net/wireless/reg.c b/net/wireless/reg.c
>>> index 5dbac37..35ba5c7 100644
>>> --- a/net/wireless/reg.c
>>> +++ b/net/wireless/reg.c
>>> @@ -1123,6 +1123,26 @@ const char *reg_initiator_name(enum nl80211_reg_initiator initiator)
>>>  }
>>>  EXPORT_SYMBOL(reg_initiator_name);
>>>
>>> +static bool reg_center_freq_of_valid(struct wiphy *wiphy,
>>> +                                  struct ieee80211_channel *chan)
>>> +{
>>> +     struct device_node *np = wiphy_dev(wiphy)->of_node;
>>> +     u32 val;
>>> +
>>> +     if (!np)
>>> +             return true;
>>> +
>>> +     if (!of_property_read_u32(np, "ieee80211-min-center-freq", &val) &&
>>> +         chan->center_freq < KHZ_TO_MHZ(val))
>>> +             return false;
>>> +
>>> +     if (!of_property_read_u32(np, "ieee80211-max-center-freq", &val) &&
>>> +         chan->center_freq > KHZ_TO_MHZ(val))
>>> +             return false;
>>
>> I suspect these functions rely on CONFIG_OF. So might not build for
>> !CONFIG_OF.
>
> I compiled it with
> # CONFIG_OF is not set
>
> Can you test it and provide a non-working config if you see a
> compilation error, please?

include/linux/of.h provides a lot of dummy static inline functions if
CONFIG_OF is not used (they also allow compiler to drop most of the
code).

-- 
Rafał

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

[Felix Fietkau]

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

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

[Felix Fietkau]

On 2016-12-28 16:59, Rafał Miłecki wrote:
> From: Rafał Miłecki <rafal-g1n6cQUeyibVItvQsEIGlw@public.gmane.org>
> 
> 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-g1n6cQUeyibVItvQsEIGlw@public.gmane.org>
> ---
>  .../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

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

[Rafał Miłecki]

On 28 December 2016 at 22:35, Felix Fietkau <nbd@nbd.name> wrote:
> On 2016-12-28 16:59, Rafa=C5=82 Mi=C5=82ecki wrote:
>> From: Rafa=C5=82 Mi=C5=82ecki <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=C5=82 Mi=C5=82ecki <rafal@milecki.pl>
>> ---
>>  .../devicetree/bindings/net/wireless/ieee80211.txt       | 16 +++++++++=
+++++++
>>  1 file changed, 16 insertions(+)
>>  create mode 100644 Documentation/devicetree/bindings/net/wireless/ieee8=
0211.txt
>>
>> diff --git a/Documentation/devicetree/bindings/net/wireless/ieee80211.tx=
t 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 =3D <0x0000 0 0 0 0>;
>> +     ieee80211-min-center-freq =3D <2437000>;
>> +     ieee80211-max-center-freq =3D <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 =3D <2412000 2422000
    2442000 2452>;
or
ieee80211-center-frequencies =3D <2412000 2422000>,
    <2442000 2452>;
for device supporting channels 1, 2, 3, 7, 8, 9?

--=20
Rafa=C5=82

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

[Rafał Miłecki]

On 28 December 2016 at 22:35, Felix Fietkau <nbd-Vt+b4OUoWG0@public.gmane.org> wrote:
> On 2016-12-28 16:59, Rafał Miłecki wrote:
>> From: Rafał Miłecki <rafal-g1n6cQUeyibVItvQsEIGlw@public.gmane.org>
>>
>> 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-g1n6cQUeyibVItvQsEIGlw@public.gmane.org>
>> ---
>>  .../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?

-- 
Rafał
--
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

Re: [PATCH 2/2] cfg80211: reg: support ieee80211-(min|max)-center-freq DT properties

[Arend van Spriel]

On 28-12-16 22:30, Rafał Miłecki wrote:
> On 28 December 2016 at 22:28, Rafał Miłecki <zajec5@gmail.com> wrote:
>> On 28 December 2016 at 22:07, 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>
>>>>
>>>> They allow specifying hardware limitations of supported channels. This
>>>> may be useful for specifying single band devices or devices that support
>>>> only some part of the whole band.
>>>> E.g. some tri-band routers have separated radios for lower and higher
>>>> part of 5 GHz band.
>>>>
>>>> Signed-off-by: Rafał Miłecki <rafal@milecki.pl>
>>>> ---
>>>>  net/wireless/reg.c | 34 ++++++++++++++++++++++++++++++++++
>>>>  1 file changed, 34 insertions(+)
>>>>
>>>> diff --git a/net/wireless/reg.c b/net/wireless/reg.c
>>>> index 5dbac37..35ba5c7 100644
>>>> --- a/net/wireless/reg.c
>>>> +++ b/net/wireless/reg.c
>>>> @@ -1123,6 +1123,26 @@ const char *reg_initiator_name(enum nl80211_reg_initiator initiator)
>>>>  }
>>>>  EXPORT_SYMBOL(reg_initiator_name);
>>>>
>>>> +static bool reg_center_freq_of_valid(struct wiphy *wiphy,
>>>> +                                  struct ieee80211_channel *chan)
>>>> +{
>>>> +     struct device_node *np = wiphy_dev(wiphy)->of_node;
>>>> +     u32 val;
>>>> +
>>>> +     if (!np)
>>>> +             return true;
>>>> +
>>>> +     if (!of_property_read_u32(np, "ieee80211-min-center-freq", &val) &&
>>>> +         chan->center_freq < KHZ_TO_MHZ(val))
>>>> +             return false;
>>>> +
>>>> +     if (!of_property_read_u32(np, "ieee80211-max-center-freq", &val) &&
>>>> +         chan->center_freq > KHZ_TO_MHZ(val))
>>>> +             return false;
>>>
>>> I suspect these functions rely on CONFIG_OF. So might not build for
>>> !CONFIG_OF.
>>
>> I compiled it with
>> # CONFIG_OF is not set
>>
>> Can you test it and provide a non-working config if you see a
>> compilation error, please?
> 
> include/linux/of.h provides a lot of dummy static inline functions if
> CONFIG_OF is not used (they also allow compiler to drop most of the
> code).

of_propeirty_read_u32 is static inline in of.h calling
of_property_read_u32_array, which has a dummy variant in of.h returning
-ENOSYS so -38. Pretty sure that is not what you want. At least it does
not allow the compiler to drop any code so probably better to do:

if (!IS_ENABLED(CONFIG_OF) || !np)
	return true;

So with this patch you change the channel to DISABLED. I am not very
familiar with reg.c so do you know if this is done before or after
calling regulatory notifier in the driver. brcmfmac will enable channels
querying the device upon regulatory notifier call, which may undo the
behavior introduced by your patch.

Regards,
Arend

Re: [PATCH 2/2] cfg80211: reg: support ieee80211-(min|max)-center-freq DT properties

[Arend van Spriel]

On 28-12-16 22:30, Rafał Miłecki wrote:
> On 28 December 2016 at 22:28, Rafał Miłecki <zajec5-Re5JQEeQqe8AvxtiuMwx3w@public.gmane.org> wrote:
>> On 28 December 2016 at 22:07, Arend van Spriel
>> <arend.vanspriel-dY08KVG/lbpWk0Htik3J/w@public.gmane.org> wrote:
>>> On 28-12-16 16:59, Rafał Miłecki wrote:
>>>> From: Rafał Miłecki <rafal-g1n6cQUeyibVItvQsEIGlw@public.gmane.org>
>>>>
>>>> They allow specifying hardware limitations of supported channels. This
>>>> may be useful for specifying single band devices or devices that support
>>>> only some part of the whole band.
>>>> E.g. some tri-band routers have separated radios for lower and higher
>>>> part of 5 GHz band.
>>>>
>>>> Signed-off-by: Rafał Miłecki <rafal-g1n6cQUeyibVItvQsEIGlw@public.gmane.org>
>>>> ---
>>>>  net/wireless/reg.c | 34 ++++++++++++++++++++++++++++++++++
>>>>  1 file changed, 34 insertions(+)
>>>>
>>>> diff --git a/net/wireless/reg.c b/net/wireless/reg.c
>>>> index 5dbac37..35ba5c7 100644
>>>> --- a/net/wireless/reg.c
>>>> +++ b/net/wireless/reg.c
>>>> @@ -1123,6 +1123,26 @@ const char *reg_initiator_name(enum nl80211_reg_initiator initiator)
>>>>  }
>>>>  EXPORT_SYMBOL(reg_initiator_name);
>>>>
>>>> +static bool reg_center_freq_of_valid(struct wiphy *wiphy,
>>>> +                                  struct ieee80211_channel *chan)
>>>> +{
>>>> +     struct device_node *np = wiphy_dev(wiphy)->of_node;
>>>> +     u32 val;
>>>> +
>>>> +     if (!np)
>>>> +             return true;
>>>> +
>>>> +     if (!of_property_read_u32(np, "ieee80211-min-center-freq", &val) &&
>>>> +         chan->center_freq < KHZ_TO_MHZ(val))
>>>> +             return false;
>>>> +
>>>> +     if (!of_property_read_u32(np, "ieee80211-max-center-freq", &val) &&
>>>> +         chan->center_freq > KHZ_TO_MHZ(val))
>>>> +             return false;
>>>
>>> I suspect these functions rely on CONFIG_OF. So might not build for
>>> !CONFIG_OF.
>>
>> I compiled it with
>> # CONFIG_OF is not set
>>
>> Can you test it and provide a non-working config if you see a
>> compilation error, please?
> 
> include/linux/of.h provides a lot of dummy static inline functions if
> CONFIG_OF is not used (they also allow compiler to drop most of the
> code).

of_propeirty_read_u32 is static inline in of.h calling
of_property_read_u32_array, which has a dummy variant in of.h returning
-ENOSYS so -38. Pretty sure that is not what you want. At least it does
not allow the compiler to drop any code so probably better to do:

if (!IS_ENABLED(CONFIG_OF) || !np)
	return true;

So with this patch you change the channel to DISABLED. I am not very
familiar with reg.c so do you know if this is done before or after
calling regulatory notifier in the driver. brcmfmac will enable channels
querying the device upon regulatory notifier call, which may undo the
behavior introduced by your patch.

Regards,
Arend

Re: [PATCH 2/2] cfg80211: reg: support ieee80211-(min|max)-center-freq DT properties

[Rafał Miłecki]

On 29 December 2016 at 09:57, Arend van Spriel
<arend.vanspriel@broadcom.com> wrote:
> On 28-12-16 22:30, Rafa=C5=82 Mi=C5=82ecki wrote:
>> On 28 December 2016 at 22:28, Rafa=C5=82 Mi=C5=82ecki <zajec5@gmail.com>=
 wrote:
>>> On 28 December 2016 at 22:07, Arend van Spriel
>>> <arend.vanspriel@broadcom.com> wrote:
>>>> On 28-12-16 16:59, Rafa=C5=82 Mi=C5=82ecki wrote:
>>>>> From: Rafa=C5=82 Mi=C5=82ecki <rafal@milecki.pl>
>>>>>
>>>>> They allow specifying hardware limitations of supported channels. Thi=
s
>>>>> may be useful for specifying single band devices or devices that supp=
ort
>>>>> only some part of the whole band.
>>>>> E.g. some tri-band routers have separated radios for lower and higher
>>>>> part of 5 GHz band.
>>>>>
>>>>> Signed-off-by: Rafa=C5=82 Mi=C5=82ecki <rafal@milecki.pl>
>>>>> ---
>>>>>  net/wireless/reg.c | 34 ++++++++++++++++++++++++++++++++++
>>>>>  1 file changed, 34 insertions(+)
>>>>>
>>>>> diff --git a/net/wireless/reg.c b/net/wireless/reg.c
>>>>> index 5dbac37..35ba5c7 100644
>>>>> --- a/net/wireless/reg.c
>>>>> +++ b/net/wireless/reg.c
>>>>> @@ -1123,6 +1123,26 @@ const char *reg_initiator_name(enum nl80211_re=
g_initiator initiator)
>>>>>  }
>>>>>  EXPORT_SYMBOL(reg_initiator_name);
>>>>>
>>>>> +static bool reg_center_freq_of_valid(struct wiphy *wiphy,
>>>>> +                                  struct ieee80211_channel *chan)
>>>>> +{
>>>>> +     struct device_node *np =3D wiphy_dev(wiphy)->of_node;
>>>>> +     u32 val;
>>>>> +
>>>>> +     if (!np)
>>>>> +             return true;
>>>>> +
>>>>> +     if (!of_property_read_u32(np, "ieee80211-min-center-freq", &val=
) &&
>>>>> +         chan->center_freq < KHZ_TO_MHZ(val))
>>>>> +             return false;
>>>>> +
>>>>> +     if (!of_property_read_u32(np, "ieee80211-max-center-freq", &val=
) &&
>>>>> +         chan->center_freq > KHZ_TO_MHZ(val))
>>>>> +             return false;
>>>>
>>>> I suspect these functions rely on CONFIG_OF. So might not build for
>>>> !CONFIG_OF.
>>>
>>> I compiled it with
>>> # CONFIG_OF is not set
>>>
>>> Can you test it and provide a non-working config if you see a
>>> compilation error, please?
>>
>> include/linux/of.h provides a lot of dummy static inline functions if
>> CONFIG_OF is not used (they also allow compiler to drop most of the
>> code).
>
> of_propeirty_read_u32 is static inline in of.h calling
> of_property_read_u32_array, which has a dummy variant in of.h returning
> -ENOSYS so -38. Pretty sure that is not what you want. At least it does
> not allow the compiler to drop any code so probably better to do:
>
> if (!IS_ENABLED(CONFIG_OF) || !np)
>         return true;

Please verify that using a compiler. If there is a problem I'll be
happy to work on it, but I need a proof it exists.

If compilers sees a:
if (!-ENOSYS && chan->center_freq < KHZ_TO_MHZ(val))
condition, it's pretty clear it can be dropped. With both conditional
blocks dropped function always returns "true" and... can be dropped.

Let me see if I can convince you with the following test:

$ grep -m 1 CONFIG_OF .config
# CONFIG_OF is not set
$ objdump --syms net/wireless/reg.o | grep -c reg_center_freq_of_valid
0

$ grep -m 1 CONFIG_OF .config
CONFIG_OF=3Dy
$ objdump --syms net/wireless/reg.o | grep -c reg_center_freq_of_valid
1


> So with this patch you change the channel to DISABLED. I am not very
> familiar with reg.c so do you know if this is done before or after
> calling regulatory notifier in the driver. brcmfmac will enable channels
> querying the device upon regulatory notifier call, which may undo the
> behavior introduced by your patch.

I'm not regulatory export, so I hope someone will review this patch.
So far I can say it works for me after trying it on SR400ac with
BCM43602.

ieee80211-min-center-freq =3D <2437000>;

[   11.986941] cfg80211: Disabling freq 2412 MHz as it's out of OF limits
[   12.000466] cfg80211: Disabling freq 2417 MHz as it's out of OF limits
[   12.013984] cfg80211: Disabling freq 2422 MHz as it's out of OF limits
[   12.027497] cfg80211: Disabling freq 2427 MHz as it's out of OF limits
[   12.041012] cfg80211: Disabling freq 2432 MHz as it's out of OF limits

root@lede:/# iw phy phy0 channels
Band 1:
        * 2412 MHz [1] (disabled)
        * 2417 MHz [2] (disabled)
        * 2422 MHz [3] (disabled)
        * 2427 MHz [4] (disabled)
        * 2432 MHz [5] (disabled)
        * 2437 MHz [6]
          Maximum TX power: 20.0 dBm
          Channel widths: 20MHz HT40- HT40+
        * 2442 MHz [7]
          Maximum TX power: 20.0 dBm
          Channel widths: 20MHz HT40- HT40+
        * 2447 MHz [8]
          Maximum TX power: 20.0 dBm
          Channel widths: 20MHz HT40-
        * 2452 MHz [9]
          Maximum TX power: 20.0 dBm
          Channel widths: 20MHz HT40-
        * 2457 MHz [10]
          Maximum TX power: 20.0 dBm
          Channel widths: 20MHz HT40-
        * 2462 MHz [11]
          Maximum TX power: 20.0 dBm
          Channel widths: 20MHz HT40-
        * 2467 MHz [12] (disabled)
        * 2472 MHz [13] (disabled)
        * 2484 MHz [14] (disabled)

--=20
Rafa=C5=82

Re: [PATCH 2/2] cfg80211: reg: support ieee80211-(min|max)-center-freq DT properties

[Rafał Miłecki]

On 29 December 2016 at 09:57, Arend van Spriel
<arend.vanspriel-dY08KVG/lbpWk0Htik3J/w@public.gmane.org> wrote:
> On 28-12-16 22:30, Rafał Miłecki wrote:
>> On 28 December 2016 at 22:28, Rafał Miłecki <zajec5-Re5JQEeQqe8AvxtiuMwx3w@public.gmane.org> wrote:
>>> On 28 December 2016 at 22:07, Arend van Spriel
>>> <arend.vanspriel-dY08KVG/lbpWk0Htik3J/w@public.gmane.org> wrote:
>>>> On 28-12-16 16:59, Rafał Miłecki wrote:
>>>>> From: Rafał Miłecki <rafal-g1n6cQUeyibVItvQsEIGlw@public.gmane.org>
>>>>>
>>>>> They allow specifying hardware limitations of supported channels. This
>>>>> may be useful for specifying single band devices or devices that support
>>>>> only some part of the whole band.
>>>>> E.g. some tri-band routers have separated radios for lower and higher
>>>>> part of 5 GHz band.
>>>>>
>>>>> Signed-off-by: Rafał Miłecki <rafal-g1n6cQUeyibVItvQsEIGlw@public.gmane.org>
>>>>> ---
>>>>>  net/wireless/reg.c | 34 ++++++++++++++++++++++++++++++++++
>>>>>  1 file changed, 34 insertions(+)
>>>>>
>>>>> diff --git a/net/wireless/reg.c b/net/wireless/reg.c
>>>>> index 5dbac37..35ba5c7 100644
>>>>> --- a/net/wireless/reg.c
>>>>> +++ b/net/wireless/reg.c
>>>>> @@ -1123,6 +1123,26 @@ const char *reg_initiator_name(enum nl80211_reg_initiator initiator)
>>>>>  }
>>>>>  EXPORT_SYMBOL(reg_initiator_name);
>>>>>
>>>>> +static bool reg_center_freq_of_valid(struct wiphy *wiphy,
>>>>> +                                  struct ieee80211_channel *chan)
>>>>> +{
>>>>> +     struct device_node *np = wiphy_dev(wiphy)->of_node;
>>>>> +     u32 val;
>>>>> +
>>>>> +     if (!np)
>>>>> +             return true;
>>>>> +
>>>>> +     if (!of_property_read_u32(np, "ieee80211-min-center-freq", &val) &&
>>>>> +         chan->center_freq < KHZ_TO_MHZ(val))
>>>>> +             return false;
>>>>> +
>>>>> +     if (!of_property_read_u32(np, "ieee80211-max-center-freq", &val) &&
>>>>> +         chan->center_freq > KHZ_TO_MHZ(val))
>>>>> +             return false;
>>>>
>>>> I suspect these functions rely on CONFIG_OF. So might not build for
>>>> !CONFIG_OF.
>>>
>>> I compiled it with
>>> # CONFIG_OF is not set
>>>
>>> Can you test it and provide a non-working config if you see a
>>> compilation error, please?
>>
>> include/linux/of.h provides a lot of dummy static inline functions if
>> CONFIG_OF is not used (they also allow compiler to drop most of the
>> code).
>
> of_propeirty_read_u32 is static inline in of.h calling
> of_property_read_u32_array, which has a dummy variant in of.h returning
> -ENOSYS so -38. Pretty sure that is not what you want. At least it does
> not allow the compiler to drop any code so probably better to do:
>
> if (!IS_ENABLED(CONFIG_OF) || !np)
>         return true;

Please verify that using a compiler. If there is a problem I'll be
happy to work on it, but I need a proof it exists.

If compilers sees a:
if (!-ENOSYS && chan->center_freq < KHZ_TO_MHZ(val))
condition, it's pretty clear it can be dropped. With both conditional
blocks dropped function always returns "true" and... can be dropped.

Let me see if I can convince you with the following test:

$ grep -m 1 CONFIG_OF .config
# CONFIG_OF is not set
$ objdump --syms net/wireless/reg.o | grep -c reg_center_freq_of_valid
0

$ grep -m 1 CONFIG_OF .config
CONFIG_OF=y
$ objdump --syms net/wireless/reg.o | grep -c reg_center_freq_of_valid
1


> So with this patch you change the channel to DISABLED. I am not very
> familiar with reg.c so do you know if this is done before or after
> calling regulatory notifier in the driver. brcmfmac will enable channels
> querying the device upon regulatory notifier call, which may undo the
> behavior introduced by your patch.

I'm not regulatory export, so I hope someone will review this patch.
So far I can say it works for me after trying it on SR400ac with
BCM43602.

ieee80211-min-center-freq = <2437000>;

[   11.986941] cfg80211: Disabling freq 2412 MHz as it's out of OF limits
[   12.000466] cfg80211: Disabling freq 2417 MHz as it's out of OF limits
[   12.013984] cfg80211: Disabling freq 2422 MHz as it's out of OF limits
[   12.027497] cfg80211: Disabling freq 2427 MHz as it's out of OF limits
[   12.041012] cfg80211: Disabling freq 2432 MHz as it's out of OF limits

root@lede:/# iw phy phy0 channels
Band 1:
        * 2412 MHz [1] (disabled)
        * 2417 MHz [2] (disabled)
        * 2422 MHz [3] (disabled)
        * 2427 MHz [4] (disabled)
        * 2432 MHz [5] (disabled)
        * 2437 MHz [6]
          Maximum TX power: 20.0 dBm
          Channel widths: 20MHz HT40- HT40+
        * 2442 MHz [7]
          Maximum TX power: 20.0 dBm
          Channel widths: 20MHz HT40- HT40+
        * 2447 MHz [8]
          Maximum TX power: 20.0 dBm
          Channel widths: 20MHz HT40-
        * 2452 MHz [9]
          Maximum TX power: 20.0 dBm
          Channel widths: 20MHz HT40-
        * 2457 MHz [10]
          Maximum TX power: 20.0 dBm
          Channel widths: 20MHz HT40-
        * 2462 MHz [11]
          Maximum TX power: 20.0 dBm
          Channel widths: 20MHz HT40-
        * 2467 MHz [12] (disabled)
        * 2472 MHz [13] (disabled)
        * 2484 MHz [14] (disabled)

-- 
Rafał
--
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

Re: [PATCH 2/2] cfg80211: reg: support ieee80211-(min|max)-center-freq DT properties

[Arend van Spriel]

On 29-12-16 10:43, Rafał Miłecki wrote:
> On 29 December 2016 at 09:57, Arend van Spriel
> <arend.vanspriel@broadcom.com> wrote:
>> On 28-12-16 22:30, Rafał Miłecki wrote:
>>> On 28 December 2016 at 22:28, Rafał Miłecki <zajec5@gmail.com> wrote:
>>>> On 28 December 2016 at 22:07, 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>
>>>>>>
>>>>>> They allow specifying hardware limitations of supported channels. This
>>>>>> may be useful for specifying single band devices or devices that support
>>>>>> only some part of the whole band.
>>>>>> E.g. some tri-band routers have separated radios for lower and higher
>>>>>> part of 5 GHz band.
>>>>>>
>>>>>> Signed-off-by: Rafał Miłecki <rafal@milecki.pl>
>>>>>> ---
>>>>>>  net/wireless/reg.c | 34 ++++++++++++++++++++++++++++++++++
>>>>>>  1 file changed, 34 insertions(+)
>>>>>>
>>>>>> diff --git a/net/wireless/reg.c b/net/wireless/reg.c
>>>>>> index 5dbac37..35ba5c7 100644
>>>>>> --- a/net/wireless/reg.c
>>>>>> +++ b/net/wireless/reg.c
>>>>>> @@ -1123,6 +1123,26 @@ const char *reg_initiator_name(enum nl80211_reg_initiator initiator)
>>>>>>  }
>>>>>>  EXPORT_SYMBOL(reg_initiator_name);
>>>>>>
>>>>>> +static bool reg_center_freq_of_valid(struct wiphy *wiphy,
>>>>>> +                                  struct ieee80211_channel *chan)
>>>>>> +{
>>>>>> +     struct device_node *np = wiphy_dev(wiphy)->of_node;
>>>>>> +     u32 val;
>>>>>> +
>>>>>> +     if (!np)
>>>>>> +             return true;
>>>>>> +
>>>>>> +     if (!of_property_read_u32(np, "ieee80211-min-center-freq", &val) &&
>>>>>> +         chan->center_freq < KHZ_TO_MHZ(val))
>>>>>> +             return false;
>>>>>> +
>>>>>> +     if (!of_property_read_u32(np, "ieee80211-max-center-freq", &val) &&
>>>>>> +         chan->center_freq > KHZ_TO_MHZ(val))
>>>>>> +             return false;
>>>>>
>>>>> I suspect these functions rely on CONFIG_OF. So might not build for
>>>>> !CONFIG_OF.
>>>>
>>>> I compiled it with
>>>> # CONFIG_OF is not set
>>>>
>>>> Can you test it and provide a non-working config if you see a
>>>> compilation error, please?
>>>
>>> include/linux/of.h provides a lot of dummy static inline functions if
>>> CONFIG_OF is not used (they also allow compiler to drop most of the
>>> code).
>>
>> of_propeirty_read_u32 is static inline in of.h calling
>> of_property_read_u32_array, which has a dummy variant in of.h returning
>> -ENOSYS so -38. Pretty sure that is not what you want. At least it does
>> not allow the compiler to drop any code so probably better to do:
>>
>> if (!IS_ENABLED(CONFIG_OF) || !np)
>>         return true;
> 
> Please verify that using a compiler. If there is a problem I'll be
> happy to work on it, but I need a proof it exists.

I am on vacation right now so not having much more than email and web
browser to use as review reference.

> If compilers sees a:
> if (!-ENOSYS && chan->center_freq < KHZ_TO_MHZ(val))
> condition, it's pretty clear it can be dropped. With both conditional
> blocks dropped function always returns "true" and... can be dropped.
> 
> Let me see if I can convince you with the following test:

No need to convince me. I made a mistake reviewing the code. Thanks for
clarifying it.

> $ grep -m 1 CONFIG_OF .config
> # CONFIG_OF is not set
> $ objdump --syms net/wireless/reg.o | grep -c reg_center_freq_of_valid
> 0
> 
> $ grep -m 1 CONFIG_OF .config
> CONFIG_OF=y
> $ objdump --syms net/wireless/reg.o | grep -c reg_center_freq_of_valid
> 1
> 
> 
>> So with this patch you change the channel to DISABLED. I am not very
>> familiar with reg.c so do you know if this is done before or after
>> calling regulatory notifier in the driver. brcmfmac will enable channels
>> querying the device upon regulatory notifier call, which may undo the
>> behavior introduced by your patch.
> 
> I'm not regulatory export, so I hope someone will review this patch.
> So far I can say it works for me after trying it on SR400ac with
> BCM43602.

But you probably do not have a mapping table for mapping country code
received in notifier to firmware regulatory code/revision. Only if you
have that, brcmfmac will update the channels in the bands.

Giving this some more consideration I am not sure if this is the proper
place to handle this. ieee80211-(min|max)-center-freq is platform
specific configuration allowing multiple cards to be used in different
(sub)bands. This has nothing to do with regulatory. So probably better
to move it to core.c or chan.c.

Regards,
Arend

Re: [PATCH 2/2] cfg80211: reg: support ieee80211-(min|max)-center-freq DT properties

[Arend van Spriel]

On 29-12-16 10:43, Rafał Miłecki wrote:
> On 29 December 2016 at 09:57, Arend van Spriel
> <arend.vanspriel-dY08KVG/lbpWk0Htik3J/w@public.gmane.org> wrote:
>> On 28-12-16 22:30, Rafał Miłecki wrote:
>>> On 28 December 2016 at 22:28, Rafał Miłecki <zajec5-Re5JQEeQqe8AvxtiuMwx3w@public.gmane.org> wrote:
>>>> On 28 December 2016 at 22:07, Arend van Spriel
>>>> <arend.vanspriel-dY08KVG/lbpWk0Htik3J/w@public.gmane.org> wrote:
>>>>> On 28-12-16 16:59, Rafał Miłecki wrote:
>>>>>> From: Rafał Miłecki <rafal-g1n6cQUeyibVItvQsEIGlw@public.gmane.org>
>>>>>>
>>>>>> They allow specifying hardware limitations of supported channels. This
>>>>>> may be useful for specifying single band devices or devices that support
>>>>>> only some part of the whole band.
>>>>>> E.g. some tri-band routers have separated radios for lower and higher
>>>>>> part of 5 GHz band.
>>>>>>
>>>>>> Signed-off-by: Rafał Miłecki <rafal-g1n6cQUeyibVItvQsEIGlw@public.gmane.org>
>>>>>> ---
>>>>>>  net/wireless/reg.c | 34 ++++++++++++++++++++++++++++++++++
>>>>>>  1 file changed, 34 insertions(+)
>>>>>>
>>>>>> diff --git a/net/wireless/reg.c b/net/wireless/reg.c
>>>>>> index 5dbac37..35ba5c7 100644
>>>>>> --- a/net/wireless/reg.c
>>>>>> +++ b/net/wireless/reg.c
>>>>>> @@ -1123,6 +1123,26 @@ const char *reg_initiator_name(enum nl80211_reg_initiator initiator)
>>>>>>  }
>>>>>>  EXPORT_SYMBOL(reg_initiator_name);
>>>>>>
>>>>>> +static bool reg_center_freq_of_valid(struct wiphy *wiphy,
>>>>>> +                                  struct ieee80211_channel *chan)
>>>>>> +{
>>>>>> +     struct device_node *np = wiphy_dev(wiphy)->of_node;
>>>>>> +     u32 val;
>>>>>> +
>>>>>> +     if (!np)
>>>>>> +             return true;
>>>>>> +
>>>>>> +     if (!of_property_read_u32(np, "ieee80211-min-center-freq", &val) &&
>>>>>> +         chan->center_freq < KHZ_TO_MHZ(val))
>>>>>> +             return false;
>>>>>> +
>>>>>> +     if (!of_property_read_u32(np, "ieee80211-max-center-freq", &val) &&
>>>>>> +         chan->center_freq > KHZ_TO_MHZ(val))
>>>>>> +             return false;
>>>>>
>>>>> I suspect these functions rely on CONFIG_OF. So might not build for
>>>>> !CONFIG_OF.
>>>>
>>>> I compiled it with
>>>> # CONFIG_OF is not set
>>>>
>>>> Can you test it and provide a non-working config if you see a
>>>> compilation error, please?
>>>
>>> include/linux/of.h provides a lot of dummy static inline functions if
>>> CONFIG_OF is not used (they also allow compiler to drop most of the
>>> code).
>>
>> of_propeirty_read_u32 is static inline in of.h calling
>> of_property_read_u32_array, which has a dummy variant in of.h returning
>> -ENOSYS so -38. Pretty sure that is not what you want. At least it does
>> not allow the compiler to drop any code so probably better to do:
>>
>> if (!IS_ENABLED(CONFIG_OF) || !np)
>>         return true;
> 
> Please verify that using a compiler. If there is a problem I'll be
> happy to work on it, but I need a proof it exists.

I am on vacation right now so not having much more than email and web
browser to use as review reference.

> If compilers sees a:
> if (!-ENOSYS && chan->center_freq < KHZ_TO_MHZ(val))
> condition, it's pretty clear it can be dropped. With both conditional
> blocks dropped function always returns "true" and... can be dropped.
> 
> Let me see if I can convince you with the following test:

No need to convince me. I made a mistake reviewing the code. Thanks for
clarifying it.

> $ grep -m 1 CONFIG_OF .config
> # CONFIG_OF is not set
> $ objdump --syms net/wireless/reg.o | grep -c reg_center_freq_of_valid
> 0
> 
> $ grep -m 1 CONFIG_OF .config
> CONFIG_OF=y
> $ objdump --syms net/wireless/reg.o | grep -c reg_center_freq_of_valid
> 1
> 
> 
>> So with this patch you change the channel to DISABLED. I am not very
>> familiar with reg.c so do you know if this is done before or after
>> calling regulatory notifier in the driver. brcmfmac will enable channels
>> querying the device upon regulatory notifier call, which may undo the
>> behavior introduced by your patch.
> 
> I'm not regulatory export, so I hope someone will review this patch.
> So far I can say it works for me after trying it on SR400ac with
> BCM43602.

But you probably do not have a mapping table for mapping country code
received in notifier to firmware regulatory code/revision. Only if you
have that, brcmfmac will update the channels in the bands.

Giving this some more consideration I am not sure if this is the proper
place to handle this. ieee80211-(min|max)-center-freq is platform
specific configuration allowing multiple cards to be used in different
(sub)bands. This has nothing to do with regulatory. So probably better
to move it to core.c or chan.c.

Regards,
Arend
--
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

Re: [PATCH 2/2] cfg80211: reg: support ieee80211-(min|max)-center-freq DT properties

[Rafał Miłecki]

On 30 December 2016 at 21:20, Arend van Spriel
<arend.vanspriel@broadcom.com> wrote:
> On 29-12-16 10:43, Rafa=C5=82 Mi=C5=82ecki wrote:
>> On 29 December 2016 at 09:57, Arend van Spriel
>> <arend.vanspriel@broadcom.com> wrote:
>>> On 28-12-16 22:30, Rafa=C5=82 Mi=C5=82ecki wrote:
>>>> On 28 December 2016 at 22:28, Rafa=C5=82 Mi=C5=82ecki <zajec5@gmail.co=
m> wrote:
>>>>> On 28 December 2016 at 22:07, Arend van Spriel
>>>>> <arend.vanspriel@broadcom.com> wrote:
>>>>>> On 28-12-16 16:59, Rafa=C5=82 Mi=C5=82ecki wrote:
>>>>>>> From: Rafa=C5=82 Mi=C5=82ecki <rafal@milecki.pl>
>>>>>>>
>>>>>>> They allow specifying hardware limitations of supported channels. T=
his
>>>>>>> may be useful for specifying single band devices or devices that su=
pport
>>>>>>> only some part of the whole band.
>>>>>>> E.g. some tri-band routers have separated radios for lower and high=
er
>>>>>>> part of 5 GHz band.
>>>>>>>
>>>>>>> Signed-off-by: Rafa=C5=82 Mi=C5=82ecki <rafal@milecki.pl>
>>>>>>> ---
>>>>>>>  net/wireless/reg.c | 34 ++++++++++++++++++++++++++++++++++
>>>>>>>  1 file changed, 34 insertions(+)
>>>>>>>
>>>>>>> diff --git a/net/wireless/reg.c b/net/wireless/reg.c
>>>>>>> index 5dbac37..35ba5c7 100644
>>>>>>> --- a/net/wireless/reg.c
>>>>>>> +++ b/net/wireless/reg.c
>>>>>>> @@ -1123,6 +1123,26 @@ const char *reg_initiator_name(enum nl80211_=
reg_initiator initiator)
>>>>>>>  }
>>>>>>>  EXPORT_SYMBOL(reg_initiator_name);
>>>>>>>
>>>>>>> +static bool reg_center_freq_of_valid(struct wiphy *wiphy,
>>>>>>> +                                  struct ieee80211_channel *chan)
>>>>>>> +{
>>>>>>> +     struct device_node *np =3D wiphy_dev(wiphy)->of_node;
>>>>>>> +     u32 val;
>>>>>>> +
>>>>>>> +     if (!np)
>>>>>>> +             return true;
>>>>>>> +
>>>>>>> +     if (!of_property_read_u32(np, "ieee80211-min-center-freq", &v=
al) &&
>>>>>>> +         chan->center_freq < KHZ_TO_MHZ(val))
>>>>>>> +             return false;
>>>>>>> +
>>>>>>> +     if (!of_property_read_u32(np, "ieee80211-max-center-freq", &v=
al) &&
>>>>>>> +         chan->center_freq > KHZ_TO_MHZ(val))
>>>>>>> +             return false;
>>>>>>
>>>>>> I suspect these functions rely on CONFIG_OF. So might not build for
>>>>>> !CONFIG_OF.
>>>>>
>>>>> I compiled it with
>>>>> # CONFIG_OF is not set
>>>>>
>>>>> Can you test it and provide a non-working config if you see a
>>>>> compilation error, please?
>>>>
>>>> include/linux/of.h provides a lot of dummy static inline functions if
>>>> CONFIG_OF is not used (they also allow compiler to drop most of the
>>>> code).
>>>
>>> of_propeirty_read_u32 is static inline in of.h calling
>>> of_property_read_u32_array, which has a dummy variant in of.h returning
>>> -ENOSYS so -38. Pretty sure that is not what you want. At least it does
>>> not allow the compiler to drop any code so probably better to do:
>>>
>>> if (!IS_ENABLED(CONFIG_OF) || !np)
>>>         return true;
>>
>> Please verify that using a compiler. If there is a problem I'll be
>> happy to work on it, but I need a proof it exists.
>
> I am on vacation right now so not having much more than email and web
> browser to use as review reference.
>
>> If compilers sees a:
>> if (!-ENOSYS && chan->center_freq < KHZ_TO_MHZ(val))
>> condition, it's pretty clear it can be dropped. With both conditional
>> blocks dropped function always returns "true" and... can be dropped.
>>
>> Let me see if I can convince you with the following test:
>
> No need to convince me. I made a mistake reviewing the code. Thanks for
> clarifying it.
>
>> $ grep -m 1 CONFIG_OF .config
>> # CONFIG_OF is not set
>> $ objdump --syms net/wireless/reg.o | grep -c reg_center_freq_of_valid
>> 0
>>
>> $ grep -m 1 CONFIG_OF .config
>> CONFIG_OF=3Dy
>> $ objdump --syms net/wireless/reg.o | grep -c reg_center_freq_of_valid
>> 1
>>
>>
>>> So with this patch you change the channel to DISABLED. I am not very
>>> familiar with reg.c so do you know if this is done before or after
>>> calling regulatory notifier in the driver. brcmfmac will enable channel=
s
>>> querying the device upon regulatory notifier call, which may undo the
>>> behavior introduced by your patch.
>>
>> I'm not regulatory export, so I hope someone will review this patch.
>> So far I can say it works for me after trying it on SR400ac with
>> BCM43602.
>
> But you probably do not have a mapping table for mapping country code
> received in notifier to firmware regulatory code/revision. Only if you
> have that, brcmfmac will update the channels in the bands.

Thanks, I'll redo my tests.


> Giving this some more consideration I am not sure if this is the proper
> place to handle this. ieee80211-(min|max)-center-freq is platform
> specific configuration allowing multiple cards to be used in different
> (sub)bands. This has nothing to do with regulatory. So probably better
> to move it to core.c or chan.c.

I can see point in this, I'll see if I can put this code in a more
proper place. Thanks for your comment!

--=20
Rafa=C5=82

Re: [PATCH 2/2] cfg80211: reg: support ieee80211-(min|max)-center-freq DT properties

[Rafał Miłecki]

On 30 December 2016 at 21:20, Arend van Spriel
<arend.vanspriel-dY08KVG/lbpWk0Htik3J/w@public.gmane.org> wrote:
> On 29-12-16 10:43, Rafał Miłecki wrote:
>> On 29 December 2016 at 09:57, Arend van Spriel
>> <arend.vanspriel-dY08KVG/lbpWk0Htik3J/w@public.gmane.org> wrote:
>>> On 28-12-16 22:30, Rafał Miłecki wrote:
>>>> On 28 December 2016 at 22:28, Rafał Miłecki <zajec5-Re5JQEeQqe8@public.gmane.orgm> wrote:
>>>>> On 28 December 2016 at 22:07, Arend van Spriel
>>>>> <arend.vanspriel-dY08KVG/lbpWk0Htik3J/w@public.gmane.org> wrote:
>>>>>> On 28-12-16 16:59, Rafał Miłecki wrote:
>>>>>>> From: Rafał Miłecki <rafal-g1n6cQUeyibVItvQsEIGlw@public.gmane.org>
>>>>>>>
>>>>>>> They allow specifying hardware limitations of supported channels. This
>>>>>>> may be useful for specifying single band devices or devices that support
>>>>>>> only some part of the whole band.
>>>>>>> E.g. some tri-band routers have separated radios for lower and higher
>>>>>>> part of 5 GHz band.
>>>>>>>
>>>>>>> Signed-off-by: Rafał Miłecki <rafal-g1n6cQUeyibVItvQsEIGlw@public.gmane.org>
>>>>>>> ---
>>>>>>>  net/wireless/reg.c | 34 ++++++++++++++++++++++++++++++++++
>>>>>>>  1 file changed, 34 insertions(+)
>>>>>>>
>>>>>>> diff --git a/net/wireless/reg.c b/net/wireless/reg.c
>>>>>>> index 5dbac37..35ba5c7 100644
>>>>>>> --- a/net/wireless/reg.c
>>>>>>> +++ b/net/wireless/reg.c
>>>>>>> @@ -1123,6 +1123,26 @@ const char *reg_initiator_name(enum nl80211_reg_initiator initiator)
>>>>>>>  }
>>>>>>>  EXPORT_SYMBOL(reg_initiator_name);
>>>>>>>
>>>>>>> +static bool reg_center_freq_of_valid(struct wiphy *wiphy,
>>>>>>> +                                  struct ieee80211_channel *chan)
>>>>>>> +{
>>>>>>> +     struct device_node *np = wiphy_dev(wiphy)->of_node;
>>>>>>> +     u32 val;
>>>>>>> +
>>>>>>> +     if (!np)
>>>>>>> +             return true;
>>>>>>> +
>>>>>>> +     if (!of_property_read_u32(np, "ieee80211-min-center-freq", &val) &&
>>>>>>> +         chan->center_freq < KHZ_TO_MHZ(val))
>>>>>>> +             return false;
>>>>>>> +
>>>>>>> +     if (!of_property_read_u32(np, "ieee80211-max-center-freq", &val) &&
>>>>>>> +         chan->center_freq > KHZ_TO_MHZ(val))
>>>>>>> +             return false;
>>>>>>
>>>>>> I suspect these functions rely on CONFIG_OF. So might not build for
>>>>>> !CONFIG_OF.
>>>>>
>>>>> I compiled it with
>>>>> # CONFIG_OF is not set
>>>>>
>>>>> Can you test it and provide a non-working config if you see a
>>>>> compilation error, please?
>>>>
>>>> include/linux/of.h provides a lot of dummy static inline functions if
>>>> CONFIG_OF is not used (they also allow compiler to drop most of the
>>>> code).
>>>
>>> of_propeirty_read_u32 is static inline in of.h calling
>>> of_property_read_u32_array, which has a dummy variant in of.h returning
>>> -ENOSYS so -38. Pretty sure that is not what you want. At least it does
>>> not allow the compiler to drop any code so probably better to do:
>>>
>>> if (!IS_ENABLED(CONFIG_OF) || !np)
>>>         return true;
>>
>> Please verify that using a compiler. If there is a problem I'll be
>> happy to work on it, but I need a proof it exists.
>
> I am on vacation right now so not having much more than email and web
> browser to use as review reference.
>
>> If compilers sees a:
>> if (!-ENOSYS && chan->center_freq < KHZ_TO_MHZ(val))
>> condition, it's pretty clear it can be dropped. With both conditional
>> blocks dropped function always returns "true" and... can be dropped.
>>
>> Let me see if I can convince you with the following test:
>
> No need to convince me. I made a mistake reviewing the code. Thanks for
> clarifying it.
>
>> $ grep -m 1 CONFIG_OF .config
>> # CONFIG_OF is not set
>> $ objdump --syms net/wireless/reg.o | grep -c reg_center_freq_of_valid
>> 0
>>
>> $ grep -m 1 CONFIG_OF .config
>> CONFIG_OF=y
>> $ objdump --syms net/wireless/reg.o | grep -c reg_center_freq_of_valid
>> 1
>>
>>
>>> So with this patch you change the channel to DISABLED. I am not very
>>> familiar with reg.c so do you know if this is done before or after
>>> calling regulatory notifier in the driver. brcmfmac will enable channels
>>> querying the device upon regulatory notifier call, which may undo the
>>> behavior introduced by your patch.
>>
>> I'm not regulatory export, so I hope someone will review this patch.
>> So far I can say it works for me after trying it on SR400ac with
>> BCM43602.
>
> But you probably do not have a mapping table for mapping country code
> received in notifier to firmware regulatory code/revision. Only if you
> have that, brcmfmac will update the channels in the bands.

Thanks, I'll redo my tests.


> Giving this some more consideration I am not sure if this is the proper
> place to handle this. ieee80211-(min|max)-center-freq is platform
> specific configuration allowing multiple cards to be used in different
> (sub)bands. This has nothing to do with regulatory. So probably better
> to move it to core.c or chan.c.

I can see point in this, I'll see if I can put this code in a more
proper place. Thanks for your comment!

-- 
Rafał
--
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

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

[Rob Herring]

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

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

[Rob Herring]

On Wed, Dec 28, 2016 at 04:59:54PM +0100, Rafał Miłecki wrote:
> From: Rafał Miłecki <rafal-g1n6cQUeyibVItvQsEIGlw@public.gmane.org>
> 
> 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-g1n6cQUeyibVItvQsEIGlw@public.gmane.org>
> ---
>  .../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-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
> More majordomo info at  http://vger.kernel.org/majordomo-info.html
--
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

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

[Rafał Miłecki]

On 3 January 2017 at 20:55, Rob Herring <robh@kernel.org> wrote:
> On Wed, Dec 28, 2016 at 04:59:54PM +0100, Rafa=C5=82 Mi=C5=82ecki wrote:
>> From: Rafa=C5=82 Mi=C5=82ecki <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=C5=82 Mi=C5=82ecki <rafal@milecki.pl>
>> ---
>>  .../devicetree/bindings/net/wireless/ieee80211.txt       | 16 +++++++++=
+++++++
>>  1 file changed, 16 insertions(+)
>>  create mode 100644 Documentation/devicetree/bindings/net/wireless/ieee8=
0211.txt
>>
>> diff --git a/Documentation/devicetree/bindings/net/wireless/ieee80211.tx=
t 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/

--=20
Rafa=C5=82

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

[Rafał Miłecki]

On 3 January 2017 at 20:55, Rob Herring <robh-DgEjT+Ai2ygdnm+yROfE0A@public.gmane.org> wrote:
> On Wed, Dec 28, 2016 at 04:59:54PM +0100, Rafał Miłecki wrote:
>> From: Rafał Miłecki <rafal-g1n6cQUeyibVItvQsEIGlw@public.gmane.org>
>>
>> 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-g1n6cQUeyibVItvQsEIGlw@public.gmane.org>
>> ---
>>  .../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/

-- 
Rafał

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

[Kalle Valo]

Rafa=C5=82 Mi=C5=82ecki <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=C5=82 Mi=C5=82ecki wrote:
>>> From: Rafa=C5=82 Mi=C5=82ecki <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?

--=20
Kalle Valo

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

[Kalle Valo]

Rafał Miłecki <zajec5-Re5JQEeQqe8AvxtiuMwx3w@public.gmane.org> writes:

> On 3 January 2017 at 20:55, Rob Herring <robh-DgEjT+Ai2ygdnm+yROfE0A@public.gmane.org> wrote:
>> On Wed, Dec 28, 2016 at 04:59:54PM +0100, Rafał Miłecki wrote:
>>> From: Rafał Miłecki <rafal-g1n6cQUeyibVItvQsEIGlw@public.gmane.org>
>>>
>>> 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?

-- 
Kalle Valo

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

[Rafał Miłecki]

On 4 January 2017 at 15:32, Kalle Valo <kvalo@codeaurora.org> wrote:
> Rafa=C5=82 Mi=C5=82ecki <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=C5=82 Mi=C5=82ecki wrote=
:
>>>> From: Rafa=C5=82 Mi=C5=82ecki <rafal@milecki.pl>
>>>>
>>>> This new file should be used for properties handled at higher level an=
d
>>>> 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."

--=20
Rafa=C5=82

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

[Rafał Miłecki]

On 4 January 2017 at 15:32, Kalle Valo <kvalo-sgV2jX0FEOL9JmXXK+q4OQ@public.gmane.org> wrote:
> Rafał Miłecki <zajec5-Re5JQEeQqe8AvxtiuMwx3w@public.gmane.org> writes:
>
>> On 3 January 2017 at 20:55, Rob Herring <robh-DgEjT+Ai2ygdnm+yROfE0A@public.gmane.org> wrote:
>>> On Wed, Dec 28, 2016 at 04:59:54PM +0100, Rafał Miłecki wrote:
>>>> From: Rafał Miłecki <rafal-g1n6cQUeyibVItvQsEIGlw@public.gmane.org>
>>>>
>>>> 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."

-- 
Rafał

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

[Arend Van Spriel]

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

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

[Arend Van Spriel]

On 4-1-2017 15:53, Rafał Miłecki wrote:
> On 4 January 2017 at 15:32, Kalle Valo <kvalo-sgV2jX0FEOL9JmXXK+q4OQ@public.gmane.org> wrote:
>> Rafał Miłecki <zajec5-Re5JQEeQqe8AvxtiuMwx3w@public.gmane.org> writes:
>>
>>> On 3 January 2017 at 20:55, Rob Herring <robh-DgEjT+Ai2ygdnm+yROfE0A@public.gmane.org> wrote:
>>>> On Wed, Dec 28, 2016 at 04:59:54PM +0100, Rafał Miłecki wrote:
>>>>> From: Rafał Miłecki <rafal-g1n6cQUeyibVItvQsEIGlw@public.gmane.org>
>>>>>
>>>>> 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