PXA3xx NAND controller support broken in 2.6.39

PXA3xx NAND controller support broken in 2.6.39

[Daniel Mack]

Hi,

Somewhere between 2.6.38 and 2.6.39, support for PXA3xx NAND
controller was broken. I bisected it down, and the relevant commits
appear to be

  4332c11 mtd: pxa3xx_nand: clean the keep configure code
  401e67e mtd: pxa3xx_nand: mtd scan id process could be defined by
driver itself
  4eb2da8 mtd: pxa3xx_nand: unify prepare command
  38caf7f mtd: pxa3xx_nand: discard wait_for_event,write_cmd,__readid function
  f8155a4 mtd: pxa3xx_nand: rework irq logic
  e353a20 mtd: pxa3xx_nand: make scan procedure more clear
  52d039f mtd: pxa3xx_nand: NULL dereference in pxa3xx_nand_probe
  9d8b104 mtd: pxa3xx_nand: set oob length in the runtime
  18c81b1 mtd: pxa3xx_nand: remove the flash info in driver structure
  227a886 mtd: pxa3xx_nand: introduce default timing to reduce read id times
  c1f8247 mtd: pxa3xx_nand: condense the flash definition
  99d3896 mtd: pxa3xx: fix build error when CONFIG_MTD_PARTITIONS is not defined

Unfortunately, this series does not consist of atomic commits and is
hence not bisectable by itself, so I can't name a specific commit
which breaks it.

On a raumfeld board (which has its board support code in mainline),
the effect is:

[    1.803796] Wait time out!!!
[    2.003795] Wait time out!!!
[    2.203780] Wait time out!!!
[    2.403779] Wait time out!!!
[    2.406643] No NAND device found.
[    2.409939] pxa3xx-nand pxa3xx-nand: failed to scan nand
[    2.418850] UBI error: ubi_init: UBI error: cannot initialize UBI, error -19


With the code in 2.6.38, it works just fine. Any detailed debug output
you want me to provide in order to fix it?

Daniel

[PATCH] MTD: pxa3xx_nand: fix nand detection issue

[Lei Wen]

When keep_config is set, the detection would goes different routine.
That the driver would read out the setting which is set previously
by bootloader. While most bootloader keep the irq mask as off, and
current driver need all irq default open, keep_config behavior would
lead to no irq at all.

Signed-off-by: Lei Wen <leiwen@marvell.com>
---
 drivers/mtd/nand/pxa3xx_nand.c |   12 +++++++-----
 1 files changed, 7 insertions(+), 5 deletions(-)

diff --git a/drivers/mtd/nand/pxa3xx_nand.c b/drivers/mtd/nand/pxa3xx_nand.c
index ff07012..9896aef 100644
--- a/drivers/mtd/nand/pxa3xx_nand.c
+++ b/drivers/mtd/nand/pxa3xx_nand.c
@@ -813,7 +813,7 @@ static int pxa3xx_nand_detect_config(struct pxa3xx_nand_info *info)
 	info->page_size = ndcr & NDCR_PAGE_SZ ? 2048 : 512;
 	/* set info fields needed to read id */
 	info->read_id_bytes = (info->page_size == 2048) ? 4 : 2;
-	info->reg_ndcr = ndcr;
+	info->reg_ndcr = ndcr & ~NDCR_INT_MASK;
 	info->cmdset = &default_cmdset;
 
 	info->ndtr0cs0 = nand_readl(info, NDTR0CS0);
@@ -882,7 +882,7 @@ static int pxa3xx_nand_scan(struct mtd_info *mtd)
 	struct pxa3xx_nand_info *info = mtd->priv;
 	struct platform_device *pdev = info->pdev;
 	struct pxa3xx_nand_platform_data *pdata = pdev->dev.platform_data;
-	struct nand_flash_dev pxa3xx_flash_ids[2] = { {NULL,}, {NULL,} };
+	struct nand_flash_dev pxa3xx_flash_ids[2], *def = NULL;
 	const struct pxa3xx_nand_flash *f = NULL;
 	struct nand_chip *chip = mtd->priv;
 	uint32_t id = -1;
@@ -942,8 +942,10 @@ static int pxa3xx_nand_scan(struct mtd_info *mtd)
 	pxa3xx_flash_ids[0].erasesize = f->page_size * f->page_per_block;
 	if (f->flash_width == 16)
 		pxa3xx_flash_ids[0].options = NAND_BUSWIDTH_16;
