All of lore.kernel.org
 help / color / mirror / Atom feed
* [PATCHv2 0/6] Further Denali NAND enhancements
@ 2011-06-03 10:32 Jamie Iles
  2011-06-03 10:32 ` [PATCHv2 1/6] nand/denali: convert to dev_() printk helpers Jamie Iles
                   ` (5 more replies)
  0 siblings, 6 replies; 13+ messages in thread
From: Jamie Iles @ 2011-06-03 10:32 UTC (permalink / raw)
  To: linux-mtd; +Cc: Jamie Iles

This series performs a couple of minor cleanups to the Denali NAND                                                                                                                                                                                                                                                             
controller driver and then introduces support for newer versions of the                                                                                                                                                                                                                                                        
IP where ECC fixup is supported in hardware.                                                                                                                                                                                                                                                                                   
                                                                                                                                                                                                                                                                                                                               
Finally add support for partitioning through platform data and cmdline                                                                                                                                                                                                                                                         
partitioning.

Jamie Iles (6):
  nand/denali: convert to dev_() printk helpers
  nand/denali: annotate pci init/exit functions with correct section
  nand/denali: allow the number of ECC bits to be set by pdata
  nand/denali: support hardware with internal ECC fixup
  nand/denali: support MTD partitioning
  mtd/denali: support for cmdline partitioning

 drivers/mtd/nand/denali.c            |  194 ++++++++++++++++++----------------
 drivers/mtd/nand/denali.h            |   10 ++
 include/linux/platform_data/denali.h |   26 +++++
 3 files changed, 139 insertions(+), 91 deletions(-)
 create mode 100644 include/linux/platform_data/denali.h

-- 
1.7.4.1

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

* [PATCHv2 1/6] nand/denali: convert to dev_() printk helpers
  2011-06-03 10:32 [PATCHv2 0/6] Further Denali NAND enhancements Jamie Iles
@ 2011-06-03 10:32 ` Jamie Iles
  2011-06-06 13:11   ` Artem Bityutskiy
  2011-06-03 10:32 ` [PATCHv2 2/6] nand/denali: annotate pci init/exit functions with correct section Jamie Iles
                   ` (4 subsequent siblings)
  5 siblings, 1 reply; 13+ messages in thread
From: Jamie Iles @ 2011-06-03 10:32 UTC (permalink / raw)
  To: linux-mtd; +Cc: Jamie Iles, Artem Bityutskiy, David Woodhouse, Chuanxiao Dong

Use the dev_() printk helpers rather than printk so the name of the
device is include.  Also remove a duplicate definition of BANK().

Cc: David Woodhouse <dwmw2@infradead.org>
Cc: Chuanxiao Dong <chuanxiao.dong@intel.com>
Cc: Artem Bityutskiy <dedekind1@gmail.com>
Signed-off-by: Jamie Iles <jamie@jamieiles.com>
---
 drivers/mtd/nand/denali.c |  133 ++++++++++++++++++++-------------------------
 1 files changed, 60 insertions(+), 73 deletions(-)

diff --git a/drivers/mtd/nand/denali.c b/drivers/mtd/nand/denali.c
index d527621..a4b4a29 100644
--- a/drivers/mtd/nand/denali.c
+++ b/drivers/mtd/nand/denali.c
@@ -175,8 +175,7 @@ static uint16_t denali_nand_reset(struct denali_nand_info *denali)
 {
 	uint32_t i;
 
-	dev_dbg(denali->dev, "%s, Line %d, Function: %s\n",
-		       __FILE__, __LINE__, __func__);
+	dev_dbg(denali->dev, "resetting\n");
 
 	for (i = 0 ; i < denali->max_banks; i++)
 		iowrite32(INTR_STATUS__RST_COMP | INTR_STATUS__TIME_OUT,
@@ -191,7 +190,8 @@ static uint16_t denali_nand_reset(struct denali_nand_info *denali)
 		if (ioread32(denali->flash_reg + INTR_STATUS(i)) &
 			INTR_STATUS__TIME_OUT)
 			dev_dbg(denali->dev,
-			"NAND Reset operation timed out on bank %d\n", i);
+				"NAND Reset operation timed out on bank %d\n",
+				i);
 	}
 
 	for (i = 0; i < denali->max_banks; i++)
@@ -228,8 +228,7 @@ static void nand_onfi_timing_set(struct denali_nand_info *denali,
 	uint16_t acc_clks;
 	uint16_t addr_2_data, re_2_we, re_2_re, we_2_re, cs_cnt;
 
-	dev_dbg(denali->dev, "%s, Line %d, Function: %s\n",
-		       __FILE__, __LINE__, __func__);
+	dev_dbg(denali->dev, "init timing\n");
 
 	en_lo = CEIL_DIV(Trp[mode], CLK_X);
 	en_hi = CEIL_DIV(Treh[mode], CLK_X);
@@ -265,8 +264,8 @@ static void nand_onfi_timing_set(struct denali_nand_info *denali,
 		acc_clks++;
 
 	if ((data_invalid - acc_clks * CLK_X) < 2)
-		dev_warn(denali->dev, "%s, Line %d: Warning!\n",
-			__FILE__, __LINE__);
+		dev_warn(denali->dev, "%s, Line %d: Warning!\n", __FILE__,
+			 __LINE__);
 
 	addr_2_data = CEIL_DIV(Tadl[mode], CLK_X);
 	re_2_we = CEIL_DIV(Trhw[mode], CLK_X);
@@ -394,9 +393,8 @@ static void get_hynix_nand_para(struct denali_nand_info *denali,
 		break;
 	default:
 		dev_warn(denali->dev,
-			"Spectra: Unknown Hynix NAND (Device ID: 0x%x)."
-			"Will use default parameter values instead.\n",
-			device_id);
+			 "unknown Hynix NAND (Device ID: 0x%x). Will use default parameter values instead.\n",
+			 device_id);
 	}
 }
 
@@ -415,8 +413,8 @@ static void find_valid_banks(struct denali_nand_info *denali)
 		index_addr_read_data(denali,
 				(uint32_t)(MODE_11 | (i << 24) | 2), &id[i]);
 
-		dev_dbg(denali->dev,
-			"Return 1st ID for bank[%d]: %x\n", i, id[i]);
+		dev_dbg(denali->dev, "Return 1st ID for bank[%d]: %x\n", i,
+			id[i]);
 
 		if (i == 0) {
 			if (!(id[i] & 0x0ff))
@@ -436,13 +434,12 @@ static void find_valid_banks(struct denali_nand_info *denali)
 		 */
 		if (denali->total_used_banks != 1) {
 			dev_err(denali->dev,
-					"Sorry, Intel CE4100 only supports "
-					"a single NAND device.\n");
+				"Sorry, Intel CE4100 only supports a single NAND device.\n");
 			BUG();
 		}
 	}
