diff options
author | Thomas Gleixner <tglx@linutronix.de> | 2005-05-31 15:39:20 -0400 |
---|---|---|
committer | Thomas Gleixner <tglx@mtd.linutronix.de> | 2005-06-29 08:15:17 -0400 |
commit | 0dfc62465ef92c7ddcb1ba223bf062453566fd0f (patch) | |
tree | 0156e9b9996cf43dd67ceed53a2b8da0a516cdec | |
parent | 7ca6448dbfb398bba36eda3c01bc14b86c3675be (diff) |
[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 <tglx@linutronix.de>
-rw-r--r-- | drivers/mtd/nand/nand_base.c | 57 | ||||
-rw-r--r-- | 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 f1db0bf9306b..bbe0283433d2 100644 --- a/drivers/mtd/nand/nand_base.c +++ b/drivers/mtd/nand/nand_base.c | |||
@@ -59,7 +59,7 @@ | |||
59 | * The AG-AND chips have nice features for speed improvement, | 59 | * The AG-AND chips have nice features for speed improvement, |
60 | * which are not supported yet. Read / program 4 pages in one go. | 60 | * which are not supported yet. Read / program 4 pages in one go. |
61 | * | 61 | * |
62 | * $Id: nand_base.c,v 1.143 2005/05/19 16:10:22 gleixner Exp $ | 62 | * $Id: nand_base.c,v 1.145 2005/05/31 20:32:53 gleixner Exp $ |
63 | * | 63 | * |
64 | * This program is free software; you can redistribute it and/or modify | 64 | * This program is free software; you can redistribute it and/or modify |
65 | * it under the terms of the GNU General Public License version 2 as | 65 | * 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) | |||
167 | 167 | ||
168 | /* De-select the NAND device */ | 168 | /* De-select the NAND device */ |
169 | this->select_chip(mtd, -1); | 169 | this->select_chip(mtd, -1); |
170 | /* Do we have a hardware controller ? */ | 170 | |
171 | if (this->controller) { | 171 | if (this->controller) { |
172 | /* Release the controller and the chip */ | ||
172 | spin_lock(&this->controller->lock); | 173 | spin_lock(&this->controller->lock); |
173 | this->controller->active = NULL; | 174 | this->controller->active = NULL; |
175 | this->state = FL_READY; | ||
176 | wake_up(&this->controller->wq); | ||
174 | spin_unlock(&this->controller->lock); | 177 | spin_unlock(&this->controller->lock); |
178 | } else { | ||
179 | /* Release the chip */ | ||
180 | spin_lock(&this->chip_lock); | ||
181 | this->state = FL_READY; | ||
182 | wake_up(&this->wq); | ||
183 | spin_unlock(&this->chip_lock); | ||
175 | } | 184 | } |
176 | /* Release the chip */ | ||
177 | spin_lock (&this->chip_lock); | ||
178 | this->state = FL_READY; | ||
179 | wake_up (&this->wq); | ||
180 | spin_unlock (&this->chip_lock); | ||
181 | } | 185 | } |
182 | 186 | ||
183 | /** | 187 | /** |
@@ -753,37 +757,34 @@ static void nand_command_lp (struct mtd_info *mtd, unsigned command, int column, | |||
753 | */ | 757 | */ |
754 | static void nand_get_device (struct nand_chip *this, struct mtd_info *mtd, int new_state) | 758 | static void nand_get_device (struct nand_chip *this, struct mtd_info *mtd, int new_state) |
755 | { | 759 | { |
756 | struct nand_chip *active = this; | 760 | struct nand_chip *active; |
757 | 761 | spinlock_t *lock; | |
762 | wait_queue_head_t *wq; | ||
758 | DECLARE_WAITQUEUE (wait, current); | 763 | DECLARE_WAITQUEUE (wait, current); |
759 | 764 | ||
760 | /* | 765 | lock = (this->controller) ? &this->controller->lock : &this->chip_lock; |
761 | * Grab the lock and see if the device is available | 766 | wq = (this->controller) ? &this->controller->wq : &this->wq; |
762 | */ | ||
763 | retry: | 767 | retry: |
768 | active = this; | ||
769 | spin_lock(lock); | ||
770 | |||
764 | /* Hardware controller shared among independend devices */ | 771 | /* Hardware controller shared among independend devices */ |
765 | if (this->controller) { | 772 | if (this->controller) { |
766 | spin_lock (&this->controller->lock); | ||
767 | if (this->controller->active) | 773 | if (this->controller->active) |
768 | active = this->controller->active; | 774 | active = this->controller->active; |
769 | else | 775 | else |
770 | this->controller->active = this; | 776 | this->controller->active = this; |
771 | spin_unlock (&this->controller->lock); | ||
772 | } | 777 | } |
773 | 778 | if (active == this && this->state == FL_READY) { | |
774 | if (active == this) { | 779 | this->state = new_state; |
775 | spin_lock (&this->chip_lock); | 780 | spin_unlock(lock); |
776 | if (this->state == FL_READY) { | 781 | return; |
777 | this->state = new_state; | 782 | } |
778 | spin_unlock (&this->chip_lock); | 783 | set_current_state(TASK_UNINTERRUPTIBLE); |
779 | return; | 784 | add_wait_queue(wq, &wait); |
780 | } | 785 | spin_unlock(lock); |
781 | } | 786 | schedule(); |
782 | set_current_state (TASK_UNINTERRUPTIBLE); | 787 | remove_wait_queue(wq, &wait); |
783 | add_wait_queue (&active->wq, &wait); | ||
784 | spin_unlock (&active->chip_lock); | ||
785 | schedule (); | ||
786 | remove_wait_queue (&active->wq, &wait); | ||
787 | goto retry; | 788 | goto retry; |
788 | } | 789 | } |
789 | 790 | ||
diff --git a/include/linux/mtd/nand.h b/include/linux/mtd/nand.h index bee78969cb21..9b5b76217584 100644 --- a/include/linux/mtd/nand.h +++ b/include/linux/mtd/nand.h | |||
@@ -5,7 +5,7 @@ | |||
5 | * Steven J. Hill <sjhill@realitydiluted.com> | 5 | * Steven J. Hill <sjhill@realitydiluted.com> |
6 | * Thomas Gleixner <tglx@linutronix.de> | 6 | * Thomas Gleixner <tglx@linutronix.de> |
7 | * | 7 | * |
8 | * $Id: nand.h,v 1.71 2005/02/09 12:12:59 gleixner Exp $ | 8 | * $Id: nand.h,v 1.73 2005/05/31 19:39:17 gleixner Exp $ |
9 | * | 9 | * |
10 | * This program is free software; you can redistribute it and/or modify | 10 | * This program is free software; you can redistribute it and/or modify |
11 | * it under the terms of the GNU General Public License version 2 as | 11 | * it under the terms of the GNU General Public License version 2 as |
@@ -253,10 +253,13 @@ struct nand_chip; | |||
253 | * struct nand_hw_control - Control structure for hardware controller (e.g ECC generator) shared among independend devices | 253 | * struct nand_hw_control - Control structure for hardware controller (e.g ECC generator) shared among independend devices |
254 | * @lock: protection lock | 254 | * @lock: protection lock |
255 | * @active: the mtd device which holds the controller currently | 255 | * @active: the mtd device which holds the controller currently |
256 | * @wq: wait queue to sleep on if a NAND operation is in progress | ||
257 | * used instead of the per chip wait queue when a hw controller is available | ||
256 | */ | 258 | */ |
257 | struct nand_hw_control { | 259 | struct nand_hw_control { |
258 | spinlock_t lock; | 260 | spinlock_t lock; |
259 | struct nand_chip *active; | 261 | struct nand_chip *active; |
262 | wait_queue_head_t wq; | ||
260 | }; | 263 | }; |
261 | 264 | ||
262 | /** | 265 | /** |