2010-05-19 01:31:52

by Bruno Randolf

[permalink] [raw]
Subject: [PATCH v2 00/20] pending ath5k + antenna patches

These are all my pending ath5k patches + the antenna support for cfg80211
rebased against 2.6.34 and based on the previous patch i sent for stable
("ath5k: consistently use rx_bufsize for RX DMA").

Also i'd like to note that there are quite a few ath5k patches which haven't
made it into linux-next yet. It would be great to get them into 2.6.35...

thanks,
bruno

---

Bruno Randolf (20):
ath5k: add debugfs file for queue debugging
ath5k: wake queues on reset
ath5k: initialize calibration timers
ath5k: move noise floor calibration into tasklet
ath5k: Stop queues only for NF calibration
ath5k: run NF calibration only every 60 seconds
ath5k: remove ATH_TRACE macro
ath5k: clarify logic when to enable spur mitigation filter
ath5k: use ath5k_softc as driver data
ath5k: add sysfs files for ANI parameters
ath5k: always calculate ANI listen time
ath5k: print error message if ANI levels are out of range
cfg80211: Add nl80211 antenna configuration
mac80211: Add antenna configuration
ath5k: Add support for antenna configuration
ath5k: remove setting ANI and antenna thru debugfs files
ath5k: fix NULL pointer in antenna configuration
ath5k: update AR5K_PHY_RESTART_DIV_GC values to match masks
ath5k: new function for setting the antenna switch table
ath5k: no need to save/restore the default antenna


drivers/net/wireless/ath/ath5k/Makefile | 1
drivers/net/wireless/ath/ath5k/ani.c | 20 +++--
drivers/net/wireless/ath/ath5k/ath5k.h | 7 ++
drivers/net/wireless/ath/ath5k/attach.c | 2 -
drivers/net/wireless/ath/ath5k/base.c | 89 ++++++++++++++++--------
drivers/net/wireless/ath/ath5k/caps.c | 7 --
drivers/net/wireless/ath/ath5k/debug.c | 107 ++++++++++++++++++++---------
drivers/net/wireless/ath/ath5k/debug.h | 9 --
drivers/net/wireless/ath/ath5k/desc.c | 7 --
drivers/net/wireless/ath/ath5k/dma.c | 13 ---
drivers/net/wireless/ath/ath5k/eeprom.c | 1
drivers/net/wireless/ath/ath5k/gpio.c | 7 --
drivers/net/wireless/ath/ath5k/pcu.c | 24 ------
drivers/net/wireless/ath/ath5k/phy.c | 89 ++++++++++++++----------
drivers/net/wireless/ath/ath5k/qcu.c | 9 --
drivers/net/wireless/ath/ath5k/reset.c | 64 +++--------------
drivers/net/wireless/ath/ath5k/sysfs.c | 116 +++++++++++++++++++++++++++++++
include/linux/nl80211.h | 12 +++
include/net/cfg80211.h | 3 +
include/net/mac80211.h | 2 +
net/mac80211/cfg.c | 16 ++++
net/mac80211/driver-ops.h | 23 ++++++
net/mac80211/driver-trace.h | 50 +++++++++++++
net/wireless/nl80211.c | 116 +++++++++++++++++++++++++++++++
24 files changed, 554 insertions(+), 240 deletions(-)
create mode 100644 drivers/net/wireless/ath/ath5k/sysfs.c

--
Signature


2010-05-19 01:32:39

by Bruno Randolf

[permalink] [raw]
Subject: [PATCH v2 09/20] ath5k: use ath5k_softc as driver data

It's our "private driver data"... It's used more often and hw is the mac80211
part. This makes more sense with the next (sysfs) patch.

Signed-off-by: Bruno Randolf <[email protected]>
---
drivers/net/wireless/ath/ath5k/base.c | 15 ++++++---------
1 files changed, 6 insertions(+), 9 deletions(-)

diff --git a/drivers/net/wireless/ath/ath5k/base.c b/drivers/net/wireless/ath/ath5k/base.c
index 496e2db..3b43b76 100644
--- a/drivers/net/wireless/ath/ath5k/base.c
+++ b/drivers/net/wireless/ath/ath5k/base.c
@@ -578,7 +578,7 @@ ath5k_pci_probe(struct pci_dev *pdev,
spin_lock_init(&sc->block);

/* Set private data */
- pci_set_drvdata(pdev, hw);
+ pci_set_drvdata(pdev, sc);

/* Setup interrupt handler */
ret = request_irq(pdev->irq, ath5k_intr, IRQF_SHARED, "ath", sc);
@@ -694,25 +694,23 @@ err:
static void __devexit
ath5k_pci_remove(struct pci_dev *pdev)
{
- struct ieee80211_hw *hw = pci_get_drvdata(pdev);
- struct ath5k_softc *sc = hw->priv;
+ struct ath5k_softc *sc = pci_get_drvdata(pdev);

ath5k_debug_finish_device(sc);
- ath5k_detach(pdev, hw);
+ ath5k_detach(pdev, sc->hw);
ath5k_hw_detach(sc->ah);
kfree(sc->ah);
free_irq(pdev->irq, sc);
pci_iounmap(pdev, sc->iobase);
pci_release_region(pdev, 0);
pci_disable_device(pdev);
- ieee80211_free_hw(hw);
+ ieee80211_free_hw(sc->hw);
}

#ifdef CONFIG_PM
static int ath5k_pci_suspend(struct device *dev)
{
- struct ieee80211_hw *hw = pci_get_drvdata(to_pci_dev(dev));
- struct ath5k_softc *sc = hw->priv;
+ struct ath5k_softc *sc = pci_get_drvdata(to_pci_dev(dev));

ath5k_led_off(sc);
return 0;
@@ -721,8 +719,7 @@ static int ath5k_pci_suspend(struct device *dev)
static int ath5k_pci_resume(struct device *dev)
{
struct pci_dev *pdev = to_pci_dev(dev);
- struct ieee80211_hw *hw = pci_get_drvdata(pdev);
- struct ath5k_softc *sc = hw->priv;
+ struct ath5k_softc *sc = pci_get_drvdata(pdev);

/*
* Suspend/Resume resets the PCI configuration space, so we have to


2010-05-20 01:12:18

by Bruno Randolf

[permalink] [raw]
Subject: Re: [ath5k-devel] [PATCH v2 13/20] cfg80211: Add nl80211 antenna configuration

On Thursday 20 May 2010 09:51:43 Luis R. Rodriguez wrote:
> On Wed, May 19, 2010 at 05:35:40PM -0700, Bruno Randolf wrote:
> > On Thursday 20 May 2010 02:07:25 you wrote:
> > > On Tue, May 18, 2010 at 6:31 PM, Bruno Randolf <[email protected]> wrote:
> > > > + * @NL80211_ATTR_ANTENNA_TX: Bitmap of antennas to use for
> > > > transmitting. + * @NL80211_ATTR_ANTENNA_RX: Bitmap of antennas to
> > > > use for receiving.
> > >
> > > This gets the job done, but that's it. The API defined allows for a
> > > hugely loose implementation on each driver.
> >
> > i tried to define it like this, in the commit log:
> > The antenna configuration is defined as a bitmap of allowed antennas.
> > This bitmap is 8 bit at the moment, each bit representing one
> > antenna. If multiple antennas are selected, the driver may use
> > diversity for receive and transmit.
> >
> > is this not precise enough?
>
> No, the commit log is just a placeholder of information, if you want to
> define API you do it through the kdoc so you can slap people when then
> submit patches that do not follow it. The kdoc above allows the flexibility
> you were looking for but that I do not think we should have since it will
> confuse the users who want to tune antenna settings on different drivers.

you are talking about the place where to put the definition, not about the
definition itself. i agree, the definition should be in the kdoc, and i'll
update the patch. what's wrong with the definition itself?

> I'd much prefer to see a consistant API for antenna settings so that if
> they know how to do it with ath5k, then they know how to do it for b43,
> or whatever.

that's what i'm trying to achieve...

> > i am mostly concerned with what i believe is the
> > most common usecase (selecting one fixed antenna, or antenna diversity).
>
> In that case, then I recommend to restrict your patch to that case, and
> forget about a bitmask. Since lagcy just has antenna A, B, or auto, how
> about defining an API for just that?
>
> > i'd say this is 99% of all use cases.
>
> For now, yes. I'm thinking about 802.11n though and for that we can
> just wait, and use some other API once someone with more time wants
> to iron it out.

yes, i'm trying to find an API for 'legacy' (as you call it).

> > but this API would also allow us to define
> > more advanved things like 'transmit on antenna 1, receive on antenna 2".
>
> Sure.
>
> > i know that there are possibly more crazy (and very rare) configurations,
> > like use several panel antennas + omni, which can't be configured with
> > this API,
>
> How about we ignore those then on this API.

thats what i do. ;)

> > but it's hard to find a common API for all possibilities, and i think it
> > would be possible to extend the API later on for such cases.
>
> Understood, how about just defining something very basic and simple
> for legacy based operation mode?

i think my implementation is quite basic and simple ;)

> > > And yet for another driver it could be something completely different
> > > in usersepace.
> >
> > what do you think that could be, realistically, given the above
> > definition?
>
> Well, anything that has to do with tx / rx antennas.

that's not very clear. can you give me an example?

> > > I think it would be better for us to define a static
> > > API for all legacy cards and another for 802.11n cards, or unify them
> > > but to be very specific about the API for antenna settings/chainmask
> > > settings.
> >
> > sure. any suggestions?
>
> Sure how about FIXED_A, FIXED_B, DIVERSITY ?

that's very ath5k centric.

what if there is a 'legacy' hardware with 3 or more antennas?
what if we want to configure RX on antenna 1, TX on antenna 2?
what if we want to use RX diversity but always TX on a fixed antenna?

these are possible and useful configurations, which are not supported right
now by ath5k but it's easy to add them.

i don't see how "my" API is too complicated, and i think it allows for a clear
configuration of these cases as well.

your criticism seems to be based on the fact that it's not clear how to handle
802.11n chainmask + antenna configuration, but this is not what my patch is
concerned about. let's go step by step...

bruno

2010-05-20 12:58:15

by Nick Kossifidis

[permalink] [raw]
Subject: Re: [PATCH v2 10/20] ath5k: add sysfs files for ANI parameters

MjAxMC81LzE5IEJydW5vIFJhbmRvbGYgPGJyMUBlaW5mYWNoLm9yZz46Cj4gL3N5cy9jbGFzcy9p
ZWVlODAyMTEvcGh5MC9kZXZpY2UvYW5pL2FuaV9tb2RlCj4gL3N5cy9jbGFzcy9pZWVlODAyMTEv
cGh5MC9kZXZpY2UvYW5pL25vaXNlX2ltbXVuaXR5X2xldmVsCj4gL3N5cy9jbGFzcy9pZWVlODAy
MTEvcGh5MC9kZXZpY2UvYW5pL3NwdXJfbGV2ZWwKPiAvc3lzL2NsYXNzL2llZWU4MDIxMS9waHkw
L2RldmljZS9hbmkvZmlyc3RlcF9sZXZlbAo+IC9zeXMvY2xhc3MvaWVlZTgwMjExL3BoeTAvZGV2
aWNlL2FuaS9vZmRtX3dlYWtfc2lnbmFsX2RldGVjdGlvbgo+IC9zeXMvY2xhc3MvaWVlZTgwMjEx
L3BoeTAvZGV2aWNlL2FuaS9jY2tfd2Vha19zaWduYWxfZGV0ZWN0aW9uCj4gL3N5cy9jbGFzcy9p
ZWVlODAyMTEvcGh5MC9kZXZpY2UvYW5pL25vaXNlX2ltbXVuaXR5X2xldmVsX21heAo+IC9zeXMv
Y2xhc3MvaWVlZTgwMjExL3BoeTAvZGV2aWNlL2FuaS9zcHVyX2xldmVsX21heAo+IC9zeXMvY2xh
c3MvaWVlZTgwMjExL3BoeTAvZGV2aWNlL2FuaS9maXJzdGVwX2xldmVsX21heAo+Cj4gc3lzZnMg
aGFzIGEgbG90IG9mIHN5bWxpbmtzLCBzbyB5b3UgY2FuIGZpbmQgdGhlIGZpbGVzIGFsc28gaW4g
b3RoZXIgbG9jYXRpb25zLAo+IGxpa2UgKGJ5IFBDSSBJRCkgL3N5cy9kZXZpY2VzL3BjaTAwMDA6
MDAvMDAwMDowMDoxMS4wL2FuaSBhbmQgb3RoZXJzLgo+Cj4gU2lnbmVkLW9mZi1ieTogQnJ1bm8g
UmFuZG9sZiA8YnIxQGVpbmZhY2gub3JnPgo+IC0tLQo+IMKgZHJpdmVycy9uZXQvd2lyZWxlc3Mv
YXRoL2F0aDVrL01ha2VmaWxlIHwgwqAgwqAxCj4gwqBkcml2ZXJzL25ldC93aXJlbGVzcy9hdGgv
YXRoNWsvYXRoNWsuaCDCoHwgwqAgwqAzICsKPiDCoGRyaXZlcnMvbmV0L3dpcmVsZXNzL2F0aC9h
dGg1ay9iYXNlLmMgwqAgfCDCoCDCoDMgKwo+IMKgZHJpdmVycy9uZXQvd2lyZWxlc3MvYXRoL2F0
aDVrL3Jlc2V0LmMgwqB8IMKgIMKgMQo+IMKgZHJpdmVycy9uZXQvd2lyZWxlc3MvYXRoL2F0aDVr
L3N5c2ZzLmMgwqB8IMKgMTE2ICsrKysrKysrKysrKysrKysrKysrKysrKysrKysrKysKPiDCoDUg
ZmlsZXMgY2hhbmdlZCwgMTIzIGluc2VydGlvbnMoKyksIDEgZGVsZXRpb25zKC0pCj4gwqBjcmVh
dGUgbW9kZSAxMDA2NDQgZHJpdmVycy9uZXQvd2lyZWxlc3MvYXRoL2F0aDVrL3N5c2ZzLmMKPgo+
IGRpZmYgLS1naXQgYS9kcml2ZXJzL25ldC93aXJlbGVzcy9hdGgvYXRoNWsvTWFrZWZpbGUgYi9k
cml2ZXJzL25ldC93aXJlbGVzcy9hdGgvYXRoNWsvTWFrZWZpbGUKPiBpbmRleCBjYzA5NTk1Li4y
MjQyYTE0IDEwMDY0NAo+IC0tLSBhL2RyaXZlcnMvbmV0L3dpcmVsZXNzL2F0aC9hdGg1ay9NYWtl
ZmlsZQo+ICsrKyBiL2RyaXZlcnMvbmV0L3dpcmVsZXNzL2F0aC9hdGg1ay9NYWtlZmlsZQo+IEBA
IC0xMyw1ICsxMyw2IEBAIGF0aDVrLXkgwqAgwqAgwqAgwqAgwqAgwqAgwqAgwqAgwqAgwqAgwqAg
wqAgwqAgwqAgwqAgKz0gYmFzZS5vCj4gwqBhdGg1ay15IMKgIMKgIMKgIMKgIMKgIMKgIMKgIMKg
IMKgIMKgIMKgIMKgIMKgIMKgIMKgIMKgKz0gbGVkLm8KPiDCoGF0aDVrLXkgwqAgwqAgwqAgwqAg
wqAgwqAgwqAgwqAgwqAgwqAgwqAgwqAgwqAgwqAgwqAgwqArPSByZmtpbGwubwo+IMKgYXRoNWst
eSDCoCDCoCDCoCDCoCDCoCDCoCDCoCDCoCDCoCDCoCDCoCDCoCDCoCDCoCDCoCDCoCs9IGFuaS5v
Cj4gK2F0aDVrLXkgwqAgwqAgwqAgwqAgwqAgwqAgwqAgwqAgwqAgwqAgwqAgwqAgwqAgwqAgwqAg
wqArPSBzeXNmcy5vCj4gwqBhdGg1ay0kKENPTkZJR19BVEg1S19ERUJVRykgwqAgwqArPSBkZWJ1
Zy5vCj4gwqBvYmotJChDT05GSUdfQVRINUspIMKgIMKgIMKgIMKgIMKgIMKgKz0gYXRoNWsubwo+
IGRpZmYgLS1naXQgYS9kcml2ZXJzL25ldC93aXJlbGVzcy9hdGgvYXRoNWsvYXRoNWsuaCBiL2Ry
aXZlcnMvbmV0L3dpcmVsZXNzL2F0aC9hdGg1ay9hdGg1ay5oCj4gaW5kZXggZDZmOWFmZS4uZWFj
ZTc0ZCAxMDA2NDQKPiAtLS0gYS9kcml2ZXJzL25ldC93aXJlbGVzcy9hdGgvYXRoNWsvYXRoNWsu
aAo+ICsrKyBiL2RyaXZlcnMvbmV0L3dpcmVsZXNzL2F0aC9hdGg1ay9hdGg1ay5oCj4gQEAgLTEx
NTAsNiArMTE1MCw5IEBAIHN0cnVjdCBhdGg1a19odyB7Cj4gwqBpbnQgYXRoNWtfaHdfYXR0YWNo
KHN0cnVjdCBhdGg1a19zb2Z0YyAqc2MpOwo+IMKgdm9pZCBhdGg1a19od19kZXRhY2goc3RydWN0
IGF0aDVrX2h3ICphaCk7Cj4KPiAraW50IGF0aDVrX3N5c2ZzX3JlZ2lzdGVyKHN0cnVjdCBhdGg1
a19zb2Z0YyAqc2MpOwo+ICt2b2lkIGF0aDVrX3N5c2ZzX3VucmVnaXN0ZXIoc3RydWN0IGF0aDVr
X3NvZnRjICpzYyk7Cj4gKwo+IMKgLyogTEVEIGZ1bmN0aW9ucyAqLwo+IMKgaW50IGF0aDVrX2lu
aXRfbGVkcyhzdHJ1Y3QgYXRoNWtfc29mdGMgKnNjKTsKPiDCoHZvaWQgYXRoNWtfbGVkX2VuYWJs
ZShzdHJ1Y3QgYXRoNWtfc29mdGMgKnNjKTsKPiBkaWZmIC0tZ2l0IGEvZHJpdmVycy9uZXQvd2ly
ZWxlc3MvYXRoL2F0aDVrL2Jhc2UuYyBiL2RyaXZlcnMvbmV0L3dpcmVsZXNzL2F0aC9hdGg1ay9i
YXNlLmMKPiBpbmRleCAzYjQzYjc2Li5kNWQ1MTRmIDEwMDY0NAo+IC0tLSBhL2RyaXZlcnMvbmV0
L3dpcmVsZXNzL2F0aC9hdGg1ay9iYXNlLmMKPiArKysgYi9kcml2ZXJzL25ldC93aXJlbGVzcy9h
dGgvYXRoNWsvYmFzZS5jCj4gQEAgLTg2MSw2ICs4NjEsOCBAQCBhdGg1a19hdHRhY2goc3RydWN0
IHBjaV9kZXYgKnBkZXYsIHN0cnVjdCBpZWVlODAyMTFfaHcgKmh3KQo+Cj4gwqAgwqAgwqAgwqBh
dGg1a19pbml0X2xlZHMoc2MpOwo+Cj4gKyDCoCDCoCDCoCBhdGg1a19zeXNmc19yZWdpc3Rlcihz
Yyk7Cj4gKwo+IMKgIMKgIMKgIMKgcmV0dXJuIDA7Cj4gwqBlcnJfcXVldWVzOgo+IMKgIMKgIMKg
IMKgYXRoNWtfdHhxX3JlbGVhc2Uoc2MpOwo+IEBAIC04OTYsNiArODk4LDcgQEAgYXRoNWtfZGV0
YWNoKHN0cnVjdCBwY2lfZGV2ICpwZGV2LCBzdHJ1Y3QgaWVlZTgwMjExX2h3ICpodykKPiDCoCDC
oCDCoCDCoGF0aDVrX2h3X3JlbGVhc2VfdHhfcXVldWUoc2MtPmFoLCBzYy0+YmhhbHEpOwo+IMKg
IMKgIMKgIMKgYXRoNWtfdW5yZWdpc3Rlcl9sZWRzKHNjKTsKPgo+ICsgwqAgwqAgwqAgYXRoNWtf
c3lzZnNfdW5yZWdpc3RlcihzYyk7Cj4gwqAgwqAgwqAgwqAvKgo+IMKgIMKgIMKgIMKgICogTkI6
IGNhbid0IHJlY2xhaW0gdGhlc2UgdW50aWwgYWZ0ZXIgaWVlZTgwMjExX2lmZGV0YWNoCj4gwqAg
wqAgwqAgwqAgKiByZXR1cm5zIGJlY2F1c2Ugd2UnbGwgZ2V0IGNhbGxlZCBiYWNrIHRvIHJlY2xh
aW0gbm9kZQo+IGRpZmYgLS1naXQgYS9kcml2ZXJzL25ldC93aXJlbGVzcy9hdGgvYXRoNWsvcmVz
ZXQuYyBiL2RyaXZlcnMvbmV0L3dpcmVsZXNzL2F0aC9hdGg1ay9yZXNldC5jCj4gaW5kZXggMjg5
MGE3Zi4uYjFkMzIzOSAxMDA2NDQKPiAtLS0gYS9kcml2ZXJzL25ldC93aXJlbGVzcy9hdGgvYXRo
NWsvcmVzZXQuYwo+ICsrKyBiL2RyaXZlcnMvbmV0L3dpcmVsZXNzL2F0aC9hdGg1ay9yZXNldC5j
Cj4gQEAgLTg1MSw3ICs4NTEsNiBAQCBzdGF0aWMgdm9pZCBhdGg1a19od19jb21taXRfZWVwcm9t
X3NldHRpbmdzKHN0cnVjdCBhdGg1a19odyAqYWgsCj4gwqAgwqAgwqAgwqAgwqAgwqAgwqAgwqAg
wqAgwqAgwqAgwqBBUjVLX1BIWV9ORl9USFJFU0g2MiwKPiDCoCDCoCDCoCDCoCDCoCDCoCDCoCDC
oCDCoCDCoCDCoCDCoGVlLT5lZV90aHJfNjJbZWVfbW9kZV0pOwo+Cj4gLQo+IMKgIMKgIMKgIMKg
LyogRmFsc2UgZGV0ZWN0IGJhY2tvZmYgZm9yIGNoYW5uZWxzCj4gwqAgwqAgwqAgwqAgKiB0aGF0
IGhhdmUgc3B1ciBub2lzZS4gV3JpdGUgdGhlIG5ldwo+IMKgIMKgIMKgIMKgICogY3ljbGljIHBv
d2VyIFJTU0kgdGhyZXNob2xkLiAqLwo+IGRpZmYgLS1naXQgYS9kcml2ZXJzL25ldC93aXJlbGVz
cy9hdGgvYXRoNWsvc3lzZnMuYyBiL2RyaXZlcnMvbmV0L3dpcmVsZXNzL2F0aC9hdGg1ay9zeXNm
cy5jCj4gbmV3IGZpbGUgbW9kZSAxMDA2NDQKPiBpbmRleCAwMDAwMDAwLi45MDc1N2RlCj4gLS0t
IC9kZXYvbnVsbAo+ICsrKyBiL2RyaXZlcnMvbmV0L3dpcmVsZXNzL2F0aC9hdGg1ay9zeXNmcy5j
Cj4gQEAgLTAsMCArMSwxMTYgQEAKPiArI2luY2x1ZGUgPGxpbnV4L2RldmljZS5oPgo+ICsjaW5j
bHVkZSA8bGludXgvcGNpLmg+Cj4gKwo+ICsjaW5jbHVkZSAiYmFzZS5oIgo+ICsjaW5jbHVkZSAi
YXRoNWsuaCIKPiArI2luY2x1ZGUgInJlZy5oIgo+ICsKPiArI2RlZmluZSBTSU1QTEVfU0hPV19T
VE9SRShuYW1lLCBnZXQsIHNldCkgwqAgwqAgwqAgwqAgwqAgwqAgwqAgwqAgwqAgwqAgwqAgwqAg
wqAgwqAgwqBcCj4gK3N0YXRpYyBzc2l6ZV90IGF0aDVrX2F0dHJfc2hvd18jI25hbWUoc3RydWN0
IGRldmljZSAqZGV2LCDCoCDCoCDCoCDCoCDCoCDCoCDCoFwKPiArIMKgIMKgIMKgIMKgIMKgIMKg
IMKgIMKgIMKgIMKgIMKgIHN0cnVjdCBkZXZpY2VfYXR0cmlidXRlICphdHRyLCDCoCDCoCDCoCDC
oCDCoCDCoCDCoCDCoCDCoFwKPiArIMKgIMKgIMKgIMKgIMKgIMKgIMKgIMKgIMKgIMKgIMKgIGNo
YXIgKmJ1ZikgwqAgwqAgwqAgwqAgwqAgwqAgwqAgwqAgwqAgwqAgwqAgwqAgwqAgwqAgwqAgwqAg
wqAgwqAgwqBcCj4gK3sgwqAgwqAgwqAgwqAgwqAgwqAgwqAgwqAgwqAgwqAgwqAgwqAgwqAgwqAg
wqAgwqAgwqAgwqAgwqAgwqAgwqAgwqAgwqAgwqAgwqAgwqAgwqAgwqAgwqAgwqAgwqAgwqAgwqAg
wqAgwqBcCj4gKyDCoCDCoCDCoCBzdHJ1Y3QgYXRoNWtfc29mdGMgKnNjID0gZGV2X2dldF9kcnZk
YXRhKGRldik7IMKgIMKgIMKgIMKgIMKgIMKgIMKgIMKgIMKgXAo+ICsgwqAgwqAgwqAgcmV0dXJu
IHNucHJpbnRmKGJ1ZiwgUEFHRV9TSVpFLCAiJWRcbiIsIGdldCk7IMKgIMKgIMKgIMKgIMKgIMKg
IMKgIMKgIMKgIFwKPiArfSDCoCDCoCDCoCDCoCDCoCDCoCDCoCDCoCDCoCDCoCDCoCDCoCDCoCDC
oCDCoCDCoCDCoCDCoCDCoCDCoCDCoCDCoCDCoCDCoCDCoCDCoCDCoCDCoCDCoCDCoCDCoCDCoCDC
oCDCoCDCoFwKPiArIMKgIMKgIMKgIMKgIMKgIMKgIMKgIMKgIMKgIMKgIMKgIMKgIMKgIMKgIMKg
IMKgIMKgIMKgIMKgIMKgIMKgIMKgIMKgIMKgIMKgIMKgIMKgIMKgIMKgIMKgIMKgIMKgIMKgIMKg
IMKgIFwKPiArc3RhdGljIHNzaXplX3QgYXRoNWtfYXR0cl9zdG9yZV8jI25hbWUoc3RydWN0IGRl
dmljZSAqZGV2LCDCoCDCoCDCoCDCoCDCoCDCoCBcCj4gKyDCoCDCoCDCoCDCoCDCoCDCoCDCoCDC
oCDCoCDCoCDCoCBzdHJ1Y3QgZGV2aWNlX2F0dHJpYnV0ZSAqYXR0ciwgwqAgwqAgwqAgwqAgwqAg
wqAgwqAgwqAgwqBcCj4gKyDCoCDCoCDCoCDCoCDCoCDCoCDCoCDCoCDCoCDCoCDCoCBjb25zdCBj
aGFyICpidWYsIHNpemVfdCBjb3VudCkgwqAgwqAgwqAgwqAgwqAgwqAgwqAgwqAgwqBcCj4gK3sg
wqAgwqAgwqAgwqAgwqAgwqAgwqAgwqAgwqAgwqAgwqAgwqAgwqAgwqAgwqAgwqAgwqAgwqAgwqAg
wqAgwqAgwqAgwqAgwqAgwqAgwqAgwqAgwqAgwqAgwqAgwqAgwqAgwqAgwqAgwqBcCj4gKyDCoCDC
oCDCoCBzdHJ1Y3QgYXRoNWtfc29mdGMgKnNjID0gZGV2X2dldF9kcnZkYXRhKGRldik7IMKgIMKg
IMKgIMKgIMKgIMKgIMKgIMKgIMKgXAo+ICsgwqAgwqAgwqAgaW50IHZhbDsgwqAgwqAgwqAgwqAg
wqAgwqAgwqAgwqAgwqAgwqAgwqAgwqAgwqAgwqAgwqAgwqAgwqAgwqAgwqAgwqAgwqAgwqAgwqAg
wqAgwqAgwqAgwqAgwqBcCj4gKyDCoCDCoCDCoCDCoCDCoCDCoCDCoCDCoCDCoCDCoCDCoCDCoCDC
oCDCoCDCoCDCoCDCoCDCoCDCoCDCoCDCoCDCoCDCoCDCoCDCoCDCoCDCoCDCoCDCoCDCoCDCoCDC
oCDCoCDCoCDCoCBcCj4gKyDCoCDCoCDCoCB2YWwgPSAoaW50KXNpbXBsZV9zdHJ0b3VsKGJ1Ziwg
TlVMTCwgMTApOyDCoCDCoCDCoCDCoCDCoCDCoCDCoCDCoCDCoCDCoCDCoCBcCj4gKyDCoCDCoCDC
oCBzZXQoc2MtPmFoLCB2YWwpOyDCoCDCoCDCoCDCoCDCoCDCoCDCoCDCoCDCoCDCoCDCoCDCoCDC
oCDCoCDCoCDCoCDCoCDCoCDCoCDCoCDCoCDCoCDCoCBcCj4gKyDCoCDCoCDCoCByZXR1cm4gY291
bnQ7IMKgIMKgIMKgIMKgIMKgIMKgIMKgIMKgIMKgIMKgIMKgIMKgIMKgIMKgIMKgIMKgIMKgIMKg
IMKgIMKgIMKgIMKgIMKgIMKgIMKgIFwKPiArfSDCoCDCoCDCoCDCoCDCoCDCoCDCoCDCoCDCoCDC
oCDCoCDCoCDCoCDCoCDCoCDCoCDCoCDCoCDCoCDCoCDCoCDCoCDCoCDCoCDCoCDCoCDCoCDCoCDC
oCDCoCDCoCDCoCDCoCDCoCDCoFwKPiArc3RhdGljIERFVklDRV9BVFRSKG5hbWUsIFNfSVJVR08g
fCBTX0lXVVNSLCDCoCDCoCDCoCDCoCDCoCDCoCDCoCDCoCDCoCDCoCDCoCDCoCDCoCDCoFwKPiAr
IMKgIMKgIMKgIMKgIMKgIMKgIMKgIMKgIMKgYXRoNWtfYXR0cl9zaG93XyMjbmFtZSwgYXRoNWtf
YXR0cl9zdG9yZV8jI25hbWUpCj4gKwo+ICsjZGVmaW5lIFNJTVBMRV9TSE9XKG5hbWUsIGdldCkg
wqAgwqAgwqAgwqAgwqAgwqAgwqAgwqAgwqAgwqAgwqAgwqAgwqAgwqAgwqAgwqAgwqAgwqAgwqAg
wqAgXAo+ICtzdGF0aWMgc3NpemVfdCBhdGg1a19hdHRyX3Nob3dfIyNuYW1lKHN0cnVjdCBkZXZp
Y2UgKmRldiwgwqAgwqAgwqAgwqAgwqAgwqAgwqBcCj4gKyDCoCDCoCDCoCDCoCDCoCDCoCDCoCDC
oCDCoCDCoCDCoCBzdHJ1Y3QgZGV2aWNlX2F0dHJpYnV0ZSAqYXR0ciwgwqAgwqAgwqAgwqAgwqAg
wqAgwqAgwqAgwqBcCj4gKyDCoCDCoCDCoCDCoCDCoCDCoCDCoCDCoCDCoCDCoCDCoCBjaGFyICpi
dWYpIMKgIMKgIMKgIMKgIMKgIMKgIMKgIMKgIMKgIMKgIMKgIMKgIMKgIMKgIMKgIMKgIMKgIMKg
IMKgXAo+ICt7IMKgIMKgIMKgIMKgIMKgIMKgIMKgIMKgIMKgIMKgIMKgIMKgIMKgIMKgIMKgIMKg
IMKgIMKgIMKgIMKgIMKgIMKgIMKgIMKgIMKgIMKgIMKgIMKgIMKgIMKgIMKgIMKgIMKgIMKgIMKg
XAo+ICsgwqAgwqAgwqAgc3RydWN0IGF0aDVrX3NvZnRjICpzYyA9IGRldl9nZXRfZHJ2ZGF0YShk
ZXYpOyDCoCDCoCDCoCDCoCDCoCDCoCDCoCDCoCDCoFwKPiArIMKgIMKgIMKgIHJldHVybiBzbnBy
aW50ZihidWYsIFBBR0VfU0laRSwgIiVkXG4iLCBnZXQpOyDCoCDCoCDCoCDCoCDCoCDCoCDCoCDC
oCDCoCBcCj4gK30gwqAgwqAgwqAgwqAgwqAgwqAgwqAgwqAgwqAgwqAgwqAgwqAgwqAgwqAgwqAg
wqAgwqAgwqAgwqAgwqAgwqAgwqAgwqAgwqAgwqAgwqAgwqAgwqAgwqAgwqAgwqAgwqAgwqAgwqAg
wqBcCj4gK3N0YXRpYyBERVZJQ0VfQVRUUihuYW1lLCBTX0lSVUdPLCBhdGg1a19hdHRyX3Nob3df
IyNuYW1lLCBOVUxMKQo+ICsKPiArLyoqKiBBTkkgKioqLwo+ICsKPiArU0lNUExFX1NIT1dfU1RP
UkUoYW5pX21vZGUsIHNjLT5hbmlfc3RhdGUuYW5pX21vZGUsIGF0aDVrX2FuaV9pbml0KTsKPiAr
U0lNUExFX1NIT1dfU1RPUkUobm9pc2VfaW1tdW5pdHlfbGV2ZWwsIHNjLT5hbmlfc3RhdGUubm9p
c2VfaW1tX2xldmVsLAo+ICsgwqAgwqAgwqAgwqAgwqAgwqAgwqAgwqAgwqAgwqAgwqAgYXRoNWtf
YW5pX3NldF9ub2lzZV9pbW11bml0eV9sZXZlbCk7Cj4gK1NJTVBMRV9TSE9XX1NUT1JFKHNwdXJf
bGV2ZWwsIHNjLT5hbmlfc3RhdGUuc3B1cl9sZXZlbCwKPiArIMKgIMKgIMKgIMKgIMKgIMKgIMKg
IMKgIMKgIMKgIMKgIGF0aDVrX2FuaV9zZXRfc3B1cl9pbW11bml0eV9sZXZlbCk7Cj4gK1NJTVBM
RV9TSE9XX1NUT1JFKGZpcnN0ZXBfbGV2ZWwsIHNjLT5hbmlfc3RhdGUuZmlyc3RlcF9sZXZlbCwK
PiArIMKgIMKgIMKgIMKgIMKgIMKgIMKgIMKgIMKgIMKgIMKgIGF0aDVrX2FuaV9zZXRfZmlyc3Rl
cF9sZXZlbCk7Cj4gK1NJTVBMRV9TSE9XX1NUT1JFKG9mZG1fd2Vha19zaWduYWxfZGV0ZWN0aW9u
LCBzYy0+YW5pX3N0YXRlLm9mZG1fd2Vha19zaWcsCj4gKyDCoCDCoCDCoCDCoCDCoCDCoCDCoCDC
oCDCoCDCoCDCoCBhdGg1a19hbmlfc2V0X29mZG1fd2Vha19zaWduYWxfZGV0ZWN0aW9uKTsKPiAr
U0lNUExFX1NIT1dfU1RPUkUoY2NrX3dlYWtfc2lnbmFsX2RldGVjdGlvbiwgc2MtPmFuaV9zdGF0
ZS5jY2tfd2Vha19zaWcsCj4gKyDCoCDCoCDCoCDCoCDCoCDCoCDCoCDCoCDCoCDCoCDCoCBhdGg1
a19hbmlfc2V0X2Nja193ZWFrX3NpZ25hbF9kZXRlY3Rpb24pOwo+ICtTSU1QTEVfU0hPVyhzcHVy
X2xldmVsX21heCwgc2MtPmFuaV9zdGF0ZS5tYXhfc3B1cl9sZXZlbCk7Cj4gKwo+ICtzdGF0aWMg
c3NpemVfdCBhdGg1a19hdHRyX3Nob3dfbm9pc2VfaW1tdW5pdHlfbGV2ZWxfbWF4KHN0cnVjdCBk
ZXZpY2UgKmRldiwKPiArIMKgIMKgIMKgIMKgIMKgIMKgIMKgIMKgIMKgIMKgIMKgIHN0cnVjdCBk
ZXZpY2VfYXR0cmlidXRlICphdHRyLAo+ICsgwqAgwqAgwqAgwqAgwqAgwqAgwqAgwqAgwqAgwqAg
wqAgY2hhciAqYnVmKQo+ICt7Cj4gKyDCoCDCoCDCoCByZXR1cm4gc25wcmludGYoYnVmLCBQQUdF
X1NJWkUsICIlZFxuIiwgQVRINUtfQU5JX01BWF9OT0lTRV9JTU1fTFZMKTsKPiArfQo+ICtzdGF0
aWMgREVWSUNFX0FUVFIobm9pc2VfaW1tdW5pdHlfbGV2ZWxfbWF4LCBTX0lSVUdPLAo+ICsgwqAg
wqAgwqAgwqAgwqAgwqAgwqAgwqAgwqBhdGg1a19hdHRyX3Nob3dfbm9pc2VfaW1tdW5pdHlfbGV2
ZWxfbWF4LCBOVUxMKTsKPiArCj4gK3N0YXRpYyBzc2l6ZV90IGF0aDVrX2F0dHJfc2hvd19maXJz
dGVwX2xldmVsX21heChzdHJ1Y3QgZGV2aWNlICpkZXYsCj4gKyDCoCDCoCDCoCDCoCDCoCDCoCDC
oCDCoCDCoCDCoCDCoCBzdHJ1Y3QgZGV2aWNlX2F0dHJpYnV0ZSAqYXR0ciwKPiArIMKgIMKgIMKg
IMKgIMKgIMKgIMKgIMKgIMKgIMKgIMKgIGNoYXIgKmJ1ZikKPiArewo+ICsgwqAgwqAgwqAgcmV0
dXJuIHNucHJpbnRmKGJ1ZiwgUEFHRV9TSVpFLCAiJWRcbiIsIEFUSDVLX0FOSV9NQVhfRklSU1RF
UF9MVkwpOwo+ICt9Cj4gK3N0YXRpYyBERVZJQ0VfQVRUUihmaXJzdGVwX2xldmVsX21heCwgU19J
UlVHTywKPiArIMKgIMKgIMKgIMKgIMKgIMKgIMKgIMKgIMKgYXRoNWtfYXR0cl9zaG93X2ZpcnN0
ZXBfbGV2ZWxfbWF4LCBOVUxMKTsKPiArCj4gK3N0YXRpYyBzdHJ1Y3QgYXR0cmlidXRlICphdGg1
a19zeXNmc19lbnRyaWVzX2FuaVtdID0gewo+ICsgwqAgwqAgwqAgJmRldl9hdHRyX2FuaV9tb2Rl
LmF0dHIsCj4gKyDCoCDCoCDCoCAmZGV2X2F0dHJfbm9pc2VfaW1tdW5pdHlfbGV2ZWwuYXR0ciwK
PiArIMKgIMKgIMKgICZkZXZfYXR0cl9zcHVyX2xldmVsLmF0dHIsCj4gKyDCoCDCoCDCoCAmZGV2
X2F0dHJfZmlyc3RlcF9sZXZlbC5hdHRyLAo+ICsgwqAgwqAgwqAgJmRldl9hdHRyX29mZG1fd2Vh
a19zaWduYWxfZGV0ZWN0aW9uLmF0dHIsCj4gKyDCoCDCoCDCoCAmZGV2X2F0dHJfY2NrX3dlYWtf
c2lnbmFsX2RldGVjdGlvbi5hdHRyLAo+ICsgwqAgwqAgwqAgJmRldl9hdHRyX25vaXNlX2ltbXVu
aXR5X2xldmVsX21heC5hdHRyLAo+ICsgwqAgwqAgwqAgJmRldl9hdHRyX3NwdXJfbGV2ZWxfbWF4
LmF0dHIsCj4gKyDCoCDCoCDCoCAmZGV2X2F0dHJfZmlyc3RlcF9sZXZlbF9tYXguYXR0ciwKPiAr
IMKgIMKgIMKgIE5VTEwKPiArfTsKPiArCj4gK3N0YXRpYyBzdHJ1Y3QgYXR0cmlidXRlX2dyb3Vw
IGF0aDVrX2F0dHJpYnV0ZV9ncm91cF9hbmkgPSB7Cj4gKyDCoCDCoCDCoCAubmFtZSA9ICJhbmki
LAo+ICsgwqAgwqAgwqAgLmF0dHJzID0gYXRoNWtfc3lzZnNfZW50cmllc19hbmksCj4gK307Cj4g
Kwo+ICsKPiArLyoqKiByZWdpc3RlciAvIHVucmVnaXN0ZXIgKioqLwo+ICsKPiAraW50Cj4gK2F0
aDVrX3N5c2ZzX3JlZ2lzdGVyKHN0cnVjdCBhdGg1a19zb2Z0YyAqc2MpCj4gK3sKPiArIMKgIMKg
IMKgIHN0cnVjdCBkZXZpY2UgKmRldiA9ICZzYy0+cGRldi0+ZGV2Owo+ICsgwqAgwqAgwqAgaW50
IGVycjsKPiArCj4gKyDCoCDCoCDCoCBlcnIgPSBzeXNmc19jcmVhdGVfZ3JvdXAoJmRldi0+a29i
aiwgJmF0aDVrX2F0dHJpYnV0ZV9ncm91cF9hbmkpOwo+ICsgwqAgwqAgwqAgaWYgKGVycikgewo+
ICsgwqAgwqAgwqAgwqAgwqAgwqAgwqAgQVRINUtfRVJSKHNjLCAiZmFpbGVkIHRvIGNyZWF0ZSBz
eXNmcyBncm91cFxuIik7Cj4gKyDCoCDCoCDCoCDCoCDCoCDCoCDCoCByZXR1cm4gZXJyOwo+ICsg
wqAgwqAgwqAgfQo+ICsKPiArIMKgIMKgIMKgIHJldHVybiAwOwo+ICt9Cj4gKwo+ICt2b2lkCj4g
K2F0aDVrX3N5c2ZzX3VucmVnaXN0ZXIoc3RydWN0IGF0aDVrX3NvZnRjICpzYykKPiArewo+ICsg
wqAgwqAgwqAgc3RydWN0IGRldmljZSAqZGV2ID0gJnNjLT5wZGV2LT5kZXY7Cj4gKwo+ICsgwqAg
wqAgwqAgc3lzZnNfcmVtb3ZlX2dyb3VwKCZkZXYtPmtvYmosICZhdGg1a19hdHRyaWJ1dGVfZ3Jv
dXBfYW5pKTsKPiArfQo+CgpBY2tlZC1ieTogTmljayBLb3NzaWZpZGlzIDxtaWNrZmxlbW1AZ21h
aWwuY29tPgoKCgotLSAKR1BHIElEOiAweEQyMURCMkRCCkFzIHlvdSByZWFkIHRoaXMgcG9zdCBn
bG9iYWwgZW50cm9weSByaXNlcy4gSGF2ZSBGdW4gOy0pCk5pY2sK

2010-05-21 01:59:47

by Bruno Randolf

[permalink] [raw]
Subject: Re: [ath5k-devel] [PATCH v2 13/20] cfg80211: Add nl80211 antenna configuration

On Friday 21 May 2010 07:05:48 Luis R. Rodriguez wrote:
> For legacy, keep it simple, use 3 settings, fixed_a, fixed_b,
> diversity, for all devices.

did you not understand my examples why i think it makes sense to use a bitmask
for "legacy"? i think they are perfectly valid use-cases. do i need to re-
iterate them a third time?

> Lets use a different API for 802.11n. Reason being that even the case
> I mentioned of an 802.11n device connecting on a legacy network needs
> to be treated differently actually.
>
> For 802.11n you have a few more considerations. You can actually TX at
> the same time on two or more different antennas at the same time. The
> data you transmit will be the contents on both chains on a dual stream
> device. So both antenna 0 and antenna 1 will both be transmitting the
> data for both stream 0 and stream 1. As it turns out the combination
> of TX'ing on two antennas at the same time at a certain dBm power will
> yield a higher received frame on the RX side. This is why when you use
> multiple chains you have to take regulatory rules into considerations
> as well, since adding more chains will increase the overall output
> power. Today ath9k handles this itself since this data is calibrated
> but the max EIRP is passed out from cfg80211. Devices which do not
> deal with these regulatory considerations likely won't support
> changing chainmasks unless they use an API to respect regulatory
> internally somehow. Perhaps the iwlagn firmware does this, beats me.
>
> The right terminology for antenna control for both TX and RX is
> chainmask and a bitmap of 8 will suffice for existing hardware and up
> to the not-yet-existant 600mbps 4 stream devices. Supporting 8 bits
> will support up to 8 streams and we do not envision using beyond that
> at this point. There is some considerations in the future for
> supporting something other than HT40, like maybe HT80 and so forth but
> those things won't be using more streams it seems.
>
> Then, some devices won't support all possible chainmask settings. This
> will vary depending on the chipset. I work for Atheros so I can only
> tell you what we can support, we'll have to check with the Intel folks
> about their chipset limitations and settings.
>
> AR5416, AR5418 can only support chainmask settings which always keep
> the first chain on. The AR9001 family and beyond cannot support the
> 0b110 chaimask (David, you had pointed out some other restrictions,
> what were they again?), the details are complex and I did not get a
> chance to review them.
>
> I would not be surprised if other vendors had similar restrictions so
> I'm thinking maybe we can express this as a requirement mask, or a set
> of requirement masks. This way userspace utilities for debugging would
> only expose certain chainmask settings.
>
> Now technically then you can incorporate the legacy API with the
> 802.11n API here somehow but it just seems cleaner to keep them
> separate.
>
> Also, David indicated that when we change the chainmask when are are
> associated we have to do an actual chip reset, this is different than
> the antenna diversity settings which an be done on the fly. We likely
> will need to reassociate for a chainmask setting, not sure.

so from my point of view this is not very different from what we can support
with the API i suggested. for RX it seems to be 100% equivalent.

the main difference as i see it is that with 802.11n you transmit on more than
one antenna, while with 'legacy' we can only transmit on one antenna at a
time.

actually i have to admit that on legacy "antenna set tx 3 (b11)" (select two
antennas for transmit) does not make much sense. i have defined it before as
"use diversity" but what about a different definition: like "bitmap of
antennas/chains to TRANSMIT". so for 802.11n that would allow you to select
multiple trasmit chains. on legacy you are only allowed to select one antenna
in the bitmap. if it is set to "0" (or a separate flag) this could enable
"follow RX antenna diversity" on legacy.

most of the other things you mention (need a reset/reassociate, regulatory
concerns...) are driver implementation issues, which can be dealt with in the
driver. i would just suggest to let the driver reject antenna/chainmask
configurations which it cannot support.

bruno

2010-05-20 12:54:24

by Nick Kossifidis

[permalink] [raw]
Subject: Re: [PATCH v2 18/20] ath5k: update AR5K_PHY_RESTART_DIV_GC values to match masks

2010/5/19 Bruno Randolf <[email protected]>:
> #define AR5K_PHY_RESTART_DIV_GC               0x001c0000
> is 3 bit wide.
>
> The previous values of 0xc and 0x8 are 4bit wide and bigger than the mask.
>
> Writing 0 and 1 to AR5K_PHY_RESTART_DIV_GC is consistent with the comments and
> initvals we have in the HAL.
>
> Signed-off-by: Bruno Randolf <[email protected]>
> ---
>  drivers/net/wireless/ath/ath5k/phy.c |    4 ++--
>  1 files changed, 2 insertions(+), 2 deletions(-)
>
> diff --git a/drivers/net/wireless/ath/ath5k/phy.c b/drivers/net/wireless/ath/ath5k/phy.c
> index d9506e7..2136930 100644
> --- a/drivers/net/wireless/ath/ath5k/phy.c
> +++ b/drivers/net/wireless/ath/ath5k/phy.c
> @@ -1768,13 +1768,13 @@ ath5k_hw_set_fast_div(struct ath5k_hw *ah, u8 ee_mode, bool enable)
>
>        if (enable) {
>                AR5K_REG_WRITE_BITS(ah, AR5K_PHY_RESTART,
> -                               AR5K_PHY_RESTART_DIV_GC, 0xc);
> +                               AR5K_PHY_RESTART_DIV_GC, 1);
>
>                AR5K_REG_ENABLE_BITS(ah, AR5K_PHY_FAST_ANT_DIV,
>                                        AR5K_PHY_FAST_ANT_DIV_EN);
>        } else {
>                AR5K_REG_WRITE_BITS(ah, AR5K_PHY_RESTART,
> -                               AR5K_PHY_RESTART_DIV_GC, 0x8);
> +                               AR5K_PHY_RESTART_DIV_GC, 0);
>
>                AR5K_REG_DISABLE_BITS(ah, AR5K_PHY_FAST_ANT_DIV,
>                                        AR5K_PHY_FAST_ANT_DIV_EN);
>

Acked-by: Nick Kossifidis <[email protected]>


--
GPG ID: 0xD21DB2DB
As you read this post global entropy rises. Have Fun ;-)
Nick

2010-05-19 01:33:32

by Bruno Randolf

[permalink] [raw]
Subject: [PATCH v2 19/20] ath5k: new function for setting the antenna switch table

Collect all pieces concering the antenna switch table into one function.
Previously it was split up between ath5k_hw_reset() and
ath5k_hw_commit_eeprom_settings().

Also we need to set the antenna switch table when ath5k_hw_set_antenna_mode()
is called manually (by "iw phy0 antenna set", for example).

I'm not sure if we need to set the switchtable at the same place in
ath5k_hw_reset() as it was before - it is set later thru
ath5k_hw_set_antenna_mode() anyways - but i leave it there to avoid
problems(?).

Plus print switchtable registers in the debugfs file.

Signed-off-by: Bruno Randolf <[email protected]>
---
drivers/net/wireless/ath/ath5k/ath5k.h | 1 +
drivers/net/wireless/ath/ath5k/debug.c | 7 +++++++
drivers/net/wireless/ath/ath5k/phy.c | 32 +++++++++++++++++++++++++++++++
drivers/net/wireless/ath/ath5k/reset.c | 33 ++++++--------------------------
4 files changed, 46 insertions(+), 27 deletions(-)

diff --git a/drivers/net/wireless/ath/ath5k/ath5k.h b/drivers/net/wireless/ath/ath5k/ath5k.h
index eace74d..cf16318 100644
--- a/drivers/net/wireless/ath/ath5k/ath5k.h
+++ b/drivers/net/wireless/ath/ath5k/ath5k.h
@@ -1286,6 +1286,7 @@ u16 ath5k_hw_radio_revision(struct ath5k_hw *ah, unsigned int chan);
int ath5k_hw_phy_disable(struct ath5k_hw *ah);
/* Antenna control */
void ath5k_hw_set_antenna_mode(struct ath5k_hw *ah, u8 ant_mode);
+void ath5k_hw_set_antenna_switch(struct ath5k_hw *ah, u8 ee_mode);
/* TX power setup */
int ath5k_hw_txpower(struct ath5k_hw *ah, struct ieee80211_channel *channel,
u8 ee_mode, u8 txpower);
diff --git a/drivers/net/wireless/ath/ath5k/debug.c b/drivers/net/wireless/ath/ath5k/debug.c
index d178d63..f3612e5 100644
--- a/drivers/net/wireless/ath/ath5k/debug.c
+++ b/drivers/net/wireless/ath/ath5k/debug.c
@@ -425,6 +425,13 @@ static ssize_t read_file_antenna(struct file *file, char __user *user_buf,
"AR5K_PHY_FAST_ANT_DIV_EN\t%d\n",
(v & AR5K_PHY_FAST_ANT_DIV_EN) != 0);

+ v = ath5k_hw_reg_read(sc->ah, AR5K_PHY_ANT_SWITCH_TABLE_0);
+ len += snprintf(buf+len, sizeof(buf)-len,
+ "\nAR5K_PHY_ANT_SWITCH_TABLE_0\t0x%08x\n", v);
+ v = ath5k_hw_reg_read(sc->ah, AR5K_PHY_ANT_SWITCH_TABLE_1);
+ len += snprintf(buf+len, sizeof(buf)-len,
+ "AR5K_PHY_ANT_SWITCH_TABLE_1\t0x%08x\n", v);
+
return simple_read_from_buffer(user_buf, count, ppos, buf, len);
}

diff --git a/drivers/net/wireless/ath/ath5k/phy.c b/drivers/net/wireless/ath/ath5k/phy.c
index 2136930..82720fc 100644
--- a/drivers/net/wireless/ath/ath5k/phy.c
+++ b/drivers/net/wireless/ath/ath5k/phy.c
@@ -1781,6 +1781,37 @@ ath5k_hw_set_fast_div(struct ath5k_hw *ah, u8 ee_mode, bool enable)
}
}

