Re: [PATCH v5 10/23] mtd: nand: denali: rework interrupt handling

From: Boris Brezillon
Date: Wed Jun 07 2017 - 09:57:08 EST


On Wed, 7 Jun 2017 20:52:19 +0900
Masahiro Yamada <yamada.masahiro@xxxxxxxxxxxxx> wrote:


> -/*
> - * This is the interrupt service routine. It handles all interrupts
> - * sent to this device. Note that on CE4100, this is a shared interrupt.
> - */
> -static irqreturn_t denali_isr(int irq, void *dev_id)
> +static uint32_t denali_wait_for_irq(struct denali_nand_info *denali,
> + uint32_t irq_mask)
> {
> - struct denali_nand_info *denali = dev_id;
> + unsigned long time_left, flags;
> uint32_t irq_status;
> - irqreturn_t result = IRQ_NONE;
>
> - spin_lock(&denali->irq_lock);
> + spin_lock_irqsave(&denali->irq_lock, flags);
>
> - /* check to see if a valid NAND chip has been selected. */
> - if (is_flash_bank_valid(denali->flash_bank)) {
> - /*
> - * check to see if controller generated the interrupt,
> - * since this is a shared interrupt
> - */
> - irq_status = denali_irq_detected(denali);
> - if (irq_status != 0) {
> - /* handle interrupt */
> - /* first acknowledge it */
> - clear_interrupt(denali, irq_status);
> - /*
> - * store the status in the device context for someone
> - * to read
> - */
> - denali->irq_status |= irq_status;
> - /* notify anyone who cares that it happened */
> - complete(&denali->complete);
> - /* tell the OS that we've handled this */
> - result = IRQ_HANDLED;
> - }
> + irq_status = denali->irq_status;
> +
> + if (irq_mask & irq_status) {
> + spin_unlock_irqrestore(&denali->irq_lock, flags);
> + return irq_status;
> }
> - spin_unlock(&denali->irq_lock);
> - return result;
> +
> + denali->irq_mask = irq_mask;
> + reinit_completion(&denali->complete);

These 2 instructions should be done before calling
denali_wait_for_irq() (for example in denali_reset_irq()), otherwise
you might loose events if they happen between your irq_status read and
the reinit_completion() call. You should also clear existing interrupts
before launching your operation, otherwise you might wakeup on previous
events.

> + spin_unlock_irqrestore(&denali->irq_lock, flags);
> +
> + time_left = wait_for_completion_timeout(&denali->complete,
> + msecs_to_jiffies(1000));
> + if (!time_left) {
> + dev_err(denali->dev, "timeout while waiting for irq 0x%x\n",
> + denali->irq_mask);
> + return 0;
> + }
> +
> + return denali->irq_status;
> }
>