2016-05-11 19:53:25

by Jaap Jan Meijer

[permalink] [raw]
Subject: [PATCH] brcmfmac: add fallback for devices that do not report per-chain values

If brcmf_cfg80211_get_station fails to determine the RSSI from the
per-chain values get all values individually as a fallback.

Signed-off-by: Jaap Jan Meijer <[email protected]>
---
.../broadcom/brcm80211/brcmfmac/cfg80211.c | 42 ++++++++++++++++++++++
1 file changed, 42 insertions(+)

diff --git a/drivers/net/wireless/broadcom/brcm80211/brcmfmac/cfg80211.c b/drivers/net/wireless/broadcom/brcm80211/brcmfmac/cfg80211.c
index d5c2a27..60229c8 100644
--- a/drivers/net/wireless/broadcom/brcm80211/brcmfmac/cfg80211.c
+++ b/drivers/net/wireless/broadcom/brcm80211/brcmfmac/cfg80211.c
@@ -2480,12 +2480,16 @@ brcmf_cfg80211_get_station(struct wiphy *wiphy, struct net_device *ndev,
const u8 *mac, struct station_info *sinfo)
{
struct brcmf_if *ifp = netdev_priv(ndev);
+ struct brcmf_scb_val_le scb_val;
s32 err = 0;
struct brcmf_sta_info_le sta_info_le;
u32 sta_flags;
u32 is_tdls_peer;
s32 total_rssi;
s32 count_rssi;
+ int rssi;
+ u32 beacon_period;
+ u32 dtim_period;
u32 i;

brcmf_dbg(TRACE, "Enter, MAC %pM\n", mac);
@@ -2569,6 +2573,44 @@ brcmf_cfg80211_get_station(struct wiphy *wiphy, struct net_device *ndev,
sinfo->filled |= BIT(NL80211_STA_INFO_SIGNAL);
total_rssi /= count_rssi;
sinfo->signal = total_rssi;
+ } else if (test_bit(BRCMF_VIF_STATUS_CONNECTED,
+ &ifp->vif->sme_state)) {
+ memset(&scb_val, 0, sizeof(scb_val));
+ err = brcmf_fil_cmd_data_get(ifp, BRCMF_C_GET_RSSI,
+ &scb_val, sizeof(scb_val));
+ if (err) {
+ brcmf_err("Could not get rssi (%d)\n", err);
+ goto done;
+ } else {
+ rssi = le32_to_cpu(scb_val.val);
+ sinfo->filled |= BIT(NL80211_STA_INFO_SIGNAL);
+ sinfo->signal = rssi;
+ brcmf_dbg(CONN, "RSSI %d dBm\n", rssi);
+ }
+ err = brcmf_fil_cmd_int_get(ifp, BRCMF_C_GET_BCNPRD,
+ &beacon_period);
+ if (err) {
+ brcmf_err("Could not get beacon period (%d)\n",
+ err);
+ goto done;
+ } else {
+ sinfo->bss_param.beacon_interval =
+ beacon_period;
+ brcmf_dbg(CONN, "Beacon peroid %d\n",
+ beacon_period);
+ }
+ err = brcmf_fil_cmd_int_get(ifp, BRCMF_C_GET_DTIMPRD,
+ &dtim_period);
+ if (err) {
+ brcmf_err("Could not get DTIM period (%d)\n",
+ err);
+ goto done;
+ } else {
+ sinfo->bss_param.dtim_period = dtim_period;
+ brcmf_dbg(CONN, "DTIM peroid %d\n",
+ dtim_period);
+ }
+ sinfo->filled |= BIT(NL80211_STA_INFO_BSS_PARAM);
}
}
done:
--
1.9.1



2016-05-13 10:42:10

by Kalle Valo

[permalink] [raw]
Subject: Re: [PATCH v2] brcmfmac: add fallback for devices that do not report per-chain values

Jaap Jan Meijer <[email protected]> writes:

> If brcmf_cfg80211_get_station fails to determine the RSSI from the
> per-chain values get the value individually as a fallback.
>
> Fixes: 1f0dc59a6de9 ("brcmfmac: rework .get_station() callback")
>
> Signed-off-by: Jaap Jan Meijer <[email protected]>
> ---
> change log:
> v2:
> - remove beacon period and dtim, these values are obtained differently
> - add reverence to fixed commit

If no objections I'm planning to send this to 4.7-rc2.

--
Kalle Valo

2016-05-27 17:52:50

