[PATCH] AT91RM9200 NAND support

[PATCH] AT91RM9200 NAND support

[Andrew Victor]

This is a NAND drivers for the Atmel AT91RM9200 SoC.

(No changes since it was committed to the MTD CVS on 21/02/2006)


Signed-off-by: Andrew Victor <andrew@sanpeople.com>


diff -urN -x CVS linux-2.6.17/drivers/mtd/nand/Kconfig linux-2.6.17-rc/drivers/mtd/nand/Kconfig
--- linux-2.6.17/drivers/mtd/nand/Kconfig	Mon Jun 19 14:44:58 2006
+++ linux-2.6.17-rc/drivers/mtd/nand/Kconfig	Tue May 16 16:45:44 2006
@@ -183,6 +183,12 @@
 	tristate "Support for NAND Flash on Sharp SL Series (C7xx + others)"
 	depends on MTD_NAND && ARCH_PXA
 
+config MTD_NAND_AT91
+	bool "AT91 NAND Access (Smart Media)"
+	depends on MTD_NAND && ARCH_AT91RM9200
+	help
+	  Enables access to the Smart Media Card interface on the AT91RM9200
+
 config MTD_NAND_NANDSIM
 	tristate "Support for NAND Flash Simulator"
 	depends on MTD_NAND && MTD_PARTITIONS
diff -urN -x CVS linux-2.6.17/drivers/mtd/nand/Makefile linux-2.6.17-rc/drivers/mtd/nand/Makefile
--- linux-2.6.17/drivers/mtd/nand/Makefile	Tue May 30 11:36:50 2006
+++ linux-2.6.17-rc/drivers/mtd/nand/Makefile	Tue May 16 16:45:44 2006
@@ -18,5 +18,6 @@
 obj-$(CONFIG_MTD_NAND_RTC_FROM4)	+= rtc_from4.o
 obj-$(CONFIG_MTD_NAND_SHARPSL)		+= sharpsl.o
 obj-$(CONFIG_MTD_NAND_NANDSIM)		+= nandsim.o
+obj-$(CONFIG_MTD_NAND_AT91)		+= at91_nand.o
 
 nand-objs = nand_base.o nand_bbt.o
diff -urN -x CVS linux-2.6.17/drivers/mtd/nand/at91_nand.c linux-2.6.17-rc/drivers/mtd/nand/at91_nand.c
--- linux-2.6.17/drivers/mtd/nand/at91_nand.c	Thu Jan  1 02:00:00 1970
+++ linux-2.6.17-rc/drivers/mtd/nand/at91_nand.c	Tue May 16 16:45:44 2006
@@ -0,0 +1,246 @@
+/*
+ * drivers/mtd/nand/at91_nand.c
+ *
+ *  Copyright (C) 2003 Rick Bronson
+ *
+ *  Derived from drivers/mtd/nand/autcpu12.c
+ *	 Copyright (c) 2001 Thomas Gleixner (gleixner@autronix.de)
+ *
+ *  Derived from drivers/mtd/spia.c
+ *	 Copyright (C) 2000 Steven J. Hill (sjhill@cotw.com)
+ *
+ * This program is free software; you can redistribute it and/or modify
+ * it under the terms of the GNU General Public License version 2 as
+ * published by the Free Software Foundation.
+ *
+ */
+
+#include <linux/slab.h>
+#include <linux/module.h>
+#include <linux/platform_device.h>
+#include <linux/mtd/mtd.h>
+#include <linux/mtd/nand.h>
+#include <linux/mtd/partitions.h>
+
+#include <asm/io.h>
+#include <asm/sizes.h>
+
+#include <asm/arch/hardware.h>
+#include <asm/arch/board.h>
+#include <asm/arch/gpio.h>
+
+struct at91_nand_host {
+	struct nand_chip	nand_chip;
+	struct mtd_info		mtd;
+	void __iomem		*io_base;
+	struct at91_nand_data	*board;
+};
+
+/*
+ * Hardware specific access to control-lines
+ */
+static void at91_nand_hwcontrol(struct mtd_info *mtd, int cmd)
+{
+	struct nand_chip *nand_chip = mtd->priv;
+	struct at91_nand_host *host = nand_chip->priv;
+
+	switch(cmd) {
+		case NAND_CTL_SETCLE:
+			nand_chip->IO_ADDR_W = host->io_base + (1 << host->board->cle);
+			break;
+		case NAND_CTL_CLRCLE:
+			nand_chip->IO_ADDR_W = host->io_base;
+			break;
+		case NAND_CTL_SETALE:
+			nand_chip->IO_ADDR_W = host->io_base + (1 << host->board->ale);
+			break;
+		case NAND_CTL_CLRALE:
+			nand_chip->IO_ADDR_W = host->io_base;
+			break;
+		case NAND_CTL_SETNCE:
+			break;
+		case NAND_CTL_CLRNCE:
+			break;
+	}
+}
+
+
+/*
+ * Read the Device Ready pin.
+ */
+static int at91_nand_device_ready(struct mtd_info *mtd)
+{
+	struct nand_chip *nand_chip = mtd->priv;
+	struct at91_nand_host *host = nand_chip->priv;
+
+	return at91_get_gpio_value(host->board->rdy_pin);
+}
+
+
+/*
+ * Enable NAND and detect card.
+ */
+static void at91_nand_enable(struct at91_nand_host *host)
+{
+	unsigned int csa;
+
+	/* Setup Smart Media, first enable the address range of CS3 */
+	csa = at91_sys_read(AT91_EBI_CSA);
+	at91_sys_write(AT91_EBI_CSA, csa | AT91_EBI_CS3A_SMC_SMARTMEDIA);
+
+	/* set the bus interface characteristics */
+	at91_sys_write(AT91_SMC_CSR(3), AT91_SMC_ACSS_STD | AT91_SMC_DBW_8 | AT91_SMC_WSEN
+				| AT91_SMC_NWS_(5)
+				| AT91_SMC_TDF_(1)
+				| AT91_SMC_RWSETUP_(0)	/* tDS Data Set up Time 30 - ns */
+				| AT91_SMC_RWHOLD_(1)	/* tDH Data Hold Time 20 - ns */
+	);
+
+	if (host->board->enable_pin)
+		at91_set_gpio_value(host->board->enable_pin, 0);
+}
+
+/*
+ * Disable NAND.
+ */
+static void at91_nand_disable(struct at91_nand_host *host)
+{
+	if (host->board->enable_pin)
+		at91_set_gpio_value(host->board->enable_pin, 1);
+}
+
+/*
+ * Probe for the NAND device.
+ */
+static int __init at91_nand_probe(struct platform_device *pdev)
+{
+	struct at91_nand_host *host;
+	struct mtd_info *mtd;
+	struct nand_chip *nand_chip;
+	int res;
+
+#ifdef CONFIG_MTD_PARTITIONS
+	struct mtd_partition *partitions = NULL;
+	int num_partitions = 0;
+#endif
+
+	/* Allocate memory for the device structure (and zero it) */
+	host = kzalloc(sizeof(struct at91_nand_host), GFP_KERNEL);
+	if (!host) {
+		printk(KERN_ERR "at91_nand: failed to allocate device structure.\n");
+		return -ENOMEM;
+	}
+
+	host->io_base = ioremap(pdev->resource[0].start,
+				pdev->resource[0].end - pdev->resource[0].start + 1);
+	if (host->io_base == NULL) {
+		printk(KERN_ERR "at91_nand: ioremap failed\n");
+		kfree(host);
+		return -EIO;
+	}
+
+	mtd = &host->mtd;
+	nand_chip = &host->nand_chip;
+	host->board = pdev->dev.platform_data;
+
+	nand_chip->priv = host;		/* link the private data structures */
+	mtd->priv = nand_chip;
+
+	/* Set address of NAND IO lines */
+	nand_chip->IO_ADDR_R = host->io_base;
+	nand_chip->IO_ADDR_W = host->io_base;
+	nand_chip->hwcontrol = at91_nand_hwcontrol;
+	nand_chip->dev_ready = at91_nand_device_ready;
+	nand_chip->eccmode = NAND_ECC_SOFT;	/* enable ECC */
+	nand_chip->chip_delay = 20;		/* 20us command delay time */
+
+	platform_set_drvdata(pdev, host);
+	at91_nand_enable(host);
+
+	if (host->board->det_pin) {
+		if (at91_get_gpio_value(host->board->det_pin)) {
+			printk ("No SmartMedia card inserted.\n");
+			res = ENXIO;
+			goto out;
+		}
+	}
+
+	/* Scan to find existance of the device */
+	if (nand_scan(mtd, 1)) {
+		res = -ENXIO;
+		goto out;
+	}
+
+#ifdef CONFIG_MTD_PARTITIONS
+	if (host->board->partition_info)
+		partitions = host->board->partition_info(mtd->size, &num_partitions);
+
+	if ((!partitions) || (num_partitions == 0)) {
+		printk(KERN_ERR "at91_nand: No parititions defined, or unsupported device.\n");
+		res = ENXIO;
+		goto out;
+	}
+
+	res = add_mtd_partitions(mtd, partitions, num_partitions);
+#else
+	res = add_mtd_device(mtd);
+#endif
+
+out:
+	if (res) {
+		at91_nand_disable(host);
+		platform_set_drvdata(pdev, NULL);
+
+		iounmap(host->io_base);
+		kfree(host);
+	}
+
+	return res;
+}
+
+/*
+ * Remove a NAND device.
+ */
+static int __devexit at91_nand_remove(struct platform_device *pdev)
+{
+	struct at91_nand_host *host = platform_get_drvdata(pdev);
+	struct mtd_info *mtd = &host->mtd;
+
+	del_mtd_partitions(mtd);
+	del_mtd_device(mtd);
+
+	at91_nand_disable(host);
+
+	iounmap(host->io_base);
+	kfree(host);
+
+	return 0;
+}
+
+static struct platform_driver at91_nand_driver = {
+	.probe		= at91_nand_probe,
+	.remove		= at91_nand_remove,
+	.driver		= {
+		.name	= "at91_nand",
+		.owner	= THIS_MODULE,
+	},
+};
+
+static int __init at91_nand_init(void)
+{
+	return platform_driver_register(&at91_nand_driver);
+}
+
+
+static void __exit at91_nand_exit(void)
+{
+	platform_driver_unregister(&at91_nand_driver);
+}
+
+
+module_init(at91_nand_init);
+module_exit(at91_nand_exit);
+
+MODULE_LICENSE("GPL");
+MODULE_AUTHOR("Rick Bronson");
+MODULE_DESCRIPTION("NAND/SmartMedia driver for AT91RM9200");

