[PATCH for v3.13 0/2] pxa3xx-nand fixes for v3.13

[PATCH for v3.13 0/2] pxa3xx-nand fixes for v3.13

[Ezequiel Garcia]

Two almost-oneline fixes intended for v3.13.

The first commit fixes a current problem that can be reproduced
on PXA boards, when use_dma = 1 and after a device fails to be detected.
The driver currently tries to release DMA resources that are not allocated.

The second commit fixes a problem that can be triggered on PXA boards
by writing a blank page (0xff).
The bug itself was found by Arnaud Ebalard while testing 2048B page size
support on the Armada 370/XP variant, yet the bug can be reproduced on the
driver's PXA variant.

Ezequiel Garcia (2):
  mtd: nand: pxa3xx: Use info->use_dma to release DMA resources
  mtd: nand: pxa3xx: Clear need_wait flag when starting a command

 drivers/mtd/nand/pxa3xx_nand.c | 3 ++-
 1 file changed, 2 insertions(+), 1 deletion(-)

-- 
1.8.1.5

[PATCH for v3.13 1/2] mtd: nand: pxa3xx: Use info->use_dma to release DMA resources

[Ezequiel Garcia]

In commit:

  commit 62e8b851783138a11da63285be0fbf69530ff73d
  Author: Ezequiel Garcia <ezequiel.garcia@free-electrons.com>
  Date:   Fri Oct 4 15:30:38 2013 -0300

  mtd: nand: pxa3xx: Allocate data buffer on detected flash size

the way the buffer is allocated was changed: the first READ_ID is issued
with a small kmalloc'ed buffer. Only once the flash page size is detected
the DMA buffers are allocated, and info->use_dma is set.

Currently, if the device detection fails, the driver checks the 'use_dma'
module parameter and tries to release unallocated DMA resources.

Fix this by checking the proper indicator of the DMA allocation, which
is 'info->use_dma'.

Signed-off-by: Ezequiel Garcia <ezequiel.garcia@free-electrons.com>
---
 drivers/mtd/nand/pxa3xx_nand.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/drivers/mtd/nand/pxa3xx_nand.c b/drivers/mtd/nand/pxa3xx_nand.c
