| Message ID | 1396340628-4570-1-git-send-email-janusz.dziedzic@tieto.com (mailing list archive) |
|---|---|
| State | Not Applicable, archived |
| Headers | show |
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")
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
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") >
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; }
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(-)