by Kalle Valo

[permalink] [raw]
Subject: Re: [v2] brcmfmac: add fallback for devices that do not report per-chain values

Jaap Jan Meijer <[email protected]> wrote:
> If brcmf_cfg80211_get_station fails to determine the RSSI from the
> per-chain values get the value individually as a fallback.
>
> Fixes: 1f0dc59a6de9 ("brcmfmac: rework .get_station() callback")
> Signed-off-by: Jaap Jan Meijer <[email protected]>
> Acked-by: Arend van Spriel <[email protected]>

Thanks, 1 patch applied to wireless-drivers.git:

94abd778a7bb brcmfmac: add fallback for devices that do not report per-chain values

--
Sent by pwcli
https://patchwork.kernel.org/patch/9084101/


2016-05-12 06:32:34

by Kalle Valo

[permalink] [raw]
Subject: Re: [PATCH] brcmfmac: add fallback for devices that do not report per-chain values

Arend van Spriel <[email protected]> writes:

> On 11-05-16 21:40, Jaap Jan Meijer wrote:
>> If brcmf_cfg80211_get_station fails to determine the RSSI from the
>> per-chain values get all values individually as a fallback.
>
> That commit message looks good.

This fixed a regression, right? So then a Fixes line would be nice:

https://git.kernel.org/cgit/linux/kernel/git/torvalds/linux.git/tree/Documentation/SubmittingPatches#n171

--
Kalle Valo

2016-05-12 21:37:38

by Arend Van Spriel

[permalink] [raw]
Subject: Re: [PATCH v2] brcmfmac: add fallback for devices that do not report per-chain values

On 12-05-16 18:25, Jaap Jan Meijer wrote:
> If brcmf_cfg80211_get_station fails to determine the RSSI from the
> per-chain values get the value individually as a fallback.
>
> Fixes: 1f0dc59a6de9 ("brcmfmac: rework .get_station() callback")

Acked-by: Arend van Spriel <[email protected]>
> Signed-off-by: Jaap Jan Meijer <[email protected]>
> ---
> change log:
> v2:
> - remove beacon period and dtim, these values are obtained differently
> - add reverence to fixed commit
> ---
> .../net/wireless/broadcom/brcm80211/brcmfmac/cfg80211.c | 16 ++++++++++++++++
> 1 file changed, 16 insertions(+)
>
> diff --git a/drivers/net/wireless/broadcom/brcm80211/brcmfmac/cfg80211.c b/drivers/net/wireless/broadcom/brcm80211/brcmfmac/cfg80211.c
> index d5c2a27..e19847c 100644
> --- a/drivers/net/wireless/broadcom/brcm80211/brcmfmac/cfg80211.c
> +++ b/drivers/net/wireless/broadcom/brcm80211/brcmfmac/cfg80211.c
> @@ -2480,12 +2480,14 @@ brcmf_cfg80211_get_station(struct wiphy *wiphy, struct net_device *ndev,
> const u8 *mac, struct station_info *sinfo)
> {
> struct brcmf_if *ifp = netdev_priv(ndev);
> + struct brcmf_scb_val_le scb_val;
> s32 err = 0;
> struct brcmf_sta_info_le sta_info_le;
> u32 sta_flags;
> u32 is_tdls_peer;
> s32 total_rssi;
> s32 count_rssi;
> + int rssi;
> u32 i;
>
> brcmf_dbg(TRACE, "Enter, MAC %pM\n", mac);
> @@ -2569,6 +2571,20 @@ brcmf_cfg80211_get_station(struct wiphy *wiphy, struct net_device *ndev,
> sinfo->filled |= BIT(NL80211_STA_INFO_SIGNAL);
> total_rssi /= count_rssi;
> sinfo->signal = total_rssi;
> + } else if (test_bit(BRCMF_VIF_STATUS_CONNECTED,
> + &ifp->vif->sme_state)) {
> + memset(&scb_val, 0, sizeof(scb_val));
> + err = brcmf_fil_cmd_data_get(ifp, BRCMF_C_GET_RSSI,
> + &scb_val, sizeof(scb_val));
> + if (err) {
> + brcmf_err("Could not get rssi (%d)\n", err);
> + goto done;
> + } else {
> + rssi = le32_to_cpu(scb_val.val);
> + sinfo->filled |= BIT(NL80211_STA_INFO_SIGNAL);
> + sinfo->signal = rssi;
> + brcmf_dbg(CONN, "RSSI %d dBm\n", rssi);
> + }
> }
> }
> done:
>

