2009-05-15 20:39:44

by Larry Finger

[permalink] [raw]
Subject: [PATCH 1/2 V2] b43legacy: Remove unnecessary MMIO in interrupt hotpath

From: Stefano Brivio <[email protected]>

This removes unnecessary MMIO accesses in the interrupt hotpath. The
patch by Michael Buesch for b43 has been ported to b43legacy.

Signed-off-by: Stefano Brivio <[email protected]>
Tested-by: Larry Finger <[email protected]>
---

John,

This is 2.6.31 material. I am submitting this for Stefano because
this patch touches a section of the code that had to be changed
to fix a locking problem.

I hope this patch does not cause any problems. My normal method
is not available while I'm away from home.

V2 does not try to eliminate any "unused" DMA rings. If any should be
eliminated, that will be in a separate patch.

Larry
---

b43legacy.h | 4 +--
main.c | 78 ++++++++++++++++--------------------------------------------
pio.c | 2 -
3 files changed, 25 insertions(+), 59 deletions(-)

Index: wireless-testing/drivers/net/wireless/b43legacy/b43legacy.h
===================================================================
--- wireless-testing.orig/drivers/net/wireless/b43legacy/b43legacy.h
+++ wireless-testing/drivers/net/wireless/b43legacy/b43legacy.h
@@ -694,8 +694,8 @@ struct b43legacy_wldev {
/* Reason code of the last interrupt. */
u32 irq_reason;
u32 dma_reason[6];
- /* saved irq enable/disable state bitfield. */
- u32 irq_savedstate;
+ /* The currently active generic-interrupt mask. */
+ u32 irq_mask;
/* Link Quality calculation context. */
struct b43legacy_noise_calculation noisecalc;
/* if > 0 MAC is suspended. if == 0 MAC is enabled. */
Index: wireless-testing/drivers/net/wireless/b43legacy/main.c
===================================================================
--- wireless-testing.orig/drivers/net/wireless/b43legacy/main.c
+++ wireless-testing/drivers/net/wireless/b43legacy/main.c
@@ -583,35 +583,6 @@ static void b43legacy_short_slot_timing_
b43legacy_set_slot_time(dev, 20);
}

-/* Enable a Generic IRQ. "mask" is the mask of which IRQs to enable.
- * Returns the _previously_ enabled IRQ mask.
- */
-static inline u32 b43legacy_interrupt_enable(struct b43legacy_wldev *dev,
- u32 mask)
-{
- u32 old_mask;
-
- old_mask = b43legacy_read32(dev, B43legacy_MMIO_GEN_IRQ_MASK);
- b43legacy_write32(dev, B43legacy_MMIO_GEN_IRQ_MASK, old_mask |
- mask);
-
- return old_mask;
-}
-
-/* Disable a Generic IRQ. "mask" is the mask of which IRQs to disable.
- * Returns the _previously_ enabled IRQ mask.
- */
-static inline u32 b43legacy_interrupt_disable(struct b43legacy_wldev *dev,
- u32 mask)
-{
- u32 old_mask;
-
- old_mask = b43legacy_read32(dev, B43legacy_MMIO_GEN_IRQ_MASK);
- b43legacy_write32(dev, B43legacy_MMIO_GEN_IRQ_MASK, old_mask & ~mask);
-
- return old_mask;
-}
-
/* Synchronize IRQ top- and bottom-half.
* IRQs must be masked before calling this.
* This must not be called with the irq_lock held.
@@ -1200,7 +1171,7 @@ static void handle_irq_beacon(struct b43
/* This is the bottom half of the asynchronous beacon update. */

/* Ignore interrupt in the future. */
- dev->irq_savedstate &= ~B43legacy_IRQ_BEACON;
+ dev->irq_mask &= ~B43legacy_IRQ_BEACON;

