[PATCH] ath10k: improve logging in firmware crash routine.

[PATCH] ath10k: improve logging in firmware crash routine.

[greearb]

From: Ben Greear <greearb@candelatech.com>

Only print error message upon failure, and print more
details in case it does find an error.

Signed-off-by: Ben Greear <greearb@candelatech.com>
---

This is on top of the firmware crash reporting patches,
not sure it would apply clean until those get in.

 drivers/net/wireless/ath/ath10k/pci.c | 5 ++---
 1 file changed, 2 insertions(+), 3 deletions(-)

diff --git a/drivers/net/wireless/ath/ath10k/pci.c b/drivers/net/wireless/ath/ath10k/pci.c
index 24688b7..085c0c8 100644
--- a/drivers/net/wireless/ath/ath10k/pci.c
+++ b/drivers/net/wireless/ath/ath10k/pci.c
@@ -929,12 +929,11 @@ static void ath10k_pci_hif_dump_area(struct ath10k *ar)
 	ret = ath10k_pci_diag_read_mem(ar, host_addr,
 				       &reg_dump_area, sizeof(u32));
 	if (ret) {
-		ath10k_err("failed to read FW dump area address: %d\n", ret);
+		ath10k_err("failed to read FW dump area address: %d  (hostaddr 0x%08X  hi-failure-state 0x%08lX)\n",
+			   ret, host_addr, HI_ITEM(hi_failure_state));
 		goto exit;
 	}
 
-	ath10k_err("target register Dump Location: 0x%08X\n", reg_dump_area);
-
 	ret = ath10k_pci_diag_read_mem(ar, reg_dump_area,
 				       &reg_dump_values[0],
 				       REG_DUMP_COUNT_QCA988X * sizeof(u32));
-- 
1.7.11.7

Re: [PATCH] ath10k: improve logging in firmware crash routine.

[Kalle Valo]

greearb@candelatech.com writes:

> From: Ben Greear <greearb@candelatech.com>
>
> Only print error message upon failure, and print more
> details in case it does find an error.
>
> Signed-off-by: Ben Greear <greearb@candelatech.com>
> ---
>
> This is on top of the firmware crash reporting patches,
> not sure it would apply clean until those get in.

Yeah, this does conflict with the firmware crash dump patches.

>  drivers/net/wireless/ath/ath10k/pci.c | 5 ++---
>  1 file changed, 2 insertions(+), 3 deletions(-)
>
> diff --git a/drivers/net/wireless/ath/ath10k/pci.c b/drivers/net/wireless/ath/ath10k/pci.c
> index 24688b7..085c0c8 100644
> --- a/drivers/net/wireless/ath/ath10k/pci.c
> +++ b/drivers/net/wireless/ath/ath10k/pci.c
> @@ -929,12 +929,11 @@ static void ath10k_pci_hif_dump_area(struct ath10k *ar)
>  	ret = ath10k_pci_diag_read_mem(ar, host_addr,
>  				       &reg_dump_area, sizeof(u32));
>  	if (ret) {
> -		ath10k_err("failed to read FW dump area address: %d\n", ret);
> +		ath10k_err("failed to read FW dump area address: %d  (hostaddr 0x%08X  hi-failure-state 0x%08lX)\n",
> +			   ret, host_addr, HI_ITEM(hi_failure_state));
>  		goto exit;
>  	}

As I reworked how the diag interface is used, this doesn't directly
apply anymore. And are these values really that important?

> -	ath10k_err("target register Dump Location: 0x%08X\n", reg_dump_area);

I actually removed this line in the firmware crash dump patchset.

-- 
Kalle Valo

Re: [PATCH] ath10k: improve logging in firmware crash routine.

[Ben Greear]

On 08/19/2014 05:11 AM, Kalle Valo wrote:
> greearb@candelatech.com writes:
>
>> From: Ben Greear <greearb@candelatech.com>
>>
>> Only print error message upon failure, and print more
>> details in case it does find an error.
>>
>> Signed-off-by: Ben Greear <greearb@candelatech.com>
>> ---
>>
>> This is on top of the firmware crash reporting patches,
>> not sure it would apply clean until those get in.
>
> Yeah, this does conflict with the firmware crash dump patches.
>
>>   drivers/net/wireless/ath/ath10k/pci.c | 5 ++---
>>   1 file changed, 2 insertions(+), 3 deletions(-)
>>
>> diff --git a/drivers/net/wireless/ath/ath10k/pci.c b/drivers/net/wireless/ath/ath10k/pci.c
>> index 24688b7..085c0c8 100644
>> --- a/drivers/net/wireless/ath/ath10k/pci.c
>> +++ b/drivers/net/wireless/ath/ath10k/pci.c
>> @@ -929,12 +929,11 @@ static void ath10k_pci_hif_dump_area(struct ath10k *ar)
>>   	ret = ath10k_pci_diag_read_mem(ar, host_addr,
>>   				       &reg_dump_area, sizeof(u32));
>>   	if (ret) {
>> -		ath10k_err("failed to read FW dump area address: %d\n", ret);
>> +		ath10k_err("failed to read FW dump area address: %d  (hostaddr 0x%08X  hi-failure-state 0x%08lX)\n",
>> +			   ret, host_addr, HI_ITEM(hi_failure_state));
>>   		goto exit;
>>   	}
>
> As I reworked how the diag interface is used, this doesn't directly
> apply anymore. And are these values really that important?
>
>> -	ath10k_err("target register Dump Location: 0x%08X\n", reg_dump_area);
>
> I actually removed this line in the firmware crash dump patchset.

Probably with your re-work, this patch is no longer useful.  At the time I wrote it,
it allowed a bit more information in error cases and less noise when everything was
OK.

Thanks,
Ben


-- 
Ben Greear <greearb@candelatech.com>
Candela Technologies Inc  http://www.candelatech.com