+	pxa3xx_flash_ids[1].name = NULL;
+	def = pxa3xx_flash_ids;
 KEEP_CONFIG:
-	if (nand_scan_ident(mtd, 1, pxa3xx_flash_ids))
+	if (nand_scan_ident(mtd, 1, def))
 		return -ENODEV;
 	/* calculate addressing information */
 	info->col_addr_cycles = (mtd->writesize >= 2048) ? 2 : 1;
@@ -954,9 +956,9 @@ KEEP_CONFIG:
 		info->row_addr_cycles = 2;
 	mtd->name = mtd_names[0];
 	chip->ecc.mode = NAND_ECC_HW;
-	chip->ecc.size = f->page_size;
+	chip->ecc.size = info->page_size;
 
-	chip->options = (f->flash_width == 16) ? NAND_BUSWIDTH_16 : 0;
+	chip->options = (info->reg_ndcr & NDCR_DWIDTH_M) ? NAND_BUSWIDTH_16 : 0;
 	chip->options |= NAND_NO_AUTOINCR;
 	chip->options |= NAND_NO_READRDY;
 
-- 
1.7.0.4

PXA3xx NAND controller support broken in 2.6.39

[Daniel Mack]

Hi Lei,

On Fri, May 20, 2011 at 6:21 AM, Lei Wen <adrian.wenl@gmail.com> wrote:
>> On a raumfeld board (which has its board support code in mainline),
>> the effect is:
>>
>> [ ? ?1.803796] Wait time out!!!
>> [ ? ?2.003795] Wait time out!!!
>> [ ? ?2.203780] Wait time out!!!
>> [ ? ?2.403779] Wait time out!!!
>> [ ? ?2.406643] No NAND device found.
>> [ ? ?2.409939] pxa3xx-nand pxa3xx-nand: failed to scan nand
>> [ ? ?2.418850] UBI error: ubi_init: UBI error: cannot initialize UBI, error -19
>>
>>
>> With the code in 2.6.38, it works just fine. Any detailed debug output
>> you want me to provide in order to fix it?
>>
> Please try the patch I just send out.
> MTD: pxa3xx_nand: fix nand detection issue

Thank for the quick response. Your patch fixes the detection, but read
operations won't succeed now:

[    1.602733] NAND device: Manufacturer ID: 0x20, Chip ID: 0xa1 (ST
Micro NAND 128MiB 1,8V 8-bit)
[    1.611426] Scanning device for bad blocks
[    1.870669] Creating 4 MTD partitions on "pxa3xx_nand-0":
[    1.876099] 0x000000000000-0x0000000a0000 : "Bootloader"
[    1.885350] 0x0000000a0000-0x0000000c0000 : "BootloaderEnvironment"
[    1.895608] 0x0000000c0000-0x000000120000 : "BootloaderSplashScreen"
[    1.905996] 0x000000120000-0x000008000000 : "UBI"
[    1.916999] UBI: attaching mtd3 to ubi0
[    1.920835] UBI: physical eraseblock size:   131072 bytes (128 KiB)
[    1.927165] UBI: logical eraseblock size:    126976 bytes
[    1.932553] UBI: smallest flash I/O unit:    2048
[    1.937227] UBI: VID header offset:          2048 (aligned 2048)
[    1.943215] UBI: data offset:                4096
[    1.950080] UBI error: ubi_io_read: error -74 (ECC error) while
reading 2048 bytes from PEB 0:2048, read 2048 bytes
[    2.041030] UBI error: ubi_io_read: error -74 (ECC error) while
reading 126976 bytes from PEB 0:4096, read 126976 bytes
[    2.053445] UBI error: ubi_io_read: error -74 (ECC error) while
reading 2048 bytes from PEB 1:2048, read 2048 bytes
[    2.144350] UBI error: ubi_io_read: error -74 (ECC error) while
reading 126976 bytes from PEB 1:4096, read 126976 bytes

(continues forever)

The same NAND content is readable just fine with 2.6.38. Is there a
similar bug for the timing parameters maybe?


Daniel

Re: [PATCH] MTD: pxa3xx_nand: fix nand detection issue

[Lei Wen]

Tested-by: Daniel Mack <zonque@gmail.com>

Also cc to MTD maillist and stable kernel.

