FW stats does provide the Rx rate information. Add this.
Tested with firmware 10.1.467.2-1.
Further investigation on firmware 999.999.0.636 indicates
that there is no Tx Rate and Rx Rate in the peer stats.
Signed-off-by: Chun-Yeow Yeoh <[email protected]>
---
drivers/net/wireless/ath/ath10k/core.h | 1 +
drivers/net/wireless/ath/ath10k/debug.c | 5 +++++
drivers/net/wireless/ath/ath10k/wmi.h | 1 +
3 files changed, 7 insertions(+)
diff --git a/drivers/net/wireless/ath/ath10k/core.h b/drivers/net/wireless/ath/ath10k/core.h
index ad209a6..ca4cdab 100644
--- a/drivers/net/wireless/ath/ath10k/core.h
+++ b/drivers/net/wireless/ath/ath10k/core.h
@@ -119,6 +119,7 @@ struct ath10k_peer_stat {
u8 peer_macaddr[ETH_ALEN];
u32 peer_rssi;
u32 peer_tx_rate;
+ u32 peer_rx_rate;
};
struct ath10k_target_stats {
diff --git a/drivers/net/wireless/ath/ath10k/debug.c b/drivers/net/wireless/ath/ath10k/debug.c
index f95defa..4662cb7 100644
--- a/drivers/net/wireless/ath/ath10k/debug.c
+++ b/drivers/net/wireless/ath/ath10k/debug.c
@@ -257,6 +257,8 @@ void ath10k_debug_read_target_stats(struct ath10k *ar,
s->peer_rssi = __le32_to_cpu(peer_stats->peer_rssi);
s->peer_tx_rate =
__le32_to_cpu(peer_stats->peer_tx_rate);
+ s->peer_rx_rate =
+ __le32_to_cpu(peer_stats->peer_rx_rate);
tmp += sizeof(struct wmi_peer_stats);
}
@@ -425,6 +427,9 @@ static ssize_t ath10k_read_fw_stats(struct file *file, char __user *user_buf,
len += scnprintf(buf + len, buf_len - len, "%30s %u\n",
"Peer TX rate",
fw_stats->peer_stat[i].peer_tx_rate);
+ len += scnprintf(buf + len, buf_len - len, "%30s %u\n",
+ "Peer RX rate",
+ fw_stats->peer_stat[i].peer_rx_rate);
len += scnprintf(buf + len, buf_len - len, "\n");
}
spin_unlock_bh(&ar->data_lock);
diff --git a/drivers/net/wireless/ath/ath10k/wmi.h b/drivers/net/wireless/ath/ath10k/wmi.h
index 084bcc5..2b2f0b7 100644
--- a/drivers/net/wireless/ath/ath10k/wmi.h
+++ b/drivers/net/wireless/ath/ath10k/wmi.h
@@ -2825,6 +2825,7 @@ struct wmi_peer_stats {
struct wmi_mac_addr peer_macaddr;
__le32 peer_rssi;
__le32 peer_tx_rate;
+ __le32 peer_rx_rate;
} __packed;
struct wmi_vdev_create_cmd {
--
1.7.9.5
> Unfortunately this isn't as simple as it seems.
>
> Stats are received via wmi_stats_event. This structure contains fields
> describing number of pdev/vdev/beer/bcn stat entries. All entries
> (structures) are appended in certain order after this header. This
> means if you change length of structure type you skew the calculation
> of all succeeding entries.
>
> IOW this means this patch breaks peer stats for 999.999.0.636.
>
> I'm quite positive wmi_pdev_stats is also broken. This implies vdev
> stats and peer stats position calculation is skewed in the first place
> and you get garbage.
>
> The main problem here is there are subtle yet crazy binary
> incompatibilities between these firmwares.
>
> The best way would probably be to implement wmi as an abstraction
> layer with completely different backends for different firmware
> branches/revisions. Otherwise you sign up for some pain..
Thanks for your explanation. I will definitely take a look on this if
I am able to figure out.
This patch seems work well with patch from Ben Greear on latest AP firmware.
----
Chun-Yeow
On 03/21/2014 09:25 AM, Kalle Valo wrote:
> Michal Kazior <[email protected]> writes:
>
>> The main problem here is there are subtle yet crazy binary
>> incompatibilities between these firmwares.
>>
>> The best way would probably be to implement wmi as an abstraction
>> layer with completely different backends for different firmware
>> branches/revisions. Otherwise you sign up for some pain..
>
> Yeah, I'm starting to believe that we will need something like that.
> Other option is just to duplicate wmi.c and wmi.h for each interface
> version. More code, but we get to keep more hair ;)
>
> But we need to talk a lot more about this. For this patch in question we
> should be able to manage with the current method of handling
> differences.
Lets not duplicate any more code than we have to. Hopefully we can
get a solid firmware built that everyone can use and let the old firmware
support eventually go away. In the meantime, we can add hacks as needed
to deal with firmware differences.
Thanks,
Ben
--
Ben Greear <[email protected]>
Candela Technologies Inc http://www.candelatech.com
Michal Kazior <[email protected]> writes:
> The main problem here is there are subtle yet crazy binary
> incompatibilities between these firmwares.
>
> The best way would probably be to implement wmi as an abstraction
> layer with completely different backends for different firmware
> branches/revisions. Otherwise you sign up for some pain..
Yeah, I'm starting to believe that we will need something like that.
Other option is just to duplicate wmi.c and wmi.h for each interface
version. More code, but we get to keep more hair ;)
But we need to talk a lot more about this. For this patch in question we
should be able to manage with the current method of handling
differences.
--
Kalle Valo
On 21 March 2014 11:00, Chun-Yeow Yeoh <[email protected]> wrote:
> FW stats does provide the Rx rate information. Add this.
> Tested with firmware 10.1.467.2-1.
>
> Further investigation on firmware 999.999.0.636 indicates
> that there is no Tx Rate and Rx Rate in the peer stats.
Incorrect.
999.999.0.636 does have Tx Rate. It doesn't have Rx Rate.
10.1.467.2-1 has both.
>
> Signed-off-by: Chun-Yeow Yeoh <[email protected]>
> ---
> drivers/net/wireless/ath/ath10k/core.h | 1 +
> drivers/net/wireless/ath/ath10k/debug.c | 5 +++++
> drivers/net/wireless/ath/ath10k/wmi.h | 1 +
> 3 files changed, 7 insertions(+)
>
> diff --git a/drivers/net/wireless/ath/ath10k/core.h b/drivers/net/wireless/ath/ath10k/core.h
> index ad209a6..ca4cdab 100644
> --- a/drivers/net/wireless/ath/ath10k/core.h
> +++ b/drivers/net/wireless/ath/ath10k/core.h
> @@ -119,6 +119,7 @@ struct ath10k_peer_stat {
> u8 peer_macaddr[ETH_ALEN];
> u32 peer_rssi;
> u32 peer_tx_rate;
> + u32 peer_rx_rate;
> };
>
> struct ath10k_target_stats {
> diff --git a/drivers/net/wireless/ath/ath10k/debug.c b/drivers/net/wireless/ath/ath10k/debug.c
> index f95defa..4662cb7 100644
> --- a/drivers/net/wireless/ath/ath10k/debug.c
> +++ b/drivers/net/wireless/ath/ath10k/debug.c
> @@ -257,6 +257,8 @@ void ath10k_debug_read_target_stats(struct ath10k *ar,
> s->peer_rssi = __le32_to_cpu(peer_stats->peer_rssi);
> s->peer_tx_rate =
> __le32_to_cpu(peer_stats->peer_tx_rate);
> + s->peer_rx_rate =
> + __le32_to_cpu(peer_stats->peer_rx_rate);
>
> tmp += sizeof(struct wmi_peer_stats);
> }
> @@ -425,6 +427,9 @@ static ssize_t ath10k_read_fw_stats(struct file *file, char __user *user_buf,
> len += scnprintf(buf + len, buf_len - len, "%30s %u\n",
> "Peer TX rate",
> fw_stats->peer_stat[i].peer_tx_rate);
> + len += scnprintf(buf + len, buf_len - len, "%30s %u\n",
> + "Peer RX rate",
> + fw_stats->peer_stat[i].peer_rx_rate);
> len += scnprintf(buf + len, buf_len - len, "\n");
> }
> spin_unlock_bh(&ar->data_lock);
> diff --git a/drivers/net/wireless/ath/ath10k/wmi.h b/drivers/net/wireless/ath/ath10k/wmi.h
> index 084bcc5..2b2f0b7 100644
> --- a/drivers/net/wireless/ath/ath10k/wmi.h
> +++ b/drivers/net/wireless/ath/ath10k/wmi.h
> @@ -2825,6 +2825,7 @@ struct wmi_peer_stats {
> struct wmi_mac_addr peer_macaddr;
> __le32 peer_rssi;
> __le32 peer_tx_rate;
> + __le32 peer_rx_rate;
> } __packed;
Unfortunately this isn't as simple as it seems.
Stats are received via wmi_stats_event. This structure contains fields
describing number of pdev/vdev/beer/bcn stat entries. All entries
(structures) are appended in certain order after this header. This
means if you change length of structure type you skew the calculation
of all succeeding entries.
IOW this means this patch breaks peer stats for 999.999.0.636.
I'm quite positive wmi_pdev_stats is also broken. This implies vdev
stats and peer stats position calculation is skewed in the first place
and you get garbage.
The main problem here is there are subtle yet crazy binary
incompatibilities between these firmwares.
The best way would probably be to implement wmi as an abstraction
layer with completely different backends for different firmware
branches/revisions. Otherwise you sign up for some pain..
Micha?