linux-wireless.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
* [PATCH v2] ath10k: extend debug code for RX path
@ 2014-07-28  6:26 Janusz Dziedzic
  2014-07-28  6:35 ` Varka Bhadram
  2014-07-29  9:48 ` Kalle Valo
  0 siblings, 2 replies; 5+ messages in thread
From: Janusz Dziedzic @ 2014-07-28  6:26 UTC (permalink / raw)
  To: ath10k; +Cc: linux-wireless, Janusz Dziedzic

Print sequence number, AMSDU_MORE flag and AC when additional
debug enabled in RX path. This is usefull for debugging purpose.

Signed-off-by: Janusz Dziedzic <janusz.dziedzic@tieto.com>
---
 drivers/net/wireless/ath/ath10k/htt_rx.c |   41 ++++++++++++++++++++++++++++--
 1 file changed, 39 insertions(+), 2 deletions(-)

diff --git a/drivers/net/wireless/ath/ath10k/htt_rx.c b/drivers/net/wireless/ath/ath10k/htt_rx.c
index d80fcda..f7c8021 100644
--- a/drivers/net/wireless/ath/ath10k/htt_rx.c
+++ b/drivers/net/wireless/ath/ath10k/htt_rx.c
@@ -818,19 +818,55 @@ static bool ath10k_htt_rx_h_channel(struct ath10k *ar,
 	return true;
 }
 