2016-05-12 16:19:46

by Jaap Jan Meijer

[permalink] [raw]
Subject: Re: [PATCH] brcmfmac: add fallback for devices that do not report per-chain values

2016-05-11 22:55 GMT+02:00 Arend van Spriel <[email protected]>:
> However, this patch changes more. I did not get back on your question
> regarding beacon period and dtim. Sorry for that as it means this patch
> still need rework. Those values are obtained differently and filled in
> by the function brcmf_fill_bss_param() (called in line 2530). When you
> do that make sure the subject prefix is 'PATCH v2' iso 'PATCH' (using
> git format-patch option --subject-prefix=..) and add a changelog after
> your signed-off line as shown below:
>
> Signed-off-by: Jaap Jan Meijer <[email protected]>
> ---
> change log:
> v2:
> - bla...
> ---
> .../broadcom/brcm80211/brcmfmac/cfg80211.c | 42
> ++++++++++++++++++++++
>
> Don't get discouraged :-D Nederlander?

I won't but I hope you won't either and yes, what gave it away? ;)


2016-05-12 8:32 GMT+02:00 Kalle Valo <[email protected]>:
>
> This fixed a regression, right? So then a Fixes line would be nice:
>
> https://git.kernel.org/cgit/linux/kernel/git/torvalds/linux.git/tree/Documentation/SubmittingPatches#n171
>
> --
> Kalle Valo

Thank you for the feedback guys, I will send a new patch shortly.

2016-05-12 16:25:44

by Jaap Jan Meijer

[permalink] [raw]
Subject: [PATCH v2] brcmfmac: add fallback for devices that do not report per-chain values

If brcmf_cfg80211_get_station fails to determine the RSSI from the
per-chain values get the value individually as a fallback.

Fixes: 1f0dc59a6de9 ("brcmfmac: rework .get_station() callback")

Signed-off-by: Jaap Jan Meijer <[email protected]>
---
change log:
v2:
- remove beacon period and dtim, these values are obtained differently
- add reverence to fixed commit
---
.../net/wireless/broadcom/brcm80211/brcmfmac/cfg80211.c | 16 ++++++++++++++++
1 file changed, 16 insertions(+)

diff --git a/drivers/net/wireless/broadcom/brcm80211/brcmfmac/cfg80211.c b/drivers/net/wireless/broadcom/brcm80211/brcmfmac/cfg80211.c
index d5c2a27..e19847c 100644
--- a/drivers/net/wireless/broadcom/brcm80211/brcmfmac/cfg80211.c
+++ b/drivers/net/wireless/broadcom/brcm80211/brcmfmac/cfg80211.c
@@ -2480,12 +2480,14 @@ brcmf_cfg80211_get_station(struct wiphy *wiphy, struct net_device *ndev,
const u8 *mac, struct station_info *sinfo)
{
struct brcmf_if *ifp = netdev_priv(ndev);
+ struct brcmf_scb_val_le scb_val;
s32 err = 0;
struct brcmf_sta_info_le sta_info_le;
u32 sta_flags;
u32 is_tdls_peer;
s32 total_rssi;
s32 count_rssi;
+ int rssi;
u32 i;

brcmf_dbg(TRACE, "Enter, MAC %pM\n", mac);
@@ -2569,6 +2571,20 @@ brcmf_cfg80211_get_station(struct wiphy *wiphy, struct net_device *ndev,
sinfo->filled |= BIT(NL80211_STA_INFO_SIGNAL);
total_rssi /= count_rssi;
sinfo->signal = total_rssi;
+ } else if (test_bit(BRCMF_VIF_STATUS_CONNECTED,
+ &ifp->vif->sme_state)) {
+ memset(&scb_val, 0, sizeof(scb_val));
+ err = brcmf_fil_cmd_data_get(ifp, BRCMF_C_GET_RSSI,
+ &scb_val, sizeof(scb_val));
+ if (err) {
+ brcmf_err("Could not get rssi (%d)\n", err);
+ goto done;
+ } else {
+ rssi = le32_to_cpu(scb_val.val);
+ sinfo->filled |= BIT(NL80211_STA_INFO_SIGNAL);
+ sinfo->signal = rssi;
+ brcmf_dbg(CONN, "RSSI %d dBm\n", rssi);
+ }
}
}
done:
--
1.9.1


2016-05-11 20:55:57

by Arend Van Spriel

