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

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Date:   Wed, 7 Jun 2017 15:57:01 +0200
From:   Boris Brezillon <boris.brezillon@...e-electrons.com>
To:     Masahiro Yamada <yamada.masahiro@...ionext.com>
Cc:     linux-mtd@...ts.infradead.org, Enrico Jorns <ejo@...gutronix.de>,
        Artem Bityutskiy <artem.bityutskiy@...ux.intel.com>,
        Dinh Nguyen <dinguyen@...nel.org>,
        Marek Vasut <marek.vasut@...il.com>,
        David Woodhouse <dwmw2@...radead.org>,
        Masami Hiramatsu <mhiramat@...nel.org>,
        Chuanxiao Dong <chuanxiao.dong@...el.com>,
        Jassi Brar <jaswinder.singh@...aro.org>,
        Cyrille Pitchen <cyrille.pitchen@...ev4u.fr>,
        linux-kernel@...r.kernel.org,
        Brian Norris <computersforpeace@...il.com>,
        Richard Weinberger <richard@....at>
Subject: Re: [PATCH v5 10/23] mtd: nand: denali: rework interrupt handling

On Wed,  7 Jun 2017 20:52:19 +0900
Masahiro Yamada <yamada.masahiro@...ionext.com> 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;
>  }
>