On Fri, May 20, 2011 at 12:20 PM, Lei Wen <leiwen@marvell.com> wrote:
> When keep_config is set, the detection would goes different routine.
> That the driver would read out the setting which is set previously
> by bootloader. While most bootloader keep the irq mask as off, and
> current driver need all irq default open, keep_config behavior would
> lead to no irq at all.
>
> Signed-off-by: Lei Wen <leiwen@marvell.com>
> ---
>  drivers/mtd/nand/pxa3xx_nand.c |   12 +++++++-----
>  1 files changed, 7 insertions(+), 5 deletions(-)
>
> diff --git a/drivers/mtd/nand/pxa3xx_nand.c b/drivers/mtd/nand/pxa3xx_nand.c
> index ff07012..9896aef 100644
> --- a/drivers/mtd/nand/pxa3xx_nand.c
> +++ b/drivers/mtd/nand/pxa3xx_nand.c
> @@ -813,7 +813,7 @@ static int pxa3xx_nand_detect_config(struct pxa3xx_nand_info *info)
>        info->page_size = ndcr & NDCR_PAGE_SZ ? 2048 : 512;
>        /* set info fields needed to read id */
>        info->read_id_bytes = (info->page_size == 2048) ? 4 : 2;
> -       info->reg_ndcr = ndcr;
> +       info->reg_ndcr = ndcr & ~NDCR_INT_MASK;
>        info->cmdset = &default_cmdset;
>
>        info->ndtr0cs0 = nand_readl(info, NDTR0CS0);
> @@ -882,7 +882,7 @@ static int pxa3xx_nand_scan(struct mtd_info *mtd)
>        struct pxa3xx_nand_info *info = mtd->priv;
>        struct platform_device *pdev = info->pdev;
>        struct pxa3xx_nand_platform_data *pdata = pdev->dev.platform_data;
> -       struct nand_flash_dev pxa3xx_flash_ids[2] = { {NULL,}, {NULL,} };
> +       struct nand_flash_dev pxa3xx_flash_ids[2], *def = NULL;
>        const struct pxa3xx_nand_flash *f = NULL;
>        struct nand_chip *chip = mtd->priv;
>        uint32_t id = -1;
> @@ -942,8 +942,10 @@ static int pxa3xx_nand_scan(struct mtd_info *mtd)
>        pxa3xx_flash_ids[0].erasesize = f->page_size * f->page_per_block;
>        if (f->flash_width == 16)
>                pxa3xx_flash_ids[0].options = NAND_BUSWIDTH_16;
> +       pxa3xx_flash_ids[1].name = NULL;
> +       def = pxa3xx_flash_ids;
>  KEEP_CONFIG:
> -       if (nand_scan_ident(mtd, 1, pxa3xx_flash_ids))
> +       if (nand_scan_ident(mtd, 1, def))
>                return -ENODEV;
>        /* calculate addressing information */
>        info->col_addr_cycles = (mtd->writesize >= 2048) ? 2 : 1;
> @@ -954,9 +956,9 @@ KEEP_CONFIG:
>                info->row_addr_cycles = 2;
>        mtd->name = mtd_names[0];
>        chip->ecc.mode = NAND_ECC_HW;
> -       chip->ecc.size = f->page_size;
> +       chip->ecc.size = info->page_size;
>
> -       chip->options = (f->flash_width == 16) ? NAND_BUSWIDTH_16 : 0;
> +       chip->options = (info->reg_ndcr & NDCR_DWIDTH_M) ? NAND_BUSWIDTH_16 : 0;
>        chip->options |= NAND_NO_AUTOINCR;
>        chip->options |= NAND_NO_READRDY;
>
> --
> 1.7.0.4
>
>
> _______________________________________________
> linux-arm-kernel mailing list
> linux-arm-kernel@lists.infradead.org
> http://lists.infradead.org/mailman/listinfo/linux-arm-kernel
>

[PATCH] MTD: pxa3xx_nand: fix nand detection issue

[Lei Wen]

Tested-by: Daniel Mack <zonque@gmail.com>

Also cc to MTD maillist and stable kernel.