-	dev_dbg(denali->dev,
-		"denali->total_used_banks: %d\n", denali->total_used_banks);
+	dev_dbg(denali->dev, "denali->total_used_banks: %d\n",
+		denali->total_used_banks);
 }
 
 /*
@@ -486,9 +483,7 @@ static uint16_t denali_nand_timing_set(struct denali_nand_info *denali)
 	uint32_t id_bytes[5], addr;
 	uint8_t i, maf_id, device_id;
 
-	dev_dbg(denali->dev,
-			"%s, Line %d, Function: %s\n",
-			__FILE__, __LINE__, __func__);
+	dev_dbg(denali->dev, "set the timing\n");
 
 	/* Use read id method to get device ID and other
 	 * params. For some NAND chips, controller can't
@@ -516,18 +511,17 @@ static uint16_t denali_nand_timing_set(struct denali_nand_info *denali)
 	}
 
 	dev_info(denali->dev,
-			"Dump timing register values:"
-			"acc_clks: %d, re_2_we: %d, re_2_re: %d\n"
-			"we_2_re: %d, addr_2_data: %d, rdwr_en_lo_cnt: %d\n"
-			"rdwr_en_hi_cnt: %d, cs_setup_cnt: %d\n",
-			ioread32(denali->flash_reg + ACC_CLKS),
-			ioread32(denali->flash_reg + RE_2_WE),
-			ioread32(denali->flash_reg + RE_2_RE),
-			ioread32(denali->flash_reg + WE_2_RE),
-			ioread32(denali->flash_reg + ADDR_2_DATA),
-			ioread32(denali->flash_reg + RDWR_EN_LO_CNT),
-			ioread32(denali->flash_reg + RDWR_EN_HI_CNT),
-			ioread32(denali->flash_reg + CS_SETUP_CNT));
+		 "Dump timing register values: acc_clks: %d, re_2_we: %d, re_2_re: %d\n"
+		 "we_2_re: %d, addr_2_data: %d, rdwr_en_lo_cnt: %d\n"
+		 "rdwr_en_hi_cnt: %d, cs_setup_cnt: %d\n",
+		 ioread32(denali->flash_reg + ACC_CLKS),
+		 ioread32(denali->flash_reg + RE_2_WE),
+		 ioread32(denali->flash_reg + RE_2_RE),
+		 ioread32(denali->flash_reg + WE_2_RE),
+		 ioread32(denali->flash_reg + ADDR_2_DATA),
+		 ioread32(denali->flash_reg + RDWR_EN_LO_CNT),
+		 ioread32(denali->flash_reg + RDWR_EN_HI_CNT),
+		 ioread32(denali->flash_reg + CS_SETUP_CNT));
 
 	find_valid_banks(denali);
 
@@ -545,8 +539,7 @@ static uint16_t denali_nand_timing_set(struct denali_nand_info *denali)
 static void denali_set_intr_modes(struct denali_nand_info *denali,
 					uint16_t INT_ENABLE)
 {
-	dev_dbg(denali->dev, "%s, Line %d, Function: %s\n",
-		       __FILE__, __LINE__, __func__);
+	dev_dbg(denali->dev, "set interrupt modes\n");
 
 	if (INT_ENABLE)
 		iowrite32(1, denali->flash_reg + GLOBAL_INT_ENABLE);
@@ -669,7 +662,6 @@ static irqreturn_t denali_isr(int irq, void *dev_id)
 	spin_unlock(&denali->irq_lock);
 	return result;
 }
-#define BANK(x) ((x) << 24)
 
 static uint32_t wait_for_irq(struct denali_nand_info *denali, uint32_t irq_mask)
 {
@@ -699,8 +691,9 @@ static uint32_t wait_for_irq(struct denali_nand_info *denali, uint32_t irq_mask)
 
 	if (comp_res == 0) {
 		/* timeout */
-		printk(KERN_ERR "timeout occurred, status = 0x%x, mask = 0x%x\n",
-				intr_status, irq_mask);
+		dev_err(&denali->mtd.dev,
+			"timeout occurred, status = 0x%x, mask = 0x%x\n",
+			intr_status, irq_mask);
 
 		intr_status = 0;
 	}
@@ -785,9 +778,8 @@ static int denali_send_pipeline_cmd(struct denali_nand_info *denali,
 
 			if (irq_status == 0) {
 				dev_err(denali->dev,
-						"cmd, page, addr on timeout "
-						"(0x%x, 0x%x, 0x%x)\n",
-						cmd, denali->page, addr);
+					"cmd, page, addr on timeout (0x%x, 0x%x, 0x%x)\n",
+					cmd, denali->page, addr);
 				status = FAIL;
 			} else {
 				cmd = MODE_01 | addr;
@@ -889,7 +881,7 @@ static void read_oob_data(struct mtd_info *mtd, uint8_t *buf, int page)
 
 		if (irq_status == 0)
 			dev_err(denali->dev, "page on OOB timeout %d\n",
-					denali->page);
+				denali->page);
 
 		/* We set the device back to MAIN_ACCESS here as I observed
 		 * instability with the controller if you do a block erase
@@ -1065,9 +1057,8 @@ static void write_page(struct mtd_info *mtd, struct nand_chip *chip,
 	irq_status = wait_for_irq(denali, irq_mask);
 
 	if (irq_status == 0) {
-		dev_err(denali->dev,
-				"timeout on write_page (type = %d)\n",
-				raw_xfer);
+		dev_err(denali->dev, "timeout on write_page (type = %d)\n",
+			raw_xfer);
 		denali->status =
 			(irq_status & INTR_STATUS__PROGRAM_FAIL) ?
 			NAND_STATUS_FAIL : PASS;
@@ -1132,9 +1123,9 @@ static int denali_read_page(struct mtd_info *mtd, struct nand_chip *chip,
 	bool check_erased_page = false;
 
 	if (page != denali->page) {
-		dev_err(denali->dev, "IN %s: page %d is not"
-				" equal to denali->page %d, investigate!!",
-				__func__, page, denali->page);
+		dev_err(denali->dev,
+			"IN %s: page %d is not equal to denali->page %d, investigate!!",
+			__func__, page, denali->page);
 		BUG();
 	}
 
@@ -1182,9 +1173,8 @@ static int denali_read_page_raw(struct mtd_info *mtd, struct nand_chip *chip,
 	uint32_t irq_mask = INTR_STATUS__DMA_CMD_COMP;
 
 	if (page != denali->page) {
-		dev_err(denali->dev, "IN %s: page %d is not"
-				" equal to denali->page %d, investigate!!",
-				__func__, page, denali->page);
+		dev_err(denali->dev, "IN %s: page %d is not equal to denali->page %d, investigate!!",
+			__func__, page, denali->page);
 		BUG();
 	}
 
@@ -1300,8 +1290,8 @@ static void denali_cmdfunc(struct mtd_info *mtd, unsigned int cmd, int col,
 		/* TODO: Read OOB data */
 		break;
 	default:
-		printk(KERN_ERR ": unsupported command"
-				" received 0x%x\n", cmd);
+		dev_err(&denali->mtd.dev, "unsupported command received 0x%x\n",
+			cmd);
 		break;
 	}
 }
@@ -1311,8 +1301,7 @@ static int denali_ecc_calculate(struct mtd_info *mtd, const uint8_t *data,
 				uint8_t *ecc_code)
 {
 	struct denali_nand_info *denali = mtd_to_denali(mtd);
-	dev_err(denali->dev,
-			"denali_ecc_calculate called unexpectedly\n");
+	dev_err(denali->dev, "denali_ecc_calculate called unexpectedly\n");
 	BUG();
 	return -EIO;
 }
@@ -1321,8 +1310,7 @@ static int denali_ecc_correct(struct mtd_info *mtd, uint8_t *data,
 				uint8_t *read_ecc, uint8_t *calc_ecc)
 {
 	struct denali_nand_info *denali = mtd_to_denali(mtd);
-	dev_err(denali->dev,
-			"denali_ecc_correct called unexpectedly\n");
+	dev_err(denali->dev, "denali_ecc_correct called unexpectedly\n");
 	BUG();
 	return -EIO;
 }
