[ath9k-devel] [RFC PATCH 2/2] ath9k: fix mixing of interrupt masks, remove ah->mask_reg

[Luis R. Rodriguez <lrodriguez@atheros.com>]

On Mon, Mar 29, 2010 at 08:39:58PM -0700, Pavel Roskin wrote:
> ah->mask_reg is used inconsistently.  ath9k_hw_init_chain_masks() uses
> it to cache the value it writes to AR_IMR.  But once
> ath9k_hw_set_interrupts() is called, ah->mask_reg starts caching the
> interrupt mask in the format used in sc->imask, which differs in several
> bits.
> 
> Use sc->imask instead of ah->mask_reg as omask ath9k_hw_set_interrupts()
> and ath9k_hw_updatetxtriglevel().  Remove ah->mask_reg, as it becomes
> write only.  Use a local variable in ath9k_hw_init_chain_masks().
> 
> Signed-off-by: Pavel Roskin <proski@gnu.org>
> Reported-by: Julia Lawall <julia@diku.dk>
> ---
> 
> Adding ath9k.h to hw.c and mac.c could raise some eyebrows, but the
> alternative is moving imask from sc to ah, which would be more
> intrusive.
> 
> I think sc and ah should be merged eventually, so all files that know
> the ah structure will know sc as well.  There is no gain from hiding sc
> from the low-level code.
> 
>  drivers/net/wireless/ath/ath9k/hw.c  |   19 +++++++++++--------
>  drivers/net/wireless/ath/ath9k/hw.h  |    1 -
>  drivers/net/wireless/ath/ath9k/mac.c |    5 ++++-
>  3 files changed, 15 insertions(+), 10 deletions(-)
> 
> diff --git a/drivers/net/wireless/ath/ath9k/hw.c b/drivers/net/wireless/ath/ath9k/hw.c
> index 77db932..791aeb9 100644
> --- a/drivers/net/wireless/ath/ath9k/hw.c
> +++ b/drivers/net/wireless/ath/ath9k/hw.c
> @@ -17,6 +17,7 @@
>  #include <linux/io.h>
>  #include <asm/unaligned.h>
>  
> +#include "ath9k.h"
>  #include "hw.h"
>  #include "rc.h"
>  #include "initvals.h"
> @@ -1120,23 +1121,25 @@ static void ath9k_hw_init_chain_masks(struct ath_hw *ah)
>  static void ath9k_hw_init_interrupt_masks(struct ath_hw *ah,
>  					  enum nl80211_iftype opmode)
>  {
> -	ah->mask_reg = AR_IMR_TXERR |
> +	u32 imr_reg;
> +
> +	imr_reg = AR_IMR_TXERR |
>  		AR_IMR_TXURN |
>  		AR_IMR_RXERR |
>  		AR_IMR_RXORN |
>  		AR_IMR_BCNMISC;
>  
>  	if (ah->config.rx_intr_mitigation)
> -		ah->mask_reg |= AR_IMR_RXINTM | AR_IMR_RXMINTR;
> +		imr_reg |= AR_IMR_RXINTM | AR_IMR_RXMINTR;
>  	else
> -		ah->mask_reg |= AR_IMR_RXOK;
> +		imr_reg |= AR_IMR_RXOK;
>  
> -	ah->mask_reg |= AR_IMR_TXOK;
> +	imr_reg |= AR_IMR_TXOK;
>  
>  	if (opmode == NL80211_IFTYPE_AP)
> -		ah->mask_reg |= AR_IMR_MIB;
> +		imr_reg |= AR_IMR_MIB;
>  
> -	REG_WRITE(ah, AR_IMR, ah->mask_reg);
> +	REG_WRITE(ah, AR_IMR, imr_reg);
>  	ah->imrs2_reg |= AR_IMR_S2_GTT;
>  	REG_WRITE(ah, AR_IMR_S2, ah->imrs2_reg);
>  
> @@ -2839,10 +2842,11 @@ EXPORT_SYMBOL(ath9k_hw_getisr);
>  
>  enum ath9k_int ath9k_hw_set_interrupts(struct ath_hw *ah, enum ath9k_int ints)
>  {
> -	u32 omask = ah->mask_reg;
>  	u32 mask, mask2;
>  	struct ath9k_hw_capabilities *pCap = &ah->caps;
>  	struct ath_common *common = ath9k_hw_common(ah);
> +	struct ath_softc *sc = (struct ath_softc *)common->priv;

This is exactly what I was trying to prevent, lets please try to figure out
another way, NACK for now.

Consider

struct ath9k_htc_priv *priv = (struct ath9k_htc_priv *) common->priv;

  Luis