Re: [PATCH] AT91RM9200 NAND support

[Thomas Gleixner]

On Tue, 2006-06-20 at 08:54 +0200, Andrew Victor wrote:
> This is a NAND drivers for the Atmel AT91RM9200 SoC.
> 
> (No changes since it was committed to the MTD CVS on 21/02/2006)

As I pointed out in my previous mail, the driver needs to be updated to
work with the latest changes in the nand driver.

> +/*
> + * Hardware specific access to control-lines
> + */
> +static void at91_nand_hwcontrol(struct mtd_info *mtd, int cmd)

This function needs to be adjusted.

> +#ifdef CONFIG_MTD_PARTITIONS
> +	if (host->board->partition_info)
> +		partitions = host->board->partition_info(mtd->size, &num_partitions);
> +
> +	if ((!partitions) || (num_partitions == 0)) {
> +		printk(KERN_ERR "at91_nand: No parititions defined, or unsupported device.\n");
> +		res = ENXIO;
> +		goto out;
> +	}
> +
> +	res = add_mtd_partitions(mtd, partitions, num_partitions);
> +#else
> +	res = add_mtd_device(mtd);
> +#endif
> +
> +out:

If you error out after a sucessful nand_scan() - i.e. chip was detected
- you have to call nand_release(). nand_scan() allocates memory which
gets freed inside nand_release()

