Skip to content

Commit

Permalink
[PATCH] bcm43xx: opencoded locking
Browse files Browse the repository at this point in the history 
As many people don't seem to like the locking "obfuscation"
in the bcm43xx driver, this patch removes it.

Signed-off-by: Michael Buesch <mb@bu3sch.de>
Signed-off-by: John W. Linville <linville@tuxdriver.com>
  • Loading branch information
Michael Buesch authored and John W. Linville committed Jul 10, 2006
1 parent b312d79 commit efa6a37
Show file tree
Hide file tree
Showing 7 changed files with 167 additions and 164 deletions.
64 changes: 14 additions & 50 deletions drivers/net/wireless/bcm43xx/bcm43xx.h
View file
Original file line number Diff line number Diff line change
Expand Up @@ -649,6 +649,19 @@ enum {
#define bcm43xx_status(bcm) atomic_read(&(bcm)->init_status)
#define bcm43xx_set_status(bcm, stat) atomic_set(&(bcm)->init_status, (stat))

/* *** THEORY OF LOCKING ***
*
* We have two different locks in the bcm43xx driver.
* => bcm->mutex: General sleeping mutex. Protects struct bcm43xx_private
* and the device registers. This mutex does _not_ protect
* against concurrency from the IRQ handler.
* => bcm->irq_lock: IRQ spinlock. Protects against IRQ handler concurrency.
*
* Please note that, if you only take the irq_lock, you are not protected
* against concurrency from the periodic work handlers.
* Most times you want to take _both_ locks.
*/

struct bcm43xx_private {
struct ieee80211_device *ieee;
struct ieee80211softmac_device *softmac;
Expand All @@ -659,7 +672,6 @@ struct bcm43xx_private {

void __iomem *mmio_addr;

/* Locking, see "theory of locking" text below. */
spinlock_t irq_lock;
struct mutex mutex;

Expand Down Expand Up @@ -691,6 +703,7 @@ struct bcm43xx_private {
struct bcm43xx_sprominfo sprom;
#define BCM43xx_NR_LEDS 4
struct bcm43xx_led leds[BCM43xx_NR_LEDS];
spinlock_t leds_lock;

/* The currently active core. */
struct bcm43xx_coreinfo *current_core;
Expand Down Expand Up @@ -763,55 +776,6 @@ struct bcm43xx_private {
};


/* *** THEORY OF LOCKING ***
*
* We have two different locks in the bcm43xx driver.
* => bcm->mutex: General sleeping mutex. Protects struct bcm43xx_private
* and the device registers.
* => bcm->irq_lock: IRQ spinlock. Protects against IRQ handler concurrency.
*
* We have three types of helper function pairs to utilize these locks.
* (Always use the helper functions.)
* 1) bcm43xx_{un}lock_noirq():
* Takes bcm->mutex. Does _not_ protect against IRQ concurrency,
* so it is almost always unsafe, if device IRQs are enabled.
* So only use this, if device IRQs are masked.
* Locking may sleep.
* You can sleep within the critical section.
* 2) bcm43xx_{un}lock_irqonly():
* Takes bcm->irq_lock. Does _not_ protect against
* bcm43xx_lock_noirq() critical sections.
* Does only protect against the IRQ handler path and other
* irqonly() critical sections.
* Locking does not sleep.
* You must not sleep within the critical section.
* 3) bcm43xx_{un}lock_irqsafe():
* This is the cummulative lock and takes both, mutex and irq_lock.
* Protects against noirq() and irqonly() critical sections (and
* the IRQ handler path).
* Locking may sleep.
* You must not sleep within the critical section.
*/

/* Lock type 1 */
#define bcm43xx_lock_noirq(bcm) mutex_lock(&(bcm)->mutex)
#define bcm43xx_unlock_noirq(bcm) mutex_unlock(&(bcm)->mutex)
/* Lock type 2 */
#define bcm43xx_lock_irqonly(bcm, flags) \
spin_lock_irqsave(&(bcm)->irq_lock, flags)
#define bcm43xx_unlock_irqonly(bcm, flags) \
spin_unlock_irqrestore(&(bcm)->irq_lock, flags)
/* Lock type 3 */
#define bcm43xx_lock_irqsafe(bcm, flags) do { \
bcm43xx_lock_noirq(bcm); \
bcm43xx_lock_irqonly(bcm, flags); \
} while (0)
#define bcm43xx_unlock_irqsafe(bcm, flags) do { \
bcm43xx_unlock_irqonly(bcm, flags); \
bcm43xx_unlock_noirq(bcm); \
} while (0)


static inline
struct bcm43xx_private * bcm43xx_priv(struct net_device *dev)
{
Expand Down
34 changes: 22 additions & 12 deletions drivers/net/wireless/bcm43xx/bcm43xx_debugfs.c
View file
Original file line number Diff line number Diff line change
Expand Up @@ -77,7 +77,8 @@ static ssize_t devinfo_read_file(struct file *file, char __user *userbuf,

down(&big_buffer_sem);

bcm43xx_lock_irqsafe(bcm, flags);
mutex_lock(&bcm->mutex);
spin_lock_irqsave(&bcm->irq_lock, flags);
if (bcm43xx_status(bcm) != BCM43xx_STAT_INITIALIZED) {
fappend("Board not initialized.\n");
goto out;
Expand Down Expand Up @@ -121,7 +122,8 @@ static ssize_t devinfo_read_file(struct file *file, char __user *userbuf,
fappend("\n");

out:
bcm43xx_unlock_irqsafe(bcm, flags);
spin_unlock_irqrestore(&bcm->irq_lock, flags);
mutex_unlock(&bcm->mutex);
res = simple_read_from_buffer(userbuf, count, ppos, buf, pos);
up(&big_buffer_sem);
return res;
Expand Down Expand Up @@ -159,7 +161,8 @@ static ssize_t spromdump_read_file(struct file *file, char __user *userbuf,
unsigned long flags;

down(&big_buffer_sem);
bcm43xx_lock_irqsafe(bcm, flags);
mutex_lock(&bcm->mutex);
spin_lock_irqsave(&bcm->irq_lock, flags);
if (bcm43xx_status(bcm) != BCM43xx_STAT_INITIALIZED) {
fappend("Board not initialized.\n");
goto out;
Expand All @@ -169,7 +172,8 @@ static ssize_t spromdump_read_file(struct file *file, char __user *userbuf,
fappend("boardflags: 0x%04x\n", bcm->sprom.boardflags);

out:
bcm43xx_unlock_irqsafe(bcm, flags);
spin_unlock_irqrestore(&bcm->irq_lock, flags);
mutex_unlock(&bcm->mutex);
res = simple_read_from_buffer(userbuf, count, ppos, buf, pos);
up(&big_buffer_sem);
return res;
Expand All @@ -188,7 +192,8 @@ static ssize_t tsf_read_file(struct file *file, char __user *userbuf,
u64 tsf;

down(&big_buffer_sem);
bcm43xx_lock_irqsafe(bcm, flags);
mutex_lock(&bcm->mutex);
spin_lock_irqsave(&bcm->irq_lock, flags);
if (bcm43xx_status(bcm) != BCM43xx_STAT_INITIALIZED) {
fappend("Board not initialized.\n");
goto out;
Expand All @@ -199,7 +204,8 @@ static ssize_t tsf_read_file(struct file *file, char __user *userbuf,
(unsigned int)(tsf & 0xFFFFFFFFULL));

out:
bcm43xx_unlock_irqsafe(bcm, flags);
spin_unlock_irqrestore(&bcm->irq_lock, flags);
mutex_unlock(&bcm->mutex);
res = simple_read_from_buffer(userbuf, count, ppos, buf, pos);
up(&big_buffer_sem);
return res;
Expand All @@ -221,7 +227,8 @@ static ssize_t tsf_write_file(struct file *file, const char __user *user_buf,
res = -EFAULT;
goto out_up;
}
bcm43xx_lock_irqsafe(bcm, flags);
mutex_lock(&bcm->mutex);
spin_lock_irqsave(&bcm->irq_lock, flags);
if (bcm43xx_status(bcm) != BCM43xx_STAT_INITIALIZED) {
printk(KERN_INFO PFX "debugfs: Board not initialized.\n");
res = -EFAULT;
Expand All @@ -237,7 +244,8 @@ static ssize_t tsf_write_file(struct file *file, const char __user *user_buf,
res = buf_size;

out_unlock:
bcm43xx_unlock_irqsafe(bcm, flags);
spin_unlock_irqrestore(&bcm->irq_lock, flags);
mutex_unlock(&bcm->mutex);
out_up:
up(&big_buffer_sem);
return res;
Expand All @@ -258,7 +266,8 @@ static ssize_t txstat_read_file(struct file *file, char __user *userbuf,
int i, cnt, j = 0;

down(&big_buffer_sem);
bcm43xx_lock_irqsafe(bcm, flags);
mutex_lock(&bcm->mutex);
spin_lock_irqsave(&bcm->irq_lock, flags);

fappend("Last %d logged xmitstatus blobs (Latest first):\n\n",
BCM43xx_NR_LOGGED_XMITSTATUS);
Expand Down Expand Up @@ -294,14 +303,15 @@ static ssize_t txstat_read_file(struct file *file, char __user *userbuf,
i = BCM43xx_NR_LOGGED_XMITSTATUS - 1;
}

bcm43xx_unlock_irqsafe(bcm, flags);
spin_unlock_irqrestore(&bcm->irq_lock, flags);
res = simple_read_from_buffer(userbuf, count, ppos, buf, pos);
bcm43xx_lock_irqsafe(bcm, flags);
spin_lock_irqsave(&bcm->irq_lock, flags);
if (*ppos == pos) {
/* Done. Drop the copied data. */
e->xmitstatus_printing = 0;
}
bcm43xx_unlock_irqsafe(bcm, flags);
spin_unlock_irqrestore(&bcm->irq_lock, flags);
mutex_unlock(&bcm->mutex);
up(&big_buffer_sem);
return res;
}
Expand Down
10 changes: 8 additions & 2 deletions drivers/net/wireless/bcm43xx/bcm43xx_leds.c
View file
Original file line number Diff line number Diff line change
Expand Up @@ -51,12 +51,12 @@ static void bcm43xx_led_blink(unsigned long d)
struct bcm43xx_private *bcm = led->bcm;
unsigned long flags;

bcm43xx_lock_irqonly(bcm, flags);
spin_lock_irqsave(&bcm->leds_lock, flags);
if (led->blink_interval) {
bcm43xx_led_changestate(led);
mod_timer(&led->blink_timer, jiffies + led->blink_interval);
}
bcm43xx_unlock_irqonly(bcm, flags);
spin_unlock_irqrestore(&bcm->leds_lock, flags);
}

static void bcm43xx_led_blink_start(struct bcm43xx_led *led,
Expand Down Expand Up @@ -177,7 +177,9 @@ void bcm43xx_leds_update(struct bcm43xx_private *bcm, int activity)
int i, turn_on;
unsigned long interval = 0;
u16 ledctl;
unsigned long flags;

spin_lock_irqsave(&bcm->leds_lock, flags);
ledctl = bcm43xx_read16(bcm, BCM43xx_MMIO_GPIO_CONTROL);
for (i = 0; i < BCM43xx_NR_LEDS; i++) {
led = &(bcm->leds[i]);
Expand Down Expand Up @@ -266,6 +268,7 @@ void bcm43xx_leds_update(struct bcm43xx_private *bcm, int activity)
ledctl &= ~(1 << i);
}
bcm43xx_write16(bcm, BCM43xx_MMIO_GPIO_CONTROL, ledctl);
spin_unlock_irqrestore(&bcm->leds_lock, flags);
}

void bcm43xx_leds_switch_all(struct bcm43xx_private *bcm, int on)
Expand All @@ -274,7 +277,9 @@ void bcm43xx_leds_switch_all(struct bcm43xx_private *bcm, int on)
u16 ledctl;
int i;
int bit_on;
unsigned long flags;

spin_lock_irqsave(&bcm->leds_lock, flags);
ledctl = bcm43xx_read16(bcm, BCM43xx_MMIO_GPIO_CONTROL);
for (i = 0; i < BCM43xx_NR_LEDS; i++) {
led = &(bcm->leds[i]);
Expand All @@ -290,4 +295,5 @@ void bcm43xx_leds_switch_all(struct bcm43xx_private *bcm, int on)
ledctl &= ~(1 << i);
}
bcm43xx_write16(bcm, BCM43xx_MMIO_GPIO_CONTROL, ledctl);
spin_unlock_irqrestore(&bcm->leds_lock, flags);
}
Loading

0 comments on commit efa6a37

Please sign in to comment.