index 3d143fe..4cdc134 100644
--- a/drivers/mtd/nand/pxa3xx_nand.c
+++ b/drivers/mtd/nand/pxa3xx_nand.c
@@ -1288,7 +1288,7 @@ static int pxa3xx_nand_init_buff(struct pxa3xx_nand_info *info)
 static void pxa3xx_nand_free_buff(struct pxa3xx_nand_info *info)
 {
 	struct platform_device *pdev = info->pdev;
-	if (use_dma) {
+	if (info->use_dma) {
 		pxa_free_dma(info->data_dma_ch);
 		dma_free_coherent(&pdev->dev, info->buf_size,
 				  info->data_buff, info->data_buff_phys);
-- 
1.8.1.5

[PATCH for v3.13 2/2] mtd: nand: pxa3xx: Clear need_wait flag when starting a command

[Ezequiel Garcia]

Currently the driver assumes all commands will eventually trigger a RnB
transition, and thus a "device is ready" IRQ.

This assumption means that on every issued command, the dev_ready completion
handler is init'ed and the need_wait flag is set.

However this is incorrect: some commands (such as NAND_CMD_STATUS) don't
make the device 'busy' and thus a RnB transition never occurs.
Given, the NAND core never calls waitfunc() after such commands, this
is not a problem.

Therefore, it's possible to only clear the need_wait flag on every command
that is started.

This fixes a current bug that can be reproduced on PXA boards by writing
blank (all 0xff'ed) to a page:

  1. The kernel issues NAND_CMD_STATUS and sets need_wait=1. The flag
     won't be cleared for this command since no RnB transition is
     involved.

  2. NAND_CMD_PAGEPROG is issued but since the data is blank, the driver
     decides not to execute the command (and no IRQ activity is
     involved).

  3. The NAND core calls waitfunc() and waits for the dev_ready
     completion, which will never end since the device _is_ already ready.

Tested-by: Arnaud Ebalard <arno@natisbad.org>
Signed-off-by: Ezequiel Garcia <ezequiel.garcia@free-electrons.com>
---
 drivers/mtd/nand/pxa3xx_nand.c | 1 +
 1 file changed, 1 insertion(+)

diff --git a/drivers/mtd/nand/pxa3xx_nand.c b/drivers/mtd/nand/pxa3xx_nand.c
index 4cdc134..53a1862 100644
--- a/drivers/mtd/nand/pxa3xx_nand.c
+++ b/drivers/mtd/nand/pxa3xx_nand.c
@@ -694,6 +694,7 @@ static void prepare_start_command(struct pxa3xx_nand_info *info, int command)
 	info->retcode		= ERR_NONE;
 	info->ecc_err_cnt	= 0;
 	info->ndcb3		= 0;
+	info->need_wait		= 0;
 
 	switch (command) {
 	case NAND_CMD_READ0:
-- 
1.8.1.5

Re: [PATCH for v3.13 0/2] pxa3xx-nand fixes for v3.13

[Ezequiel Garcia]

On Tue, Dec 10, 2013 at 09:57:14AM -0300, Ezequiel Garcia wrote:
> Two almost-oneline fixes intended for v3.13.
> 
> The first commit fixes a current problem that can be reproduced
> on PXA boards, when use_dma = 1 and after a device fails to be detected.
> The driver currently tries to release DMA resources that are not allocated.
> 
> The second commit fixes a problem that can be triggered on PXA boards
> by writing a blank page (0xff).
> The bug itself was found by Arnaud Ebalard while testing 2048B page size
> support on the Armada 370/XP variant, yet the bug can be reproduced on the
> driver's PXA variant.
> 
> Ezequiel Garcia (2):
>   mtd: nand: pxa3xx: Use info->use_dma to release DMA resources
>   mtd: nand: pxa3xx: Clear need_wait flag when starting a command
> 
>  drivers/mtd/nand/pxa3xx_nand.c | 3 ++-
>  1 file changed, 2 insertions(+), 1 deletion(-)
> 

Brian:

Do you plan to include these in a pull for Linus?
-- 
Ezequiel García, Free Electrons
Embedded Linux, Kernel and Android Engineering
http://free-electrons.com

Re: [PATCH for v3.13 0/2] pxa3xx-nand fixes for v3.13

[Brian Norris]

On Wed, Dec 11, 2013 at 05:20:16PM -0300, Ezequiel Garcia wrote:
> Brian:
> 
> Do you plan to include these in a pull for Linus?

Yes. I was giving people (myself, and others) time to look at these, but
I plan to queue them up in linux-mtd.git soon, along with removing the
'compatible' property (your other patch).

Brian

Re: [PATCH for v3.13 0/2] pxa3xx-nand fixes for v3.13

[Ezequiel Garcia]

On Wed, Dec 11, 2013 at 12:35:44PM -0800, Brian Norris wrote:
> On Wed, Dec 11, 2013 at 05:20:16PM -0300, Ezequiel Garcia wrote:
> > Brian:
> > 
> > Do you plan to include these in a pull for Linus?
> 
> Yes. I was giving people (myself, and others) time to look at these, but
> I plan to queue them up in linux-mtd.git soon, along with removing the
> 'compatible' property (your other patch).
> 

OK, thanks!

-- 
Ezequiel García, Free Electrons
Embedded Linux, Kernel and Android Engineering
http://free-electrons.com

Re: [PATCH for v3.13 2/2] mtd: nand: pxa3xx: Clear need_wait flag when starting a command

[Brian Norris]

On Tue, Dec 10, 2013 at 09:57:16AM -0300, Ezequiel Garcia wrote:
...
> This fixes a current bug that can be reproduced on PXA boards by writing
> blank (all 0xff'ed) to a page:
...

This patch doesn't apply to 3.13-rc1, so how am I supposed to send it
for the 3.13 cycle? It seems that you based this on l2-mtd.git, which
has code queued for 3.14.

Please rebase on linux-mtd.git, give it a test, and resend it so I can
get it out soon.

Thanks,
Brian

Re: [PATCH for v3.13 2/2] mtd: nand: pxa3xx: Clear need_wait flag when starting a command

[Ezequiel Garcia]

On Thu, Dec 12, 2013 at 12:35:22PM -0800, Brian Norris wrote:
> On Tue, Dec 10, 2013 at 09:57:16AM -0300, Ezequiel Garcia wrote:
> ...
> > This fixes a current bug that can be reproduced on PXA boards by writing
> > blank (all 0xff'ed) to a page:
> ...
> 
> This patch doesn't apply to 3.13-rc1, so how am I supposed to send it
> for the 3.13 cycle? It seems that you based this on l2-mtd.git, which
> has code queued for 3.14.
> 

Ugh... crap!! This patch is *not* for v3.13, and I've no idea why I said so in
the first place. The bug this patch fixes was introduced with the device ready
completion mess in:

  commit 8524028ee90d177580b496e2740efd673f8e7910
  Author: Ezequiel Garcia <ezequiel.garcia@free-electrons.com>
  Date:   Thu Nov 14 18:25:26 2013 -0300

  mtd: nand: pxa3xx: Use a completion to signal device ready

So, this fix goes on top of l2-mtd.git (for v3.14), while the ones appropriate
for v3.13 are:

http://patchwork.ozlabs.org/patch/299645/

and

http://patchwork.ozlabs.org/patch/299202/

Can you pick them, or do you prefer me to resend everything?
-- 
Ezequiel García, Free Electrons
Embedded Linux, Kernel and Android Engineering
http://free-electrons.com

Re: [PATCH for v3.13 2/2] mtd: nand: pxa3xx: Clear need_wait flag when starting a command

[Brian Norris]

On Thu, Dec 12, 2013 at 07:38:31PM -0300, Ezequiel Garcia wrote:
> On Thu, Dec 12, 2013 at 12:35:22PM -0800, Brian Norris wrote:
> > On Tue, Dec 10, 2013 at 09:57:16AM -0300, Ezequiel Garcia wrote:
> > ...
> > > This fixes a current bug that can be reproduced on PXA boards by writing
> > > blank (all 0xff'ed) to a page:
> > ...
> > 
> > This patch doesn't apply to 3.13-rc1, so how am I supposed to send it
> > for the 3.13 cycle? It seems that you based this on l2-mtd.git, which
> > has code queued for 3.14.
> > 
> 
> Ugh... crap!! This patch is *not* for v3.13, and I've no idea why I said so in
> the first place. The bug this patch fixes was introduced with the device ready
> completion mess in:
> 
>   commit 8524028ee90d177580b496e2740efd673f8e7910
>   Author: Ezequiel Garcia <ezequiel.garcia@free-electrons.com>
>   Date:   Thu Nov 14 18:25:26 2013 -0300
> 
>   mtd: nand: pxa3xx: Use a completion to signal device ready

I see :) And yes, that makes sense.

> So, this fix goes on top of l2-mtd.git (for v3.14), while the ones appropriate
> for v3.13 are:
> 
> http://patchwork.ozlabs.org/patch/299645/
> 
> and
> 
> http://patchwork.ozlabs.org/patch/299202/
> 
> Can you pick them, or do you prefer me to resend everything?

Got 'em already. Will tag them for upstream within the day, hopefully.

Please resubmit the remaining patch (for 3.14, against l2-mtd.git)
separately once I get everything else queued up properly, and we can
work from there.

Thanks,
Brian

Re: [PATCH for v3.13 1/2] mtd: nand: pxa3xx: Use info->use_dma to release DMA resources

[Brian Norris]

On Tue, Dec 10, 2013 at 09:57:15AM -0300, Ezequiel Garcia wrote:
> In commit:
> 
>   commit 62e8b851783138a11da63285be0fbf69530ff73d
>   Author: Ezequiel Garcia <ezequiel.garcia@free-electrons.com>
>   Date:   Fri Oct 4 15:30:38 2013 -0300
> 
>   mtd: nand: pxa3xx: Allocate data buffer on detected flash size
> 
> the way the buffer is allocated was changed: the first READ_ID is issued
> with a small kmalloc'ed buffer. Only once the flash page size is detected
> the DMA buffers are allocated, and info->use_dma is set.
> 
> Currently, if the device detection fails, the driver checks the 'use_dma'
> module parameter and tries to release unallocated DMA resources.
> 
> Fix this by checking the proper indicator of the DMA allocation, which
> is 'info->use_dma'.
> 
> Signed-off-by: Ezequiel Garcia <ezequiel.garcia@free-electrons.com>

Pushed only this patch to linux-mtd.git.

Thanks,
Brian