Re: [PATCH] ath10k: implement per-VDEV FW statistics

From: Kalle Valo <kvalo@qca.qualcomm.com>
To: Bartosz Markowski <bartosz.markowski@tieto.com>
Cc: linux-wireless@vger.kernel.org, ath10k@lists.infradead.org
Subject: Re: [PATCH] ath10k: implement per-VDEV FW statistics
Date: Wed, 28 Aug 2013 13:30:14 +0300	[thread overview]
Message-ID: <87fvtuglah.fsf@kamboji.qca.qualcomm.com> (raw)
In-Reply-To: <CACKGw+E9sL_5ZTHFuf-OB4LJgodZTtD-Oj9Dxp8om6u0v5Sfrg@mail.gmail.com> (Bartosz Markowski's message of "Wed, 28 Aug 2013 12:21:36 +0200")

Bartosz Markowski <bartosz.markowski@tieto.com> writes:

> On 28 August 2013 12:13, Kalle Valo <kvalo@qca.qualcomm.com> wrote:
>> Bartosz Markowski <bartosz.markowski@tieto.com> writes:
>>
>>> The WMI_REQUEST_PEER_STAT command with latst (1.0.0.716) FW
>>> can return per-VDEV statistics. Using debugfs we can fetch this info now.
>>>
>>> This is a backward compatible change. In case of older FW the VDEV
>>> statistics are simply not returned.
>>>
>>> Signed-off-by: Bartosz Markowski <bartosz.markowski@tieto.com>
>>
>> [...]
>>
>>> diff --git a/drivers/net/wireless/ath/ath10k/wmi.h b/drivers/net/wireless/ath/ath10k/wmi.h
>>> index 08860c4..626bf02 100644
>>> --- a/drivers/net/wireless/ath/ath10k/wmi.h
>>> +++ b/drivers/net/wireless/ath/ath10k/wmi.h
>>> @@ -1767,6 +1772,9 @@ struct wal_dbg_tx_stats {
>>>       /* wal pdev resets  */
>>>       __le32 pdev_resets;
>>>
>>> +     /* frames dropped due to non-availability of stateless TIDs */
>>> +     __le32 stateless_tid_alloc_failure;
>>> +
>>>       __le32 phy_underrun;
>>>
>>>       /* MPDU is more than txop limit */
>>> @@ -1825,11 +1833,10 @@ enum wmi_stats_id {
>>>
>>>  struct wmi_request_stats_cmd {
>>>       __le32 stats_id;
>>> -
>>> -     /*
>>> -      * Space to add parameters like
>>> -      * peer mac addr
>>> -      */
>>> +     /* unique id identifying the VDEV, generated by the caller */
>>> +     __le32 vdev_id;
>>> +     /* peer MAC address */
>>> +     struct wmi_mac_addr peer_macaddr;
>>>  } __packed;
>>>
>>>  /* Suspend option */
>>
>> Don't these changes break the compability with the older firmware?
>
> This new wal_dbg_tx_stats filed is present also in the .636 fw release.
> So it compatible back to the first published fw for ath10k.

Then do it in a separate patch and mention the above.

>> Also, for consistency, please _v1 and _v2 suffix when having different
>> versions of wmi structs.
>
> I did. I will add the comment (as you mention in next mail) and send a V2

Actually Johannes gave a good tip: it's better to have a common struct
which both v1 and v2 use. That way we can avoid the evil cast.

-- 
Kalle Valo

_______________________________________________
ath10k mailing list
ath10k@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/ath10k