All of lore.kernel.org
 help / color / mirror / Atom feed
* [PATCH] ath10k: extend debug code for RX path
@ 2014-07-18  8:34 ` Janusz Dziedzic
  0 siblings, 0 replies; 6+ messages in thread
From: Janusz Dziedzic @ 2014-07-18  8:34 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 |   32 ++++++++++++++++++++++++++++--
 1 file changed, 30 insertions(+), 2 deletions(-)

diff --git a/drivers/net/wireless/ath/ath10k/htt_rx.c b/drivers/net/wireless/ath/ath10k/htt_rx.c
index 6db0e49..04140e3 100644
--- a/drivers/net/wireless/ath/ath10k/htt_rx.c
+++ b/drivers/net/wireless/ath/ath10k/htt_rx.c
@@ -819,19 +819,46 @@ static bool ath10k_htt_rx_h_channel(struct ath10k *ar,
 	return true;
 }
 
+static char *tid_to_ac[] = {
+	"BE(0)",
+	"BK(1)",
+	"BK(2)",
+	"BE(3)",
+	"VI(4)",
+	"VI(5)",
+	"VO(6)",
+	"VO(7)",
+	"tid(8)",
+	"tid(9)",
+	"tid(10)",
+	"tid(11)",
+	"tid(12)",
+	"tid(13)",
+	"tid(14)",
+	"tid(15)",
+};
+
 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;
 
 	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 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),
+		   ieee80211_is_data_qos(hdr->frame_control) ?
+			(tid_to_ac[*(ieee80211_get_qos_ctl(hdr)) &
+				   IEEE80211_QOS_CTL_TID_MASK]) :
+			(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" : "",
@@ -843,7 +870,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] 6+ messages in thread
* [PATCH] ath10k: extend debug code for RX path
@ 2014-07-18  8:34 ` Janusz Dziedzic
  0 siblings, 0 replies; 6+ messages in thread
From: Janusz Dziedzic @ 2014-07-18  8:34 UTC (permalink / raw)
  To: ath10k; +Cc: Janusz Dziedzic, linux-wireless

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 |   32 ++++++++++++++++++++++++++++--
 1 file changed, 30 insertions(+), 2 deletions(-)

diff --git a/drivers/net/wireless/ath/ath10k/htt_rx.c b/drivers/net/wireless/ath/ath10k/htt_rx.c
index 6db0e49..04140e3 100644
--- a/drivers/net/wireless/ath/ath10k/htt_rx.c
+++ b/drivers/net/wireless/ath/ath10k/htt_rx.c
@@ -819,19 +819,46 @@ static bool ath10k_htt_rx_h_channel(struct ath10k *ar,
 	return true;
 }
 
+static char *tid_to_ac[] = {
+	"BE(0)",
+	"BK(1)",
+	"BK(2)",
+	"BE(3)",
+	"VI(4)",
+	"VI(5)",
+	"VO(6)",
+	"VO(7)",
+	"tid(8)",
+	"tid(9)",
+	"tid(10)",
+	"tid(11)",
+	"tid(12)",
+	"tid(13)",
+	"tid(14)",
+	"tid(15)",
+};
+
 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;
 
 	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 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),
+		   ieee80211_is_data_qos(hdr->frame_control) ?
+			(tid_to_ac[*(ieee80211_get_qos_ctl(hdr)) &
+				   IEEE80211_QOS_CTL_TID_MASK]) :
+			(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" : "",
@@ -843,7 +870,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


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

^ permalink raw reply related	[flat|nested] 6+ messages in thread
* Re: [PATCH] ath10k: extend debug code for RX path
  2014-07-18  8:34 ` Janusz Dziedzic
@ 2014-07-22 16:02   ` Kalle Valo
  -1 siblings, 0 replies; 6+ messages in thread
From: Kalle Valo @ 2014-07-22 16:02 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>
> ---
>  drivers/net/wireless/ath/ath10k/htt_rx.c |   32 ++++++++++++++++++++++++++++--
>  1 file changed, 30 insertions(+), 2 deletions(-)
>
> diff --git a/drivers/net/wireless/ath/ath10k/htt_rx.c b/drivers/net/wireless/ath/ath10k/htt_rx.c
> index 6db0e49..04140e3 100644
> --- a/drivers/net/wireless/ath/ath10k/htt_rx.c
> +++ b/drivers/net/wireless/ath/ath10k/htt_rx.c
> @@ -819,19 +819,46 @@ static bool ath10k_htt_rx_h_channel(struct ath10k *ar,
>  	return true;
>  }
>  
> +static char *tid_to_ac[] = {

static const?

> +	"BE(0)",
> +	"BK(1)",
> +	"BK(2)",
> +	"BE(3)",
> +	"VI(4)",
> +	"VI(5)",
> +	"VO(6)",
> +	"VO(7)",
> +	"tid(8)",
> +	"tid(9)",
> +	"tid(10)",
> +	"tid(11)",
> +	"tid(12)",
> +	"tid(13)",
> +	"tid(14)",
> +	"tid(15)",
> +};

I don't see much point of adding tid number to the string when you could
print it with "%i". Also about the string format, maybe this would be
clearer:

"rx skb %p len %u peer %pM tid %i (%s) sn %u"

For example "...tid 4 (VI)..." and for tid > 7 we don't print "(VI)"
part at all, just the number.

>  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;
>  
>  	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 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),

