Groups | Search | Server Info | Keyboard shortcuts | Login | Register [http] [https] [nntp] [nntps]


Groups > linux.kernel > #1208246

Re: [PATCH] mtd: nand: pxa3xx-nand: fix random command timeouts

From Ezequiel Garcia <ezequiel@vanguardiasur.com.ar>
Newsgroups linux.kernel
Subject Re: [PATCH] mtd: nand: pxa3xx-nand: fix random command timeouts
Date 2015-08-16 17:40 +0200
Message-ID <pY8cp-4f1-7@gated-at.bofh.it> (permalink)
References <pWH4C-1gT-25@gated-at.bofh.it>
Organization linux.* mail to news gateway

Show all headers | View raw


On 12 Aug 06:22 PM, Robert Jarzmik wrote:
> When 2 commands are submitted in a row, and the second is very quick,
> the completion of the second command might never come. This happens
> especially if the second command is quick, such as a status read after
> an erase.
> 
> The issue is that in the interrupt handler, the status bits are cleared
> after the new command is issued. There is a small temporal window where
> this happens :
>  - the previous command has set the command done bit
>  - the ready for a command bit is set
>  - the handler submits the next command
>    - just then, the command completes, and the command done bit is still
>      set
>  - the handler clears the "previous" command done bit
>  - the handler exits
> 
> In this flow, the "command done" of the next command will never trigger
> a new interrupt to finish the status command, as it was cleared for both
> commands.
> 
> Fix this by clearing the status bit before submitting a new command.
> 

This fix looks correct. Thanks!

Couple questions:

1. In which platform are you seeing this bug?
2. Is this a regression? (i.e. should we queue it for -stable?)

Also, one might question why we can't just write NDSR right after it's read,
before we wake the IRQ thread or start DMA. It appears this is
a requirement of BCH, as per the comment in drain_fifo.

It would be nice to put a comment explaining why we clear NDSR only
before the check to WRCMDREQ. Maybe even copy-pasting something
from the commit log?

I'd like to say "Yay, let's pick it" but I'd like to make sure this is
tested on all platforms first (unless you've tested it already).

> Signed-off-by: Robert Jarzmik <robert.jarzmik@free.fr>
> ---
>  drivers/mtd/nand/pxa3xx_nand.c | 6 +++---
>  1 file changed, 3 insertions(+), 3 deletions(-)
> 
> diff --git a/drivers/mtd/nand/pxa3xx_nand.c b/drivers/mtd/nand/pxa3xx_nand.c
> index b0737aec7caf..718097414b9c 100644
> --- a/drivers/mtd/nand/pxa3xx_nand.c
> +++ b/drivers/mtd/nand/pxa3xx_nand.c
> @@ -687,8 +687,10 @@ static irqreturn_t pxa3xx_nand_irq(int irq, void *devid)
>  		is_ready = 1;
>  	}
>  
> +	/* clear NDSR to let the controller exit the IRQ */
> +	nand_writel(info, NDSR, status);
> +
>  	if (status & NDSR_WRCMDREQ) {
> -		nand_writel(info, NDSR, NDSR_WRCMDREQ);
>  		status &= ~NDSR_WRCMDREQ;
>  		info->state = STATE_CMD_HANDLE;
>  
> @@ -709,8 +711,6 @@ static irqreturn_t pxa3xx_nand_irq(int irq, void *devid)
>  			nand_writel(info, NDCB0, info->ndcb3);
>  	}
>  
> -	/* clear NDSR to let the controller exit the IRQ */
> -	nand_writel(info, NDSR, status);
>  	if (is_completed)
>  		complete(&info->cmd_complete);
>  	if (is_ready)
> -- 
> 2.1.4
> 
> 
> ______________________________________________________
> Linux MTD discussion mailing list
> http://lists.infradead.org/mailman/listinfo/linux-mtd/

-- 
Ezequiel Garcia, VanguardiaSur
www.vanguardiasur.com.ar
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/

Back to linux.kernel | Previous | NextPrevious in thread | Next in thread | Find similar | Unroll thread


Thread

[PATCH] mtd: nand: pxa3xx-nand: fix random command timeouts Robert Jarzmik <robert.jarzmik@free.fr> - 2015-08-12 18:30 +0200
  Re: [PATCH] mtd: nand: pxa3xx-nand: fix random command timeouts Ezequiel Garcia <ezequiel@vanguardiasur.com.ar> - 2015-08-16 17:40 +0200
    Re: [PATCH] mtd: nand: pxa3xx-nand: fix random command timeouts Robert Jarzmik <robert.jarzmik@free.fr> - 2015-08-17 00:30 +0200
      Re: [PATCH] mtd: nand: pxa3xx-nand: fix random command timeouts Ezequiel Garcia <ezequiel@vanguardiasur.com.ar> - 2015-08-17 21:10 +0200
        Re: [PATCH] mtd: nand: pxa3xx-nand: fix random command timeouts Robert Jarzmik <robert.jarzmik@free.fr> - 2015-08-17 21:20 +0200
          Re: [PATCH] mtd: nand: pxa3xx-nand: fix random command timeouts Brian Norris <computersforpeace@gmail.com> - 2015-08-19 02:50 +0200
            Re: [PATCH] mtd: nand: pxa3xx-nand: fix random command timeouts Robert Jarzmik <robert.jarzmik@free.fr> - 2015-08-19 08:30 +0200

csiph-web