linux-arm-kernel.lists.infradead.org archive mirror
 help / color / mirror / Atom feed
* [PATCH V3 0/6] pxa3xx_nand update series set
@ 2010-10-22 10:14 Lei Wen
  2010-10-22 10:14 ` [PATCH V3 1/6] pxa3xx_nand: make scan procedure more clear Lei Wen
                   ` (6 more replies)
  0 siblings, 7 replies; 15+ messages in thread
From: Lei Wen @ 2010-10-22 10:14 UTC (permalink / raw)
  To: linux-arm-kernel

[PATCH V3 1/6] pxa3xx_nand: make scan procedure more clear
[PATCH V3 2/6] pxa3xx_nand: rework irq logic 
[PATCH V3 3/6] pxa3xx_nand: discard wait_for_event,write_cmd,__readid function
[PATCH V3 4/6] pxa3xx_nand: unify prepare command
[PATCH V3 5/6] pxa3xx_nand: mtd scan id process could be defined by driver itself
[PATCH V3 6/6] pxa3xx_nand: clean the keep configure code

^ permalink raw reply	[flat|nested] 15+ messages in thread

* [PATCH V3 1/6] pxa3xx_nand: make scan procedure more clear
  2010-10-22 10:14 [PATCH V3 0/6] pxa3xx_nand update series set Lei Wen
@ 2010-10-22 10:14 ` Lei Wen
  2010-10-22 10:14 ` [PATCH V3 2/6] pxa3xx_nand: rework irq logic Lei Wen
                   ` (5 subsequent siblings)
  6 siblings, 0 replies; 15+ messages in thread
From: Lei Wen @ 2010-10-22 10:14 UTC (permalink / raw)
  To: linux-arm-kernel

Signed-off-by: Lei Wen <leiwen@marvell.com>
Signed-off-by: Haojian Zhuang <haojian.zhuang@marvell.com>
Cc: Eric Miao <eric.y.miao@gmail.com>
Cc: David Woodhouse <dwmw2@infradead.org>
---
 drivers/mtd/nand/pxa3xx_nand.c |   98 ++++++++++++++++++++++------------------
 1 files changed, 54 insertions(+), 44 deletions(-)

diff --git a/drivers/mtd/nand/pxa3xx_nand.c b/drivers/mtd/nand/pxa3xx_nand.c
index 17f8518..aff28dc 100644
--- a/drivers/mtd/nand/pxa3xx_nand.c
+++ b/drivers/mtd/nand/pxa3xx_nand.c
@@ -126,6 +126,7 @@ struct pxa3xx_nand_info {
 	unsigned int 		buf_start;
 	unsigned int		buf_count;
 
+	struct mtd_info         *mtd;
 	/* DMA information */
 	int			drcmr_dat;
 	int			drcmr_cmd;
@@ -1044,34 +1045,26 @@ static void pxa3xx_nand_init_mtd(struct mtd_info *mtd,
 	this->chip_delay = 25;
 }
 
-static int pxa3xx_nand_probe(struct platform_device *pdev)
+static struct pxa3xx_nand_info *alloc_nand_resource(struct platform_device *pdev)
 {
-	struct pxa3xx_nand_platform_data *pdata;
+	struct pxa3xx_nand_platform_data *pdata = pdev->dev.platform_data;
 	struct pxa3xx_nand_info *info;
-	struct nand_chip *this;
 	struct mtd_info *mtd;
 	struct resource *r;
-	int ret = 0, irq;
-
-	pdata = pdev->dev.platform_data;
-
-	if (!pdata) {
-		dev_err(&pdev->dev, "no platform data defined\n");
-		return -ENODEV;
-	}
+	int ret, irq;
 
 	mtd = kzalloc(sizeof(struct mtd_info) + sizeof(struct pxa3xx_nand_info),
 			GFP_KERNEL);
 	if (!mtd) {
 		dev_err(&pdev->dev, "failed to allocate memory\n");
-		return -ENOMEM;
+		return NULL;
 	}
 
 	info = (struct pxa3xx_nand_info *)(&mtd[1]);
 	info->pdev = pdev;
 
-	this = &info->nand_chip;
 	mtd->priv = info;
+	info->mtd = mtd;
 	mtd->owner = THIS_MODULE;
 
 	info->clk = clk_get(&pdev->dev, NULL);
@@ -1149,31 +1142,9 @@ static int pxa3xx_nand_probe(struct platform_device *pdev)
 	}
 
 	pxa3xx_nand_init_mtd(mtd, info);
+	platform_set_drvdata(pdev, info);
 
-	platform_set_drvdata(pdev, mtd);
-
-	if (nand_scan(mtd, 1)) {
-		dev_err(&pdev->dev, "failed to scan nand\n");
-		ret = -ENXIO;
-		goto fail_free_irq;
-	}
-
-#ifdef CONFIG_MTD_PARTITIONS
-	if (mtd_has_cmdlinepart()) {
-		static const char *probes[] = { "cmdlinepart", NULL };
-		struct mtd_partition *parts;
-		int nr_parts;
-
-		nr_parts = parse_mtd_partitions(mtd, probes, &parts, 0);
-
-		if (nr_parts)
-			return add_mtd_partitions(mtd, parts, nr_parts);
-	}
-
-	return add_mtd_partitions(mtd, pdata->parts, pdata->nr_parts);
-#else
-	return 0;
-#endif
+	return info;
 
 fail_free_irq:
 	free_irq(irq, info);
@@ -1193,13 +1164,13 @@ fail_put_clk:
 	clk_put(info->clk);
 fail_free_mtd:
 	kfree(mtd);
-	return ret;
+	return NULL;
 }
 
 static int pxa3xx_nand_remove(struct platform_device *pdev)
 {
-	struct mtd_info *mtd = platform_get_drvdata(pdev);
-	struct pxa3xx_nand_info *info = mtd->priv;
+	struct pxa3xx_nand_info *info = platform_get_drvdata(pdev);
+	struct mtd_info *mtd = info->mtd;
 	struct resource *r;
 	int irq;
 
@@ -1230,11 +1201,50 @@ static int pxa3xx_nand_remove(struct platform_device *pdev)
 	return 0;
 }
 
+static int pxa3xx_nand_probe(struct platform_device *pdev)
+{
+	struct pxa3xx_nand_platform_data *pdata;
+	struct pxa3xx_nand_info *info;
+
+	pdata = pdev->dev.platform_data;
+	if (!pdata) {
+		dev_err(&pdev->dev, "no platform data defined\n");
+		return -ENODEV;
+	}
+
+	info = alloc_nand_resource(pdev);
+	if (info == NULL)
+		return -ENOMEM;
+
+	if (nand_scan(info->mtd, 1)) {
+		dev_err(&pdev->dev, "failed to scan nand\n");
+		pxa3xx_nand_remove(pdev);
+		return -ENODEV;
+	}
+
+#ifdef CONFIG_MTD_PARTITIONS
+	if (mtd_has_cmdlinepart()) {
+		static const char *probes[] = { "cmdlinepart", NULL };
+		struct mtd_partition *parts;
+		int nr_parts;
+
+		nr_parts = parse_mtd_partitions(info->mtd, probes, &parts, 0);
+
+		if (nr_parts)
+			return add_mtd_partitions(mtd, parts, nr_parts);
+	}
+
+	return add_mtd_partitions(mtd, pdata->parts, pdata->nr_parts);
+#else
+	return 0;
+#endif
+}
+
 #ifdef CONFIG_PM
 static int pxa3xx_nand_suspend(struct platform_device *pdev, pm_message_t state)
 {
-	struct mtd_info *mtd = (struct mtd_info *)platform_get_drvdata(pdev);
-	struct pxa3xx_nand_info *info = mtd->priv;
+	struct pxa3xx_nand_info *info = platform_get_drvdata(pdev);
+	struct mtd_info *mtd = info->mtd;
 
 	if (info->state != STATE_READY) {
 		dev_err(&pdev->dev, "driver busy, state = %d\n", info->state);
@@ -1246,8 +1256,8 @@ static int pxa3xx_nand_suspend(struct platform_device *pdev, pm_message_t state)
 
 static int pxa3xx_nand_resume(struct platform_device *pdev)
 {
-	struct mtd_info *mtd = (struct mtd_info *)platform_get_drvdata(pdev);
-	struct pxa3xx_nand_info *info = mtd->priv;
+	struct pxa3xx_nand_info *info = platform_get_drvdata(pdev);
+	struct mtd_info *mtd = info->mtd;
 
 	nand_writel(info, NDTR0CS0, info->ndtr0cs0);
 	nand_writel(info, NDTR1CS0, info->ndtr1cs0);
-- 
1.7.0.4

^ permalink raw reply related	[flat|nested] 15+ messages in thread

* [PATCH V3 2/6] pxa3xx_nand: rework irq logic
  2010-10-22 10:14 [PATCH V3 0/6] pxa3xx_nand update series set Lei Wen
  2010-10-22 10:14 ` [PATCH V3 1/6] pxa3xx_nand: make scan procedure more clear Lei Wen
@ 2010-10-22 10:14 ` Lei Wen
  2011-02-13  9:46   ` Eric Miao
  2010-10-22 10:14 ` [PATCH V3 3/6] pxa3xx_nand: discard wait_for_event, write_cmd, __readid function Lei Wen
                   ` (4 subsequent siblings)
  6 siblings, 1 reply; 15+ messages in thread
From: Lei Wen @ 2010-10-22 10:14 UTC (permalink / raw)
  To: linux-arm-kernel

Enable all irq when we start the nand controller, and
put all the transaction logic in the pxa3xx_nand_irq.

By doing this way, we could dramatically increase the
performance by avoid unnecessary delay.

Signed-off-by: Lei Wen <leiwen@marvell.com>
Signed-off-by: Haojian Zhuang <haojian.zhuang@marvell.com>
Cc: Eric Miao <eric.y.miao@gmail.com>
Cc: David Woodhouse <dwmw2@infradead.org>
---
 drivers/mtd/nand/pxa3xx_nand.c |  319 +++++++++++++++++++---------------------
 1 files changed, 153 insertions(+), 166 deletions(-)

diff --git a/drivers/mtd/nand/pxa3xx_nand.c b/drivers/mtd/nand/pxa3xx_nand.c
index aff28dc..dc24000 100644
--- a/drivers/mtd/nand/pxa3xx_nand.c
+++ b/drivers/mtd/nand/pxa3xx_nand.c
@@ -27,6 +27,7 @@
 #include <plat/pxa3xx_nand.h>
 
 #define	CHIP_DELAY_TIMEOUT	(2 * HZ/10)
+#define NAND_STOP_DELAY		(2 * HZ/50)
 
 /* registers and bit definitions */
 #define NDCR		(0x00) /* Control register */
@@ -52,16 +53,18 @@
 #define NDCR_ND_MODE		(0x3 << 21)
 #define NDCR_NAND_MODE   	(0x0)
 #define NDCR_CLR_PG_CNT		(0x1 << 20)
-#define NDCR_CLR_ECC		(0x1 << 19)
+#define NDCR_STOP_ON_UNCOR	(0x1 << 19)
 #define NDCR_RD_ID_CNT_MASK	(0x7 << 16)
 #define NDCR_RD_ID_CNT(x)	(((x) << 16) & NDCR_RD_ID_CNT_MASK)
 
 #define NDCR_RA_START		(0x1 << 15)
 #define NDCR_PG_PER_BLK		(0x1 << 14)
 #define NDCR_ND_ARB_EN		(0x1 << 12)
+#define NDCR_INT_MASK           (0xFFF)
 
 #define NDSR_MASK		(0xfff)
-#define NDSR_RDY		(0x1 << 11)
+#define NDSR_RDY                (0x1 << 12)
+#define NDSR_FLASH_RDY          (0x1 << 11)
 #define NDSR_CS0_PAGED		(0x1 << 10)
 #define NDSR_CS1_PAGED		(0x1 << 9)
 #define NDSR_CS0_CMDD		(0x1 << 8)
@@ -104,13 +107,15 @@ enum {
 };
 
 enum {
-	STATE_READY	= 0,
+	STATE_IDLE = 0,
 	STATE_CMD_HANDLE,
 	STATE_DMA_READING,
 	STATE_DMA_WRITING,
 	STATE_DMA_DONE,
 	STATE_PIO_READING,
 	STATE_PIO_WRITING,
+	STATE_CMD_DONE,
+	STATE_READY,
 };
 
 struct pxa3xx_nand_info {
@@ -292,7 +297,48 @@ static void pxa3xx_set_datasize(struct pxa3xx_nand_info *info)
 	}
 }
 