This looks ok.

> +		   ieee80211_is_data_qos(hdr->frame_control) ?
> +			(tid_to_ac[*(ieee80211_get_qos_ctl(hdr)) &
> +				   IEEE80211_QOS_CTL_TID_MASK]) :
> +			(is_multicast_ether_addr(ieee80211_get_DA(hdr)) ?
> +				"mcast" : "ucast"),
> +		   (__le16_to_cpu(hdr->seq_ctrl) & IEEE80211_SCTL_SEQ) >> 4,

But this is hard to read. Is it possible to have a helper function for
this?

-- 
Kalle Valo

^ permalink raw reply	[flat|nested] 6+ messages in thread
* Re: [PATCH] ath10k: extend debug code for RX path
@ 2014-07-22 16:02   ` Kalle Valo
  0 siblings, 0 replies; 6+ messages in thread
From: Kalle Valo @ 2014-07-22 16:02 UTC (permalink / raw)
  To: Janusz Dziedzic; +Cc: linux-wireless, ath10k

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>
> ---
>  drivers/net/wireless/ath/ath10k/htt_rx.c |   32 ++++++++++++++++++++++++++++--
>  1 file changed, 30 insertions(+), 2 deletions(-)
>
> diff --git a/drivers/net/wireless/ath/ath10k/htt_rx.c b/drivers/net/wireless/ath/ath10k/htt_rx.c
> index 6db0e49..04140e3 100644
> --- a/drivers/net/wireless/ath/ath10k/htt_rx.c
> +++ b/drivers/net/wireless/ath/ath10k/htt_rx.c
> @@ -819,19 +819,46 @@ static bool ath10k_htt_rx_h_channel(struct ath10k *ar,
>  	return true;
>  }
>  
> +static char *tid_to_ac[] = {

static const?

> +	"BE(0)",
> +	"BK(1)",
> +	"BK(2)",
> +	"BE(3)",
> +	"VI(4)",
> +	"VI(5)",
> +	"VO(6)",
> +	"VO(7)",
> +	"tid(8)",
> +	"tid(9)",
> +	"tid(10)",
> +	"tid(11)",
> +	"tid(12)",
> +	"tid(13)",
> +	"tid(14)",
> +	"tid(15)",
> +};

I don't see much point of adding tid number to the string when you could
print it with "%i". Also about the string format, maybe this would be
clearer:

"rx skb %p len %u peer %pM tid %i (%s) sn %u"

For example "...tid 4 (VI)..." and for tid > 7 we don't print "(VI)"
part at all, just the number.

>  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;
>  
>  	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 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),

This looks ok.

> +		   ieee80211_is_data_qos(hdr->frame_control) ?
> +			(tid_to_ac[*(ieee80211_get_qos_ctl(hdr)) &
> +				   IEEE80211_QOS_CTL_TID_MASK]) :
> +			(is_multicast_ether_addr(ieee80211_get_DA(hdr)) ?
> +				"mcast" : "ucast"),
> +		   (__le16_to_cpu(hdr->seq_ctrl) & IEEE80211_SCTL_SEQ) >> 4,

But this is hard to read. Is it possible to have a helper function for
this?

-- 
Kalle Valo

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

^ permalink raw reply	[flat|nested] 6+ messages in thread
* Re: [PATCH] ath10k: extend debug code for RX path
  2014-07-22 16:02   ` Kalle Valo
