[PATCH 0/3] ath10k: debug improvements

[PATCH 0/3] ath10k: debug improvements

[Michal Kazior]

Hi,

This adds extra prints and information for
debugging purposes.


Michal Kazior (3):
  ath10k: print wmi version info
  ath10k: dump hex bytes with dev string prefix
  ath10k: add debug dump for pci rx

 drivers/net/wireless/ath/ath10k/debug.c | 29 ++++++++++++++++++++++++++---
 drivers/net/wireless/ath/ath10k/pci.c   |  6 ++++++
 2 files changed, 32 insertions(+), 3 deletions(-)

-- 
1.8.5.3

[PATCH 1/3] ath10k: print wmi version info

[Michal Kazior]

HTT version is already printed so print WMI
version as well for consistency.

Signed-off-by: Michal Kazior <michal.kazior@tieto.com>
---
 drivers/net/wireless/ath/ath10k/debug.c | 8 ++++++--
 1 file changed, 6 insertions(+), 2 deletions(-)

diff --git a/drivers/net/wireless/ath/ath10k/debug.c b/drivers/net/wireless/ath/ath10k/debug.c
index 3756feb..f1e5916 100644
--- a/drivers/net/wireless/ath/ath10k/debug.c
+++ b/drivers/net/wireless/ath/ath10k/debug.c
@@ -126,14 +126,18 @@ EXPORT_SYMBOL(ath10k_info);
 
 void ath10k_print_driver_info(struct ath10k *ar)
 {
-	ath10k_info(ar, "%s (0x%08x, 0x%08x) fw %s api %d htt %d.%d\n",
+	ath10k_info(ar, "%s (0x%08x, 0x%08x) fw %s api %d htt %d.%d wmi %d.%d.%d.%d\n",
 		    ar->hw_params.name,
 		    ar->target_version,
 		    ar->chip_id,
 		    ar->hw->wiphy->fw_version,
 		    ar->fw_api,
 		    ar->htt.target_version_major,
-		    ar->htt.target_version_minor);
+		    ar->htt.target_version_minor,
+		    ar->fw_version_major,
+		    ar->fw_version_minor,
+		    ar->fw_version_release,
+		    ar->fw_version_build);
 	ath10k_info(ar, "debug %d debugfs %d tracing %d dfs %d testmode %d\n",
 		    config_enabled(CONFIG_ATH10K_DEBUG),
 		    config_enabled(CONFIG_ATH10K_DEBUGFS),
-- 
1.8.5.3

[PATCH 2/3] ath10k: dump hex bytes with dev string prefix

[Michal Kazior]

This makes it easier to debug hex dumps on systems
with more than a single ath10k device.

Signed-off-by: Michal Kazior <michal.kazior@tieto.com>
---
 drivers/net/wireless/ath/ath10k/debug.c | 21 ++++++++++++++++++++-
 1 file changed, 20 insertions(+), 1 deletion(-)

diff --git a/drivers/net/wireless/ath/ath10k/debug.c b/drivers/net/wireless/ath/ath10k/debug.c
index f1e5916..9f8dddc 100644
--- a/drivers/net/wireless/ath/ath10k/debug.c
+++ b/drivers/net/wireless/ath/ath10k/debug.c
@@ -1264,11 +1264,30 @@ void ath10k_dbg_dump(struct ath10k *ar,
 		     const char *msg, const char *prefix,
 		     const void *buf, size_t len)
 {
+	char linebuf[256];
+	unsigned int linebuflen;
+	const void *ptr;
+
 	if (ath10k_debug_mask & mask) {
 		if (msg)
 			ath10k_dbg(ar, mask, "%s\n", msg);
 
-		print_hex_dump_bytes(prefix, DUMP_PREFIX_OFFSET, buf, len);
+		for (ptr = buf; (ptr - buf) < len; ptr += 16) {
+			linebuflen = 0;
+			if (prefix)
+				linebuflen += scnprintf(linebuf + linebuflen,
+							sizeof(linebuf) -
+							linebuflen,
+							"%s", prefix);
+			linebuflen += scnprintf(linebuf + linebuflen,
+						sizeof(linebuf) - linebuflen,
+						"%08x: ",
+						(unsigned int)(ptr - buf));
+			hex_dump_to_buffer(ptr, len - (ptr - buf), 16, 1,
+					   linebuf + linebuflen,
+					   sizeof(linebuf) - linebuflen, true);
+			dev_printk(KERN_DEBUG, ar->dev, "%s\n", linebuf);
+		}
 	}
 
 	/* tracing code doesn't like null strings :/ */
-- 
1.8.5.3

[PATCH 3/3] ath10k: add debug dump for pci rx

[Michal Kazior]

This makes it easier to debug the device-target
communication at a very low level.

Signed-off-by: Michal Kazior <michal.kazior@tieto.com>
---
 drivers/net/wireless/ath/ath10k/pci.c | 6 ++++++
 1 file changed, 6 insertions(+)

diff --git a/drivers/net/wireless/ath/ath10k/pci.c b/drivers/net/wireless/ath/ath10k/pci.c
index 59e0ea8..8319d83 100644
--- a/drivers/net/wireless/ath/ath10k/pci.c
+++ b/drivers/net/wireless/ath/ath10k/pci.c
@@ -861,6 +861,12 @@ static void ath10k_pci_ce_recv_data(struct ath10k_ce_pipe *ce_state)
 		}
 
 		skb_put(skb, nbytes);
+
+		ath10k_dbg(ar, ATH10K_DBG_PCI, "pci rx ce pipe %d len %d\n",
+			   ce_state->id, skb->len);
+		ath10k_dbg_dump(ar, ATH10K_DBG_PCI_DUMP, NULL, "pci rx: ",
+				skb->data, skb->len);
+
 		cb->rx_completion(ar, skb, pipe_info->pipe_num);
 	}
 
-- 
1.8.5.3

Re: [PATCH 1/3] ath10k: print wmi version info

[Kalle Valo]

Michal Kazior <michal.kazior@tieto.com> writes:

> HTT version is already printed so print WMI
> version as well for consistency.
>
> Signed-off-by: Michal Kazior <michal.kazior@tieto.com>
> ---
>  drivers/net/wireless/ath/ath10k/debug.c | 8 ++++++--
>  1 file changed, 6 insertions(+), 2 deletions(-)
>
> diff --git a/drivers/net/wireless/ath/ath10k/debug.c b/drivers/net/wireless/ath/ath10k/debug.c
> index 3756feb..f1e5916 100644
> --- a/drivers/net/wireless/ath/ath10k/debug.c
> +++ b/drivers/net/wireless/ath/ath10k/debug.c
> @@ -126,14 +126,18 @@ EXPORT_SYMBOL(ath10k_info);
>  
>  void ath10k_print_driver_info(struct ath10k *ar)
>  {
> -	ath10k_info(ar, "%s (0x%08x, 0x%08x) fw %s api %d htt %d.%d\n",
> +	ath10k_info(ar, "%s (0x%08x, 0x%08x) fw %s api %d htt %d.%d wmi %d.%d.%d.%d\n",

Heh, I have been thinking about to add something similar.

But if in the future we add the u32 FW IE for WMI version I would like
to print that as well. I guess we could do that with "%d/%d.%d.%d.%d",
the %d being the u32. The idea here being that both version numbers are
good to know.

But of course for now the above is good, just thinking aloud here.

-- 
Kalle Valo

Re: [PATCH 2/3] ath10k: dump hex bytes with dev string prefix

[Kalle Valo]

Michal Kazior <michal.kazior@tieto.com> writes:

> This makes it easier to debug hex dumps on systems
> with more than a single ath10k device.
>
> Signed-off-by: Michal Kazior <michal.kazior@tieto.com>

[...]

> -		print_hex_dump_bytes(prefix, DUMP_PREFIX_OFFSET, buf, len);
> +		for (ptr = buf; (ptr - buf) < len; ptr += 16) {
> +			linebuflen = 0;
> +			if (prefix)
> +				linebuflen += scnprintf(linebuf + linebuflen,
> +							sizeof(linebuf) -
> +							linebuflen,
> +							"%s", prefix);
> +			linebuflen += scnprintf(linebuf + linebuflen,
> +						sizeof(linebuf) - linebuflen,
> +						"%08x: ",
> +						(unsigned int)(ptr - buf));
> +			hex_dump_to_buffer(ptr, len - (ptr - buf), 16, 1,
> +					   linebuf + linebuflen,
> +					   sizeof(linebuf) - linebuflen, true);
> +			dev_printk(KERN_DEBUG, ar->dev, "%s\n", linebuf);
> +		}

Would it be possible to simplify this to one scnprintf()? Something
like:

linebuflen += scnprintf(linebuf + linebuflen,
			sizeof(linebuf) - linebuflen,
			"%s%08x: ",
                        prefix ? prefix : "",
			(unsigned int)(ptr - buf));

-- 
Kalle Valo

Re: [PATCH 2/3] ath10k: dump hex bytes with dev string prefix

[Michal Kazior]

On 22 September 2014 12:52, Kalle Valo <kvalo@qca.qualcomm.com> wrote:
> Michal Kazior <michal.kazior@tieto.com> writes:
>
>> This makes it easier to debug hex dumps on systems
>> with more than a single ath10k device.
>>
>> Signed-off-by: Michal Kazior <michal.kazior@tieto.com>
>
> [...]
>
>> -             print_hex_dump_bytes(prefix, DUMP_PREFIX_OFFSET, buf, len);
>> +             for (ptr = buf; (ptr - buf) < len; ptr += 16) {
>> +                     linebuflen = 0;
>> +                     if (prefix)
>> +                             linebuflen += scnprintf(linebuf + linebuflen,
>> +                                                     sizeof(linebuf) -
>> +                                                     linebuflen,
>> +                                                     "%s", prefix);
>> +                     linebuflen += scnprintf(linebuf + linebuflen,
>> +                                             sizeof(linebuf) - linebuflen,
>> +                                             "%08x: ",
>> +                                             (unsigned int)(ptr - buf));
>> +                     hex_dump_to_buffer(ptr, len - (ptr - buf), 16, 1,
>> +                                        linebuf + linebuflen,
>> +                                        sizeof(linebuf) - linebuflen, true);
>> +                     dev_printk(KERN_DEBUG, ar->dev, "%s\n", linebuf);
>> +             }
>
> Would it be possible to simplify this to one scnprintf()? Something
> like:
>
> linebuflen += scnprintf(linebuf + linebuflen,
>                         sizeof(linebuf) - linebuflen,
>                         "%s%08x: ",
>                         prefix ? prefix : "",
>                         (unsigned int)(ptr - buf));

It should be fine. You want me to re-send it?


Michał

Re: [PATCH 2/3] ath10k: dump hex bytes with dev string prefix

[Kalle Valo]

Michal Kazior <michal.kazior@tieto.com> writes:

> On 22 September 2014 12:52, Kalle Valo <kvalo@qca.qualcomm.com> wrote:
>> Michal Kazior <michal.kazior@tieto.com> writes:
>>
>>> -             print_hex_dump_bytes(prefix, DUMP_PREFIX_OFFSET, buf, len);
>>> +             for (ptr = buf; (ptr - buf) < len; ptr += 16) {
>>> +                     linebuflen = 0;
>>> +                     if (prefix)
>>> +                             linebuflen += scnprintf(linebuf + linebuflen,
>>> +                                                     sizeof(linebuf) -
>>> +                                                     linebuflen,
>>> +                                                     "%s", prefix);
>>> +                     linebuflen += scnprintf(linebuf + linebuflen,
>>> +                                             sizeof(linebuf) - linebuflen,
>>> +                                             "%08x: ",
>>> +                                             (unsigned int)(ptr - buf));
>>> +                     hex_dump_to_buffer(ptr, len - (ptr - buf), 16, 1,
>>> +                                        linebuf + linebuflen,
>>> +                                        sizeof(linebuf) - linebuflen, true);
>>> +                     dev_printk(KERN_DEBUG, ar->dev, "%s\n", linebuf);
>>> +             }
>>
>> Would it be possible to simplify this to one scnprintf()? Something
>> like:
>>
>> linebuflen += scnprintf(linebuf + linebuflen,
>>                         sizeof(linebuf) - linebuflen,
>>                         "%s%08x: ",
>>                         prefix ? prefix : "",
>>                         (unsigned int)(ptr - buf));
>
> It should be fine. You want me to re-send it?

Yes, that would be good. I don't feel comfortable editing patches with
logic changes like this.

-- 
Kalle Valo