cmd = b43legacy_read32(dev, B43legacy_MMIO_MACCMD);
beacon0_valid = (cmd & B43legacy_MACCMD_BEACON0_VALID);
@@ -1209,7 +1180,7 @@ static void handle_irq_beacon(struct b43
/* Schedule interrupt manually, if busy. */
if (beacon0_valid && beacon1_valid) {
b43legacy_write32(dev, B43legacy_MMIO_GEN_IRQ_REASON, B43legacy_IRQ_BEACON);
- dev->irq_savedstate |= B43legacy_IRQ_BEACON;
+ dev->irq_mask |= B43legacy_IRQ_BEACON;
return;
}

@@ -1247,12 +1218,11 @@ static void b43legacy_beacon_update_trig
dev = wl->current_dev;
if (likely(dev && (b43legacy_status(dev) >= B43legacy_STAT_INITIALIZED))) {
spin_lock_irq(&wl->irq_lock);
- /* update beacon right away or defer to irq */
- dev->irq_savedstate = b43legacy_read32(dev, B43legacy_MMIO_GEN_IRQ_MASK);
+ /* Update beacon right away or defer to IRQ. */
handle_irq_beacon(dev);
/* The handler might have updated the IRQ mask. */
b43legacy_write32(dev, B43legacy_MMIO_GEN_IRQ_MASK,
- dev->irq_savedstate);
+ dev->irq_mask);
mmiowb();
spin_unlock_irq(&wl->irq_lock);
}
@@ -1398,7 +1368,7 @@ static void b43legacy_interrupt_tasklet(
if (reason & B43legacy_IRQ_TX_OK)
handle_irq_transmit_status(dev);

- b43legacy_interrupt_enable(dev, dev->irq_savedstate);
+ b43legacy_write32(dev, B43legacy_MMIO_GEN_IRQ_MASK, dev->irq_mask);
mmiowb();
spin_unlock_irqrestore(&dev->wl->irq_lock, flags);
}
@@ -1450,18 +1420,18 @@ static irqreturn_t b43legacy_interrupt_h
struct b43legacy_wldev *dev = dev_id;
u32 reason;

- if (!dev)
- return IRQ_NONE;
+ B43legacy_WARN_ON(!dev);

spin_lock(&dev->wl->irq_lock);

- if (b43legacy_status(dev) < B43legacy_STAT_STARTED)
+ if (unlikely(b43legacy_status(dev) < B43legacy_STAT_STARTED))
+ /* This can only happen on shared IRQ lines. */
goto out;
reason = b43legacy_read32(dev, B43legacy_MMIO_GEN_IRQ_REASON);
if (reason == 0xffffffff) /* shared IRQ */
goto out;
ret = IRQ_HANDLED;
- reason &= b43legacy_read32(dev, B43legacy_MMIO_GEN_IRQ_MASK);
+ reason &= dev->irq_mask;
if (!reason)
goto out;

@@ -1485,10 +1455,9 @@ static irqreturn_t b43legacy_interrupt_h
& 0x0000DC00;

