[PATCH] ath10k: remove not needed warning when peer unmap

[PATCH] ath10k: remove not needed warning when peer unmap

[Janusz Dziedzic]

Remove not needed warning when get peer unmap
event from the firmware. This is not critical
message. Instead print this as a debug message.

Signed-off-by: Janusz Dziedzic <janusz.dziedzic@tieto.com>
---
 drivers/net/wireless/ath/ath10k/txrx.c |    2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/drivers/net/wireless/ath/ath10k/txrx.c b/drivers/net/wireless/ath/ath10k/txrx.c
index 82669a7..b7f4acc 100644
--- a/drivers/net/wireless/ath/ath10k/txrx.c
+++ b/drivers/net/wireless/ath/ath10k/txrx.c
@@ -200,7 +200,7 @@ void ath10k_peer_unmap_event(struct ath10k_htt *htt,
 	spin_lock_bh(&ar->data_lock);
 	peer = ath10k_peer_find_by_id(ar, ev->peer_id);
 	if (!peer) {
-		ath10k_warn("peer-unmap-event: unknown peer id %d\n",
+		ath10k_dbg(ATH10K_DBG_HTT, "peer-unmap-event: unknown peer id %d\n",
 			    ev->peer_id);
 		goto exit;
 	}
-- 
1.7.9.5

Re: [PATCH] ath10k: remove not needed warning when peer unmap

[Kalle Valo]

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

> Remove not needed warning when get peer unmap
> event from the firmware. This is not critical
> message. Instead print this as a debug message.

I don't agree with that statement. If that would be true, we could
remove a lot of warnings from ath10k. We have these warnings to catch
problems early, which again improves the quality of the driver.

Your commit log was again missing the "why?" part. I assume the reason
for this patch is the problem of seeing the warning "unknown peer id 2"
when putting the interface is down, which again is a spurious event from
the firmware? You should document that in the commit log as well as add
a short comment to the code explaining why we only print a debug message
when that happens.

Other idea I had would be to keep the warning message but add a new test
to detect this problematic case, but I guess for that we would need to
add a new state "stopping" to catch that? For example, something like
this:

if state == stopping and event->id == 2
        dbg("foo")
else
        warn("bar")

-- 
Kalle Valo

Re: [PATCH] ath10k: remove not needed warning when peer unmap

[Kalle Valo]

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

> Remove not needed warning when get peer unmap
> event from the firmware. This is not critical
> message. Instead print this as a debug message.
>
> Signed-off-by: Janusz Dziedzic <janusz.dziedzic@tieto.com>

This one has a new checkpatch warning:

drivers/net/wireless/ath/ath10k/txrx.c:204: CHECK: Alignment should match open parenthesis

-- 
Kalle Valo

Re: [PATCH] ath10k: remove not needed warning when peer unmap

[Ben Greear]

On 04/02/2014 12:31 AM, Kalle Valo wrote:
> Janusz Dziedzic <janusz.dziedzic@tieto.com> writes:
>
>> Remove not needed warning when get peer unmap
>> event from the firmware. This is not critical
>> message. Instead print this as a debug message.
>
> I don't agree with that statement. If that would be true, we could
> remove a lot of warnings from ath10k. We have these warnings to catch
> problems early, which again improves the quality of the driver.

I see this message all the time, by the way...  I have been ignoring
it so far, but if it's  real issue then I can pay more attention.

Thanks,
Ben

>
> Your commit log was again missing the "why?" part. I assume the reason
> for this patch is the problem of seeing the warning "unknown peer id 2"
> when putting the interface is down, which again is a spurious event from
> the firmware? You should document that in the commit log as well as add
> a short comment to the code explaining why we only print a debug message
> when that happens.
>
> Other idea I had would be to keep the warning message but add a new test
> to detect this problematic case, but I guess for that we would need to
> add a new state "stopping" to catch that? For example, something like
> this:
>
> if state == stopping and event->id == 2
>          dbg("foo")
> else
>          warn("bar")
>


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