[PATCH 3/6] ath5k: continue reset sequence if gain calibration fails

[PATCH 3/6] ath5k: continue reset sequence if gain calibration fails

[Bob Copeland]

If gain calibration fails, then ath5k_hw_reset will skip writing some
important registers like the interrupt mask.  In legacy_hal, only an
error is emitted in this case but the reset proceeds, so follow suit
here.

Changes to reset.c
Changes-licensed-under: ISC

Changes to base.c
Changes-licensed-under: 3-Clause-BSD

Signed-off-by: Bob Copeland <me@bobcopeland.com>
---
 drivers/net/wireless/ath5k/reset.c |    1 -
 1 files changed, 0 insertions(+), 1 deletions(-)

diff --git a/drivers/net/wireless/ath5k/reset.c b/drivers/net/wireless/ath5k/reset.c
index c7cd380..3c8d3d6 100644
--- a/drivers/net/wireless/ath5k/reset.c
+++ b/drivers/net/wireless/ath5k/reset.c
@@ -829,7 +829,6 @@ int ath5k_hw_reset(struct ath5k_hw *ah, enum nl80211_iftype op_mode,
 			AR5K_PHY_AGCCTL_CAL, 0, false)) {
 		ATH5K_ERR(ah->ah_sc, "gain calibration timeout (%uMHz)\n",
 			channel->center_freq);
-		return -EAGAIN;
 	}
 
 	/*
-- 
1.6.0.6

Re: [PATCH 3/6] ath5k: continue reset sequence if gain calibration fails

[Nick Kossifidis]

2009/1/22 Bob Copeland <me@bobcopeland.com>:
> If gain calibration fails, then ath5k_hw_reset will skip writing some
> important registers like the interrupt mask.  In legacy_hal, only an
> error is emitted in this case but the reset proceeds, so follow suit
> here.
>
> Changes to reset.c
> Changes-licensed-under: ISC
>
> Changes to base.c
> Changes-licensed-under: 3-Clause-BSD
>
> Signed-off-by: Bob Copeland <me@bobcopeland.com>
> ---
>  drivers/net/wireless/ath5k/reset.c |    1 -
>  1 files changed, 0 insertions(+), 1 deletions(-)
>
> diff --git a/drivers/net/wireless/ath5k/reset.c b/drivers/net/wireless/ath5k/reset.c
> index c7cd380..3c8d3d6 100644
> --- a/drivers/net/wireless/ath5k/reset.c
> +++ b/drivers/net/wireless/ath5k/reset.c
> @@ -829,7 +829,6 @@ int ath5k_hw_reset(struct ath5k_hw *ah, enum nl80211_iftype op_mode,
>                        AR5K_PHY_AGCCTL_CAL, 0, false)) {
>                ATH5K_ERR(ah->ah_sc, "gain calibration timeout (%uMHz)\n",
>                        channel->center_freq);
> -               return -EAGAIN;
>        }
>
>        /*

Again i've already fixed that on my local branch (i said i'll update
reset code and i did, i just have to make some tests first), it's what
both HALs do. Notice that if offset calibration has not completed, i/q
calibration can't work (according to patent doc) and  noise floor
calibration always fails on some chips. IMHO we must check if the bit
got cleared while calling phy_calibrate because if it's not cleared
there is no meaning to perform i/q and nf calibration. That's why i
haven't removed that return yet. I always believe that the best thing
to do is to find a sane timeout interval that works on most cases
instead of just ignoring the fact that calibration timed out of
failed.

-- 
GPG ID: 0xD21DB2DB
As you read this post global entropy rises. Have Fun ;-)
Nick

Re: [PATCH 3/6] ath5k: continue reset sequence if gain calibration fails

[Bob Copeland]

On Thu, 22 Jan 2009 19:36:45 +0200, Nick Kossifidis wrote
> Again i've already fixed that on my local branch (i said i'll update
> reset code and i did, i just have to make some tests first), it's what
> both HALs do. Notice that if offset calibration has not completed, i/q
> calibration can't work (according to patent doc) and  noise floor
> calibration always fails on some chips.

Fair enough, sorry for stepping on your toes :(  This one can be dropped
too.

Point is taken, although if gain calibration does fail, can we do anything
about it other than log an error?  Looping forever in reset() is probably
not a good idea.

OTOH, I'm pretty sure miscalibrated phy is responsible for all the 
"unsupported jumbo" errors; I hexdumped some of those and they're all 
just noise.

(Side note, any reason we call reset() from ath5k_config_interface?
We're not changing channels, just setting the bssid.)

-- 
Bob Copeland %% www.bobcopeland.com

Re: [PATCH 3/6] ath5k: continue reset sequence if gain calibration fails

[Nick Kossifidis]

2009/1/22 Bob Copeland <me@bobcopeland.com>:
>
> Point is taken, although if gain calibration does fail, can we do anything
> about it other than log an error?  Looping forever in reset() is probably
> not a good idea.
>

We can check if the bit got cleared on next phy_calibrate and if it
hasn't we can reschedule it or schedule a reset.

> OTOH, I'm pretty sure miscalibrated phy is responsible for all the
> "unsupported jumbo" errors; I hexdumped some of those and they're all
> just noise.
>

Don't we get a phy error interrupt ?  I haven't looked into this ;-(

> (Side note, any reason we call reset() from ath5k_config_interface?
> We're not changing channels, just setting the bssid.)
>

Hmm, i'll check that and come back to you, i think we have to
re-initialize PCU but i'm not sure. Also if it's supposed to set the
bssid on a virtual if, we also have to set bssid mask to allow all
configured bssids for all vifs.

-- 
GPG ID: 0xD21DB2DB
As you read this post global entropy rises. Have Fun ;-)
Nick

Re: [PATCH 3/6] ath5k: continue reset sequence if gain calibration fails

[Bob Copeland]

On Thu, 22 Jan 2009 21:03:17 +0200, Nick Kossifidis wrote
> > OTOH, I'm pretty sure miscalibrated phy is responsible for all the
> > "unsupported jumbo" errors; I hexdumped some of those and they're all
> > just noise.
> >
> 
> Don't we get a phy error interrupt ?  I haven't looked into this ;-(

I printk-ed rs.rs_status, it's 0, so AR5K_RXERR_PHY is unset.  

I think the jumbo flag is supposed to indicate the packet is larger
than the buffer size.  However, we have a buffer size of 2500 so that
shouldn't happen for standard frames.  I did check into whether there
was a corruption issue, like skb_tailroom was smaller than a full 
packet because of an skb reuse bug or something like that.  But no, 
all were > 2500 bytes (incl roundup for cache line).  That's when 
I did the unmap and a hexdump and saw they have no 802.11 headers or
anything of the sort.  Felix suggested we just drop the warning.

> > (Side note, any reason we call reset() from ath5k_config_interface?
> > We're not changing channels, just setting the bssid.)
> >
> 
> Hmm, i'll check that and come back to you, i think we have to
> re-initialize PCU but i'm not sure. Also if it's supposed to set the
> bssid on a virtual if, we also have to set bssid mask to allow all
> configured bssids for all vifs.

Cool, let me know.  Yeah, configure_interface() doesn't do anything
with the bssid mask (other than reloading it inside hw_set_associd);
in fact it's all-1s forever right now, it seems.

--  
Bob Copeland %% www.bobcopeland.com