[PATCH] mtd: nand: do FIFO processing in nand_get_device()
Sebastian Andrzej Siewior
bigeasy at linutronix.de
Sun Dec 6 06:17:57 PST 2015
* Peter Zijlstra | 2015-11-30 17:15:49 [+0100]:
>On Wed, Nov 25, 2015 at 06:35:43PM +0100, Sebastian Andrzej Siewior wrote:
>> +++ b/drivers/mtd/nand/nand_base.c
>> @@ -137,8 +137,25 @@ static void nand_release_device(struct mtd_info *mtd)
>> /* Release the controller and the chip */
>> spin_lock(&chip->controller->lock);
>> chip->controller->active = NULL;
>> - chip->state = FL_READY;
>> - wake_up(&chip->controller->wq);
>> +
>> + if (waitqueue_active(&chip->controller->wq)) {
>> + wait_queue_head_t *q;
>> + wait_queue_t *waiter;
>> + unsigned long flags;
>> +
>> + q = &chip->controller->wq;
>> + chip->state = FL_HANDOVER;
>> +
>> + spin_lock_irqsave(&q->lock, flags);
>
>This lock is actually not required, as your add/remove_wait_queue calls
>are also under chip->controller->lock.
yeah. And there can be only one wakeup and only after this function here
made it happen. So yes, not required. dropped it.
>> +
>> + chip->controller->handover_waiter = waiter;
>
>You could consider using ->active for this; as it stands you never use
>both at the same time. Its a tad icky, but it avoids adding that new
>field.
There is this FL_PM_SUSPENDED which derefences `active` even if its state
is not FL_READY. So I think that we could go boom here.
Also in order to share that member we need either an union or an
explicit cast because it is a different type. Puh.
My estimate here is 3 * pointer + 2 * sizeof spinlock. So on 32bit we
should always end up in kmalloc-32 (except with spinlock debugging which
inflates this struct anyway so this extra pointer should not matter
much).
Sebastian
More information about the linux-mtd
mailing list