b43legacy_interrupt_ack(dev, reason);
- /* disable all IRQs. They are enabled again in the bottom half. */
- dev->irq_savedstate = b43legacy_interrupt_disable(dev,
- B43legacy_IRQ_ALL);
- /* save the reason code and call our bottom half. */
+ /* Disable all IRQs. They are enabled again in the bottom half. */
+ b43legacy_write32(dev, B43legacy_MMIO_GEN_IRQ_MASK, 0);
+ /* Save the reason code and call our bottom half. */
dev->irq_reason = reason;
tasklet_schedule(&dev->isr_tasklet);
out:
@@ -1948,7 +1917,8 @@ void b43legacy_mac_enable(struct b43lega

/* Re-enable IRQs. */
spin_lock_irq(&dev->wl->irq_lock);
- b43legacy_interrupt_enable(dev, dev->irq_savedstate);
+ b43legacy_write32(dev, B43legacy_MMIO_GEN_IRQ_MASK,
+ dev->irq_mask);
spin_unlock_irq(&dev->wl->irq_lock);
}
}
@@ -1967,10 +1937,9 @@ void b43legacy_mac_suspend(struct b43leg
/* Mask IRQs before suspending MAC. Otherwise
* the MAC stays busy and won't suspend. */
spin_lock_irq(&dev->wl->irq_lock);
- tmp = b43legacy_interrupt_disable(dev, B43legacy_IRQ_ALL);
+ b43legacy_write32(dev, B43legacy_MMIO_GEN_IRQ_MASK, 0);
spin_unlock_irq(&dev->wl->irq_lock);
b43legacy_synchronize_irq(dev);
- dev->irq_savedstate = tmp;

b43legacy_power_saving_ctl_bits(dev, -1, 1);
b43legacy_write32(dev, B43legacy_MMIO_MACCTL,
@@ -2659,7 +2628,6 @@ static int b43legacy_op_dev_config(struc
int antenna_tx;
int antenna_rx;
int err = 0;
- u32 savedirqs;

antenna_tx = B43legacy_ANTENNA_DEFAULT;
antenna_rx = B43legacy_ANTENNA_DEFAULT;
@@ -2699,7 +2667,7 @@ static int b43legacy_op_dev_config(struc
spin_unlock_irqrestore(&wl->irq_lock, flags);
goto out_unlock_mutex;
}
- savedirqs = b43legacy_interrupt_disable(dev, B43legacy_IRQ_ALL);
+ b43legacy_write32(dev, B43legacy_MMIO_GEN_IRQ_MASK, 0);
spin_unlock_irqrestore(&wl->irq_lock, flags);
b43legacy_synchronize_irq(dev);

@@ -2738,7 +2706,7 @@ static int b43legacy_op_dev_config(struc
}

spin_lock_irqsave(&wl->irq_lock, flags);
- b43legacy_interrupt_enable(dev, savedirqs);
+ b43legacy_write32(dev, B43legacy_MMIO_GEN_IRQ_MASK, dev->irq_mask);
mmiowb();
spin_unlock_irqrestore(&wl->irq_lock, flags);
out_unlock_mutex:
@@ -2801,7 +2769,6 @@ static void b43legacy_op_bss_info_change
struct b43legacy_wldev *dev;
struct b43legacy_phy *phy;
unsigned long flags;
- u32 savedirqs;

mutex_lock(&wl->mutex);
B43legacy_WARN_ON(wl->vif != vif);
@@ -2817,7 +2784,7 @@ static void b43legacy_op_bss_info_change
spin_unlock_irqrestore(&wl->irq_lock, flags);
goto out_unlock_mutex;
}
- savedirqs = b43legacy_interrupt_disable(dev, B43legacy_IRQ_ALL);
+ b43legacy_write32(dev, B43legacy_MMIO_GEN_IRQ_MASK, 0);

if (changed & BSS_CHANGED_BSSID) {
spin_unlock_irqrestore(&wl->irq_lock, flags);
@@ -2862,7 +2829,7 @@ static void b43legacy_op_bss_info_change
b43legacy_mac_enable(dev);

spin_lock_irqsave(&wl->irq_lock, flags);
- b43legacy_interrupt_enable(dev, savedirqs);
+ b43legacy_write32(dev, B43legacy_MMIO_GEN_IRQ_MASK, dev->irq_mask);
/* XXX: why? */
mmiowb();
spin_unlock_irqrestore(&wl->irq_lock, flags);
@@ -2922,8 +2889,7 @@ static void b43legacy_wireless_core_stop
* setting the status to INITIALIZED, as the interrupt handler
* won't care about IRQs then. */
spin_lock_irqsave(&wl->irq_lock, flags);
- dev->irq_savedstate = b43legacy_interrupt_disable(dev,
- B43legacy_IRQ_ALL);
+ b43legacy_write32(dev, B43legacy_MMIO_GEN_IRQ_MASK, 0);
b43legacy_read32(dev, B43legacy_MMIO_GEN_IRQ_MASK); /* flush */
spin_unlock_irqrestore(&wl->irq_lock, flags);
b43legacy_synchronize_irq(dev);
@@ -2963,7 +2929,7 @@ static int b43legacy_wireless_core_start

/* Start data flow (TX/RX) */
b43legacy_mac_enable(dev);
- b43legacy_interrupt_enable(dev, dev->irq_savedstate);
+ b43legacy_write32(dev, B43legacy_MMIO_GEN_IRQ_MASK, dev->irq_mask);

/* Start maintenance work */
b43legacy_periodic_tasks_setup(dev);
@@ -3126,7 +3092,7 @@ static void setup_struct_wldev_for_init(
/* IRQ related flags */
dev->irq_reason = 0;
memset(dev->dma_reason, 0, sizeof(dev->dma_reason));
- dev->irq_savedstate = B43legacy_IRQ_MASKTEMPLATE;
+ dev->irq_mask = B43legacy_IRQ_MASKTEMPLATE;

dev->mac_suspended = 1;

Index: wireless-testing/drivers/net/wireless/b43legacy/pio.c
===================================================================
--- wireless-testing.orig/drivers/net/wireless/b43legacy/pio.c
+++ wireless-testing/drivers/net/wireless/b43legacy/pio.c
@@ -443,7 +443,7 @@ int b43legacy_pio_init(struct b43legacy_
pio->queue3 = queue;

if (dev->dev->id.revision < 3)
- dev->irq_savedstate |= B43legacy_IRQ_PIO_WORKAROUND;
+ dev->irq_mask |= B43legacy_IRQ_PIO_WORKAROUND;

b43legacydbg(dev->wl, "PIO initialized\n");
err = 0;





2009-05-15 20:55:14

by Michael Büsch

[permalink] [raw]
Subject: Re: [PATCH 1/2 V2] b43legacy: Remove unnecessary MMIO in interrupt hotpath

On Friday 15 May 2009 22:39:20 Larry Finger wrote:
> From: Stefano Brivio <[email protected]>
>
> This removes unnecessary MMIO accesses in the interrupt hotpath. The
> patch by Michael Buesch for b43 has been ported to b43legacy.
>
> Signed-off-by: Stefano Brivio <[email protected]>
> Tested-by: Larry Finger <[email protected]>
> ---
>
> John,
>
> This is 2.6.31 material. I am submitting this for Stefano because
> this patch touches a section of the code that had to be changed
> to fix a locking problem.
>
> I hope this patch does not cause any problems. My normal method
> is not available while I'm away from home.
>
> V2 does not try to eliminate any "unused" DMA rings. If any should be
> eliminated, that will be in a separate patch.

ACK

--
Greetings, Michael.