+void
+ath5k_hw_set_antenna_switch(struct ath5k_hw *ah, u8 ee_mode)
+{
+ u8 ant0, ant1;
+
+ /*
+ * In case a fixed antenna was set as default
+ * use the same switch table twice.
+ */
+ if (ah->ah_ant_mode == AR5K_ANTMODE_FIXED_A)
+ ant0 = ant1 = AR5K_ANT_SWTABLE_A;
+ else if (ah->ah_ant_mode == AR5K_ANTMODE_FIXED_B)
+ ant0 = ant1 = AR5K_ANT_SWTABLE_B;
+ else {
+ ant0 = AR5K_ANT_SWTABLE_A;
+ ant1 = AR5K_ANT_SWTABLE_B;
+ }
+
+ /* Set antenna idle switch table */
+ AR5K_REG_WRITE_BITS(ah, AR5K_PHY_ANT_CTL,
+ AR5K_PHY_ANT_CTL_SWTABLE_IDLE,
+ (ah->ah_ant_ctl[ee_mode][AR5K_ANT_CTL] |
+ AR5K_PHY_ANT_CTL_TXRX_EN));
+
+ /* Set antenna switch tables */
+ ath5k_hw_reg_write(ah, ah->ah_ant_ctl[ee_mode][ant0],
+ AR5K_PHY_ANT_SWITCH_TABLE_0);
+ ath5k_hw_reg_write(ah, ah->ah_ant_ctl[ee_mode][ant1],
+ AR5K_PHY_ANT_SWITCH_TABLE_1);
+}
+
/*
* Set antenna operating mode
*/
@@ -1900,6 +1931,7 @@ ath5k_hw_set_antenna_mode(struct ath5k_hw *ah, u8 ant_mode)
if (sta_id1)
AR5K_REG_ENABLE_BITS(ah, AR5K_STA_ID1, sta_id1);

+ ath5k_hw_set_antenna_switch(ah, ee_mode);
/* Note: set diversity before default antenna
* because it won't work correctly */
ath5k_hw_set_fast_div(ah, ee_mode, fast_div);
diff --git a/drivers/net/wireless/ath/ath5k/reset.c b/drivers/net/wireless/ath/ath5k/reset.c
index b1d3239..cf8fd48 100644
--- a/drivers/net/wireless/ath/ath5k/reset.c
+++ b/drivers/net/wireless/ath/ath5k/reset.c
@@ -730,7 +730,7 @@ static void ath5k_hw_tweak_initval_settings(struct ath5k_hw *ah,
}

