On Sunday 28 May 2006 18:37, you wrote:
> On Sun, May 28, 2006 at 06:29:01PM +0200, Michael Buesch wrote:
> > Well, I would like to know it for sure. ;)
> > Could you load the kernel which crashed and look there. It is
> > quite important to know if it is caused by a shared IRQ or not.
>
> OK, i'm running the kernel that crashed now. it's the same:
Ok Jason, could you please test the following patch and try to reproduce
with it?
Jory, could you also test this patch on your machine with the shared
bcm43xx IRQ? While testing, put some load on one of the devices sharing
the IRQ with bcm43xx (I think it was eth0 for example).
Index: wireless-dev/drivers/net/wireless/bcm43xx/bcm43xx.h
===================================================================
--- wireless-dev.orig/drivers/net/wireless/bcm43xx/bcm43xx.h 2006-05-28
23:57:01.000000000 +0200
+++ wireless-dev/drivers/net/wireless/bcm43xx/bcm43xx.h 2006-05-29
00:16:50.000000000 +0200
@@ -647,9 +647,7 @@
void __iomem *mmio_addr;
unsigned int mmio_len;
- /* Do not use the lock directly. Use the bcm43xx_lock* helper
- * functions, to be MMIO-safe. */
- spinlock_t _lock;
+ spinlock_t lock;
/* Driver status flags. */
u32 initialized:1, /* init_board() succeed */
@@ -704,6 +702,9 @@
/* Number of available 80211 cores. */
int nr_80211_available;
+ /* If 0, the MAC is enabled. Suspended otherwise. */
+ int mac_suspended;
+
u32 chipcommon_capabilities;
/* Reason code of the last interrupt. */
@@ -744,6 +745,7 @@
/* Debugging stuff follows. */
#ifdef CONFIG_BCM43XX_DEBUG
struct bcm43xx_dfsentry *dfsentry;
+ atomic_t periodic_work_in_progress;
#endif
};
@@ -753,8 +755,8 @@
* the *_mmio lock functions.
* MMIO read-access is allowed, though.
*/
-#define bcm43xx_lock(bcm, flags) spin_lock_irqsave(&(bcm)->_lock, flags)
-#define bcm43xx_unlock(bcm, flags) spin_unlock_irqrestore(&(bcm)->_lock,
flags)
+#define bcm43xx_lock(bcm, flags) spin_lock_irqsave(&(bcm)->lock, flags)
+#define bcm43xx_unlock(bcm, flags) spin_unlock_irqrestore(&(bcm)->lock,
flags)
/* bcm43xx_(un)lock_mmio() protect struct bcm43xx_private and MMIO.
* MMIO write-access to the device is allowed.
* All MMIO writes are flushed on unlock, so it is guaranteed to not
Index: wireless-dev/drivers/net/wireless/bcm43xx/bcm43xx_main.c
===================================================================
--- wireless-dev.orig/drivers/net/wireless/bcm43xx/bcm43xx_main.c
2006-05-28 23:57:01.000000000 +0200
+++ wireless-dev/drivers/net/wireless/bcm43xx/bcm43xx_main.c 2006-05-29
00:41:58.000000000 +0200
@@ -496,20 +496,36 @@
return old_mask;
}
-/* Make sure we don't receive more data from the device. */
+/* Disable IRQs on the device and make sure no IRQ handler
+ * (or tasklet) is running on return.
+ */
static int bcm43xx_disable_interrupts_sync(struct bcm43xx_private *bcm, u32
*oldstate)
{
u32 old;
unsigned long flags;
+ unsigned int irq;
bcm43xx_lock_mmio(bcm, flags);
- if (bcm43xx_is_initializing(bcm) || bcm->shutting_down) {
+ if (unlikely(bcm43xx_is_initializing(bcm) ||
+ bcm->shutting_down)) {
bcm43xx_unlock_mmio(bcm, flags);
return -EBUSY;
}
+ bcm43xx_mac_suspend(bcm);
+ irq = bcm->irq;
old = bcm43xx_interrupt_disable(bcm, BCM43xx_IRQ_ALL);
- tasklet_disable(&bcm->isr_tasklet);
+ bcm43xx_read32(bcm, BCM43xx_MMIO_GEN_IRQ_MASK); /* dummy read. */
+
+ /* Unlock, so running IRQ handlers or tasklets can return
+ * and don't deadlock.
+ * The access of isr_tasklet after unlocking is Ok, because
+ * it can not race after synchronizing IRQ.
+ */
bcm43xx_unlock_mmio(bcm, flags);
+
+ synchronize_irq(irq);
+ tasklet_disable(&bcm->isr_tasklet);
+
if (oldstate)
*oldstate = old;
@@ -1868,8 +1884,9 @@
if (!bcm)
return IRQ_NONE;
- spin_lock(&bcm->_lock);
-
+ /* Do the reason checking unlocked, as we could otherwise deadlock
+ * with the periodic work handler.
+ */
reason = bcm43xx_read32(bcm, BCM43xx_MMIO_GEN_IRQ_REASON);
if (reason == 0xffffffff) {
/* irq not for us (shared irq) */
@@ -1880,6 +1897,9 @@
if (!reason)
goto out;
+ assert(!atomic_read(&bcm->periodic_work_in_progress));
+ spin_lock(&bcm->lock);
+
bcm->dma_reason[0] = bcm43xx_read32(bcm, BCM43xx_MMIO_DMA1_REASON)
& 0x0001dc00;
bcm->dma_reason[1] = bcm43xx_read32(bcm, BCM43xx_MMIO_DMA2_REASON)
@@ -1907,7 +1927,7 @@
out:
mmiowb();
- spin_unlock(&bcm->_lock);
+ spin_unlock(&bcm->lock);
return ret;
}
@@ -2271,13 +2291,17 @@
/* http://bcm-specs.sipsolutions.net/EnableMac */
void bcm43xx_mac_enable(struct bcm43xx_private *bcm)
{
- bcm43xx_write32(bcm, BCM43xx_MMIO_STATUS_BITFIELD,
- bcm43xx_read32(bcm, BCM43xx_MMIO_STATUS_BITFIELD)
- | BCM43xx_SBF_MAC_ENABLED);
- bcm43xx_write32(bcm, BCM43xx_MMIO_GEN_IRQ_REASON, BCM43xx_IRQ_READY);
- bcm43xx_read32(bcm, BCM43xx_MMIO_STATUS_BITFIELD); /* dummy read */
- bcm43xx_read32(bcm, BCM43xx_MMIO_GEN_IRQ_REASON); /* dummy read */
- bcm43xx_power_saving_ctl_bits(bcm, -1, -1);
+ bcm->mac_suspended--;
+ if (!bcm->mac_suspended) {
+ bcm43xx_write32(bcm, BCM43xx_MMIO_STATUS_BITFIELD,
+ bcm43xx_read32(bcm,
BCM43xx_MMIO_STATUS_BITFIELD)
+ | BCM43xx_SBF_MAC_ENABLED);
+ bcm43xx_write32(bcm, BCM43xx_MMIO_GEN_IRQ_REASON,
BCM43xx_IRQ_READY);
+ bcm43xx_read32(bcm, BCM43xx_MMIO_STATUS_BITFIELD); /* dummy
read */
+ bcm43xx_read32(bcm, BCM43xx_MMIO_GEN_IRQ_REASON); /* dummy read
*/
+ bcm43xx_power_saving_ctl_bits(bcm, -1, -1);
+ }
+ assert(bcm->mac_suspended >= 0);
}
/* http://bcm-specs.sipsolutions.net/SuspendMAC */
@@ -2286,18 +2310,23 @@
int i;
u32 tmp;
- bcm43xx_power_saving_ctl_bits(bcm, -1, 1);
- bcm43xx_write32(bcm, BCM43xx_MMIO_STATUS_BITFIELD,
- bcm43xx_read32(bcm, BCM43xx_MMIO_STATUS_BITFIELD)
- & ~BCM43xx_SBF_MAC_ENABLED);
- bcm43xx_read32(bcm, BCM43xx_MMIO_GEN_IRQ_REASON); /* dummy read */
- for (i = 100000; i; i--) {
- tmp = bcm43xx_read32(bcm, BCM43xx_MMIO_GEN_IRQ_REASON);
- if (tmp & BCM43xx_IRQ_READY)
- return;
- udelay(10);
+ assert(bcm->mac_suspended >= 0);
+ if (!bcm->mac_suspended) {
+ bcm43xx_power_saving_ctl_bits(bcm, -1, 1);
+ bcm43xx_write32(bcm, BCM43xx_MMIO_STATUS_BITFIELD,
+ bcm43xx_read32(bcm,
BCM43xx_MMIO_STATUS_BITFIELD)
+ & ~BCM43xx_SBF_MAC_ENABLED);
+ bcm43xx_read32(bcm, BCM43xx_MMIO_GEN_IRQ_REASON); /* dummy read
*/
+ for (i = 100000; i; i--) {
+ tmp = bcm43xx_read32(bcm, BCM43xx_MMIO_GEN_IRQ_REASON);
+ if (tmp & BCM43xx_IRQ_READY)
+ goto out;
+ udelay(10);
+ }
+ printkl(KERN_ERR PFX "MAC suspend failed\n");
}
- printkl(KERN_ERR PFX "MAC suspend failed\n");
+out:
+ bcm->mac_suspended++;
}
void bcm43xx_set_iwmode(struct bcm43xx_private *bcm,
@@ -3109,8 +3138,22 @@
struct bcm43xx_private *bcm = (struct bcm43xx_private *)d;
unsigned long flags;
unsigned int state;
+ u32 savedirqs;
+ int err;
- bcm43xx_lock_mmio(bcm, flags);
+ err = bcm43xx_disable_interrupts_sync(bcm, &savedirqs);
+ assert(!err);
+#ifdef CONFIG_BCM43XX_DEBUG
+ atomic_inc(&bcm->periodic_work_in_progress);
+#endif
+ /* Periodic work can take a long time so we don't want
+ * to disable IRQs on the CPU.
+ * spin_lock() is deadlock safe here, because we previously
+ * disabled and synced IRQs on the device. No IRQ is supposed
+ * to happen before bcm43xx_interrupt_enable(), so it
+ * can't deadlock with the spin_lock in the IRQ handler.
+ */
+ spin_lock(&bcm->lock);
assert(bcm->initialized);
state = bcm->periodic_state;
@@ -3122,10 +3165,18 @@
bcm43xx_periodic_every30sec(bcm);
bcm43xx_periodic_every15sec(bcm);
bcm->periodic_state = state + 1;
-
mod_timer(&bcm->periodic_tasks, jiffies + (HZ * 15));
- bcm43xx_unlock_mmio(bcm, flags);
+ local_irq_save(flags);
+ bcm43xx_interrupt_enable(bcm, savedirqs);
+ tasklet_enable(&bcm->isr_tasklet);
+ bcm43xx_mac_enable(bcm);
+ mmiowb();
+ spin_unlock(&bcm->lock);
+#ifdef CONFIG_BCM43XX_DEBUG
+ atomic_dec(&bcm->periodic_work_in_progress);
+#endif
+ local_irq_restore(flags);
}
static void bcm43xx_periodic_tasks_delete(struct bcm43xx_private *bcm)
@@ -3687,9 +3738,11 @@
static int bcm43xx_net_stop(struct net_device *net_dev)
{
struct bcm43xx_private *bcm = bcm43xx_priv(net_dev);
+ int err;
ieee80211softmac_stop(net_dev);
- bcm43xx_disable_interrupts_sync(bcm, NULL);
+ err = bcm43xx_disable_interrupts_sync(bcm, NULL);
+ assert(!err);
bcm43xx_free_board(bcm);
return 0;
@@ -3709,7 +3762,8 @@
bcm->pci_dev = pci_dev;
bcm->net_dev = net_dev;
bcm->bad_frames_preempt = modparam_bad_frames_preempt;
- spin_lock_init(&bcm->_lock);
+ bcm->mac_suspended = 1;
+ spin_lock_init(&bcm->lock);
tasklet_init(&bcm->isr_tasklet,
(void (*)(unsigned long))bcm43xx_interrupt_tasklet,
(unsigned long)bcm);
Index: wireless-dev/drivers/net/wireless/bcm43xx/bcm43xx_phy.c
===================================================================
--- wireless-dev.orig/drivers/net/wireless/bcm43xx/bcm43xx_phy.c
2006-05-26 01:01:48.000000000 +0200
+++ wireless-dev/drivers/net/wireless/bcm43xx/bcm43xx_phy.c 2006-05-28
23:57:41.000000000 +0200
@@ -1410,7 +1410,10 @@
u16 bcm43xx_phy_lo_g_deviation_subval(struct bcm43xx_private *bcm, u16 control)
{
struct bcm43xx_phyinfo *phy = bcm43xx_current_phy(bcm);
+ u16 ret;
+ unsigned long flags;
+ local_irq_save(flags);
if (phy->connected) {
bcm43xx_phy_write(bcm, 0x15, 0xE300);
control <<= 8;
@@ -1430,8 +1433,10 @@
bcm43xx_phy_write(bcm, 0x0015, control | 0xFFE0);
udelay(8);
}
+ ret = bcm43xx_phy_read(bcm, 0x002D);
+ local_irq_restore(flags);
- return bcm43xx_phy_read(bcm, 0x002D);
+ return ret;
}
static u32 bcm43xx_phy_lo_g_singledeviation(struct bcm43xx_private *bcm, u16
control)
--
Greetings Michael.
_______________________________________________
Bcm43xx-dev mailing list
[email protected]
http://lists.berlios.de/mailman/listinfo/bcm43xx-dev