@ 2014-07-24  7:17     ` Janusz Dziedzic
  -1 siblings, 0 replies; 6+ messages in thread
From: Janusz Dziedzic @ 2014-07-24  7:17 UTC (permalink / raw)
  To: Kalle Valo; +Cc: ath10k, linux-wireless

On 22 July 2014 18:02, Kalle Valo <kvalo@qca.qualcomm.com> wrote:
> 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>
>> ---
>>  drivers/net/wireless/ath/ath10k/htt_rx.c |   32 ++++++++++++++++++++++++++++--
>>  1 file changed, 30 insertions(+), 2 deletions(-)
>>
>> diff --git a/drivers/net/wireless/ath/ath10k/htt_rx.c b/drivers/net/wireless/ath/ath10k/htt_rx.c
>> index 6db0e49..04140e3 100644
>> --- a/drivers/net/wireless/ath/ath10k/htt_rx.c
>> +++ b/drivers/net/wireless/ath/ath10k/htt_rx.c
>> @@ -819,19 +819,46 @@ static bool ath10k_htt_rx_h_channel(struct ath10k *ar,
>>       return true;
>>  }
>>
>> +static char *tid_to_ac[] = {
>
> static const?
>
>> +     "BE(0)",
>> +     "BK(1)",
>> +     "BK(2)",
>> +     "BE(3)",
>> +     "VI(4)",
>> +     "VI(5)",
>> +     "VO(6)",
>> +     "VO(7)",
>> +     "tid(8)",
>> +     "tid(9)",
>> +     "tid(10)",
>> +     "tid(11)",
>> +     "tid(12)",
>> +     "tid(13)",
>> +     "tid(14)",
>> +     "tid(15)",
>> +};
>
> I don't see much point of adding tid number to the string when you could
> print it with "%i". Also about the string format, maybe this would be
> clearer:
>
> "rx skb %p len %u peer %pM tid %i (%s) sn %u"
>
> For example "...tid 4 (VI)..." and for tid > 7 we don't print "(VI)"
> part at all, just the number.
>
tid don't need to be always present (eg sta with disabled qos).
So I can use:
- table as above
- buffer[] + snprintf() + %i
Depends what you prefer here.

>>  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;
>>
>>       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 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),
>
> This looks ok.
>
>> +                ieee80211_is_data_qos(hdr->frame_control) ?
>> +                     (tid_to_ac[*(ieee80211_get_qos_ctl(hdr)) &
>> +                                IEEE80211_QOS_CTL_TID_MASK]) :
>> +                     (is_multicast_ether_addr(ieee80211_get_DA(hdr)) ?
>> +                             "mcast" : "ucast"),
>> +                (__le16_to_cpu(hdr->seq_ctrl) & IEEE80211_SCTL_SEQ) >> 4,
>
> But this is hard to read. Is it possible to have a helper function for
> this?
Sure I can put this in the function.

BR
Janusz

^ permalink raw reply	[flat|nested] 6+ messages in thread
* Re: [PATCH] ath10k: extend debug code for RX path
@ 2014-07-24  7:17     ` Janusz Dziedzic
  0 siblings, 0 replies; 6+ messages in thread
From: Janusz Dziedzic @ 2014-07-24  7:17 UTC (permalink / raw)
  To: Kalle Valo; +Cc: linux-wireless, ath10k

On 22 July 2014 18:02, Kalle Valo <kvalo@qca.qualcomm.com> wrote:
> 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>
>> ---
>>  drivers/net/wireless/ath/ath10k/htt_rx.c |   32 ++++++++++++++++++++++++++++--
>>  1 file changed, 30 insertions(+), 2 deletions(-)
>>
>> diff --git a/drivers/net/wireless/ath/ath10k/htt_rx.c b/drivers/net/wireless/ath/ath10k/htt_rx.c
>> index 6db0e49..04140e3 100644
>> --- a/drivers/net/wireless/ath/ath10k/htt_rx.c
>> +++ b/drivers/net/wireless/ath/ath10k/htt_rx.c
>> @@ -819,19 +819,46 @@ static bool ath10k_htt_rx_h_channel(struct ath10k *ar,
>>       return true;
>>  }
>>
>> +static char *tid_to_ac[] = {
>
> static const?
>
>> +     "BE(0)",
>> +     "BK(1)",
>> +     "BK(2)",
>> +     "BE(3)",
>> +     "VI(4)",
>> +     "VI(5)",
>> +     "VO(6)",
>> +     "VO(7)",
>> +     "tid(8)",
>> +     "tid(9)",
>> +     "tid(10)",
>> +     "tid(11)",
>> +     "tid(12)",
>> +     "tid(13)",
>> +     "tid(14)",
>> +     "tid(15)",
>> +};
>
> I don't see much point of adding tid number to the string when you could
> print it with "%i". Also about the string format, maybe this would be
> clearer:
>
> "rx skb %p len %u peer %pM tid %i (%s) sn %u"
>
> For example "...tid 4 (VI)..." and for tid > 7 we don't print "(VI)"
> part at all, just the number.
>
tid don't need to be always present (eg sta with disabled qos).
So I can use:
- table as above
- buffer[] + snprintf() + %i
Depends what you prefer here.

>>  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;
>>
>>       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 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),
>
> This looks ok.
>
>> +                ieee80211_is_data_qos(hdr->frame_control) ?
>> +                     (tid_to_ac[*(ieee80211_get_qos_ctl(hdr)) &
>> +                                IEEE80211_QOS_CTL_TID_MASK]) :
>> +                     (is_multicast_ether_addr(ieee80211_get_DA(hdr)) ?
>> +                             "mcast" : "ucast"),
>> +                (__le16_to_cpu(hdr->seq_ctrl) & IEEE80211_SCTL_SEQ) >> 4,
>
> But this is hard to read. Is it possible to have a helper function for
> this?
Sure I can put this in the function.

BR
Janusz

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

^ permalink raw reply	[flat|nested] 6+ messages in thread
end of thread, other threads:[~2014-07-24  7:18 UTC | newest]

Thread overview: 6+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2014-07-18  8:34 [PATCH] ath10k: extend debug code for RX path Janusz Dziedzic
2014-07-18  8:34 ` Janusz Dziedzic
2014-07-22 16:02 ` Kalle Valo
2014-07-22 16:02   ` Kalle Valo
2014-07-24  7:17   ` Janusz Dziedzic
2014-07-24  7:17     ` Janusz Dziedzic

This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.