From mboxrd@z Thu Jan 1 00:00:00 1970 From: Luis R. Rodriguez Date: Tue, 30 Mar 2010 09:52:19 -0700 Subject: [ath9k-devel] [RFC PATCH 2/2] ath9k: fix mixing of interrupt masks, remove ah->mask_reg In-Reply-To: <20100330033924.7683.13491.stgit@mj.roinet.com> References: <20100330033848.7683.63114.stgit@mj.roinet.com> <20100330033924.7683.13491.stgit@mj.roinet.com> Message-ID: <20100330165219.GA25095@tux> List-Id: MIME-Version: 1.0 Content-Type: text/plain; charset="us-ascii" Content-Transfer-Encoding: 7bit To: ath9k-devel@lists.ath9k.org 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 > Reported-by: Julia Lawall > --- > > 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 > #include > > +#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