On Fri, May 20, 2011 at 12:20 PM, Lei Wen <leiwen@marvell.com> wrote:
> When keep_config is set, the detection would goes different routine.
> That the driver would read out the setting which is set previously
> by bootloader. While most bootloader keep the irq mask as off, and
> current driver need all irq default open, keep_config behavior would
> lead to no irq at all.
>
> Signed-off-by: Lei Wen <leiwen@marvell.com>
> ---
> ?drivers/mtd/nand/pxa3xx_nand.c | ? 12 +++++++-----
> ?1 files changed, 7 insertions(+), 5 deletions(-)
>
> diff --git a/drivers/mtd/nand/pxa3xx_nand.c b/drivers/mtd/nand/pxa3xx_nand.c
> index ff07012..9896aef 100644
> --- a/drivers/mtd/nand/pxa3xx_nand.c
> +++ b/drivers/mtd/nand/pxa3xx_nand.c
> @@ -813,7 +813,7 @@ static int pxa3xx_nand_detect_config(struct pxa3xx_nand_info *info)
> ? ? ? ?info->page_size = ndcr & NDCR_PAGE_SZ ? 2048 : 512;
> ? ? ? ?/* set info fields needed to read id */
> ? ? ? ?info->read_id_bytes = (info->page_size == 2048) ? 4 : 2;
> - ? ? ? info->reg_ndcr = ndcr;
> + ? ? ? info->reg_ndcr = ndcr & ~NDCR_INT_MASK;
> ? ? ? ?info->cmdset = &default_cmdset;
>
> ? ? ? ?info->ndtr0cs0 = nand_readl(info, NDTR0CS0);
> @@ -882,7 +882,7 @@ static int pxa3xx_nand_scan(struct mtd_info *mtd)
> ? ? ? ?struct pxa3xx_nand_info *info = mtd->priv;
> ? ? ? ?struct platform_device *pdev = info->pdev;
> ? ? ? ?struct pxa3xx_nand_platform_data *pdata = pdev->dev.platform_data;
> - ? ? ? struct nand_flash_dev pxa3xx_flash_ids[2] = { {NULL,}, {NULL,} };
> + ? ? ? struct nand_flash_dev pxa3xx_flash_ids[2], *def = NULL;
> ? ? ? ?const struct pxa3xx_nand_flash *f = NULL;
> ? ? ? ?struct nand_chip *chip = mtd->priv;
> ? ? ? ?uint32_t id = -1;
> @@ -942,8 +942,10 @@ static int pxa3xx_nand_scan(struct mtd_info *mtd)
> ? ? ? ?pxa3xx_flash_ids[0].erasesize = f->page_size * f->page_per_block;
> ? ? ? ?if (f->flash_width == 16)
> ? ? ? ? ? ? ? ?pxa3xx_flash_ids[0].options = NAND_BUSWIDTH_16;
> + ? ? ? pxa3xx_flash_ids[1].name = NULL;
> + ? ? ? def = pxa3xx_flash_ids;
> ?KEEP_CONFIG:
> - ? ? ? if (nand_scan_ident(mtd, 1, pxa3xx_flash_ids))
> + ? ? ? if (nand_scan_ident(mtd, 1, def))
> ? ? ? ? ? ? ? ?return -ENODEV;
> ? ? ? ?/* calculate addressing information */
> ? ? ? ?info->col_addr_cycles = (mtd->writesize >= 2048) ? 2 : 1;
> @@ -954,9 +956,9 @@ KEEP_CONFIG:
> ? ? ? ? ? ? ? ?info->row_addr_cycles = 2;
> ? ? ? ?mtd->name = mtd_names[0];
> ? ? ? ?chip->ecc.mode = NAND_ECC_HW;
> - ? ? ? chip->ecc.size = f->page_size;
> + ? ? ? chip->ecc.size = info->page_size;
>
> - ? ? ? chip->options = (f->flash_width == 16) ? NAND_BUSWIDTH_16 : 0;
> + ? ? ? chip->options = (info->reg_ndcr & NDCR_DWIDTH_M) ? NAND_BUSWIDTH_16 : 0;
> ? ? ? ?chip->options |= NAND_NO_AUTOINCR;
> ? ? ? ?chip->options |= NAND_NO_READRDY;
>
> --
> 1.7.0.4
>
>
> _______________________________________________
> linux-arm-kernel mailing list
> linux-arm-kernel at lists.infradead.org
> http://lists.infradead.org/mailman/listinfo/linux-arm-kernel
>

Re: [stable] [PATCH] MTD: pxa3xx_nand: fix nand detection issue