-static int prepare_read_prog_cmd(struct pxa3xx_nand_info *info,
+/**
+ * NOTE: it is a must to set ND_RUN firstly, then write
+ * command buffer, otherwise, it does not work.
+ * We enable all the interrupt@the same time, and
+ * let pxa3xx_nand_irq to handle all logic.
+ */
+static void pxa3xx_nand_start(struct pxa3xx_nand_info *info)
+{
+	uint32_t ndcr;
+
+	ndcr = info->reg_ndcr;
+	ndcr |= info->use_ecc ? NDCR_ECC_EN : 0;
+	ndcr |= info->use_dma ? NDCR_DMA_EN : 0;
+	ndcr |= NDCR_ND_RUN;
+
+	/* clear status bits and run */
+	nand_writel(info, NDCR, 0);
+	nand_writel(info, NDSR, NDSR_MASK);
+	nand_writel(info, NDCR, ndcr);
+}
+
+static void pxa3xx_nand_stop(struct pxa3xx_nand_info *info)
+{
+	uint32_t ndcr;
+	int timeout = NAND_STOP_DELAY;
+
+	/* wait RUN bit in NDCR become 0 */
+	ndcr = nand_readl(info, NDCR);
+	while ((ndcr & NDCR_ND_RUN) && (timeout -- > 0)) {
+		ndcr = nand_readl(info, NDCR);
+		udelay(1);
+	}
+
+	if (timeout <= 0) {
+		ndcr &= ~NDCR_ND_RUN;
+		nand_writel(info, NDCR, ndcr);
+	}
+	/* clear status bits */
+	nand_writel(info, NDSR, NDSR_MASK);
+}
+
+static void prepare_read_prog_cmd(struct pxa3xx_nand_info *info,
 		uint16_t cmd, int column, int page_addr)
 {
 	const struct pxa3xx_nand_cmdset *cmdset = info->cmdset;
@@ -319,21 +365,18 @@ static int prepare_read_prog_cmd(struct pxa3xx_nand_info *info,
 
 	if (cmd == cmdset->program)
 		info->ndcb0 |= NDCB0_CMD_TYPE(1) | NDCB0_AUTO_RS;
-
-	return 0;
 }
 
-static int prepare_erase_cmd(struct pxa3xx_nand_info *info,
+static void prepare_erase_cmd(struct pxa3xx_nand_info *info,
 			uint16_t cmd, int page_addr)
 {
 	info->ndcb0 = cmd | ((cmd & 0xff00) ? NDCB0_DBC : 0);
 	info->ndcb0 |= NDCB0_CMD_TYPE(2) | NDCB0_AUTO_RS | NDCB0_ADDR_CYC(3);
 	info->ndcb1 = page_addr;
 	info->ndcb2 = 0;
-	return 0;
 }
 
-static int prepare_other_cmd(struct pxa3xx_nand_info *info, uint16_t cmd)
+static void prepare_other_cmd(struct pxa3xx_nand_info *info, uint16_t cmd)
 {
 	const struct pxa3xx_nand_cmdset *cmdset = info->cmdset;
 
@@ -343,7 +386,7 @@ static int prepare_other_cmd(struct pxa3xx_nand_info *info, uint16_t cmd)
 
 	info->oob_size = 0;
 	if (cmd == cmdset->read_id) {
-		info->ndcb0 |= NDCB0_CMD_TYPE(3);
+		info->ndcb0 |= NDCB0_CMD_TYPE(3) | NDCB0_ADDR_CYC(1);
 		info->data_size = 8;
 	} else if (cmd == cmdset->read_status) {
 		info->ndcb0 |= NDCB0_CMD_TYPE(4);
@@ -352,9 +395,7 @@ static int prepare_other_cmd(struct pxa3xx_nand_info *info, uint16_t cmd)
 		   cmd == cmdset->unlock) {
 		info->ndcb0 |= NDCB0_CMD_TYPE(5);
 	} else
-		return -EINVAL;
-
-	return 0;
+		BUG();
 }
 
 static void enable_int(struct pxa3xx_nand_info *info, uint32_t int_mask)
@@ -402,10 +443,8 @@ static int write_cmd(struct pxa3xx_nand_info *info)
 	return 0;
 }
 
-static int handle_data_pio(struct pxa3xx_nand_info *info)
+static void handle_data_pio(struct pxa3xx_nand_info *info)
 {
-	int ret, timeout = CHIP_DELAY_TIMEOUT;
-
 	switch (info->state) {
 	case STATE_PIO_WRITING:
 		__raw_writesl(info->mmio_base + NDDB, info->data_buff,
@@ -413,14 +452,6 @@ static int handle_data_pio(struct pxa3xx_nand_info *info)
 		if (info->oob_size > 0)
 			__raw_writesl(info->mmio_base + NDDB, info->oob_buff,
 					DIV_ROUND_UP(info->oob_size, 4));
-
-		enable_int(info, NDSR_CS0_BBD | NDSR_CS0_CMDD);
-
-		ret = wait_for_completion_timeout(&info->cmd_complete, timeout);
-		if (!ret) {
-			printk(KERN_ERR "program command time out\n");
-			return -1;
-		}
 		break;
 	case STATE_PIO_READING:
 		__raw_readsl(info->mmio_base + NDDB, info->data_buff,
@@ -432,14 +463,11 @@ static int handle_data_pio(struct pxa3xx_nand_info *info)
 	default:
 		printk(KERN_ERR "%s: invalid state %d\n", __func__,
 				info->state);
-		return -EINVAL;
+		BUG();
 	}
-
-	info->state = STATE_READY;
-	return 0;
 }
 
-static void start_data_dma(struct pxa3xx_nand_info *info, int dir_out)
+static void start_data_dma(struct pxa3xx_nand_info *info)
 {
 	struct pxa_dma_desc *desc = info->data_desc;
 	int dma_len = ALIGN(info->data_size + info->oob_size, 32);
@@ -447,14 +475,21 @@ static void start_data_dma(struct pxa3xx_nand_info *info, int dir_out)
 	desc->ddadr = DDADR_STOP;
 	desc->dcmd = DCMD_ENDIRQEN | DCMD_WIDTH4 | DCMD_BURST32 | dma_len;
 
-	if (dir_out) {
+	switch (info->state) {
+	case STATE_DMA_WRITING:
 		desc->dsadr = info->data_buff_phys;
 		desc->dtadr = info->mmio_phys + NDDB;
 		desc->dcmd |= DCMD_INCSRCADDR | DCMD_FLOWTRG;
-	} else {
+		break;
+	case STATE_DMA_READING:
 		desc->dtadr = info->data_buff_phys;
 		desc->dsadr = info->mmio_phys + NDDB;
 		desc->dcmd |= DCMD_INCTRGADDR | DCMD_FLOWSRC;
+		break;
+	default:
+		printk(KERN_ERR "%s: invalid state %d\n", __func__,
+				info->state);
+		BUG();
 	}
 
 	DRCMR(info->drcmr_dat) = DRCMR_MAPVLD | info->data_dma_ch;
@@ -472,93 +507,60 @@ static void pxa3xx_nand_data_dma_irq(int channel, void *data)
 
 	if (dcsr & DCSR_BUSERR) {
 		info->retcode = ERR_DMABUSERR;
-		complete(&info->cmd_complete);
 	}
 
-	if (info->state == STATE_DMA_WRITING) {
-		info->state = STATE_DMA_DONE;
-		enable_int(info, NDSR_CS0_BBD | NDSR_CS0_CMDD);
-	} else {
-		info->state = STATE_READY;
-		complete(&info->cmd_complete);
-	}
+	info->state = STATE_DMA_DONE;
+	enable_int(info, NDCR_INT_MASK);
+	nand_writel(info, NDSR, NDSR_WRDREQ | NDSR_RDDREQ);
 }
 
 static irqreturn_t pxa3xx_nand_irq(int irq, void *devid)
 {
 	struct pxa3xx_nand_info *info = devid;
-	unsigned int status;
+	unsigned int status, is_completed = 0;
 
 	status = nand_readl(info, NDSR);
 
-	if (status & (NDSR_RDDREQ | NDSR_DBERR | NDSR_SBERR)) {
-		if (status & NDSR_DBERR)
-			info->retcode = ERR_DBERR;
-		else if (status & NDSR_SBERR)
-			info->retcode = ERR_SBERR;
-
-		disable_int(info, NDSR_RDDREQ | NDSR_DBERR | NDSR_SBERR);
-
-		if (info->use_dma) {
-			info->state = STATE_DMA_READING;
-			start_data_dma(info, 0);
-		} else {
-			info->state = STATE_PIO_READING;
-			complete(&info->cmd_complete);
-		}
-	} else if (status & NDSR_WRDREQ) {
-		disable_int(info, NDSR_WRDREQ);
+	if (status & NDSR_DBERR)
+		info->retcode = ERR_DBERR;
+	if (status & NDSR_SBERR)
+		info->retcode = ERR_SBERR;
+	if (status & (NDSR_RDDREQ | NDSR_WRDREQ)) {
+		/* whether use dma to transfer data */
 		if (info->use_dma) {
-			info->state = STATE_DMA_WRITING;
-			start_data_dma(info, 1);
+			disable_int(info, NDCR_INT_MASK);
+			info->state = (status & NDSR_RDDREQ) ?
+				      STATE_DMA_READING : STATE_DMA_WRITING;
+			start_data_dma(info);
+			goto NORMAL_IRQ_EXIT;
 		} else {
-			info->state = STATE_PIO_WRITING;
-			complete(&info->cmd_complete);
+			info->state = (status & NDSR_RDDREQ) ?
+				      STATE_PIO_READING : STATE_PIO_WRITING;
+			handle_data_pio(info);
 		}
-	} else if (status & (NDSR_CS0_BBD | NDSR_CS0_CMDD)) {
-		if (status & NDSR_CS0_BBD)
-			info->retcode = ERR_BBERR;
-
-		disable_int(info, NDSR_CS0_BBD | NDSR_CS0_CMDD);
-		info->state = STATE_READY;
-		complete(&info->cmd_complete);
 	}
-	nand_writel(info, NDSR, status);
-	return IRQ_HANDLED;
-}
-
-static int pxa3xx_nand_do_cmd(struct pxa3xx_nand_info *info, uint32_t event)
-{
-	uint32_t ndcr;
-	int ret, timeout = CHIP_DELAY_TIMEOUT;
-
-	if (write_cmd(info)) {
-		info->retcode = ERR_SENDCMD;
-		goto fail_stop;
+	if (status & NDSR_CS0_CMDD) {
+		info->state = STATE_CMD_DONE;
+		is_completed = 1;
 	}
+	if (status & NDSR_FLASH_RDY)
+		info->state = STATE_READY;
 
-	info->state = STATE_CMD_HANDLE;
-
-	enable_int(info, event);
-
-	ret = wait_for_completion_timeout(&info->cmd_complete, timeout);
-	if (!ret) {
-		printk(KERN_ERR "command execution timed out\n");
-		info->retcode = ERR_SENDCMD;
-		goto fail_stop;
+	if (status & NDSR_WRCMDREQ) {
+		nand_writel(info, NDSR, NDSR_WRCMDREQ);
+		status &= ~NDSR_WRCMDREQ;
+		info->state = STATE_CMD_HANDLE;
+		nand_writel(info, NDCB0, info->ndcb0);
+		nand_writel(info, NDCB0, info->ndcb1);
+		nand_writel(info, NDCB0, info->ndcb2);
 	}
 
-	if (info->use_dma == 0 && info->data_size > 0)
-		if (handle_data_pio(info))
-			goto fail_stop;
-
-	return 0;
-
-fail_stop:
-	ndcr = nand_readl(info, NDCR);
-	nand_writel(info, NDCR, ndcr & ~NDCR_ND_RUN);
-	udelay(10);
-	return -ETIMEDOUT;
+	/* clear NDSR to let the controller exit the IRQ */
+	nand_writel(info, NDSR, status);
+	if (is_completed)
+		complete(&info->cmd_complete);
+NORMAL_IRQ_EXIT:
+	return IRQ_HANDLED;
 }
 
 static int pxa3xx_nand_dev_ready(struct mtd_info *mtd)
@@ -580,14 +582,12 @@ static void pxa3xx_nand_cmdfunc(struct mtd_info *mtd, unsigned command,
 {
 	struct pxa3xx_nand_info *info = mtd->priv;
 	const struct pxa3xx_nand_cmdset *cmdset = info->cmdset;
-	int ret;
+	int ret, exec_cmd = 0;
 
 	info->use_dma = (use_dma) ? 1 : 0;
 	info->use_ecc = 0;
 	info->data_size = 0;
-	info->state = STATE_READY;
-
-	init_completion(&info->cmd_complete);
+	info->state = 0;
 
 	switch (command) {
 	case NAND_CMD_READOOB:
@@ -596,14 +596,12 @@ static void pxa3xx_nand_cmdfunc(struct mtd_info *mtd, unsigned command,
 		info->buf_start = mtd->writesize + column;
 		memset(info->data_buff, 0xFF, info->buf_count);
 
-		if (prepare_read_prog_cmd(info, cmdset->read1, column, page_addr))
-			break;
-
-		pxa3xx_nand_do_cmd(info, NDSR_RDDREQ | NDSR_DBERR | NDSR_SBERR);
-
+		prepare_read_prog_cmd(info, cmdset->read1, column, page_addr);
 		/* We only are OOB, so if the data has error, does not matter */
 		if (info->retcode == ERR_DBERR)
 			info->retcode = ERR_NONE;
+
+		exec_cmd = 1;
 		break;
 
 	case NAND_CMD_READ0:
@@ -613,11 +611,7 @@ static void pxa3xx_nand_cmdfunc(struct mtd_info *mtd, unsigned command,
 		info->buf_count = mtd->writesize + mtd->oobsize;
 		memset(info->data_buff, 0xFF, info->buf_count);
 
-		if (prepare_read_prog_cmd(info, cmdset->read1, column, page_addr))
-			break;
-
-		pxa3xx_nand_do_cmd(info, NDSR_RDDREQ | NDSR_DBERR | NDSR_SBERR);
-
+		prepare_read_prog_cmd(info, cmdset->read1, column, page_addr);
 		if (info->retcode == ERR_DBERR) {
 			/* for blank page (all 0xff), HW will calculate its ECC as
 			 * 0, which is different from the ECC information within
@@ -626,6 +620,8 @@ static void pxa3xx_nand_cmdfunc(struct mtd_info *mtd, unsigned command,
 			if (is_buf_blank(info->data_buff, mtd->writesize))
 				info->retcode = ERR_NONE;
 		}
+
+		exec_cmd = 1;
 		break;
 	case NAND_CMD_SEQIN:
 		info->buf_start = column;
@@ -639,17 +635,13 @@ static void pxa3xx_nand_cmdfunc(struct mtd_info *mtd, unsigned command,
 	case NAND_CMD_PAGEPROG:
 		info->use_ecc = (info->seqin_column >= mtd->writesize) ? 0 : 1;
 
-		if (prepare_read_prog_cmd(info, cmdset->program,
-				info->seqin_column, info->seqin_page_addr))
-			break;
-
-		pxa3xx_nand_do_cmd(info, NDSR_WRDREQ);
+		prepare_read_prog_cmd(info, cmdset->program,
+				info->seqin_column, info->seqin_page_addr);
+		exec_cmd = 1;
 		break;
 	case NAND_CMD_ERASE1:
-		if (prepare_erase_cmd(info, cmdset->erase, page_addr))
-			break;
-
-		pxa3xx_nand_do_cmd(info, NDSR_CS0_BBD | NDSR_CS0_CMDD);
+		prepare_erase_cmd(info, cmdset->erase, page_addr);
+		exec_cmd = 1;
 		break;
 	case NAND_CMD_ERASE2:
 		break;
@@ -660,39 +652,35 @@ static void pxa3xx_nand_cmdfunc(struct mtd_info *mtd, unsigned command,
 		info->buf_count = (command == NAND_CMD_READID) ?
 				info->read_id_bytes : 1;
 
-		if (prepare_other_cmd(info, (command == NAND_CMD_READID) ?
-				cmdset->read_id : cmdset->read_status))
-			break;
-
-		pxa3xx_nand_do_cmd(info, NDSR_RDDREQ);
+		prepare_other_cmd(info, (command == NAND_CMD_READID) ?
+				cmdset->read_id : cmdset->read_status);
+		exec_cmd = 1;
 		break;
 	case NAND_CMD_RESET:
-		if (prepare_other_cmd(info, cmdset->reset))
-			break;
-
-		ret = pxa3xx_nand_do_cmd(info, NDSR_CS0_CMDD);
-		if (ret == 0) {
-			int timeout = 2;
-			uint32_t ndcr;
-
-			while (timeout--) {
-				if (nand_readl(info, NDSR) & NDSR_RDY)
-					break;
-				msleep(10);
-			}
-
-			ndcr = nand_readl(info, NDCR);
-			nand_writel(info, NDCR, ndcr & ~NDCR_ND_RUN);
-		}
+		prepare_other_cmd(info, cmdset->reset);
+		exec_cmd = 1;
 		break;
 	default:
 		printk(KERN_ERR "non-supported command.\n");
 		break;
 	}
 
-	if (info->retcode == ERR_DBERR) {
-		printk(KERN_ERR "double bit error @ page %08x\n", page_addr);
-		info->retcode = ERR_NONE;
+	if (exec_cmd) {
+		init_completion(&info->cmd_complete);
+		pxa3xx_nand_start(info);
+
+		ret = wait_for_completion_timeout(&info->cmd_complete,
+				CHIP_DELAY_TIMEOUT);
+		if (!ret) {
+			printk(KERN_ERR "Wait time out!!!\n");
+			/* Stop State Machine for next command cycle */
+			pxa3xx_nand_stop(info);
+		}
+		info->state = STATE_IDLE;
+		if (info->retcode == ERR_DBERR) {
+			printk(KERN_ERR "double bit error @ page %08x\n", page_addr);
+			info->retcode = ERR_NONE;
+		}
 	}
 }
 
@@ -807,10 +795,7 @@ static int __readid(struct pxa3xx_nand_info *info, uint32_t *id)
 	uint32_t ndcr;
 	uint8_t  id_buff[8];
 
-	if (prepare_other_cmd(info, cmdset->read_id)) {
-		printk(KERN_ERR "failed to prepare command\n");
-		return -EINVAL;
-	}
+	prepare_other_cmd(info, cmdset->read_id);
 
 	/* Send command */
 	if (write_cmd(info))
@@ -836,7 +821,7 @@ static int pxa3xx_nand_config_flash(struct pxa3xx_nand_info *info,
 {
 	struct platform_device *pdev = info->pdev;
 	struct pxa3xx_nand_platform_data *pdata = pdev->dev.platform_data;
-	uint32_t ndcr = 0x00000FFF; /* disable all interrupts */
+	uint32_t ndcr = 0x0; /* enable all interrupts */
 
 	if (f->page_size != 2048 && f->page_size != 512)
 		return -EINVAL;
@@ -887,11 +872,12 @@ static int pxa3xx_nand_detect_config(struct pxa3xx_nand_info *info)
 	info->read_id_bytes = (info->page_size == 2048) ? 4 : 2;
 	info->reg_ndcr = ndcr;
 
-	if (__readid(info, &id))
+	pxa3xx_nand_cmdfunc(info->mtd, NAND_CMD_READID, 0, 0);
+	id = *((uint16_t *)(info->data_buff));
+	if (id == 0)
 		return -ENODEV;
 
 	/* Lookup the flash id */
-	id = (id >> 8) & 0xff;		/* device id is byte 2 */
 	for (i = 0; nand_flash_ids[i].name != NULL; i++) {
 		if (id == nand_flash_ids[i].id) {
 			type =  &nand_flash_ids[i];
@@ -935,8 +921,8 @@ static int pxa3xx_nand_detect_flash(struct pxa3xx_nand_info *info,
 	/* we use default timing to detect id */
 	f = DEFAULT_FLASH_TYPE;
 	pxa3xx_nand_config_flash(info, f);
-	if (__readid(info, &id))
-		goto fail_detect;
+	pxa3xx_nand_cmdfunc(info->mtd, NAND_CMD_READID, 0, 0);
+	id = *((uint16_t *)(info->data_buff));
 
 	for (i=0; i<ARRAY_SIZE(builtin_flash_types) + pdata->num_flash - 1; i++) {
 		/* we first choose the flash definition from platfrom */
@@ -954,7 +940,6 @@ static int pxa3xx_nand_detect_flash(struct pxa3xx_nand_info *info,
 	dev_warn(&info->pdev->dev,
 		 "failed to detect configured nand flash; found %04x instead of\n",
 		 id);
-fail_detect:
 	return -ENODEV;
 }
 
@@ -1176,10 +1161,6 @@ static int pxa3xx_nand_remove(struct platform_device *pdev)
 
 	platform_set_drvdata(pdev, NULL);
 
-	del_mtd_device(mtd);
-#ifdef CONFIG_MTD_PARTITIONS
-	del_mtd_partitions(mtd);
-#endif
 	irq = platform_get_irq(pdev, 0);
 	if (irq >= 0)
 		free_irq(irq, info);
@@ -1197,7 +1178,13 @@ static int pxa3xx_nand_remove(struct platform_device *pdev)
 	clk_disable(info->clk);
 	clk_put(info->clk);
 
-	kfree(mtd);
+	if (mtd) {
+		del_mtd_device(mtd);
+#ifdef CONFIG_MTD_PARTITIONS
+		del_mtd_partitions(mtd);
+#endif
+		kfree(mtd);
+	}
 	return 0;
 }
 
@@ -1231,10 +1218,10 @@ static int pxa3xx_nand_probe(struct platform_device *pdev)
 		nr_parts = parse_mtd_partitions(info->mtd, probes, &parts, 0);
 
 		if (nr_parts)
-			return add_mtd_partitions(mtd, parts, nr_parts);
+			return add_mtd_partitions(info->mtd, parts, nr_parts);
 	}
 
-	return add_mtd_partitions(mtd, pdata->parts, pdata->nr_parts);
+	return add_mtd_partitions(info->mtd, pdata->parts, pdata->nr_parts);
 #else
 	return 0;
 #endif
@@ -1246,7 +1233,7 @@ static int pxa3xx_nand_suspend(struct platform_device *pdev, pm_message_t state)
 	struct pxa3xx_nand_info *info = platform_get_drvdata(pdev);
 	struct mtd_info *mtd = info->mtd;
 
-	if (info->state != STATE_READY) {
+	if (info->state) {
 		dev_err(&pdev->dev, "driver busy, state = %d\n", info->state);
 		return -EAGAIN;
 	}
-- 
1.7.0.4

^ permalink raw reply related	[flat|nested] 15+ messages in thread

* [PATCH V3 3/6] pxa3xx_nand: discard wait_for_event, write_cmd, __readid function
  2010-10-22 10:14 [PATCH V3 0/6] pxa3xx_nand update series set Lei Wen
  2010-10-22 10:14 ` [PATCH V3 1/6] pxa3xx_nand: make scan procedure more clear Lei Wen
  2010-10-22 10:14 ` [PATCH V3 2/6] pxa3xx_nand: rework irq logic Lei Wen
@ 2010-10-22 10:14 ` Lei Wen
  2011-02-13  9:47   ` Eric Miao
  2010-10-22 10:14 ` [PATCH V3 4/6] pxa3xx_nand: unify prepare command Lei Wen
                   ` (3 subsequent siblings)
  6 siblings, 1 reply; 15+ messages in thread
From: Lei Wen @ 2010-10-22 10:14 UTC (permalink / raw)
  To: linux-arm-kernel

Since we have rework the irq process, we don't need additional
delay in wait_for_event. Also write_cmd and __readid is also
discarded for the same reason.

Signed-off-by: Lei Wen <leiwen@marvell.com>
Signed-off-by: Haojian Zhuang <haojian.zhuang@marvell.com>
Cc: Eric Miao <eric.y.miao@gmail.com>
Cc: David Woodhouse <dwmw2@infradead.org>
---
 drivers/mtd/nand/pxa3xx_nand.c |   77 +---------------------------------------
 1 files changed, 1 insertions(+), 76 deletions(-)

diff --git a/drivers/mtd/nand/pxa3xx_nand.c b/drivers/mtd/nand/pxa3xx_nand.c
index dc24000..a7f623f 100644
--- a/drivers/mtd/nand/pxa3xx_nand.c
+++ b/drivers/mtd/nand/pxa3xx_nand.c
@@ -258,25 +258,6 @@ static void pxa3xx_nand_set_timing(struct pxa3xx_nand_info *info,
 	nand_writel(info, NDTR1CS0, ndtr1);
 }
 
-#define WAIT_EVENT_TIMEOUT	10
-
-static int wait_for_event(struct pxa3xx_nand_info *info, uint32_t event)
-{
-	int timeout = WAIT_EVENT_TIMEOUT;
-	uint32_t ndsr;
-
-	while (timeout--) {
-		ndsr = nand_readl(info, NDSR) & NDSR_MASK;
-		if (ndsr & event) {
-			nand_writel(info, NDSR, ndsr);
-			return 0;
-		}
-		udelay(10);
-	}
-
-	return -ETIMEDOUT;
-}
-
 static void pxa3xx_set_datasize(struct pxa3xx_nand_info *info)
 {
 	int oob_enable = info->reg_ndcr & NDCR_SPARE_EN;
@@ -414,35 +395,6 @@ static void disable_int(struct pxa3xx_nand_info *info, uint32_t int_mask)
 	nand_writel(info, NDCR, ndcr | int_mask);
 }
 
-/* NOTE: it is a must to set ND_RUN firstly, then write command buffer
- * otherwise, it does not work
- */
-static int write_cmd(struct pxa3xx_nand_info *info)
-{
-	uint32_t ndcr;
-
-	/* clear status bits and run */
-	nand_writel(info, NDSR, NDSR_MASK);
-
-	ndcr = info->reg_ndcr;
-
-	ndcr |= info->use_ecc ? NDCR_ECC_EN : 0;
-	ndcr |= info->use_dma ? NDCR_DMA_EN : 0;
-	ndcr |= NDCR_ND_RUN;
-
-	nand_writel(info, NDCR, ndcr);
-
-	if (wait_for_event(info, NDSR_WRCMDREQ)) {
-		printk(KERN_ERR "timed out writing command\n");
-		return -ETIMEDOUT;
-	}
-
-	nand_writel(info, NDCB0, info->ndcb0);
-	nand_writel(info, NDCB0, info->ndcb1);
-	nand_writel(info, NDCB0, info->ndcb2);
-	return 0;
-}
-
 static void handle_data_pio(struct pxa3xx_nand_info *info)
 {
 	switch (info->state) {
@@ -789,33 +741,6 @@ static int pxa3xx_nand_ecc_correct(struct mtd_info *mtd,
 	return 0;
 }
 
-static int __readid(struct pxa3xx_nand_info *info, uint32_t *id)
-{
-	const struct pxa3xx_nand_cmdset *cmdset = info->cmdset;
-	uint32_t ndcr;
-	uint8_t  id_buff[8];
-
-	prepare_other_cmd(info, cmdset->read_id);
-
-	/* Send command */
-	if (write_cmd(info))
-		goto fail_timeout;
-
-	/* Wait for CMDDM(command done successfully) */
-	if (wait_for_event(info, NDSR_RDDREQ))
-		goto fail_timeout;
-
-	__raw_readsl(info->mmio_base + NDDB, id_buff, 2);
-	*id = id_buff[0] | (id_buff[1] << 8);
-	return 0;
-
-fail_timeout:
-	ndcr = nand_readl(info, NDCR);
-	nand_writel(info, NDCR, ndcr & ~NDCR_ND_RUN);
-	udelay(10);
-	return -ETIMEDOUT;
-}
-
 static int pxa3xx_nand_config_flash(struct pxa3xx_nand_info *info,
 				    const struct pxa3xx_nand_flash *f)
 {
@@ -868,7 +793,7 @@ static int pxa3xx_nand_detect_config(struct pxa3xx_nand_info *info)
 
 	page_per_block = ndcr & NDCR_PG_PER_BLK ? 64 : 32;
 	info->page_size = ndcr & NDCR_PAGE_SZ ? 2048 : 512;
-	/* set info fields needed to __readid */
+	/* set info fields needed to read id */
 	info->read_id_bytes = (info->page_size == 2048) ? 4 : 2;
 	info->reg_ndcr = ndcr;
 
-- 
1.7.0.4

^ permalink raw reply related	[flat|nested] 15+ messages in thread

* [PATCH V3 4/6] pxa3xx_nand: unify prepare command
  2010-10-22 10:14 [PATCH V3 0/6] pxa3xx_nand update series set Lei Wen
                   ` (2 preceding siblings ...)
  2010-10-22 10:14 ` [PATCH V3 3/6] pxa3xx_nand: discard wait_for_event, write_cmd, __readid function Lei Wen
@ 2010-10-22 10:14 ` Lei Wen
  2010-10-22 10:14 ` [PATCH V3 5/6] pxa3xx_nand: mtd scan id process could be defined by driver itself Lei Wen
                   ` (2 subsequent siblings)
  6 siblings, 0 replies; 15+ messages in thread
From: Lei Wen @ 2010-10-22 10:14 UTC (permalink / raw)
  To: linux-arm-kernel

Make the interface simpler which could make both debug
and enhancement easier.

Signed-off-by: Lei Wen <leiwen@marvell.com>
Signed-off-by: Haojian Zhuang <haojian.zhuang@marvell.com>
Cc: Eric Miao <eric.y.miao@gmail.com>
Cc: David Woodhouse <dwmw2@infradead.org>
---
 drivers/mtd/nand/pxa3xx_nand.c |  260 +++++++++++++++++++++-------------------
 1 files changed, 134 insertions(+), 126 deletions(-)

diff --git a/drivers/mtd/nand/pxa3xx_nand.c b/drivers/mtd/nand/pxa3xx_nand.c
index a7f623f..0ca2b88 100644
--- a/drivers/mtd/nand/pxa3xx_nand.c
+++ b/drivers/mtd/nand/pxa3xx_nand.c
@@ -28,6 +28,7 @@
 
 #define	CHIP_DELAY_TIMEOUT	(2 * HZ/10)
 #define NAND_STOP_DELAY		(2 * HZ/50)
+#define PAGE_CHUNK_SIZE		(2048)
 
 /* registers and bit definitions */
 #define NDCR		(0x00) /* Control register */
@@ -77,6 +78,7 @@
 #define NDSR_RDDREQ		(0x1 << 1)
 #define NDSR_WRCMDREQ		(0x1)
 
+#define NDCB0_ST_ROW_EN         (0x1 << 26)
 #define NDCB0_AUTO_RS		(0x1 << 25)
 #define NDCB0_CSEL		(0x1 << 24)
 #define NDCB0_CMD_TYPE_MASK	(0x7 << 21)
@@ -319,66 +321,6 @@ static void pxa3xx_nand_stop(struct pxa3xx_nand_info *info)
 	nand_writel(info, NDSR, NDSR_MASK);
 }
 
-static void prepare_read_prog_cmd(struct pxa3xx_nand_info *info,
-		uint16_t cmd, int column, int page_addr)
-{
-	const struct pxa3xx_nand_cmdset *cmdset = info->cmdset;
-	pxa3xx_set_datasize(info);
-
-	/* generate values for NDCBx registers */
-	info->ndcb0 = cmd | ((cmd & 0xff00) ? NDCB0_DBC : 0);
-	info->ndcb1 = 0;
-	info->ndcb2 = 0;
-	info->ndcb0 |= NDCB0_ADDR_CYC(info->row_addr_cycles + info->col_addr_cycles);
-
-	if (info->col_addr_cycles == 2) {
-		/* large block, 2 cycles for column address
-		 * row address starts from 3rd cycle
-		 */
-		info->ndcb1 |= page_addr << 16;
-		if (info->row_addr_cycles == 3)
-			info->ndcb2 = (page_addr >> 16) & 0xff;
-	} else
-		/* small block, 1 cycles for column address
-		 * row address starts from 2nd cycle
-		 */
-		info->ndcb1 = page_addr << 8;
-
-	if (cmd == cmdset->program)
-		info->ndcb0 |= NDCB0_CMD_TYPE(1) | NDCB0_AUTO_RS;
-}
-
-static void prepare_erase_cmd(struct pxa3xx_nand_info *info,
-			uint16_t cmd, int page_addr)
-{
-	info->ndcb0 = cmd | ((cmd & 0xff00) ? NDCB0_DBC : 0);
-	info->ndcb0 |= NDCB0_CMD_TYPE(2) | NDCB0_AUTO_RS | NDCB0_ADDR_CYC(3);
-	info->ndcb1 = page_addr;
-	info->ndcb2 = 0;
-}
-
-static void prepare_other_cmd(struct pxa3xx_nand_info *info, uint16_t cmd)
-{
-	const struct pxa3xx_nand_cmdset *cmdset = info->cmdset;
-
-	info->ndcb0 = cmd | ((cmd & 0xff00) ? NDCB0_DBC : 0);
-	info->ndcb1 = 0;
-	info->ndcb2 = 0;
-
-	info->oob_size = 0;
-	if (cmd == cmdset->read_id) {
-		info->ndcb0 |= NDCB0_CMD_TYPE(3) | NDCB0_ADDR_CYC(1);
-		info->data_size = 8;
-	} else if (cmd == cmdset->read_status) {
-		info->ndcb0 |= NDCB0_CMD_TYPE(4);
-		info->data_size = 8;
-	} else if (cmd == cmdset->reset || cmd == cmdset->lock ||
-		   cmd == cmdset->unlock) {
-		info->ndcb0 |= NDCB0_CMD_TYPE(5);
-	} else
-		BUG();
-}
-
 static void enable_int(struct pxa3xx_nand_info *info, uint32_t int_mask)
 {
 	uint32_t ndcr;
@@ -529,94 +471,164 @@ static inline int is_buf_blank(uint8_t *buf, size_t len)
 	return 1;
 }
 
-static void pxa3xx_nand_cmdfunc(struct mtd_info *mtd, unsigned command,
-				int column, int page_addr)
+static int prepare_command_pool(struct pxa3xx_nand_info *info, int command,
+		uint16_t column, int page_addr)
 {
-	struct pxa3xx_nand_info *info = mtd->priv;
-	const struct pxa3xx_nand_cmdset *cmdset = info->cmdset;
-	int ret, exec_cmd = 0;
+	uint16_t cmd;
+	int addr_cycle, exec_cmd, ndcb0;
+	struct mtd_info *mtd = info->mtd;
+
+	ndcb0 = 0;
+	addr_cycle = 0;
+	exec_cmd = 1;
 
-	info->use_dma = (use_dma) ? 1 : 0;
-	info->use_ecc = 0;
-	info->data_size = 0;
-	info->state = 0;
+	/* reset data and oob column point to handle data */
+	info->buf_start		= 0;
+	info->buf_count		= 0;
+	info->oob_size		= 0;
+	info->use_ecc		= 0;
 
 	switch (command) {
+	case NAND_CMD_READ0:
+	case NAND_CMD_PAGEPROG:
+		info->use_ecc = 1;
 	case NAND_CMD_READOOB:
-		/* disable HW ECC to get all the OOB data */
-		info->buf_count = mtd->writesize + mtd->oobsize;
-		info->buf_start = mtd->writesize + column;
-		memset(info->data_buff, 0xFF, info->buf_count);
-
-		prepare_read_prog_cmd(info, cmdset->read1, column, page_addr);
-		/* We only are OOB, so if the data has error, does not matter */
-		if (info->retcode == ERR_DBERR)
-			info->retcode = ERR_NONE;
-
-		exec_cmd = 1;
+		pxa3xx_set_datasize(info);
+		break;
+	case NAND_CMD_SEQIN:
+		exec_cmd = 0;
+		break;
+	default:
+		info->ndcb1 = 0;
+		info->ndcb2 = 0;
 		break;
+	}
+
+	info->ndcb0 = ndcb0;
+	addr_cycle = NDCB0_ADDR_CYC(info->row_addr_cycles
+		     + info->col_addr_cycles);
 
+	switch (command) {
+	case NAND_CMD_READOOB:
 	case NAND_CMD_READ0:
-		info->use_ecc = 1;
-		info->retcode = ERR_NONE;
-		info->buf_start = column;
-		info->buf_count = mtd->writesize + mtd->oobsize;
-		memset(info->data_buff, 0xFF, info->buf_count);
+		cmd  = info->cmdset->read1;
+		if (command == NAND_CMD_READOOB)
+			info->buf_start = mtd->writesize + column;
+		else
+			info->buf_start = column;
 
-		prepare_read_prog_cmd(info, cmdset->read1, column, page_addr);
-		if (info->retcode == ERR_DBERR) {
-			/* for blank page (all 0xff), HW will calculate its ECC as
-			 * 0, which is different from the ECC information within
-			 * OOB, ignore such double bit errors
-			 */
-			if (is_buf_blank(info->data_buff, mtd->writesize))
-				info->retcode = ERR_NONE;
-		}
+		if (unlikely(info->page_size < PAGE_CHUNK_SIZE))
+			info->ndcb0 |= NDCB0_CMD_TYPE(0)
+					| addr_cycle
+					| (cmd & NDCB0_CMD1_MASK);
+		else
+			info->ndcb0 |= NDCB0_CMD_TYPE(0)
+					| NDCB0_DBC
+					| addr_cycle
+					| cmd;
 
-		exec_cmd = 1;
-		break;
 	case NAND_CMD_SEQIN:
-		info->buf_start = column;
+		/* small page addr setting */
+		if (unlikely(info->page_size < PAGE_CHUNK_SIZE)) {
+			info->ndcb1 = ((page_addr & 0xFFFFFF) << 8)
+					| (column & 0xFF);
+
+			info->ndcb2 = 0;
+		}
+		else {
+			info->ndcb1 = ((page_addr & 0xFFFF) << 16)
+					| (column & 0xFFFF);
+
+			if (page_addr & 0xFF0000)
+				info->ndcb2 = (page_addr & 0xFF0000) >> 16;
+			else
+				info->ndcb2 = 0;
+		}
+
 		info->buf_count = mtd->writesize + mtd->oobsize;
-		memset(info->data_buff, 0xff, info->buf_count);
+		memset(info->data_buff, 0xFF, info->buf_count);
 
-		/* save column/page_addr for next CMD_PAGEPROG */
-		info->seqin_column = column;
-		info->seqin_page_addr = page_addr;
 		break;
+
 	case NAND_CMD_PAGEPROG:
-		info->use_ecc = (info->seqin_column >= mtd->writesize) ? 0 : 1;
+		if (is_buf_blank(info->data_buff, (mtd->writesize + mtd->oobsize))) {
+			exec_cmd = 0;
+			break;
+		}
 
-		prepare_read_prog_cmd(info, cmdset->program,
-				info->seqin_column, info->seqin_page_addr);
-		exec_cmd = 1;
-		break;
-	case NAND_CMD_ERASE1:
-		prepare_erase_cmd(info, cmdset->erase, page_addr);
-		exec_cmd = 1;
-		break;
-	case NAND_CMD_ERASE2:
+		cmd = info->cmdset->program;
+		info->ndcb0 |= NDCB0_CMD_TYPE(0x1)
+				| NDCB0_AUTO_RS
+				| NDCB0_ST_ROW_EN
+				| NDCB0_DBC
+				| cmd
+				| addr_cycle;
 		break;
+
 	case NAND_CMD_READID:
+		cmd = info->cmdset->read_id;
+		info->buf_count = info->read_id_bytes;
+		info->ndcb0 |= NDCB0_CMD_TYPE(3)
+				| NDCB0_ADDR_CYC(1)
+				| cmd;
+
+		info->data_size = 8;
+		break;
 	case NAND_CMD_STATUS:
-		info->use_dma = 0;	/* force PIO read */
-		info->buf_start = 0;
-		info->buf_count = (command == NAND_CMD_READID) ?
-				info->read_id_bytes : 1;
-
-		prepare_other_cmd(info, (command == NAND_CMD_READID) ?
-				cmdset->read_id : cmdset->read_status);
-		exec_cmd = 1;
+		cmd = info->cmdset->read_status;
+		info->buf_count = 1;
+		info->ndcb0 |= NDCB0_CMD_TYPE(4)
+				| NDCB0_ADDR_CYC(1)
+				| cmd;
+
+		info->data_size = 8;
+		break;
+
+	case NAND_CMD_ERASE1:
+		cmd = info->cmdset->erase;
+		info->ndcb0 |= NDCB0_CMD_TYPE(2)
+				| NDCB0_AUTO_RS
+				| NDCB0_ADDR_CYC(3)
+				| NDCB0_DBC
+				| cmd;
+		info->ndcb1 = page_addr;
+		info->ndcb2 = 0;
+
 		break;
 	case NAND_CMD_RESET:
-		prepare_other_cmd(info, cmdset->reset);
-		exec_cmd = 1;
+		cmd = info->cmdset->reset;
+		info->ndcb0 |= NDCB0_CMD_TYPE(5)
+				| cmd;
+
+		break;
+
+	case NAND_CMD_ERASE2:
+		exec_cmd = 0;
 		break;
+
 	default:
-		printk(KERN_ERR "non-supported command.\n");
+		exec_cmd = 0;
+		printk(KERN_ERR "pxa3xx-nand: non-supported command %x\n", command);
 		break;
 	}
 
+	return exec_cmd;
+}
+
+static void pxa3xx_nand_cmdfunc(struct mtd_info *mtd, unsigned command,
+		int column, int page_addr)
+{
+	struct pxa3xx_nand_info *info = mtd->priv;
+	int ret, exec_cmd;
+
+	/* if this is a x16 device ,then convert the input
+	 * "byte" address into a "word" address appropriate
+	 * for indexing a word-oriented device
+	 */
+	if (info->reg_ndcr & NDCR_DWIDTH_M)
+		column /= 2;
+
+	exec_cmd = prepare_command_pool(info, command, column, page_addr);
 	if (exec_cmd) {
 		init_completion(&info->cmd_complete);
 		pxa3xx_nand_start(info);
@@ -629,10 +641,6 @@ static void pxa3xx_nand_cmdfunc(struct mtd_info *mtd, unsigned command,
 			pxa3xx_nand_stop(info);
 		}
 		info->state = STATE_IDLE;
-		if (info->retcode == ERR_DBERR) {
-			printk(KERN_ERR "double bit error @ page %08x\n", page_addr);
-			info->retcode = ERR_NONE;
-		}
 	}
 }
 
-- 
1.7.0.4

^ permalink raw reply related	[flat|nested] 15+ messages in thread

* [PATCH V3 5/6] pxa3xx_nand: mtd scan id process could be defined by driver itself
  2010-10-22 10:14 [PATCH V3 0/6] pxa3xx_nand update series set Lei Wen
                   ` (3 preceding siblings ...)
  2010-10-22 10:14 ` [PATCH V3 4/6] pxa3xx_nand: unify prepare command Lei Wen
@ 2010-10-22 10:14 ` Lei Wen
  2010-10-22 10:14 ` [PATCH V3 6/6] pxa3xx_nand: clean the keep configure code Lei Wen
  2010-10-22 10:15 ` [PATCH V3 0/6] pxa3xx_nand update series set Haojian Zhuang
  6 siblings, 0 replies; 15+ messages in thread
From: Lei Wen @ 2010-10-22 10:14 UTC (permalink / raw)
  To: linux-arm-kernel

Different NAND driver may require its unique detection. For pxa3xx_nand,
it use its self id database to get the necessary info.

Signed-off-by: Lei Wen <leiwen@marvell.com>
Signed-off-by: Haojian Zhuang <haojian.zhuang@marvell.com>
Cc: Eric Miao <eric.y.miao@gmail.com>
Cc: David Woodhouse <dwmw2@infradead.org>
---
 drivers/mtd/nand/pxa3xx_nand.c |  258 ++++++++++++++++++++++++++--------------
 1 files changed, 168 insertions(+), 90 deletions(-)

diff --git a/drivers/mtd/nand/pxa3xx_nand.c b/drivers/mtd/nand/pxa3xx_nand.c
index 0ca2b88..b5dc5d7 100644
--- a/drivers/mtd/nand/pxa3xx_nand.c
+++ b/drivers/mtd/nand/pxa3xx_nand.c
@@ -123,6 +123,7 @@ enum {
 struct pxa3xx_nand_info {
 	struct nand_chip	nand_chip;
 
+	struct nand_hw_control	controller;
 	struct platform_device	 *pdev;
 	struct pxa3xx_nand_cmdset *cmdset;
 
@@ -157,6 +158,7 @@ struct pxa3xx_nand_info {
 
 	int			use_ecc;	/* use HW ECC ? */
 	int			use_dma;	/* use DMA ? */
+	int			is_ready;
 
 	unsigned int		page_size;	/* page size of attached chip */
 	unsigned int		data_size;	/* data size in FIFO */
@@ -223,6 +225,8 @@ static struct pxa3xx_nand_flash builtin_flash_types[] = {
 /* Define a default flash type setting serve as flash detecting only */
 #define DEFAULT_FLASH_TYPE (&builtin_flash_types[0])
 
+static const char *mtd_names[] = {"pxa3xx_nand-0", NULL};
+
 #define NDTR0_tCH(c)	(min((c), 7) << 19)
 #define NDTR0_tCS(c)	(min((c), 7) << 16)
 #define NDTR0_tWH(c)	(min((c), 7) << 11)
@@ -437,8 +441,10 @@ static irqreturn_t pxa3xx_nand_irq(int irq, void *devid)
 		info->state = STATE_CMD_DONE;
 		is_completed = 1;
 	}
-	if (status & NDSR_FLASH_RDY)
+	if (status & NDSR_FLASH_RDY) {
+		info->is_ready = 1;
 		info->state = STATE_READY;
+	}
 
 	if (status & NDSR_WRCMDREQ) {
 		nand_writel(info, NDSR, NDSR_WRCMDREQ);
@@ -487,6 +493,7 @@ static int prepare_command_pool(struct pxa3xx_nand_info *info, int command,
 	info->buf_count		= 0;
 	info->oob_size		= 0;
 	info->use_ecc		= 0;
+	info->is_ready		= 0;
 
 	switch (command) {
 	case NAND_CMD_READ0:
@@ -840,42 +847,6 @@ static int pxa3xx_nand_detect_config(struct pxa3xx_nand_info *info)
 	return 0;
 }
 
-static int pxa3xx_nand_detect_flash(struct pxa3xx_nand_info *info,
-				    const struct pxa3xx_nand_platform_data *pdata)
-{
-	const struct pxa3xx_nand_flash *f;
-	uint32_t id = -1;
-	int i;
-
-	if (pdata->keep_config)
-		if (pxa3xx_nand_detect_config(info) == 0)
-			return 0;
-
-	/* we use default timing to detect id */
-	f = DEFAULT_FLASH_TYPE;
-	pxa3xx_nand_config_flash(info, f);
-	pxa3xx_nand_cmdfunc(info->mtd, NAND_CMD_READID, 0, 0);
-	id = *((uint16_t *)(info->data_buff));
-
-	for (i=0; i<ARRAY_SIZE(builtin_flash_types) + pdata->num_flash - 1; i++) {
-		/* we first choose the flash definition from platfrom */
-		if (i < pdata->num_flash)
-			f = pdata->flash + i;
-		else
-			f = &builtin_flash_types[i - pdata->num_flash + 1];
-		if (f->chip_id == id) {
-			dev_info(&info->pdev->dev, "detect chip id: 0x%x\n", id);
-			pxa3xx_nand_config_flash(info, f);
-			return 0;
-		}
-	}
-
-	dev_warn(&info->pdev->dev,
-		 "failed to detect configured nand flash; found %04x instead of\n",
-		 id);
-	return -ENODEV;
-}
-
 /* the maximum possible buffer size for large page with OOB data
  * is: 2048 + 64 = 2112 bytes, allocate a page here for both the
  * data buffer and the DMA descriptor
@@ -917,56 +888,152 @@ static int pxa3xx_nand_init_buff(struct pxa3xx_nand_info *info)
 	return 0;
 }
 
-static struct nand_ecclayout hw_smallpage_ecclayout = {
-	.eccbytes = 6,
-	.eccpos = {8, 9, 10, 11, 12, 13 },
-	.oobfree = { {2, 6} }
-};
+static int pxa3xx_nand_read_page_hwecc(struct mtd_info *mtd,
+		struct nand_chip *chip, uint8_t *buf, int page)
+{
+	struct pxa3xx_nand_info *info = mtd->priv;
 
-static struct nand_ecclayout hw_largepage_ecclayout = {
-	.eccbytes = 24,
-	.eccpos = {
-		40, 41, 42, 43, 44, 45, 46, 47,
-		48, 49, 50, 51, 52, 53, 54, 55,
-		56, 57, 58, 59, 60, 61, 62, 63},
-	.oobfree = { {2, 38} }
-};
+	chip->read_buf(mtd, buf, mtd->writesize);
+	chip->read_buf(mtd, chip->oob_poi, mtd->oobsize);
+
+	if (info->retcode == ERR_SBERR) {
+		switch (info->use_ecc) {
+			case 1:
+				mtd->ecc_stats.corrected ++;
+				break;
+
+			case 0:
+			default:
+				break;
+		}
+	}
+	else if (info->retcode == ERR_DBERR) {
+		int buf_blank;
+
+		buf_blank = is_buf_blank(buf, mtd->writesize);
+		if (!buf_blank)
+			mtd->ecc_stats.failed++;
+	}
+
+	return 0;
+}
+
+static void pxa3xx_nand_write_page_hwecc(struct mtd_info *mtd,
+		struct nand_chip *chip, const uint8_t *buf)
+{
+	chip->write_buf(mtd, buf, mtd->writesize);
+	chip->write_buf(mtd, chip->oob_poi, mtd->oobsize);
+}
+
+static void pxa3xx_nand_erase_cmd(struct mtd_info *mtd, int page)
+{
+	struct nand_chip *chip = mtd->priv;
+	/* Send commands to erase a block */
+	chip->cmdfunc(mtd, NAND_CMD_ERASE1, -1, page);
+}
 
-static void pxa3xx_nand_init_mtd(struct mtd_info *mtd,
-				 struct pxa3xx_nand_info *info)
+static int pxa3xx_nand_sensing(struct pxa3xx_nand_info *info)
 {
-	struct nand_chip *this = &info->nand_chip;
-
-	this->options = (info->reg_ndcr & NDCR_DWIDTH_C) ? NAND_BUSWIDTH_16: 0;
-
-	this->waitfunc		= pxa3xx_nand_waitfunc;
-	this->select_chip	= pxa3xx_nand_select_chip;
-	this->dev_ready		= pxa3xx_nand_dev_ready;
-	this->cmdfunc		= pxa3xx_nand_cmdfunc;
-	this->read_word		= pxa3xx_nand_read_word;
-	this->read_byte		= pxa3xx_nand_read_byte;
-	this->read_buf		= pxa3xx_nand_read_buf;
-	this->write_buf		= pxa3xx_nand_write_buf;
-	this->verify_buf	= pxa3xx_nand_verify_buf;
-
-	this->ecc.mode		= NAND_ECC_HW;
-	this->ecc.hwctl		= pxa3xx_nand_ecc_hwctl;
-	this->ecc.calculate	= pxa3xx_nand_ecc_calculate;
-	this->ecc.correct	= pxa3xx_nand_ecc_correct;
-	this->ecc.size		= info->page_size;
-
-	if (info->page_size == 2048)
-		this->ecc.layout = &hw_largepage_ecclayout;
+	struct mtd_info *mtd = info->mtd;
+	struct nand_chip *chip = mtd->priv;
+
+	/* use the common timing to make a try */
+	pxa3xx_nand_config_flash(info, &builtin_flash_types[0]);
+	chip->cmdfunc(mtd, NAND_CMD_RESET, 0, 0);
+	if (info->is_ready)
+		return 1;
 	else
-		this->ecc.layout = &hw_smallpage_ecclayout;
+		return 0;
+}
 
-	this->chip_delay = 25;
+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;
+	const struct pxa3xx_nand_flash *f = NULL;
+	struct nand_chip *chip = mtd->priv;
+	uint32_t id = -1;
+	int i, ret;
+
+	if (pdata->keep_config && !pxa3xx_nand_detect_config(info))
+		return 0;
+
+	ret = pxa3xx_nand_sensing(info);
+	if (!ret) {
+		kfree(mtd);
+		info->mtd = NULL;
+		printk(KERN_INFO "There is no nand chip on cs 0!\n");
+
+		return -EINVAL;
+	}
+
+	chip->cmdfunc(mtd, NAND_CMD_READID, 0, 0);
+	id = *((uint16_t *)(info->data_buff));
+	if (id != 0)
+		printk(KERN_INFO "Detect a flash id %x\n", id);
+	else {
+		kfree(mtd);
+		info->mtd = NULL;
+		printk(KERN_WARNING "Read out ID 0, potential timing set wrong!!\n");
+
+		return -EINVAL;
+	}
+
+	for (i=0; i<ARRAY_SIZE(builtin_flash_types) + pdata->num_flash - 1; i++) {
+		if (i < pdata->num_flash)
+			f = pdata->flash + i;
+		else
+			f = &builtin_flash_types[i - pdata->num_flash + 1];
+
+		/* find the chip in default list */
+		if (f->chip_id == id) {
+			pxa3xx_nand_config_flash(info, f);
+			mtd->writesize = f->page_size;
+			mtd->writesize_shift = ffs(mtd->writesize) - 1;
+			mtd->writesize_mask = (1 << mtd->writesize_shift) - 1;
+			mtd->oobsize = mtd->writesize / 32;
+			mtd->erasesize = f->page_size * f->page_per_block;
+			mtd->erasesize_shift = ffs(mtd->erasesize) - 1;
+			mtd->erasesize_mask = (1 << mtd->erasesize_shift) - 1;
+
+			mtd->name = mtd_names[0];
+			break;
+		}
+	}
+
+	if (i >= (ARRAY_SIZE(builtin_flash_types) + pdata->num_flash)) {
+		kfree(mtd);
+		info->mtd = NULL;
+		printk(KERN_ERR "ERROR!! flash not defined!!!\n");
+
+		return -EINVAL;
+	}
+
+	chip->ecc.mode = NAND_ECC_HW;
+	chip->ecc.size = f->page_size;
+	chip->chipsize 	= (uint64_t)f->num_blocks * f->page_per_block
+			  * f->page_size;
+	mtd->size = chip->chipsize;
+
+	/* Calculate the address shift from the page size */
+	chip->page_shift = ffs(mtd->writesize) - 1;
+	chip->pagemask = mtd_div_by_ws(chip->chipsize, mtd) - 1;
+	chip->numchips = 1;
+	chip->bbt_erase_shift = chip->phys_erase_shift = ffs(mtd->erasesize) - 1;
+
+	chip->options = (f->flash_width == 16) ? NAND_BUSWIDTH_16: 0;
+	chip->options |= NAND_NO_AUTOINCR;
+	chip->options |= NAND_NO_READRDY;
+	chip->options |= NAND_USE_FLASH_BBT;
+
+	return nand_scan_tail(mtd);
 }
 
 static struct pxa3xx_nand_info *alloc_nand_resource(struct platform_device *pdev)
 {
-	struct pxa3xx_nand_platform_data *pdata = pdev->dev.platform_data;
 	struct pxa3xx_nand_info *info;
+	struct nand_chip *chip;
 	struct mtd_info *mtd;
 	struct resource *r;
 	int ret, irq;
@@ -979,12 +1046,32 @@ static struct pxa3xx_nand_info *alloc_nand_resource(struct platform_device *pdev
 	}
 
 	info = (struct pxa3xx_nand_info *)(&mtd[1]);
+	chip = (struct nand_chip *)(&mtd[1]);
 	info->pdev = pdev;
-
-	mtd->priv = info;
 	info->mtd = mtd;
+	mtd->priv = info;
 	mtd->owner = THIS_MODULE;
 
+	chip->ecc.read_page	= pxa3xx_nand_read_page_hwecc;
+	chip->ecc.write_page	= pxa3xx_nand_write_page_hwecc;
+	chip->ecc.hwctl		= pxa3xx_nand_ecc_hwctl;
+	chip->ecc.calculate	= pxa3xx_nand_ecc_calculate;
+	chip->ecc.correct	= pxa3xx_nand_ecc_correct;
+	chip->controller        = &info->controller;
+	chip->waitfunc		= pxa3xx_nand_waitfunc;
+	chip->select_chip	= pxa3xx_nand_select_chip;
+	chip->dev_ready		= pxa3xx_nand_dev_ready;
+	chip->cmdfunc		= pxa3xx_nand_cmdfunc;
+	chip->read_word		= pxa3xx_nand_read_word;
+	chip->read_byte		= pxa3xx_nand_read_byte;
+	chip->read_buf		= pxa3xx_nand_read_buf;
+	chip->write_buf		= pxa3xx_nand_write_buf;
+	chip->verify_buf	= pxa3xx_nand_verify_buf;
+	chip->erase_cmd		= pxa3xx_nand_erase_cmd;
+	chip->scan_bbt		= nand_default_bbt;
+
+	spin_lock_init(&chip->controller->lock);
+	init_waitqueue_head(&chip->controller->wq);
 	info->clk = clk_get(&pdev->dev, NULL);
 	if (IS_ERR(info->clk)) {
 		dev_err(&pdev->dev, "failed to get nand clock\n");
@@ -1052,21 +1139,12 @@ static struct pxa3xx_nand_info *alloc_nand_resource(struct platform_device *pdev
 		goto fail_free_buf;
 	}
 
-	ret = pxa3xx_nand_detect_flash(info, pdata);
-	if (ret) {
-		dev_err(&pdev->dev, "failed to detect flash\n");
-		ret = -ENODEV;
-		goto fail_free_irq;
-	}
-
-	pxa3xx_nand_init_mtd(mtd, info);
 	platform_set_drvdata(pdev, info);
 
 	return info;
 
-fail_free_irq:
-	free_irq(irq, info);
 fail_free_buf:
+	free_irq(irq, info);
 	if (use_dma) {
 		pxa_free_dma(info->data_dma_ch);
 		dma_free_coherent(&pdev->dev, info->data_buff_size,
@@ -1136,7 +1214,7 @@ static int pxa3xx_nand_probe(struct platform_device *pdev)
 	if (info == NULL)
 		return -ENOMEM;
 
-	if (nand_scan(info->mtd, 1)) {
+	if (pxa3xx_nand_scan(info->mtd)) {
 		dev_err(&pdev->dev, "failed to scan nand\n");
 		pxa3xx_nand_remove(pdev);
 		return -ENODEV;
-- 
1.7.0.4

^ permalink raw reply related	[flat|nested] 15+ messages in thread

* [PATCH V3 6/6] pxa3xx_nand: clean the keep configure code
  2010-10-22 10:14 [PATCH V3 0/6] pxa3xx_nand update series set Lei Wen
                   ` (4 preceding siblings ...)
  2010-10-22 10:14 ` [PATCH V3 5/6] pxa3xx_nand: mtd scan id process could be defined by driver itself Lei Wen
@ 2010-10-22 10:14 ` Lei Wen
  2010-10-22 10:15 ` [PATCH V3 0/6] pxa3xx_nand update series set Haojian Zhuang
  6 siblings, 0 replies; 15+ messages in thread
From: Lei Wen @ 2010-10-22 10:14 UTC (permalink / raw)
  To: linux-arm-kernel

Signed-off-by: Lei Wen <leiwen@marvell.com>
Signed-off-by: Haojian Zhuang <haojian.zhuang@marvell.com>
Cc: Eric Miao <eric.y.miao@gmail.com>
Cc: David Woodhouse <dwmw2@infradead.org>
---
 arch/arm/plat-pxa/include/plat/pxa3xx_nand.h |    1 +
 drivers/mtd/nand/pxa3xx_nand.c               |  104 +++++++++-----------------
 2 files changed, 36 insertions(+), 69 deletions(-)

diff --git a/arch/arm/plat-pxa/include/plat/pxa3xx_nand.h b/arch/arm/plat-pxa/include/plat/pxa3xx_nand.h
index 01a8448..0bd4e88 100644
--- a/arch/arm/plat-pxa/include/plat/pxa3xx_nand.h
+++ b/arch/arm/plat-pxa/include/plat/pxa3xx_nand.h
@@ -30,6 +30,7 @@ struct pxa3xx_nand_cmdset {
 };
 
 struct pxa3xx_nand_flash {
+	char		*name;
 	uint32_t	chip_id;
 	unsigned int	page_per_block; /* Pages per block (PG_PER_BLK) */
 	unsigned int	page_size;	/* Page size in bytes (PAGE_SZ) */
diff --git a/drivers/mtd/nand/pxa3xx_nand.c b/drivers/mtd/nand/pxa3xx_nand.c
index b5dc5d7..7901bde 100644
--- a/drivers/mtd/nand/pxa3xx_nand.c
+++ b/drivers/mtd/nand/pxa3xx_nand.c
@@ -211,15 +211,15 @@ static struct pxa3xx_nand_timing timing[] = {
 };
 
 static struct pxa3xx_nand_flash builtin_flash_types[] = {
-	{      0,   0, 2048,  8,  8,    0, &default_cmdset, &timing[0] },
-	{ 0x46ec,  32,  512, 16, 16, 4096, &default_cmdset, &timing[1] },
-	{ 0xdaec,  64, 2048,  8,  8, 2048, &default_cmdset, &timing[1] },
-	{ 0xd7ec, 128, 4096,  8,  8, 8192, &default_cmdset, &timing[1] },
-	{ 0xa12c,  64, 2048,  8,  8, 1024, &default_cmdset, &timing[2] },
-	{ 0xb12c,  64, 2048, 16, 16, 1024, &default_cmdset, &timing[2] },
-	{ 0xdc2c,  64, 2048,  8,  8, 4096, &default_cmdset, &timing[2] },
-	{ 0xcc2c,  64, 2048, 16, 16, 4096, &default_cmdset, &timing[2] },
-	{ 0xba20,  64, 2048, 16, 16, 2048, &default_cmdset, &timing[3] },
+{ "DEFAULT FLASH",      0,   0, 2048,  8,  8,    0, &default_cmdset, &timing[0] },
+{ "64MiB 16-bit",  0x46ec,  32,  512, 16, 16, 4096, &default_cmdset, &timing[1] },
+{ "256MiB 8-bit",  0xdaec,  64, 2048,  8,  8, 2048, &default_cmdset, &timing[1] },
+{ "4GiB 8-bit",    0xd7ec, 128, 4096,  8,  8, 8192, &default_cmdset, &timing[1] },
+{ "128MiB 8-bit",  0xa12c,  64, 2048,  8,  8, 1024, &default_cmdset, &timing[2] },
+{ "128MiB 16-bit", 0xb12c,  64, 2048, 16, 16, 1024, &default_cmdset, &timing[2] },
+{ "512MiB 8-bit",  0xdc2c,  64, 2048,  8,  8, 4096, &default_cmdset, &timing[2] },
+{ "512MiB 16-bit", 0xcc2c,  64, 2048, 16, 16, 4096, &default_cmdset, &timing[2] },
+{ "256MiB 16-bit", 0xba20,  64, 2048, 16, 16, 2048, &default_cmdset, &timing[3] },
 };
 
 /* Define a default flash type setting serve as flash detecting only */
@@ -772,7 +772,6 @@ static int pxa3xx_nand_config_flash(struct pxa3xx_nand_info *info,
 	/* calculate flash information */
 	info->cmdset = f->cmdset;
 	info->page_size = f->page_size;
-	info->oob_buff = info->data_buff + f->page_size;
 	info->read_id_bytes = (f->page_size == 2048) ? 4 : 2;
 
 	/* calculate addressing information */
@@ -802,44 +801,12 @@ static int pxa3xx_nand_config_flash(struct pxa3xx_nand_info *info,
 static int pxa3xx_nand_detect_config(struct pxa3xx_nand_info *info)
 {
 	uint32_t ndcr = nand_readl(info, NDCR);
-	struct nand_flash_dev *type = NULL;
-	uint32_t id = -1, page_per_block, num_blocks;
-	int i;
 
-	page_per_block = ndcr & NDCR_PG_PER_BLK ? 64 : 32;
 	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;
 
-	pxa3xx_nand_cmdfunc(info->mtd, NAND_CMD_READID, 0, 0);
-	id = *((uint16_t *)(info->data_buff));
-	if (id == 0)
-		return -ENODEV;
-
-	/* Lookup the flash id */
-	for (i = 0; nand_flash_ids[i].name != NULL; i++) {
-		if (id == nand_flash_ids[i].id) {
-			type =  &nand_flash_ids[i];
-			break;
-		}
-	}
-
-	if (!type)
-		return -ENODEV;
-
-	/* fill the missing flash information */
-	i = __ffs(page_per_block * info->page_size);
-	num_blocks = type->chipsize << (20 - i);
-
-	/* calculate addressing information */
-	info->col_addr_cycles = (info->page_size == 2048) ? 2 : 1;
-
-	if (num_blocks * page_per_block > 65536)
-		info->row_addr_cycles = 3;
-	else
-		info->row_addr_cycles = 2;
-
 	info->ndtr0cs0 = nand_readl(info, NDTR0CS0);
 	info->ndtr1cs0 = nand_readl(info, NDTR1CS0);
 	info->cmdset = &default_cmdset;
@@ -951,13 +918,15 @@ 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,}};
 	const struct pxa3xx_nand_flash *f = NULL;
 	struct nand_chip *chip = mtd->priv;
 	uint32_t id = -1;
+	uint64_t chipsize;
 	int i, ret;
 
 	if (pdata->keep_config && !pxa3xx_nand_detect_config(info))
-		return 0;
+		goto KEEP_CONFIG;
 
 	ret = pxa3xx_nand_sensing(info);
 	if (!ret) {
@@ -987,22 +956,11 @@ static int pxa3xx_nand_scan(struct mtd_info *mtd)
 			f = &builtin_flash_types[i - pdata->num_flash + 1];
 
 		/* find the chip in default list */
-		if (f->chip_id == id) {
-			pxa3xx_nand_config_flash(info, f);
-			mtd->writesize = f->page_size;
-			mtd->writesize_shift = ffs(mtd->writesize) - 1;
-			mtd->writesize_mask = (1 << mtd->writesize_shift) - 1;
-			mtd->oobsize = mtd->writesize / 32;
-			mtd->erasesize = f->page_size * f->page_per_block;
-			mtd->erasesize_shift = ffs(mtd->erasesize) - 1;
-			mtd->erasesize_mask = (1 << mtd->erasesize_shift) - 1;
-
-			mtd->name = mtd_names[0];
+		if (f->chip_id == id)
 			break;
-		}
 	}
 
-	if (i >= (ARRAY_SIZE(builtin_flash_types) + pdata->num_flash)) {
+	if (i >= (ARRAY_SIZE(builtin_flash_types) + pdata->num_flash - 1)) {
 		kfree(mtd);
 		info->mtd = NULL;
 		printk(KERN_ERR "ERROR!! flash not defined!!!\n");
@@ -1010,20 +968,28 @@ static int pxa3xx_nand_scan(struct mtd_info *mtd)
 		return -EINVAL;
 	}
 
+	pxa3xx_nand_config_flash(info, f);
+	pxa3xx_flash_ids[0].name = f->name;
+	pxa3xx_flash_ids[0].id = (f->chip_id >> 8) & 0xffff;
+	pxa3xx_flash_ids[0].pagesize = f->page_size;
+	chipsize = (uint64_t)f->num_blocks * f->page_per_block * f->page_size;
+	pxa3xx_flash_ids[0].chipsize = chipsize >> 20;
+	pxa3xx_flash_ids[0].erasesize = f->page_size * f->page_per_block;
+	pxa3xx_flash_ids[0].options = (f->flash_width == 16) ? NAND_BUSWIDTH_16 : 0;
+KEEP_CONFIG:
+	if (nand_scan_ident(mtd, 1, pxa3xx_flash_ids))
+		return -ENODEV;
+	/* calculate addressing information */
+	info->col_addr_cycles = (mtd->writesize >= 2048) ? 2 : 1;
+	info->oob_buff = info->data_buff + mtd->writesize;
+	if ((mtd->size >> chip->page_shift) > 65536)
+		info->row_addr_cycles = 3;
+	else
+		info->row_addr_cycles = 2;
+	mtd->name = mtd_names[0];
 	chip->ecc.mode = NAND_ECC_HW;
-	chip->ecc.size = f->page_size;
-	chip->chipsize 	= (uint64_t)f->num_blocks * f->page_per_block
-			  * f->page_size;
-	mtd->size = chip->chipsize;
-
-	/* Calculate the address shift from the page size */
-	chip->page_shift = ffs(mtd->writesize) - 1;
-	chip->pagemask = mtd_div_by_ws(chip->chipsize, mtd) - 1;
-	chip->numchips = 1;
-	chip->bbt_erase_shift = chip->phys_erase_shift = ffs(mtd->erasesize) - 1;
-
-	chip->options = (f->flash_width == 16) ? NAND_BUSWIDTH_16: 0;
-	chip->options |= NAND_NO_AUTOINCR;
+	chip->ecc.size = mtd->writesize;
+	chip->options = (info->reg_ndcr & NDCR_DWIDTH_M) ? NAND_BUSWIDTH_16: 0;
 	chip->options |= NAND_NO_READRDY;
 	chip->options |= NAND_USE_FLASH_BBT;
 
-- 
1.7.0.4

^ permalink raw reply related	[flat|nested] 15+ messages in thread

* [PATCH V3 0/6] pxa3xx_nand update series set
  2010-10-22 10:14 [PATCH V3 0/6] pxa3xx_nand update series set Lei Wen
                   ` (5 preceding siblings ...)
  2010-10-22 10:14 ` [PATCH V3 6/6] pxa3xx_nand: clean the keep configure code Lei Wen
@ 2010-10-22 10:15 ` Haojian Zhuang
  2010-11-19  4:42   ` Lei Wen
  6 siblings, 1 reply; 15+ messages in thread
From: Haojian Zhuang @ 2010-10-22 10:15 UTC (permalink / raw)
  To: linux-arm-kernel

On Fri, Oct 22, 2010 at 6:14 PM, Lei Wen <leiwen@marvell.com> wrote:
> [PATCH V3 1/6] pxa3xx_nand: make scan procedure more clear
> [PATCH V3 2/6] pxa3xx_nand: rework irq logic
> [PATCH V3 3/6] pxa3xx_nand: discard wait_for_event,write_cmd,__readid function
> [PATCH V3 4/6] pxa3xx_nand: unify prepare command
> [PATCH V3 5/6] pxa3xx_nand: mtd scan id process could be defined by driver itself
> [PATCH V3 6/6] pxa3xx_nand: clean the keep configure code
>

Hi Eric and all,

Patches are refreshed. Please help to review.

Best Regards
Haojian

^ permalink raw reply	[flat|nested] 15+ messages in thread

* [PATCH V3 0/6] pxa3xx_nand update series set
  2010-10-22 10:15 ` [PATCH V3 0/6] pxa3xx_nand update series set Haojian Zhuang
@ 2010-11-19  4:42   ` Lei Wen
  2010-12-25  4:31     ` Lei Wen
  0 siblings, 1 reply; 15+ messages in thread
From: Lei Wen @ 2010-11-19  4:42 UTC (permalink / raw)
  To: linux-arm-kernel

Hi David,

Would you mind check whether those patches need any fix, or could be merged now?

Thanks,
Lei

On Fri, Oct 22, 2010 at 6:15 PM, Haojian Zhuang
<haojian.zhuang@gmail.com> wrote:
> On Fri, Oct 22, 2010 at 6:14 PM, Lei Wen <leiwen@marvell.com> wrote:
>> [PATCH V3 1/6] pxa3xx_nand: make scan procedure more clear
>> [PATCH V3 2/6] pxa3xx_nand: rework irq logic
>> [PATCH V3 3/6] pxa3xx_nand: discard wait_for_event,write_cmd,__readid function
>> [PATCH V3 4/6] pxa3xx_nand: unify prepare command
>> [PATCH V3 5/6] pxa3xx_nand: mtd scan id process could be defined by driver itself
>> [PATCH V3 6/6] pxa3xx_nand: clean the keep configure code
>>
>
> Hi Eric and all,
>
> Patches are refreshed. Please help to review.
>
> Best Regards
> Haojian
>

^ permalink raw reply	[flat|nested] 15+ messages in thread

* [PATCH V3 0/6] pxa3xx_nand update series set
  2010-11-19  4:42   ` Lei Wen
@ 2010-12-25  4:31     ` Lei Wen
  2011-02-12 14:57       ` Eric Miao
  0 siblings, 1 reply; 15+ messages in thread
From: Lei Wen @ 2010-12-25  4:31 UTC (permalink / raw)
  To: linux-arm-kernel

Ping...

Should we restart this review?...

Best regards,
Lei

On Fri, Nov 19, 2010 at 12:42 PM, Lei Wen <adrian.wenl@gmail.com> wrote:
> Hi David,
>
> Would you mind check whether those patches need any fix, or could be merged now?
>
> Thanks,
> Lei
>
> On Fri, Oct 22, 2010 at 6:15 PM, Haojian Zhuang
> <haojian.zhuang@gmail.com> wrote:
>> On Fri, Oct 22, 2010 at 6:14 PM, Lei Wen <leiwen@marvell.com> wrote:
>>> [PATCH V3 1/6] pxa3xx_nand: make scan procedure more clear
>>> [PATCH V3 2/6] pxa3xx_nand: rework irq logic
>>> [PATCH V3 3/6] pxa3xx_nand: discard wait_for_event,write_cmd,__readid function
>>> [PATCH V3 4/6] pxa3xx_nand: unify prepare command
>>> [PATCH V3 5/6] pxa3xx_nand: mtd scan id process could be defined by driver itself
>>> [PATCH V3 6/6] pxa3xx_nand: clean the keep configure code
>>>
>>
>> Hi Eric and all,
>>
>> Patches are refreshed. Please help to review.
>>
>> Best Regards
>> Haojian
>>
>

^ permalink raw reply	[flat|nested] 15+ messages in thread

* [PATCH V3 0/6] pxa3xx_nand update series set
  2010-12-25  4:31     ` Lei Wen
@ 2011-02-12 14:57       ` Eric Miao
  2011-02-12 15:01         ` Lei Wen
  0 siblings, 1 reply; 15+ messages in thread
From: Eric Miao @ 2011-02-12 14:57 UTC (permalink / raw)
  To: linux-arm-kernel

On Sat, Dec 25, 2010 at 12:31 PM, Lei Wen <adrian.wenl@gmail.com> wrote:
> Ping...
>
> Should we restart this review?...
>

Sure. Do you have any update to this patch series? If so,
please post.

^ permalink raw reply	[flat|nested] 15+ messages in thread

* [PATCH V3 0/6] pxa3xx_nand update series set
  2011-02-12 14:57       ` Eric Miao
@ 2011-02-12 15:01         ` Lei Wen
  0 siblings, 0 replies; 15+ messages in thread
From: Lei Wen @ 2011-02-12 15:01 UTC (permalink / raw)
  To: linux-arm-kernel

Hi Eric,

On Sat, Feb 12, 2011 at 10:57 PM, Eric Miao <eric.y.miao@gmail.com> wrote:
> On Sat, Dec 25, 2010 at 12:31 PM, Lei Wen <adrian.wenl@gmail.com> wrote:
>> Ping...
>>
>> Should we restart this review?...
>>
>
> Sure. Do you have any update to this patch series? If so,
> please post.
>

Nothing new for this patch set. Just wondering whether it could be accepted now
or need further refine. :(

Best regards,
Lei

^ permalink raw reply	[flat|nested] 15+ messages in thread

* [PATCH V3 2/6] pxa3xx_nand: rework irq logic
  2010-10-22 10:14 ` [PATCH V3 2/6] pxa3xx_nand: rework irq logic Lei Wen
@ 2011-02-13  9:46   ` Eric Miao
  2011-02-13 15:46     ` Lei Wen
  0 siblings, 1 reply; 15+ messages in thread
From: Eric Miao @ 2011-02-13  9:46 UTC (permalink / raw)
  To: linux-arm-kernel

Hi Lei,

I like this idea very much. There are several places for improvement,
though, you mind take a look?

> ?static int pxa3xx_nand_dev_ready(struct mtd_info *mtd)
> @@ -580,14 +582,12 @@ static void pxa3xx_nand_cmdfunc(struct mtd_info *mtd, unsigned command,
> ?{
> ? ? ? ?struct pxa3xx_nand_info *info = mtd->priv;
> ? ? ? ?const struct pxa3xx_nand_cmdset *cmdset = info->cmdset;
> - ? ? ? int ret;
> + ? ? ? int ret, exec_cmd = 0;
>
> ? ? ? ?info->use_dma = (use_dma) ? 1 : 0;
> ? ? ? ?info->use_ecc = 0;
> ? ? ? ?info->data_size = 0;
> - ? ? ? info->state = STATE_READY;
> -
> - ? ? ? init_completion(&info->cmd_complete);
> + ? ? ? info->state = 0;
>
> ? ? ? ?switch (command) {
> ? ? ? ?case NAND_CMD_READOOB:
> @@ -596,14 +596,12 @@ static void pxa3xx_nand_cmdfunc(struct mtd_info *mtd, unsigned command,
> ? ? ? ? ? ? ? ?info->buf_start = mtd->writesize + column;
> ? ? ? ? ? ? ? ?memset(info->data_buff, 0xFF, info->buf_count);
>
> - ? ? ? ? ? ? ? if (prepare_read_prog_cmd(info, cmdset->read1, column, page_addr))
> - ? ? ? ? ? ? ? ? ? ? ? break;
> -
> - ? ? ? ? ? ? ? pxa3xx_nand_do_cmd(info, NDSR_RDDREQ | NDSR_DBERR | NDSR_SBERR);
> -
> + ? ? ? ? ? ? ? prepare_read_prog_cmd(info, cmdset->read1, column, page_addr);
> ? ? ? ? ? ? ? ?/* We only are OOB, so if the data has error, does not matter */
> ? ? ? ? ? ? ? ?if (info->retcode == ERR_DBERR)
> ? ? ? ? ? ? ? ? ? ? ? ?info->retcode = ERR_NONE;

Note the above statement is executed after a do_cmd() is done, so
info->retcode contains a valid return code. While the actual code
to perform the command has been moved to below.

For each transaction, there are three steps:

  1. prepare/fill the command
  2. perform the command
  3. handle the return status

I actually though of merging steps 2) and 3) together, but step
3) could be different, and that the timeout for different type of
commands might be different, that's why I introduced the do_cmd()
function.

> +
> + ? ? ? ? ? ? ? exec_cmd = 1;

Maybe for those cases where commands are not executed, just simply
return will avoid introducing this exec_cmd variable.

.....

> @@ -807,10 +795,7 @@ static int __readid(struct pxa3xx_nand_info *info, uint32_t *id)
> ? ? ? ?uint32_t ndcr;
> ? ? ? ?uint8_t ?id_buff[8];
>

I saw __readid() is replaced by pxa3xx_nand_cmdfunc(NAND_CMD_READID)
in the code below.  Is it possible to completely remove this function
now??

^ permalink raw reply	[flat|nested] 15+ messages in thread

* [PATCH V3 3/6] pxa3xx_nand: discard wait_for_event, write_cmd, __readid function
  2010-10-22 10:14 ` [PATCH V3 3/6] pxa3xx_nand: discard wait_for_event, write_cmd, __readid function Lei Wen
@ 2011-02-13  9:47   ` Eric Miao
  0 siblings, 0 replies; 15+ messages in thread
From: Eric Miao @ 2011-02-13  9:47 UTC (permalink / raw)
  To: linux-arm-kernel

On Fri, Oct 22, 2010 at 6:14 PM, Lei Wen <leiwen@marvell.com> wrote:
> Since we have rework the irq process, we don't need additional
> delay in wait_for_event. Also write_cmd and __readid is also
> discarded for the same reason.
>
> Signed-off-by: Lei Wen <leiwen@marvell.com>
> Signed-off-by: Haojian Zhuang <haojian.zhuang@marvell.com>
> Cc: Eric Miao <eric.y.miao@gmail.com>
> Cc: David Woodhouse <dwmw2@infradead.org>

Ack.

> ---
> ?drivers/mtd/nand/pxa3xx_nand.c | ? 77 +---------------------------------------
> ?1 files changed, 1 insertions(+), 76 deletions(-)
>
> diff --git a/drivers/mtd/nand/pxa3xx_nand.c b/drivers/mtd/nand/pxa3xx_nand.c
> index dc24000..a7f623f 100644
> --- a/drivers/mtd/nand/pxa3xx_nand.c
> +++ b/drivers/mtd/nand/pxa3xx_nand.c
> @@ -258,25 +258,6 @@ static void pxa3xx_nand_set_timing(struct pxa3xx_nand_info *info,
> ? ? ? ?nand_writel(info, NDTR1CS0, ndtr1);
> ?}
>
> -#define WAIT_EVENT_TIMEOUT ? ? 10
> -
> -static int wait_for_event(struct pxa3xx_nand_info *info, uint32_t event)
> -{
> - ? ? ? int timeout = WAIT_EVENT_TIMEOUT;
> - ? ? ? uint32_t ndsr;
> -
> - ? ? ? while (timeout--) {
> - ? ? ? ? ? ? ? ndsr = nand_readl(info, NDSR) & NDSR_MASK;
> - ? ? ? ? ? ? ? if (ndsr & event) {
> - ? ? ? ? ? ? ? ? ? ? ? nand_writel(info, NDSR, ndsr);
> - ? ? ? ? ? ? ? ? ? ? ? return 0;
> - ? ? ? ? ? ? ? }
> - ? ? ? ? ? ? ? udelay(10);
> - ? ? ? }
> -
> - ? ? ? return -ETIMEDOUT;
> -}
> -
> ?static void pxa3xx_set_datasize(struct pxa3xx_nand_info *info)
> ?{
> ? ? ? ?int oob_enable = info->reg_ndcr & NDCR_SPARE_EN;
> @@ -414,35 +395,6 @@ static void disable_int(struct pxa3xx_nand_info *info, uint32_t int_mask)
> ? ? ? ?nand_writel(info, NDCR, ndcr | int_mask);
> ?}
>
> -/* NOTE: it is a must to set ND_RUN firstly, then write command buffer
> - * otherwise, it does not work
> - */
> -static int write_cmd(struct pxa3xx_nand_info *info)
> -{
> - ? ? ? uint32_t ndcr;
> -
> - ? ? ? /* clear status bits and run */
> - ? ? ? nand_writel(info, NDSR, NDSR_MASK);
> -
> - ? ? ? ndcr = info->reg_ndcr;
> -
> - ? ? ? ndcr |= info->use_ecc ? NDCR_ECC_EN : 0;
> - ? ? ? ndcr |= info->use_dma ? NDCR_DMA_EN : 0;
> - ? ? ? ndcr |= NDCR_ND_RUN;
> -
> - ? ? ? nand_writel(info, NDCR, ndcr);
> -
> - ? ? ? if (wait_for_event(info, NDSR_WRCMDREQ)) {
> - ? ? ? ? ? ? ? printk(KERN_ERR "timed out writing command\n");
> - ? ? ? ? ? ? ? return -ETIMEDOUT;
> - ? ? ? }
> -
> - ? ? ? nand_writel(info, NDCB0, info->ndcb0);
> - ? ? ? nand_writel(info, NDCB0, info->ndcb1);
> - ? ? ? nand_writel(info, NDCB0, info->ndcb2);
> - ? ? ? return 0;
> -}
> -
> ?static void handle_data_pio(struct pxa3xx_nand_info *info)
> ?{
> ? ? ? ?switch (info->state) {
> @@ -789,33 +741,6 @@ static int pxa3xx_nand_ecc_correct(struct mtd_info *mtd,
> ? ? ? ?return 0;
> ?}
>
> -static int __readid(struct pxa3xx_nand_info *info, uint32_t *id)
> -{
> - ? ? ? const struct pxa3xx_nand_cmdset *cmdset = info->cmdset;
> - ? ? ? uint32_t ndcr;
> - ? ? ? uint8_t ?id_buff[8];
> -
> - ? ? ? prepare_other_cmd(info, cmdset->read_id);
> -
> - ? ? ? /* Send command */
> - ? ? ? if (write_cmd(info))
> - ? ? ? ? ? ? ? goto fail_timeout;
> -
> - ? ? ? /* Wait for CMDDM(command done successfully) */
> - ? ? ? if (wait_for_event(info, NDSR_RDDREQ))
> - ? ? ? ? ? ? ? goto fail_timeout;
> -
> - ? ? ? __raw_readsl(info->mmio_base + NDDB, id_buff, 2);
> - ? ? ? *id = id_buff[0] | (id_buff[1] << 8);
> - ? ? ? return 0;
> -
> -fail_timeout:
> - ? ? ? ndcr = nand_readl(info, NDCR);
> - ? ? ? nand_writel(info, NDCR, ndcr & ~NDCR_ND_RUN);
> - ? ? ? udelay(10);
> - ? ? ? return -ETIMEDOUT;
> -}
> -
> ?static int pxa3xx_nand_config_flash(struct pxa3xx_nand_info *info,
> ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ?const struct pxa3xx_nand_flash *f)
> ?{
> @@ -868,7 +793,7 @@ static int pxa3xx_nand_detect_config(struct pxa3xx_nand_info *info)
>
> ? ? ? ?page_per_block = ndcr & NDCR_PG_PER_BLK ? 64 : 32;
> ? ? ? ?info->page_size = ndcr & NDCR_PAGE_SZ ? 2048 : 512;
> - ? ? ? /* set info fields needed to __readid */
> + ? ? ? /* set info fields needed to read id */
> ? ? ? ?info->read_id_bytes = (info->page_size == 2048) ? 4 : 2;
> ? ? ? ?info->reg_ndcr = ndcr;
>
> --
> 1.7.0.4
>
>

^ permalink raw reply	[flat|nested] 15+ messages in thread

* [PATCH V3 2/6] pxa3xx_nand: rework irq logic
  2011-02-13  9:46   ` Eric Miao
@ 2011-02-13 15:46     ` Lei Wen
  0 siblings, 0 replies; 15+ messages in thread
From: Lei Wen @ 2011-02-13 15:46 UTC (permalink / raw)
  To: linux-arm-kernel

Hi Eric,

On Sun, Feb 13, 2011 at 5:46 PM, Eric Miao <eric.y.miao@gmail.com> wrote:
> Hi Lei,
>
> I like this idea very much. There are several places for improvement,
> though, you mind take a look?
>
>> ?static int pxa3xx_nand_dev_ready(struct mtd_info *mtd)
>> @@ -580,14 +582,12 @@ static void pxa3xx_nand_cmdfunc(struct mtd_info *mtd, unsigned command,
>> ?{
>> ? ? ? ?struct pxa3xx_nand_info *info = mtd->priv;
>> ? ? ? ?const struct pxa3xx_nand_cmdset *cmdset = info->cmdset;
>> - ? ? ? int ret;
>> + ? ? ? int ret, exec_cmd = 0;
>>
>> ? ? ? ?info->use_dma = (use_dma) ? 1 : 0;
>> ? ? ? ?info->use_ecc = 0;
>> ? ? ? ?info->data_size = 0;
>> - ? ? ? info->state = STATE_READY;
>> -
>> - ? ? ? init_completion(&info->cmd_complete);
>> + ? ? ? info->state = 0;
>>
>> ? ? ? ?switch (command) {
>> ? ? ? ?case NAND_CMD_READOOB:
>> @@ -596,14 +596,12 @@ static void pxa3xx_nand_cmdfunc(struct mtd_info *mtd, unsigned command,
>> ? ? ? ? ? ? ? ?info->buf_start = mtd->writesize + column;
>> ? ? ? ? ? ? ? ?memset(info->data_buff, 0xFF, info->buf_count);
>>
>> - ? ? ? ? ? ? ? if (prepare_read_prog_cmd(info, cmdset->read1, column, page_addr))
>> - ? ? ? ? ? ? ? ? ? ? ? break;
>> -
>> - ? ? ? ? ? ? ? pxa3xx_nand_do_cmd(info, NDSR_RDDREQ | NDSR_DBERR | NDSR_SBERR);
>> -
>> + ? ? ? ? ? ? ? prepare_read_prog_cmd(info, cmdset->read1, column, page_addr);
>> ? ? ? ? ? ? ? ?/* We only are OOB, so if the data has error, does not matter */
>> ? ? ? ? ? ? ? ?if (info->retcode == ERR_DBERR)
>> ? ? ? ? ? ? ? ? ? ? ? ?info->retcode = ERR_NONE;
>
> Note the above statement is executed after a do_cmd() is done, so
> info->retcode contains a valid return code. While the actual code
> to perform the command has been moved to below.

It is my fault not delete those line here...
In my following 0004 patch, I would unify the prepare step into one function.
BTW, for OOB data, I think we also shouldn't omit the DB error.
Like BCH crc algorithm, it also protect the oob area, so it is reasonable to
be aware that area...
>
> For each transaction, there are three steps:
>
> ?1. prepare/fill the command
> ?2. perform the command
> ?3. handle the return status
>
> I actually though of merging steps 2) and 3) together, but step
> 3) could be different, and that the timeout for different type of
> commands might be different, that's why I introduced the do_cmd()
> function.

I agree. :)
That is what I want to do in this patch set, unify the prepare behavior
into one function, and then execute it.

>
>> +
>> + ? ? ? ? ? ? ? exec_cmd = 1;
>
> Maybe for those cases where commands are not executed, just simply
> return will avoid introducing this exec_cmd variable.

For I unify the prepare process into one function according to different
command, simply return would be unable to notify the execute action...
The exec_cmd would be made as the return value for the prepare function.

>
> .....
>
>> @@ -807,10 +795,7 @@ static int __readid(struct pxa3xx_nand_info *info, uint32_t *id)
>> ? ? ? ?uint32_t ndcr;
>> ? ? ? ?uint8_t ?id_buff[8];
>>
>
> I saw __readid() is replaced by pxa3xx_nand_cmdfunc(NAND_CMD_READID)
> in the code below. ?Is it possible to completely remove this function
> now??

I made the remove behavior in the patch 0003. :)

Thanks,
Lei

^ permalink raw reply	[flat|nested] 15+ messages in thread

end of thread, other threads:[~2011-02-13 15:46 UTC | newest]

Thread overview: 15+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2010-10-22 10:14 [PATCH V3 0/6] pxa3xx_nand update series set Lei Wen
2010-10-22 10:14 ` [PATCH V3 1/6] pxa3xx_nand: make scan procedure more clear Lei Wen
2010-10-22 10:14 ` [PATCH V3 2/6] pxa3xx_nand: rework irq logic Lei Wen
2011-02-13  9:46   ` Eric Miao
2011-02-13 15:46     ` Lei Wen
2010-10-22 10:14 ` [PATCH V3 3/6] pxa3xx_nand: discard wait_for_event, write_cmd, __readid function Lei Wen
2011-02-13  9:47   ` Eric Miao
2010-10-22 10:14 ` [PATCH V3 4/6] pxa3xx_nand: unify prepare command Lei Wen
2010-10-22 10:14 ` [PATCH V3 5/6] pxa3xx_nand: mtd scan id process could be defined by driver itself Lei Wen
2010-10-22 10:14 ` [PATCH V3 6/6] pxa3xx_nand: clean the keep configure code Lei Wen
2010-10-22 10:15 ` [PATCH V3 0/6] pxa3xx_nand update series set Haojian Zhuang
2010-11-19  4:42   ` Lei Wen
2010-12-25  4:31     ` Lei Wen
2011-02-12 14:57       ` Eric Miao
2011-02-12 15:01         ` Lei Wen

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).