> +	if (res) {
> +		at91_nand_disable(host);
> +		platform_set_drvdata(pdev, NULL);
> +
> +		iounmap(host->io_base);
> +		kfree(host);
> +	}
> +
> +	return res;
> +}
> +
> +/*
> + * Remove a NAND device.
> + */
> +static int __devexit at91_nand_remove(struct platform_device *pdev)
> +{
> +	struct at91_nand_host *host = platform_get_drvdata(pdev);
> +	struct mtd_info *mtd = &host->mtd;
> +
> +	del_mtd_partitions(mtd);
> +	del_mtd_device(mtd);

nand_release() does this and you have to call it - otherwise you get a
memory leak !

	tglx

Re: [PATCH] AT91RM9200 NAND support

[Andrew Victor]

hi Thomas,

> As I pointed out in my previous mail, the driver needs to be updated to
> work with the latest changes in the nand driver.

I didn't see that message - haven't been following the MTD mailing list
lately.


> > +/*
> > + * Hardware specific access to control-lines
> > + */
> > +static void at91_nand_hwcontrol(struct mtd_info *mtd, int cmd)
> 
> This function needs to be adjusted.

Some hints of how it should be adjusted would be useful.
(I don't actually have NAND hardware to test it with).


> If you error out after a sucessful nand_scan() - i.e. chip was detected
> - you have to call nand_release(). nand_scan() allocates memory which
> gets freed inside nand_release()

OK, will add calls to nand_release().



Regards,
  Andrew Victor

Re: [PATCH] AT91RM9200 NAND support

[Savin Zlobec]

[-- Attachment #1: Type: text/plain, Size: 419 bytes --]

Andrew Victor wrote:

>hi Thomas,
>
>  
>
>>As I pointed out in my previous mail, the driver needs to be updated to
>>work with the latest changes in the nand driver.
>>    
>>
I'am working on this right now. I've changed the at91 driver and I'am 
going to
test it when I finish the changes on YAFFS - read/write_ecc and oobblock are
not part of mtd_info any more...

I'am attaching the at91_nand.c changes.
 
--
savin

[-- Attachment #2: at91_nand.diff --]
[-- Type: text/x-patch, Size: 1528 bytes --]

--- drivers/mtd.old/nand/at91_nand.c	2006-06-20 09:09:21.000000000 +0200
+++ drivers/mtd/nand/at91_nand.c	2006-06-20 09:33:21.000000000 +0200
@@ -39,12 +39,13 @@
 /*
  * Hardware specific access to control-lines
  */
-static void at91_nand_hwcontrol(struct mtd_info *mtd, int cmd)
+static void at91_nand_hwcontrol(struct mtd_info *mtd, int cmd, unsigned int ctrl)
 {
 	struct nand_chip *nand_chip = mtd->priv;
 	struct at91_nand_host *host = nand_chip->priv;
 
-	switch(cmd) {
+	if (ctrl & NAND_CTRL_CHANGE) {
+		switch(cmd) {
 		case NAND_CTL_SETCLE:
 			nand_chip->IO_ADDR_W = host->io_base + (1 << host->board->cle);
 			break;
@@ -61,7 +62,10 @@
 			break;
 		case NAND_CTL_CLRNCE:
 			break;
+		}
 	}
+	if (cmd != NAND_CMD_NONE)
+		writeb(cmd, chip->IO_ADDR_W);
 }
 
 
@@ -145,13 +149,14 @@
 
 	nand_chip->priv = host;		/* link the private data structures */
 	mtd->priv = nand_chip;
+	mtd->owner = THIS_MODULE;
 
 	/* Set address of NAND IO lines */
 	nand_chip->IO_ADDR_R = host->io_base;
 	nand_chip->IO_ADDR_W = host->io_base;
-	nand_chip->hwcontrol = at91_nand_hwcontrol;
+	nand_chip->cmd_ctrl = at91_nand_hwcontrol;
 	nand_chip->dev_ready = at91_nand_device_ready;
-	nand_chip->eccmode = NAND_ECC_SOFT;	/* enable ECC */
+	nand_chip->ecc.mode = NAND_ECC_SOFT;	/* enable ECC */
 	nand_chip->chip_delay = 20;		/* 20us command delay time */
 
 	platform_set_drvdata(pdev, host);
@@ -170,6 +175,7 @@
 		res = -ENXIO;
 		goto out;
 	}
+	nand_release(mtd);
 
 #ifdef CONFIG_MTD_PARTITIONS
 	if (host->board->partition_info)

Re: [PATCH] AT91RM9200 NAND support

[Thomas Gleixner]

On Tue, 2006-06-20 at 09:43 +0200, Savin Zlobec wrote:
> I'am working on this right now. I've changed the at91 driver and I'am 
> going to
> test it when I finish the changes on YAFFS - read/write_ecc and oobblock are
> not part of mtd_info any more...
> 
> I'am attaching the at91_nand.c changes.

Sorry, I doubt that any of these changes will actually work. The patch
below might save your time.

	tglx


Index: mtd/drivers/mtd/nand/at91_nand.c
===================================================================
--- mtd.orig/drivers/mtd/nand/at91_nand.c
+++ mtd/drivers/mtd/nand/at91_nand.c
@@ -37,33 +37,22 @@ struct at91_nand_host {
 };
 
 /*
- * Hardware specific access to control-lines
+ * Hardware specific way to write commands and address bytes to
+ * the chip.
  */
-static void at91_nand_hwcontrol(struct mtd_info *mtd, int cmd)
+static void at91_nand_cmd(struct mtd_info *mtd, int cmd, unsigned int ctrl)
 {
 	struct nand_chip *nand_chip = mtd->priv;
 	struct at91_nand_host *host = nand_chip->priv;
 
-	switch(cmd) {
-		case NAND_CTL_SETCLE:
-			nand_chip->IO_ADDR_W = host->io_base + (1 << host->board->cle);
-			break;
-		case NAND_CTL_CLRCLE:
-			nand_chip->IO_ADDR_W = host->io_base;
-			break;
-		case NAND_CTL_SETALE:
-			nand_chip->IO_ADDR_W = host->io_base + (1 << host->board->ale);
-			break;
-		case NAND_CTL_CLRALE:
-			nand_chip->IO_ADDR_W = host->io_base;
-			break;
-		case NAND_CTL_SETNCE:
-			break;
-		case NAND_CTL_CLRNCE:
-			break;
-	}
-}
+	if (cmd == NAND_CMD_NONE)
+		return;
 
+	if (ctrl & NAND_CLE)
+		writeb(cmd, host->io_base + (1 << host->board->cle));
+	else
+		writeb(cmd, host->io_base + (1 << host->board->ale));
+}
 
 /*
  * Read the Device Ready pin.
@@ -89,11 +78,12 @@ static void at91_nand_enable(struct at91
 	at91_sys_write(AT91_EBI_CSA, csa | AT91_EBI_CS3A_SMC_SMARTMEDIA);
 
 	/* set the bus interface characteristics */
-	at91_sys_write(AT91_SMC_CSR(3), AT91_SMC_ACSS_STD | AT91_SMC_DBW_8 | AT91_SMC_WSEN
-				| AT91_SMC_NWS_(5)
-				| AT91_SMC_TDF_(1)
-				| AT91_SMC_RWSETUP_(0)	/* tDS Data Set up Time 30 - ns */
-				| AT91_SMC_RWHOLD_(1)	/* tDH Data Hold Time 20 - ns */
+	at91_sys_write(AT91_SMC_CSR(3), AT91_SMC_ACSS_STD | AT91_SMC_DBW_8
+		       | AT91_SMC_WSEN
+		       | AT91_SMC_NWS_(5)
+		       | AT91_SMC_TDF_(1)
+		       | AT91_SMC_RWSETUP_(0)	/* tDS Data Set up Time 30 - ns */
+		       | AT91_SMC_RWHOLD_(1)	/* tDH Data Hold Time 20 - ns */
 	);
 
 	if (host->board->enable_pin)
@@ -145,13 +135,14 @@ static int __init at91_nand_probe(struct
 
 	nand_chip->priv = host;		/* link the private data structures */
 	mtd->priv = nand_chip;
+	mtd->owner = THIS_MODULE;
 
 	/* Set address of NAND IO lines */
 	nand_chip->IO_ADDR_R = host->io_base;
 	nand_chip->IO_ADDR_W = host->io_base;
-	nand_chip->hwcontrol = at91_nand_hwcontrol;
+	nand_chip->cmd_ctrl = at91_nand_cmd;
 	nand_chip->dev_ready = at91_nand_device_ready;
-	nand_chip->eccmode = NAND_ECC_SOFT;	/* enable ECC */
+	nand_chip->ecc.mode = NAND_ECC_SOFT;	/* enable ECC */
 	nand_chip->chip_delay = 20;		/* 20us command delay time */
 
 	platform_set_drvdata(pdev, host);
@@ -173,28 +164,30 @@ static int __init at91_nand_probe(struct
 
 #ifdef CONFIG_MTD_PARTITIONS
 	if (host->board->partition_info)
-		partitions = host->board->partition_info(mtd->size, &num_partitions);
+		partitions = host->board->partition_info(mtd->size,
+							 &num_partitions);
 
 	if ((!partitions) || (num_partitions == 0)) {
-		printk(KERN_ERR "at91_nand: No parititions defined, or unsupported device.\n");
+		printk(KERN_ERR "at91_nand: No parititions defined, "
+		       "or unsupported device.\n");
 		res = ENXIO;
-		goto out;
+		goto release;
 	}
 
 	res = add_mtd_partitions(mtd, partitions, num_partitions);
 #else
 	res = add_mtd_device(mtd);
 #endif
+	if (!res)
+		return res;
 
-out:
-	if (res) {
-		at91_nand_disable(host);
-		platform_set_drvdata(pdev, NULL);
-	
-		iounmap(host->io_base);
-		kfree(host);
-	}
-
+ release:
+	nand_release(mtd);
+ out:
+	at91_nand_disable(host);
+	platform_set_drvdata(pdev, NULL);
+	iounmap(host->io_base);
+	kfree(host);
 	return res;
 }
 
@@ -206,8 +199,7 @@ static int __devexit at91_nand_remove(st
 	struct at91_nand_host *host = platform_get_drvdata(pdev);
 	struct mtd_info *mtd = &host->mtd;
 
-	del_mtd_partitions(mtd);
-	del_mtd_device(mtd);
+	nand_release(mtd);
 
 	at91_nand_disable(host);
 

Re: [PATCH] AT91RM9200 NAND support

[Thomas Gleixner]

Andrew,

On Tue, 2006-06-20 at 10:00 +0200, Thomas Gleixner wrote:
> +	if (cmd == NAND_CMD_NONE)
> +		return;
>  
> +	if (ctrl & NAND_CLE)
> +		writeb(cmd, host->io_base + (1 << host->board->cle));
> +	else
> +		writeb(cmd, host->io_base + (1 << host->board->ale));
> +}

Can you please fixup those host->board->cle and ale values at the point
where they actually are initialized ? 

	tglx

Re: [PATCH] AT91RM9200 NAND support

[David Woodhouse]

On Tue, 2006-06-20 at 09:08 +0200, Thomas Gleixner wrote:
> As I pointed out in my previous mail, the driver needs to be updated to
> work with the latest changes in the nand driver. 

Apologies for not pulling this into the git tree earlier, btw. If I'd
done that _before_ the recent round of changes, then it would have been
updated with the others.

-- 
dwmw2

Re: [PATCH] AT91RM9200 NAND support

[Savin Zlobec]

Thomas Gleixner wrote:

>On Tue, 2006-06-20 at 09:43 +0200, Savin Zlobec wrote:
>  
>
>>I'am working on this right now. I've changed the at91 driver and I'am 
>>going to
>>test it when I finish the changes on YAFFS - read/write_ecc and oobblock are
>>not part of mtd_info any more...
>>
>>I'am attaching the at91_nand.c changes.
>>    
>>
>
>Sorry, I doubt that any of these changes will actually work. The patch
>below might save your time.
>  
>
Thanks (I did a quick diff of other nand drivers, but missed the #error...).

I've put the kernel with latest mtd on my board and got the following:

Nand is recognized as 'NAND device: Manufacturer ID: 0x98, Chip ID: 0x75 
(Toshiba NAND 32MiB 3,3V 8-bit)',
should be 'NAND device: Manufacturer ID: 0xec, Chip ID: 0x75 (Samsung 
NAND 32MiB 3,3V 8-bit)'.

No bad blocks are detected at initial nand scan, but when running 
flash_eraseall all blocks are found bad.

Re: [PATCH] AT91RM9200 NAND support

[Thomas Gleixner]

On Tue, 2006-06-20 at 10:07 +0100, David Woodhouse wrote:
> On Tue, 2006-06-20 at 09:08 +0200, Thomas Gleixner wrote:
> > As I pointed out in my previous mail, the driver needs to be updated to
> > work with the latest changes in the nand driver. 
> 
> Apologies for not pulling this into the git tree earlier, btw. If I'd
> done that _before_ the recent round of changes, then it would have been
> updated with the others.

Fixed it already. When its confirmed to work, I put it in

	tglx

Re: [PATCH] AT91RM9200 NAND support

[Thomas Gleixner]

On Tue, 2006-06-20 at 11:07 +0200, Savin Zlobec wrote:
> I've put the kernel with latest mtd on my board and got the following:
> 
> Nand is recognized as 'NAND device: Manufacturer ID: 0x98, Chip ID: 0x75 
> (Toshiba NAND 32MiB 3,3V 8-bit)',
> should be 'NAND device: Manufacturer ID: 0xec, Chip ID: 0x75 (Samsung 
> NAND 32MiB 3,3V 8-bit)'.
> 
> No bad blocks are detected at initial nand scan, but when running 
> flash_eraseall all blocks are found bad.

Hmm. The interface has not changed, AFAICT. What version of
flash_eraseall are you using and which commandline options ?

Can you verify with the latest mtd-utils from 

http://git.infradead.org/?p=mtd-utils.git;a=summary

Thanks,

	tglx

Re: [PATCH] AT91RM9200 NAND support

[Savin Zlobec]

Thomas Gleixner wrote:

>On Tue, 2006-06-20 at 11:07 +0200, Savin Zlobec wrote:
>  
>
>>I've put the kernel with latest mtd on my board and got the following:
>>
>>Nand is recognized as 'NAND device: Manufacturer ID: 0x98, Chip ID: 0x75 
>>(Toshiba NAND 32MiB 3,3V 8-bit)',
>>should be 'NAND device: Manufacturer ID: 0xec, Chip ID: 0x75 (Samsung 
>>NAND 32MiB 3,3V 8-bit)'.
>>
>>No bad blocks are detected at initial nand scan, but when running 
>>flash_eraseall all blocks are found bad.
>>    
>>
>
>Hmm. The interface has not changed, AFAICT. What version of
>flash_eraseall are you using and which commandline options ?
>
>Can you verify with the latest mtd-utils from 
>
>http://git.infradead.org/?p=mtd-utils.git;a=summary
>  
>
I did the following modifications to nand_base.c :

--- drivers/mtd/nand/nand_base.c.orig   2006-06-20 12:19:10.000000000 +0200
+++ drivers/mtd/nand/nand_base.c        2006-06-20 12:18:51.000000000 +0200
@@ -1134,10 +1134,6 @@
                else
                        buf += ops->ooblen;

-               readlen -= ops->ooblen;
-               if (!readlen)
-                       break;
-
                if (!(chip->options & NAND_NO_READRDY)) {
                        /*
                         * Apply delay or wait for ready/busy pin. Do this
@@ -1151,6 +1147,10 @@
                                nand_wait_ready(mtd);
                }

+               readlen -= ops->ooblen;
+               if (!readlen)
+                       break;
+
                /* Increment page address */
                realpage++;

And managed to erase the nand flash  and mount JFFS2 on it, but writting 
still
didn't work -  got  errors like :

Data CRC d507eb40 != calculated CRC 2e617dde for node at 00e4f700

The I put a call to nand_wait_ready at the top of nand_command function and
as far as I could test everything worked (exept that the flash is still 
recognized
as Toshiba (0x98) not Samsung (0xec)).

It looks (to me) that there are still some parts of the code that should 
wait for
nand to get ready before sending commands.

savin

Re: [PATCH] AT91RM9200 NAND support

[Thomas Gleixner]

Savin,

On Tue, 2006-06-20 at 12:49 +0200, Savin Zlobec wrote:

Patch is correct, applied. Thanks

> And managed to erase the nand flash  and mount JFFS2 on it, but writting 
> still
> didn't work -  got  errors like :
> 
> Data CRC d507eb40 != calculated CRC 2e617dde for node at 00e4f700
> 
> The I put a call to nand_wait_ready at the top of nand_command function and
> as far as I could test everything worked (exept that the flash is still 
> recognized as Toshiba (0x98) not Samsung (0xec)).

Well, we read the manufacturer ID. When we get 98H, how should we know
that this is a Samsung part ? And I doubt that this is a quad bit flip.

> It looks (to me) that there are still some parts of the code that should 
> wait for nand to get ready before sending commands.

The only point where this really matters is, when we read data from
those chips. They have a horrible feature, which automatically loads the
next page into the internal buffer. There are only two places where we
actually read from the device. On has the check at the correct place
already, the other fixed you up.

Can you please remove the nand_wait_ready() call in nand_command() and
test the following patch ? It disables the ready busy pin and uses the
chip_delay. Please check, whether the 20us are correct. You can safely
set it to 50 without breaking stuff.

	tglx


Index: mtd/drivers/mtd/nand/at91_nand.c
===================================================================
--- mtd.orig/drivers/mtd/nand/at91_nand.c
+++ mtd/drivers/mtd/nand/at91_nand.c
@@ -141,7 +141,6 @@ static int __init at91_nand_probe(struct
 	nand_chip->IO_ADDR_R = host->io_base;
 	nand_chip->IO_ADDR_W = host->io_base;
 	nand_chip->cmd_ctrl = at91_nand_cmd;
-	nand_chip->dev_ready = at91_nand_device_ready;
 	nand_chip->ecc.mode = NAND_ECC_SOFT;	/* enable ECC */
 	nand_chip->chip_delay = 20;		/* 20us command delay time */
 

Re: [PATCH] AT91RM9200 NAND support

[Savin Zlobec]

Thomas Gleixner wrote:

>Savin,
>
>On Tue, 2006-06-20 at 12:49 +0200, Savin Zlobec wrote:
>
>Patch is correct, applied. Thanks
>
>  
>
>>And managed to erase the nand flash  and mount JFFS2 on it, but writting 
>>still
>>didn't work -  got  errors like :
>>
>>Data CRC d507eb40 != calculated CRC 2e617dde for node at 00e4f700
>>
>>The I put a call to nand_wait_ready at the top of nand_command function and
>>as far as I could test everything worked (exept that the flash is still 
>>recognized as Toshiba (0x98) not Samsung (0xec)).
>>    
>>
>
>Well, we read the manufacturer ID. When we get 98H, how should we know
>that this is a Samsung part ? And I doubt that this is a quad bit flip.
>  
>
What bothers me is that MTD from 2.6.17 reads manufacturer ID = 0xec,  and
the latest git MTD reads 0x98. The chip on my board is Samsung.

>  
>
>>It looks (to me) that there are still some parts of the code that should 
>>wait for nand to get ready before sending commands.
>>    
>>
>
>The only point where this really matters is, when we read data from
>those chips. They have a horrible feature, which automatically loads the
>next page into the internal buffer. There are only two places where we
>actually read from the device. On has the check at the correct place
>already, the other fixed you up.
>
>Can you please remove the nand_wait_ready() call in nand_command() and
>test the following patch ? It disables the ready busy pin and uses the
>chip_delay. Please check, whether the 20us are correct. You can safely
>set it to 50 without breaking stuff.
>  
>
It doesn't work with 20us nor with 50us.

savin

Re: [PATCH] AT91RM9200 NAND support

[Thomas Gleixner]

On Tue, 2006-06-20 at 13:42 +0200, Savin Zlobec wrote:
> >Well, we read the manufacturer ID. When we get 98H, how should we know
> >that this is a Samsung part ? And I doubt that this is a quad bit flip.
> > 
> What bothers me is that MTD from 2.6.17 reads manufacturer ID = 0xec,  and
> the latest git MTD reads 0x98. The chip on my board is Samsung.

Thats indeed strange. Whats the exact part number ?

> >Can you please remove the nand_wait_ready() call in nand_command() and
> >test the following patch ? It disables the ready busy pin and uses the
> >chip_delay. Please check, whether the 20us are correct. You can safely
> >set it to 50 without breaking stuff.
> >
> It doesn't work with 20us nor with 50us.

Ok, revert the patch. I really need to know which code path triggers
this behaviour. Can you apply the patch below and compile the kernel
with CONFIG_DEBUG_INFO.

When the chip is not in READY state on entry of nand_command() debug
info is printed. Please decode the kernel addresses of "Last caller" and
"Current caller" with 

addr2line -e vmlinux 0xNNNNNNNNN

Thanks

	tglx

diff --git a/drivers/mtd/nand/nand_base.c b/drivers/mtd/nand/nand_base.c
index 77406fc..b6e08ea 100644
--- a/drivers/mtd/nand/nand_base.c
+++ b/drivers/mtd/nand/nand_base.c
@@ -430,6 +430,9 @@ static void nand_wait_ready(struct mtd_i
 	led_trigger_event(nand_led_trigger, LED_OFF);
 }
 
+static void *last_caller;
+static int last_command;
+
 /**
  * nand_command - [DEFAULT] Send command to NAND device
  * @mtd:	MTD device structure
@@ -446,6 +449,20 @@ static void nand_command(struct mtd_info
 	register struct nand_chip *chip = mtd->priv;
 	int ctrl = NAND_CTRL_CLE | NAND_CTRL_CHANGE;
 
+	if (command != NAND_CMD_RESET && command != NAND_CMD_STATUS &&
+	    chip->dev_ready && !chip->dev_ready(mtd)) {
+
+		printk("Chip not ready in nand_command():\n");
+		printk("Last caller: %p\n", last_caller);
+		printk("Last command: 0x%02x\n", last_command);
+		printk("Current caller: %p\n", __builtin_return_address(0););
+		printk("Current command: 0x%02x\n", command);
+
+		nand_wait_ready(mtd);
+	}
+	last_caller = __builtin_return_address(0);
+	last_command = command;
+
 	/*
 	 * Write out the command to the device.
 	 */

 

Re: [PATCH] AT91RM9200 NAND support

[Savin Zlobec]

Thomas Gleixner wrote:

>On Tue, 2006-06-20 at 13:42 +0200, Savin Zlobec wrote:
>  
>
>>>Well, we read the manufacturer ID. When we get 98H, how should we know
>>>that this is a Samsung part ? And I doubt that this is a quad bit flip.
>>>
>>>      
>>>
>>What bothers me is that MTD from 2.6.17 reads manufacturer ID = 0xec,  and
>>the latest git MTD reads 0x98. The chip on my board is Samsung.
>>    
>>
>
>Thats indeed strange. Whats the exact part number ?
>
>  
>
>>>Can you please remove the nand_wait_ready() call in nand_command() and
>>>test the following patch ? It disables the ready busy pin and uses the
>>>chip_delay. Please check, whether the 20us are correct. You can safely
>>>set it to 50 without breaking stuff.
>>>
>>>      
>>>
>>It doesn't work with 20us nor with 50us.
>>    
>>
>
>Ok, revert the patch. I really need to know which code path triggers
>this behaviour. Can you apply the patch below and compile the kernel
>with CONFIG_DEBUG_INFO.
>
>When the chip is not in READY state on entry of nand_command() debug
>info is printed. Please decode the kernel addresses of "Last caller" and
>"Current caller" with 
>
>addr2line -e vmlinux 0xNNNNNNNNN
>  
>
Here it is.
    savin

Chip not ready in nand_command():
Last caller: c012aa04 (nand_base.c:389)
Last command: 0x70
Current caller: c012c0e8 (nand_base.c:1720)
Current command: 0x60

384     static int nand_check_wp(struct mtd_info *mtd)
385     {
386             struct nand_chip *chip = mtd->priv;
387             /* Check the WP bit */
388             chip->cmdfunc(mtd, NAND_CMD_STATUS, -1, -1);
389             return (chip->read_byte(mtd) & NAND_STATUS_WP) ? 0 : 1;
390     }


1715    static void single_erase_cmd(struct mtd_info *mtd, int page)
1716    {
1717            struct nand_chip *chip = mtd->priv;
1718            /* Send commands to erase a block */
1719            chip->cmdfunc(mtd, NAND_CMD_ERASE1, -1, page);
1720            chip->cmdfunc(mtd, NAND_CMD_ERASE2, -1, -1);
1721    }

Re: [PATCH] AT91RM9200 NAND support

[Thomas Gleixner]

Savin,

On Tue, 2006-06-20 at 14:28 +0200, Savin Zlobec wrote:
> Chip not ready in nand_command():
> Last caller: c012aa04 (nand_base.c:389)
> Last command: 0x70
> Current caller: c012c0e8 (nand_base.c:1720)
> Current command: 0x60
> 
> 384     static int nand_check_wp(struct mtd_info *mtd)
> 385     {
> 386             struct nand_chip *chip = mtd->priv;
> 387             /* Check the WP bit */
> 388             chip->cmdfunc(mtd, NAND_CMD_STATUS, -1, -1);
> 389             return (chip->read_byte(mtd) & NAND_STATUS_WP) ? 0 : 1;
> 390     }
>
> 1715    static void single_erase_cmd(struct mtd_info *mtd, int page)
> 1716    {
> 1717            struct nand_chip *chip = mtd->priv;
> 1718            /* Send commands to erase a block */
> 1719            chip->cmdfunc(mtd, NAND_CMD_ERASE1, -1, page);
> 1720            chip->cmdfunc(mtd, NAND_CMD_ERASE2, -1, -1);
> 1721    }

The status command does not influence ready/busy. The one before that
nand_check_wp() call, is a chip reset command, but we wait for the chip
to become ready again.

Please give me the exact part number, so I can lookup the data sheet.

	tglx

Re: [PATCH] AT91RM9200 NAND support

[Thomas Gleixner]

On Tue, 2006-06-20 at 14:44 +0200, Thomas Gleixner wrote:
> Savin,
> 
> On Tue, 2006-06-20 at 14:28 +0200, Savin Zlobec wrote:
> > Chip not ready in nand_command():
> > Last caller: c012aa04 (nand_base.c:389)
> > Last command: 0x70
> > Current caller: c012c0e8 (nand_base.c:1720)
> > Current command: 0x60
> > 
> > 384     static int nand_check_wp(struct mtd_info *mtd)
> > 385     {
> > 386             struct nand_chip *chip = mtd->priv;
> > 387             /* Check the WP bit */
> > 388             chip->cmdfunc(mtd, NAND_CMD_STATUS, -1, -1);
> > 389             return (chip->read_byte(mtd) & NAND_STATUS_WP) ? 0 : 1;
> > 390     }
> >
> > 1715    static void single_erase_cmd(struct mtd_info *mtd, int page)
> > 1716    {
> > 1717            struct nand_chip *chip = mtd->priv;
> > 1718            /* Send commands to erase a block */
> > 1719            chip->cmdfunc(mtd, NAND_CMD_ERASE1, -1, page);
> > 1720            chip->cmdfunc(mtd, NAND_CMD_ERASE2, -1, -1);
> > 1721    }
> 
> The status command does not influence ready/busy. The one before that
> nand_check_wp() call, is a chip reset command, but we wait for the chip
> to become ready again.
> 
> Please give me the exact part number, so I can lookup the data sheet.

Oops, looked at the wrong place. The culprit is elsewhere.

Please replace the debug patch by the one below.

	tglx

diff --git a/drivers/mtd/nand/nand_base.c b/drivers/mtd/nand/nand_base.c
index 77406fc..b61a534 100644
--- a/drivers/mtd/nand/nand_base.c
+++ b/drivers/mtd/nand/nand_base.c
@@ -430,6 +430,9 @@ static void nand_wait_ready(struct mtd_i
 	led_trigger_event(nand_led_trigger, LED_OFF);
 }
 
+static void *last_caller;
+static int last_command;
+
 /**
  * nand_command - [DEFAULT] Send command to NAND device
  * @mtd:	MTD device structure
@@ -446,6 +449,22 @@ static void nand_command(struct mtd_info
 	register struct nand_chip *chip = mtd->priv;
 	int ctrl = NAND_CTRL_CLE | NAND_CTRL_CHANGE;
 
+	if (command != NAND_CMD_RESET && command != NAND_CMD_STATUS) {
+
+		if (chip->dev_ready && !chip->dev_ready(mtd)) {
+			printk("Chip not ready in nand_command():\n");
+			printk("Last caller: %p\n", last_caller);
+			printk("Last command: 0x%02x\n", last_command);
+			printk("Current caller: %p\n",
+			       __builtin_return_address(0););
+			printk("Current command: 0x%02x\n", command);
+
+			nand_wait_ready(mtd);
+		}
+		last_caller = __builtin_return_address(0);
+		last_command = command;
+	}
+
 	/*
 	 * Write out the command to the device.
 	 */

Re: [PATCH] AT91RM9200 NAND support

[Savin Zlobec]

Thomas Gleixner wrote:

>Savin,
>
>On Tue, 2006-06-20 at 14:28 +0200, Savin Zlobec wrote:
>  
>
>>Chip not ready in nand_command():
>>Last caller: c012aa04 (nand_base.c:389)
>>Last command: 0x70
>>Current caller: c012c0e8 (nand_base.c:1720)
>>Current command: 0x60
>>
>>384     static int nand_check_wp(struct mtd_info *mtd)
>>385     {
>>386             struct nand_chip *chip = mtd->priv;
>>387             /* Check the WP bit */
>>388             chip->cmdfunc(mtd, NAND_CMD_STATUS, -1, -1);
>>389             return (chip->read_byte(mtd) & NAND_STATUS_WP) ? 0 : 1;
>>390     }
>>
>>1715    static void single_erase_cmd(struct mtd_info *mtd, int page)
>>1716    {
>>1717            struct nand_chip *chip = mtd->priv;
>>1718            /* Send commands to erase a block */
>>1719            chip->cmdfunc(mtd, NAND_CMD_ERASE1, -1, page);
>>1720            chip->cmdfunc(mtd, NAND_CMD_ERASE2, -1, -1);
>>1721    }
>>    
>>
>
>The status command does not influence ready/busy. The one before that
>nand_check_wp() call, is a chip reset command, but we wait for the chip
>to become ready again.
>
>Please give me the exact part number, so I can lookup the data sheet.
>  
>
The part is K9F5608U0C. And I've got some more debug info.

Thanks,
    savin

Chip not ready in nand_command():
Last caller: c012afa8 (nand_base.c:736)
Last command: 0x70
Current caller: c012bf8c (nand_base.c:1374)
Current command: 0x80

716     static int nand_wait(struct mtd_info *mtd, struct nand_chip 
*chip, int state)
...
733             if ((state == FL_ERASING) && (chip->options & NAND_IS_AND))
734                     chip->cmdfunc(mtd, NAND_CMD_STATUS_MULTI, -1, -1);
735             else
736                     chip->cmdfunc(mtd, NAND_CMD_STATUS, -1, -1);

1367    static int nand_write_page(struct mtd_info *mtd, struct 
nand_chip *chip,
1368                               const uint8_t *buf, int page, int cached)
1369    {
1370            int status;
1371
1372            chip->cmdfunc(mtd, NAND_CMD_SEQIN, 0x00, page);
1373
1374            chip->ecc.write_page(mtd, chip, buf);

Chip not ready in nand_command():
Last caller: c012afa8 (nand_base.c:736)
Last command: 0x70
Current caller: c012b644 (nand_base.c:988)
Current command: 0x00

953     static int nand_do_read_ops(struct mtd_info *mtd, loff_t from,
...
987                             if (likely(sndcmd)) {
988                                     chip->cmdfunc(mtd, 
NAND_CMD_READ0, 0x00, page);
989                                     sndcmd = 0;
990                             }

Re: [PATCH] AT91RM9200 NAND support

[Savin Zlobec]

Thomas Gleixner wrote:

>On Tue, 2006-06-20 at 14:44 +0200, Thomas Gleixner wrote:
>  
>
> Oops, looked at the wrong place. The culprit is elsewhere.
>
>Please replace the debug patch by the one below.
>  
>
[...]

Here is what I get.
    savin

Chip not ready in nand_command():
Last caller: c012bd44 (nand_base.c:1638)
Last command: 0x10
Current caller: c012c12c (nand_base.c:1728)
Current command: 0x60

Chip not ready in nand_command():
Last caller: c012bffc (nand_base.c:1393)
Last command: 0x10
Current caller: c012bfd0 (nand_base.c:1382)
Current command: 0x80

Chip not ready in nand_command():
Last caller: c012bd44 (nand_base.c:1638)
Last command: 0x10
Current caller: c012b688 (nand_base.c:996)
Current command: 0x00

961     static int nand_do_read_ops(struct mtd_info *mtd, loff_t from,
996     chip->cmdfunc(mtd, NAND_CMD_READ0, 0x00, page);

1375    static int nand_write_page(struct mtd_info *mtd, struct 
nand_chip *chip,
1382    chip->ecc.write_page(mtd, chip, buf);
1393    status = chip->waitfunc(mtd, chip, FL_WRITING);

1582    static int nand_do_write_oob(struct mtd_info *mtd, loff_t to,
1638    status = chip->waitfunc(mtd, chip, FL_WRITING);

1723    static void single_erase_cmd(struct mtd_info *mtd, int page)
1728    chip->cmdfunc(mtd, NAND_CMD_ERASE2, -1, -1);

Re: [PATCH] AT91RM9200 NAND support

[Thomas Gleixner]

On Tue, 2006-06-20 at 15:01 +0200, Savin Zlobec wrote:
> Chip not ready in nand_command():
> Last caller: c012afa8 (nand_base.c:736)
> Last command: 0x70
> Current caller: c012bf8c (nand_base.c:1374)
> Current command: 0x80
> 
> 716     static int nand_wait(struct mtd_info *mtd, struct nand_chip *chip, int state)
> ...
> 733             if ((state == FL_ERASING) && (chip->options & NAND_IS_AND))
> 734                     chip->cmdfunc(mtd, NAND_CMD_STATUS_MULTI, -1, -1);
> 735             else
> 736                     chip->cmdfunc(mtd, NAND_CMD_STATUS, -1, -1);
> 
> 1367    static int nand_write_page(struct mtd_info *mtd, struct nand_chip *chip,
> 1368                               const uint8_t *buf, int page, int cached)
> 1369    {
> 1370            int status;
> 1371
> 1372            chip->cmdfunc(mtd, NAND_CMD_SEQIN, 0x00, page);
> 1373
> 1374            chip->ecc.write_page(mtd, chip, buf);
> 
> Chip not ready in nand_command():
> Last caller: c012afa8 (nand_base.c:736)
> Last command: 0x70
> Current caller: c012b644 (nand_base.c:988)
> Current command: 0x00
> 
> 953     static int nand_do_read_ops(struct mtd_info *mtd, loff_t from,
> ...
> 987                             if (likely(sndcmd)) {
> 988                                     chip->cmdfunc(mtd, NAND_CMD_READ0, 0x00, page);
> 989                                     sndcmd = 0;
> 990                             }

This gets even more mysterious. I both cases the previous function was
nand_wait(), which blocks in the wait function until ready state is
reached. 

I really have no clue, how the chip gets into busy state between the
return from nand_wait() and the next commmand. 

Is there anything playing with the enable pin of the nand chip between
those commands ? Those chips have an autoread feature on power on. Is
the power switched off ?

Have you any other modifications to at91_nand.c I'm not aware of ?

	tglx

Re: [PATCH] AT91RM9200 NAND support

[Savin Zlobec]

Thomas Gleixner wrote:

>This gets even more mysterious. I both cases the previous function was
>nand_wait(), which blocks in the wait function until ready state is
>reached. 
>
>I really have no clue, how the chip gets into busy state between the
>return from nand_wait() and the next commmand. 
>
>Is there anything playing with the enable pin of the nand chip between
>those commands ? Those chips have an autoread feature on power on. Is
>the power switched off ?
>  
>
No.

>Have you any other modifications to at91_nand.c I'm not aware of ?
>  
>
No.

I've pinpointed the location to the command switch at the end of 
nand_command fn:

        switch (command) {

        case NAND_CMD_PAGEPROG:
        case NAND_CMD_ERASE1:
        case NAND_CMD_ERASE2:
        case NAND_CMD_SEQIN:
        case NAND_CMD_STATUS:
                chip->cmd_ctrl(mtd, NAND_CMD_NONE, NAND_NCE);
^^^^^^^^^^^^^
if I wait for ready at this point then I can copy files to jffs2 
partition on my
nand without problems (...well as far as I can tell from a short test...).

savin

Re: [PATCH] AT91RM9200 NAND support

[Thomas Gleixner]

On Tue, 2006-06-20 at 15:48 +0200, Savin Zlobec wrote:
> I've pinpointed the location to the command switch at the end of 
> nand_command fn:
> 
>         switch (command) {
> 
>         case NAND_CMD_PAGEPROG:
>         case NAND_CMD_ERASE1:
>         case NAND_CMD_ERASE2:
>         case NAND_CMD_SEQIN:
>         case NAND_CMD_STATUS:
>                 chip->cmd_ctrl(mtd, NAND_CMD_NONE, NAND_NCE);
> ^^^^^^^^^^^^^
> if I wait for ready at this point then I can copy files to jffs2 
> partition on my
> nand without problems (...well as far as I can tell from a short test...).

Can you try to wait only, when the STATUS command is given ?

I have no idea why this happens. Never seen that before.

	tglx

Re: [PATCH] AT91RM9200 NAND support

[Savin Zlobec]

Thomas Gleixner wrote:

>On Tue, 2006-06-20 at 15:48 +0200, Savin Zlobec wrote:
>  
>
>>I've pinpointed the location to the command switch at the end of 
>>nand_command fn:
>>
>>        switch (command) {
>>
>>        case NAND_CMD_PAGEPROG:
>>        case NAND_CMD_ERASE1:
>>        case NAND_CMD_ERASE2:
>>        case NAND_CMD_SEQIN:
>>        case NAND_CMD_STATUS:
>>                chip->cmd_ctrl(mtd, NAND_CMD_NONE, NAND_NCE);
>>^^^^^^^^^^^^^
>>if I wait for ready at this point then I can copy files to jffs2 
>>partition on my
>>nand without problems (...well as far as I can tell from a short test...).
>>    
>>
>
>Can you try to wait only, when the STATUS command is given ?
>
>I have no idea why this happens. Never seen that before.
>  
>
Indeed it is the STATUS command. If I don't wait after it I can't copy
a single file on the jffs2 partition, inserting a wait there seems to solve
the problem.

Thanks,
    savin

PS:

The MTD in kernel 2.6.17 doesn't have this wait either, but works for
me... different code paths - different timings ? I'll try to find some time
to check this.

Re: [PATCH] AT91RM9200 NAND support

[Thomas Gleixner]

On Tue, 2006-06-20 at 16:24 +0200, Savin Zlobec wrote:
> >I have no idea why this happens. Never seen that before.
> >  
> >
> Indeed it is the STATUS command. If I don't wait after it I can't copy
> a single file on the jffs2 partition, inserting a wait there seems to solve
> the problem.

Well, I really would be interested to find out why this happens. It
might hit more users.

> The MTD in kernel 2.6.17 doesn't have this wait either, but works for
> me... different code paths - different timings ? I'll try to find some time
> to check this.

I guess its a timing problem, which was hidden by slower code. Can you
figure out how long the R/B pin is low in this case ?

	tglx

Re: [PATCH] AT91RM9200 NAND support

[Thomas Gleixner]

On Tue, 2006-06-20 at 16:24 +0200, Savin Zlobec wrote:
> Indeed it is the STATUS command. If I don't wait after it I can't copy
> a single file on the jffs2 partition, inserting a wait there seems to solve
> the problem.

Well, we also delay the codepath in nand_wait() with this. It feels
terribly wrong and the problem is just hidden by that check.

Can you please remove the wait again and add a check of the r/b pin to 

static int nand_wait(struct mtd_info *mtd, struct nand_chip *chip, int state)
{

just before the 

	return status;
}

	tglx

Re: [PATCH] AT91RM9200 NAND support

[Savin Zlobec]

Thomas Gleixner wrote:

>On Tue, 2006-06-20 at 16:24 +0200, Savin Zlobec wrote:
>  
>
>>Indeed it is the STATUS command. If I don't wait after it I can't copy
>>a single file on the jffs2 partition, inserting a wait there seems to solve
>>the problem.
>>    
>>
>
>Well, we also delay the codepath in nand_wait() with this. It feels
>terribly wrong and the problem is just hidden by that check.
>
>Can you please remove the wait again and add a check of the r/b pin to 
>
>static int nand_wait(struct mtd_info *mtd, struct nand_chip *chip, int state)
>{
>
>just before the 
>
>	return status;
>}
>  
>
The problem is in nand_wait, the function is called (when writting files 
to jffs2) with
chip->state=FL_READING and state=FL_WRITING and consequently the waiting
is terminated instantly (chip->state != state) leaving the chip in busy 
state.

savin

Re: [PATCH] AT91RM9200 NAND support

[Thomas Gleixner]

Savin,

On Tue, 2006-06-20 at 17:53 +0200, Savin Zlobec wrote:
> >
> The problem is in nand_wait, the function is called (when writting files 
> to jffs2) with
> chip->state=FL_READING and state=FL_WRITING and consequently the waiting
> is terminated instantly (chip->state != state) leaving the chip in busy 
> state.

The problem is the calling code. Doh - where is the brown paperbag ?

Thanks for tracking that down.

	tglx

diff --git a/drivers/mtd/nand/nand_base.c b/drivers/mtd/nand/nand_base.c
index 77406fc..0fd1052 100644
--- a/drivers/mtd/nand/nand_base.c
+++ b/drivers/mtd/nand/nand_base.c
@@ -501,7 +501,6 @@ static void nand_command(struct mtd_info
 	case NAND_CMD_ERASE2:
 	case NAND_CMD_SEQIN:
 	case NAND_CMD_STATUS:
-		chip->cmd_ctrl(mtd, NAND_CMD_NONE, NAND_NCE);
 		return;
 
 	case NAND_CMD_RESET:
@@ -1532,7 +1531,7 @@ static int nand_write(struct mtd_info *m
 	if (!len)
 		return 0;
 
-	nand_get_device(chip, mtd, FL_READING);
+	nand_get_device(chip, mtd, FL_WRITING);
 
 	chip->ops.len = len;
 	chip->ops.datbuf = (uint8_t *)buf;
@@ -1659,7 +1658,7 @@ static int nand_write_oob(struct mtd_inf
 		return -EINVAL;
 	}
 
-	nand_get_device(chip, mtd, FL_READING);
+	nand_get_device(chip, mtd, FL_WRITING);
 
 	switch(ops->mode) {
 	case MTD_OOB_PLACE:

Re: [PATCH] AT91RM9200 NAND support

[Savin Zlobec]

Thomas Gleixner wrote:

>Savin,
>
>On Tue, 2006-06-20 at 17:53 +0200, Savin Zlobec wrote:
>  
>
>>The problem is in nand_wait, the function is called (when writting files 
>>to jffs2) with
>>chip->state=FL_READING and state=FL_WRITING and consequently the waiting
>>is terminated instantly (chip->state != state) leaving the chip in busy 
>>state.
>>    
>>
>
>The problem is the calling code. Doh - where is the brown paperbag ?
>
>Thanks for tracking that down.
>
>  
>
Just found it too. I guess something was indeed terribly wrong :-).

savin