static void ath5k_hw_commit_eeprom_settings(struct ath5k_hw *ah,
- struct ieee80211_channel *channel, u8 *ant, u8 ee_mode)
+ struct ieee80211_channel *channel, u8 ee_mode)
{
struct ath5k_eeprom_info *ee = &ah->ah_capabilities.cap_eeprom;
s16 cck_ofdm_pwr_delta;
@@ -764,17 +764,9 @@ static void ath5k_hw_commit_eeprom_settings(struct ath5k_hw *ah,
ee->ee_cck_ofdm_gain_delta;
}

- /* Set antenna idle switch table */
- AR5K_REG_WRITE_BITS(ah, AR5K_PHY_ANT_CTL,
- AR5K_PHY_ANT_CTL_SWTABLE_IDLE,
- (ah->ah_ant_ctl[ee_mode][0] |
- AR5K_PHY_ANT_CTL_TXRX_EN));
-
- /* Set antenna switch tables */
- ath5k_hw_reg_write(ah, ah->ah_ant_ctl[ee_mode][ant[0]],
- AR5K_PHY_ANT_SWITCH_TABLE_0);
- ath5k_hw_reg_write(ah, ah->ah_ant_ctl[ee_mode][ant[1]],
- AR5K_PHY_ANT_SWITCH_TABLE_1);
+ /* XXX: necessary here? is called from ath5k_hw_set_antenna_mode()
+ * too */
+ ath5k_hw_set_antenna_switch(ah, ee_mode);

/* Noise floor threshold */
ath5k_hw_reg_write(ah,
@@ -890,7 +882,7 @@ int ath5k_hw_reset(struct ath5k_hw *ah, enum nl80211_iftype op_mode,
struct ath_common *common = ath5k_hw_common(ah);
u32 s_seq[10], s_ant, s_led[3], staid1_flags, tsf_up, tsf_lo;
u32 phy_tst1;
- u8 mode, freq, ee_mode, ant[2];
+ u8 mode, freq, ee_mode;
int i, ret;

s_ant = 0;
@@ -1113,21 +1105,8 @@ int ath5k_hw_reset(struct ath5k_hw *ah, enum nl80211_iftype op_mode,
AR5K_TXCFG_B_MODE);
}

- /*
- * In case a fixed antenna was set as default
- * use the same switch table twice.
- */
- if (ah->ah_ant_mode == AR5K_ANTMODE_FIXED_A)
- ant[0] = ant[1] = AR5K_ANT_SWTABLE_A;
- else if (ah->ah_ant_mode == AR5K_ANTMODE_FIXED_B)
- ant[0] = ant[1] = AR5K_ANT_SWTABLE_B;
- else {
- ant[0] = AR5K_ANT_SWTABLE_A;
- ant[1] = AR5K_ANT_SWTABLE_B;
- }
-
/* Commit values from EEPROM */
- ath5k_hw_commit_eeprom_settings(ah, channel, ant, ee_mode);
+ ath5k_hw_commit_eeprom_settings(ah, channel, ee_mode);

} else {
/*


2010-05-19 01:32:18

by Bruno Randolf

[permalink] [raw]
Subject: [PATCH v2 05/20] ath5k: Stop queues only for NF calibration

As far as we know, only NF calibration interferes with RX/TX so we can
leave the queues enabled for the other calibrations.

BTW: Stopping the queues is not enough for avoiding transmissions, since there
might be packets in the queue + beacons are also sent regularly! But i leave it
like this until we have a better solution (stopping TX DMA?).

Signed-off-by: Bruno Randolf <[email protected]>
---
drivers/net/wireless/ath/ath5k/base.c | 11 ++++++-----
1 files changed, 6 insertions(+), 5 deletions(-)

diff --git a/drivers/net/wireless/ath/ath5k/base.c b/drivers/net/wireless/ath/ath5k/base.c
index 88c7314..3206ed6 100644
--- a/drivers/net/wireless/ath/ath5k/base.c
+++ b/drivers/net/wireless/ath/ath5k/base.c
@@ -2785,10 +2785,6 @@ ath5k_tasklet_calibrate(unsigned long data)
/* Only full calibration for now */
ah->ah_cal_mask |= AR5K_CALIBRATION_FULL;

- /* Stop queues so that calibration
- * doesn't interfere with tx */
- ieee80211_stop_queues(sc->hw);
-
ATH5K_DBG(sc, ATH5K_DEBUG_CALIBRATE, "channel %u/%x\n",
ieee80211_frequency_to_channel(sc->curchan->center_freq),
sc->curchan->hw_value);
@@ -2806,8 +2802,13 @@ ath5k_tasklet_calibrate(unsigned long data)
ieee80211_frequency_to_channel(
sc->curchan->center_freq));

+ /* TODO: We don't need to run noise floor calibration as often
+ * as I/Q calibration.*/
+
+ /* Noise floor calibration interrupts rx/tx path while I/Q calibration
+ * doesn't. Stop queues so that calibration doesn't interfere with tx */
+ ieee80211_stop_queues(sc->hw);
ath5k_hw_update_noise_floor(ah);
- /* Wake queues */
ieee80211_wake_queues(sc->hw);

ah->ah_cal_mask &= ~AR5K_CALIBRATION_FULL;


2010-05-19 01:32:30

by Bruno Randolf

[permalink] [raw]
Subject: [PATCH v2 07/20] ath5k: remove ATH_TRACE macro

Now that we have ftrace, it is not needed any more.

Signed-off-by: Bruno Randolf <[email protected]>
---
drivers/net/wireless/ath/ath5k/attach.c | 2 --
drivers/net/wireless/ath/ath5k/caps.c | 7 -------
drivers/net/wireless/ath/ath5k/debug.c | 1 -
drivers/net/wireless/ath/ath5k/debug.h | 8 --------
drivers/net/wireless/ath/ath5k/desc.c | 7 -------
drivers/net/wireless/ath/ath5k/dma.c | 13 -------------
drivers/net/wireless/ath/ath5k/eeprom.c | 1 -
drivers/net/wireless/ath/ath5k/gpio.c | 7 -------
drivers/net/wireless/ath/ath5k/pcu.c | 24 ------------------------
drivers/net/wireless/ath/ath5k/phy.c | 13 -------------
drivers/net/wireless/ath/ath5k/qcu.c | 9 ---------
drivers/net/wireless/ath/ath5k/reset.c | 7 -------
12 files changed, 0 insertions(+), 99 deletions(-)

diff --git a/drivers/net/wireless/ath/ath5k/attach.c b/drivers/net/wireless/ath/ath5k/attach.c
index e0c244b..ef2dc1d 100644
--- a/drivers/net/wireless/ath/ath5k/attach.c
+++ b/drivers/net/wireless/ath/ath5k/attach.c
@@ -351,8 +351,6 @@ err_free:
*/
void ath5k_hw_detach(struct ath5k_hw *ah)
{
- ATH5K_TRACE(ah->ah_sc);
-
__set_bit(ATH_STAT_INVALID, ah->ah_sc->status);

if (ah->ah_rf_banks != NULL)
diff --git a/drivers/net/wireless/ath/ath5k/caps.c b/drivers/net/wireless/ath/ath5k/caps.c
index 74f0071..beae519 100644
--- a/drivers/net/wireless/ath/ath5k/caps.c
+++ b/drivers/net/wireless/ath/ath5k/caps.c
@@ -34,7 +34,6 @@ int ath5k_hw_set_capabilities(struct ath5k_hw *ah)
{
u16 ee_header;

- ATH5K_TRACE(ah->ah_sc);
/* Capabilities stored in the EEPROM */
ee_header = ah->ah_capabilities.cap_eeprom.ee_header;

@@ -123,8 +122,6 @@ int ath5k_hw_get_capability(struct ath5k_hw *ah,
enum ath5k_capability_type cap_type,
u32 capability, u32 *result)
{
- ATH5K_TRACE(ah->ah_sc);
-
switch (cap_type) {
case AR5K_CAP_NUM_TXQUEUES:
if (result) {
@@ -173,8 +170,6 @@ yes:
int ath5k_hw_enable_pspoll(struct ath5k_hw *ah, u8 *bssid,
u16 assoc_id)
{
- ATH5K_TRACE(ah->ah_sc);
-
if (ah->ah_version == AR5K_AR5210) {
AR5K_REG_DISABLE_BITS(ah, AR5K_STA_ID1,
AR5K_STA_ID1_NO_PSPOLL | AR5K_STA_ID1_DEFAULT_ANTENNA);
@@ -186,8 +181,6 @@ int ath5k_hw_enable_pspoll(struct ath5k_hw *ah, u8 *bssid,

int ath5k_hw_disable_pspoll(struct ath5k_hw *ah)
{
- ATH5K_TRACE(ah->ah_sc);
-
if (ah->ah_version == AR5K_AR5210) {
AR5K_REG_ENABLE_BITS(ah, AR5K_STA_ID1,
AR5K_STA_ID1_NO_PSPOLL | AR5K_STA_ID1_DEFAULT_ANTENNA);
diff --git a/drivers/net/wireless/ath/ath5k/debug.c b/drivers/net/wireless/ath/ath5k/debug.c
index c77a6ad..97c5ada 100644
--- a/drivers/net/wireless/ath/ath5k/debug.c
+++ b/drivers/net/wireless/ath/ath5k/debug.c
@@ -307,7 +307,6 @@ static const struct {
{ ATH5K_DEBUG_DUMP_RX, "dumprx", "print received skb content" },
{ ATH5K_DEBUG_DUMP_TX, "dumptx", "print transmit skb content" },
{ ATH5K_DEBUG_DUMPBANDS, "dumpbands", "dump bands" },
- { ATH5K_DEBUG_TRACE, "trace", "trace function calls" },
{ ATH5K_DEBUG_ANI, "ani", "adaptive noise immunity" },
{ ATH5K_DEBUG_ANY, "all", "show all debug levels" },
};
diff --git a/drivers/net/wireless/ath/ath5k/debug.h b/drivers/net/wireless/ath/ath5k/debug.h
index c7a0769..606ae94 100644
--- a/drivers/net/wireless/ath/ath5k/debug.h
+++ b/drivers/net/wireless/ath/ath5k/debug.h
@@ -116,18 +116,12 @@ enum ath5k_debug_level {
ATH5K_DEBUG_DUMP_RX = 0x00000100,
ATH5K_DEBUG_DUMP_TX = 0x00000200,
ATH5K_DEBUG_DUMPBANDS = 0x00000400,
- ATH5K_DEBUG_TRACE = 0x00001000,
ATH5K_DEBUG_ANI = 0x00002000,
ATH5K_DEBUG_ANY = 0xffffffff
};

#ifdef CONFIG_ATH5K_DEBUG

-#define ATH5K_TRACE(_sc) do { \
- if (unlikely((_sc)->debug.level & ATH5K_DEBUG_TRACE)) \
- printk(KERN_DEBUG "ath5k trace %s:%d\n", __func__, __LINE__); \
- } while (0)
-
#define ATH5K_DBG(_sc, _m, _fmt, ...) do { \
if (unlikely((_sc)->debug.level & (_m) && net_ratelimit())) \
ATH5K_PRINTK(_sc, KERN_DEBUG, "(%s:%d): " _fmt, \
@@ -169,8 +163,6 @@ ath5k_debug_printtxbuf(struct ath5k_softc *sc, struct ath5k_buf *bf);

#include <linux/compiler.h>

-#define ATH5K_TRACE(_sc) typecheck(struct ath5k_softc *, (_sc))
-
static inline void __attribute__ ((format (printf, 3, 4)))
ATH5K_DBG(struct ath5k_softc *sc, unsigned int m, const char *fmt, ...) {}

diff --git a/drivers/net/wireless/ath/ath5k/desc.c b/drivers/net/wireless/ath/ath5k/desc.c
index 7d7b646..da5dbb6 100644
--- a/drivers/net/wireless/ath/ath5k/desc.c
+++ b/drivers/net/wireless/ath/ath5k/desc.c
@@ -176,7 +176,6 @@ static int ath5k_hw_setup_4word_tx_desc(struct ath5k_hw *ah,
struct ath5k_hw_4w_tx_ctl *tx_ctl;
unsigned int frame_len;

- ATH5K_TRACE(ah->ah_sc);
tx_ctl = &desc->ud.ds_tx5212.tx_ctl;

/*
@@ -342,8 +341,6 @@ static int ath5k_hw_proc_2word_tx_status(struct ath5k_hw *ah,
struct ath5k_hw_2w_tx_ctl *tx_ctl;
struct ath5k_hw_tx_status *tx_status;

- ATH5K_TRACE(ah->ah_sc);
-
tx_ctl = &desc->ud.ds_tx5210.tx_ctl;
tx_status = &desc->ud.ds_tx5210.tx_stat;

@@ -396,8 +393,6 @@ static int ath5k_hw_proc_4word_tx_status(struct ath5k_hw *ah,
struct ath5k_hw_4w_tx_ctl *tx_ctl;
struct ath5k_hw_tx_status *tx_status;

- ATH5K_TRACE(ah->ah_sc);
-
tx_ctl = &desc->ud.ds_tx5212.tx_ctl;
tx_status = &desc->ud.ds_tx5212.tx_stat;

@@ -490,7 +485,6 @@ static int ath5k_hw_setup_rx_desc(struct ath5k_hw *ah, struct ath5k_desc *desc,
{
struct ath5k_hw_rx_ctl *rx_ctl;

- ATH5K_TRACE(ah->ah_sc);
rx_ctl = &desc->ud.ds_rx.rx_ctl;

/*
@@ -593,7 +587,6 @@ static int ath5k_hw_proc_5212_rx_status(struct ath5k_hw *ah,
struct ath5k_hw_rx_status *rx_status;
struct ath5k_hw_rx_error *rx_err;

- ATH5K_TRACE(ah->ah_sc);
rx_status = &desc->ud.ds_rx.u.rx_stat;

/* Overlay on error */
diff --git a/drivers/net/wireless/ath/ath5k/dma.c b/drivers/net/wireless/ath/ath5k/dma.c
index 941b511..484f318 100644
--- a/drivers/net/wireless/ath/ath5k/dma.c
+++ b/drivers/net/wireless/ath/ath5k/dma.c
@@ -48,7 +48,6 @@
*/
void ath5k_hw_start_rx_dma(struct ath5k_hw *ah)
{
- ATH5K_TRACE(ah->ah_sc);
ath5k_hw_reg_write(ah, AR5K_CR_RXE, AR5K_CR);
ath5k_hw_reg_read(ah, AR5K_CR);
}
@@ -62,7 +61,6 @@ int ath5k_hw_stop_rx_dma(struct ath5k_hw *ah)
{
unsigned int i;

- ATH5K_TRACE(ah->ah_sc);
ath5k_hw_reg_write(ah, AR5K_CR_RXD, AR5K_CR);

/*
@@ -96,8 +94,6 @@ u32 ath5k_hw_get_rxdp(struct ath5k_hw *ah)
*/
void ath5k_hw_set_rxdp(struct ath5k_hw *ah, u32 phys_addr)
{
- ATH5K_TRACE(ah->ah_sc);
-
ath5k_hw_reg_write(ah, phys_addr, AR5K_RXDP);
}

@@ -125,7 +121,6 @@ int ath5k_hw_start_tx_dma(struct ath5k_hw *ah, unsigned int queue)
{
u32 tx_queue;

- ATH5K_TRACE(ah->ah_sc);
AR5K_ASSERT_ENTRY(queue, ah->ah_capabilities.cap_queues.q_tx_num);

/* Return if queue is declared inactive */
@@ -186,7 +181,6 @@ int ath5k_hw_stop_tx_dma(struct ath5k_hw *ah, unsigned int queue)
unsigned int i = 40;
u32 tx_queue, pending;

- ATH5K_TRACE(ah->ah_sc);
AR5K_ASSERT_ENTRY(queue, ah->ah_capabilities.cap_queues.q_tx_num);

/* Return if queue is declared inactive */
@@ -297,7 +291,6 @@ u32 ath5k_hw_get_txdp(struct ath5k_hw *ah, unsigned int queue)
{
u16 tx_reg;

- ATH5K_TRACE(ah->ah_sc);
AR5K_ASSERT_ENTRY(queue, ah->ah_capabilities.cap_queues.q_tx_num);

/*
@@ -340,7 +333,6 @@ int ath5k_hw_set_txdp(struct ath5k_hw *ah, unsigned int queue, u32 phys_addr)
{
u16 tx_reg;

- ATH5K_TRACE(ah->ah_sc);
AR5K_ASSERT_ENTRY(queue, ah->ah_capabilities.cap_queues.q_tx_num);

/*
@@ -400,8 +392,6 @@ int ath5k_hw_update_tx_triglevel(struct ath5k_hw *ah, bool increase)
u32 trigger_level, imr;
int ret = -EIO;

- ATH5K_TRACE(ah->ah_sc);
-
/*
* Disable interrupts by setting the mask
*/
@@ -451,7 +441,6 @@ done:
*/
bool ath5k_hw_is_intr_pending(struct ath5k_hw *ah)
{
- ATH5K_TRACE(ah->ah_sc);
return ath5k_hw_reg_read(ah, AR5K_INTPEND) == 1 ? 1 : 0;
}

@@ -475,8 +464,6 @@ int ath5k_hw_get_isr(struct ath5k_hw *ah, enum ath5k_int *interrupt_mask)
{
u32 data;

- ATH5K_TRACE(ah->ah_sc);
-
/*
* Read interrupt status from the Interrupt Status register
* on 5210
diff --git a/drivers/net/wireless/ath/ath5k/eeprom.c b/drivers/net/wireless/ath/ath5k/eeprom.c
index ed02636..155a82c 100644
--- a/drivers/net/wireless/ath/ath5k/eeprom.c
+++ b/drivers/net/wireless/ath/ath5k/eeprom.c
@@ -35,7 +35,6 @@ static int ath5k_hw_eeprom_read(struct ath5k_hw *ah, u32 offset, u16 *data)
{
u32 status, timeout;

- ATH5K_TRACE(ah->ah_sc);
/*
* Initialize EEPROM access
*/
diff --git a/drivers/net/wireless/ath/ath5k/gpio.c b/drivers/net/wireless/ath/ath5k/gpio.c
index 64a27e7..bc90503 100644
--- a/drivers/net/wireless/ath/ath5k/gpio.c
+++ b/drivers/net/wireless/ath/ath5k/gpio.c
@@ -34,8 +34,6 @@ void ath5k_hw_set_ledstate(struct ath5k_hw *ah, unsigned int state)
/*5210 has different led mode handling*/
u32 led_5210;

- ATH5K_TRACE(ah->ah_sc);
-
/*Reset led status*/
if (ah->ah_version != AR5K_AR5210)
AR5K_REG_DISABLE_BITS(ah, AR5K_PCICFG,
@@ -82,7 +80,6 @@ void ath5k_hw_set_ledstate(struct ath5k_hw *ah, unsigned int state)
*/
int ath5k_hw_set_gpio_input(struct ath5k_hw *ah, u32 gpio)
{
- ATH5K_TRACE(ah->ah_sc);
if (gpio >= AR5K_NUM_GPIO)
return -EINVAL;

@@ -98,7 +95,6 @@ int ath5k_hw_set_gpio_input(struct ath5k_hw *ah, u32 gpio)
*/
int ath5k_hw_set_gpio_output(struct ath5k_hw *ah, u32 gpio)
{
- ATH5K_TRACE(ah->ah_sc);
if (gpio >= AR5K_NUM_GPIO)
return -EINVAL;

@@ -114,7 +110,6 @@ int ath5k_hw_set_gpio_output(struct ath5k_hw *ah, u32 gpio)
*/
u32 ath5k_hw_get_gpio(struct ath5k_hw *ah, u32 gpio)
{
- ATH5K_TRACE(ah->ah_sc);
if (gpio >= AR5K_NUM_GPIO)
return 0xffffffff;

@@ -129,7 +124,6 @@ u32 ath5k_hw_get_gpio(struct ath5k_hw *ah, u32 gpio)
int ath5k_hw_set_gpio(struct ath5k_hw *ah, u32 gpio, u32 val)
{
u32 data;
- ATH5K_TRACE(ah->ah_sc);

if (gpio >= AR5K_NUM_GPIO)
return -EINVAL;
@@ -153,7 +147,6 @@ void ath5k_hw_set_gpio_intr(struct ath5k_hw *ah, unsigned int gpio,
{
u32 data;

- ATH5K_TRACE(ah->ah_sc);
if (gpio >= AR5K_NUM_GPIO)
return;

diff --git a/drivers/net/wireless/ath/ath5k/pcu.c b/drivers/net/wireless/ath/ath5k/pcu.c
index 5212e27..86fdb6d 100644
--- a/drivers/net/wireless/ath/ath5k/pcu.c
+++ b/drivers/net/wireless/ath/ath5k/pcu.c
@@ -59,8 +59,6 @@ int ath5k_hw_set_opmode(struct ath5k_hw *ah, enum nl80211_iftype op_mode)

beacon_reg = 0;

- ATH5K_TRACE(ah->ah_sc);
-
switch (op_mode) {
case NL80211_IFTYPE_ADHOC:
pcu_reg |= AR5K_STA_ID1_ADHOC | AR5K_STA_ID1_KEYSRCH_MODE;
@@ -173,7 +171,6 @@ void ath5k_hw_set_ack_bitrate_high(struct ath5k_hw *ah, bool high)
*/
static int ath5k_hw_set_ack_timeout(struct ath5k_hw *ah, unsigned int timeout)
{
- ATH5K_TRACE(ah->ah_sc);
if (ath5k_hw_clocktoh(ah, AR5K_REG_MS(0xffffffff, AR5K_TIME_OUT_ACK))
<= timeout)
return -EINVAL;
@@ -192,7 +189,6 @@ static int ath5k_hw_set_ack_timeout(struct ath5k_hw *ah, unsigned int timeout)
*/
static int ath5k_hw_set_cts_timeout(struct ath5k_hw *ah, unsigned int timeout)
{
- ATH5K_TRACE(ah->ah_sc);
if (ath5k_hw_clocktoh(ah, AR5K_REG_MS(0xffffffff, AR5K_TIME_OUT_CTS))
<= timeout)
return -EINVAL;
@@ -297,7 +293,6 @@ int ath5k_hw_set_lladdr(struct ath5k_hw *ah, const u8 *mac)
u32 low_id, high_id;
u32 pcu_reg;

- ATH5K_TRACE(ah->ah_sc);
/* Set new station ID */
memcpy(common->macaddr, mac, ETH_ALEN);

@@ -357,7 +352,6 @@ void ath5k_hw_set_associd(struct ath5k_hw *ah)
void ath5k_hw_set_bssid_mask(struct ath5k_hw *ah, const u8 *mask)
{
struct ath_common *common = ath5k_hw_common(ah);
- ATH5K_TRACE(ah->ah_sc);

/* Cache bssid mask so that we can restore it
* on reset */
@@ -382,7 +376,6 @@ void ath5k_hw_set_bssid_mask(struct ath5k_hw *ah, const u8 *mask)
*/
void ath5k_hw_start_rx_pcu(struct ath5k_hw *ah)
{
- ATH5K_TRACE(ah->ah_sc);
AR5K_REG_DISABLE_BITS(ah, AR5K_DIAG_SW, AR5K_DIAG_SW_DIS_RX);
}

@@ -397,7 +390,6 @@ void ath5k_hw_start_rx_pcu(struct ath5k_hw *ah)
*/
void ath5k_hw_stop_rx_pcu(struct ath5k_hw *ah)
{
- ATH5K_TRACE(ah->ah_sc);
AR5K_REG_ENABLE_BITS(ah, AR5K_DIAG_SW, AR5K_DIAG_SW_DIS_RX);
}

@@ -406,8 +398,6 @@ void ath5k_hw_stop_rx_pcu(struct ath5k_hw *ah)
*/
void ath5k_hw_set_mcast_filter(struct ath5k_hw *ah, u32 filter0, u32 filter1)
{
- ATH5K_TRACE(ah->ah_sc);
- /* Set the multicat filter */
ath5k_hw_reg_write(ah, filter0, AR5K_MCAST_FILTER0);
ath5k_hw_reg_write(ah, filter1, AR5K_MCAST_FILTER1);
}
@@ -427,7 +417,6 @@ u32 ath5k_hw_get_rx_filter(struct ath5k_hw *ah)
{
u32 data, filter = 0;

- ATH5K_TRACE(ah->ah_sc);
filter = ath5k_hw_reg_read(ah, AR5K_RX_FILTER);

/*Radar detection for 5212*/
@@ -457,8 +446,6 @@ void ath5k_hw_set_rx_filter(struct ath5k_hw *ah, u32 filter)
{
u32 data = 0;

- ATH5K_TRACE(ah->ah_sc);
-
/* Set PHY error filter register on 5212*/
if (ah->ah_version == AR5K_AR5212) {
if (filter & AR5K_RX_FILTER_RADARERR)
@@ -533,8 +520,6 @@ u64 ath5k_hw_get_tsf64(struct ath5k_hw *ah)

WARN_ON( i == ATH5K_MAX_TSF_READ );

- ATH5K_TRACE(ah->ah_sc);
-
return (((u64)tsf_upper1 << 32) | tsf_lower);
}

@@ -548,8 +533,6 @@ u64 ath5k_hw_get_tsf64(struct ath5k_hw *ah)
*/
void ath5k_hw_set_tsf64(struct ath5k_hw *ah, u64 tsf64)
{
- ATH5K_TRACE(ah->ah_sc);
-
ath5k_hw_reg_write(ah, tsf64 & 0xffffffff, AR5K_TSF_L32);
ath5k_hw_reg_write(ah, (tsf64 >> 32) & 0xffffffff, AR5K_TSF_U32);
}
@@ -565,8 +548,6 @@ void ath5k_hw_reset_tsf(struct ath5k_hw *ah)
{
u32 val;

- ATH5K_TRACE(ah->ah_sc);
-
val = ath5k_hw_reg_read(ah, AR5K_BEACON) | AR5K_BEACON_RESET_TSF;

/*
@@ -586,7 +567,6 @@ void ath5k_hw_init_beacon(struct ath5k_hw *ah, u32 next_beacon, u32 interval)
{
u32 timer1, timer2, timer3;

- ATH5K_TRACE(ah->ah_sc);
/*
* Set the additional timers by mode
*/
@@ -674,7 +654,6 @@ int ath5k_hw_reset_key(struct ath5k_hw *ah, u16 entry)
unsigned int i, type;
u16 micentry = entry + AR5K_KEYTABLE_MIC_OFFSET;

- ATH5K_TRACE(ah->ah_sc);
AR5K_ASSERT_ENTRY(entry, AR5K_KEYTABLE_SIZE);

type = ath5k_hw_reg_read(ah, AR5K_KEYTABLE_TYPE(entry));
@@ -749,8 +728,6 @@ int ath5k_hw_set_key(struct ath5k_hw *ah, u16 entry,
bool is_tkip;
const u8 *key_ptr;

- ATH5K_TRACE(ah->ah_sc);
-
is_tkip = (key->alg == ALG_TKIP);

/*
@@ -836,7 +813,6 @@ int ath5k_hw_set_key_lladdr(struct ath5k_hw *ah, u16 entry, const u8 *mac)
{
u32 low_id, high_id;

- ATH5K_TRACE(ah->ah_sc);
/* Invalid entry (key table overflow) */
AR5K_ASSERT_ENTRY(entry, AR5K_KEYTABLE_SIZE);

diff --git a/drivers/net/wireless/ath/ath5k/phy.c b/drivers/net/wireless/ath/ath5k/phy.c
index 0b24081..f369e7e 100644
--- a/drivers/net/wireless/ath/ath5k/phy.c
+++ b/drivers/net/wireless/ath/ath5k/phy.c
@@ -378,8 +378,6 @@ enum ath5k_rfgain ath5k_hw_gainf_calibrate(struct ath5k_hw *ah)
u32 data, type;
struct ath5k_eeprom_info *ee = &ah->ah_capabilities.cap_eeprom;

- ATH5K_TRACE(ah->ah_sc);
-
if (ah->ah_rf_banks == NULL ||
ah->ah_gain.g_state == AR5K_RFGAIN_INACTIVE)
return AR5K_RFGAIN_INACTIVE;
@@ -1353,7 +1351,6 @@ ath5k_hw_rf511x_iq_calibrate(struct ath5k_hw *ah)
u32 i_pwr, q_pwr;
s32 iq_corr, i_coff, i_coffd, q_coff, q_coffd;
int i;
- ATH5K_TRACE(ah->ah_sc);

if (!ah->ah_calibration ||
ath5k_hw_reg_read(ah, AR5K_PHY_IQ) & AR5K_PHY_IQ_RUN)
@@ -1680,7 +1677,6 @@ ath5k_hw_set_spur_mitigation_filter(struct ath5k_hw *ah,

int ath5k_hw_phy_disable(struct ath5k_hw *ah)
{
- ATH5K_TRACE(ah->ah_sc);
/*Just a try M.F.*/
ath5k_hw_reg_write(ah, AR5K_PHY_ACT_DISABLE, AR5K_PHY_ACT);

@@ -1696,8 +1692,6 @@ u16 ath5k_hw_radio_revision(struct ath5k_hw *ah, unsigned int chan)
u32 srev;
u16 ret;

- ATH5K_TRACE(ah->ah_sc);
-
/*
* Set the radio chip access register
*/
@@ -1742,8 +1736,6 @@ u16 ath5k_hw_radio_revision(struct ath5k_hw *ah, unsigned int chan)
static void /*TODO:Boundary check*/
ath5k_hw_set_def_antenna(struct ath5k_hw *ah, u8 ant)
{
- ATH5K_TRACE(ah->ah_sc);
-
if (ah->ah_version != AR5K_AR5210)
ath5k_hw_reg_write(ah, ant & 0x7, AR5K_DEFAULT_ANTENNA);
}
@@ -1803,8 +1795,6 @@ ath5k_hw_set_antenna_mode(struct ath5k_hw *ah, u8 ant_mode)

def_ant = ah->ah_def_ant;

- ATH5K_TRACE(ah->ah_sc);
-
switch (channel->hw_value & CHANNEL_MODES) {
case CHANNEL_A:
case CHANNEL_T:
@@ -2970,7 +2960,6 @@ ath5k_hw_txpower(struct ath5k_hw *ah, struct ieee80211_channel *channel,
u8 type;
int ret;

- ATH5K_TRACE(ah->ah_sc);
if (txpower > AR5K_TUNE_MAX_TXPOWER) {
ATH5K_ERR(ah->ah_sc, "invalid tx power: %u\n", txpower);
return -EINVAL;
@@ -3066,8 +3055,6 @@ int ath5k_hw_set_txpower_limit(struct ath5k_hw *ah, u8 txpower)
struct ieee80211_channel *channel = ah->ah_current_channel;
u8 ee_mode;

- ATH5K_TRACE(ah->ah_sc);
-
switch (channel->hw_value & CHANNEL_MODES) {
case CHANNEL_A:
case CHANNEL_T:
diff --git a/drivers/net/wireless/ath/ath5k/qcu.c b/drivers/net/wireless/ath/ath5k/qcu.c
index f5831da..4186ff4 100644
--- a/drivers/net/wireless/ath/ath5k/qcu.c
+++ b/drivers/net/wireless/ath/ath5k/qcu.c
@@ -31,7 +31,6 @@ Queue Control Unit, DFS Control Unit Functions
int ath5k_hw_get_tx_queueprops(struct ath5k_hw *ah, int queue,
struct ath5k_txq_info *queue_info)
{
- ATH5K_TRACE(ah->ah_sc);
memcpy(queue_info, &ah->ah_txq[queue], sizeof(struct ath5k_txq_info));
return 0;
}
@@ -42,7 +41,6 @@ int ath5k_hw_get_tx_queueprops(struct ath5k_hw *ah, int queue,
int ath5k_hw_set_tx_queueprops(struct ath5k_hw *ah, int queue,
const struct ath5k_txq_info *queue_info)
{
- ATH5K_TRACE(ah->ah_sc);
AR5K_ASSERT_ENTRY(queue, ah->ah_capabilities.cap_queues.q_tx_num);

if (ah->ah_txq[queue].tqi_type == AR5K_TX_QUEUE_INACTIVE)
@@ -69,8 +67,6 @@ int ath5k_hw_setup_tx_queue(struct ath5k_hw *ah, enum ath5k_tx_queue queue_type,
unsigned int queue;
int ret;

- ATH5K_TRACE(ah->ah_sc);
-
/*
* Get queue by type
*/
@@ -149,7 +145,6 @@ int ath5k_hw_setup_tx_queue(struct ath5k_hw *ah, enum ath5k_tx_queue queue_type,
u32 ath5k_hw_num_tx_pending(struct ath5k_hw *ah, unsigned int queue)
{
u32 pending;
- ATH5K_TRACE(ah->ah_sc);
AR5K_ASSERT_ENTRY(queue, ah->ah_capabilities.cap_queues.q_tx_num);

/* Return if queue is declared inactive */
@@ -177,7 +172,6 @@ u32 ath5k_hw_num_tx_pending(struct ath5k_hw *ah, unsigned int queue)
*/
void ath5k_hw_release_tx_queue(struct ath5k_hw *ah, unsigned int queue)
{
- ATH5K_TRACE(ah->ah_sc);
if (WARN_ON(queue >= ah->ah_capabilities.cap_queues.q_tx_num))
return;

@@ -195,7 +189,6 @@ int ath5k_hw_reset_tx_queue(struct ath5k_hw *ah, unsigned int queue)
u32 cw_min, cw_max, retry_lg, retry_sh;
struct ath5k_txq_info *tq = &ah->ah_txq[queue];

- ATH5K_TRACE(ah->ah_sc);
AR5K_ASSERT_ENTRY(queue, ah->ah_capabilities.cap_queues.q_tx_num);

tq = &ah->ah_txq[queue];
@@ -523,8 +516,6 @@ int ath5k_hw_set_slot_time(struct ath5k_hw *ah, unsigned int slot_time)
{
u32 slot_time_clock = ath5k_hw_htoclock(ah, slot_time);

- ATH5K_TRACE(ah->ah_sc);
-
if (slot_time < 6 || slot_time_clock > AR5K_SLOT_TIME_MAX)
return -EINVAL;

diff --git a/drivers/net/wireless/ath/ath5k/reset.c b/drivers/net/wireless/ath/ath5k/reset.c
index 44bbbf2..5e23e61 100644
--- a/drivers/net/wireless/ath/ath5k/reset.c
+++ b/drivers/net/wireless/ath/ath5k/reset.c
@@ -201,8 +201,6 @@ static int ath5k_hw_nic_reset(struct ath5k_hw *ah, u32 val)
int ret;
u32 mask = val ? val : ~0U;

- ATH5K_TRACE(ah->ah_sc);
-
/* Read-and-clear RX Descriptor Pointer*/
ath5k_hw_reg_read(ah, AR5K_RXDP);

@@ -246,7 +244,6 @@ static int ath5k_hw_set_power(struct ath5k_hw *ah, enum ath5k_power_mode mode,
unsigned int i;
u32 staid, data;

- ATH5K_TRACE(ah->ah_sc);
staid = ath5k_hw_reg_read(ah, AR5K_STA_ID1);

switch (mode) {
@@ -393,8 +390,6 @@ int ath5k_hw_nic_wakeup(struct ath5k_hw *ah, int flags, bool initial)
mode = 0;
clock = 0;

- ATH5K_TRACE(ah->ah_sc);
-
/* Wakeup the device */
ret = ath5k_hw_set_power(ah, AR5K_PM_AWAKE, true, 0);
if (ret) {
@@ -899,8 +894,6 @@ int ath5k_hw_reset(struct ath5k_hw *ah, enum nl80211_iftype op_mode,
u8 mode, freq, ee_mode, ant[2];
int i, ret;

- ATH5K_TRACE(ah->ah_sc);
-
s_ant = 0;
ee_mode = 0;
staid1_flags = 0;


2010-05-19 01:33:27

by Bruno Randolf

[permalink] [raw]
Subject: [PATCH v2 18/20] ath5k: update AR5K_PHY_RESTART_DIV_GC values to match masks

#define AR5K_PHY_RESTART_DIV_GC 0x001c0000
is 3 bit wide.

The previous values of 0xc and 0x8 are 4bit wide and bigger than the mask.

Writing 0 and 1 to AR5K_PHY_RESTART_DIV_GC is consistent with the comments and
initvals we have in the HAL.

Signed-off-by: Bruno Randolf <[email protected]>
---
drivers/net/wireless/ath/ath5k/phy.c | 4 ++--
1 files changed, 2 insertions(+), 2 deletions(-)

diff --git a/drivers/net/wireless/ath/ath5k/phy.c b/drivers/net/wireless/ath/ath5k/phy.c
index d9506e7..2136930 100644
--- a/drivers/net/wireless/ath/ath5k/phy.c
+++ b/drivers/net/wireless/ath/ath5k/phy.c
@@ -1768,13 +1768,13 @@ ath5k_hw_set_fast_div(struct ath5k_hw *ah, u8 ee_mode, bool enable)

if (enable) {
AR5K_REG_WRITE_BITS(ah, AR5K_PHY_RESTART,
- AR5K_PHY_RESTART_DIV_GC, 0xc);
+ AR5K_PHY_RESTART_DIV_GC, 1);

AR5K_REG_ENABLE_BITS(ah, AR5K_PHY_FAST_ANT_DIV,
AR5K_PHY_FAST_ANT_DIV_EN);
} else {
AR5K_REG_WRITE_BITS(ah, AR5K_PHY_RESTART,
- AR5K_PHY_RESTART_DIV_GC, 0x8);
+ AR5K_PHY_RESTART_DIV_GC, 0);

AR5K_REG_DISABLE_BITS(ah, AR5K_PHY_FAST_ANT_DIV,
AR5K_PHY_FAST_ANT_DIV_EN);


2010-05-19 01:32:34

by Bruno Randolf

[permalink] [raw]
Subject: [PATCH v2 08/20] ath5k: clarify logic when to enable spur mitigation filter

The old code logically did not make sense and seems to have been confused by
the fact that we could have newer EEPROMs on older hardware. In any case the
spur mitigation filter was set if the srev was >= AR5K_SREV_AR5424.

Spur info is available only from EEPROM versions bigger than 5.3 but but the
EEPOM routines will use static values for older versions, so that should be
o.k.

Signed-off-by: Bruno Randolf <[email protected]>
---
drivers/net/wireless/ath/ath5k/reset.c | 15 +++++----------
1 files changed, 5 insertions(+), 10 deletions(-)

diff --git a/drivers/net/wireless/ath/ath5k/reset.c b/drivers/net/wireless/ath/ath5k/reset.c
index 5e23e61..2890a7f 100644
--- a/drivers/net/wireless/ath/ath5k/reset.c
+++ b/drivers/net/wireless/ath/ath5k/reset.c
@@ -1090,22 +1090,17 @@ int ath5k_hw_reset(struct ath5k_hw *ah, enum nl80211_iftype op_mode,
/* Write OFDM timings on 5212*/
if (ah->ah_version == AR5K_AR5212 &&
channel->hw_value & CHANNEL_OFDM) {
- struct ath5k_eeprom_info *ee =
- &ah->ah_capabilities.cap_eeprom;

ret = ath5k_hw_write_ofdm_timings(ah, channel);
if (ret)
return ret;

- /* Note: According to docs we can have a newer
- * EEPROM on old hardware, so we need to verify
- * that our hardware is new enough to have spur
- * mitigation registers (delta phase etc) */
- if (ah->ah_mac_srev >= AR5K_SREV_AR5424 ||
- (ah->ah_mac_srev >= AR5K_SREV_AR5424 &&
- ee->ee_version >= AR5K_EEPROM_VERSION_5_3))
+ /* Spur info is available only from EEPROM versions
+ * bigger than 5.3 but but the EEPOM routines will use
+ * static values for older versions */
+ if (ah->ah_mac_srev >= AR5K_SREV_AR5424)
ath5k_hw_set_spur_mitigation_filter(ah,
- channel);
+ channel);
}

/*Enable/disable 802.11b mode on 5111


2010-05-20 22:06:10

by Luis R. Rodriguez

[permalink] [raw]
Subject: Re: [ath5k-devel] [PATCH v2 13/20] cfg80211: Add nl80211 antenna configuration

On Wed, May 19, 2010 at 11:43 PM, Luis R. Rodriguez
<[email protected]> wrote:
> On Wed, May 19, 2010 at 10:36 PM, Bruno Randolf <[email protected]> wrote:
>> On Thursday 20 May 2010 14:17:19 you wrote:
>>> None of the legacy 802.11 drivers we support have more than 2
>>> antennas, I am also not aware of any.
>>
>> i have heard of some solutions based on atheros chipsets with more than 2
>> antennas ("pre-11n RangeMax", "large phased array switch"). please check
>> internally.
>
> I will. David, are you aware of legacy (non 802.11n) devices with more
> than 2 antennas?

I picked David's brain and we're pretty certain these devices do not
exist in the market.

>>> You're right, then if you really don't mind lets think 802.11n through
>>> well then.
>>
>> i don't mind to do that, but as i said i dont know much about 802.11n yet.
>
> Thanks, give me some time to think about this then and get back to you.

OK so back to the drawing board, this is what I recommend:

For legacy, keep it simple, use 3 settings, fixed_a, fixed_b,
diversity, for all devices.

Lets use a different API for 802.11n. Reason being that even the case
I mentioned of an 802.11n device connecting on a legacy network needs
to be treated differently actually.

For 802.11n you have a few more considerations. You can actually TX at
the same time on two or more different antennas at the same time. The
data you transmit will be the contents on both chains on a dual stream
device. So both antenna 0 and antenna 1 will both be transmitting the
data for both stream 0 and stream 1. As it turns out the combination
of TX'ing on two antennas at the same time at a certain dBm power will
yield a higher received frame on the RX side. This is why when you use
multiple chains you have to take regulatory rules into considerations
as well, since adding more chains will increase the overall output
power. Today ath9k handles this itself since this data is calibrated
but the max EIRP is passed out from cfg80211. Devices which do not
deal with these regulatory considerations likely won't support
changing chainmasks unless they use an API to respect regulatory
internally somehow. Perhaps the iwlagn firmware does this, beats me.

The right terminology for antenna control for both TX and RX is
chainmask and a bitmap of 8 will suffice for existing hardware and up
to the not-yet-existant 600mbps 4 stream devices. Supporting 8 bits
will support up to 8 streams and we do not envision using beyond that
at this point. There is some considerations in the future for
supporting something other than HT40, like maybe HT80 and so forth but
those things won't be using more streams it seems.

Then, some devices won't support all possible chainmask settings. This
will vary depending on the chipset. I work for Atheros so I can only
tell you what we can support, we'll have to check with the Intel folks
about their chipset limitations and settings.

AR5416, AR5418 can only support chainmask settings which always keep
the first chain on. The AR9001 family and beyond cannot support the
0b110 chaimask (David, you had pointed out some other restrictions,
what were they again?), the details are complex and I did not get a
chance to review them.

I would not be surprised if other vendors had similar restrictions so
I'm thinking maybe we can express this as a requirement mask, or a set
of requirement masks. This way userspace utilities for debugging would
only expose certain chainmask settings.

Now technically then you can incorporate the legacy API with the
802.11n API here somehow but it just seems cleaner to keep them
separate.

Also, David indicated that when we change the chainmask when are are
associated we have to do an actual chip reset, this is different than
the antenna diversity settings which an be done on the fly. We likely
will need to reassociate for a chainmask setting, not sure.

Luis

2010-05-20 12:52:26

by Nick Kossifidis

[permalink] [raw]
Subject: Re: [PATCH v2 05/20] ath5k: Stop queues only for NF calibration

2010/5/19 Bruno Randolf <[email protected]>:
> As far as we know, only NF calibration interferes with RX/TX so we can
> leave the queues enabled for the other calibrations.
>
> BTW: Stopping the queues is not enough for avoiding transmissions, since there
> might be packets in the queue + beacons are also sent regularly! But i leave it
> like this until we have a better solution (stopping TX DMA?).
>
> Signed-off-by: Bruno Randolf <[email protected]>
> ---
>  drivers/net/wireless/ath/ath5k/base.c |   11 ++++++-----
>  1 files changed, 6 insertions(+), 5 deletions(-)
>
> diff --git a/drivers/net/wireless/ath/ath5k/base.c b/drivers/net/wireless/ath/ath5k/base.c
> index 88c7314..3206ed6 100644
> --- a/drivers/net/wireless/ath/ath5k/base.c
> +++ b/drivers/net/wireless/ath/ath5k/base.c
> @@ -2785,10 +2785,6 @@ ath5k_tasklet_calibrate(unsigned long data)
>        /* Only full calibration for now */
>        ah->ah_cal_mask |= AR5K_CALIBRATION_FULL;
>
> -       /* Stop queues so that calibration
> -        * doesn't interfere with tx */
> -       ieee80211_stop_queues(sc->hw);
> -
>        ATH5K_DBG(sc, ATH5K_DEBUG_CALIBRATE, "channel %u/%x\n",
>                ieee80211_frequency_to_channel(sc->curchan->center_freq),
>                sc->curchan->hw_value);
> @@ -2806,8 +2802,13 @@ ath5k_tasklet_calibrate(unsigned long data)
>                        ieee80211_frequency_to_channel(
>                                sc->curchan->center_freq));
>
> +       /* TODO: We don't need to run noise floor calibration as often
> +        * as I/Q calibration.*/
> +
> +       /* Noise floor calibration interrupts rx/tx path while I/Q calibration
> +        * doesn't. Stop queues so that calibration doesn't interfere with tx */
> +       ieee80211_stop_queues(sc->hw);
>        ath5k_hw_update_noise_floor(ah);
> -       /* Wake queues */
>        ieee80211_wake_queues(sc->hw);
>
>        ah->ah_cal_mask &= ~AR5K_CALIBRATION_FULL;
>

Acked-by: Nick Kossifidis <[email protected]>



--
GPG ID: 0xD21DB2DB
As you read this post global entropy rises. Have Fun ;-)
Nick

2010-05-20 05:36:23

by Bruno Randolf

[permalink] [raw]
Subject: Re: [ath5k-devel] [PATCH v2 13/20] cfg80211: Add nl80211 antenna configuration

On Thursday 20 May 2010 14:17:19 you wrote:
> None of the legacy 802.11 drivers we support have more than 2
> antennas, I am also not aware of any.

i have heard of some solutions based on atheros chipsets with more than 2
antennas ("pre-11n RangeMax", "large phased array switch"). please check
internally.

> But -- I can think of using an 802.11n device in legacy
> mode of operation using specific antennas.... so there's your example
> of a valid case for this.

thanks! :)

> Legacy devices are dead. I don't know anyone in the industry making
> them, the 1 stream 802.11n devices are cheaper today, so there is no
> point in the market for it.

that might be true from a chipset manufacturers marketing perspective - but we
work on the linux kernel... ;)

as you know there are millions of so called "legacy" chipsets out there and
people are going to continue to use them wether your marketing declares them
"dead" or not... i think it's worth to properly support them.

also please don't forget that some people use the linux kernel not just for
standard use cases, but for research and developing new solutions. we should
provide the flexibility to support that, if possible.

> > and we are sure we don't want to support more than 2
> > antennas - well, we could save 6 bits... is it really worth it?
>
> You're right, then if you really don't mind lets think 802.11n through
> well then.

i don't mind to do that, but as i said i dont know much about 802.11n yet.

bruno

2010-05-19 01:47:44

by Luis R. Rodriguez

[permalink] [raw]
Subject: Re: [PATCH v2 00/20] pending ath5k + antenna patches

On Tue, May 18, 2010 at 6:30 PM, Bruno Randolf <[email protected]> wrote:
> These are all my pending ath5k patches + the antenna support for cfg80211
> rebased against 2.6.34 and based on the previous patch i sent for stable
> ("ath5k: consistently use rx_bufsize for RX DMA").

No need to bunch these up together, in fact it would help the review
process if instead you spit these up by series which are relevant to
each other.

The stable fix is one alone, the antenna stuff should be a separate
series, then the other stuff if they are not related as a separate
series.

> Also i'd like to note that there are quite a few ath5k patches which haven't
> made it into linux-next yet. It would be great to get them into 2.6.35...

Unless its a fix, I think its too late.

Luis

2010-05-19 01:32:08

by Bruno Randolf

[permalink] [raw]
Subject: [PATCH v2 03/20] ath5k: initialize calibration timers

Initialize calibration timers on reset, since otherwise they might be in the
future and the calibration tasklet might not be scheduled for a long time.

Signed-off-by: Bruno Randolf <[email protected]>
---
drivers/net/wireless/ath/ath5k/base.c | 2 ++
1 files changed, 2 insertions(+), 0 deletions(-)

diff --git a/drivers/net/wireless/ath/ath5k/base.c b/drivers/net/wireless/ath/ath5k/base.c
index 9f9107b..de59a6e 100644
--- a/drivers/net/wireless/ath/ath5k/base.c
+++ b/drivers/net/wireless/ath/ath5k/base.c
@@ -2926,6 +2926,8 @@ ath5k_reset(struct ath5k_softc *sc, struct ieee80211_channel *chan)

ath5k_ani_init(ah, ah->ah_sc->ani_state.ani_mode);

+ ah->ah_cal_next_full = jiffies;
+ ah->ah_cal_next_ani = jiffies;
/*
* Change channels and update the h/w rate map if we're switching;
* e.g. 11a to 11b/g.


2010-05-19 01:32:55

by Bruno Randolf

[permalink] [raw]
Subject: [PATCH v2 12/20] ath5k: print error message if ANI levels are out of range

Since we have sysfs to manually set the ANI levels, we should print errors to
the kernel log if the values are out of bounds.

Signed-off-by: Bruno Randolf <[email protected]>
---
drivers/net/wireless/ath/ath5k/ani.c | 11 +++++------
1 files changed, 5 insertions(+), 6 deletions(-)

diff --git a/drivers/net/wireless/ath/ath5k/ani.c b/drivers/net/wireless/ath/ath5k/ani.c
index 987e3d3..26dbe65 100644
--- a/drivers/net/wireless/ath/ath5k/ani.c
+++ b/drivers/net/wireless/ath/ath5k/ani.c
@@ -74,8 +74,8 @@ ath5k_ani_set_noise_immunity_level(struct ath5k_hw *ah, int level)
const s8 fr[] = { -78, -80 };
#endif
if (level < 0 || level >= ARRAY_SIZE(sz)) {
- ATH5K_DBG_UNLIMIT(ah->ah_sc, ATH5K_DEBUG_ANI,
- "level out of range %d", level);
+ ATH5K_ERR(ah->ah_sc, "noise immuniy level %d out of range",
+ level);
return;
}

@@ -106,8 +106,8 @@ ath5k_ani_set_spur_immunity_level(struct ath5k_hw *ah, int level)

if (level < 0 || level >= ARRAY_SIZE(val) ||
level > ah->ah_sc->ani_state.max_spur_level) {
- ATH5K_DBG_UNLIMIT(ah->ah_sc, ATH5K_DEBUG_ANI,
- "level out of range %d", level);
+ ATH5K_ERR(ah->ah_sc, "spur immunity level %d out of range",
+ level);
return;
}

@@ -130,8 +130,7 @@ ath5k_ani_set_firstep_level(struct ath5k_hw *ah, int level)
const int val[] = { 0, 4, 8 };

if (level < 0 || level >= ARRAY_SIZE(val)) {
- ATH5K_DBG_UNLIMIT(ah->ah_sc, ATH5K_DEBUG_ANI,
- "level out of range %d", level);
+ ATH5K_ERR(ah->ah_sc, "firstep level %d out of range", level);
return;
}



2010-05-20 12:59:03

by Nick Kossifidis

[permalink] [raw]
Subject: Re: [PATCH v2 11/20] ath5k: always calculate ANI listen time

2010/5/19 Bruno Randolf <[email protected]>:
> Calculate 'listen' time also when automatic ANI is off, since this and the
> "busy" time is useful information also in manual mode.
>
> Signed-off-by: Bruno Randolf <[email protected]>
> ---
>  drivers/net/wireless/ath/ath5k/ani.c |    9 +++++----
>  1 files changed, 5 insertions(+), 4 deletions(-)
>
> diff --git a/drivers/net/wireless/ath/ath5k/ani.c b/drivers/net/wireless/ath/ath5k/ani.c
> index f2311ab..987e3d3 100644
> --- a/drivers/net/wireless/ath/ath5k/ani.c
> +++ b/drivers/net/wireless/ath/ath5k/ani.c
> @@ -481,14 +481,15 @@ ath5k_ani_calibration(struct ath5k_hw *ah)
>        struct ath5k_ani_state *as = &ah->ah_sc->ani_state;
>        int listen, ofdm_high, ofdm_low, cck_high, cck_low;
>
> -       if (as->ani_mode != ATH5K_ANI_MODE_AUTO)
> -               return;
> -
>        /* get listen time since last call and add it to the counter because we
> -        * might not have restarted the "ani period" last time */
> +        * might not have restarted the "ani period" last time.
> +        * always do this to calculate the busy time also in manual mode */
>        listen = ath5k_hw_ani_get_listen_time(ah, as);
>        as->listen_time += listen;
>
> +       if (as->ani_mode != ATH5K_ANI_MODE_AUTO)
> +               return;
> +
>        ath5k_ani_save_and_clear_phy_errors(ah, as);
>
>        ofdm_high = as->listen_time * ATH5K_ANI_OFDM_TRIG_HIGH / 1000;
>

Acked-by: Nick Kossifidis <[email protected]>



--
GPG ID: 0xD21DB2DB
As you read this post global entropy rises. Have Fun ;-)
Nick

2010-05-19 01:33:11

by Bruno Randolf

[permalink] [raw]
Subject: [PATCH v2 15/20] ath5k: Add support for antenna configuration

Support setting the antenna configuration via cfg/mac80211. At the moment only
allow the simple pre-defined configurations we already have (fixed antenna A/B
or diversity), but more advanced settings should be possible later.

Signed-off-by: Bruno Randolf <[email protected]>
---
drivers/net/wireless/ath/ath5k/base.c | 34 +++++++++++++++++++++++++++++++++
1 files changed, 34 insertions(+), 0 deletions(-)

diff --git a/drivers/net/wireless/ath/ath5k/base.c b/drivers/net/wireless/ath/ath5k/base.c
index d5d514f..5f24e4a 100644
--- a/drivers/net/wireless/ath/ath5k/base.c
+++ b/drivers/net/wireless/ath/ath5k/base.c
@@ -256,6 +256,8 @@ static void ath5k_sw_scan_start(struct ieee80211_hw *hw);
static void ath5k_sw_scan_complete(struct ieee80211_hw *hw);
static void ath5k_set_coverage_class(struct ieee80211_hw *hw,
u8 coverage_class);
+static int ath5k_set_antenna(struct ieee80211_hw *hw, u8 tx_ant, u8 rx_ant);
+static int ath5k_get_antenna(struct ieee80211_hw *hw, u8 *tx_ant, u8 *rx_ant);

static const struct ieee80211_ops ath5k_hw_ops = {
.tx = ath5k_tx,
@@ -277,6 +279,8 @@ static const struct ieee80211_ops ath5k_hw_ops = {
.sw_scan_start = ath5k_sw_scan_start,
.sw_scan_complete = ath5k_sw_scan_complete,
.set_coverage_class = ath5k_set_coverage_class,
+ .set_antenna = ath5k_set_antenna,
+ .get_antenna = ath5k_get_antenna,
};

/*
@@ -3483,3 +3487,33 @@ static void ath5k_set_coverage_class(struct ieee80211_hw *hw, u8 coverage_class)
ath5k_hw_set_coverage_class(sc->ah, coverage_class);
mutex_unlock(&sc->lock);
}
+
+static int ath5k_set_antenna(struct ieee80211_hw *hw, u8 tx_ant, u8 rx_ant)
+{
+ struct ath5k_softc *sc = hw->priv;
+
+ if (tx_ant == 1 && rx_ant == 1)
+ ath5k_hw_set_antenna_mode(sc->ah, AR5K_ANTMODE_FIXED_A);
+ else if (tx_ant == 2 && rx_ant == 2)
+ ath5k_hw_set_antenna_mode(sc->ah, AR5K_ANTMODE_FIXED_B);
+ else if (tx_ant == 3 && rx_ant == 3)
+ ath5k_hw_set_antenna_mode(sc->ah, AR5K_ANTMODE_DEFAULT);
+ else
+ return -EINVAL;
+ return 0;
+}
+
+static int ath5k_get_antenna(struct ieee80211_hw *hw, u8 *tx_ant, u8 *rx_ant)
+{
+ struct ath5k_softc *sc = hw->priv;
+
+ switch (sc->ah->ah_ant_mode) {
+ case AR5K_ANTMODE_FIXED_A:
+ *tx_ant = 1; *rx_ant = 1; break;
+ case AR5K_ANTMODE_FIXED_B:
+ *tx_ant = 2; *rx_ant = 2; break;
+ case AR5K_ANTMODE_DEFAULT:
+ *tx_ant = 3; *rx_ant = 3; break;
+ }
+ return 0;
+}


2010-05-20 12:48:53

by Nick Kossifidis

[permalink] [raw]
Subject: Re: [PATCH v2 03/20] ath5k: initialize calibration timers

2010/5/19 Bruno Randolf <[email protected]>:
> Initialize calibration timers on reset, since otherwise they might be in the
> future and the calibration tasklet might not be scheduled for a long time.
>
> Signed-off-by: Bruno Randolf <[email protected]>
> ---
>  drivers/net/wireless/ath/ath5k/base.c |    2 ++
>  1 files changed, 2 insertions(+), 0 deletions(-)
>
> diff --git a/drivers/net/wireless/ath/ath5k/base.c b/drivers/net/wireless/ath/ath5k/base.c
> index 9f9107b..de59a6e 100644
> --- a/drivers/net/wireless/ath/ath5k/base.c
> +++ b/drivers/net/wireless/ath/ath5k/base.c
> @@ -2926,6 +2926,8 @@ ath5k_reset(struct ath5k_softc *sc, struct ieee80211_channel *chan)
>
>        ath5k_ani_init(ah, ah->ah_sc->ani_state.ani_mode);
>
> +       ah->ah_cal_next_full = jiffies;
> +       ah->ah_cal_next_ani = jiffies;
>        /*
>         * Change channels and update the h/w rate map if we're switching;
>         * e.g. 11a to 11b/g.
>

Acked-by: Nick Kossifidis <[email protected]>



--
GPG ID: 0xD21DB2DB
As you read this post global entropy rises. Have Fun ;-)
Nick

2010-05-19 01:33:37

by Bruno Randolf

[permalink] [raw]
Subject: [PATCH v2 20/20] ath5k: no need to save/restore the default antenna

Since ath5k_hw_set_antenna_mode() always writes the default antenna register
and is called at the end of reset, there is no need to separately save and
restore the default antenna.

Signed-off-by: Bruno Randolf <[email protected]>
---
drivers/net/wireless/ath/ath5k/reset.c | 8 +-------
1 files changed, 1 insertions(+), 7 deletions(-)

diff --git a/drivers/net/wireless/ath/ath5k/reset.c b/drivers/net/wireless/ath/ath5k/reset.c
index cf8fd48..3f24d46 100644
--- a/drivers/net/wireless/ath/ath5k/reset.c
+++ b/drivers/net/wireless/ath/ath5k/reset.c
@@ -880,12 +880,11 @@ int ath5k_hw_reset(struct ath5k_hw *ah, enum nl80211_iftype op_mode,
struct ieee80211_channel *channel, bool change_channel)
{
struct ath_common *common = ath5k_hw_common(ah);
- u32 s_seq[10], s_ant, s_led[3], staid1_flags, tsf_up, tsf_lo;
+ u32 s_seq[10], s_led[3], staid1_flags, tsf_up, tsf_lo;
u32 phy_tst1;
u8 mode, freq, ee_mode;
int i, ret;

- s_ant = 0;
ee_mode = 0;
staid1_flags = 0;
tsf_up = 0;
@@ -982,9 +981,6 @@ int ath5k_hw_reset(struct ath5k_hw *ah, enum nl80211_iftype op_mode,
}
}

- /* Save default antenna */
- s_ant = ath5k_hw_reg_read(ah, AR5K_DEFAULT_ANTENNA);
-
if (ah->ah_version == AR5K_AR5212) {
/* Restore normal 32/40MHz clock operation
* to avoid register access delay on certain
@@ -1144,8 +1140,6 @@ int ath5k_hw_reset(struct ath5k_hw *ah, enum nl80211_iftype op_mode,
ath5k_hw_reg_write(ah, tsf_lo, AR5K_TSF_L32);
}
}
-
- ath5k_hw_reg_write(ah, s_ant, AR5K_DEFAULT_ANTENNA);
}

/* Ledstate */


2010-05-20 06:44:12

by Luis R. Rodriguez

[permalink] [raw]
Subject: Re: [ath5k-devel] [PATCH v2 13/20] cfg80211: Add nl80211 antenna configuration

On Wed, May 19, 2010 at 10:36 PM, Bruno Randolf <[email protected]> wrote:
> On Thursday 20 May 2010 14:17:19 you wrote:
>> None of the legacy 802.11 drivers we support have more than 2
>> antennas, I am also not aware of any.
>
> i have heard of some solutions based on atheros chipsets with more than 2
> antennas ("pre-11n RangeMax", "large phased array switch"). please check
> internally.

I will. David, are you aware of legacy (non 802.11n) devices with more
than 2 antennas?

>> But -- I can think of using an 802.11n device in legacy
>> mode of operation using specific antennas.... so there's your example
>> of a valid case for this.
>
> thanks! :)
>
>> Legacy devices are dead. I don't know anyone in the industry making
>> them, the 1 stream 802.11n devices are cheaper today, so there is no
>> point in the market for it.
>
> that might be true from a chipset manufacturers marketing perspective - but we
> work on the linux kernel... ;)
>
> as you know there are millions of so called "legacy" chipsets out there and
> people are going to continue to use them wether your marketing declares them
> "dead" or not... i think it's worth to properly support them.
>
> also please don't forget that some people use the linux kernel not just for
> standard use cases, but for research and developing new solutions. we should
> provide the flexibility to support that, if possible.

Heh, yeah but what goes into the Linux kernel are drivers for hardware
silicon companies make. Who is making new legacy chipsets still?

>> > and we are sure we don't want to support more than 2
>> > antennas - well, we could save 6 bits... is it really worth it?
>>
>> You're right, then if you really don't mind lets think 802.11n through
>> well then.
>
> i don't mind to do that, but as i said i dont know much about 802.11n yet.

Thanks, give me some time to think about this then and get back to you.

Luis

2010-05-20 05:17:39

by Luis R. Rodriguez

[permalink] [raw]
Subject: Re: [ath5k-devel] [PATCH v2 13/20] cfg80211: Add nl80211 antenna configuration

>> > that's not very clear. can you give me an example?
>>
>> iw dev wlan0 set_tx_antenna 4
>
> so you want to transmit on antenna 3. if the card has 3 antennas -
> why not?

None of the legacy 802.11 drivers we support have more than 2
antennas, I am also not aware of any.

>> > what if there is a 'legacy' hardware with 3 or more antennas?
>>
>> Like what?
>
> i can't find an actual real world example for that, but it thought it might be
> possible, from what i know from the atheros eeprom which seems to be prepared
> for up to 6 antennas.

That's for 802.11n, I think it'd be much cleaner to treat that
separately. But -- I can think of using an 802.11n device in legacy
mode of operation using specific antennas.... so there's your example
of a valid case for this.

>> > i don't see how "my" API is too complicated, and i think it allows for a
>> > clear configuration of these cases as well.
>> >
>> > your criticism seems to be based on the fact that it's not clear how to
>> > handle 802.11n chainmask + antenna configuration, but this is not what
>> > my patch is concerned about. let's go step by step...
>>
>> No no, that is my fault, I brought that up, I was hoping we could
>> address it but it seems that we can't as I don't have time to think
>> about this further in a unified clean API. But if its just going to be
>> legacy then I don't see why we would use a large bitmap.
>
> i wanted it to be easily extensibe for 802.11n

If we do want to address 802.11n then I say we go back to the drawing
board and really think about how antennas settings work for 802.11n
and consider chainmask settings.

> and possibly 'legacy' with more antennas in the future.

Legacy devices are dead. I don't know anyone in the industry making
them, the 1 stream 802.11n devices are cheaper today, so there is no
point in the market for it.

> if there really are not any pre-N cards with more than
> 2 antennas out there,

Yup, I think this is the case :)

> and we are sure we don't want to support more than 2
> antennas - well, we could save 6 bits... is it really worth it?

You're right, then if you really don't mind lets think 802.11n through
well then.

Luis

2010-05-19 01:32:45

by Bruno Randolf

[permalink] [raw]
Subject: [PATCH v2 10/20] ath5k: add sysfs files for ANI parameters

/sys/class/ieee80211/phy0/device/ani/ani_mode
/sys/class/ieee80211/phy0/device/ani/noise_immunity_level
/sys/class/ieee80211/phy0/device/ani/spur_level
/sys/class/ieee80211/phy0/device/ani/firstep_level
/sys/class/ieee80211/phy0/device/ani/ofdm_weak_signal_detection
/sys/class/ieee80211/phy0/device/ani/cck_weak_signal_detection
/sys/class/ieee80211/phy0/device/ani/noise_immunity_level_max
/sys/class/ieee80211/phy0/device/ani/spur_level_max
/sys/class/ieee80211/phy0/device/ani/firstep_level_max

sysfs has a lot of symlinks, so you can find the files also in other locations,
like (by PCI ID) /sys/devices/pci0000:00/0000:00:11.0/ani and others.

Signed-off-by: Bruno Randolf <[email protected]>
---
drivers/net/wireless/ath/ath5k/Makefile | 1
drivers/net/wireless/ath/ath5k/ath5k.h | 3 +
drivers/net/wireless/ath/ath5k/base.c | 3 +
drivers/net/wireless/ath/ath5k/reset.c | 1
drivers/net/wireless/ath/ath5k/sysfs.c | 116 +++++++++++++++++++++++++++++++
5 files changed, 123 insertions(+), 1 deletions(-)
create mode 100644 drivers/net/wireless/ath/ath5k/sysfs.c

diff --git a/drivers/net/wireless/ath/ath5k/Makefile b/drivers/net/wireless/ath/ath5k/Makefile
index cc09595..2242a14 100644
--- a/drivers/net/wireless/ath/ath5k/Makefile
+++ b/drivers/net/wireless/ath/ath5k/Makefile
@@ -13,5 +13,6 @@ ath5k-y += base.o
ath5k-y += led.o
ath5k-y += rfkill.o
ath5k-y += ani.o
+ath5k-y += sysfs.o
ath5k-$(CONFIG_ATH5K_DEBUG) += debug.o
obj-$(CONFIG_ATH5K) += ath5k.o
diff --git a/drivers/net/wireless/ath/ath5k/ath5k.h b/drivers/net/wireless/ath/ath5k/ath5k.h
index d6f9afe..eace74d 100644
--- a/drivers/net/wireless/ath/ath5k/ath5k.h
+++ b/drivers/net/wireless/ath/ath5k/ath5k.h
@@ -1150,6 +1150,9 @@ struct ath5k_hw {
int ath5k_hw_attach(struct ath5k_softc *sc);
void ath5k_hw_detach(struct ath5k_hw *ah);

+int ath5k_sysfs_register(struct ath5k_softc *sc);
+void ath5k_sysfs_unregister(struct ath5k_softc *sc);
+
/* LED functions */
int ath5k_init_leds(struct ath5k_softc *sc);
void ath5k_led_enable(struct ath5k_softc *sc);
diff --git a/drivers/net/wireless/ath/ath5k/base.c b/drivers/net/wireless/ath/ath5k/base.c
index 3b43b76..d5d514f 100644
--- a/drivers/net/wireless/ath/ath5k/base.c
+++ b/drivers/net/wireless/ath/ath5k/base.c
@@ -861,6 +861,8 @@ ath5k_attach(struct pci_dev *pdev, struct ieee80211_hw *hw)

ath5k_init_leds(sc);

+ ath5k_sysfs_register(sc);
+
return 0;
err_queues:
ath5k_txq_release(sc);
@@ -896,6 +898,7 @@ ath5k_detach(struct pci_dev *pdev, struct ieee80211_hw *hw)
ath5k_hw_release_tx_queue(sc->ah, sc->bhalq);
ath5k_unregister_leds(sc);

+ ath5k_sysfs_unregister(sc);
/*
* NB: can't reclaim these until after ieee80211_ifdetach
* returns because we'll get called back to reclaim node
diff --git a/drivers/net/wireless/ath/ath5k/reset.c b/drivers/net/wireless/ath/ath5k/reset.c
index 2890a7f..b1d3239 100644
--- a/drivers/net/wireless/ath/ath5k/reset.c
+++ b/drivers/net/wireless/ath/ath5k/reset.c
@@ -851,7 +851,6 @@ static void ath5k_hw_commit_eeprom_settings(struct ath5k_hw *ah,
AR5K_PHY_NF_THRESH62,
ee->ee_thr_62[ee_mode]);

-
/* False detect backoff for channels
* that have spur noise. Write the new
* cyclic power RSSI threshold. */
diff --git a/drivers/net/wireless/ath/ath5k/sysfs.c b/drivers/net/wireless/ath/ath5k/sysfs.c
new file mode 100644
index 0000000..90757de
--- /dev/null
+++ b/drivers/net/wireless/ath/ath5k/sysfs.c
@@ -0,0 +1,116 @@
+#include <linux/device.h>
+#include <linux/pci.h>
+
+#include "base.h"
+#include "ath5k.h"
+#include "reg.h"
+
+#define SIMPLE_SHOW_STORE(name, get, set) \
+static ssize_t ath5k_attr_show_##name(struct device *dev, \
+ struct device_attribute *attr, \
+ char *buf) \
+{ \
+ struct ath5k_softc *sc = dev_get_drvdata(dev); \
+ return snprintf(buf, PAGE_SIZE, "%d\n", get); \
+} \
+ \
+static ssize_t ath5k_attr_store_##name(struct device *dev, \
+ struct device_attribute *attr, \
+ const char *buf, size_t count) \
+{ \
+ struct ath5k_softc *sc = dev_get_drvdata(dev); \
+ int val; \
+ \
+ val = (int)simple_strtoul(buf, NULL, 10); \
+ set(sc->ah, val); \
+ return count; \
+} \
+static DEVICE_ATTR(name, S_IRUGO | S_IWUSR, \
+ ath5k_attr_show_##name, ath5k_attr_store_##name)
+
+#define SIMPLE_SHOW(name, get) \
+static ssize_t ath5k_attr_show_##name(struct device *dev, \
+ struct device_attribute *attr, \
+ char *buf) \
+{ \
+ struct ath5k_softc *sc = dev_get_drvdata(dev); \
+ return snprintf(buf, PAGE_SIZE, "%d\n", get); \
+} \
+static DEVICE_ATTR(name, S_IRUGO, ath5k_attr_show_##name, NULL)
+
+/*** ANI ***/
+
+SIMPLE_SHOW_STORE(ani_mode, sc->ani_state.ani_mode, ath5k_ani_init);
+SIMPLE_SHOW_STORE(noise_immunity_level, sc->ani_state.noise_imm_level,
+ ath5k_ani_set_noise_immunity_level);
+SIMPLE_SHOW_STORE(spur_level, sc->ani_state.spur_level,
+ ath5k_ani_set_spur_immunity_level);
+SIMPLE_SHOW_STORE(firstep_level, sc->ani_state.firstep_level,
+ ath5k_ani_set_firstep_level);
+SIMPLE_SHOW_STORE(ofdm_weak_signal_detection, sc->ani_state.ofdm_weak_sig,
+ ath5k_ani_set_ofdm_weak_signal_detection);
+SIMPLE_SHOW_STORE(cck_weak_signal_detection, sc->ani_state.cck_weak_sig,
+ ath5k_ani_set_cck_weak_signal_detection);
+SIMPLE_SHOW(spur_level_max, sc->ani_state.max_spur_level);
+
+static ssize_t ath5k_attr_show_noise_immunity_level_max(struct device *dev,
+ struct device_attribute *attr,
+ char *buf)
+{
+ return snprintf(buf, PAGE_SIZE, "%d\n", ATH5K_ANI_MAX_NOISE_IMM_LVL);
+}
+static DEVICE_ATTR(noise_immunity_level_max, S_IRUGO,
+ ath5k_attr_show_noise_immunity_level_max, NULL);
+
+static ssize_t ath5k_attr_show_firstep_level_max(struct device *dev,
+ struct device_attribute *attr,
+ char *buf)
+{
+ return snprintf(buf, PAGE_SIZE, "%d\n", ATH5K_ANI_MAX_FIRSTEP_LVL);
+}
+static DEVICE_ATTR(firstep_level_max, S_IRUGO,
+ ath5k_attr_show_firstep_level_max, NULL);
+
+static struct attribute *ath5k_sysfs_entries_ani[] = {
+ &dev_attr_ani_mode.attr,
+ &dev_attr_noise_immunity_level.attr,
+ &dev_attr_spur_level.attr,
+ &dev_attr_firstep_level.attr,
+ &dev_attr_ofdm_weak_signal_detection.attr,
+ &dev_attr_cck_weak_signal_detection.attr,
+ &dev_attr_noise_immunity_level_max.attr,
+ &dev_attr_spur_level_max.attr,
+ &dev_attr_firstep_level_max.attr,
+ NULL
+};
+
+static struct attribute_group ath5k_attribute_group_ani = {
+ .name = "ani",
+ .attrs = ath5k_sysfs_entries_ani,
+};
+
+
+/*** register / unregister ***/
+
+int
+ath5k_sysfs_register(struct ath5k_softc *sc)
+{
+ struct device *dev = &sc->pdev->dev;
+ int err;
+
+ err = sysfs_create_group(&dev->kobj, &ath5k_attribute_group_ani);
+ if (err) {
+ ATH5K_ERR(sc, "failed to create sysfs group\n");
+ return err;
+ }
+
+ return 0;
+}
+
+void
+ath5k_sysfs_unregister(struct ath5k_softc *sc)
+{
+ struct device *dev = &sc->pdev->dev;
+
+ sysfs_remove_group(&dev->kobj, &ath5k_attribute_group_ani);
+}


2010-05-19 01:33:01

by Bruno Randolf

[permalink] [raw]
Subject: [PATCH v2 13/20] cfg80211: Add nl80211 antenna configuration

Allow setting TX and RX antenna configuration via nl80211/cfg80211.

The antenna configuration is defined as a bitmap of allowed antennas. This
bitmap is 8 bit at the moment, each bit representing one antenna. If multiple
antennas are selected, the driver may use diversity for receive and transmit.
This allows for a simple, yet flexible configuration interface for antennas,
while drivers may reject configurations they cannot support.

Signed-off-by: Bruno Randolf <[email protected]>
---
include/linux/nl80211.h | 12 +++++
include/net/cfg80211.h | 3 +
net/wireless/nl80211.c | 116 +++++++++++++++++++++++++++++++++++++++++++++++
3 files changed, 131 insertions(+), 0 deletions(-)

diff --git a/include/linux/nl80211.h b/include/linux/nl80211.h
index b7c77f9..46a2c76 100644
--- a/include/linux/nl80211.h
+++ b/include/linux/nl80211.h
@@ -341,6 +341,9 @@
* of any other interfaces, and other interfaces will again take
* precedence when they are used.
*
+ * @NL80211_CMD_SET_ANTENNA: Set a bitmap of antennas to use.
+ * @NL80211_CMD_GET_ANTENNA: Get antenna configuration from driver.
+ *
* @NL80211_CMD_MAX: highest used command number
* @__NL80211_CMD_AFTER_LAST: internal use
*/
@@ -441,6 +444,9 @@ enum nl80211_commands {

NL80211_CMD_SET_CHANNEL,

+ NL80211_CMD_SET_ANTENNA,
+ NL80211_CMD_GET_ANTENNA,
+
/* add new commands above here */

/* used to define NL80211_CMD_MAX below */
@@ -725,6 +731,9 @@ enum nl80211_commands {
* @NL80211_ATTR_AP_ISOLATE: (AP mode) Do not forward traffic between stations
* connected to this BSS.
*
+ * @NL80211_ATTR_ANTENNA_TX: Bitmap of antennas to use for transmitting.
+ * @NL80211_ATTR_ANTENNA_RX: Bitmap of antennas to use for receiving.
+ *
* @NL80211_ATTR_MAX: highest attribute number currently defined
* @__NL80211_ATTR_AFTER_LAST: internal use
*/
@@ -882,6 +891,9 @@ enum nl80211_attrs {

NL80211_ATTR_AP_ISOLATE,

+ NL80211_ATTR_ANTENNA_TX,
+ NL80211_ATTR_ANTENNA_RX,
+
/* add attributes here, update the policy in nl80211.c */

__NL80211_ATTR_AFTER_LAST,
diff --git a/include/net/cfg80211.h b/include/net/cfg80211.h
index b44a2e5..8861f40 100644
--- a/include/net/cfg80211.h
+++ b/include/net/cfg80211.h
@@ -1176,6 +1176,9 @@ struct cfg80211_ops {
int (*set_cqm_rssi_config)(struct wiphy *wiphy,
struct net_device *dev,
s32 rssi_thold, u32 rssi_hyst);
+
+ int (*set_antenna)(struct wiphy *wiphy, u8 tx_ant, u8 rx_ant);
+ int (*get_antenna)(struct wiphy *wiphy, u8 *tx_ant, u8 *rx_ant);
};

/*
diff --git a/net/wireless/nl80211.c b/net/wireless/nl80211.c
index aaa1aad..29998e0 100644
--- a/net/wireless/nl80211.c
+++ b/net/wireless/nl80211.c
@@ -153,6 +153,8 @@ static const struct nla_policy nl80211_policy[NL80211_ATTR_MAX+1] = {
[NL80211_ATTR_CQM] = { .type = NLA_NESTED, },
[NL80211_ATTR_LOCAL_STATE_CHANGE] = { .type = NLA_FLAG },
[NL80211_ATTR_AP_ISOLATE] = { .type = NLA_U8 },
+ [NL80211_ATTR_ANTENNA_TX] = { .type = NLA_U8 },
+ [NL80211_ATTR_ANTENNA_RX] = { .type = NLA_U8 },
};

/* policy for the attributes */
@@ -590,6 +592,8 @@ static int nl80211_send_wiphy(struct sk_buff *msg, u32 pid, u32 seq, int flags,
NLA_PUT_U32(msg, i, NL80211_CMD_SET_WIPHY_NETNS);
}
CMD(set_channel, SET_CHANNEL);
+ CMD(set_antenna, SET_ANTENNA);
+ CMD(get_antenna, GET_ANTENNA);

#undef CMD

@@ -4963,6 +4967,106 @@ out:
return err;
}

+static int nl80211_set_antenna(struct sk_buff *skb, struct genl_info *info)
+{
+ struct cfg80211_registered_device *rdev;
+ int res;
+ u8 rx_ant = 0, tx_ant = 0;
+
+ if (!info->attrs[NL80211_ATTR_WIPHY] ||
+ !info->attrs[NL80211_ATTR_ANTENNA_TX] ||
+ !info->attrs[NL80211_ATTR_ANTENNA_RX]) {
+ return -EINVAL;
+ }
+
+ tx_ant = nla_get_u8(info->attrs[NL80211_ATTR_ANTENNA_TX]);
+ rx_ant = nla_get_u8(info->attrs[NL80211_ATTR_ANTENNA_RX]);
+
+ rtnl_lock();
+
+ rdev = cfg80211_get_dev_from_info(info);
+ if (IS_ERR(rdev)) {
+ res = -ENODEV;
+ goto unlock_rtnl;
+ }
+
+ if (!rdev->ops->set_antenna) {
+ res = -EOPNOTSUPP;
+ goto unlock_rdev;
+ }
+
+ res = rdev->ops->set_antenna(&rdev->wiphy, tx_ant, rx_ant);
+
+ unlock_rdev:
+ cfg80211_unlock_rdev(rdev);
+
+ unlock_rtnl:
+ rtnl_unlock();
+ return res;
+}
+
+static int nl80211_get_antenna(struct sk_buff *skb, struct genl_info *info)
+{
+ struct cfg80211_registered_device *rdev;
+ struct sk_buff *msg;
+ void *hdr;
+ int res;
+ u8 tx_ant, rx_ant;
+
+ if (!info->attrs[NL80211_ATTR_WIPHY])
+ return -EINVAL;
+
+ rtnl_lock();
+
+ rdev = cfg80211_get_dev_from_info(info);
+ if (IS_ERR(rdev)) {
+ res = -ENODEV;
+ goto unlock_rtnl;
+ }
+
+ if (!rdev->ops->get_antenna) {
+ res = -EOPNOTSUPP;
+ goto unlock_rdev;
+ }
+
+ msg = nlmsg_new(NLMSG_DEFAULT_SIZE, GFP_KERNEL);
+ if (!msg) {
+ res = -ENOMEM;
+ goto unlock_rdev;
+ }
+
+ hdr = nl80211hdr_put(msg, info->snd_pid, info->snd_seq, 0,
+ NL80211_CMD_GET_ANTENNA);
+ if (!hdr) {
+ res = -ENOMEM;
+ goto free_msg;
+ }
+
+ res = rdev->ops->get_antenna(&rdev->wiphy, &tx_ant, &rx_ant);
+ if (res)
+ goto free_msg;
+
+ NLA_PUT_U8(msg, NL80211_ATTR_ANTENNA_TX, tx_ant);
+ NLA_PUT_U8(msg, NL80211_ATTR_ANTENNA_RX, rx_ant);
+
+ genlmsg_end(msg, hdr);
+ res = genlmsg_reply(msg, info);
+ goto unlock_rdev;
+
+ nla_put_failure:
+ res = -ENOBUFS;
+
+ free_msg:
+ nlmsg_free(msg);
+
+ unlock_rdev:
+ cfg80211_unlock_rdev(rdev);
+
+ unlock_rtnl:
+ rtnl_unlock();
+ return res;
+}
+
static struct genl_ops nl80211_ops[] = {
{
.cmd = NL80211_CMD_GET_WIPHY,
@@ -5279,6 +5383,18 @@ static struct genl_ops nl80211_ops[] = {
.policy = nl80211_policy,
.flags = GENL_ADMIN_PERM,
},
+ {
+ .cmd = NL80211_CMD_SET_ANTENNA,
+ .doit = nl80211_set_antenna,
+ .policy = nl80211_policy,
+ .flags = GENL_ADMIN_PERM,
+ },
+ {
+ .cmd = NL80211_CMD_GET_ANTENNA,
+ .doit = nl80211_get_antenna,
+ .policy = nl80211_policy,
+ /* can be retrieved by unprivileged users */
+ },
};

static struct genl_multicast_group nl80211_mlme_mcgrp = {


2010-05-21 17:11:05

by Luis R. Rodriguez

[permalink] [raw]
Subject: Re: [ath5k-devel] [PATCH v2 13/20] cfg80211: Add nl80211 antenna configuration

On Thu, May 20, 2010 at 06:59:33PM -0700, Bruno Randolf wrote:
> On Friday 21 May 2010 07:05:48 Luis R. Rodriguez wrote:
> > For legacy, keep it simple, use 3 settings, fixed_a, fixed_b,
> > diversity, for all devices.
>
> did you not understand my examples why i think it makes sense to use a bitmask
> for "legacy"? i think they are perfectly valid use-cases. do i need to re-
> iterate them a third time?

Hehe, well from what I gather is that you indicate some other legacy cards
would have a very different setup than the typical two anntennas and
diversity modes. I see those cases as being reallllllly rare and not
worth considering. Did I miss anything, if so please smack me.

> > Lets use a different API for 802.11n. Reason being that even the case
> > I mentioned of an 802.11n device connecting on a legacy network needs
> > to be treated differently actually.
> >
> > For 802.11n you have a few more considerations. You can actually TX at
> > the same time on two or more different antennas at the same time. The
> > data you transmit will be the contents on both chains on a dual stream
> > device. So both antenna 0 and antenna 1 will both be transmitting the
> > data for both stream 0 and stream 1. As it turns out the combination
> > of TX'ing on two antennas at the same time at a certain dBm power will
> > yield a higher received frame on the RX side. This is why when you use
> > multiple chains you have to take regulatory rules into considerations
> > as well, since adding more chains will increase the overall output
> > power. Today ath9k handles this itself since this data is calibrated
> > but the max EIRP is passed out from cfg80211. Devices which do not
> > deal with these regulatory considerations likely won't support
> > changing chainmasks unless they use an API to respect regulatory
> > internally somehow. Perhaps the iwlagn firmware does this, beats me.
> >
> > The right terminology for antenna control for both TX and RX is
> > chainmask and a bitmap of 8 will suffice for existing hardware and up
> > to the not-yet-existant 600mbps 4 stream devices. Supporting 8 bits
> > will support up to 8 streams and we do not envision using beyond that
> > at this point. There is some considerations in the future for
> > supporting something other than HT40, like maybe HT80 and so forth but
> > those things won't be using more streams it seems.
> >
> > Then, some devices won't support all possible chainmask settings. This
> > will vary depending on the chipset. I work for Atheros so I can only
> > tell you what we can support, we'll have to check with the Intel folks
> > about their chipset limitations and settings.
> >
> > AR5416, AR5418 can only support chainmask settings which always keep
> > the first chain on. The AR9001 family and beyond cannot support the
> > 0b110 chaimask (David, you had pointed out some other restrictions,
> > what were they again?), the details are complex and I did not get a
> > chance to review them.
> >
> > I would not be surprised if other vendors had similar restrictions so
> > I'm thinking maybe we can express this as a requirement mask, or a set
> > of requirement masks. This way userspace utilities for debugging would
> > only expose certain chainmask settings.
> >
> > Now technically then you can incorporate the legacy API with the
> > 802.11n API here somehow but it just seems cleaner to keep them
> > separate.
> >
> > Also, David indicated that when we change the chainmask when are are
> > associated we have to do an actual chip reset, this is different than
> > the antenna diversity settings which an be done on the fly. We likely
> > will need to reassociate for a chainmask setting, not sure.
>
> so from my point of view this is not very different from what we can support
> with the API i suggested. for RX it seems to be 100% equivalent.

Well I agree, the API *supports it* but I want *clean, clear and consistant
API*. And it just seems cleaner to separate the two.

> the main difference as i see it is that with 802.11n you transmit on more than
> one antenna, while with 'legacy' we can only transmit on one antenna at a
> time.

True, but note how the fact that you transmit over two antennas actually
has regulatory implications. Now, ath9k handles this within ath9k_hw already
but this itself seems like a worthy reason for this API to be separated.
While I think it is great for ath9k_hw to do this, wouldn't it be nice
if we can eventually instead expose the gain by using different chains
at the same time and do the regulatory calculation for all devices within
cfg80211?

> actually i have to admit that on legacy "antenna set tx 3 (b11)" (select two
> antennas for transmit) does not make much sense. i have defined it before as
> "use diversity" but what about a different definition: like "bitmap of
> antennas/chains to TRANSMIT".

Right, and while that *works*, I think it would be clearer to just use a
clear "diveristy" knob.

> so for 802.11n that would allow you to select multiple trasmit chains.

Instead of leaving the API to be interpreted by the mode of operation
I think it would be much cleaner to just make your desires clearer and
have the API define it well, and let the driver reject/accept it.

> on legacy you are only allowed to select one antenna
> in the bitmap. if it is set to "0" (or a separate flag) this could enable
> "follow RX antenna diversity" on legacy.

Sure that is one way, but it seems cleaner and easier for legacy purposes
to just define an API that only fits legacy.

> most of the other things you mention (need a reset/reassociate, regulatory
> concerns...) are driver implementation issues, which can be dealt with in the
> driver.

Well so some of these things *could* be handled in mac80211 as well. For
example, we may want to just dissociate upon a tx/rx chain setting change
for all devices, but not for legacy. The regulatory stuff is another thing
which could eventually be made more generic accross the board.

Additionally, suppose you write an iw-tweak-gui thingy, and you want to
provide expose tx/rx chainmask settings. Since some cards do not support
some chainmask settings we may want to allow for a query of unsupported
chainmasks and that way the GUI application could just grey-out the
unsupported chainmask settings instead of letting the user figure out
by trial and error that they are indeed not supported.

> i would just suggest to let the driver reject antenna/chainmask
> configurations which it cannot support.

The unified API works, but I think we can provide a cleaner API
if we split them. What real benefit do we get if we keep them together?
I just imagine this resulting in more convoluted code.

Luis

2010-05-21 20:28:38

by Luis R. Rodriguez

[permalink] [raw]
Subject: Re: [ath5k-devel] [PATCH v2 13/20] cfg80211: Add nl80211 antenna configuration

On Fri, May 21, 2010 at 12:10 PM, Felix Fietkau <[email protected]> wrote:
> On 2010-05-21 7:11 PM, Luis R. Rodriguez wrote:
>> On Thu, May 20, 2010 at 06:59:33PM -0700, Bruno Randolf wrote:
>>> so from my point of view this is not very different from what we can support
>>> with the API i suggested. for RX it seems to be 100% equivalent.
>>
>> Well I agree, the API *supports it* but I want *clean, clear and consistant
>> API*. And it just seems cleaner to separate the two.
>>
>>> the main difference as i see it is that with 802.11n you transmit on more than
>>> one antenna, while with 'legacy' we can only transmit on one antenna at a
>>> time.
>>
>> True, but note how the fact that you transmit over two antennas actually
>> has regulatory implications. Now, ath9k handles this within ath9k_hw already
>> but this itself seems like a worthy reason for this API to be separated.
>> While I think it is great for ath9k_hw to do this, wouldn't it be nice
>> if we can eventually instead expose the gain by using different chains
>> at the same time and do the regulatory calculation for all devices within
>> cfg80211?
>>
>>> actually i have to admit that on legacy "antenna set tx 3 (b11)" (select two
>>> antennas for transmit) does not make much sense. i have defined it before as
>>> "use diversity" but what about a different definition: like "bitmap of
>>> antennas/chains to TRANSMIT".
>>
>> Right, and while that *works*, I think it would be clearer to just use a
>> clear "diveristy" knob.
> Splitting it by mode of operation (11n vs legacy) does not work, because
> in AP mode you're doing both at the same time and there is an overlap in
> both settings.

Ah, hm, good point.

> I think that Bruno's suggestion of keeping them as one setting makes
> sense. About the regulatory concerns: where in the code does the
> chainmask currently affect the regulatory constraints?

I'm not sure, this was based on a quick review with David, I'll have
to review and poke at it but IIRC this was related to the chainmask
gains and I think we may get that from the EEPROM.

>>> so for 802.11n that would allow you to select  multiple trasmit chains.
>>
>> Instead of leaving the API to be interpreted by the mode of operation
>> I think it would be much cleaner to just make your desires clearer and
>> have the API define it well, and let the driver reject/accept it.
>>
>>> on legacy you are only allowed to select one antenna
>>> in the bitmap. if it is set to "0" (or a separate flag) this could enable
>>> "follow RX antenna diversity" on legacy.
>>
>> Sure that is one way, but it seems cleaner and easier for legacy purposes
>> to just define an API that only fits legacy.
>>
>>> most of the other things you mention (need a reset/reassociate, regulatory
>>> concerns...) are driver implementation issues, which can be dealt with in the
>>> driver.
>>
>> Well so some of these things *could* be handled in mac80211 as well. For
>> example, we may want to just dissociate upon a tx/rx chain setting change
>> for all devices, but not for legacy. The regulatory stuff is another thing
>> which could eventually be made more generic accross the board.
>>
>> Additionally, suppose you write an iw-tweak-gui thingy, and you want to
>> provide expose tx/rx chainmask settings. Since some cards do not support
>> some chainmask settings we may want to allow for a query of unsupported
>> chainmasks and that way the GUI application could just grey-out the
>> unsupported chainmask settings instead of letting the user figure out
>> by trial and error that they are indeed not supported.
> The API should just provide a bitmask of possible chains/antennas to
> user space, which will be used as a mask for any values that the user
> space sets. That's easy for a GUI utility to process

The bitmask of possible chains/antennas makes more sense, we could
just add it to the general phy info request, it would just be a matter
of piggy backing a new attribute back.

Luis

2010-05-20 22:02:29

by David Quan

[permalink] [raw]
Subject: RE: [ath5k-devel] [PATCH v2 13/20] cfg80211: Add nl80211 antenna configuration

rangemax is a proprietary design by netgear and their 3rd party, there is no way for us to control
their antenna array. yes, this is a legacy non 11n design with more than 2 antenna.


-----Original Message-----
From: [email protected] [mailto:[email protected]] On Behalf Of Luis R. Rodriguez
Sent: Wednesday, May 19, 2010 11:44 PM
To: Bruno Randolf; David Quan
Cc: [email protected]; [email protected]; [email protected]
Subject: Re: [ath5k-devel] [PATCH v2 13/20] cfg80211: Add nl80211 antenna configuration

On Wed, May 19, 2010 at 10:36 PM, Bruno Randolf <[email protected]> wrote:
> On Thursday 20 May 2010 14:17:19 you wrote:
>> None of the legacy 802.11 drivers we support have more than 2
>> antennas, I am also not aware of any.
>
> i have heard of some solutions based on atheros chipsets with more than 2
> antennas ("pre-11n RangeMax", "large phased array switch"). please check
> internally.

I will. David, are you aware of legacy (non 802.11n) devices with more
than 2 antennas?

>> But -- I can think of using an 802.11n device in legacy
>> mode of operation using specific antennas.... so there's your example
>> of a valid case for this.
>
> thanks! :)
>
>> Legacy devices are dead. I don't know anyone in the industry making
>> them, the 1 stream 802.11n devices are cheaper today, so there is no
>> point in the market for it.
>
> that might be true from a chipset manufacturers marketing perspective - but we
> work on the linux kernel... ;)
>
> as you know there are millions of so called "legacy" chipsets out there and
> people are going to continue to use them wether your marketing declares them
> "dead" or not... i think it's worth to properly support them.
>
> also please don't forget that some people use the linux kernel not just for
> standard use cases, but for research and developing new solutions. we should
> provide the flexibility to support that, if possible.

Heh, yeah but what goes into the Linux kernel are drivers for hardware
silicon companies make. Who is making new legacy chipsets still?

>> > and we are sure we don't want to support more than 2
>> > antennas - well, we could save 6 bits... is it really worth it?
>>
>> You're right, then if you really don't mind lets think 802.11n through
>> well then.
>
> i don't mind to do that, but as i said i dont know much about 802.11n yet.

Thanks, give me some time to think about this then and get back to you.

Luis

2010-05-19 01:33:16

by Bruno Randolf

[permalink] [raw]
Subject: [PATCH v2 16/20] ath5k: remove setting ANI and antenna thru debugfs files

Remove the possibility to set ANI parameters and antenna iconfiguration thru
debugfs, since we have proper interfaces for both now:

* /sys/class/ieee80211/phy0/device/ani/* for ANI

* "iw antenna set X" for antenna configuration.

Signed-off-by: Bruno Randolf <[email protected]>
---
drivers/net/wireless/ath/ath5k/debug.c | 33 +-------------------------------
1 files changed, 1 insertions(+), 32 deletions(-)

diff --git a/drivers/net/wireless/ath/ath5k/debug.c b/drivers/net/wireless/ath/ath5k/debug.c
index 97c5ada..d178d63 100644
--- a/drivers/net/wireless/ath/ath5k/debug.c
+++ b/drivers/net/wireless/ath/ath5k/debug.c
@@ -439,16 +439,7 @@ static ssize_t write_file_antenna(struct file *file,
if (copy_from_user(buf, userbuf, min(count, sizeof(buf))))
return -EFAULT;

- if (strncmp(buf, "diversity", 9) == 0) {
- ath5k_hw_set_antenna_mode(sc->ah, AR5K_ANTMODE_DEFAULT);
- printk(KERN_INFO "ath5k debug: enable diversity\n");
- } else if (strncmp(buf, "fixed-a", 7) == 0) {
- ath5k_hw_set_antenna_mode(sc->ah, AR5K_ANTMODE_FIXED_A);
- printk(KERN_INFO "ath5k debugfs: fixed antenna A\n");
- } else if (strncmp(buf, "fixed-b", 7) == 0) {
- ath5k_hw_set_antenna_mode(sc->ah, AR5K_ANTMODE_FIXED_B);
- printk(KERN_INFO "ath5k debug: fixed antenna B\n");
- } else if (strncmp(buf, "clear", 5) == 0) {
+ if (strncmp(buf, "clear", 5) == 0) {
for (i = 0; i < ARRAY_SIZE(sc->stats.antenna_rx); i++) {
sc->stats.antenna_rx[i] = 0;
sc->stats.antenna_tx[i] = 0;
@@ -694,28 +685,6 @@ static ssize_t write_file_ani(struct file *file,
ath5k_ani_init(sc->ah, ATH5K_ANI_MODE_OFF);
} else if (strncmp(buf, "ani-on", 6) == 0) {
ath5k_ani_init(sc->ah, ATH5K_ANI_MODE_AUTO);
- } else if (strncmp(buf, "noise-low", 9) == 0) {
- ath5k_ani_set_noise_immunity_level(sc->ah, 0);
- } else if (strncmp(buf, "noise-high", 10) == 0) {
- ath5k_ani_set_noise_immunity_level(sc->ah,
- ATH5K_ANI_MAX_NOISE_IMM_LVL);
- } else if (strncmp(buf, "spur-low", 8) == 0) {
- ath5k_ani_set_spur_immunity_level(sc->ah, 0);
- } else if (strncmp(buf, "spur-high", 9) == 0) {
- ath5k_ani_set_spur_immunity_level(sc->ah,
- sc->ani_state.max_spur_level);
- } else if (strncmp(buf, "fir-low", 7) == 0) {
- ath5k_ani_set_firstep_level(sc->ah, 0);
- } else if (strncmp(buf, "fir-high", 8) == 0) {
- ath5k_ani_set_firstep_level(sc->ah, ATH5K_ANI_MAX_FIRSTEP_LVL);
- } else if (strncmp(buf, "ofdm-off", 8) == 0) {
- ath5k_ani_set_ofdm_weak_signal_detection(sc->ah, false);
- } else if (strncmp(buf, "ofdm-on", 7) == 0) {
- ath5k_ani_set_ofdm_weak_signal_detection(sc->ah, true);
- } else if (strncmp(buf, "cck-off", 7) == 0) {
- ath5k_ani_set_cck_weak_signal_detection(sc->ah, false);
- } else if (strncmp(buf, "cck-on", 6) == 0) {
- ath5k_ani_set_cck_weak_signal_detection(sc->ah, true);
}
return count;
}


2010-05-19 17:07:46

by Luis R. Rodriguez

[permalink] [raw]
Subject: Re: [PATCH v2 13/20] cfg80211: Add nl80211 antenna configuration

On Tue, May 18, 2010 at 6:31 PM, Bruno Randolf <[email protected]> wrote:

> + * @NL80211_ATTR_ANTENNA_TX: Bitmap of antennas to use for transmitting.
> + * @NL80211_ATTR_ANTENNA_RX: Bitmap of antennas to use for receiving.

This gets the job done, but that's it. The API defined allows for a
hugely loose implementation on each driver. Users could end up using
for ath5k:

iw dev wlan0 set_tx_antenna 1 # for AR5K_ANTMODE_FIXED_A
iw dev wlan0 set_tx_antenna 2 # for AR5K_ANTMODE_FIXED_B
iw dev wlan0 set_tx_antenna 3 # for AR5K_ANTMODE_DEFAULT

And yet for another driver it could be something completely different
in usersepace. I think it would be better for us to define a static
API for all legacy cards and another for 802.11n cards, or unify them
but to be very specific about the API for antenna settings/chainmask
settings.

Luis

2010-05-19 13:00:12

by John W. Linville

[permalink] [raw]
Subject: Re: [PATCH v2 00/20] pending ath5k + antenna patches

On Wed, May 19, 2010 at 10:30:39AM +0900, Bruno Randolf wrote:

> Also i'd like to note that there are quite a few ath5k patches which haven't
> made it into linux-next yet. It would be great to get them into 2.6.35...

That probably won't happen. The exception would be for self-contained
bug fixes. As for linux-next, Stephen usually asks not to merge
anything into linux-next feeder trees during the merge window unless
they are intended to go to the current release. So, non-bugfix patches
probably won't be going to linux-next either for another week or so.

John
--
John W. Linville Someday the world will need a hero, and you
[email protected] might be all we have. Be ready.

2010-05-20 22:14:03

by Sam Ng

[permalink] [raw]
Subject: RE: [ath5k-devel] [PATCH v2 13/20] cfg80211: Add nl80211 antenna configuration

SnVzdCB0byBiZSBhY2N1cmF0ZSwgd2UgZG8gaGF2ZSBwcmUtbGxuIGRldmljZXMgdGhhdCBzdXBw
b3J0IG1vcmUgdGhhbiAxIGFudGVubmEgaWUuIHNsb3cvZmFzdCBkaXZlcnNpdHkgYW5kDQpyYW5n
ZW1heCBzdHlsZSAnYmVhbWZvcm1pbmcnLiBIb3dldmVyLCBhbGwgdGhlc2UgZGV2aWNlcyBoYXZl
IG9ubHkgMSBjaGFpbi4NCg0KU2FtDQoNCi0tLS0tT3JpZ2luYWwgTWVzc2FnZS0tLS0tDQpGcm9t
OiBtY2dyb2ZAZ21haWwuY29tIFttYWlsdG86bWNncm9mQGdtYWlsLmNvbV0gT24gQmVoYWxmIE9m
IEx1aXMgUi4gUm9kcmlndWV6DQpTZW50OiBUaHVyc2RheSwgTWF5IDIwLCAyMDEwIDM6MDYgUE0N
ClRvOiBCcnVubyBSYW5kb2xmOyBEYXZpZCBRdWFuOyBTYW0gTmcNCkNjOiBhdGg1ay1kZXZlbEBs
aXN0cy5hdGg1ay5vcmc7IGxpbnV4LXdpcmVsZXNzQHZnZXIua2VybmVsLm9yZzsgbGludmlsbGVA
dHV4ZHJpdmVyLmNvbQ0KU3ViamVjdDogUmU6IFthdGg1ay1kZXZlbF0gW1BBVENIIHYyIDEzLzIw
XSBjZmc4MDIxMTogQWRkIG5sODAyMTEgYW50ZW5uYSBjb25maWd1cmF0aW9uDQoNCk9uIFdlZCwg
TWF5IDE5LCAyMDEwIGF0IDExOjQzIFBNLCBMdWlzIFIuIFJvZHJpZ3Vleg0KPGxyb2RyaWd1ZXpA
YXRoZXJvcy5jb20+IHdyb3RlOg0KPiBPbiBXZWQsIE1heSAxOSwgMjAxMCBhdCAxMDozNiBQTSwg
QnJ1bm8gUmFuZG9sZiA8YnIxQGVpbmZhY2gub3JnPiB3cm90ZToNCj4+IE9uIFRodXJzZGF5IDIw
IE1heSAyMDEwIDE0OjE3OjE5IHlvdSB3cm90ZToNCj4+PiBOb25lIG9mIHRoZSBsZWdhY3kgODAy
LjExIGRyaXZlcnMgd2Ugc3VwcG9ydCBoYXZlIG1vcmUgdGhhbiAyDQo+Pj4gYW50ZW5uYXMsIEkg
YW0gYWxzbyBub3QgYXdhcmUgb2YgYW55Lg0KPj4NCj4+IGkgaGF2ZSBoZWFyZCBvZiBzb21lIHNv
bHV0aW9ucyBiYXNlZCBvbiBhdGhlcm9zIGNoaXBzZXRzIHdpdGggbW9yZSB0aGFuIDINCj4+IGFu
dGVubmFzICgicHJlLTExbiBSYW5nZU1heCIsICJsYXJnZSBwaGFzZWQgYXJyYXkgc3dpdGNoIiku
IHBsZWFzZSBjaGVjaw0KPj4gaW50ZXJuYWxseS4NCj4NCj4gSSB3aWxsLiBEYXZpZCwgYXJlIHlv
dSBhd2FyZSBvZiBsZWdhY3kgKG5vbiA4MDIuMTFuKSBkZXZpY2VzIHdpdGggbW9yZQ0KPiB0aGFu
IDIgYW50ZW5uYXM/DQoNCkkgcGlja2VkIERhdmlkJ3MgYnJhaW4gYW5kIHdlJ3JlIHByZXR0eSBj
ZXJ0YWluIHRoZXNlIGRldmljZXMgZG8gbm90DQpleGlzdCBpbiB0aGUgbWFya2V0Lg0KDQo+Pj4g
WW91J3JlIHJpZ2h0LCB0aGVuIGlmIHlvdSByZWFsbHkgZG9uJ3QgbWluZCBsZXRzIHRoaW5rIDgw
Mi4xMW4gdGhyb3VnaA0KPj4+IHdlbGwgdGhlbi4NCj4+DQo+PiBpIGRvbid0IG1pbmQgdG8gZG8g
dGhhdCwgYnV0IGFzIGkgc2FpZCBpIGRvbnQga25vdyBtdWNoIGFib3V0IDgwMi4xMW4geWV0Lg0K
Pg0KPiBUaGFua3MsIGdpdmUgbWUgc29tZSB0aW1lIHRvIHRoaW5rIGFib3V0IHRoaXMgdGhlbiBh
bmQgZ2V0IGJhY2sgdG8geW91Lg0KDQpPSyBzbyBiYWNrIHRvIHRoZSBkcmF3aW5nIGJvYXJkLCB0
aGlzIGlzIHdoYXQgSSByZWNvbW1lbmQ6DQoNCkZvciBsZWdhY3ksIGtlZXAgaXQgc2ltcGxlLCB1
c2UgMyBzZXR0aW5ncywgZml4ZWRfYSwgZml4ZWRfYiwNCmRpdmVyc2l0eSwgZm9yIGFsbCBkZXZp
Y2VzLg0KDQpMZXRzIHVzZSBhIGRpZmZlcmVudCBBUEkgZm9yIDgwMi4xMW4uIFJlYXNvbiBiZWlu
ZyB0aGF0IGV2ZW4gdGhlIGNhc2UNCkkgbWVudGlvbmVkIG9mIGFuIDgwMi4xMW4gZGV2aWNlIGNv
bm5lY3Rpbmcgb24gYSBsZWdhY3kgbmV0d29yayBuZWVkcw0KdG8gYmUgdHJlYXRlZCBkaWZmZXJl
bnRseSBhY3R1YWxseS4NCg0KRm9yIDgwMi4xMW4geW91IGhhdmUgYSBmZXcgbW9yZSBjb25zaWRl
cmF0aW9ucy4gWW91IGNhbiBhY3R1YWxseSBUWCBhdA0KdGhlIHNhbWUgdGltZSBvbiB0d28gb3Ig
bW9yZSBkaWZmZXJlbnQgYW50ZW5uYXMgYXQgdGhlIHNhbWUgdGltZS4gVGhlDQpkYXRhIHlvdSB0
cmFuc21pdCB3aWxsIGJlIHRoZSBjb250ZW50cyBvbiBib3RoIGNoYWlucyBvbiBhIGR1YWwgc3Ry
ZWFtDQpkZXZpY2UuIFNvIGJvdGggYW50ZW5uYSAwIGFuZCBhbnRlbm5hIDEgd2lsbCBib3RoIGJl
IHRyYW5zbWl0dGluZyB0aGUNCmRhdGEgZm9yIGJvdGggc3RyZWFtIDAgYW5kIHN0cmVhbSAxLiBB
cyBpdCB0dXJucyBvdXQgdGhlIGNvbWJpbmF0aW9uDQpvZiBUWCdpbmcgb24gdHdvIGFudGVubmFz
IGF0IHRoZSBzYW1lIHRpbWUgYXQgYSBjZXJ0YWluIGRCbSBwb3dlciB3aWxsDQp5aWVsZCBhIGhp
Z2hlciByZWNlaXZlZCBmcmFtZSBvbiB0aGUgUlggc2lkZS4gVGhpcyBpcyB3aHkgd2hlbiB5b3Ug
dXNlDQptdWx0aXBsZSBjaGFpbnMgeW91IGhhdmUgdG8gdGFrZSByZWd1bGF0b3J5IHJ1bGVzIGlu
dG8gY29uc2lkZXJhdGlvbnMNCmFzIHdlbGwsIHNpbmNlIGFkZGluZyBtb3JlIGNoYWlucyB3aWxs
IGluY3JlYXNlIHRoZSBvdmVyYWxsIG91dHB1dA0KcG93ZXIuIFRvZGF5IGF0aDlrIGhhbmRsZXMg
dGhpcyBpdHNlbGYgc2luY2UgdGhpcyBkYXRhIGlzIGNhbGlicmF0ZWQNCmJ1dCB0aGUgbWF4IEVJ
UlAgaXMgcGFzc2VkIG91dCBmcm9tIGNmZzgwMjExLiBEZXZpY2VzIHdoaWNoIGRvIG5vdA0KZGVh
bCB3aXRoIHRoZXNlIHJlZ3VsYXRvcnkgY29uc2lkZXJhdGlvbnMgbGlrZWx5IHdvbid0IHN1cHBv
cnQNCmNoYW5naW5nIGNoYWlubWFza3MgdW5sZXNzIHRoZXkgdXNlIGFuIEFQSSB0byByZXNwZWN0
IHJlZ3VsYXRvcnkNCmludGVybmFsbHkgc29tZWhvdy4gUGVyaGFwcyB0aGUgaXdsYWduIGZpcm13
YXJlIGRvZXMgdGhpcywgYmVhdHMgbWUuDQoNClRoZSByaWdodCB0ZXJtaW5vbG9neSBmb3IgYW50
ZW5uYSBjb250cm9sIGZvciBib3RoIFRYIGFuZCBSWCBpcw0KY2hhaW5tYXNrIGFuZCBhIGJpdG1h
cCBvZiA4IHdpbGwgc3VmZmljZSBmb3IgZXhpc3RpbmcgaGFyZHdhcmUgYW5kIHVwDQp0byB0aGUg
bm90LXlldC1leGlzdGFudCA2MDBtYnBzIDQgc3RyZWFtIGRldmljZXMuIFN1cHBvcnRpbmcgOCBi
aXRzDQp3aWxsIHN1cHBvcnQgdXAgdG8gOCBzdHJlYW1zIGFuZCB3ZSBkbyBub3QgZW52aXNpb24g
dXNpbmcgYmV5b25kIHRoYXQNCmF0IHRoaXMgcG9pbnQuIFRoZXJlIGlzIHNvbWUgY29uc2lkZXJh
dGlvbnMgaW4gdGhlIGZ1dHVyZSBmb3INCnN1cHBvcnRpbmcgc29tZXRoaW5nIG90aGVyIHRoYW4g
SFQ0MCwgbGlrZSBtYXliZSBIVDgwIGFuZCBzbyBmb3J0aCBidXQNCnRob3NlIHRoaW5ncyB3b24n
dCBiZSB1c2luZyBtb3JlIHN0cmVhbXMgaXQgc2VlbXMuDQoNClRoZW4sIHNvbWUgZGV2aWNlcyB3
b24ndCBzdXBwb3J0IGFsbCBwb3NzaWJsZSBjaGFpbm1hc2sgc2V0dGluZ3MuIFRoaXMNCndpbGwg
dmFyeSBkZXBlbmRpbmcgb24gdGhlIGNoaXBzZXQuIEkgd29yayBmb3IgQXRoZXJvcyBzbyBJIGNh
biBvbmx5DQp0ZWxsIHlvdSB3aGF0IHdlIGNhbiBzdXBwb3J0LCB3ZSdsbCBoYXZlIHRvIGNoZWNr
IHdpdGggdGhlIEludGVsIGZvbGtzDQphYm91dCB0aGVpciBjaGlwc2V0IGxpbWl0YXRpb25zIGFu
ZCBzZXR0aW5ncy4NCg0KQVI1NDE2LCBBUjU0MTggY2FuIG9ubHkgc3VwcG9ydCBjaGFpbm1hc2sg
c2V0dGluZ3Mgd2hpY2ggYWx3YXlzIGtlZXANCnRoZSBmaXJzdCBjaGFpbiBvbi4gVGhlIEFSOTAw
MSBmYW1pbHkgYW5kIGJleW9uZCBjYW5ub3Qgc3VwcG9ydCB0aGUNCjBiMTEwIGNoYWltYXNrIChE
YXZpZCwgeW91IGhhZCBwb2ludGVkIG91dCBzb21lIG90aGVyIHJlc3RyaWN0aW9ucywNCndoYXQg
d2VyZSB0aGV5IGFnYWluPyksIHRoZSBkZXRhaWxzIGFyZSBjb21wbGV4IGFuZCBJIGRpZCBub3Qg
Z2V0IGENCmNoYW5jZSB0byByZXZpZXcgdGhlbS4NCg0KSSB3b3VsZCBub3QgYmUgc3VycHJpc2Vk
IGlmIG90aGVyIHZlbmRvcnMgaGFkIHNpbWlsYXIgcmVzdHJpY3Rpb25zIHNvDQpJJ20gdGhpbmtp
bmcgbWF5YmUgd2UgY2FuIGV4cHJlc3MgdGhpcyBhcyBhIHJlcXVpcmVtZW50IG1hc2ssIG9yIGEg
c2V0DQpvZiByZXF1aXJlbWVudCBtYXNrcy4gVGhpcyB3YXkgdXNlcnNwYWNlIHV0aWxpdGllcyBm
b3IgZGVidWdnaW5nIHdvdWxkDQpvbmx5IGV4cG9zZSBjZXJ0YWluIGNoYWlubWFzayBzZXR0aW5n
cy4NCg0KTm93IHRlY2huaWNhbGx5IHRoZW4geW91IGNhbiBpbmNvcnBvcmF0ZSB0aGUgbGVnYWN5
IEFQSSB3aXRoIHRoZQ0KODAyLjExbiBBUEkgaGVyZSBzb21laG93IGJ1dCBpdCBqdXN0IHNlZW1z
IGNsZWFuZXIgdG8ga2VlcCB0aGVtDQpzZXBhcmF0ZS4NCg0KQWxzbywgRGF2aWQgaW5kaWNhdGVk
IHRoYXQgd2hlbiB3ZSBjaGFuZ2UgdGhlIGNoYWlubWFzayB3aGVuIGFyZSBhcmUNCmFzc29jaWF0
ZWQgd2UgaGF2ZSB0byBkbyBhbiBhY3R1YWwgY2hpcCByZXNldCwgdGhpcyBpcyBkaWZmZXJlbnQg
dGhhbg0KdGhlIGFudGVubmEgZGl2ZXJzaXR5IHNldHRpbmdzIHdoaWNoIGFuIGJlIGRvbmUgb24g
dGhlIGZseS4gV2UgbGlrZWx5DQp3aWxsIG5lZWQgdG8gcmVhc3NvY2lhdGUgZm9yIGEgY2hhaW5t
YXNrIHNldHRpbmcsIG5vdCBzdXJlLg0KDQogIEx1aXMNCg==

2010-05-19 01:32:50

by Bruno Randolf

[permalink] [raw]
Subject: [PATCH v2 11/20] ath5k: always calculate ANI listen time

Calculate 'listen' time also when automatic ANI is off, since this and the
"busy" time is useful information also in manual mode.

Signed-off-by: Bruno Randolf <[email protected]>
---
drivers/net/wireless/ath/ath5k/ani.c | 9 +++++----
1 files changed, 5 insertions(+), 4 deletions(-)

diff --git a/drivers/net/wireless/ath/ath5k/ani.c b/drivers/net/wireless/ath/ath5k/ani.c
index f2311ab..987e3d3 100644
--- a/drivers/net/wireless/ath/ath5k/ani.c
+++ b/drivers/net/wireless/ath/ath5k/ani.c
@@ -481,14 +481,15 @@ ath5k_ani_calibration(struct ath5k_hw *ah)
struct ath5k_ani_state *as = &ah->ah_sc->ani_state;
int listen, ofdm_high, ofdm_low, cck_high, cck_low;

- if (as->ani_mode != ATH5K_ANI_MODE_AUTO)
- return;
-
/* get listen time since last call and add it to the counter because we
- * might not have restarted the "ani period" last time */
+ * might not have restarted the "ani period" last time.
+ * always do this to calculate the busy time also in manual mode */
listen = ath5k_hw_ani_get_listen_time(ah, as);
as->listen_time += listen;

+ if (as->ani_mode != ATH5K_ANI_MODE_AUTO)
+ return;
+
ath5k_ani_save_and_clear_phy_errors(ah, as);

ofdm_high = as->listen_time * ATH5K_ANI_OFDM_TRIG_HIGH / 1000;


2010-05-20 01:26:52

by Luis R. Rodriguez

[permalink] [raw]
Subject: Re: [ath5k-devel] [PATCH v2 13/20] cfg80211: Add nl80211 antenna configuration

On Wed, May 19, 2010 at 6:12 PM, Bruno Randolf <[email protected]> wrote:
> On Thursday 20 May 2010 09:51:43 Luis R. Rodriguez wrote:
>> On Wed, May 19, 2010 at 05:35:40PM -0700, Bruno Randolf wrote:
>> > On Thursday 20 May 2010 02:07:25 you wrote:
>> > > On Tue, May 18, 2010 at 6:31 PM, Bruno Randolf <[email protected]> wrote:
>> > > > + * @NL80211_ATTR_ANTENNA_TX: Bitmap of antennas to use for
>> > > > transmitting. + * @NL80211_ATTR_ANTENNA_RX: Bitmap of antennas to
>> > > > use for receiving.
>> > >
>> > > This gets the job done, but that's it. The API defined allows for a
>> > > hugely loose implementation on each driver.
>> >
>> > i tried to define it like this, in the commit log:
>> >     The antenna configuration is defined as a bitmap of allowed antennas.
>> >     This bitmap is 8 bit at the moment, each bit representing one
>> >     antenna. If multiple antennas are selected, the driver may use
>> >     diversity for receive and transmit.
>> >
>> > is this not precise enough?
>>
>> No, the commit log is just a placeholder of information, if you want to
>> define API you do it through the kdoc so you can slap people when then
>> submit patches that do not follow it. The kdoc above allows the flexibility
>> you were looking for but that I do not think we should have since it will
>> confuse the users who want to tune antenna settings on different drivers.
>
> you are talking about the place where to put the definition, not about the
> definition itself. i agree, the definition should be in the kdoc, and i'll
> update the patch. what's wrong with the definition itself?

Why are you using a bitmask for only 3 possibly different settings?

>> Understood, how about just defining something very basic and simple
>> for legacy based operation mode?
>
> i think my implementation is quite basic and simple ;)

Oh it is, I think it can be much simpler though.

>> > > And yet for another driver it could be something completely different
>> > > in usersepace.
>> >
>> > what do you think that could be, realistically, given the above
>> > definition?
>>
>> Well, anything that has to do with tx / rx antennas.
>
> that's not very clear. can you give me an example?

iw dev wlan0 set_tx_antenna 4

>> > > I think it would be better for us to define a static
>> > > API for all legacy cards and another for 802.11n cards, or unify them
>> > > but to be very specific about the API for antenna settings/chainmask
>> > > settings.
>> >
>> > sure. any suggestions?
>>
>> Sure how about FIXED_A, FIXED_B, DIVERSITY ?
>
> that's very ath5k centric.
>
> what if there is a 'legacy' hardware with 3 or more antennas?

Like what?

> what if we want to configure RX on antenna 1, TX on antenna 2?

Are you not using a value for TX and RX? Would that now allow for this?

> what if we want to use RX diversity but always TX on a fixed antenna?

You have this for both, RX and TX, and specify that on the kdoc (?)

> these are possible and useful configurations, which are not supported right
> now by ath5k but it's easy to add them.
>
> i don't see how "my" API is too complicated, and i think it allows for a clear
> configuration of these cases as well.
>
> your criticism seems to be based on the fact that it's not clear how to handle
> 802.11n chainmask + antenna configuration, but this is not what my patch is
> concerned about. let's go step by step...

No no, that is my fault, I brought that up, I was hoping we could
address it but it seems that we can't as I don't have time to think
about this further in a unified clean API. But if its just going to be
legacy then I don't see why we would use a large bitmap.

Luis

2010-05-19 01:32:24

by Bruno Randolf

[permalink] [raw]
Subject: [PATCH v2 06/20] ath5k: run NF calibration only every 60 seconds

Since NF calibration interferes with TX and RX and also has been the cause of
other problems (when it's run concurrently with ath5k_reset) we want to run it
less often - every 60 seconds for now.

Signed-off-by: Bruno Randolf <[email protected]>
---
drivers/net/wireless/ath/ath5k/ath5k.h | 2 ++
drivers/net/wireless/ath/ath5k/base.c | 18 +++++++++++-------
2 files changed, 13 insertions(+), 7 deletions(-)

diff --git a/drivers/net/wireless/ath/ath5k/ath5k.h b/drivers/net/wireless/ath/ath5k/ath5k.h
index 131e8b3..d6f9afe 100644
--- a/drivers/net/wireless/ath/ath5k/ath5k.h
+++ b/drivers/net/wireless/ath/ath5k/ath5k.h
@@ -204,6 +204,7 @@
#define AR5K_TUNE_TPC_TXPOWER false
#define ATH5K_TUNE_CALIBRATION_INTERVAL_FULL 10000 /* 10 sec */
#define ATH5K_TUNE_CALIBRATION_INTERVAL_ANI 1000 /* 1 sec */
+#define ATH5K_TUNE_CALIBRATION_INTERVAL_NF 60000 /* 60 sec */

#define AR5K_INIT_CARR_SENSE_EN 1

@@ -1118,6 +1119,7 @@ struct ath5k_hw {
/* Calibration timestamp */
unsigned long ah_cal_next_full;
unsigned long ah_cal_next_ani;
+ unsigned long ah_cal_next_nf;

/* Calibration mask */
u8 ah_cal_mask;
diff --git a/drivers/net/wireless/ath/ath5k/base.c b/drivers/net/wireless/ath/ath5k/base.c
index 3206ed6..496e2db 100644
--- a/drivers/net/wireless/ath/ath5k/base.c
+++ b/drivers/net/wireless/ath/ath5k/base.c
@@ -2802,14 +2802,16 @@ ath5k_tasklet_calibrate(unsigned long data)
ieee80211_frequency_to_channel(
sc->curchan->center_freq));

- /* TODO: We don't need to run noise floor calibration as often
- * as I/Q calibration.*/
-
/* Noise floor calibration interrupts rx/tx path while I/Q calibration
- * doesn't. Stop queues so that calibration doesn't interfere with tx */
- ieee80211_stop_queues(sc->hw);
- ath5k_hw_update_noise_floor(ah);
- ieee80211_wake_queues(sc->hw);
+ * doesn't. We stop the queues so that calibration doesn't interfere
+ * with TX and don't run it as often */
+ if (time_is_before_eq_jiffies(ah->ah_cal_next_nf)) {
+ ah->ah_cal_next_nf = jiffies +
+ msecs_to_jiffies(ATH5K_TUNE_CALIBRATION_INTERVAL_NF);
+ ieee80211_stop_queues(sc->hw);
+ ath5k_hw_update_noise_floor(ah);
+ ieee80211_wake_queues(sc->hw);
+ }

ah->ah_cal_mask &= ~AR5K_CALIBRATION_FULL;
}
@@ -2930,6 +2932,8 @@ ath5k_reset(struct ath5k_softc *sc, struct ieee80211_channel *chan)

ah->ah_cal_next_full = jiffies;
ah->ah_cal_next_ani = jiffies;
+ ah->ah_cal_next_nf = jiffies;
+
/*
* Change channels and update the h/w rate map if we're switching;
* e.g. 11a to 11b/g.


2010-05-20 00:35:48

by Bruno Randolf

[permalink] [raw]
Subject: Re: [PATCH v2 13/20] cfg80211: Add nl80211 antenna configuration

On Thursday 20 May 2010 02:07:25 you wrote:
> On Tue, May 18, 2010 at 6:31 PM, Bruno Randolf <[email protected]> wrote:
> > + * @NL80211_ATTR_ANTENNA_TX: Bitmap of antennas to use for transmitting.
> > + * @NL80211_ATTR_ANTENNA_RX: Bitmap of antennas to use for receiving.
>
> This gets the job done, but that's it. The API defined allows for a
> hugely loose implementation on each driver.

i tried to define it like this, in the commit log:

The antenna configuration is defined as a bitmap of allowed antennas. This
bitmap is 8 bit at the moment, each bit representing one antenna. If
multiple antennas are selected, the driver may use diversity for receive
and transmit.

is this not precise enough? i am mostly concerned with what i believe is the
most common usecase (selecting one fixed antenna, or antenna diversity). i'd
say this is 99% of all use cases. but this API would also allow us to define
more advanved things like 'transmit on antenna 1, receive on antenna 2". i
know that there are possibly more crazy (and very rare) configurations, like
use several panel antennas + omni, which can't be configured with this API,
but it's hard to find a common API for all possibilities, and i think it would
be possible to extend the API later on for such cases.

> And yet for another driver it could be something completely different
> in usersepace.

what do you think that could be, realistically, given the above definition?

> I think it would be better for us to define a static
> API for all legacy cards and another for 802.11n cards, or unify them
> but to be very specific about the API for antenna settings/chainmask
> settings.

sure. any suggestions?

bruno

2010-05-19 01:33:06

by Bruno Randolf

[permalink] [raw]
Subject: [PATCH v2 14/20] mac80211: Add antenna configuration

Allow antenna configuration by calling driver's function for it.

Signed-off-by: Bruno Randolf <[email protected]>
---
include/net/mac80211.h | 2 ++
net/mac80211/cfg.c | 16 ++++++++++++++
net/mac80211/driver-ops.h | 23 ++++++++++++++++++++
net/mac80211/driver-trace.h | 50 +++++++++++++++++++++++++++++++++++++++++++
4 files changed, 91 insertions(+), 0 deletions(-)

diff --git a/include/net/mac80211.h b/include/net/mac80211.h
index 389e86a..d60edd8 100644
--- a/include/net/mac80211.h
+++ b/include/net/mac80211.h
@@ -1723,6 +1723,8 @@ struct ieee80211_ops {
void (*flush)(struct ieee80211_hw *hw, bool drop);
void (*channel_switch)(struct ieee80211_hw *hw,
struct ieee80211_channel_switch *ch_switch);
+ int (*set_antenna)(struct ieee80211_hw *hw, u8 tx_ant, u8 rx_ant);
+ int (*get_antenna)(struct ieee80211_hw *hw, u8 *tx_ant, u8 *rx_ant);
};

/**
diff --git a/net/mac80211/cfg.c b/net/mac80211/cfg.c
index c7000a6..efd04bc 100644
--- a/net/mac80211/cfg.c
+++ b/net/mac80211/cfg.c
@@ -1560,6 +1560,20 @@ static int ieee80211_action(struct wiphy *wiphy, struct net_device *dev,
channel_type, buf, len, cookie);
}

+static int ieee80211_set_antenna(struct wiphy *wiphy, u8 tx_ant, u8 rx_ant)
+{
+ struct ieee80211_local *local = wiphy_priv(wiphy);
+
+ return drv_set_antenna(local, tx_ant, rx_ant);
+}
+
+static int ieee80211_get_antenna(struct wiphy *wiphy, u8 *tx_ant, u8 *rx_ant)
+{
+ struct ieee80211_local *local = wiphy_priv(wiphy);
+
+ return drv_get_antenna(local, tx_ant, rx_ant);
+}
+
struct cfg80211_ops mac80211_config_ops = {
.add_virtual_intf = ieee80211_add_iface,
.del_virtual_intf = ieee80211_del_iface,
@@ -1611,4 +1625,6 @@ struct cfg80211_ops mac80211_config_ops = {
.cancel_remain_on_channel = ieee80211_cancel_remain_on_channel,
.action = ieee80211_action,
.set_cqm_rssi_config = ieee80211_set_cqm_rssi_config,
+ .set_antenna = ieee80211_set_antenna,
+ .get_antenna = ieee80211_get_antenna,
};
diff --git a/net/mac80211/driver-ops.h b/net/mac80211/driver-ops.h
index 5662bb5..9974383 100644
--- a/net/mac80211/driver-ops.h
+++ b/net/mac80211/driver-ops.h
@@ -384,4 +384,27 @@ static inline void drv_channel_switch(struct ieee80211_local *local,
trace_drv_channel_switch(local, ch_switch);
}

+
+static inline int drv_set_antenna(struct ieee80211_local *local,
+ u8 tx_ant, u8 rx_ant)
+{
+ int ret = -EOPNOTSUPP;
+ might_sleep();
+ if (local->ops->set_antenna)
+ ret = local->ops->set_antenna(&local->hw, tx_ant, rx_ant);
+ trace_drv_set_antenna(local, tx_ant, rx_ant, ret);
+ return ret;
+}
+
+static inline int drv_get_antenna(struct ieee80211_local *local,
+ u8 *tx_ant, u8 *rx_ant)
+{
+ int ret = -EOPNOTSUPP;
+ might_sleep();
+ if (local->ops->get_antenna)
+ ret = local->ops->get_antenna(&local->hw, tx_ant, rx_ant);
+ trace_drv_get_antenna(local, *tx_ant, *rx_ant, ret);
+ return ret;
+}
+
#endif /* __MAC80211_DRIVER_OPS */
diff --git a/net/mac80211/driver-trace.h b/net/mac80211/driver-trace.h
index 6a9b234..8f08ff3 100644
--- a/net/mac80211/driver-trace.h
+++ b/net/mac80211/driver-trace.h
@@ -802,6 +802,56 @@ TRACE_EVENT(drv_channel_switch,
)
);

+TRACE_EVENT(drv_set_antenna,
+ TP_PROTO(struct ieee80211_local *local, u8 tx_ant, u8 rx_ant, int ret),
+
+ TP_ARGS(local, tx_ant, rx_ant, ret),
+
+ TP_STRUCT__entry(
+ LOCAL_ENTRY
+ __field(u8, tx_ant)
+ __field(u8, rx_ant)
+ __field(int, ret)
+ ),
+
+ TP_fast_assign(
+ LOCAL_ASSIGN;
+ __entry->tx_ant = tx_ant;
+ __entry->rx_ant = rx_ant;
+ __entry->ret = ret;
+ ),
+
+ TP_printk(
+ LOCAL_PR_FMT " tx_ant:%d rx_ant:%d ret:%d",
+ LOCAL_PR_ARG, __entry->tx_ant, __entry->rx_ant, __entry->ret
+ )
+);
+
+TRACE_EVENT(drv_get_antenna,
+ TP_PROTO(struct ieee80211_local *local, u8 tx_ant, u8 rx_ant, int ret),
+
+ TP_ARGS(local, tx_ant, rx_ant, ret),
+
+ TP_STRUCT__entry(
+ LOCAL_ENTRY
+ __field(u8, tx_ant)
+ __field(u8, rx_ant)
+ __field(int, ret)
+ ),
+
+ TP_fast_assign(
+ LOCAL_ASSIGN;
+ __entry->tx_ant = tx_ant;
+ __entry->rx_ant = rx_ant;
+ __entry->ret = ret;
+ ),
+
+ TP_printk(
+ LOCAL_PR_FMT " tx_ant:%d rx_ant:%d ret:%d",
+ LOCAL_PR_ARG, __entry->tx_ant, __entry->rx_ant, __entry->ret
+ )
+);
+
/*
* Tracing for API calls that drivers call.
*/


2010-05-20 02:21:59

by Bruno Randolf

[permalink] [raw]
Subject: Re: [ath5k-devel] [PATCH v2 13/20] cfg80211: Add nl80211 antenna configuration

On Thursday 20 May 2010 10:26:29 you wrote:
> >> > > > + * @NL80211_ATTR_ANTENNA_TX: Bitmap of antennas to use for
> >> > > > transmitting. + * @NL80211_ATTR_ANTENNA_RX: Bitmap of antennas to
> >> > > > use for receiving.
> >> > >
> >> > > This gets the job done, but that's it. The API defined allows for a
> >> > > hugely loose implementation on each driver.
> >> >
> >> > i tried to define it like this, in the commit log:
> >> > The antenna configuration is defined as a bitmap of allowed
> >> > antennas. This bitmap is 8 bit at the moment, each bit representing
> >> > one antenna. If multiple antennas are selected, the driver may use
> >> > diversity for receive and transmit.
> >> >
> >> > is this not precise enough?
> >>
> >> No, the commit log is just a placeholder of information, if you want to
> >> define API you do it through the kdoc so you can slap people when then
> >> submit patches that do not follow it. The kdoc above allows the
> >> flexibility you were looking for but that I do not think we should have
> >> since it will confuse the users who want to tune antenna settings on
> >> different drivers.
> >
> > you are talking about the place where to put the definition, not about
> > the definition itself. i agree, the definition should be in the kdoc,
> > and i'll update the patch. what's wrong with the definition itself?
>
> Why are you using a bitmask for only 3 possibly different settings?

because there are more than 3 different settings, like i mentioned at the end
of my last mail.

so, just talking about ath5k, right now we support only the 3 different
settings, you mentioned (fixed a, fixed b, default: diversity), true. other
drivers might support a different number of settings though, so just assuming
everyone follows these 3 definitions would be not enough, imho.

and it's very easy to add to ath5k:
- "RX on A, TX on B" or "RX on B, TX on A" (this makes sense if you want to
use a "big" (high dB) antenna for listening, but use a "small" (low dB)
antenna for sending within the regulatory limits).
- TX on a fixed antenna, while using RX diversity

that's why i decided for a bitmap.

> >> > > And yet for another driver it could be something completely
> >> > > different in usersepace.
> >> >
> >> > what do you think that could be, realistically, given the above
> >> > definition?
> >>
> >> Well, anything that has to do with tx / rx antennas.
> >
> > that's not very clear. can you give me an example?
>
> iw dev wlan0 set_tx_antenna 4

so you want to transmit on antenna 3. if the card has 3 antennas -
why not?

> > what if there is a 'legacy' hardware with 3 or more antennas?
>
> Like what?

i can't find an actual real world example for that, but it thought it might be
possible, from what i know from the atheros eeprom which seems to be prepared
for up to 6 antennas.

> > what if we want to configure RX on antenna 1, TX on antenna 2?
>
> Are you not using a value for TX and RX? Would that now allow for this?

there are different values for TX and RX, so it would allow for that.

> > i don't see how "my" API is too complicated, and i think it allows for a
> > clear configuration of these cases as well.
> >
> > your criticism seems to be based on the fact that it's not clear how to
> > handle 802.11n chainmask + antenna configuration, but this is not what
> > my patch is concerned about. let's go step by step...
>
> No no, that is my fault, I brought that up, I was hoping we could
> address it but it seems that we can't as I don't have time to think
> about this further in a unified clean API. But if its just going to be
> legacy then I don't see why we would use a large bitmap.

i wanted it to be easily extensibe for 802.11n and possibly 'legacy' with more
antennas in the future. if there really are not any pre-N cards with more than
2 antennas out there, and we are sure we don't want to support more than 2
antennas - well, we could save 6 bits... is it really worth it?

bruno

2010-05-21 19:10:47

by Felix Fietkau

[permalink] [raw]
Subject: Re: [ath5k-devel] [PATCH v2 13/20] cfg80211: Add nl80211 antenna configuration

On 2010-05-21 7:11 PM, Luis R. Rodriguez wrote:
> On Thu, May 20, 2010 at 06:59:33PM -0700, Bruno Randolf wrote:
>> so from my point of view this is not very different from what we can support
>> with the API i suggested. for RX it seems to be 100% equivalent.
>
> Well I agree, the API *supports it* but I want *clean, clear and consistant
> API*. And it just seems cleaner to separate the two.
>
>> the main difference as i see it is that with 802.11n you transmit on more than
>> one antenna, while with 'legacy' we can only transmit on one antenna at a
>> time.
>
> True, but note how the fact that you transmit over two antennas actually
> has regulatory implications. Now, ath9k handles this within ath9k_hw already
> but this itself seems like a worthy reason for this API to be separated.
> While I think it is great for ath9k_hw to do this, wouldn't it be nice
> if we can eventually instead expose the gain by using different chains
> at the same time and do the regulatory calculation for all devices within
> cfg80211?
>
>> actually i have to admit that on legacy "antenna set tx 3 (b11)" (select two
>> antennas for transmit) does not make much sense. i have defined it before as
>> "use diversity" but what about a different definition: like "bitmap of
>> antennas/chains to TRANSMIT".
>
> Right, and while that *works*, I think it would be clearer to just use a
> clear "diveristy" knob.
Splitting it by mode of operation (11n vs legacy) does not work, because
in AP mode you're doing both at the same time and there is an overlap in
both settings.
I think that Bruno's suggestion of keeping them as one setting makes
sense. About the regulatory concerns: where in the code does the
chainmask currently affect the regulatory constraints?

>> so for 802.11n that would allow you to select multiple trasmit chains.
>
> Instead of leaving the API to be interpreted by the mode of operation
> I think it would be much cleaner to just make your desires clearer and
> have the API define it well, and let the driver reject/accept it.
>
>> on legacy you are only allowed to select one antenna
>> in the bitmap. if it is set to "0" (or a separate flag) this could enable
>> "follow RX antenna diversity" on legacy.
>
> Sure that is one way, but it seems cleaner and easier for legacy purposes
> to just define an API that only fits legacy.
>
>> most of the other things you mention (need a reset/reassociate, regulatory
>> concerns...) are driver implementation issues, which can be dealt with in the
>> driver.
>
> Well so some of these things *could* be handled in mac80211 as well. For
> example, we may want to just dissociate upon a tx/rx chain setting change
> for all devices, but not for legacy. The regulatory stuff is another thing
> which could eventually be made more generic accross the board.
>
> Additionally, suppose you write an iw-tweak-gui thingy, and you want to
> provide expose tx/rx chainmask settings. Since some cards do not support
> some chainmask settings we may want to allow for a query of unsupported
> chainmasks and that way the GUI application could just grey-out the
> unsupported chainmask settings instead of letting the user figure out
> by trial and error that they are indeed not supported.
The API should just provide a bitmask of possible chains/antennas to
user space, which will be used as a mask for any values that the user
space sets. That's easy for a GUI utility to process

- Felix

2010-05-20 12:56:59

by Nick Kossifidis

[permalink] [raw]
Subject: Re: [PATCH v2 07/20] ath5k: remove ATH_TRACE macro

2010/5/19 Bruno Randolf <[email protected]>:
> Now that we have ftrace, it is not needed any more.
>
> Signed-off-by: Bruno Randolf <[email protected]>
> ---
>  drivers/net/wireless/ath/ath5k/attach.c |    2 --
>  drivers/net/wireless/ath/ath5k/caps.c   |    7 -------
>  drivers/net/wireless/ath/ath5k/debug.c  |    1 -
>  drivers/net/wireless/ath/ath5k/debug.h  |    8 --------
>  drivers/net/wireless/ath/ath5k/desc.c   |    7 -------
>  drivers/net/wireless/ath/ath5k/dma.c    |   13 -------------
>  drivers/net/wireless/ath/ath5k/eeprom.c |    1 -
>  drivers/net/wireless/ath/ath5k/gpio.c   |    7 -------
>  drivers/net/wireless/ath/ath5k/pcu.c    |   24 ------------------------
>  drivers/net/wireless/ath/ath5k/phy.c    |   13 -------------
>  drivers/net/wireless/ath/ath5k/qcu.c    |    9 ---------
>  drivers/net/wireless/ath/ath5k/reset.c  |    7 -------
>  12 files changed, 0 insertions(+), 99 deletions(-)
>
> diff --git a/drivers/net/wireless/ath/ath5k/attach.c b/drivers/net/wireless/ath/ath5k/attach.c
> index e0c244b..ef2dc1d 100644
> --- a/drivers/net/wireless/ath/ath5k/attach.c
> +++ b/drivers/net/wireless/ath/ath5k/attach.c
> @@ -351,8 +351,6 @@ err_free:
>  */
>  void ath5k_hw_detach(struct ath5k_hw *ah)
>  {
> -       ATH5K_TRACE(ah->ah_sc);
> -
>        __set_bit(ATH_STAT_INVALID, ah->ah_sc->status);
>
>        if (ah->ah_rf_banks != NULL)
> diff --git a/drivers/net/wireless/ath/ath5k/caps.c b/drivers/net/wireless/ath/ath5k/caps.c
> index 74f0071..beae519 100644
> --- a/drivers/net/wireless/ath/ath5k/caps.c
> +++ b/drivers/net/wireless/ath/ath5k/caps.c
> @@ -34,7 +34,6 @@ int ath5k_hw_set_capabilities(struct ath5k_hw *ah)
>  {
>        u16 ee_header;
>
> -       ATH5K_TRACE(ah->ah_sc);
>        /* Capabilities stored in the EEPROM */
>        ee_header = ah->ah_capabilities.cap_eeprom.ee_header;
>
> @@ -123,8 +122,6 @@ int ath5k_hw_get_capability(struct ath5k_hw *ah,
>                enum ath5k_capability_type cap_type,
>                u32 capability, u32 *result)
>  {
> -       ATH5K_TRACE(ah->ah_sc);
> -
>        switch (cap_type) {
>        case AR5K_CAP_NUM_TXQUEUES:
>                if (result) {
> @@ -173,8 +170,6 @@ yes:
>  int ath5k_hw_enable_pspoll(struct ath5k_hw *ah, u8 *bssid,
>                u16 assoc_id)
>  {
> -       ATH5K_TRACE(ah->ah_sc);
> -
>        if (ah->ah_version == AR5K_AR5210) {
>                AR5K_REG_DISABLE_BITS(ah, AR5K_STA_ID1,
>                        AR5K_STA_ID1_NO_PSPOLL | AR5K_STA_ID1_DEFAULT_ANTENNA);
> @@ -186,8 +181,6 @@ int ath5k_hw_enable_pspoll(struct ath5k_hw *ah, u8 *bssid,
>
>  int ath5k_hw_disable_pspoll(struct ath5k_hw *ah)
>  {
> -       ATH5K_TRACE(ah->ah_sc);
> -
>        if (ah->ah_version == AR5K_AR5210) {
>                AR5K_REG_ENABLE_BITS(ah, AR5K_STA_ID1,
>                        AR5K_STA_ID1_NO_PSPOLL | AR5K_STA_ID1_DEFAULT_ANTENNA);
> diff --git a/drivers/net/wireless/ath/ath5k/debug.c b/drivers/net/wireless/ath/ath5k/debug.c
> index c77a6ad..97c5ada 100644
> --- a/drivers/net/wireless/ath/ath5k/debug.c
> +++ b/drivers/net/wireless/ath/ath5k/debug.c
> @@ -307,7 +307,6 @@ static const struct {
>        { ATH5K_DEBUG_DUMP_RX,  "dumprx",       "print received skb content" },
>        { ATH5K_DEBUG_DUMP_TX,  "dumptx",       "print transmit skb content" },
>        { ATH5K_DEBUG_DUMPBANDS, "dumpbands",   "dump bands" },
> -       { ATH5K_DEBUG_TRACE,    "trace",        "trace function calls" },
>        { ATH5K_DEBUG_ANI,      "ani",          "adaptive noise immunity" },
>        { ATH5K_DEBUG_ANY,      "all",          "show all debug levels" },
>  };
> diff --git a/drivers/net/wireless/ath/ath5k/debug.h b/drivers/net/wireless/ath/ath5k/debug.h
> index c7a0769..606ae94 100644
> --- a/drivers/net/wireless/ath/ath5k/debug.h
> +++ b/drivers/net/wireless/ath/ath5k/debug.h
> @@ -116,18 +116,12 @@ enum ath5k_debug_level {
>        ATH5K_DEBUG_DUMP_RX     = 0x00000100,
>        ATH5K_DEBUG_DUMP_TX     = 0x00000200,
>        ATH5K_DEBUG_DUMPBANDS   = 0x00000400,
> -       ATH5K_DEBUG_TRACE       = 0x00001000,
>        ATH5K_DEBUG_ANI         = 0x00002000,
>        ATH5K_DEBUG_ANY         = 0xffffffff
>  };
>
>  #ifdef CONFIG_ATH5K_DEBUG
>
> -#define ATH5K_TRACE(_sc) do { \
> -       if (unlikely((_sc)->debug.level & ATH5K_DEBUG_TRACE)) \
> -               printk(KERN_DEBUG "ath5k trace %s:%d\n", __func__, __LINE__); \
> -       } while (0)
> -
>  #define ATH5K_DBG(_sc, _m, _fmt, ...) do { \
>        if (unlikely((_sc)->debug.level & (_m) && net_ratelimit())) \
>                ATH5K_PRINTK(_sc, KERN_DEBUG, "(%s:%d): " _fmt, \
> @@ -169,8 +163,6 @@ ath5k_debug_printtxbuf(struct ath5k_softc *sc, struct ath5k_buf *bf);
>
>  #include <linux/compiler.h>
>
> -#define ATH5K_TRACE(_sc) typecheck(struct ath5k_softc *, (_sc))
> -
>  static inline void __attribute__ ((format (printf, 3, 4)))
>  ATH5K_DBG(struct ath5k_softc *sc, unsigned int m, const char *fmt, ...) {}
>
> diff --git a/drivers/net/wireless/ath/ath5k/desc.c b/drivers/net/wireless/ath/ath5k/desc.c
> index 7d7b646..da5dbb6 100644
> --- a/drivers/net/wireless/ath/ath5k/desc.c
> +++ b/drivers/net/wireless/ath/ath5k/desc.c
> @@ -176,7 +176,6 @@ static int ath5k_hw_setup_4word_tx_desc(struct ath5k_hw *ah,
>        struct ath5k_hw_4w_tx_ctl *tx_ctl;
>        unsigned int frame_len;
>
> -       ATH5K_TRACE(ah->ah_sc);
>        tx_ctl = &desc->ud.ds_tx5212.tx_ctl;
>
>        /*
> @@ -342,8 +341,6 @@ static int ath5k_hw_proc_2word_tx_status(struct ath5k_hw *ah,
>        struct ath5k_hw_2w_tx_ctl *tx_ctl;
>        struct ath5k_hw_tx_status *tx_status;
>
> -       ATH5K_TRACE(ah->ah_sc);
> -
>        tx_ctl = &desc->ud.ds_tx5210.tx_ctl;
>        tx_status = &desc->ud.ds_tx5210.tx_stat;
>
> @@ -396,8 +393,6 @@ static int ath5k_hw_proc_4word_tx_status(struct ath5k_hw *ah,
>        struct ath5k_hw_4w_tx_ctl *tx_ctl;
>        struct ath5k_hw_tx_status *tx_status;
>
> -       ATH5K_TRACE(ah->ah_sc);
> -
>        tx_ctl = &desc->ud.ds_tx5212.tx_ctl;
>        tx_status = &desc->ud.ds_tx5212.tx_stat;
>
> @@ -490,7 +485,6 @@ static int ath5k_hw_setup_rx_desc(struct ath5k_hw *ah, struct ath5k_desc *desc,
>  {
>        struct ath5k_hw_rx_ctl *rx_ctl;
>
> -       ATH5K_TRACE(ah->ah_sc);
>        rx_ctl = &desc->ud.ds_rx.rx_ctl;
>
>        /*
> @@ -593,7 +587,6 @@ static int ath5k_hw_proc_5212_rx_status(struct ath5k_hw *ah,
>        struct ath5k_hw_rx_status *rx_status;
>        struct ath5k_hw_rx_error *rx_err;
>
> -       ATH5K_TRACE(ah->ah_sc);
>        rx_status = &desc->ud.ds_rx.u.rx_stat;
>
>        /* Overlay on error */
> diff --git a/drivers/net/wireless/ath/ath5k/dma.c b/drivers/net/wireless/ath/ath5k/dma.c
> index 941b511..484f318 100644
> --- a/drivers/net/wireless/ath/ath5k/dma.c
> +++ b/drivers/net/wireless/ath/ath5k/dma.c
> @@ -48,7 +48,6 @@
>  */
>  void ath5k_hw_start_rx_dma(struct ath5k_hw *ah)
>  {
> -       ATH5K_TRACE(ah->ah_sc);
>        ath5k_hw_reg_write(ah, AR5K_CR_RXE, AR5K_CR);
>        ath5k_hw_reg_read(ah, AR5K_CR);
>  }
> @@ -62,7 +61,6 @@ int ath5k_hw_stop_rx_dma(struct ath5k_hw *ah)
>  {
>        unsigned int i;
>
> -       ATH5K_TRACE(ah->ah_sc);
>        ath5k_hw_reg_write(ah, AR5K_CR_RXD, AR5K_CR);
>
>        /*
> @@ -96,8 +94,6 @@ u32 ath5k_hw_get_rxdp(struct ath5k_hw *ah)
>  */
>  void ath5k_hw_set_rxdp(struct ath5k_hw *ah, u32 phys_addr)
>  {
> -       ATH5K_TRACE(ah->ah_sc);
> -
>        ath5k_hw_reg_write(ah, phys_addr, AR5K_RXDP);
>  }
>
> @@ -125,7 +121,6 @@ int ath5k_hw_start_tx_dma(struct ath5k_hw *ah, unsigned int queue)
>  {
>        u32 tx_queue;
>
> -       ATH5K_TRACE(ah->ah_sc);
>        AR5K_ASSERT_ENTRY(queue, ah->ah_capabilities.cap_queues.q_tx_num);
>
>        /* Return if queue is declared inactive */
> @@ -186,7 +181,6 @@ int ath5k_hw_stop_tx_dma(struct ath5k_hw *ah, unsigned int queue)
>        unsigned int i = 40;
>        u32 tx_queue, pending;
>
> -       ATH5K_TRACE(ah->ah_sc);
>        AR5K_ASSERT_ENTRY(queue, ah->ah_capabilities.cap_queues.q_tx_num);
>
>        /* Return if queue is declared inactive */
> @@ -297,7 +291,6 @@ u32 ath5k_hw_get_txdp(struct ath5k_hw *ah, unsigned int queue)
>  {
>        u16 tx_reg;
>
> -       ATH5K_TRACE(ah->ah_sc);
>        AR5K_ASSERT_ENTRY(queue, ah->ah_capabilities.cap_queues.q_tx_num);
>
>        /*
> @@ -340,7 +333,6 @@ int ath5k_hw_set_txdp(struct ath5k_hw *ah, unsigned int queue, u32 phys_addr)
>  {
>        u16 tx_reg;
>
> -       ATH5K_TRACE(ah->ah_sc);
>        AR5K_ASSERT_ENTRY(queue, ah->ah_capabilities.cap_queues.q_tx_num);
>
>        /*
> @@ -400,8 +392,6 @@ int ath5k_hw_update_tx_triglevel(struct ath5k_hw *ah, bool increase)
>        u32 trigger_level, imr;
>        int ret = -EIO;
>
> -       ATH5K_TRACE(ah->ah_sc);
> -
>        /*
>         * Disable interrupts by setting the mask
>         */
> @@ -451,7 +441,6 @@ done:
>  */
>  bool ath5k_hw_is_intr_pending(struct ath5k_hw *ah)
>  {
> -       ATH5K_TRACE(ah->ah_sc);
>        return ath5k_hw_reg_read(ah, AR5K_INTPEND) == 1 ? 1 : 0;
>  }
>
> @@ -475,8 +464,6 @@ int ath5k_hw_get_isr(struct ath5k_hw *ah, enum ath5k_int *interrupt_mask)
>  {
>        u32 data;
>
> -       ATH5K_TRACE(ah->ah_sc);
> -
>        /*
>         * Read interrupt status from the Interrupt Status register
>         * on 5210
> diff --git a/drivers/net/wireless/ath/ath5k/eeprom.c b/drivers/net/wireless/ath/ath5k/eeprom.c
> index ed02636..155a82c 100644
> --- a/drivers/net/wireless/ath/ath5k/eeprom.c
> +++ b/drivers/net/wireless/ath/ath5k/eeprom.c
> @@ -35,7 +35,6 @@ static int ath5k_hw_eeprom_read(struct ath5k_hw *ah, u32 offset, u16 *data)
>  {
>        u32 status, timeout;
>
> -       ATH5K_TRACE(ah->ah_sc);
>        /*
>         * Initialize EEPROM access
>         */
> diff --git a/drivers/net/wireless/ath/ath5k/gpio.c b/drivers/net/wireless/ath/ath5k/gpio.c
> index 64a27e7..bc90503 100644
> --- a/drivers/net/wireless/ath/ath5k/gpio.c
> +++ b/drivers/net/wireless/ath/ath5k/gpio.c
> @@ -34,8 +34,6 @@ void ath5k_hw_set_ledstate(struct ath5k_hw *ah, unsigned int state)
>        /*5210 has different led mode handling*/
>        u32 led_5210;
>
> -       ATH5K_TRACE(ah->ah_sc);
> -
>        /*Reset led status*/
>        if (ah->ah_version != AR5K_AR5210)
>                AR5K_REG_DISABLE_BITS(ah, AR5K_PCICFG,
> @@ -82,7 +80,6 @@ void ath5k_hw_set_ledstate(struct ath5k_hw *ah, unsigned int state)
>  */
>  int ath5k_hw_set_gpio_input(struct ath5k_hw *ah, u32 gpio)
>  {
> -       ATH5K_TRACE(ah->ah_sc);
>        if (gpio >= AR5K_NUM_GPIO)
>                return -EINVAL;
>
> @@ -98,7 +95,6 @@ int ath5k_hw_set_gpio_input(struct ath5k_hw *ah, u32 gpio)
>  */
>  int ath5k_hw_set_gpio_output(struct ath5k_hw *ah, u32 gpio)
>  {
> -       ATH5K_TRACE(ah->ah_sc);
>        if (gpio >= AR5K_NUM_GPIO)
>                return -EINVAL;
>
> @@ -114,7 +110,6 @@ int ath5k_hw_set_gpio_output(struct ath5k_hw *ah, u32 gpio)
>  */
>  u32 ath5k_hw_get_gpio(struct ath5k_hw *ah, u32 gpio)
>  {
> -       ATH5K_TRACE(ah->ah_sc);
>        if (gpio >= AR5K_NUM_GPIO)
>                return 0xffffffff;
>
> @@ -129,7 +124,6 @@ u32 ath5k_hw_get_gpio(struct ath5k_hw *ah, u32 gpio)
>  int ath5k_hw_set_gpio(struct ath5k_hw *ah, u32 gpio, u32 val)
>  {
>        u32 data;
> -       ATH5K_TRACE(ah->ah_sc);
>
>        if (gpio >= AR5K_NUM_GPIO)
>                return -EINVAL;
> @@ -153,7 +147,6 @@ void ath5k_hw_set_gpio_intr(struct ath5k_hw *ah, unsigned int gpio,
>  {
>        u32 data;
>
> -       ATH5K_TRACE(ah->ah_sc);
>        if (gpio >= AR5K_NUM_GPIO)
>                return;
>
> diff --git a/drivers/net/wireless/ath/ath5k/pcu.c b/drivers/net/wireless/ath/ath5k/pcu.c
> index 5212e27..86fdb6d 100644
> --- a/drivers/net/wireless/ath/ath5k/pcu.c
> +++ b/drivers/net/wireless/ath/ath5k/pcu.c
> @@ -59,8 +59,6 @@ int ath5k_hw_set_opmode(struct ath5k_hw *ah, enum nl80211_iftype op_mode)
>
>        beacon_reg = 0;
>
> -       ATH5K_TRACE(ah->ah_sc);
> -
>        switch (op_mode) {
>        case NL80211_IFTYPE_ADHOC:
>                pcu_reg |= AR5K_STA_ID1_ADHOC | AR5K_STA_ID1_KEYSRCH_MODE;
> @@ -173,7 +171,6 @@ void ath5k_hw_set_ack_bitrate_high(struct ath5k_hw *ah, bool high)
>  */
>  static int ath5k_hw_set_ack_timeout(struct ath5k_hw *ah, unsigned int timeout)
>  {
> -       ATH5K_TRACE(ah->ah_sc);
>        if (ath5k_hw_clocktoh(ah, AR5K_REG_MS(0xffffffff, AR5K_TIME_OUT_ACK))
>                        <= timeout)
>                return -EINVAL;
> @@ -192,7 +189,6 @@ static int ath5k_hw_set_ack_timeout(struct ath5k_hw *ah, unsigned int timeout)
>  */
>  static int ath5k_hw_set_cts_timeout(struct ath5k_hw *ah, unsigned int timeout)
>  {
> -       ATH5K_TRACE(ah->ah_sc);
>        if (ath5k_hw_clocktoh(ah, AR5K_REG_MS(0xffffffff, AR5K_TIME_OUT_CTS))
>                        <= timeout)
>                return -EINVAL;
> @@ -297,7 +293,6 @@ int ath5k_hw_set_lladdr(struct ath5k_hw *ah, const u8 *mac)
>        u32 low_id, high_id;
>        u32 pcu_reg;
>
> -       ATH5K_TRACE(ah->ah_sc);
>        /* Set new station ID */
>        memcpy(common->macaddr, mac, ETH_ALEN);
>
> @@ -357,7 +352,6 @@ void ath5k_hw_set_associd(struct ath5k_hw *ah)
>  void ath5k_hw_set_bssid_mask(struct ath5k_hw *ah, const u8 *mask)
>  {
>        struct ath_common *common = ath5k_hw_common(ah);
> -       ATH5K_TRACE(ah->ah_sc);
>
>        /* Cache bssid mask so that we can restore it
>         * on reset */
> @@ -382,7 +376,6 @@ void ath5k_hw_set_bssid_mask(struct ath5k_hw *ah, const u8 *mask)
>  */
>  void ath5k_hw_start_rx_pcu(struct ath5k_hw *ah)
>  {
> -       ATH5K_TRACE(ah->ah_sc);
>        AR5K_REG_DISABLE_BITS(ah, AR5K_DIAG_SW, AR5K_DIAG_SW_DIS_RX);
>  }
>
> @@ -397,7 +390,6 @@ void ath5k_hw_start_rx_pcu(struct ath5k_hw *ah)
>  */
>  void ath5k_hw_stop_rx_pcu(struct ath5k_hw *ah)
>  {
> -       ATH5K_TRACE(ah->ah_sc);
>        AR5K_REG_ENABLE_BITS(ah, AR5K_DIAG_SW, AR5K_DIAG_SW_DIS_RX);
>  }
>
> @@ -406,8 +398,6 @@ void ath5k_hw_stop_rx_pcu(struct ath5k_hw *ah)
>  */
>  void ath5k_hw_set_mcast_filter(struct ath5k_hw *ah, u32 filter0, u32 filter1)
>  {
> -       ATH5K_TRACE(ah->ah_sc);
> -       /* Set the multicat filter */
>        ath5k_hw_reg_write(ah, filter0, AR5K_MCAST_FILTER0);
>        ath5k_hw_reg_write(ah, filter1, AR5K_MCAST_FILTER1);
>  }
> @@ -427,7 +417,6 @@ u32 ath5k_hw_get_rx_filter(struct ath5k_hw *ah)
>  {
>        u32 data, filter = 0;
>
> -       ATH5K_TRACE(ah->ah_sc);
>        filter = ath5k_hw_reg_read(ah, AR5K_RX_FILTER);
>
>        /*Radar detection for 5212*/
> @@ -457,8 +446,6 @@ void ath5k_hw_set_rx_filter(struct ath5k_hw *ah, u32 filter)
>  {
>        u32 data = 0;
>
> -       ATH5K_TRACE(ah->ah_sc);
> -
>        /* Set PHY error filter register on 5212*/
>        if (ah->ah_version == AR5K_AR5212) {
>                if (filter & AR5K_RX_FILTER_RADARERR)
> @@ -533,8 +520,6 @@ u64 ath5k_hw_get_tsf64(struct ath5k_hw *ah)
>
>        WARN_ON( i == ATH5K_MAX_TSF_READ );
>
> -       ATH5K_TRACE(ah->ah_sc);
> -
>        return (((u64)tsf_upper1 << 32) | tsf_lower);
>  }
>
> @@ -548,8 +533,6 @@ u64 ath5k_hw_get_tsf64(struct ath5k_hw *ah)
>  */
>  void ath5k_hw_set_tsf64(struct ath5k_hw *ah, u64 tsf64)
>  {
> -       ATH5K_TRACE(ah->ah_sc);
> -
>        ath5k_hw_reg_write(ah, tsf64 & 0xffffffff, AR5K_TSF_L32);
>        ath5k_hw_reg_write(ah, (tsf64 >> 32) & 0xffffffff, AR5K_TSF_U32);
>  }
> @@ -565,8 +548,6 @@ void ath5k_hw_reset_tsf(struct ath5k_hw *ah)
>  {
>        u32 val;
>
> -       ATH5K_TRACE(ah->ah_sc);
> -
>        val = ath5k_hw_reg_read(ah, AR5K_BEACON) | AR5K_BEACON_RESET_TSF;
>
>        /*
> @@ -586,7 +567,6 @@ void ath5k_hw_init_beacon(struct ath5k_hw *ah, u32 next_beacon, u32 interval)
>  {
>        u32 timer1, timer2, timer3;
>
> -       ATH5K_TRACE(ah->ah_sc);
>        /*
>         * Set the additional timers by mode
>         */
> @@ -674,7 +654,6 @@ int ath5k_hw_reset_key(struct ath5k_hw *ah, u16 entry)
>        unsigned int i, type;
>        u16 micentry = entry + AR5K_KEYTABLE_MIC_OFFSET;
>
> -       ATH5K_TRACE(ah->ah_sc);
>        AR5K_ASSERT_ENTRY(entry, AR5K_KEYTABLE_SIZE);
>
>        type = ath5k_hw_reg_read(ah, AR5K_KEYTABLE_TYPE(entry));
> @@ -749,8 +728,6 @@ int ath5k_hw_set_key(struct ath5k_hw *ah, u16 entry,
>        bool is_tkip;
>        const u8 *key_ptr;
>
> -       ATH5K_TRACE(ah->ah_sc);
> -
>        is_tkip = (key->alg == ALG_TKIP);
>
>        /*
> @@ -836,7 +813,6 @@ int ath5k_hw_set_key_lladdr(struct ath5k_hw *ah, u16 entry, const u8 *mac)
>  {
>        u32 low_id, high_id;
>
> -       ATH5K_TRACE(ah->ah_sc);
>         /* Invalid entry (key table overflow) */
>        AR5K_ASSERT_ENTRY(entry, AR5K_KEYTABLE_SIZE);
>
> diff --git a/drivers/net/wireless/ath/ath5k/phy.c b/drivers/net/wireless/ath/ath5k/phy.c
> index 0b24081..f369e7e 100644
> --- a/drivers/net/wireless/ath/ath5k/phy.c
> +++ b/drivers/net/wireless/ath/ath5k/phy.c
> @@ -378,8 +378,6 @@ enum ath5k_rfgain ath5k_hw_gainf_calibrate(struct ath5k_hw *ah)
>        u32 data, type;
>        struct ath5k_eeprom_info *ee = &ah->ah_capabilities.cap_eeprom;
>
> -       ATH5K_TRACE(ah->ah_sc);
> -
>        if (ah->ah_rf_banks == NULL ||
>        ah->ah_gain.g_state == AR5K_RFGAIN_INACTIVE)
>                return AR5K_RFGAIN_INACTIVE;
> @@ -1353,7 +1351,6 @@ ath5k_hw_rf511x_iq_calibrate(struct ath5k_hw *ah)
>        u32 i_pwr, q_pwr;
>        s32 iq_corr, i_coff, i_coffd, q_coff, q_coffd;
>        int i;
> -       ATH5K_TRACE(ah->ah_sc);
>
>        if (!ah->ah_calibration ||
>                ath5k_hw_reg_read(ah, AR5K_PHY_IQ) & AR5K_PHY_IQ_RUN)
> @@ -1680,7 +1677,6 @@ ath5k_hw_set_spur_mitigation_filter(struct ath5k_hw *ah,
>
>  int ath5k_hw_phy_disable(struct ath5k_hw *ah)
>  {
> -       ATH5K_TRACE(ah->ah_sc);
>        /*Just a try M.F.*/
>        ath5k_hw_reg_write(ah, AR5K_PHY_ACT_DISABLE, AR5K_PHY_ACT);
>
> @@ -1696,8 +1692,6 @@ u16 ath5k_hw_radio_revision(struct ath5k_hw *ah, unsigned int chan)
>        u32 srev;
>        u16 ret;
>
> -       ATH5K_TRACE(ah->ah_sc);
> -
>        /*
>         * Set the radio chip access register
>         */
> @@ -1742,8 +1736,6 @@ u16 ath5k_hw_radio_revision(struct ath5k_hw *ah, unsigned int chan)
>  static void /*TODO:Boundary check*/
>  ath5k_hw_set_def_antenna(struct ath5k_hw *ah, u8 ant)
>  {
> -       ATH5K_TRACE(ah->ah_sc);
> -
>        if (ah->ah_version != AR5K_AR5210)
>                ath5k_hw_reg_write(ah, ant & 0x7, AR5K_DEFAULT_ANTENNA);
>  }
> @@ -1803,8 +1795,6 @@ ath5k_hw_set_antenna_mode(struct ath5k_hw *ah, u8 ant_mode)
>
>        def_ant = ah->ah_def_ant;
>
> -       ATH5K_TRACE(ah->ah_sc);
> -
>        switch (channel->hw_value & CHANNEL_MODES) {
>        case CHANNEL_A:
>        case CHANNEL_T:
> @@ -2970,7 +2960,6 @@ ath5k_hw_txpower(struct ath5k_hw *ah, struct ieee80211_channel *channel,
>        u8 type;
>        int ret;
>
> -       ATH5K_TRACE(ah->ah_sc);
>        if (txpower > AR5K_TUNE_MAX_TXPOWER) {
>                ATH5K_ERR(ah->ah_sc, "invalid tx power: %u\n", txpower);
>                return -EINVAL;
> @@ -3066,8 +3055,6 @@ int ath5k_hw_set_txpower_limit(struct ath5k_hw *ah, u8 txpower)
>        struct ieee80211_channel *channel = ah->ah_current_channel;
>        u8 ee_mode;
>
> -       ATH5K_TRACE(ah->ah_sc);
> -
>        switch (channel->hw_value & CHANNEL_MODES) {
>        case CHANNEL_A:
>        case CHANNEL_T:
> diff --git a/drivers/net/wireless/ath/ath5k/qcu.c b/drivers/net/wireless/ath/ath5k/qcu.c
> index f5831da..4186ff4 100644
> --- a/drivers/net/wireless/ath/ath5k/qcu.c
> +++ b/drivers/net/wireless/ath/ath5k/qcu.c
> @@ -31,7 +31,6 @@ Queue Control Unit, DFS Control Unit Functions
>  int ath5k_hw_get_tx_queueprops(struct ath5k_hw *ah, int queue,
>                struct ath5k_txq_info *queue_info)
>  {
> -       ATH5K_TRACE(ah->ah_sc);
>        memcpy(queue_info, &ah->ah_txq[queue], sizeof(struct ath5k_txq_info));
>        return 0;
>  }
> @@ -42,7 +41,6 @@ int ath5k_hw_get_tx_queueprops(struct ath5k_hw *ah, int queue,
>  int ath5k_hw_set_tx_queueprops(struct ath5k_hw *ah, int queue,
>                                const struct ath5k_txq_info *queue_info)
>  {
> -       ATH5K_TRACE(ah->ah_sc);
>        AR5K_ASSERT_ENTRY(queue, ah->ah_capabilities.cap_queues.q_tx_num);
>
>        if (ah->ah_txq[queue].tqi_type == AR5K_TX_QUEUE_INACTIVE)
> @@ -69,8 +67,6 @@ int ath5k_hw_setup_tx_queue(struct ath5k_hw *ah, enum ath5k_tx_queue queue_type,
>        unsigned int queue;
>        int ret;
>
> -       ATH5K_TRACE(ah->ah_sc);
> -
>        /*
>         * Get queue by type
>         */
> @@ -149,7 +145,6 @@ int ath5k_hw_setup_tx_queue(struct ath5k_hw *ah, enum ath5k_tx_queue queue_type,
>  u32 ath5k_hw_num_tx_pending(struct ath5k_hw *ah, unsigned int queue)
>  {
>        u32 pending;
> -       ATH5K_TRACE(ah->ah_sc);
>        AR5K_ASSERT_ENTRY(queue, ah->ah_capabilities.cap_queues.q_tx_num);
>
>        /* Return if queue is declared inactive */
> @@ -177,7 +172,6 @@ u32 ath5k_hw_num_tx_pending(struct ath5k_hw *ah, unsigned int queue)
>  */
>  void ath5k_hw_release_tx_queue(struct ath5k_hw *ah, unsigned int queue)
>  {
> -       ATH5K_TRACE(ah->ah_sc);
>        if (WARN_ON(queue >= ah->ah_capabilities.cap_queues.q_tx_num))
>                return;
>
> @@ -195,7 +189,6 @@ int ath5k_hw_reset_tx_queue(struct ath5k_hw *ah, unsigned int queue)
>        u32 cw_min, cw_max, retry_lg, retry_sh;
>        struct ath5k_txq_info *tq = &ah->ah_txq[queue];
>
> -       ATH5K_TRACE(ah->ah_sc);
>        AR5K_ASSERT_ENTRY(queue, ah->ah_capabilities.cap_queues.q_tx_num);
>
>        tq = &ah->ah_txq[queue];
> @@ -523,8 +516,6 @@ int ath5k_hw_set_slot_time(struct ath5k_hw *ah, unsigned int slot_time)
>  {
>        u32 slot_time_clock = ath5k_hw_htoclock(ah, slot_time);
>
> -       ATH5K_TRACE(ah->ah_sc);
> -
>        if (slot_time < 6 || slot_time_clock > AR5K_SLOT_TIME_MAX)
>                return -EINVAL;
>
> diff --git a/drivers/net/wireless/ath/ath5k/reset.c b/drivers/net/wireless/ath/ath5k/reset.c
> index 44bbbf2..5e23e61 100644
> --- a/drivers/net/wireless/ath/ath5k/reset.c
> +++ b/drivers/net/wireless/ath/ath5k/reset.c
> @@ -201,8 +201,6 @@ static int ath5k_hw_nic_reset(struct ath5k_hw *ah, u32 val)
>        int ret;
>        u32 mask = val ? val : ~0U;
>
> -       ATH5K_TRACE(ah->ah_sc);
> -
>        /* Read-and-clear RX Descriptor Pointer*/
>        ath5k_hw_reg_read(ah, AR5K_RXDP);
>
> @@ -246,7 +244,6 @@ static int ath5k_hw_set_power(struct ath5k_hw *ah, enum ath5k_power_mode mode,
>        unsigned int i;
>        u32 staid, data;
>
> -       ATH5K_TRACE(ah->ah_sc);
>        staid = ath5k_hw_reg_read(ah, AR5K_STA_ID1);
>
>        switch (mode) {
> @@ -393,8 +390,6 @@ int ath5k_hw_nic_wakeup(struct ath5k_hw *ah, int flags, bool initial)
>        mode = 0;
>        clock = 0;
>
> -       ATH5K_TRACE(ah->ah_sc);
> -
>        /* Wakeup the device */
>        ret = ath5k_hw_set_power(ah, AR5K_PM_AWAKE, true, 0);
>        if (ret) {
> @@ -899,8 +894,6 @@ int ath5k_hw_reset(struct ath5k_hw *ah, enum nl80211_iftype op_mode,
>        u8 mode, freq, ee_mode, ant[2];
>        int i, ret;
>
> -       ATH5K_TRACE(ah->ah_sc);
> -
>        s_ant = 0;
>        ee_mode = 0;
>        staid1_flags = 0;
>

Acked-by: Nick Kossifidis <[email protected]>


--
GPG ID: 0xD21DB2DB
As you read this post global entropy rises. Have Fun ;-)
Nick

2010-05-20 12:51:08

by Nick Kossifidis

[permalink] [raw]
Subject: Re: [ath5k-devel] [PATCH v2 02/20] ath5k: wake queues on reset

2010/5/19 Bruno Randolf <[email protected]>:
> We can wake all queues after a chip reset since everything should be set up and
> we are ready to transmit. If we don't do that we might end up starting up with
> stopped queues, not beeing able to transmit. (This started to happen after
> "ath5k: clean up queue manipulation" but since periodic calibration also
> stopped and started the queues this effect was hidden most of the time).
>
> This way we can also get rid of the superfluous ath5k_reset_wake() function.
>
> Signed-off-by: Bruno Randolf <[email protected]>
> ---
>  drivers/net/wireless/ath/ath5k/base.c |   17 +++--------------
>  1 files changed, 3 insertions(+), 14 deletions(-)
>
> diff --git a/drivers/net/wireless/ath/ath5k/base.c b/drivers/net/wireless/ath/ath5k/base.c
> index fbd5fc7..9f9107b 100644
> --- a/drivers/net/wireless/ath/ath5k/base.c
> +++ b/drivers/net/wireless/ath/ath5k/base.c
> @@ -222,7 +222,6 @@ static int ath5k_tx(struct ieee80211_hw *hw, struct sk_buff *skb);
>  static int ath5k_tx_queue(struct ieee80211_hw *hw, struct sk_buff *skb,
>                struct ath5k_txq *txq);
>  static int ath5k_reset(struct ath5k_softc *sc, struct ieee80211_channel *chan);
> -static int ath5k_reset_wake(struct ath5k_softc *sc);
>  static int ath5k_start(struct ieee80211_hw *hw);
>  static void ath5k_stop(struct ieee80211_hw *hw);
>  static int ath5k_add_interface(struct ieee80211_hw *hw,
> @@ -2770,7 +2769,7 @@ ath5k_tasklet_reset(unsigned long data)
>  {
>        struct ath5k_softc *sc = (void *)data;
>
> -       ath5k_reset_wake(sc);
> +       ath5k_reset(sc, sc->curchan);
>  }
>
>  /*
> @@ -2941,23 +2940,13 @@ ath5k_reset(struct ath5k_softc *sc, struct ieee80211_channel *chan)
>        ath5k_beacon_config(sc);
>        /* intrs are enabled by ath5k_beacon_config */
>
> +       ieee80211_wake_queues(sc->hw);
> +
>        return 0;
>  err:
>        return ret;
>  }
>
> -static int
> -ath5k_reset_wake(struct ath5k_softc *sc)
> -{
> -       int ret;
> -
> -       ret = ath5k_reset(sc, sc->curchan);
> -       if (!ret)
> -               ieee80211_wake_queues(sc->hw);
> -
> -       return ret;
> -}
> -
>  static int ath5k_start(struct ieee80211_hw *hw)
>  {
>        return ath5k_init(hw->priv);
>

Acked-by: Nick Kossifidis <[email protected]>


--
GPG ID: 0xD21DB2DB
As you read this post global entropy rises. Have Fun ;-)
Nick

2010-05-19 01:32:13

by Bruno Randolf

[permalink] [raw]
Subject: [PATCH v2 04/20] ath5k: move noise floor calibration into tasklet

Seperate noise floor calibration from other PHY calibration and move it to the
tasklet. This is the first step to more separation of different calibrations.

Also move out ath5k_hw_request_rfgain_probe(ah) so we have one clean function
for I/Q calibration on 5111x parts.

Signed-off-by: Bruno Randolf <[email protected]>
---
drivers/net/wireless/ath/ath5k/ath5k.h | 1 +
drivers/net/wireless/ath/ath5k/base.c | 1 +
drivers/net/wireless/ath/ath5k/phy.c | 33 ++++++++++----------------------
3 files changed, 12 insertions(+), 23 deletions(-)

diff --git a/drivers/net/wireless/ath/ath5k/ath5k.h b/drivers/net/wireless/ath/ath5k/ath5k.h
index 2785946..131e8b3 100644
--- a/drivers/net/wireless/ath/ath5k/ath5k.h
+++ b/drivers/net/wireless/ath/ath5k/ath5k.h
@@ -1270,6 +1270,7 @@ int ath5k_hw_channel(struct ath5k_hw *ah, struct ieee80211_channel *channel);
void ath5k_hw_init_nfcal_hist(struct ath5k_hw *ah);
int ath5k_hw_phy_calibrate(struct ath5k_hw *ah,
struct ieee80211_channel *channel);
+void ath5k_hw_update_noise_floor(struct ath5k_hw *ah);
/* Spur mitigation */
bool ath5k_hw_chan_has_spur_noise(struct ath5k_hw *ah,
struct ieee80211_channel *channel);
diff --git a/drivers/net/wireless/ath/ath5k/base.c b/drivers/net/wireless/ath/ath5k/base.c
index de59a6e..88c7314 100644
--- a/drivers/net/wireless/ath/ath5k/base.c
+++ b/drivers/net/wireless/ath/ath5k/base.c
@@ -2806,6 +2806,7 @@ ath5k_tasklet_calibrate(unsigned long data)
ieee80211_frequency_to_channel(
sc->curchan->center_freq));

+ ath5k_hw_update_noise_floor(ah);
/* Wake queues */
ieee80211_wake_queues(sc->hw);

diff --git a/drivers/net/wireless/ath/ath5k/phy.c b/drivers/net/wireless/ath/ath5k/phy.c
index 3ce9afb..0b24081 100644
--- a/drivers/net/wireless/ath/ath5k/phy.c
+++ b/drivers/net/wireless/ath/ath5k/phy.c
@@ -1167,7 +1167,7 @@ static s16 ath5k_hw_get_median_noise_floor(struct ath5k_hw *ah)
* The median of the values in the history is then loaded into the
* hardware for its own use for RSSI and CCA measurements.
*/
-static void ath5k_hw_update_noise_floor(struct ath5k_hw *ah)
+void ath5k_hw_update_noise_floor(struct ath5k_hw *ah)
{
struct ath5k_eeprom_info *ee = &ah->ah_capabilities.cap_eeprom;
u32 val;
@@ -1248,7 +1248,6 @@ static void ath5k_hw_update_noise_floor(struct ath5k_hw *ah)
/*
* Perform a PHY calibration on RF5110
* -Fix BPSK/QAM Constellation (I/Q correction)
- * -Calculate Noise Floor
*/
static int ath5k_hw_rf5110_calibrate(struct ath5k_hw *ah,
struct ieee80211_channel *channel)
@@ -1335,8 +1334,6 @@ static int ath5k_hw_rf5110_calibrate(struct ath5k_hw *ah,
return ret;
}

- ath5k_hw_update_noise_floor(ah);
-
/*
* Re-enable RX/TX and beacons
*/
@@ -1348,10 +1345,10 @@ static int ath5k_hw_rf5110_calibrate(struct ath5k_hw *ah,
}

/*
- * Perform a PHY calibration on RF5111/5112 and newer chips
+ * Perform I/Q calibration on RF5111/5112 and newer chips
*/
-static int ath5k_hw_rf511x_calibrate(struct ath5k_hw *ah,
- struct ieee80211_channel *channel)
+static int
+ath5k_hw_rf511x_iq_calibrate(struct ath5k_hw *ah)
{
u32 i_pwr, q_pwr;
s32 iq_corr, i_coff, i_coffd, q_coff, q_coffd;
@@ -1360,10 +1357,9 @@ static int ath5k_hw_rf511x_calibrate(struct ath5k_hw *ah,

if (!ah->ah_calibration ||
ath5k_hw_reg_read(ah, AR5K_PHY_IQ) & AR5K_PHY_IQ_RUN)
- goto done;
+ return 0;

/* Calibration has finished, get the results and re-run */
-
/* work around empty results which can apparently happen on 5212 */
for (i = 0; i <= 10; i++) {
iq_corr = ath5k_hw_reg_read(ah, AR5K_PHY_IQRES_CAL_CORR);
@@ -1384,7 +1380,7 @@ static int ath5k_hw_rf511x_calibrate(struct ath5k_hw *ah,

/* protect against divide by 0 and loss of sign bits */
if (i_coffd == 0 || q_coffd < 2)
- goto done;
+ return -1;

i_coff = (-iq_corr) / i_coffd;
i_coff = clamp(i_coff, -32, 31); /* signed 6 bit */
@@ -1410,17 +1406,6 @@ static int ath5k_hw_rf511x_calibrate(struct ath5k_hw *ah,
AR5K_PHY_IQ_CAL_NUM_LOG_MAX, 15);
AR5K_REG_ENABLE_BITS(ah, AR5K_PHY_IQ, AR5K_PHY_IQ_RUN);

-done:
-
- /* TODO: Separate noise floor calibration from I/Q calibration
- * since noise floor calibration interrupts rx path while I/Q
- * calibration doesn't. We don't need to run noise floor calibration
- * as often as I/Q calibration.*/
- ath5k_hw_update_noise_floor(ah);
-
- /* Initiate a gain_F calibration */
- ath5k_hw_request_rfgain_probe(ah);
-
return 0;
}

@@ -1434,8 +1419,10 @@ int ath5k_hw_phy_calibrate(struct ath5k_hw *ah,

if (ah->ah_radio == AR5K_RF5110)
ret = ath5k_hw_rf5110_calibrate(ah, channel);
- else
- ret = ath5k_hw_rf511x_calibrate(ah, channel);
+ else {
+ ret = ath5k_hw_rf511x_iq_calibrate(ah);
+ ath5k_hw_request_rfgain_probe(ah);
+ }

return ret;
}


2010-05-19 01:32:06

by Bruno Randolf

[permalink] [raw]
Subject: [PATCH v2 02/20] ath5k: wake queues on reset

We can wake all queues after a chip reset since everything should be set up and
we are ready to transmit. If we don't do that we might end up starting up with
stopped queues, not beeing able to transmit. (This started to happen after
"ath5k: clean up queue manipulation" but since periodic calibration also
stopped and started the queues this effect was hidden most of the time).

This way we can also get rid of the superfluous ath5k_reset_wake() function.

Signed-off-by: Bruno Randolf <[email protected]>
---
drivers/net/wireless/ath/ath5k/base.c | 17 +++--------------
1 files changed, 3 insertions(+), 14 deletions(-)

diff --git a/drivers/net/wireless/ath/ath5k/base.c b/drivers/net/wireless/ath/ath5k/base.c
index fbd5fc7..9f9107b 100644
--- a/drivers/net/wireless/ath/ath5k/base.c
+++ b/drivers/net/wireless/ath/ath5k/base.c
@@ -222,7 +222,6 @@ static int ath5k_tx(struct ieee80211_hw *hw, struct sk_buff *skb);
static int ath5k_tx_queue(struct ieee80211_hw *hw, struct sk_buff *skb,
struct ath5k_txq *txq);
static int ath5k_reset(struct ath5k_softc *sc, struct ieee80211_channel *chan);
-static int ath5k_reset_wake(struct ath5k_softc *sc);
static int ath5k_start(struct ieee80211_hw *hw);
static void ath5k_stop(struct ieee80211_hw *hw);
static int ath5k_add_interface(struct ieee80211_hw *hw,
@@ -2770,7 +2769,7 @@ ath5k_tasklet_reset(unsigned long data)
{
struct ath5k_softc *sc = (void *)data;

- ath5k_reset_wake(sc);
+ ath5k_reset(sc, sc->curchan);
}

/*
@@ -2941,23 +2940,13 @@ ath5k_reset(struct ath5k_softc *sc, struct ieee80211_channel *chan)
ath5k_beacon_config(sc);
/* intrs are enabled by ath5k_beacon_config */

+ ieee80211_wake_queues(sc->hw);
+
return 0;
err:
return ret;
}

-static int
-ath5k_reset_wake(struct ath5k_softc *sc)
-{
- int ret;
-
- ret = ath5k_reset(sc, sc->curchan);
- if (!ret)
- ieee80211_wake_queues(sc->hw);
-
- return ret;
-}
-
static int ath5k_start(struct ieee80211_hw *hw)
{
return ath5k_init(hw->priv);


2010-05-19 01:33:22

by Bruno Randolf

[permalink] [raw]
Subject: [PATCH v2 17/20] ath5k: fix NULL pointer in antenna configuration

If the channel is not set yet and we configure the antennas just store the
setting. It will be activated during the next reset, when the channel is set.

Signed-off-by: Bruno Randolf <[email protected]>
---
drivers/net/wireless/ath/ath5k/phy.c | 7 +++++++
1 files changed, 7 insertions(+), 0 deletions(-)

diff --git a/drivers/net/wireless/ath/ath5k/phy.c b/drivers/net/wireless/ath/ath5k/phy.c
index f369e7e..d9506e7 100644
--- a/drivers/net/wireless/ath/ath5k/phy.c
+++ b/drivers/net/wireless/ath/ath5k/phy.c
@@ -1793,6 +1793,13 @@ ath5k_hw_set_antenna_mode(struct ath5k_hw *ah, u8 ant_mode)
u8 def_ant, tx_ant, ee_mode;
u32 sta_id1 = 0;

+ /* if channel is not initialized yet we can't set the antennas
+ * so just store the mode. it will be set on the next reset */
+ if (channel == NULL) {
+ ah->ah_ant_mode = ant_mode;
+ return;
+ }
+
def_ant = ah->ah_def_ant;

switch (channel->hw_value & CHANNEL_MODES) {


2010-05-24 00:46:06

by Bruno Randolf

[permalink] [raw]
Subject: Re: [ath5k-devel] [PATCH v2 13/20] cfg80211: Add nl80211 antenna configuration

On Saturday 22 May 2010 02:11:02 Luis R. Rodriguez wrote:
> > > For legacy, keep it simple, use 3 settings, fixed_a, fixed_b,
> > > diversity, for all devices.
> >
> > did you not understand my examples why i think it makes sense to use a
> > bitmask for "legacy"? i think they are perfectly valid use-cases. do i
> > need to re- iterate them a third time?
>
> Hehe, well from what I gather is that you indicate some other legacy cards
> would have a very different setup than the typical two anntennas and
> diversity modes. I see those cases as being reallllllly rare and not
> worth considering. Did I miss anything, if so please smack me.

sorry, gotta smack you ;)

i'm talking about cards with 2 antennas (but still believe it's good to keep
the API flexible enough to be able to handle more antennas). i agree that
these setups are rare, but i think we should support them because it's easily
possible. why limit possibilities? also please note that it's perfectly
feasible to suport these modes with ath5k as well (it's just not implemented
yet):

* case 1: send on antenna 1, receive on antenna 2: why would you want to do
this? to have a low gain antenna for TX in order to keep within the regulatory
constraints and a high gain antenna for RX in order to receive weaker signals.
this means "speak softly, but listen harder". it can be useful especially for
outdoor links.

that would translate to a RX antenna bitmap of 0b01 and TX antenna 0b10.

* case 2: is the same like above, but using RX diversity on both antennas.

that would be RX antenna 0, give that we use "0" for diversity, and TX antenna
1.

* case 3: are special setups in research and development where people might
have more than 2 antennas or might want to do things which don't apparently
make much sense for normal operation. while this does not have a high
priority, i think it's good to have an API which can support a wide variety of
configurations.

> True, but note how the fact that you transmit over two antennas actually
> has regulatory implications. Now, ath9k handles this within ath9k_hw
> already but this itself seems like a worthy reason for this API to be
> separated. While I think it is great for ath9k_hw to do this, wouldn't it
> be nice if we can eventually instead expose the gain by using different
> chains at the same time and do the regulatory calculation for all devices
> within cfg80211?

yes, that would be good. i think we can add that in addition to the antenna
API i suggested.

> Right, and while that *works*, I think it would be clearer to just use a
> clear "diveristy" knob.

yes i agree. so we could use the special value of "0" to indicate diversity,
because it does not make sense to transmit on zero antennas - or create a flag
for "use diversity".

> > most of the other things you mention (need a reset/reassociate,
> > regulatory concerns...) are driver implementation issues, which can be
> > dealt with in the driver.
>
> Well so some of these things *could* be handled in mac80211 as well. For
> example, we may want to just dissociate upon a tx/rx chain setting change
> for all devices, but not for legacy. The regulatory stuff is another thing
> which could eventually be made more generic accross the board.

i see no reason why this could not be done with the bitmap API i suggested, or
additionally to it.

bruno

2010-05-24 09:37:27

by RHS Linux User

[permalink] [raw]
Subject: Re: [ath5k-devel] [PATCH v2 13/20] cfg80211: Add nl80211 antenna configuration


Hi,

One other *important* case:

on port A a low noise preamp and a receive antenna.

on port B a power amplifier and a seperate antenna

In this way transmit noise is ( mostly ) kept out of the low noise
receive channel.

warm regards,
wiz


On Mon, 24 May 2010, Bruno Randolf wrote:

> On Saturday 22 May 2010 02:11:02 Luis R. Rodriguez wrote:
> > > > For legacy, keep it simple, use 3 settings, fixed_a, fixed_b,
> > > > diversity, for all devices.
> > >
> > > did you not understand my examples why i think it makes sense to use a
> > > bitmask for "legacy"? i think they are perfectly valid use-cases. do i
> > > need to re- iterate them a third time?
> >
> > Hehe, well from what I gather is that you indicate some other legacy cards
> > would have a very different setup than the typical two anntennas and
> > diversity modes. I see those cases as being reallllllly rare and not
> > worth considering. Did I miss anything, if so please smack me.
>
> sorry, gotta smack you ;)
>
> i'm talking about cards with 2 antennas (but still believe it's good to keep
> the API flexible enough to be able to handle more antennas). i agree that
> these setups are rare, but i think we should support them because it's easily
> possible. why limit possibilities? also please note that it's perfectly
> feasible to suport these modes with ath5k as well (it's just not implemented
> yet):
>
> * case 1: send on antenna 1, receive on antenna 2: why would you want to do
> this? to have a low gain antenna for TX in order to keep within the regulatory
> constraints and a high gain antenna for RX in order to receive weaker signals.
> this means "speak softly, but listen harder". it can be useful especially for
> outdoor links.
>
> that would translate to a RX antenna bitmap of 0b01 and TX antenna 0b10.
>
> * case 2: is the same like above, but using RX diversity on both antennas.
>
> that would be RX antenna 0, give that we use "0" for diversity, and TX antenna
> 1.
>
> * case 3: are special setups in research and development where people might
> have more than 2 antennas or might want to do things which don't apparently
> make much sense for normal operation. while this does not have a high
> priority, i think it's good to have an API which can support a wide variety of
> configurations.
>
> > True, but note how the fact that you transmit over two antennas actually
> > has regulatory implications. Now, ath9k handles this within ath9k_hw
> > already but this itself seems like a worthy reason for this API to be
> > separated. While I think it is great for ath9k_hw to do this, wouldn't it
> > be nice if we can eventually instead expose the gain by using different
> > chains at the same time and do the regulatory calculation for all devices
> > within cfg80211?
>
> yes, that would be good. i think we can add that in addition to the antenna
> API i suggested.
>
> > Right, and while that *works*, I think it would be clearer to just use a
> > clear "diveristy" knob.
>
> yes i agree. so we could use the special value of "0" to indicate diversity,
> because it does not make sense to transmit on zero antennas - or create a flag
> for "use diversity".
>
> > > most of the other things you mention (need a reset/reassociate,
> > > regulatory concerns...) are driver implementation issues, which can be
> > > dealt with in the driver.
> >
> > Well so some of these things *could* be handled in mac80211 as well. For
> > example, we may want to just dissociate upon a tx/rx chain setting change
> > for all devices, but not for legacy. The regulatory stuff is another thing
> > which could eventually be made more generic accross the board.
>
> i see no reason why this could not be done with the bitmap API i suggested, or
> additionally to it.
>
> bruno
> _______________________________________________
> ath5k-devel mailing list
> [email protected]
> https://lists.ath5k.org/mailman/listinfo/ath5k-devel
>


2010-05-20 00:51:46

by Luis R. Rodriguez

[permalink] [raw]
Subject: Re: [ath5k-devel] [PATCH v2 13/20] cfg80211: Add nl80211 antenna configuration

On Wed, May 19, 2010 at 05:35:40PM -0700, Bruno Randolf wrote:
> On Thursday 20 May 2010 02:07:25 you wrote:
> > On Tue, May 18, 2010 at 6:31 PM, Bruno Randolf <[email protected]> wrote:
> > > + * @NL80211_ATTR_ANTENNA_TX: Bitmap of antennas to use for transmitting.
> > > + * @NL80211_ATTR_ANTENNA_RX: Bitmap of antennas to use for receiving.
> >
> > This gets the job done, but that's it. The API defined allows for a
> > hugely loose implementation on each driver.
>
> i tried to define it like this, in the commit log:
>
> The antenna configuration is defined as a bitmap of allowed antennas. This
> bitmap is 8 bit at the moment, each bit representing one antenna. If
> multiple antennas are selected, the driver may use diversity for receive
> and transmit.
>
> is this not precise enough?

No, the commit log is just a placeholder of information, if you want to
define API you do it through the kdoc so you can slap people when then
submit patches that do not follow it. The kdoc above allows the flexibility
you were looking for but that I do not think we should have since it will
confuse the users who want to tune antenna settings on different drivers.

I'd much prefer to see a consistant API for antenna settings so that if
they know how to do it with ath5k, then they know how to do it for b43,
or whatever.

> i am mostly concerned with what i believe is the
> most common usecase (selecting one fixed antenna, or antenna diversity).

In that case, then I recommend to restrict your patch to that case, and
forget about a bitmask. Since lagcy just has antenna A, B, or auto, how
about defining an API for just that?

> i'd say this is 99% of all use cases.

For now, yes. I'm thinking about 802.11n though and for that we can
just wait, and use some other API once someone with more time wants
to iron it out.

> but this API would also allow us to define
> more advanved things like 'transmit on antenna 1, receive on antenna 2".

Sure.

> i know that there are possibly more crazy (and very rare) configurations, like
> use several panel antennas + omni, which can't be configured with this API,

How about we ignore those then on this API.

> but it's hard to find a common API for all possibilities, and i think it would
> be possible to extend the API later on for such cases.

Understood, how about just defining something very basic and simple
for legacy based operation mode?

> > And yet for another driver it could be something completely different
> > in usersepace.
>
> what do you think that could be, realistically, given the above definition?

Well, anything that has to do with tx / rx antennas.

> > I think it would be better for us to define a static
> > API for all legacy cards and another for 802.11n cards, or unify them
> > but to be very specific about the API for antenna settings/chainmask
> > settings.
>
> sure. any suggestions?

Sure how about FIXED_A, FIXED_B, DIVERSITY ?

Luis

2010-05-20 22:24:28

by Luis R. Rodriguez

[permalink] [raw]
Subject: Re: [ath5k-devel] [PATCH v2 13/20] cfg80211: Add nl80211 antenna configuration

Just for clarification, these e-mails are on a public mailing list.

On Thu, May 20, 2010 at 03:14:01PM -0700, Sam Ng wrote:
> Just to be accurate, we do have pre-lln devices that support
> more than 1 antenna ie. slow/fast diversity

Right, this is the typical fixed anntena a, fixed anntena b, or
antenna diversity mode.

> and rangemax style 'beamforming'. However, all these devices
> have only 1 chain.

Thanks for the clarification Sam.

Luis

2010-05-20 12:50:16

by Nick Kossifidis

[permalink] [raw]
Subject: Re: [PATCH v2 04/20] ath5k: move noise floor calibration into tasklet

2010/5/19 Bruno Randolf <[email protected]>:
> Seperate noise floor calibration from other PHY calibration and move it to the
> tasklet. This is the first step to more separation of different calibrations.
>
> Also move out ath5k_hw_request_rfgain_probe(ah) so we have one clean function
> for I/Q calibration on 5111x parts.
>
> Signed-off-by: Bruno Randolf <[email protected]>
> ---
>  drivers/net/wireless/ath/ath5k/ath5k.h |    1 +
>  drivers/net/wireless/ath/ath5k/base.c  |    1 +
>  drivers/net/wireless/ath/ath5k/phy.c   |   33 ++++++++++----------------------
>  3 files changed, 12 insertions(+), 23 deletions(-)
>
> diff --git a/drivers/net/wireless/ath/ath5k/ath5k.h b/drivers/net/wireless/ath/ath5k/ath5k.h
> index 2785946..131e8b3 100644
> --- a/drivers/net/wireless/ath/ath5k/ath5k.h
> +++ b/drivers/net/wireless/ath/ath5k/ath5k.h
> @@ -1270,6 +1270,7 @@ int ath5k_hw_channel(struct ath5k_hw *ah, struct ieee80211_channel *channel);
>  void ath5k_hw_init_nfcal_hist(struct ath5k_hw *ah);
>  int ath5k_hw_phy_calibrate(struct ath5k_hw *ah,
>                           struct ieee80211_channel *channel);
> +void ath5k_hw_update_noise_floor(struct ath5k_hw *ah);
>  /* Spur mitigation */
>  bool ath5k_hw_chan_has_spur_noise(struct ath5k_hw *ah,
>                                  struct ieee80211_channel *channel);
> diff --git a/drivers/net/wireless/ath/ath5k/base.c b/drivers/net/wireless/ath/ath5k/base.c
> index de59a6e..88c7314 100644
> --- a/drivers/net/wireless/ath/ath5k/base.c
> +++ b/drivers/net/wireless/ath/ath5k/base.c
> @@ -2806,6 +2806,7 @@ ath5k_tasklet_calibrate(unsigned long data)
>                        ieee80211_frequency_to_channel(
>                                sc->curchan->center_freq));
>
> +       ath5k_hw_update_noise_floor(ah);
>        /* Wake queues */
>        ieee80211_wake_queues(sc->hw);
>
> diff --git a/drivers/net/wireless/ath/ath5k/phy.c b/drivers/net/wireless/ath/ath5k/phy.c
> index 3ce9afb..0b24081 100644
> --- a/drivers/net/wireless/ath/ath5k/phy.c
> +++ b/drivers/net/wireless/ath/ath5k/phy.c
> @@ -1167,7 +1167,7 @@ static s16 ath5k_hw_get_median_noise_floor(struct ath5k_hw *ah)
>  * The median of the values in the history is then loaded into the
>  * hardware for its own use for RSSI and CCA measurements.
>  */
> -static void ath5k_hw_update_noise_floor(struct ath5k_hw *ah)
> +void ath5k_hw_update_noise_floor(struct ath5k_hw *ah)
>  {
>        struct ath5k_eeprom_info *ee = &ah->ah_capabilities.cap_eeprom;
>        u32 val;
> @@ -1248,7 +1248,6 @@ static void ath5k_hw_update_noise_floor(struct ath5k_hw *ah)
>  /*
>  * Perform a PHY calibration on RF5110
>  * -Fix BPSK/QAM Constellation (I/Q correction)
> - * -Calculate Noise Floor
>  */
>  static int ath5k_hw_rf5110_calibrate(struct ath5k_hw *ah,
>                struct ieee80211_channel *channel)
> @@ -1335,8 +1334,6 @@ static int ath5k_hw_rf5110_calibrate(struct ath5k_hw *ah,
>                return ret;
>        }
>
> -       ath5k_hw_update_noise_floor(ah);
> -
>        /*
>         * Re-enable RX/TX and beacons
>         */
> @@ -1348,10 +1345,10 @@ static int ath5k_hw_rf5110_calibrate(struct ath5k_hw *ah,
>  }
>
>  /*
> - * Perform a PHY calibration on RF5111/5112 and newer chips
> + * Perform I/Q calibration on RF5111/5112 and newer chips
>  */
> -static int ath5k_hw_rf511x_calibrate(struct ath5k_hw *ah,
> -               struct ieee80211_channel *channel)
> +static int
> +ath5k_hw_rf511x_iq_calibrate(struct ath5k_hw *ah)
>  {
>        u32 i_pwr, q_pwr;
>        s32 iq_corr, i_coff, i_coffd, q_coff, q_coffd;
> @@ -1360,10 +1357,9 @@ static int ath5k_hw_rf511x_calibrate(struct ath5k_hw *ah,
>
>        if (!ah->ah_calibration ||
>                ath5k_hw_reg_read(ah, AR5K_PHY_IQ) & AR5K_PHY_IQ_RUN)
> -               goto done;
> +               return 0;
>
>        /* Calibration has finished, get the results and re-run */
> -
>        /* work around empty results which can apparently happen on 5212 */
>        for (i = 0; i <= 10; i++) {
>                iq_corr = ath5k_hw_reg_read(ah, AR5K_PHY_IQRES_CAL_CORR);
> @@ -1384,7 +1380,7 @@ static int ath5k_hw_rf511x_calibrate(struct ath5k_hw *ah,
>
>        /* protect against divide by 0 and loss of sign bits */
>        if (i_coffd == 0 || q_coffd < 2)
> -               goto done;
> +               return -1;
>
>        i_coff = (-iq_corr) / i_coffd;
>        i_coff = clamp(i_coff, -32, 31); /* signed 6 bit */
> @@ -1410,17 +1406,6 @@ static int ath5k_hw_rf511x_calibrate(struct ath5k_hw *ah,
>                        AR5K_PHY_IQ_CAL_NUM_LOG_MAX, 15);
>        AR5K_REG_ENABLE_BITS(ah, AR5K_PHY_IQ, AR5K_PHY_IQ_RUN);
>
> -done:
> -
> -       /* TODO: Separate noise floor calibration from I/Q calibration
> -        * since noise floor calibration interrupts rx path while I/Q
> -        * calibration doesn't. We don't need to run noise floor calibration
> -        * as often as I/Q calibration.*/
> -       ath5k_hw_update_noise_floor(ah);
> -
> -       /* Initiate a gain_F calibration */
> -       ath5k_hw_request_rfgain_probe(ah);
> -
>        return 0;
>  }
>
> @@ -1434,8 +1419,10 @@ int ath5k_hw_phy_calibrate(struct ath5k_hw *ah,
>
>        if (ah->ah_radio == AR5K_RF5110)
>                ret = ath5k_hw_rf5110_calibrate(ah, channel);
> -       else
> -               ret = ath5k_hw_rf511x_calibrate(ah, channel);
> +       else {
> +               ret = ath5k_hw_rf511x_iq_calibrate(ah);
> +               ath5k_hw_request_rfgain_probe(ah);
> +       }
>
>        return ret;
>  }
>

Acked-by: Nick Kossifidis <[email protected]>



--
GPG ID: 0xD21DB2DB
As you read this post global entropy rises. Have Fun ;-)
Nick

2010-05-19 01:31:57

by Bruno Randolf

[permalink] [raw]
Subject: [PATCH v2 01/20] ath5k: add debugfs file for queue debugging

Signed-off-by: Bruno Randolf <[email protected]>
---
drivers/net/wireless/ath/ath5k/debug.c | 66 ++++++++++++++++++++++++++++++++
drivers/net/wireless/ath/ath5k/debug.h | 1
2 files changed, 67 insertions(+), 0 deletions(-)

diff --git a/drivers/net/wireless/ath/ath5k/debug.c b/drivers/net/wireless/ath/ath5k/debug.c
index 6fb5c5f..c77a6ad 100644
--- a/drivers/net/wireless/ath/ath5k/debug.c
+++ b/drivers/net/wireless/ath/ath5k/debug.c
@@ -729,6 +729,66 @@ static const struct file_operations fops_ani = {
};


+/* debugfs: queues etc */
+
+static ssize_t read_file_queue(struct file *file, char __user *user_buf,
+ size_t count, loff_t *ppos)
+{
+ struct ath5k_softc *sc = file->private_data;
+ char buf[700];
+ unsigned int len = 0;
+
+ struct ath5k_txq *txq;
+ struct ath5k_buf *bf, *bf0;
+ int i, n = 0;
+
+ len += snprintf(buf+len, sizeof(buf)-len,
+ "available txbuffers: %d\n", sc->txbuf_len);
+
+ for (i = 0; i < ARRAY_SIZE(sc->txqs); i++) {
+ txq = &sc->txqs[i];
+
+ len += snprintf(buf+len, sizeof(buf)-len,
+ "%02d: %ssetup\n", i, txq->setup ? "" : "not ");
+
+ if (!txq->setup)
+ continue;
+
+ list_for_each_entry_safe(bf, bf0, &txq->q, list)
+ n++;
+ len += snprintf(buf+len, sizeof(buf)-len, " len: %d\n", n);
+ }
+
+ return simple_read_from_buffer(user_buf, count, ppos, buf, len);
+}
+
+static ssize_t write_file_queue(struct file *file,
+ const char __user *userbuf,
+ size_t count, loff_t *ppos)
+{
+ struct ath5k_softc *sc = file->private_data;
+ char buf[20];
+
+ if (copy_from_user(buf, userbuf, min(count, sizeof(buf))))
+ return -EFAULT;
+
+ if (strncmp(buf, "start", 5) == 0)
+ ieee80211_wake_queues(sc->hw);
+ else if (strncmp(buf, "stop", 4) == 0)
+ ieee80211_stop_queues(sc->hw);
+
+ return count;
+}
+
+
+static const struct file_operations fops_queue = {
+ .read = read_file_queue,
+ .write = write_file_queue,
+ .open = ath5k_debugfs_open,
+ .owner = THIS_MODULE,
+};
+
+
/* init */

void
@@ -772,6 +832,11 @@ ath5k_debug_init_device(struct ath5k_softc *sc)
S_IWUSR | S_IRUSR,
sc->debug.debugfs_phydir, sc,
&fops_ani);
+
+ sc->debug.debugfs_queue = debugfs_create_file("queue",
+ S_IWUSR | S_IRUSR,
+ sc->debug.debugfs_phydir, sc,
+ &fops_queue);
}

void
@@ -790,6 +855,7 @@ ath5k_debug_finish_device(struct ath5k_softc *sc)
debugfs_remove(sc->debug.debugfs_antenna);
debugfs_remove(sc->debug.debugfs_frameerrors);
debugfs_remove(sc->debug.debugfs_ani);
+ debugfs_remove(sc->debug.debugfs_queue);
debugfs_remove(sc->debug.debugfs_phydir);
}

diff --git a/drivers/net/wireless/ath/ath5k/debug.h b/drivers/net/wireless/ath/ath5k/debug.h
index ddd5b3a..c7a0769 100644
--- a/drivers/net/wireless/ath/ath5k/debug.h
+++ b/drivers/net/wireless/ath/ath5k/debug.h
@@ -77,6 +77,7 @@ struct ath5k_dbg_info {
struct dentry *debugfs_antenna;
struct dentry *debugfs_frameerrors;
struct dentry *debugfs_ani;
+ struct dentry *debugfs_queue;
};

/**


2010-06-04 21:21:47

by Luis R. Rodriguez

[permalink] [raw]
Subject: Re: [ath5k-devel] [PATCH v2 13/20] cfg80211: Add nl80211 antenna configuration

On Fri, Jun 04, 2010 at 12:30:09PM -0700, John W. Linville wrote:
> On Wed, May 19, 2010 at 10:31:48AM +0900, Bruno Randolf wrote:
> > Allow setting TX and RX antenna configuration via nl80211/cfg80211.
> >
> > The antenna configuration is defined as a bitmap of allowed antennas. This
> > bitmap is 8 bit at the moment, each bit representing one antenna. If multiple
> > antennas are selected, the driver may use diversity for receive and transmit.
> > This allows for a simple, yet flexible configuration interface for antennas,
> > while drivers may reject configurations they cannot support.
> >
> > Signed-off-by: Bruno Randolf <[email protected]>
>
> This spawned a long thread, that isn't clearly resolved IMHO. So, I
> haven't merged it, or the other patches that follow it in this series.
>
> Am I mistaken? Did this get resolved? If not, are some of the later
> patches still interesting?
>
> Unless I hear otherwise, I'll be dropping the rest of this series...

I don't think the API is defined clean enough yet and can yield
inconsistant interpretations. I'd like to see this clarified on
the documentation for both legacy and 802.11n. If the plan is
to support only legacy right now then I think the API can be
simpler and clearer with only three options passed, Fixed_A
Fixed_B and diversity.

Luis

2010-06-04 21:53:46

by Luis R. Rodriguez

[permalink] [raw]
Subject: Re: [ath5k-devel] [PATCH v2 13/20] cfg80211: Add nl80211 antenna configuration

On Fri, Jun 4, 2010 at 2:21 PM, Luis R. Rodriguez
<[email protected]> wrote:
> On Fri, Jun 04, 2010 at 12:30:09PM -0700, John W. Linville wrote:
>> On Wed, May 19, 2010 at 10:31:48AM +0900, Bruno Randolf wrote:
>> > Allow setting TX and RX antenna configuration via nl80211/cfg80211.
>> >
>> > The antenna configuration is defined as a bitmap of allowed antennas. This
>> > bitmap is 8 bit at the moment, each bit representing one antenna. If multiple
>> > antennas are selected, the driver may use diversity for receive and transmit.
>> > This allows for a simple, yet flexible configuration interface for antennas,
>> > while drivers may reject configurations they cannot support.
>> >
>> > Signed-off-by: Bruno Randolf <[email protected]>
>>
>> This spawned a long thread, that isn't clearly resolved IMHO.  So, I
>> haven't merged it, or the other patches that follow it in this series.
>>
>> Am I mistaken?  Did this get resolved?  If not, are some of the later
>> patches still interesting?
>>
>> Unless I hear otherwise, I'll be dropping the rest of this series...
>
> I don't think the API is defined clean enough yet and can yield
> inconsistant interpretations. I'd like to see this clarified on
> the documentation for both legacy and 802.11n. If the plan is
> to support only legacy right now then I think the API can be
> simpler and clearer with only three options passed, Fixed_A
> Fixed_B and diversity.

Oh and the rest of the series should not be dropped but I did think it
would have been prudent for the patches to be separated and that this
particular set of patches for antenna API be separated out to another
series so that the other stuff can get merged.

Luis

2010-06-04 19:45:13

by John W. Linville

[permalink] [raw]
Subject: Re: [PATCH v2 13/20] cfg80211: Add nl80211 antenna configuration

On Wed, May 19, 2010 at 10:31:48AM +0900, Bruno Randolf wrote:
> Allow setting TX and RX antenna configuration via nl80211/cfg80211.
>
> The antenna configuration is defined as a bitmap of allowed antennas. This
> bitmap is 8 bit at the moment, each bit representing one antenna. If multiple
> antennas are selected, the driver may use diversity for receive and transmit.
> This allows for a simple, yet flexible configuration interface for antennas,
> while drivers may reject configurations they cannot support.
>
> Signed-off-by: Bruno Randolf <[email protected]>

This spawned a long thread, that isn't clearly resolved IMHO. So, I
haven't merged it, or the other patches that follow it in this series.

Am I mistaken? Did this get resolved? If not, are some of the later
patches still interesting?

Unless I hear otherwise, I'll be dropping the rest of this series...

John
--
John W. Linville Someday the world will need a hero, and you
[email protected] might be all we have. Be ready.

2010-06-07 03:45:30

by Bruno Randolf

[permalink] [raw]
Subject: Re: [ath5k-devel] [PATCH v2 13/20] cfg80211: Add nl80211 antenna configuration

On Saturday 05 June 2010 06:21:45 you wrote:
> I don't think the API is defined clean enough yet and can yield
> inconsistant interpretations. I'd like to see this clarified on
> the documentation for both legacy and 802.11n. If the plan is
> to support only legacy right now then I think the API can be
> simpler and clearer with only three options passed, Fixed_A
> Fixed_B and diversity.

luis, i thought we had this resolved during the last thread... i gave you
examples why fixed_a, fixed_b and diversity is not enough.

anyhow, there are a few places where i have to make the definition clearer in
order to avoid inconsistent interpretations, that's right. i will resend this
so we can continue any discussions based on that.

john, i will resend the rest of the series, the ath5k part and the antenna
configuration as separate series. sorry for mixing that up before...

bruno