2014-03-21 05:34:58

by Chun-Yeow Yeoh

[permalink] [raw]
Subject: [PATCH] ath10k: fix the peer mac address in getting stats

Using the macro to convert the MAC address from WMI word
format to char array has lead to the wrong peer mac
address printed out while retrieving the peer stats from
FW. Fix this.

Signed-off-by: Chun-Yeow Yeoh <[email protected]>
---
drivers/net/wireless/ath/ath10k/debug.c | 4 ++--
drivers/net/wireless/ath/ath10k/wmi.h | 10 ----------
2 files changed, 2 insertions(+), 12 deletions(-)

diff --git a/drivers/net/wireless/ath/ath10k/debug.c b/drivers/net/wireless/ath/ath10k/debug.c
index 6debd28..f95defa 100644
--- a/drivers/net/wireless/ath/ath10k/debug.c
+++ b/drivers/net/wireless/ath/ath10k/debug.c
@@ -252,8 +252,8 @@ void ath10k_debug_read_target_stats(struct ath10k *ar,
peer_stats = (struct wmi_peer_stats *)tmp;
s = &stats->peer_stat[i];

- WMI_MAC_ADDR_TO_CHAR_ARRAY(&peer_stats->peer_macaddr,
- s->peer_macaddr);
+ memcpy(s->peer_macaddr, &peer_stats->peer_macaddr.addr,
+ ETH_ALEN);
s->peer_rssi = __le32_to_cpu(peer_stats->peer_rssi);
s->peer_tx_rate =
__le32_to_cpu(peer_stats->peer_tx_rate);
diff --git a/drivers/net/wireless/ath/ath10k/wmi.h b/drivers/net/wireless/ath/ath10k/wmi.h
index e78a7ad..084bcc5 100644
--- a/drivers/net/wireless/ath/ath10k/wmi.h
+++ b/drivers/net/wireless/ath/ath10k/wmi.h
@@ -198,16 +198,6 @@ struct wmi_mac_addr {
} __packed;
} __packed;

-/* macro to convert MAC address from WMI word format to char array */
-#define WMI_MAC_ADDR_TO_CHAR_ARRAY(pwmi_mac_addr, c_macaddr) do { \
- (c_macaddr)[0] = ((pwmi_mac_addr)->word0) & 0xff; \
- (c_macaddr)[1] = (((pwmi_mac_addr)->word0) >> 8) & 0xff; \
- (c_macaddr)[2] = (((pwmi_mac_addr)->word0) >> 16) & 0xff; \
- (c_macaddr)[3] = (((pwmi_mac_addr)->word0) >> 24) & 0xff; \
- (c_macaddr)[4] = ((pwmi_mac_addr)->word1) & 0xff; \
- (c_macaddr)[5] = (((pwmi_mac_addr)->word1) >> 8) & 0xff; \
- } while (0)
-
struct wmi_cmd_map {
u32 init_cmdid;
u32 start_scan_cmdid;
--
1.7.9.5



2014-03-24 08:30:44

by Michal Kazior

[permalink] [raw]
Subject: Re: [PATCH] ath10k: fix the peer mac address in getting stats

On 24 March 2014 09:22, Kalle Valo <[email protected]> wrote:
> Michal Kazior <[email protected]> writes:

[...]

>> Other than that the patch looks good.
>>
>> Reviewed-By: Michał Kazior <[email protected]>
>
> Thanks, I added this to the patch.
>
> Let's just hope that the l with the extra line doesn't cause any charset
> problems :) BTW, what is the name of the character? Never seen that
> before.

http://www.fileformat.info/info/unicode/char/142/index.htm

I don't care much if you s/ł/l/. I already commit patches with an l.


Michał

2014-03-24 08:33:12

by Kalle Valo

[permalink] [raw]
Subject: Re: [PATCH] ath10k: fix the peer mac address in getting stats

Chun-Yeow Yeoh <[email protected]> writes:

> Using the macro to convert the MAC address from WMI word
> format to char array has lead to the wrong peer mac
> address printed out while retrieving the peer stats from
> FW. Fix this.
>
> Signed-off-by: Chun-Yeow Yeoh <[email protected]>