+static const char * const tid_to_ac[] = {
+	"BE",
+	"BK",
+	"BK",
+	"BE",
+	"VI",
+	"VI",
+	"VO",
+	"VO",
+};
+
+static char *ath10k_get_tid(struct ieee80211_hdr *hdr, char *out, size_t size)
+{
+	u8 *qc;
+	int tid;
+
+	if (!ieee80211_is_data_qos(hdr->frame_control))
+		return "";
+
+	qc = ieee80211_get_qos_ctl(hdr);
+	tid = *qc & IEEE80211_QOS_CTL_TID_MASK;
+	if (tid < 8)
+		snprintf(out, size, "tid %d (%s)", tid, tid_to_ac[tid]);
+	else
+		snprintf(out, size, "tid %d", tid);
+
+	return out;
+}
+
 static void ath10k_process_rx(struct ath10k *ar,
 			      struct ieee80211_rx_status *rx_status,
 			      struct sk_buff *skb)
 {
 	struct ieee80211_rx_status *status;
+	struct ieee80211_hdr *hdr = (struct ieee80211_hdr *)skb->data;
+	char tid[32];
 
 	status = IEEE80211_SKB_RXCB(skb);
 	*status = *rx_status;
 
 	ath10k_dbg(ATH10K_DBG_DATA,
-		   "rx skb %p len %u %s%s%s%s%s %srate_idx %u vht_nss %u freq %u band %u flag 0x%x fcs-err %imic-err %i\n",
+		   "rx skb %p len %u peer %pM %s %s sn %u %s%s%s%s%s %srate_idx %u vht_nss %u freq %u band %u flag 0x%x fcs-err %i mic-err %i amsdu-more %i\n",
 		   skb,
 		   skb->len,
+		   ieee80211_get_SA(hdr),
+		   ath10k_get_tid(hdr, tid, sizeof(tid)),
+		   is_multicast_ether_addr(ieee80211_get_DA(hdr)) ?
+							"mcast" : "ucast",
+		   (__le16_to_cpu(hdr->seq_ctrl) & IEEE80211_SCTL_SEQ) >> 4,
 		   status->flag == 0 ? "legacy" : "",
 		   status->flag & RX_FLAG_HT ? "ht" : "",
 		   status->flag & RX_FLAG_VHT ? "vht" : "",
@@ -842,7 +878,8 @@ static void ath10k_process_rx(struct ath10k *ar,
 		   status->freq,
 		   status->band, status->flag,
 		   !!(status->flag & RX_FLAG_FAILED_FCS_CRC),
-		   !!(status->flag & RX_FLAG_MMIC_ERROR));
+		   !!(status->flag & RX_FLAG_MMIC_ERROR),
+		   !!(status->flag & RX_FLAG_AMSDU_MORE));
 	ath10k_dbg_dump(ATH10K_DBG_HTT_DUMP, NULL, "rx skb: ",
 			skb->data, skb->len);
 
-- 
1.7.9.5


^ permalink raw reply related	[flat|nested] 5+ messages in thread

* Re: [PATCH v2] ath10k: extend debug code for RX path
  2014-07-28  6:26 [PATCH v2] ath10k: extend debug code for RX path Janusz Dziedzic
@ 2014-07-28  6:35 ` Varka Bhadram
  2014-07-28  7:52   ` Janusz Dziedzic
  2014-07-29  9:48 ` Kalle Valo
  1 sibling, 1 reply; 5+ messages in thread
From: Varka Bhadram @ 2014-07-28  6:35 UTC (permalink / raw)
  To: Janusz Dziedzic, ath10k; +Cc: linux-wireless

On 07/28/2014 11:56 AM, Janusz Dziedzic wrote:
> Print sequence number, AMSDU_MORE flag and AC when additional
> debug enabled in RX path. This is usefull for debugging purpose.
>
> Signed-off-by: Janusz Dziedzic<janusz.dziedzic@tieto.com>
> ---
>   drivers/net/wireless/ath/ath10k/htt_rx.c |   41 ++++++++++++++++++++++++++++--
>   1 file changed, 39 insertions(+), 2 deletions(-)
>
> diff --git a/drivers/net/wireless/ath/ath10k/htt_rx.c b/drivers/net/wireless/ath/ath10k/htt_rx.c
> index d80fcda..f7c8021 100644
> --- a/drivers/net/wireless/ath/ath10k/htt_rx.c
> +++ b/drivers/net/wireless/ath/ath10k/htt_rx.c
> @@ -818,19 +818,55 @@ static bool ath10k_htt_rx_h_channel(struct ath10k *ar,
>   	return true;
>   }
>   
> +static const char * const tid_to_ac[] = {

No space required '*' and const.

static const char *const tid_to_ac[] = {


> +	"BE",
> +	"BK",
> +	"BK",
> +	"BE",
> +	"VI",
> +	"VI",
> +	"VO",
> +	"VO",
> +};
> +
> +static char *ath10k_get_tid(struct ieee80211_hdr *hdr, char *out, size_t size)
> +{
> +	u8 *qc;
> +	int tid;
> +
> +	if (!ieee80211_is_data_qos(hdr->frame_control))
> +		return "";

return NULL would be more readable...?

> +
> +	qc = ieee80211_get_qos_ctl(hdr);
> +	tid = *qc & IEEE80211_QOS_CTL_TID_MASK;
> +	if (tid < 8)
> +		snprintf(out, size, "tid %d (%s)", tid, tid_to_ac[tid]);
> +	else
> +		snprintf(out, size, "tid %d", tid);
> +
> +	return out;
> +}
> +
>   static void ath10k_process_rx(struct ath10k *ar,
>   			      struct ieee80211_rx_status *rx_status,
>   			      struct sk_buff *skb)
>   {
>   	struct ieee80211_rx_status *status;
> +	struct ieee80211_hdr *hdr = (struct ieee80211_hdr *)skb->data;
> +	char tid[32];
>   
>   	status = IEEE80211_SKB_RXCB(skb);
>   	*status = *rx_status;
>   
>   	ath10k_dbg(ATH10K_DBG_DATA,
> -		   "rx skb %p len %u %s%s%s%s%s %srate_idx %u vht_nss %u freq %u band %u flag 0x%x fcs-err %imic-err %i\n",
> +		   "rx skb %p len %u peer %pM %s %s sn %u %s%s%s%s%s %srate_idx %u vht_nss %u freq %u band %u flag 0x%x fcs-err %i mic-err %i amsdu-more %i\n",
>   		   skb,
>   		   skb->len,
> +		   ieee80211_get_SA(hdr),
> +		   ath10k_get_tid(hdr, tid, sizeof(tid)),
> +		   is_multicast_ether_addr(ieee80211_get_DA(hdr)) ?
> +							"mcast" : "ucast",

should match open paranthesis...

is_multicast_ether_addr(ieee80211_get_DA(hdr)) ?
			"mcast" : "ucast",

> +		   (__le16_to_cpu(hdr->seq_ctrl) & IEEE80211_SCTL_SEQ) >> 4,
>   		   status->flag == 0 ? "legacy" : "",
>   		   status->flag & RX_FLAG_HT ? "ht" : "",
>   		   status->flag & RX_FLAG_VHT ? "vht" : "",
> @@ -842,7 +878,8 @@ static void ath10k_process_rx(struct ath10k *ar,
>   		   status->freq,
>   		   status->band, status->flag,
>   		   !!(status->flag & RX_FLAG_FAILED_FCS_CRC),
> -		   !!(status->flag & RX_FLAG_MMIC_ERROR));
> +		   !!(status->flag & RX_FLAG_MMIC_ERROR),
> +		   !!(status->flag & RX_FLAG_AMSDU_MORE));
>   	ath10k_dbg_dump(ATH10K_DBG_HTT_DUMP, NULL, "rx skb: ",
>   			skb->data, skb->len);
>   


-- 
Regards,
Varka Bhadram.


^ permalink raw reply	[flat|nested] 5+ messages in thread

* Re: [PATCH v2] ath10k: extend debug code for RX path
  2014-07-28  6:35 ` Varka Bhadram
@ 2014-07-28  7:52   ` Janusz Dziedzic
  2014-07-28  8:07     ` Kalle Valo
  0 siblings, 1 reply; 5+ messages in thread
From: Janusz Dziedzic @ 2014-07-28  7:52 UTC (permalink / raw)
  To: Varka Bhadram; +Cc: ath10k@lists.infradead.org, linux-wireless

On 28 July 2014 08:35, Varka Bhadram <varkabhadram@gmail.com> wrote:
> On 07/28/2014 11:56 AM, Janusz Dziedzic wrote:
>>
>> Print sequence number, AMSDU_MORE flag and AC when additional
>> debug enabled in RX path. This is usefull for debugging purpose.
>>
>> Signed-off-by: Janusz Dziedzic<janusz.dziedzic@tieto.com>
>> ---
>>   drivers/net/wireless/ath/ath10k/htt_rx.c |   41
>> ++++++++++++++++++++++++++++--
>>   1 file changed, 39 insertions(+), 2 deletions(-)
>>
>> diff --git a/drivers/net/wireless/ath/ath10k/htt_rx.c
>> b/drivers/net/wireless/ath/ath10k/htt_rx.c
>> index d80fcda..f7c8021 100644
>> --- a/drivers/net/wireless/ath/ath10k/htt_rx.c
>> +++ b/drivers/net/wireless/ath/ath10k/htt_rx.c
>> @@ -818,19 +818,55 @@ static bool ath10k_htt_rx_h_channel(struct ath10k
>> *ar,
>>         return true;
>>   }
>>   +static const char * const tid_to_ac[] = {
>
>
> No space required '*' and const.
>
Seems checkpatch didn't find this.

> static const char *const tid_to_ac[] = {
>
>
>
>> +       "BE",
>> +       "BK",
>> +       "BK",
>> +       "BE",
>> +       "VI",
>> +       "VI",
>> +       "VO",
>> +       "VO",
>> +};
>> +
>> +static char *ath10k_get_tid(struct ieee80211_hdr *hdr, char *out, size_t
>> size)
>> +{
>> +       u8 *qc;
>> +       int tid;
>> +
>> +       if (!ieee80211_is_data_qos(hdr->frame_control))
>> +               return "";
>
>
> return NULL would be more readable...?
>
We don't need extra "(null)" in printf, empty string is what I expect
here (same as other params in this dbg info).
Anyway I can return "out" here and initialize as empty before calling
ath10k_get_tid().

>
>> +
>> +       qc = ieee80211_get_qos_ctl(hdr);
>> +       tid = *qc & IEEE80211_QOS_CTL_TID_MASK;
>> +       if (tid < 8)
>> +               snprintf(out, size, "tid %d (%s)", tid, tid_to_ac[tid]);
>> +       else
>> +               snprintf(out, size, "tid %d", tid);
>> +
>> +       return out;
>> +}
>> +
>>   static void ath10k_process_rx(struct ath10k *ar,
>>                               struct ieee80211_rx_status *rx_status,
>>                               struct sk_buff *skb)
>>   {
>>         struct ieee80211_rx_status *status;
>> +       struct ieee80211_hdr *hdr = (struct ieee80211_hdr *)skb->data;
>> +       char tid[32];
>>         status = IEEE80211_SKB_RXCB(skb);
>>         *status = *rx_status;
>>         ath10k_dbg(ATH10K_DBG_DATA,
>> -                  "rx skb %p len %u %s%s%s%s%s %srate_idx %u vht_nss %u
>> freq %u band %u flag 0x%x fcs-err %imic-err %i\n",
>> +                  "rx skb %p len %u peer %pM %s %s sn %u %s%s%s%s%s
>> %srate_idx %u vht_nss %u freq %u band %u flag 0x%x fcs-err %i mic-err %i
>> amsdu-more %i\n",
>>                    skb,
>>                    skb->len,
>> +                  ieee80211_get_SA(hdr),
>> +                  ath10k_get_tid(hdr, tid, sizeof(tid)),
>> +                  is_multicast_ether_addr(ieee80211_get_DA(hdr)) ?
>> +                                                       "mcast" : "ucast",
>
>
> should match open paranthesis...
>
> is_multicast_ether_addr(ieee80211_get_DA(hdr)) ?
>
>                         "mcast" : "ucast",
>
Could you describe this more (I didn't find clear description about this)?

BR
Janusz

^ permalink raw reply	[flat|nested] 5+ messages in thread

* Re: [PATCH v2] ath10k: extend debug code for RX path
  2014-07-28  7:52   ` Janusz Dziedzic
@ 2014-07-28  8:07     ` Kalle Valo
  0 siblings, 0 replies; 5+ messages in thread
From: Kalle Valo @ 2014-07-28  8:07 UTC (permalink / raw)
  To: Janusz Dziedzic; +Cc: Varka Bhadram, linux-wireless, ath10k@lists.infradead.org

Janusz Dziedzic <janusz.dziedzic@tieto.com> writes:

> On 28 July 2014 08:35, Varka Bhadram <varkabhadram@gmail.com> wrote:
>> On 07/28/2014 11:56 AM, Janusz Dziedzic wrote:
>>>
>>> Print sequence number, AMSDU_MORE flag and AC when additional
>>> debug enabled in RX path. This is usefull for debugging purpose.
>>>
>>> Signed-off-by: Janusz Dziedzic<janusz.dziedzic@tieto.com>
>>> ---
>>>   drivers/net/wireless/ath/ath10k/htt_rx.c |   41
>>> ++++++++++++++++++++++++++++--
>>>   1 file changed, 39 insertions(+), 2 deletions(-)
>>>
>>> diff --git a/drivers/net/wireless/ath/ath10k/htt_rx.c
>>> b/drivers/net/wireless/ath/ath10k/htt_rx.c
>>> index d80fcda..f7c8021 100644
>>> --- a/drivers/net/wireless/ath/ath10k/htt_rx.c
>>> +++ b/drivers/net/wireless/ath/ath10k/htt_rx.c
>>> @@ -818,19 +818,55 @@ static bool ath10k_htt_rx_h_channel(struct ath10k
>>> *ar,
>>>         return true;
>>>   }
>>>   +static const char * const tid_to_ac[] = {
>>
>>
>> No space required '*' and const.
>
> Seems checkpatch didn't find this.

>From a quick grep I saw that kernel has both "* const" and "*const". I
would say keep it as it is.

>>> +static char *ath10k_get_tid(struct ieee80211_hdr *hdr, char *out, size_t
>>> size)
>>> +{
>>> +       u8 *qc;
>>> +       int tid;
>>> +
>>> +       if (!ieee80211_is_data_qos(hdr->frame_control))
>>> +               return "";
>>
>>
>> return NULL would be more readable...?
>>
> We don't need extra "(null)" in printf, empty string is what I expect
> here (same as other params in this dbg info).
> Anyway I can return "out" here and initialize as empty before calling
> ath10k_get_tid().

IMHO return "" is more readable here.

>>> +                  is_multicast_ether_addr(ieee80211_get_DA(hdr)) ?
>>> +                                                       "mcast" : "ucast",
>>
>>
>> should match open paranthesis...
>>
>> is_multicast_ether_addr(ieee80211_get_DA(hdr)) ?
>>
>>                         "mcast" : "ucast",
>>
> Could you describe this more (I didn't find clear description about this)?

AFAICS there is no open parenthesis here to which align the indentation.

-- 
Kalle Valo

^ permalink raw reply	[flat|nested] 5+ messages in thread

* Re: [PATCH v2] ath10k: extend debug code for RX path
  2014-07-28  6:26 [PATCH v2] ath10k: extend debug code for RX path Janusz Dziedzic
  2014-07-28  6:35 ` Varka Bhadram
@ 2014-07-29  9:48 ` Kalle Valo
  1 sibling, 0 replies; 5+ messages in thread
From: Kalle Valo @ 2014-07-29  9:48 UTC (permalink / raw)
  To: Janusz Dziedzic; +Cc: ath10k, linux-wireless

Janusz Dziedzic <janusz.dziedzic@tieto.com> writes:

> Print sequence number, AMSDU_MORE flag and AC when additional
> debug enabled in RX path. This is usefull for debugging purpose.
>
> Signed-off-by: Janusz Dziedzic <janusz.dziedzic@tieto.com>

Thanks, applied.

-- 
Kalle Valo

^ permalink raw reply	[flat|nested] 5+ messages in thread

end of thread, other threads:[~2014-07-29  9:48 UTC | newest]

Thread overview: 5+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2014-07-28  6:26 [PATCH v2] ath10k: extend debug code for RX path Janusz Dziedzic
2014-07-28  6:35 ` Varka Bhadram
2014-07-28  7:52   ` Janusz Dziedzic
2014-07-28  8:07     ` Kalle Valo
2014-07-29  9:48 ` Kalle Valo

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).