* [ath9k-devel] [RFC PATCH 1/2] ath9k: rename symbols in enum ath9k_internal_frame_type to avoid confusion @ 2010-03-30 3:39 Pavel Roskin 2010-03-30 3:39 ` [ath9k-devel] [RFC PATCH 2/2] ath9k: fix mixing of interrupt masks, remove ah->mask_reg Pavel Roskin 2010-03-30 16:57 ` [ath9k-devel] [RFC PATCH 1/2] ath9k: rename symbols in enum ath9k_internal_frame_type to avoid confusion Luis R. Rodriguez 0 siblings, 2 replies; 5+ messages in thread From: Pavel Roskin @ 2010-03-30 3:39 UTC (permalink / raw) To: ath9k-devel Symbols starting with "ATH9K_INT" are also used for interrupt mask. Signed-off-by: Pavel Roskin <proski@gnu.org> --- drivers/net/wireless/ath/ath9k/rc.h | 6 +++--- drivers/net/wireless/ath/ath9k/virtual.c | 2 +- drivers/net/wireless/ath/ath9k/xmit.c | 6 +++--- 3 files changed, 7 insertions(+), 7 deletions(-) diff --git a/drivers/net/wireless/ath/ath9k/rc.h b/drivers/net/wireless/ath/ath9k/rc.h index 36083dd..3d8d40c 100644 --- a/drivers/net/wireless/ath/ath9k/rc.h +++ b/drivers/net/wireless/ath/ath9k/rc.h @@ -176,9 +176,9 @@ struct ath_rate_priv { #define ATH_TX_INFO_UNDERRUN (1 << 4) enum ath9k_internal_frame_type { - ATH9K_NOT_INTERNAL, - ATH9K_INT_PAUSE, - ATH9K_INT_UNPAUSE + ATH9K_IFT_NOT_INTERNAL, + ATH9K_IFT_PAUSE, + ATH9K_IFT_UNPAUSE }; int ath_rate_control_register(void); diff --git a/drivers/net/wireless/ath/ath9k/virtual.c b/drivers/net/wireless/ath/ath9k/virtual.c index a43fbf8..e95aaa3 100644 --- a/drivers/net/wireless/ath/ath9k/virtual.c +++ b/drivers/net/wireless/ath/ath9k/virtual.c @@ -218,7 +218,7 @@ static int ath9k_send_nullfunc(struct ath_wiphy *aphy, memset(&txctl, 0, sizeof(struct ath_tx_control)); txctl.txq = &sc->tx.txq[sc->tx.hwq_map[ATH9K_WME_AC_VO]]; - txctl.frame_type = ps ? ATH9K_INT_PAUSE : ATH9K_INT_UNPAUSE; + txctl.frame_type = ps ? ATH9K_IFT_PAUSE : ATH9K_IFT_UNPAUSE; if (ath_tx_start(aphy->hw, skb, &txctl) != 0) goto exit; diff --git a/drivers/net/wireless/ath/ath9k/xmit.c b/drivers/net/wireless/ath/ath9k/xmit.c index a3b6cf2..7e1fad4 100644 --- a/drivers/net/wireless/ath/ath9k/xmit.c +++ b/drivers/net/wireless/ath/ath9k/xmit.c @@ -1568,12 +1568,12 @@ static int ath_tx_setup_buffer(struct ieee80211_hw *hw, struct ath_buf *bf, tx_info->pad[0] = 0; switch (txctl->frame_type) { - case ATH9K_NOT_INTERNAL: + case ATH9K_IFT_NOT_INTERNAL: break; - case ATH9K_INT_PAUSE: + case ATH9K_IFT_PAUSE: tx_info->pad[0] |= ATH_TX_INFO_FRAME_TYPE_PAUSE; /* fall through */ - case ATH9K_INT_UNPAUSE: + case ATH9K_IFT_UNPAUSE: tx_info->pad[0] |= ATH_TX_INFO_FRAME_TYPE_INTERNAL; break; } ^ permalink raw reply related [flat|nested] 5+ messages in thread
* [ath9k-devel] [RFC PATCH 2/2] ath9k: fix mixing of interrupt masks, remove ah->mask_reg 2010-03-30 3:39 [ath9k-devel] [RFC PATCH 1/2] ath9k: rename symbols in enum ath9k_internal_frame_type to avoid confusion Pavel Roskin @ 2010-03-30 3:39 ` Pavel Roskin 2010-03-30 16:52 ` Luis R. Rodriguez 2010-03-30 16:57 ` [ath9k-devel] [RFC PATCH 1/2] ath9k: rename symbols in enum ath9k_internal_frame_type to avoid confusion Luis R. Rodriguez 1 sibling, 1 reply; 5+ messages in thread From: Pavel Roskin @ 2010-03-30 3:39 UTC (permalink / raw) To: ath9k-devel 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; + u32 omask = sc->imask; ath_print(common, ATH_DBG_INTERRUPT, "0x%x => 0x%x\n", omask, ints); @@ -2911,7 +2915,6 @@ enum ath9k_int ath9k_hw_set_interrupts(struct ath_hw *ah, enum ath9k_int ints) AR_IMR_S2_TSFOOR | AR_IMR_S2_GTT | AR_IMR_S2_CST); ah->imrs2_reg |= mask2; REG_WRITE(ah, AR_IMR_S2, ah->imrs2_reg); - ah->mask_reg = ints; if (!(pCap->hw_caps & ATH9K_HW_CAP_AUTOSLEEP)) { if (ints & ATH9K_INT_TIM_TIMER) diff --git a/drivers/net/wireless/ath/ath9k/hw.h b/drivers/net/wireless/ath/ath9k/hw.h index 6b03e16..906aaee 100644 --- a/drivers/net/wireless/ath/ath9k/hw.h +++ b/drivers/net/wireless/ath/ath9k/hw.h @@ -478,7 +478,6 @@ struct ath_hw { struct ath9k_tx_queue_info txq[ATH9K_NUM_TX_QUEUES]; int16_t curchan_rad_index; - u32 mask_reg; u32 imrs2_reg; u32 txok_interrupt_mask; u32 txerr_interrupt_mask; diff --git a/drivers/net/wireless/ath/ath9k/mac.c b/drivers/net/wireless/ath/ath9k/mac.c index 7af823a..67da800 100644 --- a/drivers/net/wireless/ath/ath9k/mac.c +++ b/drivers/net/wireless/ath/ath9k/mac.c @@ -15,6 +15,7 @@ */ #include "hw.h" +#include "ath9k.h" static void ath9k_hw_set_txq_interrupts(struct ath_hw *ah, struct ath9k_tx_queue_info *qi) @@ -99,13 +100,15 @@ EXPORT_SYMBOL(ath9k_hw_numtxpending); */ bool ath9k_hw_updatetxtriglevel(struct ath_hw *ah, bool bIncTrigLevel) { + struct ath_common *common = ath9k_hw_common(ah); + struct ath_softc *sc = (struct ath_softc *)common->priv; u32 txcfg, curLevel, newLevel; enum ath9k_int omask; if (ah->tx_trig_level >= ah->config.max_txtrig_level) return false; - omask = ath9k_hw_set_interrupts(ah, ah->mask_reg & ~ATH9K_INT_GLOBAL); + omask = ath9k_hw_set_interrupts(ah, sc->imask & ~ATH9K_INT_GLOBAL); txcfg = REG_READ(ah, AR_TXCFG); curLevel = MS(txcfg, AR_FTRIG); ^ permalink raw reply related [flat|nested] 5+ messages in thread
* [ath9k-devel] [RFC PATCH 2/2] ath9k: fix mixing of interrupt masks, remove ah->mask_reg 2010-03-30 3:39 ` [ath9k-devel] [RFC PATCH 2/2] ath9k: fix mixing of interrupt masks, remove ah->mask_reg Pavel Roskin @ 2010-03-30 16:52 ` Luis R. Rodriguez 2010-03-31 21:54 ` Pavel Roskin 0 siblings, 1 reply; 5+ messages in thread From: Luis R. Rodriguez @ 2010-03-30 16:52 UTC (permalink / raw) To: ath9k-devel 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 ^ permalink raw reply [flat|nested] 5+ messages in thread
* [ath9k-devel] [RFC PATCH 2/2] ath9k: fix mixing of interrupt masks, remove ah->mask_reg 2010-03-30 16:52 ` Luis R. Rodriguez @ 2010-03-31 21:54 ` Pavel Roskin 0 siblings, 0 replies; 5+ messages in thread From: Pavel Roskin @ 2010-03-31 21:54 UTC (permalink / raw) To: ath9k-devel On Tue, 2010-03-30 at 09:52 -0700, Luis R. Rodriguez wrote: > 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; OK, then let's move imask from sc to ah. It looks like ah->mask_reg was misused exactly because sc->imask wasn't available in some files. -- Regards, Pavel Roskin ^ permalink raw reply [flat|nested] 5+ messages in thread
* [ath9k-devel] [RFC PATCH 1/2] ath9k: rename symbols in enum ath9k_internal_frame_type to avoid confusion 2010-03-30 3:39 [ath9k-devel] [RFC PATCH 1/2] ath9k: rename symbols in enum ath9k_internal_frame_type to avoid confusion Pavel Roskin 2010-03-30 3:39 ` [ath9k-devel] [RFC PATCH 2/2] ath9k: fix mixing of interrupt masks, remove ah->mask_reg Pavel Roskin @ 2010-03-30 16:57 ` Luis R. Rodriguez 1 sibling, 0 replies; 5+ messages in thread From: Luis R. Rodriguez @ 2010-03-30 16:57 UTC (permalink / raw) To: ath9k-devel On Mon, Mar 29, 2010 at 08:39:18PM -0700, Pavel Roskin wrote: > Symbols starting with "ATH9K_INT" are also used for interrupt mask. > > Signed-off-by: Pavel Roskin <proski@gnu.org> Acked-by: Luis R. Rodriguez <lrodriguez@atheros.com> Luis ^ permalink raw reply [flat|nested] 5+ messages in thread
end of thread, other threads:[~2010-03-31 21:54 UTC | newest] Thread overview: 5+ messages (download: mbox.gz follow: Atom feed -- links below jump to the message on this page -- 2010-03-30 3:39 [ath9k-devel] [RFC PATCH 1/2] ath9k: rename symbols in enum ath9k_internal_frame_type to avoid confusion Pavel Roskin 2010-03-30 3:39 ` [ath9k-devel] [RFC PATCH 2/2] ath9k: fix mixing of interrupt masks, remove ah->mask_reg Pavel Roskin 2010-03-30 16:52 ` Luis R. Rodriguez 2010-03-31 21:54 ` Pavel Roskin 2010-03-30 16:57 ` [ath9k-devel] [RFC PATCH 1/2] ath9k: rename symbols in enum ath9k_internal_frame_type to avoid confusion Luis R. Rodriguez
This is an external index of several public inboxes, see mirroring instructions on how to clone and mirror all data and code used by this external index.