Thanks, applied.

--
Kalle Valo

2014-03-21 06:51:23

by Michal Kazior

[permalink] [raw]
Subject: Re: [PATCH] ath10k: fix the peer mac address in getting stats

On 21 March 2014 06:34, Chun-Yeow Yeoh <[email protected]> wrote:
> Using the macro to convert the MAC address from WMI word
> format to char array has lead to the wrong peer mac
> address printed out while retrieving the peer stats from
> FW. Fix this.
>
> Signed-off-by: Chun-Yeow Yeoh <[email protected]>
> ---
> drivers/net/wireless/ath/ath10k/debug.c | 4 ++--
> drivers/net/wireless/ath/ath10k/wmi.h | 10 ----------
> 2 files changed, 2 insertions(+), 12 deletions(-)
>
> diff --git a/drivers/net/wireless/ath/ath10k/debug.c b/drivers/net/wireless/ath/ath10k/debug.c
> index 6debd28..f95defa 100644
> --- a/drivers/net/wireless/ath/ath10k/debug.c
> +++ b/drivers/net/wireless/ath/ath10k/debug.c
> @@ -252,8 +252,8 @@ void ath10k_debug_read_target_stats(struct ath10k *ar,
> peer_stats = (struct wmi_peer_stats *)tmp;
> s = &stats->peer_stat[i];
>
> - WMI_MAC_ADDR_TO_CHAR_ARRAY(&peer_stats->peer_macaddr,
> - s->peer_macaddr);
> + memcpy(s->peer_macaddr, &peer_stats->peer_macaddr.addr,
> + ETH_ALEN);

I noticed checkpatch started suggesting to prefer ether_addr_copy()
over memcpy(). Not a big deal though.

Other than that the patch looks good.

Reviewed-By: Micha? Kazior <[email protected]>


Micha?

2014-03-24 08:22:28

by Kalle Valo

[permalink] [raw]
Subject: Re: [PATCH] ath10k: fix the peer mac address in getting stats

Michal Kazior <[email protected]> writes:

> On 21 March 2014 06:34, Chun-Yeow Yeoh <[email protected]> wrote:
>> Using the macro to convert the MAC address from WMI word
>> format to char array has lead to the wrong peer mac
>> address printed out while retrieving the peer stats from
>> FW. Fix this.
>>
>> Signed-off-by: Chun-Yeow Yeoh <[email protected]>
>> ---
>> drivers/net/wireless/ath/ath10k/debug.c | 4 ++--
>> drivers/net/wireless/ath/ath10k/wmi.h | 10 ----------
>> 2 files changed, 2 insertions(+), 12 deletions(-)
>>
>> diff --git a/drivers/net/wireless/ath/ath10k/debug.c b/drivers/net/wireless/ath/ath10k/debug.c
>> index 6debd28..f95defa 100644
>> --- a/drivers/net/wireless/ath/ath10k/debug.c
>> +++ b/drivers/net/wireless/ath/ath10k/debug.c
>> @@ -252,8 +252,8 @@ void ath10k_debug_read_target_stats(struct ath10k *ar,
>> peer_stats = (struct wmi_peer_stats *)tmp;
>> s = &stats->peer_stat[i];
>>
>> - WMI_MAC_ADDR_TO_CHAR_ARRAY(&peer_stats->peer_macaddr,
>> - s->peer_macaddr);
>> + memcpy(s->peer_macaddr, &peer_stats->peer_macaddr.addr,
>> + ETH_ALEN);
>
> I noticed checkpatch started suggesting to prefer ether_addr_copy()
> over memcpy(). Not a big deal though.

Yeah, especially that I haven't updated my checkpatch for few months and
I don't see this warning yet :) (I'll try to keep ath10k checkpatch
clean whenever possible.)

We need to switch ath10k to use ether_addr_copy() everywhere in a
separate patch anyway.

> Other than that the patch looks good.
>
> Reviewed-By: Michał Kazior <[email protected]>

Thanks, I added this to the patch.

Let's just hope that the l with the extra line doesn't cause any charset
problems :) BTW, what is the name of the character? Never seen that
before.

--
Kalle Valo