From 0dfc62465ef92c7ddcb1ba223bf062453566fd0f Mon Sep 17 00:00:00 2001 From: Thomas Gleixner Date: Tue, 31 May 2005 20:39:20 +0100 Subject: [PATCH] [MTD] NAND: Reorganize chip locking 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 --- drivers/mtd/nand/nand_base.c | 57 ++++++++++++++++++------------------ include/linux/mtd/nand.h | 5 +++- 2 files changed, 33 insertions(+), 29 deletions(-) diff --git a/drivers/mtd/nand/nand_base.c b/drivers/mtd/nand/nand_base.c index f1db0bf9306..bbe0283433d 100644 --- a/drivers/mtd/nand/nand_base.c +++ b/drivers/mtd/nand/nand_base.c @@ -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 @@ -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); } /** @@ -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; } diff --git a/include/linux/mtd/nand.h b/include/linux/mtd/nand.h index bee78969cb2..9b5b7621758 100644 --- a/include/linux/mtd/nand.h +++ b/include/linux/mtd/nand.h @@ -5,7 +5,7 @@ * Steven J. Hill * Thomas Gleixner * - * $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 @@ -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; }; /**