Skip to content

Commit

Permalink
[MTD] NAND: Reorganize chip locking
Browse files Browse the repository at this point in the history
The code was wrong in several aspects. The locking order was
inconsistent, the device aquire code did not reset a variable
after a wakeup and the wakeup handling was not working for
applications where multiple chips are sharing a single
hardware controller.
When a hardware controller is available the locking is now
reduced to the hardware controller lock and the waitqueue is
moved to the hardware controller structure in order to avoid
a wake_up_all().

The problem was pointed out by Ben Dooks, who also found the
missing variable reset as main cause for his deadlock problem.

Signed-off-by: Thomas Gleixner <tglx@linutronix.de>
  • Loading branch information
Thomas Gleixner authored and Thomas Gleixner committed Jun 29, 2005
1 parent 7ca6448 commit 0dfc624
Show file tree
Hide file tree
Showing 2 changed files with 33 additions and 29 deletions.
57 changes: 29 additions & 28 deletions drivers/mtd/nand/nand_base.c
Original file line number Diff line number Diff line change
Expand Up @@ -59,7 +59,7 @@
* The AG-AND chips have nice features for speed improvement,
* which are not supported yet. Read / program 4 pages in one go.
*
* $Id: nand_base.c,v 1.143 2005/05/19 16:10:22 gleixner Exp $
* $Id: nand_base.c,v 1.145 2005/05/31 20:32:53 gleixner Exp $
*
* This program is free software; you can redistribute it and/or modify
* it under the terms of the GNU General Public License version 2 as
Expand Down Expand Up @@ -167,17 +167,21 @@ static void nand_release_device (struct mtd_info *mtd)

/* De-select the NAND device */
this->select_chip(mtd, -1);
/* Do we have a hardware controller ? */

if (this->controller) {
/* Release the controller and the chip */
spin_lock(&this->controller->lock);
this->controller->active = NULL;
this->state = FL_READY;
wake_up(&this->controller->wq);
spin_unlock(&this->controller->lock);
} else {
/* Release the chip */
spin_lock(&this->chip_lock);
this->state = FL_READY;
wake_up(&this->wq);
spin_unlock(&this->chip_lock);
}
/* Release the chip */
spin_lock (&this->chip_lock);
this->state = FL_READY;
wake_up (&this->wq);
spin_unlock (&this->chip_lock);
}

/**
Expand Down Expand Up @@ -753,37 +757,34 @@ static void nand_command_lp (struct mtd_info *mtd, unsigned command, int column,
*/
static void nand_get_device (struct nand_chip *this, struct mtd_info *mtd, int new_state)
{
struct nand_chip *active = this;

struct nand_chip *active;
spinlock_t *lock;
wait_queue_head_t *wq;
DECLARE_WAITQUEUE (wait, current);

/*
* Grab the lock and see if the device is available
*/
lock = (this->controller) ? &this->controller->lock : &this->chip_lock;
wq = (this->controller) ? &this->controller->wq : &this->wq;
retry:
active = this;
spin_lock(lock);

/* Hardware controller shared among independend devices */
if (this->controller) {
spin_lock (&this->controller->lock);
if (this->controller->active)
active = this->controller->active;
else
this->controller->active = this;
spin_unlock (&this->controller->lock);
}

if (active == this) {
spin_lock (&this->chip_lock);
if (this->state == FL_READY) {
this->state = new_state;
spin_unlock (&this->chip_lock);
return;
}
}
set_current_state (TASK_UNINTERRUPTIBLE);
add_wait_queue (&active->wq, &wait);
spin_unlock (&active->chip_lock);
schedule ();
remove_wait_queue (&active->wq, &wait);
if (active == this && this->state == FL_READY) {
this->state = new_state;
spin_unlock(lock);
return;
}
set_current_state(TASK_UNINTERRUPTIBLE);
add_wait_queue(wq, &wait);
spin_unlock(lock);
schedule();
remove_wait_queue(wq, &wait);
goto retry;
}

Expand Down
5 changes: 4 additions & 1 deletion include/linux/mtd/nand.h
Original file line number Diff line number Diff line change
Expand Up @@ -5,7 +5,7 @@
* Steven J. Hill <sjhill@realitydiluted.com>
* Thomas Gleixner <tglx@linutronix.de>
*
* $Id: nand.h,v 1.71 2005/02/09 12:12:59 gleixner Exp $
* $Id: nand.h,v 1.73 2005/05/31 19:39:17 gleixner Exp $
*
* This program is free software; you can redistribute it and/or modify
* it under the terms of the GNU General Public License version 2 as
Expand Down Expand Up @@ -253,10 +253,13 @@ struct nand_chip;
* struct nand_hw_control - Control structure for hardware controller (e.g ECC generator) shared among independend devices
* @lock: protection lock
* @active: the mtd device which holds the controller currently
* @wq: wait queue to sleep on if a NAND operation is in progress
* used instead of the per chip wait queue when a hw controller is available
*/
struct nand_hw_control {
spinlock_t lock;
struct nand_chip *active;
wait_queue_head_t wq;
};

/**
Expand Down

0 comments on commit 0dfc624

Please sign in to comment.