On Wednesday 28 April 2010 00:21:20 Benoit PAPILLAULT wrote:
> Christian Lamparter a écrit :
> > This patch adds a basic get_survey for ar9170.
> >
> > Survey data from wlan1
> > frequency: 2412 MHz
> > noise: -85 dBm
> >
> > TODO:
> > Currently, the noise level is updated only by a channel change.
> > Now, we could simply add another ar9170_set_channel to always get
> > a fresh result, but then we risk a RF lockup.
> >
> It seems to be a good start. The code is very similar to what is used in
> ath9k. Just few questions below.
Naaa, If it was, It would have started with [PATCH] :-D
As you pointed out at the end, there is still some important
work left on the TODO.
> > ---
> > diff --git a/drivers/net/wireless/ath/ar9170/phy.c b/drivers/net/wireless/ath/ar9170/phy.c
> > index 45a415e..31ff163 100644
> > --- a/drivers/net/wireless/ath/ar9170/phy.c
> > +++ b/drivers/net/wireless/ath/ar9170/phy.c
> > @@ -1584,6 +1584,31 @@ static int ar9170_calc_noise_dbm(u32 raw_noise)
> > return (raw_noise & 0xff) >> 1;
> > }
> >
> > +int ar9170_get_noisefloor(struct ar9170 *ar)
> > +{
> > + static const u32 phy_regs[] = {
> > + 0x1c5864, 0x1c6864, 0x1c7864,
> > + 0x1c59bc, 0x1c69bc, 0x1c79bc };
> >
> Maybe #define would be more appropriate. Moreover, it's clear in my
> notes that some ar9170 registers are just ath9k registers + 0x1bc000.
I several files full of #defines for the RF,BB and MAC (and USB) in carl9170.
But I don't want to do mix those, because not all registers in those
files have been verified & tested yet.
So I copied the magics values from the original firmware...
> > + u32 phy_res[ARRAY_SIZE(phy_regs)];
> > + int err, i;
> > +
> > + BUILD_BUG_ON(ARRAY_SIZE(phy_regs) != ARRAY_SIZE(ar->noise));
> > +
> > + err = ar9170_read_mreg(ar, ARRAY_SIZE(phy_regs), phy_regs, phy_res);
> > + if (err)
> > + return err;
> > +
> > + for (i = 0; i < ARRAY_SIZE(phy_regs); i++) {
> > + ar->noise[i] = ar9170_calc_noise_dbm(
> > + (phy_res[i] >> 19) & 0x1ff);
> > +
> > + ar->noise[i + 3] = ar9170_calc_noise_dbm(
> > + (phy_res[i + 3] >> 23) & 0x1ff);
> > + }
> > +
> > + return 0;
> > +}
> > +
> > int ar9170_set_channel(struct ar9170 *ar, struct ieee80211_channel *channel,
> > enum ar9170_rf_init_mode rfi, enum ar9170_bw bw)
> > {
> > @@ -1708,12 +1733,12 @@ int ar9170_set_channel(struct ar9170 *ar, struct ieee80211_channel *channel,
> > }
> > }
> >
> > - for (i = 0; i < 2; i++) {
> > + for (i = 0; i < 3; i++) {
> >
> Why using 3 RX channels ? ar9170 is always 2x2, isn't it ? And why read
> 3 values since only one will be used in ar9170_op_get_survey?
ah, I think that's because the first CCA & EXT_CCA values are the
combinded result of both chains (might have something to do with
Smart Antenna and Maximal Ratio Combining techniques, whoever
I can't give you any reference for that, simply because most
of the papers I have are from Atheros' marketing department ;-) )
Also, this is not a hot path. We can easily save all calibration
results and make them accessible through the debug interface together
with other phy/rf related variables (e.g.: mib counters and ani registers)
> Maybe we should combine the 3 values before reporting a single value ?
> > ar->noise[i] = ar9170_calc_noise_dbm(
> > - (le32_to_cpu(vals[2 + i]) >> 19) & 0x1ff);
> > + (le32_to_cpu(vals[i + 1]) >> 19) & 0x1ff);
> >
> > - ar->noise[i + 2] = ar9170_calc_noise_dbm(
> > - (le32_to_cpu(vals[5 + i]) >> 23) & 0x1ff);
> > + ar->noise[i + 3] = ar9170_calc_noise_dbm(
> > + (le32_to_cpu(vals[i + 4]) >> 23) & 0x1ff);
> > }
> >
> > ar->channel = channel;
> >
> Moreover (but my patch for ath9k has the very same error), I think we
> are reported the noise floor calibration result which is not the noise
> at all... that might be another story anyway.
True, but hey we've reported these noise figures for a very long time now
and no one complained, so the delta can't be that important in RL :-D.
Of course we could also initiate another NF calibration right here,
but due to the number of people reporting PHY problems with ar9170,
I'm somewhat nervous about that.
Regards,
Chr
--
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