[permalink] [raw]
Subject: Re: [PATCH] brcmfmac: add fallback for devices that do not report per-chain values



On 11-05-16 21:40, Jaap Jan Meijer wrote:
> If brcmf_cfg80211_get_station fails to determine the RSSI from the
> per-chain values get all values individually as a fallback.

That commit message looks good.

> Signed-off-by: Jaap Jan Meijer <[email protected]>
> ---
> .../broadcom/brcm80211/brcmfmac/cfg80211.c | 42 ++++++++++++++++++++++
> 1 file changed, 42 insertions(+)
>
> diff --git a/drivers/net/wireless/broadcom/brcm80211/brcmfmac/cfg80211.c b/drivers/net/wireless/broadcom/brcm80211/brcmfmac/cfg80211.c
> index d5c2a27..60229c8 100644
> --- a/drivers/net/wireless/broadcom/brcm80211/brcmfmac/cfg80211.c
> +++ b/drivers/net/wireless/broadcom/brcm80211/brcmfmac/cfg80211.c
> @@ -2480,12 +2480,16 @@ brcmf_cfg80211_get_station(struct wiphy *wiphy, struct net_device *ndev,
> const u8 *mac, struct station_info *sinfo)
> {
> struct brcmf_if *ifp = netdev_priv(ndev);
> + struct brcmf_scb_val_le scb_val;
> s32 err = 0;
> struct brcmf_sta_info_le sta_info_le;
> u32 sta_flags;
> u32 is_tdls_peer;
> s32 total_rssi;
> s32 count_rssi;
> + int rssi;
> + u32 beacon_period;
> + u32 dtim_period;
> u32 i;
>
> brcmf_dbg(TRACE, "Enter, MAC %pM\n", mac);
> @@ -2569,6 +2573,44 @@ brcmf_cfg80211_get_station(struct wiphy *wiphy, struct net_device *ndev,
> sinfo->filled |= BIT(NL80211_STA_INFO_SIGNAL);
> total_rssi /= count_rssi;
> sinfo->signal = total_rssi;
> + } else if (test_bit(BRCMF_VIF_STATUS_CONNECTED,
> + &ifp->vif->sme_state)) {
> + memset(&scb_val, 0, sizeof(scb_val));
> + err = brcmf_fil_cmd_data_get(ifp, BRCMF_C_GET_RSSI,
> + &scb_val, sizeof(scb_val));
> + if (err) {
> + brcmf_err("Could not get rssi (%d)\n", err);
> + goto done;
> + } else {
> + rssi = le32_to_cpu(scb_val.val);
> + sinfo->filled |= BIT(NL80211_STA_INFO_SIGNAL);
> + sinfo->signal = rssi;
> + brcmf_dbg(CONN, "RSSI %d dBm\n", rssi);
> + }

However, this patch changes more. I did not get back on your question
regarding beacon period and dtim. Sorry for that as it means this patch
still need rework. Those values are obtained differently and filled in
by the function brcmf_fill_bss_param() (called in line 2530). When you
do that make sure the subject prefix is 'PATCH v2' iso 'PATCH' (using
git format-patch option --subject-prefix=..) and add a changelog after
your signed-off line as shown below:

Signed-off-by: Jaap Jan Meijer <[email protected]>
---
change log:
v2:
- bla...
---
.../broadcom/brcm80211/brcmfmac/cfg80211.c | 42
++++++++++++++++++++++

Don't get discouraged :-D Nederlander?

Regards,
Arend

> + err = brcmf_fil_cmd_int_get(ifp, BRCMF_C_GET_BCNPRD,
> + &beacon_period);
> + if (err) {
> + brcmf_err("Could not get beacon period (%d)\n",
> + err);
> + goto done;
> + } else {
> + sinfo->bss_param.beacon_interval =
> + beacon_period;
> + brcmf_dbg(CONN, "Beacon peroid %d\n",
> + beacon_period);
> + }
> + err = brcmf_fil_cmd_int_get(ifp, BRCMF_C_GET_DTIMPRD,
> + &dtim_period);
> + if (err) {
> + brcmf_err("Could not get DTIM period (%d)\n",
> + err);
> + goto done;
> + } else {
> + sinfo->bss_param.dtim_period = dtim_period;
> + brcmf_dbg(CONN, "DTIM peroid %d\n",
> + dtim_period);
> + }
> + sinfo->filled |= BIT(NL80211_STA_INFO_BSS_PARAM);
> }
> }
> done:
>