@@ -1330,8 +1318,7 @@ static int denali_ecc_correct(struct mtd_info *mtd, uint8_t *data,
 static void denali_ecc_hwctl(struct mtd_info *mtd, int mode)
 {
 	struct denali_nand_info *denali = mtd_to_denali(mtd);
-	dev_err(denali->dev,
-			"denali_ecc_hwctl called unexpectedly\n");
+	dev_err(denali->dev, "denali_ecc_hwctl called unexpectedly\n");
 	BUG();
 }
 /* end NAND core entry points */
@@ -1432,9 +1419,11 @@ static int denali_pci_probe(struct pci_dev *dev, const struct pci_device_id *id)
 	if (!denali)
 		return -ENOMEM;
 
+	denali->dev = &dev->dev;
+
 	ret = pci_enable_device(dev);
 	if (ret) {
-		printk(KERN_ERR "Spectra: pci_enable_device failed.\n");
+		dev_err(denali->dev, "Spectra: pci_enable_device failed.\n");
 		goto failed_alloc_memery;
 	}
 
@@ -1443,8 +1432,7 @@ static int denali_pci_probe(struct pci_dev *dev, const struct pci_device_id *id)
 		 * ONFI timing mode 1 and below.
 		 */
 		if (onfi_timing_mode < -1 || onfi_timing_mode > 1) {
-			printk(KERN_ERR "Intel CE4100 only supports"
-					" ONFI timing mode 1 or below\n");
+			dev_err(denali->dev, "Intel CE4100 only supports ONFI timing mode 1 or below\n");
 			ret = -EINVAL;
 			goto failed_enable_dev;
 		}
@@ -1468,38 +1456,37 @@ static int denali_pci_probe(struct pci_dev *dev, const struct pci_device_id *id)
 	/* Is 32-bit DMA supported? */
 	ret = dma_set_mask(&dev->dev, DMA_BIT_MASK(32));
 	if (ret) {
-		printk(KERN_ERR "Spectra: no usable DMA configuration\n");
+		dev_err(denali->dev, "Spectra: no usable DMA configuration\n");
 		goto failed_enable_dev;
 	}
 	denali->buf.dma_buf = dma_map_single(&dev->dev, denali->buf.buf,
 					     DENALI_BUF_SIZE,
 					     DMA_BIDIRECTIONAL);
 
-	if (dma_mapping_error(&dev->dev, denali->buf.dma_buf)) {
+	if (dma_mapping_error(denali->dev, denali->buf.dma_buf)) {
 		dev_err(&dev->dev, "Spectra: failed to map DMA buffer\n");
 		goto failed_enable_dev;
 	}
 
 	pci_set_master(dev);
-	denali->dev = &dev->dev;
 	denali->mtd.dev.parent = &dev->dev;
 
 	ret = pci_request_regions(dev, DENALI_NAND_NAME);
 	if (ret) {
-		printk(KERN_ERR "Spectra: Unable to request memory regions\n");
+		dev_err(denali->dev, "Spectra: Unable to request memory regions\n");
 		goto failed_dma_map;
 	}
 
 	denali->flash_reg = ioremap_nocache(csr_base, csr_len);
 	if (!denali->flash_reg) {
-		printk(KERN_ERR "Spectra: Unable to remap memory region\n");
+		dev_err(denali->dev, "Spectra: Unable to remap memory region\n");
 		ret = -ENOMEM;
 		goto failed_req_regions;
 	}
 
 	denali->flash_mem = ioremap_nocache(mem_base, mem_len);
 	if (!denali->flash_mem) {
-		printk(KERN_ERR "Spectra: ioremap_nocache failed!");
+		dev_err(denali->dev, "Spectra: ioremap_nocache failed!");
 		ret = -ENOMEM;
 		goto failed_remap_reg;
 	}
@@ -1511,7 +1498,7 @@ static int denali_pci_probe(struct pci_dev *dev, const struct pci_device_id *id)
 	 * initilization is finished*/
 	if (request_irq(dev->irq, denali_isr, IRQF_SHARED,
 			DENALI_NAND_NAME, denali)) {
-		printk(KERN_ERR "Spectra: Unable to allocate IRQ\n");
+		dev_err(denali->dev, "Spectra: Unable to allocate IRQ\n");
 		ret = -ENODEV;
 		goto failed_remap_mem;
 	}
@@ -1544,8 +1531,8 @@ static int denali_pci_probe(struct pci_dev *dev, const struct pci_device_id *id)
 	 */
 	if (denali->mtd.writesize > NAND_MAX_PAGESIZE + NAND_MAX_OOBSIZE) {
 		ret = -ENODEV;
-		printk(KERN_ERR "Spectra: device size not supported by this "
-			"version of MTD.");
+		dev_err(denali->dev,
+			"device size not supported by this version of MTD.");
 		goto failed_req_irq;
 	}
 
@@ -1595,8 +1582,8 @@ static int denali_pci_probe(struct pci_dev *dev, const struct pci_device_id *id)
 	} else if (denali->mtd.oobsize < (denali->bbtskipbytes +
 			ECC_8BITS * (denali->mtd.writesize /
 			ECC_SECTOR_SIZE))) {
-		printk(KERN_ERR "Your NAND chip OOB is not large enough to"
-				" contain 8bit ECC correction codes");
+		dev_err(&denali->mtd.dev,
+			"Your NAND chip OOB is not large enough to contain 8bit ECC correction codes");
 		goto failed_req_irq;
 	} else {
 		denali->nand.ecc.layout = &nand_8bit_oob;
@@ -1646,7 +1633,7 @@ static int denali_pci_probe(struct pci_dev *dev, const struct pci_device_id *id)
 
 	ret = mtd_device_register(&denali->mtd, NULL, 0);
 	if (ret) {
-		dev_err(&dev->dev, "Spectra: Failed to register MTD: %d\n",
+		dev_err(denali->dev, "Spectra: Failed to register MTD: %d\n",
 				ret);
 		goto failed_req_irq;
 	}
@@ -1701,7 +1688,7 @@ static struct pci_driver denali_pci_driver = {
 
 static int __devinit denali_init(void)
 {
-	printk(KERN_INFO "Spectra MTD driver\n");
+	pr_info(KERN_INFO "Spectra MTD driver\n");
 	return pci_register_driver(&denali_pci_driver);
 }
 
-- 
1.7.4.1

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

* [PATCHv2 2/6] nand/denali: annotate pci init/exit functions with correct section
  2011-06-03 10:32 [PATCHv2 0/6] Further Denali NAND enhancements Jamie Iles
  2011-06-03 10:32 ` [PATCHv2 1/6] nand/denali: convert to dev_() printk helpers Jamie Iles
@ 2011-06-03 10:32 ` Jamie Iles
  2011-06-06 13:16   ` Artem Bityutskiy
  2011-06-03 10:32 ` [PATCHv2 3/6] nand/denali: allow the number of ECC bits to be set by pdata Jamie Iles
                   ` (3 subsequent siblings)
  5 siblings, 1 reply; 13+ messages in thread
From: Jamie Iles @ 2011-06-03 10:32 UTC (permalink / raw)
  To: linux-mtd; +Cc: Jamie Iles, Artem Bityutskiy, David Woodhouse, Chuanxiao Dong

The module init exit functions should be annotated with __init and
__exit.

Cc: David Woodhouse <dwmw2@infradead.org>
Cc: Chuanxiao Dong <chuanxiao.dong@intel.com>
Cc: Artem Bityutskiy <dedekind1@gmail.com>
Signed-off-by: Jamie Iles <jamie@jamieiles.com>
---
 drivers/mtd/nand/denali.c |    4 ++--
 1 files changed, 2 insertions(+), 2 deletions(-)

diff --git a/drivers/mtd/nand/denali.c b/drivers/mtd/nand/denali.c
index a4b4a29..491458a 100644
--- a/drivers/mtd/nand/denali.c
+++ b/drivers/mtd/nand/denali.c
@@ -1686,14 +1686,14 @@ static struct pci_driver denali_pci_driver = {
 	.remove = denali_pci_remove,
 };
 
-static int __devinit denali_init(void)
+static int __init denali_init(void)
 {
 	pr_info(KERN_INFO "Spectra MTD driver\n");
 	return pci_register_driver(&denali_pci_driver);
 }
 
 /* Free memory */
-static void __devexit denali_exit(void)
+static void __exit denali_exit(void)
 {
 	pci_unregister_driver(&denali_pci_driver);
 }
-- 
1.7.4.1

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

* [PATCHv2 3/6] nand/denali: allow the number of ECC bits to be set by pdata
  2011-06-03 10:32 [PATCHv2 0/6] Further Denali NAND enhancements Jamie Iles
  2011-06-03 10:32 ` [PATCHv2 1/6] nand/denali: convert to dev_() printk helpers Jamie Iles
  2011-06-03 10:32 ` [PATCHv2 2/6] nand/denali: annotate pci init/exit functions with correct section Jamie Iles
@ 2011-06-03 10:32 ` Jamie Iles
  2011-06-03 10:32 ` [PATCHv2 4/6] nand/denali: support hardware with internal ECC fixup Jamie Iles
                   ` (2 subsequent siblings)
  5 siblings, 0 replies; 13+ messages in thread
From: Jamie Iles @ 2011-06-03 10:32 UTC (permalink / raw)
  To: linux-mtd; +Cc: Jamie Iles, Artem Bityutskiy, David Woodhouse, Chuanxiao Dong

Rather than having the number of ECC bits to be used set by a
preprocessor definition, allow it to be set by platform_data.  If there
is no platform_data then default to 8 bit ECC.

Changes since v1: fixup compile error introduced from a bad rebase.

Cc: David Woodhouse <dwmw2@infradead.org>
Cc: Chuanxiao Dong <chuanxiao.dong@intel.com>
Cc: Artem Bityutskiy <dedekind1@gmail.com>
Signed-off-by: Jamie Iles <jamie@jamieiles.com>
---
 drivers/mtd/nand/denali.c            |   24 ++++++++++++------------
 drivers/mtd/nand/denali.h            |    1 +
 include/linux/platform_data/denali.h |   21 +++++++++++++++++++++
 3 files changed, 34 insertions(+), 12 deletions(-)
 create mode 100644 include/linux/platform_data/denali.h

diff --git a/drivers/mtd/nand/denali.c b/drivers/mtd/nand/denali.c
index 491458a..5557d7b 100644
--- a/drivers/mtd/nand/denali.c
+++ b/drivers/mtd/nand/denali.c
@@ -26,6 +26,7 @@
 #include <linux/pci.h>
 #include <linux/mtd/mtd.h>
 #include <linux/module.h>
+#include <linux/platform_data/denali.h>
 
 #include "denali.h"
 
@@ -60,8 +61,6 @@ MODULE_PARM_DESC(onfi_timing_mode, "Overrides default ONFI setting."
  * valid or not */
 #define CHIP_SELECT_INVALID	-1
 
-#define SUPPORT_8BITECC		1
-
 /* This macro divides two integers and rounds fractional values up
  * to the nearest integer value. */
 #define CEIL_DIV(X, Y) (((X)%(Y)) ? ((X)/(Y)+1) : ((X)/(Y)))
@@ -357,11 +356,8 @@ static void get_toshiba_nand_para(struct denali_nand_info *denali)
 			ioread32(denali->flash_reg + DEVICE_SPARE_AREA_SIZE);
 		iowrite32(tmp,
 				denali->flash_reg + LOGICAL_PAGE_SPARE_SIZE);
-#if SUPPORT_15BITECC
-		iowrite32(15, denali->flash_reg + ECC_CORRECTION);
-#elif SUPPORT_8BITECC
-		iowrite32(8, denali->flash_reg + ECC_CORRECTION);
-#endif
+		iowrite32(denali->nr_ecc_bits,
+			  denali->flash_reg + ECC_CORRECTION);
 	}
 }
 
@@ -385,11 +381,8 @@ static void get_hynix_nand_para(struct denali_nand_info *denali,
 		iowrite32(spare_size,
 				denali->flash_reg + LOGICAL_PAGE_SPARE_SIZE);
 		iowrite32(0, denali->flash_reg + DEVICE_WIDTH);
-#if SUPPORT_15BITECC
-		iowrite32(15, denali->flash_reg + ECC_CORRECTION);
-#elif SUPPORT_8BITECC
-		iowrite32(8, denali->flash_reg + ECC_CORRECTION);
-#endif
+		iowrite32(denali->nr_ecc_bits,
+			  denali->flash_reg + ECC_CORRECTION);
 		break;
 	default:
 		dev_warn(denali->dev,
@@ -1414,6 +1407,7 @@ static int denali_pci_probe(struct pci_dev *dev, const struct pci_device_id *id)
 	resource_size_t csr_base, mem_base;
 	unsigned long csr_len, mem_len;
 	struct denali_nand_info *denali;
+	struct denali_nand_pdata *pdata;
 
 	denali = kzalloc(sizeof(*denali), GFP_KERNEL);
 	if (!denali)
@@ -1421,6 +1415,12 @@ static int denali_pci_probe(struct pci_dev *dev, const struct pci_device_id *id)
 
 	denali->dev = &dev->dev;
 
+	pdata = dev_get_platdata(denali->dev);
+	if (pdata && pdata->nr_ecc_bits > 8)
+		denali->nr_ecc_bits = pdata->nr_ecc_bits;
+	else
+		denali->nr_ecc_bits = 8;
+
 	ret = pci_enable_device(dev);
 	if (ret) {
 		dev_err(denali->dev, "Spectra: pci_enable_device failed.\n");
diff --git a/drivers/mtd/nand/denali.h b/drivers/mtd/nand/denali.h
index fabb9d5..b428ce3 100644
--- a/drivers/mtd/nand/denali.h
+++ b/drivers/mtd/nand/denali.h
@@ -494,6 +494,7 @@ struct denali_nand_info {
 	uint32_t blksperchip;
 	uint32_t bbtskipbytes;
 	uint32_t max_banks;
+	int nr_ecc_bits;
 };
 
 #endif /*_LLD_NAND_*/
diff --git a/include/linux/platform_data/denali.h b/include/linux/platform_data/denali.h
new file mode 100644
index 0000000..cfdb775
--- /dev/null
+++ b/include/linux/platform_data/denali.h
@@ -0,0 +1,21 @@
+/*
+ * NAND flash controller device driver platform data.
+ * Copyright © 2011, Picochip
+ *
+ * This program is free software; you can redistribute it and/or modify it
+ * under the terms and conditions of the GNU General Public License,
+ * version 2, as published by the Free Software Foundation.
+ *
+ * This program is distributed in the hope it will be useful, but WITHOUT
+ * ANY WARRANTY; without even the implied warranty of MERCHANTABILITY or
+ * FITNESS FOR A PARTICULAR PURPOSE.  See the GNU General Public License for
+ * more details.
+ */
+#ifndef __DENALI_PDATA_H__
+#define __DENALI_PDATA_H__
+
+struct denali_nand_pdata {
+	int	nr_ecc_bits;
+};
+
+#endif /* __DENALI_PDATA_H__ */
-- 
1.7.4.1

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

* [PATCHv2 4/6] nand/denali: support hardware with internal ECC fixup
  2011-06-03 10:32 [PATCHv2 0/6] Further Denali NAND enhancements Jamie Iles
                   ` (2 preceding siblings ...)
  2011-06-03 10:32 ` [PATCHv2 3/6] nand/denali: allow the number of ECC bits to be set by pdata Jamie Iles
@ 2011-06-03 10:32 ` Jamie Iles
  2011-06-03 10:32 ` [PATCHv2 5/6] nand/denali: support MTD partitioning Jamie Iles
  2011-06-03 10:32 ` [PATCHv2 6/6] mtd/denali: support for cmdline partitioning Jamie Iles
  5 siblings, 0 replies; 13+ messages in thread
From: Jamie Iles @ 2011-06-03 10:32 UTC (permalink / raw)
  To: linux-mtd; +Cc: Jamie Iles, Artem Bityutskiy, David Woodhouse, Chuanxiao Dong

Some versions of the IP have the ECC fixup handled in hardware.  For
these devices we have a new interrupt status/enable mapping that tells
us when we have an uncorrectable error.  Unfortunately this replaces
existing interrupt bits and there is no way to probe for this capability
so we have to do it through platform data.

Cc: David Woodhouse <dwmw2@infradead.org>
Cc: Chuanxiao Dong <chuanxiao.dong@intel.com>
Cc: Artem Bityutskiy <dedekind1@gmail.com>
Signed-off-by: Jamie Iles <jamie@jamieiles.com>
---
 drivers/mtd/nand/denali.c            |   18 +++++++++++++++---
 drivers/mtd/nand/denali.h            |    9 +++++++++
 include/linux/platform_data/denali.h |    1 +
 3 files changed, 25 insertions(+), 3 deletions(-)

diff --git a/drivers/mtd/nand/denali.c b/drivers/mtd/nand/denali.c
index 5557d7b..229696b 100644
--- a/drivers/mtd/nand/denali.c
+++ b/drivers/mtd/nand/denali.c
@@ -55,7 +55,8 @@ MODULE_PARM_DESC(onfi_timing_mode, "Overrides default ONFI setting."
 			INTR_STATUS__TIME_OUT | \
 			INTR_STATUS__ERASE_FAIL | \
 			INTR_STATUS__RST_COMP | \
-			INTR_STATUS__ERASE_COMP)
+			INTR_STATUS__ERASE_COMP | \
+			INTR_STATUS__ECC_UNCOR_ERR)
 
 /* indicates whether or not the internal value for the flash bank is
  * valid or not */
@@ -913,6 +914,14 @@ static bool handle_ecc(struct denali_nand_info *denali, uint8_t *buf,
 {
 	bool check_erased_page = false;
 
+	if (denali->have_hw_ecc_fixup &&
+	    (irq_status & INTR_STATUS__ECC_UNCOR_ERR)) {
+		clear_interrupts(denali);
+		denali_set_intr_modes(denali, true);
+
+		return true;
+	}
+
 	if (irq_status & INTR_STATUS__ECC_ERR) {
 		/* read the ECC errors. we'll ignore them for now */
 		uint32_t err_address = 0, err_correction_info = 0;
@@ -1111,8 +1120,9 @@ static int denali_read_page(struct mtd_info *mtd, struct nand_chip *chip,
 	size_t size = denali->mtd.writesize + denali->mtd.oobsize;
 
 	uint32_t irq_status = 0;
-	uint32_t irq_mask = INTR_STATUS__ECC_TRANSACTION_DONE |
-			    INTR_STATUS__ECC_ERR;
+	uint32_t irq_mask = denali->have_hw_ecc_fixup ?
+		(INTR_STATUS__ECC_TRANSACTION_DONE | INTR_STATUS__ECC_ERR) :
+		(INTR_STATUS__DMA_CMD_COMP | INTR_STATUS__ECC_UNCOR_ERR);
 	bool check_erased_page = false;
 
 	if (page != denali->page) {
@@ -1427,6 +1437,8 @@ static int denali_pci_probe(struct pci_dev *dev, const struct pci_device_id *id)
 		goto failed_alloc_memery;
 	}
 
+	denali->have_hw_ecc_fixup = pdata ? pdata->have_hw_ecc_fixup : false;
+
 	if (id->driver_data == INTEL_CE4100) {
 		/* Due to a silicon limitation, we can only support
 		 * ONFI timing mode 1 and below.
diff --git a/drivers/mtd/nand/denali.h b/drivers/mtd/nand/denali.h
index b428ce3..f0b5e91 100644
--- a/drivers/mtd/nand/denali.h
+++ b/drivers/mtd/nand/denali.h
@@ -231,6 +231,14 @@
 #define     INTR_STATUS__PIPE_CMD_ERR			0x4000
 #define     INTR_STATUS__PAGE_XFER_INC			0x8000
 
+/*
+ * Some versions of the IP have the ECC fixup handled in hardware.  In this
+ * configuration we only get interrupted when the error is uncorrectable.
+ * Unfortunately this bit replaces INTR_STATUS__ECC_TRANSACTION_DONE from the
+ * old IP.
+ */
+#define     INTR_STATUS__ECC_UNCOR_ERR			0x0001
+
 #define     INTR_EN__ECC_TRANSACTION_DONE		0x0001
 #define     INTR_EN__ECC_ERR				0x0002
 #define     INTR_EN__DMA_CMD_COMP			0x0004
@@ -495,6 +503,7 @@ struct denali_nand_info {
 	uint32_t bbtskipbytes;
 	uint32_t max_banks;
 	int nr_ecc_bits;
+	bool have_hw_ecc_fixup;
 };
 
 #endif /*_LLD_NAND_*/
diff --git a/include/linux/platform_data/denali.h b/include/linux/platform_data/denali.h
index cfdb775..3767333 100644
--- a/include/linux/platform_data/denali.h
+++ b/include/linux/platform_data/denali.h
@@ -16,6 +16,7 @@
 
 struct denali_nand_pdata {
 	int	nr_ecc_bits;
+	bool	have_hw_ecc_fixup;
 };
 
 #endif /* __DENALI_PDATA_H__ */
-- 
1.7.4.1

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

* [PATCHv2 5/6] nand/denali: support MTD partitioning
  2011-06-03 10:32 [PATCHv2 0/6] Further Denali NAND enhancements Jamie Iles
                   ` (3 preceding siblings ...)
  2011-06-03 10:32 ` [PATCHv2 4/6] nand/denali: support hardware with internal ECC fixup Jamie Iles
@ 2011-06-03 10:32 ` Jamie Iles
  2011-06-03 10:32 ` [PATCHv2 6/6] mtd/denali: support for cmdline partitioning Jamie Iles
  5 siblings, 0 replies; 13+ messages in thread
From: Jamie Iles @ 2011-06-03 10:32 UTC (permalink / raw)
  To: linux-mtd; +Cc: Jamie Iles, Artem Bityutskiy, David Woodhouse, Chuanxiao Dong

Allow the platform to specify partitions through platform data.  If
there are no partitions specified then the whole device will be
registered.

Cc: David Woodhouse <dwmw2@infradead.org>
Cc: Chuanxiao Dong <chuanxiao.dong@intel.com>
Cc: Artem Bityutskiy <dedekind1@gmail.com>
Signed-off-by: Jamie Iles <jamie@jamieiles.com>
---
 drivers/mtd/nand/denali.c            |    8 +++++++-
 include/linux/platform_data/denali.h |    8 ++++++--
 2 files changed, 13 insertions(+), 3 deletions(-)

diff --git a/drivers/mtd/nand/denali.c b/drivers/mtd/nand/denali.c
index 229696b..e24e214 100644
--- a/drivers/mtd/nand/denali.c
+++ b/drivers/mtd/nand/denali.c
@@ -25,6 +25,7 @@
 #include <linux/slab.h>
 #include <linux/pci.h>
 #include <linux/mtd/mtd.h>
+#include <linux/mtd/partitions.h>
 #include <linux/module.h>
 #include <linux/platform_data/denali.h>
 
@@ -1643,7 +1644,12 @@ static int denali_pci_probe(struct pci_dev *dev, const struct pci_device_id *id)
 		goto failed_req_irq;
 	}
 
-	ret = mtd_device_register(&denali->mtd, NULL, 0);
+	if (pdata && pdata->parts)
+		ret = mtd_device_register(&denali->mtd, pdata->parts,
+					 pdata->nr_parts);
+	else
+		ret = mtd_device_register(&denali->mtd, NULL, 0);
+
 	if (ret) {
 		dev_err(denali->dev, "Spectra: Failed to register MTD: %d\n",
 				ret);
diff --git a/include/linux/platform_data/denali.h b/include/linux/platform_data/denali.h
index 3767333..141da35 100644
--- a/include/linux/platform_data/denali.h
+++ b/include/linux/platform_data/denali.h
@@ -14,9 +14,13 @@
 #ifndef __DENALI_PDATA_H__
 #define __DENALI_PDATA_H__
 
+struct mtd_partition;
+
 struct denali_nand_pdata {
-	int	nr_ecc_bits;
-	bool	have_hw_ecc_fixup;
+	int				nr_ecc_bits;
+	bool				have_hw_ecc_fixup;
+	const struct mtd_partition	*parts;
+	unsigned int			nr_parts;
 };
 
 #endif /* __DENALI_PDATA_H__ */
-- 
1.7.4.1

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

* [PATCHv2 6/6] mtd/denali: support for cmdline partitioning
  2011-06-03 10:32 [PATCHv2 0/6] Further Denali NAND enhancements Jamie Iles
                   ` (4 preceding siblings ...)
  2011-06-03 10:32 ` [PATCHv2 5/6] nand/denali: support MTD partitioning Jamie Iles
@ 2011-06-03 10:32 ` Jamie Iles
  2011-06-06 13:18   ` Artem Bityutskiy
  5 siblings, 1 reply; 13+ messages in thread
From: Jamie Iles @ 2011-06-03 10:32 UTC (permalink / raw)
  To: linux-mtd; +Cc: Jamie Iles, Artem Bityutskiy, David Woodhouse, Chuanxiao Dong

Add support for command line partitioning.  This is favoured over
platform data based partitions.

Cc: David Woodhouse <dwmw2@infradead.org>
Cc: Chuanxiao Dong <chuanxiao.dong@intel.com>
Cc: Artem Bityutskiy <dedekind1@gmail.com>
Signed-off-by: Jamie Iles <jamie@jamieiles.com>
---
 drivers/mtd/nand/denali.c |    9 ++++++++-
 1 files changed, 8 insertions(+), 1 deletions(-)

diff --git a/drivers/mtd/nand/denali.c b/drivers/mtd/nand/denali.c
index e24e214..c48f5dd 100644
--- a/drivers/mtd/nand/denali.c
+++ b/drivers/mtd/nand/denali.c
@@ -1419,6 +1419,9 @@ static int denali_pci_probe(struct pci_dev *dev, const struct pci_device_id *id)
 	unsigned long csr_len, mem_len;
 	struct denali_nand_info *denali;
 	struct denali_nand_pdata *pdata;
+	struct mtd_partition *partitions = NULL;
+	int nr_parts = 0;
+	const char *part_probes[] = { "cmdlinepart", NULL };
 
 	denali = kzalloc(sizeof(*denali), GFP_KERNEL);
 	if (!denali)
@@ -1644,7 +1647,11 @@ static int denali_pci_probe(struct pci_dev *dev, const struct pci_device_id *id)
 		goto failed_req_irq;
 	}
 
-	if (pdata && pdata->parts)
+	nr_parts = parse_mtd_partitions(&denali->mtd, part_probes,
+					&partitions, 0);
+	if (partitions && nr_parts > 0)
+		ret = mtd_device_register(&denali->mtd, partitions, nr_parts);
+	else if (pdata && pdata->parts)
 		ret = mtd_device_register(&denali->mtd, pdata->parts,
 					 pdata->nr_parts);
 	else
-- 
1.7.4.1

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

* Re: [PATCHv2 1/6] nand/denali: convert to dev_() printk helpers
  2011-06-03 10:32 ` [PATCHv2 1/6] nand/denali: convert to dev_() printk helpers Jamie Iles
@ 2011-06-06 13:11   ` Artem Bityutskiy
  2011-06-06 13:22     ` Jamie Iles
  2011-06-06 13:51     ` Jamie Iles
  0 siblings, 2 replies; 13+ messages in thread
From: Artem Bityutskiy @ 2011-06-06 13:11 UTC (permalink / raw)
  To: Jamie Iles; +Cc: David Woodhouse, linux-mtd, Chuanxiao Dong

On Fri, 2011-06-03 at 11:32 +0100, Jamie Iles wrote:
>  	if ((data_invalid - acc_clks * CLK_X) < 2)
> -		dev_warn(denali->dev, "%s, Line %d: Warning!\n",
> -			__FILE__, __LINE__);
> +		dev_warn(denali->dev, "%s, Line %d: Warning!\n", __FILE__,
> +			 __LINE__);

Do you really need __FILE__ and __LINE__ here? And may be you could also
change the warning message? Just printing "warning!" is a bit silly.

>  
>  	addr_2_data = CEIL_DIV(Tadl[mode], CLK_X);
>  	re_2_we = CEIL_DIV(Trhw[mode], CLK_X);
> @@ -394,9 +393,8 @@ static void get_hynix_nand_para(struct denali_nand_info *denali,
>  		break;
>  	default:
>  		dev_warn(denali->dev,
> -			"Spectra: Unknown Hynix NAND (Device ID: 0x%x)."
> -			"Will use default parameter values instead.\n",
> -			device_id);
> +			 "unknown Hynix NAND (Device ID: 0x%x). Will use default parameter values instead.\n",
> +			 device_id);

Would also be nice to remove the final dot while on it, as
Documentation/CodingStyle suggests in "Chapter 13: Printing kernel
messages".

> @@ -415,8 +413,8 @@ static void find_valid_banks(struct denali_nand_info *denali)
>  		index_addr_read_data(denali,
>  				(uint32_t)(MODE_11 | (i << 24) | 2), &id[i]);
>  
> -		dev_dbg(denali->dev,
> -			"Return 1st ID for bank[%d]: %x\n", i, id[i]);
> +		dev_dbg(denali->dev, "Return 1st ID for bank[%d]: %x\n", i,
> +			id[i]);
>  
>  		if (i == 0) {
>  			if (!(id[i] & 0x0ff))
> @@ -436,13 +434,12 @@ static void find_valid_banks(struct denali_nand_info *denali)
>  		 */
>  		if (denali->total_used_banks != 1) {
>  			dev_err(denali->dev,
> -					"Sorry, Intel CE4100 only supports "
> -					"a single NAND device.\n");
> +				"Sorry, Intel CE4100 only supports a single NAND device.\n");
>  			BUG();

Just a suggestion for a separate patch - doing BUG() is really bad
practice - it is better to return and error up. But this is not related
to this patch...

>  	dev_info(denali->dev,
> -			"Dump timing register values:"
> -			"acc_clks: %d, re_2_we: %d, re_2_re: %d\n"
> -			"we_2_re: %d, addr_2_data: %d, rdwr_en_lo_cnt: %d\n"
> -			"rdwr_en_hi_cnt: %d, cs_setup_cnt: %d\n",
> -			ioread32(denali->flash_reg + ACC_CLKS),
> -			ioread32(denali->flash_reg + RE_2_WE),
> -			ioread32(denali->flash_reg + RE_2_RE),
> -			ioread32(denali->flash_reg + WE_2_RE),
> -			ioread32(denali->flash_reg + ADDR_2_DATA),
> -			ioread32(denali->flash_reg + RDWR_EN_LO_CNT),
> -			ioread32(denali->flash_reg + RDWR_EN_HI_CNT),
> -			ioread32(denali->flash_reg + CS_SETUP_CNT));
> +		 "Dump timing register values: acc_clks: %d, re_2_we: %d, re_2_re: %d\n"
> +		 "we_2_re: %d, addr_2_data: %d, rdwr_en_lo_cnt: %d\n"
> +		 "rdwr_en_hi_cnt: %d, cs_setup_cnt: %d\n",
> +		 ioread32(denali->flash_reg + ACC_CLKS),
> +		 ioread32(denali->flash_reg + RE_2_WE),
> +		 ioread32(denali->flash_reg + RE_2_RE),
> +		 ioread32(denali->flash_reg + WE_2_RE),
> +		 ioread32(denali->flash_reg + ADDR_2_DATA),
> +		 ioread32(denali->flash_reg + RDWR_EN_LO_CNT),
> +		 ioread32(denali->flash_reg + RDWR_EN_HI_CNT),
> +		 ioread32(denali->flash_reg + CS_SETUP_CNT));

I really doubt (but not 100% sure) that multiple \n are handled
correctly. In the kernel all messages have to be prefixed with the
"print level", and the lines after \n will probably not have it. This is
why we have this "KERN_CONT" thing. I suggest you to re-write this print
and use multiple 'dev_info()' calls.

And BTW, why 'dev_info()'? I think it will not be compiled out when
debugging is disabled, and the above info does look like debugging
stuff, so it begs for 'dev_dbg()'.

>  static uint32_t wait_for_irq(struct denali_nand_info *denali, uint32_t irq_mask)
>  {
> @@ -699,8 +691,9 @@ static uint32_t wait_for_irq(struct denali_nand_info *denali, uint32_t irq_mask)
>  
>  	if (comp_res == 0) {
>  		/* timeout */
> -		printk(KERN_ERR "timeout occurred, status = 0x%x, mask = 0x%x\n",
> -				intr_status, irq_mask);
> +		dev_err(&denali->mtd.dev,
> +			"timeout occurred, status = 0x%x, mask = 0x%x\n",
> +			intr_status, irq_mask);

Could this fit 2 lines instead?

>  
>  		intr_status = 0;
>  	}
> @@ -785,9 +778,8 @@ static int denali_send_pipeline_cmd(struct denali_nand_info *denali,
>  
>  			if (irq_status == 0) {
>  				dev_err(denali->dev,
> -						"cmd, page, addr on timeout "
> -						"(0x%x, 0x%x, 0x%x)\n",
> -						cmd, denali->page, addr);
> +					"cmd, page, addr on timeout (0x%x, 0x%x, 0x%x)\n",
> +					cmd, denali->page, addr);
>  				status = FAIL;
>  			} else {
>  				cmd = MODE_01 | addr;
> @@ -889,7 +881,7 @@ static void read_oob_data(struct mtd_info *mtd, uint8_t *buf, int page)
>  
>  		if (irq_status == 0)
>  			dev_err(denali->dev, "page on OOB timeout %d\n",
> -					denali->page);
> +				denali->page);

Did not check, but looks like this can be onelinere, it does not fit 80
lines?

>  
>  		/* We set the device back to MAIN_ACCESS here as I observed
>  		 * instability with the controller if you do a block erase
> @@ -1065,9 +1057,8 @@ static void write_page(struct mtd_info *mtd, struct nand_chip *chip,
>  	irq_status = wait_for_irq(denali, irq_mask);
>  
>  	if (irq_status == 0) {
> -		dev_err(denali->dev,
> -				"timeout on write_page (type = %d)\n",
> -				raw_xfer);
> +		dev_err(denali->dev, "timeout on write_page (type = %d)\n",
> +			raw_xfer);

And this looks like it could be a one-liner ... Could you please check
other messages WRT fitting less lines.

>  		denali->status =
>  			(irq_status & INTR_STATUS__PROGRAM_FAIL) ?
>  			NAND_STATUS_FAIL : PASS;
> @@ -1132,9 +1123,9 @@ static int denali_read_page(struct mtd_info *mtd, struct nand_chip *chip,
>  	bool check_erased_page = false;
>  
>  	if (page != denali->page) {
> -		dev_err(denali->dev, "IN %s: page %d is not"
> -				" equal to denali->page %d, investigate!!",
> -				__func__, page, denali->page);
> +		dev_err(denali->dev,
> +			"IN %s: page %d is not equal to denali->page %d, investigate!!",
> +			__func__, page, denali->page);

I think this "IN" is not needed, and "investigate!!" sounds
"interesting" ... Not really sure we need __func__ here - it just blows
the code size, no?

>  		BUG();

Similar side-note about this BUG() statement...

>  	}
>  
> @@ -1182,9 +1173,8 @@ static int denali_read_page_raw(struct mtd_info *mtd, struct nand_chip *chip,
>  	uint32_t irq_mask = INTR_STATUS__DMA_CMD_COMP;
>  
>  	if (page != denali->page) {
> -		dev_err(denali->dev, "IN %s: page %d is not"
> -				" equal to denali->page %d, investigate!!",
> -				__func__, page, denali->page);
> +		dev_err(denali->dev, "IN %s: page %d is not equal to denali->page %d, investigate!!",
> +			__func__, page, denali->page);

Ditto.

>  	denali->flash_mem = ioremap_nocache(mem_base, mem_len);
>  	if (!denali->flash_mem) {
> -		printk(KERN_ERR "Spectra: ioremap_nocache failed!");
> +		dev_err(denali->dev, "Spectra: ioremap_nocache failed!");

For consistency with other messages, I'd removed the exclamation mark
here, in on some other messages.

> -		dev_err(&dev->dev, "Spectra: Failed to register MTD: %d\n",
> +		dev_err(denali->dev, "Spectra: Failed to register MTD: %d\n",
>  				ret);
One line?

>  		goto failed_req_irq;
>  	}
> @@ -1701,7 +1688,7 @@ static struct pci_driver denali_pci_driver = {
>  
>  static int __devinit denali_init(void)
>  {
> -	printk(KERN_INFO "Spectra MTD driver\n");
> +	pr_info(KERN_INFO "Spectra MTD driver\n");

Remove KERN_INFO please.

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

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

* Re: [PATCHv2 2/6] nand/denali: annotate pci init/exit functions with correct section
  2011-06-03 10:32 ` [PATCHv2 2/6] nand/denali: annotate pci init/exit functions with correct section Jamie Iles
@ 2011-06-06 13:16   ` Artem Bityutskiy
  0 siblings, 0 replies; 13+ messages in thread
From: Artem Bityutskiy @ 2011-06-06 13:16 UTC (permalink / raw)
  To: Jamie Iles; +Cc: David Woodhouse, linux-mtd, Chuanxiao Dong

On Fri, 2011-06-03 at 11:32 +0100, Jamie Iles wrote:
> The module init exit functions should be annotated with __init and
> __exit.
> 
> Cc: David Woodhouse <dwmw2@infradead.org>
> Cc: Chuanxiao Dong <chuanxiao.dong@intel.com>
> Cc: Artem Bityutskiy <dedekind1@gmail.com>
> Signed-off-by: Jamie Iles <jamie@jamieiles.com>

Would you please use "mtd: denali:" prefix?

I cannot apply this patch because it depends on the previous patch. You
could re-structure the code and make simpler patches like this go first,
then they could be taken quicker.

Thanks!

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

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

* Re: [PATCHv2 6/6] mtd/denali: support for cmdline partitioning
  2011-06-03 10:32 ` [PATCHv2 6/6] mtd/denali: support for cmdline partitioning Jamie Iles
@ 2011-06-06 13:18   ` Artem Bityutskiy
  0 siblings, 0 replies; 13+ messages in thread
From: Artem Bityutskiy @ 2011-06-06 13:18 UTC (permalink / raw)
  To: Jamie Iles; +Cc: David Woodhouse, linux-mtd, Chuanxiao Dong

On Fri, 2011-06-03 at 11:32 +0100, Jamie Iles wrote:
> Add support for command line partitioning.  This is favoured over
> platform data based partitions.
> 
> Cc: David Woodhouse <dwmw2@infradead.org>
> Cc: Chuanxiao Dong <chuanxiao.dong@intel.com>
> Cc: Artem Bityutskiy <dedekind1@gmail.com>
> Signed-off-by: Jamie Iles <jamie@jamieiles.com>
> ---
>  drivers/mtd/nand/denali.c |    9 ++++++++-
>  1 files changed, 8 insertions(+), 1 deletions(-)

I've just taken a set of patches from "Dmitry Eremin-Solenikov
<dbaryshkov@gmail.com>" which simplify the way to use just cmdlinepart
parser, please, take a look - it is in the l2 tree.

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

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

* Re: [PATCHv2 1/6] nand/denali: convert to dev_() printk helpers
  2011-06-06 13:11   ` Artem Bityutskiy
@ 2011-06-06 13:22     ` Jamie Iles
  2011-06-06 13:51     ` Jamie Iles
  1 sibling, 0 replies; 13+ messages in thread
From: Jamie Iles @ 2011-06-06 13:22 UTC (permalink / raw)
  To: Artem Bityutskiy; +Cc: Jamie Iles, linux-mtd, David Woodhouse, Chuanxiao Dong

On Mon, Jun 06, 2011 at 04:11:55PM +0300, Artem Bityutskiy wrote:
> On Fri, 2011-06-03 at 11:32 +0100, Jamie Iles wrote:
> >  	if ((data_invalid - acc_clks * CLK_X) < 2)
> > -		dev_warn(denali->dev, "%s, Line %d: Warning!\n",
> > -			__FILE__, __LINE__);
> > +		dev_warn(denali->dev, "%s, Line %d: Warning!\n", __FILE__,
> > +			 __LINE__);
> 
> Do you really need __FILE__ and __LINE__ here? And may be you could also
> change the warning message? Just printing "warning!" is a bit silly.

OK, fair point (and the rest of your comments).  I'll respin this and 
try to rationalise the messages at the same time.

Jamie

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

* Re: [PATCHv2 1/6] nand/denali: convert to dev_() printk helpers
  2011-06-06 13:11   ` Artem Bityutskiy
  2011-06-06 13:22     ` Jamie Iles
@ 2011-06-06 13:51     ` Jamie Iles
  2011-06-06 15:35       ` Artem Bityutskiy
  1 sibling, 1 reply; 13+ messages in thread
From: Jamie Iles @ 2011-06-06 13:51 UTC (permalink / raw)
  To: Artem Bityutskiy, Chuanxiao Dong; +Cc: Jamie Iles, linux-mtd, David Woodhouse

On Mon, Jun 06, 2011 at 04:11:55PM +0300, Artem Bityutskiy wrote:
> On Fri, 2011-06-03 at 11:32 +0100, Jamie Iles wrote:
> >  	if ((data_invalid - acc_clks * CLK_X) < 2)
> > -		dev_warn(denali->dev, "%s, Line %d: Warning!\n",
> > -			__FILE__, __LINE__);
> > +		dev_warn(denali->dev, "%s, Line %d: Warning!\n", __FILE__,
> > +			 __LINE__);
> 
> Do you really need __FILE__ and __LINE__ here? And may be you could also
> change the warning message? Just printing "warning!" is a bit silly.

I'm afraid I can't work out from the Denali documentation that we have 
what this is actually warning about.  Chuanxiao, is there a better error 
message we can print here that can help users to debug it?  Is this just 
an unsupported ONFI timing mode or something more sinister?

Jamie

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

* Re: [PATCHv2 1/6] nand/denali: convert to dev_() printk helpers
  2011-06-06 13:51     ` Jamie Iles
@ 2011-06-06 15:35       ` Artem Bityutskiy
  0 siblings, 0 replies; 13+ messages in thread
From: Artem Bityutskiy @ 2011-06-06 15:35 UTC (permalink / raw)
  To: Jamie Iles; +Cc: David Woodhouse, linux-mtd, Chuanxiao Dong

On Mon, 2011-06-06 at 14:51 +0100, Jamie Iles wrote:
> On Mon, Jun 06, 2011 at 04:11:55PM +0300, Artem Bityutskiy wrote:
> > On Fri, 2011-06-03 at 11:32 +0100, Jamie Iles wrote:
> > >  	if ((data_invalid - acc_clks * CLK_X) < 2)
> > > -		dev_warn(denali->dev, "%s, Line %d: Warning!\n",
> > > -			__FILE__, __LINE__);
> > > +		dev_warn(denali->dev, "%s, Line %d: Warning!\n", __FILE__,
> > > +			 __LINE__);
> > 
> > Do you really need __FILE__ and __LINE__ here? And may be you could also
> > change the warning message? Just printing "warning!" is a bit silly.
> 
> I'm afraid I can't work out from the Denali documentation that we have 
> what this is actually warning about.  Chuanxiao, is there a better error 
> message we can print here that can help users to debug it?  Is this just 
> an unsupported ONFI timing mode or something more sinister?

Well, we could kill it or change with "WARN_ON()" may be? Better to het
a comment from Chuanxiao of course ...

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

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

end of thread, other threads:[~2011-06-06 15:39 UTC | newest]

Thread overview: 13+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2011-06-03 10:32 [PATCHv2 0/6] Further Denali NAND enhancements Jamie Iles
2011-06-03 10:32 ` [PATCHv2 1/6] nand/denali: convert to dev_() printk helpers Jamie Iles
2011-06-06 13:11   ` Artem Bityutskiy
2011-06-06 13:22     ` Jamie Iles
2011-06-06 13:51     ` Jamie Iles
2011-06-06 15:35       ` Artem Bityutskiy
2011-06-03 10:32 ` [PATCHv2 2/6] nand/denali: annotate pci init/exit functions with correct section Jamie Iles
2011-06-06 13:16   ` Artem Bityutskiy
2011-06-03 10:32 ` [PATCHv2 3/6] nand/denali: allow the number of ECC bits to be set by pdata Jamie Iles
2011-06-03 10:32 ` [PATCHv2 4/6] nand/denali: support hardware with internal ECC fixup Jamie Iles
2011-06-03 10:32 ` [PATCHv2 5/6] nand/denali: support MTD partitioning Jamie Iles
2011-06-03 10:32 ` [PATCHv2 6/6] mtd/denali: support for cmdline partitioning Jamie Iles
2011-06-06 13:18   ` Artem Bityutskiy

This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.