Here's the long expected finished serialization series for ath9k
which fixes usage of ath9k on PCI devices with SMP systems. The
CPU notifier is perhaps overkill but technically it is correct.
I finally managed to get a system to test this thing on and was able
to reproduce and at least for me and Adel Gadllah it fixes the issue
on STA, AP and IBSS modes.
I would usually wait on for more testing but at this point I'm quite
happy with the results. The serialization components are candidates for
stable, the cpu notifier is not required we assume we don't want to support
CPU hotplug for 2.6.27 and 2.6.28 :)
Luis R. Rodriguez (4):
ath9k: implement IO serialization
ath9k: AR9280 PCI devices must serialize IO as well
ath9k: add cpu notifier to enhance device configuration
ath9k: remove dummy PCI "retry timeout" fix
drivers/net/wireless/ath9k/ath9k.h | 8 ++++
drivers/net/wireless/ath9k/hw.c | 73 +++++++++++++++++++++++++++++++++++-
drivers/net/wireless/ath9k/hw.h | 11 ++++-
drivers/net/wireless/ath9k/main.c | 30 +++++++++++++++
drivers/net/wireless/ath9k/pci.c | 22 ++---------
5 files changed, 123 insertions(+), 21 deletions(-)
Remove the PCI retry timeout code as that was just taken from ipw2100
due to historical reasons but in reality its a no-op, additionally its
simply incorrect as each PCI devices has its own custom PCI configuration
space on PCI config space >= 0x40. Not to mention we were trying to write
0 to a place that already has 0 on it.
Cc: Matthew Garrett <[email protected]>
Cc: Ben Cahill <[email protected]>
Cc: Inaky Perez-Gonzalez <[email protected]>
Tested-by: Adel Gadllah <[email protected]>
Signed-off-by: Luis R. Rodriguez <[email protected]>
---
drivers/net/wireless/ath9k/pci.c | 18 ------------------
1 files changed, 0 insertions(+), 18 deletions(-)
diff --git a/drivers/net/wireless/ath9k/pci.c b/drivers/net/wireless/ath9k/pci.c
index c1ba207..a1a9b90 100644
--- a/drivers/net/wireless/ath9k/pci.c
+++ b/drivers/net/wireless/ath9k/pci.c
@@ -87,7 +87,6 @@ static int ath_pci_probe(struct pci_dev *pdev, const struct pci_device_id *id)
struct ath_softc *sc;
struct ieee80211_hw *hw;
u8 csz;
- u32 val;
int ret = 0;
struct ath_hw *ah;
@@ -134,14 +133,6 @@ static int ath_pci_probe(struct pci_dev *pdev, const struct pci_device_id *id)
pci_set_master(pdev);
- /*
- * Disable the RETRY_TIMEOUT register (0x41) to keep
- * PCI Tx retries from interfering with C3 CPU state.
- */
- pci_read_config_dword(pdev, 0x40, &val);
- if ((val & 0x0000ff00) != 0)
- pci_write_config_dword(pdev, 0x40, val & 0xffff00ff);
-
ret = pci_request_region(pdev, 0, "ath9k");
if (ret) {
dev_err(&pdev->dev, "PCI memory region reserve error\n");
@@ -257,21 +248,12 @@ static int ath_pci_resume(struct pci_dev *pdev)
struct ieee80211_hw *hw = pci_get_drvdata(pdev);
struct ath_wiphy *aphy = hw->priv;
struct ath_softc *sc = aphy->sc;
- u32 val;
int err;
err = pci_enable_device(pdev);
if (err)
return err;
pci_restore_state(pdev);
- /*
- * Suspend/Resume resets the PCI configuration space, so we have to
- * re-disable the RETRY_TIMEOUT register (0x41) to keep
- * PCI Tx retries from interfering with C3 CPU state
- */
- pci_read_config_dword(pdev, 0x40, &val);
- if ((val & 0x0000ff00) != 0)
- pci_write_config_dword(pdev, 0x40, val & 0xffff00ff);
/* Enable LED */
ath9k_hw_cfg_output(sc->sc_ah, ATH_LED_PIN,
--
1.6.0.6
Upon CPU Hotplug the number of CPUs can change and we can tweak
our driver configuration as such. Keep in mind CPU hotplug also
occurs during suspend/resume.
Since we have a notifier now we can rely on num_present_cpus()
rather than num_possible_cpus() to enable/disable serialization.
Signed-off-by: Luis R. Rodriguez <[email protected]>
---
drivers/net/wireless/ath9k/ath9k.h | 7 +++++
drivers/net/wireless/ath9k/hw.c | 49 ++++++++++++++++++++++-------------
drivers/net/wireless/ath9k/hw.h | 3 ++
drivers/net/wireless/ath9k/main.c | 29 +++++++++++++++++++++
drivers/net/wireless/ath9k/pci.c | 4 +++
5 files changed, 74 insertions(+), 18 deletions(-)
diff --git a/drivers/net/wireless/ath9k/ath9k.h b/drivers/net/wireless/ath9k/ath9k.h
index 90ee2e3..a2e9b1e 100644
--- a/drivers/net/wireless/ath9k/ath9k.h
+++ b/drivers/net/wireless/ath9k/ath9k.h
@@ -22,6 +22,7 @@
#include <net/mac80211.h>
#include <linux/leds.h>
#include <linux/rfkill.h>
+#include <linux/cpu.h>
#include "hw.h"
#include "rc.h"
@@ -554,6 +555,9 @@ struct ath_bus_ops {
struct ath_wiphy;
+int ath9k_cpu_callback(struct notifier_block *nfb,
+ unsigned long action, void *hcpu);
+
struct ath_softc {
struct ieee80211_hw *hw;
struct device *dev;
@@ -573,6 +577,9 @@ struct ath_softc {
unsigned long wiphy_scheduler_int;
int wiphy_scheduler_index;
+ /* This should or _needs_ to be __cpuinit (?) */
+ struct notifier_block cpu_notifer;
+
struct tasklet_struct intr_tq;
struct tasklet_struct bcon_tasklet;
struct ath_hw *sc_ah;
diff --git a/drivers/net/wireless/ath9k/hw.c b/drivers/net/wireless/ath9k/hw.c
index 05df570..3bbc257 100644
--- a/drivers/net/wireless/ath9k/hw.c
+++ b/drivers/net/wireless/ath9k/hw.c
@@ -16,6 +16,7 @@
#include <linux/io.h>
#include <asm/unaligned.h>
+#include <linux/cpu.h>
#include "ath9k.h"
#include "initvals.h"
@@ -385,6 +386,35 @@ static const char *ath9k_hw_devname(u16 devid)
return NULL;
}
+/*
+ * We can slap on to here things we need to tune depending
+ * on the number of CPUs. This will happen on CPU hotplug
+ * which is also used for suspend/resume.
+ */
+void ath9k_hw_config_for_cpus(struct ath_hw *ah)
+{
+ /*
+ * We need this for PCI devices only (Cardbus, PCI, miniPCI)
+ * _and_ if on non-uniprocessor systems (Multiprocessor/HT).
+ * This means we use it for all AR5416 devices, and the few
+ * minor PCI AR9280 devices out there.
+ *
+ * Serialization is required because these devices do not handle
+ * well the case of two concurrent reads/writes due to the latency
+ * involved. During one read/write another read/write can be issued
+ * on another CPU while the previous read/write may still be working
+ * on our hardware, if we hit this case the hardware poops in a loop.
+ * We prevent this by serializing reads and writes.
+ *
+ * This issue is not present on PCI-Express devices or pre-AR5416
+ * devices (legacy, 802.11abg).
+ */
+ if (num_present_cpus() > 1)
+ ah->config.serialize_regmode = SER_REG_MODE_AUTO;
+ else
+ ah->config.serialize_regmode = SER_REG_MODE_OFF;
+}
+
static void ath9k_hw_set_defaults(struct ath_hw *ah)
{
int i;
@@ -424,24 +454,7 @@ static void ath9k_hw_set_defaults(struct ath_hw *ah)
ah->config.intr_mitigation = 1;
- /*
- * We need this for PCI devices only (Cardbus, PCI, miniPCI)
- * _and_ if on non-uniprocessor systems (Multiprocessor/HT).
- * This means we use it for all AR5416 devices, and the few
- * minor PCI AR9280 devices out there.
- *
- * Serialization is required because these devices do not handle
- * well the case of two concurrent reads/writes due to the latency
- * involved. During one read/write another read/write can be issued
- * on another CPU while the previous read/write may still be working
- * on our hardware, if we hit this case the hardware poops in a loop.
- * We prevent this by serializing reads and writes.
- *
- * This issue is not present on PCI-Express devices or pre-AR5416
- * devices (legacy, 802.11abg).
- */
- if (num_possible_cpus() > 1)
- ah->config.serialize_regmode = SER_REG_MODE_AUTO;
+ ath9k_hw_config_for_cpus(ah);
}
static struct ath_hw *ath9k_hw_newstate(u16 devid, struct ath_softc *sc,
diff --git a/drivers/net/wireless/ath9k/hw.h b/drivers/net/wireless/ath9k/hw.h
index 790bfa9..c2d32bb 100644
--- a/drivers/net/wireless/ath9k/hw.h
+++ b/drivers/net/wireless/ath9k/hw.h
@@ -570,6 +570,9 @@ struct ath_hw {
void ath9k_iowrite32(struct ath_hw *ah, u32 reg_offset, u32 val);
unsigned int ath9k_ioread32(struct ath_hw *ah, u32 reg_offset);
+/* Lets us tweak the device per CPU changes */
+void ath9k_hw_config_for_cpus(struct ath_hw *ah);
+
/* Attach, Detach, Reset */
const char *ath9k_hw_probe(u16 vendorid, u16 devid);
void ath9k_hw_detach(struct ath_hw *ah);
diff --git a/drivers/net/wireless/ath9k/main.c b/drivers/net/wireless/ath9k/main.c
index 2ad6414..94ea72a 100644
--- a/drivers/net/wireless/ath9k/main.c
+++ b/drivers/net/wireless/ath9k/main.c
@@ -2787,6 +2787,35 @@ struct ieee80211_ops ath9k_ops = {
.sw_scan_complete = ath9k_sw_scan_complete,
};
+int ath9k_cpu_callback(struct notifier_block *nfb,
+ unsigned long action, void *hcpu)
+{
+ struct ath_softc *sc = container_of(nfb, struct ath_softc, cpu_notifer);
+ int old_serial_mode;
+
+ old_serial_mode = sc->sc_ah->config.serialize_regmode;
+
+ switch (action) {
+ case CPU_ONLINE:
+ case CPU_ONLINE_FROZEN:
+ ath9k_hw_config_for_cpus(sc->sc_ah);
+ break;
+ case CPU_DEAD:
+ case CPU_DEAD_FROZEN:
+ ath9k_hw_config_for_cpus(sc->sc_ah);
+ break;
+ }
+
+ if (unlikely(old_serial_mode != sc->sc_ah->config.serialize_regmode)) {
+ DPRINTF(sc, ATH_DBG_RESET,
+ "serialize_regmode has changed due to CPU "
+ "count to %d\n",
+ sc->sc_ah->config.serialize_regmode);
+ }
+
+ return NOTIFY_OK;
+}
+
static struct {
u32 version;
const char * name;
diff --git a/drivers/net/wireless/ath9k/pci.c b/drivers/net/wireless/ath9k/pci.c
index 9a58baa..c1ba207 100644
--- a/drivers/net/wireless/ath9k/pci.c
+++ b/drivers/net/wireless/ath9k/pci.c
@@ -181,6 +181,9 @@ static int ath_pci_probe(struct pci_dev *pdev, const struct pci_device_id *id)
goto bad3;
}
+ sc->cpu_notifer.notifier_call = ath9k_cpu_callback;
+ register_hotcpu_notifier(&sc->cpu_notifer);
+
/* setup interrupt service routine */
if (request_irq(pdev->irq, ath_isr, IRQF_SHARED, "ath", sc)) {
@@ -223,6 +226,7 @@ static void ath_pci_remove(struct pci_dev *pdev)
struct ath_wiphy *aphy = hw->priv;
struct ath_softc *sc = aphy->sc;
+ unregister_hotcpu_notifier(&sc->cpu_notifer);
ath_cleanup(sc);
}
--
1.6.0.6
Cc: [email protected]
Signed-off-by: Luis R. Rodriguez <[email protected]>
---
drivers/net/wireless/ath9k/hw.c | 9 ++++++++-
1 files changed, 8 insertions(+), 1 deletions(-)
diff --git a/drivers/net/wireless/ath9k/hw.c b/drivers/net/wireless/ath9k/hw.c
index c6e6899..05df570 100644
--- a/drivers/net/wireless/ath9k/hw.c
+++ b/drivers/net/wireless/ath9k/hw.c
@@ -660,8 +660,15 @@ static struct ath_hw *ath9k_hw_do_attach(u16 devid, struct ath_softc *sc,
goto bad;
}
+ /*
+ * All PCI devices should be put here.
+ * XXX: remove ah->is_pciexpress and use pdev->is_pcie, then
+ * we can just check for !pdev->is_pcie here, but
+ * consideration must be taken for handling AHB as well.
+ */
if (ah->config.serialize_regmode == SER_REG_MODE_AUTO) {
- if (ah->hw_version.macVersion == AR_SREV_VERSION_5416_PCI) {
+ if (ah->hw_version.macVersion == AR_SREV_VERSION_5416_PCI ||
+ (AR_SREV_9280(ah) && !ah->is_pciexpress)) {
ah->config.serialize_regmode =
SER_REG_MODE_ON;
} else {
--
1.6.0.6
On Tuesday 10 March 2009 07:45:29 W. van den Akker wrote:
> Thanks Luis,
>
> I will test it on my system.
>
> To be continued...
>
> gr,
> WIllem
>
I tested this one on my system with IPerf. It looks good.
Until now I didnt get a hang anymore.
Used IPerf -P 2|4|8 and all went well..
Tested with the wireless-testing of 10-3 (so not with the inline
functions mentioned earlier by Luis).
Looks like we have a winner :)
gr,
Willem
--
This message has been scanned for viruses and
dangerous content by MailScanner, and is
believed to be clean.
Thanks Luis,
I will test it on my system.
To be continued...
gr,
WIllem
> Here's the long expected finished serialization series for ath9k
> which fixes usage of ath9k on PCI devices with SMP systems. The
> CPU notifier is perhaps overkill but technically it is correct.
>
> I finally managed to get a system to test this thing on and was able
> to reproduce and at least for me and Adel Gadllah it fixes the issue
> on STA, AP and IBSS modes.
>
> I would usually wait on for more testing but at this point I'm quite
> happy with the results. The serialization components are candidates for
> stable, the cpu notifier is not required we assume we don't want to
> support
> CPU hotplug for 2.6.27 and 2.6.28 :)
>
> Luis R. Rodriguez (4):
> ath9k: implement IO serialization
> ath9k: AR9280 PCI devices must serialize IO as well
> ath9k: add cpu notifier to enhance device configuration
> ath9k: remove dummy PCI "retry timeout" fix
>
> drivers/net/wireless/ath9k/ath9k.h | 8 ++++
> drivers/net/wireless/ath9k/hw.c | 73
> +++++++++++++++++++++++++++++++++++-
> drivers/net/wireless/ath9k/hw.h | 11 ++++-
> drivers/net/wireless/ath9k/main.c | 30 +++++++++++++++
> drivers/net/wireless/ath9k/pci.c | 22 ++---------
> 5 files changed, 123 insertions(+), 21 deletions(-)
>
> --
> To unsubscribe from this list: send the line "unsubscribe linux-wireless"
> in
> the body of a message to [email protected]
> More majordomo info at http://vger.kernel.org/majordomo-info.html
>
> --
> This message has been scanned for viruses and
> dangerous content by MailScanner, and is
> believed to be clean.
>
>
--
This message has been scanned for viruses and
dangerous content by MailScanner, and is
believed to be clean.
All 802.11n PCI devices (Cardbus, PCI, mini-PCI) require
serialization of IO when on non-uniprocessor systems. PCI
express devices not not require this.
This should fix our only last standing open ath9k kernel.org
bugzilla bug report:
http://bugzilla.kernel.org/show_bug.cgi?id=12110
A port is probably required to older kernels and I can work on
that.
Tested-by: Adel Gadllah <[email protected]>
Signed-off-by: Luis R. Rodriguez <[email protected]>
---
drivers/net/wireless/ath9k/ath9k.h | 1 +
drivers/net/wireless/ath9k/hw.c | 51 ++++++++++++++++++++++++++++++++++++
drivers/net/wireless/ath9k/hw.h | 8 ++++-
drivers/net/wireless/ath9k/main.c | 1 +
4 files changed, 59 insertions(+), 2 deletions(-)
diff --git a/drivers/net/wireless/ath9k/ath9k.h b/drivers/net/wireless/ath9k/ath9k.h
index f0b105a..90ee2e3 100644
--- a/drivers/net/wireless/ath9k/ath9k.h
+++ b/drivers/net/wireless/ath9k/ath9k.h
@@ -579,6 +579,7 @@ struct ath_softc {
void __iomem *mem;
int irq;
spinlock_t sc_resetlock;
+ spinlock_t sc_serial_rw;
struct mutex mutex;
u8 curbssid[ETH_ALEN];
diff --git a/drivers/net/wireless/ath9k/hw.c b/drivers/net/wireless/ath9k/hw.c
index c10b33b..c6e6899 100644
--- a/drivers/net/wireless/ath9k/hw.c
+++ b/drivers/net/wireless/ath9k/hw.c
@@ -37,6 +37,38 @@ static u32 ath9k_hw_ini_fixup(struct ath_hw *ah,
static void ath9k_hw_9280_spur_mitigate(struct ath_hw *ah, struct ath9k_channel *chan);
static void ath9k_hw_spur_mitigate(struct ath_hw *ah, struct ath9k_channel *chan);
+/*
+ * Read and write, they both share the same lock. We do this to serialize
+ * reads and writes on Atheros 802.11n PCI devices only. This is required
+ * as the FIFO on these devices can only accept sanely 2 requests. After
+ * that the device goes bananas. Serializing the reads/writes prevents this
+ * from happening.
+ */
+
+void ath9k_iowrite32(struct ath_hw *ah, u32 reg_offset, u32 val)
+{
+ if (ah->config.serialize_regmode == SER_REG_MODE_ON) {
+ unsigned long flags;
+ spin_lock_irqsave(&ah->ah_sc->sc_serial_rw, flags);
+ iowrite32(val, ah->ah_sc->mem + reg_offset);
+ spin_unlock_irqrestore(&ah->ah_sc->sc_serial_rw, flags);
+ } else
+ iowrite32(val, ah->ah_sc->mem + reg_offset);
+}
+
+unsigned int ath9k_ioread32(struct ath_hw *ah, u32 reg_offset)
+{
+ u32 val;
+ if (ah->config.serialize_regmode == SER_REG_MODE_ON) {
+ unsigned long flags;
+ spin_lock_irqsave(&ah->ah_sc->sc_serial_rw, flags);
+ val = ioread32(ah->ah_sc->mem + reg_offset);
+ spin_unlock_irqrestore(&ah->ah_sc->sc_serial_rw, flags);
+ } else
+ val = ioread32(ah->ah_sc->mem + reg_offset);
+ return val;
+}
+
/********************/
/* Helper Functions */
/********************/
@@ -391,6 +423,25 @@ static void ath9k_hw_set_defaults(struct ath_hw *ah)
}
ah->config.intr_mitigation = 1;
+
+ /*
+ * We need this for PCI devices only (Cardbus, PCI, miniPCI)
+ * _and_ if on non-uniprocessor systems (Multiprocessor/HT).
+ * This means we use it for all AR5416 devices, and the few
+ * minor PCI AR9280 devices out there.
+ *
+ * Serialization is required because these devices do not handle
+ * well the case of two concurrent reads/writes due to the latency
+ * involved. During one read/write another read/write can be issued
+ * on another CPU while the previous read/write may still be working
+ * on our hardware, if we hit this case the hardware poops in a loop.
+ * We prevent this by serializing reads and writes.
+ *
+ * This issue is not present on PCI-Express devices or pre-AR5416
+ * devices (legacy, 802.11abg).
+ */
+ if (num_possible_cpus() > 1)
+ ah->config.serialize_regmode = SER_REG_MODE_AUTO;
}
static struct ath_hw *ath9k_hw_newstate(u16 devid, struct ath_softc *sc,
diff --git a/drivers/net/wireless/ath9k/hw.h b/drivers/net/wireless/ath9k/hw.h
index 89936a0..790bfa9 100644
--- a/drivers/net/wireless/ath9k/hw.h
+++ b/drivers/net/wireless/ath9k/hw.h
@@ -42,8 +42,8 @@
#define AR5416_MAGIC 0x19641014
/* Register read/write primitives */
-#define REG_WRITE(_ah, _reg, _val) iowrite32(_val, _ah->ah_sc->mem + _reg)
-#define REG_READ(_ah, _reg) ioread32(_ah->ah_sc->mem + _reg)
+#define REG_WRITE(_ah, _reg, _val) ath9k_iowrite32(_ah, _reg, _val)
+#define REG_READ(_ah, _reg) ath9k_ioread32(_ah, _reg)
#define SM(_v, _f) (((_v) << _f##_S) & _f)
#define MS(_v, _f) (((_v) & _f) >> _f##_S)
@@ -566,6 +566,10 @@ struct ath_hw {
struct ar5416IniArray iniModesTxGain;
};
+/* Read and write */
+void ath9k_iowrite32(struct ath_hw *ah, u32 reg_offset, u32 val);
+unsigned int ath9k_ioread32(struct ath_hw *ah, u32 reg_offset);
+
/* Attach, Detach, Reset */
const char *ath9k_hw_probe(u16 vendorid, u16 devid);
void ath9k_hw_detach(struct ath_hw *ah);
diff --git a/drivers/net/wireless/ath9k/main.c b/drivers/net/wireless/ath9k/main.c
index 5268697..2ad6414 100644
--- a/drivers/net/wireless/ath9k/main.c
+++ b/drivers/net/wireless/ath9k/main.c
@@ -1370,6 +1370,7 @@ static int ath_init(u16 devid, struct ath_softc *sc)
spin_lock_init(&sc->wiphy_lock);
spin_lock_init(&sc->sc_resetlock);
+ spin_lock_init(&sc->sc_serial_rw);
mutex_init(&sc->mutex);
tasklet_init(&sc->intr_tq, ath9k_tasklet, (unsigned long)sc);
tasklet_init(&sc->bcon_tasklet, ath_beacon_tasklet,
--
1.6.0.6