On Tue, Oct 05, 2010 at 07:00:01PM -0700, Bruno Randolf wrote:
> On Wed October 6 2010 10:18:07 Luis R. Rodriguez wrote:
> > On Tue, Oct 5, 2010 at 2:55 AM, Bruno Randolf <br1@xxxxxxxxxxx> wrote:
> > > Update ath9k to use the common cycle counters.
> > >
> > > This also includes other changes from Felix Fietkaus "[PATCH 2/4]
> > > ath9k_hw: merge codepaths that access the cycle counter registers".
> > >
> > > Compile tested only. ath9k team please review...
> > >
> > > Signed-off-by: Felix Fietkau <nbd@xxxxxxxxxxx>
> > > Signed-off-by: Bruno Randolf <br1@xxxxxxxxxxx>
> > > ---
> > > drivers/net/wireless/ath/ath9k/ani.c | 87
> > > +++------------------------ drivers/net/wireless/ath/ath9k/ani.h
> > > | 6 --
> > > drivers/net/wireless/ath/ath9k/ar5008_phy.c | 9 +--
> > > drivers/net/wireless/ath/ath9k/ar9003_phy.c | 18 +++---
> > > 4 files changed, 22 insertions(+), 98 deletions(-)
> > >
> > > diff --git a/drivers/net/wireless/ath/ath9k/ani.c
> > > b/drivers/net/wireless/ath/ath9k/ani.c index 0496f96..0c0d01d 100644
> > > --- a/drivers/net/wireless/ath/ath9k/ani.c
> > > +++ b/drivers/net/wireless/ath/ath9k/ani.c
> > > @@ -549,47 +549,21 @@ static u8 ath9k_hw_chan_2_clockrate_mhz(struct
> > > ath_hw *ah)
> > >
> > > static int32_t ath9k_hw_ani_get_listen_time(struct ath_hw *ah)
> > > {
> > > - struct ar5416AniState *aniState;
> > > struct ath_common *common = ath9k_hw_common(ah);
> > > - u32 txFrameCount, rxFrameCount, cycleCount;
> > > - int32_t listenTime;
> > > + int32_t listen_time;
> > > + int32_t clock_rate;
> > >
> > > - txFrameCount = REG_READ(ah, AR_TFCNT);
> > > - rxFrameCount = REG_READ(ah, AR_RFCNT);
> > > - cycleCount = REG_READ(ah, AR_CCCNT);
> > > + ath_hw_cycle_counters_lock(common);
> >
> > Note the lock call here. This is what I'd like to see avoided. Can you
> > perhaps have the lock called within the core driver instead, that is,
> > add the lock call for the caller of ath9k_hw_ani_get_listen_time(). In
> > this case I see ath9k_hw_ani_get_listen_time() is static though and so
> > is its caller but ultimately we get to ath9k_hw_ani_monitor(). What
> > I'm saying is how about calling the
> > ath_hw_cycle_counters_lock(common); right before
> > ath9k_hw_ani_monitor().
>
> that would hold the lock for an unnecessarily long amount of time.
I see..
> i'm not working with ath9k, so can i leave it to you to deal with this in
> subsequent patches? in the mean time, if you prefer, i can omit the lock.
I can't think of a good alternative, so just ahead with it, if it creates a
problem we can remove it.
Luis
--
To unsubscribe from this list: send the line "unsubscribe linux-wireless" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at http://vger.kernel.org/majordomo-info.html