[Greg KH]

On Fri, Jun 03, 2011 at 11:11:54PM +0800, Lei Wen wrote:
> Tested-by: Daniel Mack <zonque@gmail.com>
> 
> Also cc to MTD maillist and stable kernel.


<formletter>

This is not the correct way to submit patches for inclusion in the
stable kernel tree.  Please read Documentation/stable_kernel_rules.txt
for how to do this properly.

</formletter>

[stable] [PATCH] MTD: pxa3xx_nand: fix nand detection issue

[Greg KH]

On Fri, Jun 03, 2011 at 11:11:54PM +0800, Lei Wen wrote:
> Tested-by: Daniel Mack <zonque@gmail.com>
> 
> Also cc to MTD maillist and stable kernel.


<formletter>

This is not the correct way to submit patches for inclusion in the
stable kernel tree.  Please read Documentation/stable_kernel_rules.txt
for how to do this properly.

</formletter>

Re: [stable] [PATCH] MTD: pxa3xx_nand: fix nand detection issue

[Daniel Mack]

On Sat, Jun 4, 2011 at 12:32 AM, Greg KH <greg@kroah.com> wrote:
> On Fri, Jun 03, 2011 at 11:11:54PM +0800, Lei Wen wrote:
>> Tested-by: Daniel Mack <zonque@gmail.com>
>>
>> Also cc to MTD maillist and stable kernel.
>
> <formletter>
>
> This is not the correct way to submit patches for inclusion in the
> stable kernel tree.  Please read Documentation/stable_kernel_rules.txt
> for how to do this properly.
>
> </formletter>

Jup.

Lei - when you collect the patches for this driver to send them out,
make sure every one gets a "Cc: stable@kernel.org" line under the
S-o-b, so it can be picked automatically later on.

Just to be sure, I count four patches now:

 - 1 from you regarding keep_config chip detection and DMA support
(which is *not* the one this thread refers to, you sent an amended
version to me privately, and it might make sense to split this patch
again)
 - 1 from me for the blank page ECC issue
 - 2 from Alex Lin (one for removal of unused variable, one plugs a memory leak)


Thanks,
Daniel

[stable] [PATCH] MTD: pxa3xx_nand: fix nand detection issue

[Daniel Mack]

On Sat, Jun 4, 2011 at 12:32 AM, Greg KH <greg@kroah.com> wrote:
> On Fri, Jun 03, 2011 at 11:11:54PM +0800, Lei Wen wrote:
>> Tested-by: Daniel Mack <zonque@gmail.com>
>>
>> Also cc to MTD maillist and stable kernel.
>
> <formletter>
>
> This is not the correct way to submit patches for inclusion in the
> stable kernel tree. ?Please read Documentation/stable_kernel_rules.txt
> for how to do this properly.
>
> </formletter>

Jup.

Lei - when you collect the patches for this driver to send them out,
make sure every one gets a "Cc: stable at kernel.org" line under the
S-o-b, so it can be picked automatically later on.

Just to be sure, I count four patches now:

 - 1 from you regarding keep_config chip detection and DMA support
(which is *not* the one this thread refers to, you sent an amended
version to me privately, and it might make sense to split this patch
again)
 - 1 from me for the blank page ECC issue
 - 2 from Alex Lin (one for removal of unused variable, one plugs a memory leak)


Thanks,
Daniel

Re: [PATCH] MTD: pxa3xx_nand: fix nand detection issue

[Artem Bityutskiy]

On Fri, 2011-06-03 at 23:11 +0800, Lei Wen wrote:
> Tested-by: Daniel Mack <zonque@gmail.com>
> 
> Also cc to MTD maillist and stable kernel.

Please, re-send the patch to the MTD mailing list and add the prober
-stable CC tag.

-- 
Best Regards,
Artem Bityutskiy (Артём Битюцкий)

[PATCH] MTD: pxa3xx_nand: fix nand detection issue

[Artem Bityutskiy]

On Fri, 2011-06-03 at 23:11 +0800, Lei Wen wrote:
> Tested-by: Daniel Mack <zonque@gmail.com>
> 
> Also cc to MTD maillist and stable kernel.

Please, re-send the patch to the MTD mailing list and add the prober
-stable CC tag.

-- 
Best Regards,
Artem Bityutskiy (????? ????????)