linux-kernel.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
* [PATCH v7 00/16] mtd: nand: denali: Denali NAND IP improvements
@ 2017-06-13 13:45 Masahiro Yamada
  2017-06-13 13:45 ` [PATCH v7 01/16] mtd: nand: denali: set NAND_ECC_CUSTOM_PAGE_ACCESS Masahiro Yamada
                   ` (16 more replies)
  0 siblings, 17 replies; 19+ messages in thread
From: Masahiro Yamada @ 2017-06-13 13:45 UTC (permalink / raw)
  To: linux-mtd
  Cc: Enrico Jorns, Artem Bityutskiy, Dinh Nguyen, Boris Brezillon,
	Marek Vasut, David Woodhouse, Masami Hiramatsu, Chuanxiao Dong,
	Jassi Brar, Masahiro Yamada, Cyrille Pitchen, linux-kernel,
	Brian Norris, Richard Weinberger


This patch series intends to solve various problems.

[1] The driver just retrieves the OOB area as-is
    whereas the controller uses syndrome page layout.
[2] ONFi devices are not working
[3] It can not read Bad Block Marker

Outstanding changes are:
 - Fix raw/oob callbacks for syndrome page layout
 - Implement setup_data_interface() callback
 - Fix/implement more commands for ONFi devices
 - Allow to skip the driver internal bounce buffer
 - Support PIO in case DMA is not supported
 - Switch from ->cmdfunc over to ->cmd_ctrl

18 patches were merged by v2.
11 patches were merged by v3.
2 patches were merged by v4.
5 patches were merged by v5.
Here is the rest of the series.

v1: https://lkml.org/lkml/2016/11/26/144
v2: https://lkml.org/lkml/2017/3/22/804
v3: https://lkml.org/lkml/2017/3/30/90
v4: https://lkml.org/lkml/2017/6/5/1005


Masahiro Yamada (16):
  mtd: nand: denali: set NAND_ECC_CUSTOM_PAGE_ACCESS
  mtd: nand: denali: remove unneeded find_valid_banks()
  mtd: nand: denali: handle timing parameters by setup_data_interface()
  mtd: nand: denali: rework interrupt handling
  mtd: nand: denali: switch over to cmd_ctrl instead of cmdfunc
  mtd: nand: denali: fix bank reset function to detect the number of
    chips
  mtd: nand: denali: use interrupt instead of polling for bank reset
  mtd: nand: denali: propagate page to helpers via function argument
  mtd: nand: denali: merge struct nand_buf into struct denali_nand_info
  mtd: nand: denali: use flag instead of register macro for direction
  mtd: nand: denali: fix raw and oob accessors for syndrome page layout
  mtd: nand: denali: support hardware-assisted erased page detection
  mtd: nand: denali: skip driver internal bounce buffer when possible
  mtd: nand: denali: use non-managed kmalloc() for DMA buffer
  mtd: nand: denali: enable bad block table scan
  mtd: nand: denali: avoid magic numbers and rename for clarification

 drivers/mtd/nand/denali.c     | 1724 ++++++++++++++++++-----------------------
 drivers/mtd/nand/denali.h     |   63 +-
 drivers/mtd/nand/denali_dt.c  |   15 +-
 drivers/mtd/nand/denali_pci.c |   22 +-
 4 files changed, 806 insertions(+), 1018 deletions(-)

-- 
2.7.4

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

* [PATCH v7 01/16] mtd: nand: denali: set NAND_ECC_CUSTOM_PAGE_ACCESS
  2017-06-13 13:45 [PATCH v7 00/16] mtd: nand: denali: Denali NAND IP improvements Masahiro Yamada
@ 2017-06-13 13:45 ` Masahiro Yamada
  2017-06-13 13:45 ` [PATCH v7 02/16] mtd: nand: denali: remove unneeded find_valid_banks() Masahiro Yamada
                   ` (15 subsequent siblings)
  16 siblings, 0 replies; 19+ messages in thread
From: Masahiro Yamada @ 2017-06-13 13:45 UTC (permalink / raw)
  To: linux-mtd
  Cc: Enrico Jorns, Artem Bityutskiy, Dinh Nguyen, Boris Brezillon,
	Marek Vasut, David Woodhouse, Masami Hiramatsu, Chuanxiao Dong,
	Jassi Brar, Masahiro Yamada, Cyrille Pitchen, linux-kernel,
	Brian Norris, Richard Weinberger

The denali_cmdfunc() actually does nothing valuable for
NAND_CMD_{PAGEPROG,READ0,SEQIN}.

For NAND_CMD_{READ0,SEQIN}, it copies "page" to "denali->page", then
denali_read_page(_raw) compares them just for the sanity check.
(Inconsistently, this check is missing from denali_write_page(_raw).)

The Denali controller is equipped with high level read/write interface,
so let's skip unneeded call of cmdfunc().

If NAND_ECC_CUSTOM_PAGE_ACCESS is set, nand_write_page() will not
call ->waitfunc hook.  So, ->write_page(_raw) hooks should directly
return -EIO on failure.  The error handling of page writes will be
much simpler.

Signed-off-by: Masahiro Yamada <yamada.masahiro@socionext.com>
---

Changes in v7: None
Changes in v6:
  - Squash two commits to adjust to commit 41145649f4ac
    (mtd: nand: Wait for PAGEPROG to finish in drivers setting NAND_ECC_CUSTOM_PAGE_ACCESS)

Changes in v5: None
Changes in v4: None
Changes in v3: None
Changes in v2:
  - Newly added

 drivers/mtd/nand/denali.c | 41 ++++++++++++-----------------------------
 drivers/mtd/nand/denali.h |  1 -
 2 files changed, 12 insertions(+), 30 deletions(-)

diff --git a/drivers/mtd/nand/denali.c b/drivers/mtd/nand/denali.c
index 2b4618bb8d72..7133a33b4ad3 100644
--- a/drivers/mtd/nand/denali.c
+++ b/drivers/mtd/nand/denali.c
@@ -998,13 +998,16 @@ static void denali_setup_dma(struct denali_nand_info *denali, int op)
  * configuration details.
  */
 static int write_page(struct mtd_info *mtd, struct nand_chip *chip,
-			const uint8_t *buf, bool raw_xfer)
+			const uint8_t *buf, int page, bool raw_xfer)
 {
 	struct denali_nand_info *denali = mtd_to_denali(mtd);
 	dma_addr_t addr = denali->buf.dma_buf;
 	size_t size = mtd->writesize + mtd->oobsize;
 	uint32_t irq_status;
 	uint32_t irq_mask = INTR__DMA_CMD_COMP | INTR__PROGRAM_FAIL;
+	int ret = 0;
+
+	denali->page = page;
 
 	/*
 	 * if it is a raw xfer, we want to disable ecc and send the spare area.
@@ -1036,13 +1039,13 @@ static int write_page(struct mtd_info *mtd, struct nand_chip *chip,
 	if (irq_status == 0) {
 		dev_err(denali->dev, "timeout on write_page (type = %d)\n",
 			raw_xfer);
-		denali->status = NAND_STATUS_FAIL;
+		ret = -EIO;
 	}
 
 	denali_enable_dma(denali, false);
 	dma_sync_single_for_cpu(denali->dev, addr, size, DMA_TO_DEVICE);
 
-	return 0;
+	return ret;
 }
 
 /* NAND core entry points */
@@ -1059,7 +1062,7 @@ static int denali_write_page(struct mtd_info *mtd, struct nand_chip *chip,
 	 * for regular page writes, we let HW handle all the ECC
 	 * data written to the device.
 	 */
-	return write_page(mtd, chip, buf, false);
+	return write_page(mtd, chip, buf, page, false);
 }
 
 /*
@@ -1075,7 +1078,7 @@ static int denali_write_page_raw(struct mtd_info *mtd, struct nand_chip *chip,
 	 * for raw page writes, we want to disable ECC and simply write
 	 * whatever data is in the buffer.
 	 */
-	return write_page(mtd, chip, buf, true);
+	return write_page(mtd, chip, buf, page, true);
 }
 
 static int denali_write_oob(struct mtd_info *mtd, struct nand_chip *chip,
@@ -1105,12 +1108,7 @@ static int denali_read_page(struct mtd_info *mtd, struct nand_chip *chip,
 	unsigned long uncor_ecc_flags = 0;
 	int stat = 0;
 
-	if (page != denali->page) {
-		dev_err(denali->dev,
-			"IN %s: page %d is not equal to denali->page %d",
-			__func__, page, denali->page);
-		BUG();
-	}
+	denali->page = page;
 
 	setup_ecc_for_xfer(denali, true, false);
 
@@ -1154,12 +1152,7 @@ static int denali_read_page_raw(struct mtd_info *mtd, struct nand_chip *chip,
 	size_t size = mtd->writesize + mtd->oobsize;
 	uint32_t irq_mask = INTR__DMA_CMD_COMP;
 
-	if (page != denali->page) {
-		dev_err(denali->dev,
-			"IN %s: page %d is not equal to denali->page %d",
-			__func__, page, denali->page);
-		BUG();
-	}
+	denali->page = page;
 
 	setup_ecc_for_xfer(denali, false, true);
 	denali_enable_dma(denali, true);
@@ -1204,12 +1197,7 @@ static void denali_select_chip(struct mtd_info *mtd, int chip)
 
 static int denali_waitfunc(struct mtd_info *mtd, struct nand_chip *chip)
 {
-	struct denali_nand_info *denali = mtd_to_denali(mtd);
-	int status = denali->status;
-
-	denali->status = 0;
-
-	return status;
+	return 0;
 }
 
 static int denali_erase(struct mtd_info *mtd, int page)
@@ -1238,8 +1226,6 @@ static void denali_cmdfunc(struct mtd_info *mtd, unsigned int cmd, int col,
 	int i;
 
 	switch (cmd) {
-	case NAND_CMD_PAGEPROG:
-		break;
 	case NAND_CMD_STATUS:
 		read_status(denali);
 		break;
@@ -1259,10 +1245,6 @@ static void denali_cmdfunc(struct mtd_info *mtd, unsigned int cmd, int col,
 			write_byte_to_buf(denali, id);
 		}
 		break;
-	case NAND_CMD_READ0:
-	case NAND_CMD_SEQIN:
-		denali->page = page;
-		break;
 	case NAND_CMD_RESET:
 		reset_bank(denali);
 		break;
@@ -1605,6 +1587,7 @@ int denali_init(struct denali_nand_info *denali)
 
 	mtd_set_ooblayout(mtd, &denali_ooblayout_ops);
 
+	chip->ecc.options |= NAND_ECC_CUSTOM_PAGE_ACCESS;
 	chip->ecc.read_page = denali_read_page;
 	chip->ecc.read_page_raw = denali_read_page_raw;
 	chip->ecc.write_page = denali_write_page;
diff --git a/drivers/mtd/nand/denali.h b/drivers/mtd/nand/denali.h
index a06ed741b550..352d8328b94a 100644
--- a/drivers/mtd/nand/denali.h
+++ b/drivers/mtd/nand/denali.h
@@ -323,7 +323,6 @@ struct nand_buf {
 struct denali_nand_info {
 	struct nand_chip nand;
 	int flash_bank; /* currently selected chip */
-	int status;
 	int platform;
 	struct nand_buf buf;
 	struct device *dev;
-- 
2.7.4

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

* [PATCH v7 02/16] mtd: nand: denali: remove unneeded find_valid_banks()
  2017-06-13 13:45 [PATCH v7 00/16] mtd: nand: denali: Denali NAND IP improvements Masahiro Yamada
  2017-06-13 13:45 ` [PATCH v7 01/16] mtd: nand: denali: set NAND_ECC_CUSTOM_PAGE_ACCESS Masahiro Yamada
@ 2017-06-13 13:45 ` Masahiro Yamada
  2017-06-13 13:45 ` [PATCH v7 03/16] mtd: nand: denali: handle timing parameters by setup_data_interface() Masahiro Yamada
                   ` (14 subsequent siblings)
  16 siblings, 0 replies; 19+ messages in thread
From: Masahiro Yamada @ 2017-06-13 13:45 UTC (permalink / raw)
  To: linux-mtd
  Cc: Enrico Jorns, Artem Bityutskiy, Dinh Nguyen, Boris Brezillon,
	Marek Vasut, David Woodhouse, Masami Hiramatsu, Chuanxiao Dong,
	Jassi Brar, Masahiro Yamada, Cyrille Pitchen, linux-kernel,
	Brian Norris, Richard Weinberger

The function find_valid_banks() issues the Read ID (0x90) command,
then compares the first byte (Manufacturer ID) of each bank with
the one of bank0.

This is equivalent to what nand_scan_ident() does.  The number of
chips is detected there, so this is unneeded.

What is worse for find_valid_banks() is that, if multiple chips are
connected to INTEL_CE4100 platform, it crashes the kernel by BUG().
This is what we should avoid.  This function is just harmful and
unneeded.

Signed-off-by: Masahiro Yamada <yamada.masahiro@socionext.com>
---

Changes in v7: None
Changes in v6: None
Changes in v5: None
Changes in v4: None
Changes in v3: None
Changes in v2:
  - Newly added

 drivers/mtd/nand/denali.c | 47 -----------------------------------------------
 drivers/mtd/nand/denali.h |  1 -
 2 files changed, 48 deletions(-)

diff --git a/drivers/mtd/nand/denali.c b/drivers/mtd/nand/denali.c
index 7133a33b4ad3..122df4c6126d 100644
--- a/drivers/mtd/nand/denali.c
+++ b/drivers/mtd/nand/denali.c
@@ -338,51 +338,6 @@ static void get_samsung_nand_para(struct denali_nand_info *denali,
 }
 
 /*
- * determines how many NAND chips are connected to the controller. Note for
- * Intel CE4100 devices we don't support more than one device.
- */
-static void find_valid_banks(struct denali_nand_info *denali)
-{
-	uint32_t id[denali->max_banks];
-	int i;
-
-	denali->total_used_banks = 1;
-	for (i = 0; i < denali->max_banks; i++) {
-		index_addr(denali, MODE_11 | (i << 24) | 0, 0x90);
-		index_addr(denali, MODE_11 | (i << 24) | 1, 0);
-		index_addr_read_data(denali, MODE_11 | (i << 24) | 2, &id[i]);
-
-		dev_dbg(denali->dev,
-			"Return 1st ID for bank[%d]: %x\n", i, id[i]);
-
-		if (i == 0) {
-			if (!(id[i] & 0x0ff))
-				break; /* WTF? */
-		} else {
-			if ((id[i] & 0x0ff) == (id[0] & 0x0ff))
-				denali->total_used_banks++;
-			else
-				break;
-		}
-	}
-
-	if (denali->platform == INTEL_CE4100) {
-		/*
-		 * Platform limitations of the CE4100 device limit
-		 * users to a single chip solution for NAND.
-		 * Multichip support is not enabled.
-		 */
-		if (denali->total_used_banks != 1) {
-			dev_err(denali->dev,
-				"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);
-}
-
-/*
  * Use the configuration feature register to determine the maximum number of
  * banks that the hardware supports.
  */
@@ -439,8 +394,6 @@ static uint16_t denali_nand_timing_set(struct denali_nand_info *denali)
 			ioread32(denali->flash_reg + RDWR_EN_HI_CNT),
 			ioread32(denali->flash_reg + CS_SETUP_CNT));
 
-	find_valid_banks(denali);
-
 	/*
 	 * If the user specified to override the default timings
 	 * with a specific ONFI mode, we apply those changes here.
diff --git a/drivers/mtd/nand/denali.h b/drivers/mtd/nand/denali.h
index 352d8328b94a..0e4a8965f6f1 100644
--- a/drivers/mtd/nand/denali.h
+++ b/drivers/mtd/nand/denali.h
@@ -326,7 +326,6 @@ struct denali_nand_info {
 	int platform;
 	struct nand_buf buf;
 	struct device *dev;
-	int total_used_banks;
 	int page;
 	void __iomem *flash_reg;	/* Register Interface */
 	void __iomem *flash_mem;	/* Host Data/Command Interface */
-- 
2.7.4

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

* [PATCH v7 03/16] mtd: nand: denali: handle timing parameters by setup_data_interface()
  2017-06-13 13:45 [PATCH v7 00/16] mtd: nand: denali: Denali NAND IP improvements Masahiro Yamada
  2017-06-13 13:45 ` [PATCH v7 01/16] mtd: nand: denali: set NAND_ECC_CUSTOM_PAGE_ACCESS Masahiro Yamada
  2017-06-13 13:45 ` [PATCH v7 02/16] mtd: nand: denali: remove unneeded find_valid_banks() Masahiro Yamada
@ 2017-06-13 13:45 ` Masahiro Yamada
  2017-06-13 13:45 ` [PATCH v7 04/16] mtd: nand: denali: rework interrupt handling Masahiro Yamada
                   ` (13 subsequent siblings)
  16 siblings, 0 replies; 19+ messages in thread
From: Masahiro Yamada @ 2017-06-13 13:45 UTC (permalink / raw)
  To: linux-mtd
  Cc: Enrico Jorns, Artem Bityutskiy, Dinh Nguyen, Boris Brezillon,
	Marek Vasut, David Woodhouse, Masami Hiramatsu, Chuanxiao Dong,
	Jassi Brar, Masahiro Yamada, Cyrille Pitchen, linux-kernel,
	Brian Norris, Richard Weinberger

Handling timing parameters in a driver's own way should be avoided
because it duplicates efforts of drivers/mtd/nand/nand_timings.c
Besides, this driver hard-codes Intel specific parameters such as
CLK_X=5, CLK_MULTI=4.  Taking a certain device (Samsung K9WAG08U1A)
into account by get_samsung_nand_para() is weird as well.

Now, the core framework provides .setup_data_interface() hook, which
handles timing parameters in a generic manner.

While I am working on this, I found even more issues in the current
code, so fixed the following as well:

- In recent IP versions, WE_2_RE and TWHR2 share the same register.
  Likewise for ADDR_2_DATA and TCWAW, CS_SETUP_CNT and TWB.  When
  updating one, the other must be masked.  Otherwise, the other will
  be set to 0, then timing settings will be broken.

- The recent IP release expanded the ADDR_2_DATA to 7-bit wide.
  This register is related to tADL.  As commit 74a332e78e8f ("mtd:
  nand: timings: Fix tADL_min for ONFI 4.0 chips") addressed, the
  ONFi 4.0 increased the minimum of tADL to 400 nsec.  This may not
  fit in the 6-bit ADDR_2_DATA in older versions.  Check the IP
  revision and handle this correctly, otherwise the register value
  would wrap around.

Signed-off-by: Masahiro Yamada <yamada.masahiro@socionext.com>
---

Changes in v7: None
Changes in v6:
  - Rebase on commit 104e442a67cfba4d0cc982384761befb917fb6a1
    (mtd: nand: Pass the CS line to ->setup_data_interface() )

Changes in v5: None
Changes in v4: None
Changes in v3: None
Changes in v2:
  - Newly added

 drivers/mtd/nand/denali.c     | 345 +++++++++++++++---------------------------
 drivers/mtd/nand/denali.h     |  26 ++--
 drivers/mtd/nand/denali_dt.c  |   3 +-
 drivers/mtd/nand/denali_pci.c |   6 +-
 4 files changed, 139 insertions(+), 241 deletions(-)

diff --git a/drivers/mtd/nand/denali.c b/drivers/mtd/nand/denali.c
index 122df4c6126d..ca2b6b8850ba 100644
--- a/drivers/mtd/nand/denali.c
+++ b/drivers/mtd/nand/denali.c
@@ -28,17 +28,6 @@
 
 MODULE_LICENSE("GPL");
 
-/*
- * We define a module parameter that allows the user to override
- * the hardware and decide what timing mode should be used.
- */
-#define NAND_DEFAULT_TIMINGS	-1
-
-static int onfi_timing_mode = NAND_DEFAULT_TIMINGS;
-module_param(onfi_timing_mode, int, S_IRUGO);
-MODULE_PARM_DESC(onfi_timing_mode,
-	   "Overrides default ONFI setting. -1 indicates use default timings");
-
 #define DENALI_NAND_NAME    "denali-nand"
 
 /*
@@ -63,10 +52,12 @@ MODULE_PARM_DESC(onfi_timing_mode,
 #define CHIP_SELECT_INVALID	-1
 
 /*
- * This macro divides two integers and rounds fractional values up
- * to the nearest integer value.
+ * The bus interface clock, clk_x, is phase aligned with the core clock.  The
+ * clk_x is an integral multiple N of the core clk.  The value N is configured
+ * at IP delivery time, and its available value is 4, 5, or 6.  We need to align
+ * to the largest value to make it work with any possible configuration.
  */
-#define CEIL_DIV(X, Y) (((X)%(Y)) ? ((X)/(Y)+1) : ((X)/(Y)))
+#define DENALI_CLK_X_MULT	6
 
 /*
  * this macro allows us to convert from an MTD structure to our own
@@ -196,148 +187,6 @@ static uint16_t denali_nand_reset(struct denali_nand_info *denali)
 }
 
 /*
- * this routine calculates the ONFI timing values for a given mode and
- * programs the clocking register accordingly. The mode is determined by
- * the get_onfi_nand_para routine.
- */
-static void nand_onfi_timing_set(struct denali_nand_info *denali,
-								uint16_t mode)
-{
-	uint16_t Trea[6] = {40, 30, 25, 20, 20, 16};
-	uint16_t Trp[6] = {50, 25, 17, 15, 12, 10};
-	uint16_t Treh[6] = {30, 15, 15, 10, 10, 7};
-	uint16_t Trc[6] = {100, 50, 35, 30, 25, 20};
-	uint16_t Trhoh[6] = {0, 15, 15, 15, 15, 15};
-	uint16_t Trloh[6] = {0, 0, 0, 0, 5, 5};
-	uint16_t Tcea[6] = {100, 45, 30, 25, 25, 25};
-	uint16_t Tadl[6] = {200, 100, 100, 100, 70, 70};
-	uint16_t Trhw[6] = {200, 100, 100, 100, 100, 100};
-	uint16_t Trhz[6] = {200, 100, 100, 100, 100, 100};
-	uint16_t Twhr[6] = {120, 80, 80, 60, 60, 60};
-	uint16_t Tcs[6] = {70, 35, 25, 25, 20, 15};
-
-	uint16_t data_invalid_rhoh, data_invalid_rloh, data_invalid;
-	uint16_t dv_window = 0;
-	uint16_t en_lo, en_hi;
-	uint16_t acc_clks;
-	uint16_t addr_2_data, re_2_we, re_2_re, we_2_re, cs_cnt;
-
-	en_lo = CEIL_DIV(Trp[mode], CLK_X);
-	en_hi = CEIL_DIV(Treh[mode], CLK_X);
-#if ONFI_BLOOM_TIME
-	if ((en_hi * CLK_X) < (Treh[mode] + 2))
-		en_hi++;
-#endif
-
-	if ((en_lo + en_hi) * CLK_X < Trc[mode])
-		en_lo += CEIL_DIV((Trc[mode] - (en_lo + en_hi) * CLK_X), CLK_X);
-
-	if ((en_lo + en_hi) < CLK_MULTI)
-		en_lo += CLK_MULTI - en_lo - en_hi;
-
-	while (dv_window < 8) {
-		data_invalid_rhoh = en_lo * CLK_X + Trhoh[mode];
-
-		data_invalid_rloh = (en_lo + en_hi) * CLK_X + Trloh[mode];
-
-		data_invalid = data_invalid_rhoh < data_invalid_rloh ?
-					data_invalid_rhoh : data_invalid_rloh;
-
-		dv_window = data_invalid - Trea[mode];
-
-		if (dv_window < 8)
-			en_lo++;
-	}
-
-	acc_clks = CEIL_DIV(Trea[mode], CLK_X);
-
-	while (acc_clks * CLK_X - Trea[mode] < 3)
-		acc_clks++;
-
-	if (data_invalid - acc_clks * CLK_X < 2)
-		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);
-	re_2_re = CEIL_DIV(Trhz[mode], CLK_X);
-	we_2_re = CEIL_DIV(Twhr[mode], CLK_X);
-	cs_cnt = CEIL_DIV((Tcs[mode] - Trp[mode]), CLK_X);
-	if (cs_cnt == 0)
-		cs_cnt = 1;
-
-	if (Tcea[mode]) {
-		while (cs_cnt * CLK_X + Trea[mode] < Tcea[mode])
-			cs_cnt++;
-	}
-
-#if MODE5_WORKAROUND
-	if (mode == 5)
-		acc_clks = 5;
-#endif
-
-	/* Sighting 3462430: Temporary hack for MT29F128G08CJABAWP:B */
-	if (ioread32(denali->flash_reg + MANUFACTURER_ID) == 0 &&
-		ioread32(denali->flash_reg + DEVICE_ID) == 0x88)
-		acc_clks = 6;
-
-	iowrite32(acc_clks, denali->flash_reg + ACC_CLKS);
-	iowrite32(re_2_we, denali->flash_reg + RE_2_WE);
-	iowrite32(re_2_re, denali->flash_reg + RE_2_RE);
-	iowrite32(we_2_re, denali->flash_reg + WE_2_RE);
-	iowrite32(addr_2_data, denali->flash_reg + ADDR_2_DATA);
-	iowrite32(en_lo, denali->flash_reg + RDWR_EN_LO_CNT);
-	iowrite32(en_hi, denali->flash_reg + RDWR_EN_HI_CNT);
-	iowrite32(cs_cnt, denali->flash_reg + CS_SETUP_CNT);
-}
-
-/* queries the NAND device to see what ONFI modes it supports. */
-static uint16_t get_onfi_nand_para(struct denali_nand_info *denali)
-{
-	int i;
-
-	/*
-	 * we needn't to do a reset here because driver has already
-	 * reset all the banks before
-	 */
-	if (!(ioread32(denali->flash_reg + ONFI_TIMING_MODE) &
-		ONFI_TIMING_MODE__VALUE))
-		return FAIL;
-
-	for (i = 5; i > 0; i--) {
-		if (ioread32(denali->flash_reg + ONFI_TIMING_MODE) &
-			(0x01 << i))
-			break;
-	}
-
-	nand_onfi_timing_set(denali, i);
-
-	/*
-	 * By now, all the ONFI devices we know support the page cache
-	 * rw feature. So here we enable the pipeline_rw_ahead feature
-	 */
-	/* iowrite32(1, denali->flash_reg + CACHE_WRITE_ENABLE); */
-	/* iowrite32(1, denali->flash_reg + CACHE_READ_ENABLE);  */
-
-	return PASS;
-}
-
-static void get_samsung_nand_para(struct denali_nand_info *denali,
-							uint8_t device_id)
-{
-	if (device_id == 0xd3) { /* Samsung K9WAG08U1A */
-		/* Set timing register values according to datasheet */
-		iowrite32(5, denali->flash_reg + ACC_CLKS);
-		iowrite32(20, denali->flash_reg + RE_2_WE);
-		iowrite32(12, denali->flash_reg + WE_2_RE);
-		iowrite32(14, denali->flash_reg + ADDR_2_DATA);
-		iowrite32(3, denali->flash_reg + RDWR_EN_LO_CNT);
-		iowrite32(2, denali->flash_reg + RDWR_EN_HI_CNT);
-		iowrite32(2, denali->flash_reg + CS_SETUP_CNT);
-	}
-}
-
-/*
  * Use the configuration feature register to determine the maximum number of
  * banks that the hardware supports.
  */
@@ -352,58 +201,6 @@ static void detect_max_banks(struct denali_nand_info *denali)
 		denali->max_banks <<= 1;
 }
 
-static uint16_t denali_nand_timing_set(struct denali_nand_info *denali)
-{
-	uint16_t status = PASS;
-	uint32_t id_bytes[8], addr;
-	uint8_t maf_id, device_id;
-	int i;
-
-	/*
-	 * Use read id method to get device ID and other params.
-	 * For some NAND chips, controller can't report the correct
-	 * device ID by reading from DEVICE_ID register
-	 */
-	addr = MODE_11 | BANK(denali->flash_bank);
-	index_addr(denali, addr | 0, 0x90);
-	index_addr(denali, addr | 1, 0);
-	for (i = 0; i < 8; i++)
-		index_addr_read_data(denali, addr | 2, &id_bytes[i]);
-	maf_id = id_bytes[0];
-	device_id = id_bytes[1];
-
-	if (ioread32(denali->flash_reg + ONFI_DEVICE_NO_OF_LUNS) &
-		ONFI_DEVICE_NO_OF_LUNS__ONFI_DEVICE) { /* ONFI 1.0 NAND */
-		if (FAIL == get_onfi_nand_para(denali))
-			return FAIL;
-	} else if (maf_id == 0xEC) { /* Samsung NAND */
-		get_samsung_nand_para(denali, device_id);
-	}
-
-	dev_info(denali->dev,
-			"Dump timing register values:\n"
-			"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));
-
-	/*
-	 * If the user specified to override the default timings
-	 * with a specific ONFI mode, we apply those changes here.
-	 */
-	if (onfi_timing_mode != NAND_DEFAULT_TIMINGS)
-		nand_onfi_timing_set(denali, onfi_timing_mode);
-
-	return status;
-}
-
 static void denali_set_intr_modes(struct denali_nand_info *denali,
 					uint16_t INT_ENABLE)
 {
@@ -1209,7 +1006,121 @@ static void denali_cmdfunc(struct mtd_info *mtd, unsigned int cmd, int col,
 		break;
 	}
 }
-/* end NAND core entry points */
+
+#define DIV_ROUND_DOWN_ULL(ll, d) \
+	({ unsigned long long _tmp = (ll); do_div(_tmp, d); _tmp; })
+
+static int denali_setup_data_interface(struct mtd_info *mtd, int chipnr,
+				       const struct nand_data_interface *conf)
+{
+	struct denali_nand_info *denali = mtd_to_denali(mtd);
+	const struct nand_sdr_timings *timings;
+	unsigned long t_clk;
+	int acc_clks, re_2_we, re_2_re, we_2_re, addr_2_data;
+	int rdwr_en_lo, rdwr_en_hi, rdwr_en_lo_hi, cs_setup;
+	int addr_2_data_mask;
+	uint32_t tmp;
+
+	timings = nand_get_sdr_timings(conf);
+	if (IS_ERR(timings))
+		return PTR_ERR(timings);
+
+	/* clk_x period in picoseconds */
+	t_clk = DIV_ROUND_DOWN_ULL(1000000000000ULL, denali->clk_x_rate);
+	if (!t_clk)
+		return -EINVAL;
+
+	if (chipnr == NAND_DATA_IFACE_CHECK_ONLY)
+		return 0;
+
+	/* tREA -> ACC_CLKS */
+	acc_clks = DIV_ROUND_UP(timings->tREA_max, t_clk);
+	acc_clks = min_t(int, acc_clks, ACC_CLKS__VALUE);
+
+	tmp = ioread32(denali->flash_reg + ACC_CLKS);
+	tmp &= ~ACC_CLKS__VALUE;
+	tmp |= acc_clks;
+	iowrite32(tmp, denali->flash_reg + ACC_CLKS);
+
+	/* tRWH -> RE_2_WE */
+	re_2_we = DIV_ROUND_UP(timings->tRHW_min, t_clk);
+	re_2_we = min_t(int, re_2_we, RE_2_WE__VALUE);
+
+	tmp = ioread32(denali->flash_reg + RE_2_WE);
+	tmp &= ~RE_2_WE__VALUE;
+	tmp |= re_2_we;
+	iowrite32(tmp, denali->flash_reg + RE_2_WE);
+
+	/* tRHZ -> RE_2_RE */
+	re_2_re = DIV_ROUND_UP(timings->tRHZ_max, t_clk);
+	re_2_re = min_t(int, re_2_re, RE_2_RE__VALUE);
+
+	tmp = ioread32(denali->flash_reg + RE_2_RE);
+	tmp &= ~RE_2_RE__VALUE;
+	tmp |= re_2_re;
+	iowrite32(tmp, denali->flash_reg + RE_2_RE);
+
+	/* tWHR -> WE_2_RE */
+	we_2_re = DIV_ROUND_UP(timings->tWHR_min, t_clk);
+	we_2_re = min_t(int, we_2_re, TWHR2_AND_WE_2_RE__WE_2_RE);
+
+	tmp = ioread32(denali->flash_reg + TWHR2_AND_WE_2_RE);
+	tmp &= ~TWHR2_AND_WE_2_RE__WE_2_RE;
+	tmp |= we_2_re;
+	iowrite32(tmp, denali->flash_reg + TWHR2_AND_WE_2_RE);
+
+	/* tADL -> ADDR_2_DATA */
+
+	/* for older versions, ADDR_2_DATA is only 6 bit wide */
+	addr_2_data_mask = TCWAW_AND_ADDR_2_DATA__ADDR_2_DATA;
+	if (denali->revision < 0x0501)
+		addr_2_data_mask >>= 1;
+
+	addr_2_data = DIV_ROUND_UP(timings->tADL_min, t_clk);
+	addr_2_data = min_t(int, addr_2_data, addr_2_data_mask);
+
+	tmp = ioread32(denali->flash_reg + TCWAW_AND_ADDR_2_DATA);
+	tmp &= ~addr_2_data_mask;
+	tmp |= addr_2_data;
+	iowrite32(tmp, denali->flash_reg + TCWAW_AND_ADDR_2_DATA);
+
+	/* tREH, tWH -> RDWR_EN_HI_CNT */
+	rdwr_en_hi = DIV_ROUND_UP(max(timings->tREH_min, timings->tWH_min),
+				  t_clk);
+	rdwr_en_hi = min_t(int, rdwr_en_hi, RDWR_EN_HI_CNT__VALUE);
+
+	tmp = ioread32(denali->flash_reg + RDWR_EN_HI_CNT);
+	tmp &= ~RDWR_EN_HI_CNT__VALUE;
+	tmp |= rdwr_en_hi;
+	iowrite32(tmp, denali->flash_reg + RDWR_EN_HI_CNT);
+
+	/* tRP, tWP -> RDWR_EN_LO_CNT */
+	rdwr_en_lo = DIV_ROUND_UP(max(timings->tRP_min, timings->tWP_min),
+				  t_clk);
+	rdwr_en_lo_hi = DIV_ROUND_UP(max(timings->tRC_min, timings->tWC_min),
+				     t_clk);
+	rdwr_en_lo_hi = max(rdwr_en_lo_hi, DENALI_CLK_X_MULT);
+	rdwr_en_lo = max(rdwr_en_lo, rdwr_en_lo_hi - rdwr_en_hi);
+	rdwr_en_lo = min_t(int, rdwr_en_lo, RDWR_EN_LO_CNT__VALUE);
+
+	tmp = ioread32(denali->flash_reg + RDWR_EN_LO_CNT);
+	tmp &= ~RDWR_EN_LO_CNT__VALUE;
+	tmp |= rdwr_en_lo;
+	iowrite32(tmp, denali->flash_reg + RDWR_EN_LO_CNT);
+
+	/* tCS, tCEA -> CS_SETUP_CNT */
+	cs_setup = max3((int)DIV_ROUND_UP(timings->tCS_min, t_clk) - rdwr_en_lo,
+			(int)DIV_ROUND_UP(timings->tCEA_max, t_clk) - acc_clks,
+			0);
+	cs_setup = min_t(int, cs_setup, CS_SETUP_CNT__VALUE);
+
+	tmp = ioread32(denali->flash_reg + CS_SETUP_CNT);
+	tmp &= ~CS_SETUP_CNT__VALUE;
+	tmp |= cs_setup;
+	iowrite32(tmp, denali->flash_reg + CS_SETUP_CNT);
+
+	return 0;
+}
 
 /* Initialization code to bring the device up to a known good state */
 static void denali_hw_init(struct denali_nand_info *denali)
@@ -1241,7 +1152,6 @@ static void denali_hw_init(struct denali_nand_info *denali)
 	/* Should set value for these registers when init */
 	iowrite32(0, denali->flash_reg + TWO_ROW_ADDR_CYCLES);
 	iowrite32(1, denali->flash_reg + ECC_ENABLE);
-	denali_nand_timing_set(denali);
 	denali_irq_init(denali);
 }
 
@@ -1416,17 +1326,6 @@ int denali_init(struct denali_nand_info *denali)
 	struct mtd_info *mtd = nand_to_mtd(chip);
 	int ret;
 
-	if (denali->platform == INTEL_CE4100) {
-		/*
-		 * Due to a silicon limitation, we can only support
-		 * ONFI timing mode 1 and below.
-		 */
-		if (onfi_timing_mode < -1 || onfi_timing_mode > 1) {
-			pr_err("Intel CE4100 only supports ONFI timing mode 1 or below\n");
-			return -EINVAL;
-		}
-	}
-
 	/* allocate a temporary buffer for nand_scan_ident() */
 	denali->buf.buf = devm_kzalloc(denali->dev, PAGE_SIZE,
 					GFP_DMA | GFP_KERNEL);
@@ -1460,6 +1359,10 @@ int denali_init(struct denali_nand_info *denali)
 	chip->onfi_set_features = nand_onfi_get_set_features_notsupp;
 	chip->onfi_get_features = nand_onfi_get_set_features_notsupp;
 
+	/* clk rate info is needed for setup_data_interface */
+	if (denali->clk_x_rate)
+		chip->setup_data_interface = denali_setup_data_interface;
+
 	/*
 	 * scan for NAND devices attached to the controller
 	 * this is the first stage in a two step process to register
diff --git a/drivers/mtd/nand/denali.h b/drivers/mtd/nand/denali.h
index 0e4a8965f6f1..fb473895a79d 100644
--- a/drivers/mtd/nand/denali.h
+++ b/drivers/mtd/nand/denali.h
@@ -72,11 +72,14 @@
 #define GLOBAL_INT_ENABLE			0xf0
 #define     GLOBAL_INT_EN_FLAG				BIT(0)
 
-#define WE_2_RE					0x100
-#define     WE_2_RE__VALUE				GENMASK(5, 0)
+#define TWHR2_AND_WE_2_RE			0x100
+#define     TWHR2_AND_WE_2_RE__WE_2_RE			GENMASK(5, 0)
+#define     TWHR2_AND_WE_2_RE__TWHR2			GENMASK(13, 8)
 
-#define ADDR_2_DATA				0x110
-#define     ADDR_2_DATA__VALUE				GENMASK(5, 0)
+#define TCWAW_AND_ADDR_2_DATA			0x110
+/* The width of ADDR_2_DATA is 6 bit for old IP, 7 bit for new IP */
+#define     TCWAW_AND_ADDR_2_DATA__ADDR_2_DATA		GENMASK(6, 0)
+#define     TCWAW_AND_ADDR_2_DATA__TCWAW		GENMASK(13, 8)
 
 #define RE_2_WE					0x120
 #define     RE_2_WE__VALUE				GENMASK(5, 0)
@@ -128,6 +131,7 @@
 
 #define CS_SETUP_CNT				0x220
 #define     CS_SETUP_CNT__VALUE				GENMASK(4, 0)
+#define     CS_SETUP_CNT__TWB				GENMASK(17, 12)
 
 #define SPARE_AREA_SKIP_BYTES			0x230
 #define     SPARE_AREA_SKIP_BYTES__VALUE		GENMASK(5, 0)
@@ -294,16 +298,8 @@
 #define     CHNL_ACTIVE__CHANNEL2			BIT(2)
 #define     CHNL_ACTIVE__CHANNEL3			BIT(3)
 
-#define FAIL 1                  /*failed flag*/
 #define PASS 0                  /*success flag*/
 
-#define CLK_X  5
-#define CLK_MULTI 4
-
-#define ONFI_BLOOM_TIME         1
-#define MODE5_WORKAROUND        0
-
-
 #define MODE_00    0x00000000
 #define MODE_01    0x04000000
 #define MODE_10    0x08000000
@@ -316,14 +312,10 @@ struct nand_buf {
 	dma_addr_t dma_buf;
 };
 
-#define INTEL_CE4100	1
-#define INTEL_MRST	2
-#define DT		3
-
 struct denali_nand_info {
 	struct nand_chip nand;
+	unsigned long clk_x_rate;	/* bus interface clock rate */
 	int flash_bank; /* currently selected chip */
-	int platform;
 	struct nand_buf buf;
 	struct device *dev;
 	int page;
diff --git a/drivers/mtd/nand/denali_dt.c b/drivers/mtd/nand/denali_dt.c
index be598230c108..ebcce50f4005 100644
--- a/drivers/mtd/nand/denali_dt.c
+++ b/drivers/mtd/nand/denali_dt.c
@@ -96,7 +96,6 @@ static int denali_dt_probe(struct platform_device *pdev)
 		denali->ecc_caps = data->ecc_caps;
 	}
 
-	denali->platform = DT;
 	denali->dev = &pdev->dev;
 	denali->irq = platform_get_irq(pdev, 0);
 	if (denali->irq < 0) {
@@ -121,6 +120,8 @@ static int denali_dt_probe(struct platform_device *pdev)
 	}
 	clk_prepare_enable(dt->clk);
 
+	denali->clk_x_rate = clk_get_rate(dt->clk);
+
 	ret = denali_init(denali);
 	if (ret)
 		goto out_disable_clk;
diff --git a/drivers/mtd/nand/denali_pci.c b/drivers/mtd/nand/denali_pci.c
index 37dc0934c24c..6217525c1000 100644
--- a/drivers/mtd/nand/denali_pci.c
+++ b/drivers/mtd/nand/denali_pci.c
@@ -19,6 +19,9 @@
 
 #define DENALI_NAND_NAME    "denali-nand-pci"
 
+#define INTEL_CE4100	1
+#define INTEL_MRST	2
+
 /* List of platforms this NAND controller has be integrated into */
 static const struct pci_device_id denali_pci_ids[] = {
 	{ PCI_VDEVICE(INTEL, 0x0701), INTEL_CE4100 },
@@ -47,13 +50,11 @@ static int denali_pci_probe(struct pci_dev *dev, const struct pci_device_id *id)
 	}
 
 	if (id->driver_data == INTEL_CE4100) {
-		denali->platform = INTEL_CE4100;
 		mem_base = pci_resource_start(dev, 0);
 		mem_len = pci_resource_len(dev, 1);
 		csr_base = pci_resource_start(dev, 1);
 		csr_len = pci_resource_len(dev, 1);
 	} else {
-		denali->platform = INTEL_MRST;
 		csr_base = pci_resource_start(dev, 0);
 		csr_len = pci_resource_len(dev, 0);
 		mem_base = pci_resource_start(dev, 1);
@@ -69,6 +70,7 @@ static int denali_pci_probe(struct pci_dev *dev, const struct pci_device_id *id)
 	denali->irq = dev->irq;
 	denali->ecc_caps = &denali_pci_ecc_caps;
 	denali->nand.ecc.options |= NAND_ECC_MAXIMIZE;
+	denali->clk_x_rate = 200000000;		/* 200 MHz */
 
 	ret = pci_request_regions(dev, DENALI_NAND_NAME);
 	if (ret) {
-- 
2.7.4

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

* [PATCH v7 04/16] mtd: nand: denali: rework interrupt handling
  2017-06-13 13:45 [PATCH v7 00/16] mtd: nand: denali: Denali NAND IP improvements Masahiro Yamada
                   ` (2 preceding siblings ...)
  2017-06-13 13:45 ` [PATCH v7 03/16] mtd: nand: denali: handle timing parameters by setup_data_interface() Masahiro Yamada
@ 2017-06-13 13:45 ` Masahiro Yamada
  2017-06-13 13:45 ` [PATCH v7 05/16] mtd: nand: denali: switch over to cmd_ctrl instead of cmdfunc Masahiro Yamada
                   ` (12 subsequent siblings)
  16 siblings, 0 replies; 19+ messages in thread
From: Masahiro Yamada @ 2017-06-13 13:45 UTC (permalink / raw)
  To: linux-mtd
  Cc: Enrico Jorns, Artem Bityutskiy, Dinh Nguyen, Boris Brezillon,
	Marek Vasut, David Woodhouse, Masami Hiramatsu, Chuanxiao Dong,
	Jassi Brar, Masahiro Yamada, Cyrille Pitchen, linux-kernel,
	Brian Norris, Richard Weinberger

Simplify the interrupt handling and fix issues:

- The register field view of INTR_EN / INTR_STATUS is different
  among IP versions.  The global macro DENALI_IRQ_ALL is hard-coded
  for Intel platforms.  The interrupt mask should be determined at
  run-time depending on the running platform.

- wait_for_irq() loops do {} while() until interested flags are
  asserted.  The logic can be simplified.

- The spin_lock() guard seems too complex (and suspicious in a race
  condition if wait_for_completion_timeout() bails out by timeout).

- denali->complete is reused again and again, but reinit_completion()
  is missing.  Add it.

Re-work the code to make it more robust and easier to handle.

While we are here, also rename the jump label "failed_req_irq" to
more appropriate "disable_irq".

Signed-off-by: Masahiro Yamada <yamada.masahiro@socionext.com>
---

Changes in v7:
  - Add comment "return immediately if the IRQ has already happened"

Changes in v6: None
Changes in v5: None
Changes in v4: None
Changes in v3: None
Changes in v2:
  - Newly added

 drivers/mtd/nand/denali.c | 317 +++++++++++++++++-----------------------------
 drivers/mtd/nand/denali.h |   1 +
 2 files changed, 117 insertions(+), 201 deletions(-)

diff --git a/drivers/mtd/nand/denali.c b/drivers/mtd/nand/denali.c
index ca2b6b8850ba..2acdc5f42a00 100644
--- a/drivers/mtd/nand/denali.c
+++ b/drivers/mtd/nand/denali.c
@@ -31,26 +31,13 @@ MODULE_LICENSE("GPL");
 #define DENALI_NAND_NAME    "denali-nand"
 
 /*
- * We define a macro here that combines all interrupts this driver uses into
- * a single constant value, for convenience.
- */
-#define DENALI_IRQ_ALL	(INTR__DMA_CMD_COMP | \
-			INTR__ECC_TRANSACTION_DONE | \
-			INTR__ECC_ERR | \
-			INTR__PROGRAM_FAIL | \
-			INTR__LOAD_COMP | \
-			INTR__PROGRAM_COMP | \
-			INTR__TIME_OUT | \
-			INTR__ERASE_FAIL | \
-			INTR__RST_COMP | \
-			INTR__ERASE_COMP)
-
-/*
  * indicates whether or not the internal value for the flash bank is
  * valid or not
  */
 #define CHIP_SELECT_INVALID	-1
 
+#define DENALI_NR_BANKS		4
+
 /*
  * The bus interface clock, clk_x, is phase aligned with the core clock.  The
  * clk_x is an integral multiple N of the core clk.  The value N is configured
@@ -85,14 +72,6 @@ static inline struct denali_nand_info *mtd_to_denali(struct mtd_info *mtd)
  */
 #define BANK(x) ((x) << 24)
 
-/* forward declarations */
-static void clear_interrupts(struct denali_nand_info *denali);
-static uint32_t wait_for_irq(struct denali_nand_info *denali,
-							uint32_t irq_mask);
-static void denali_irq_enable(struct denali_nand_info *denali,
-							uint32_t int_mask);
-static uint32_t read_interrupt_status(struct denali_nand_info *denali);
-
 /*
  * Certain operations for the denali NAND controller use an indexed mode to
  * read/write data. The operation is performed by writing the address value
@@ -143,22 +122,6 @@ static void read_status(struct denali_nand_info *denali)
 		write_byte_to_buf(denali, 0);
 }
 
-/* resets a specific device connected to the core */
-static void reset_bank(struct denali_nand_info *denali)
-{
-	uint32_t irq_status;
-	uint32_t irq_mask = INTR__RST_COMP | INTR__TIME_OUT;
-
-	clear_interrupts(denali);
-
-	iowrite32(1 << denali->flash_bank, denali->flash_reg + DEVICE_RESET);
-
-	irq_status = wait_for_irq(denali, irq_mask);
-
-	if (irq_status & INTR__TIME_OUT)
-		dev_err(denali->dev, "reset bank failed.\n");
-}
-
 /* Reset the flash controller */
 static uint16_t denali_nand_reset(struct denali_nand_info *denali)
 {
@@ -201,169 +164,124 @@ static void detect_max_banks(struct denali_nand_info *denali)
 		denali->max_banks <<= 1;
 }
 
-static void denali_set_intr_modes(struct denali_nand_info *denali,
-					uint16_t INT_ENABLE)
+static void denali_enable_irq(struct denali_nand_info *denali)
 {
-	if (INT_ENABLE)
-		iowrite32(1, denali->flash_reg + GLOBAL_INT_ENABLE);
-	else
-		iowrite32(0, denali->flash_reg + GLOBAL_INT_ENABLE);
-}
+	int i;
 
-/*
- * validation function to verify that the controlling software is making
- * a valid request
- */
-static inline bool is_flash_bank_valid(int flash_bank)
-{
-	return flash_bank >= 0 && flash_bank < 4;
+	for (i = 0; i < DENALI_NR_BANKS; i++)
+		iowrite32(U32_MAX, denali->flash_reg + INTR_EN(i));
+	iowrite32(GLOBAL_INT_EN_FLAG, denali->flash_reg + GLOBAL_INT_ENABLE);
 }
 
-static void denali_irq_init(struct denali_nand_info *denali)
+static void denali_disable_irq(struct denali_nand_info *denali)
 {
-	uint32_t int_mask;
 	int i;
 
-	/* Disable global interrupts */
-	denali_set_intr_modes(denali, false);
-
-	int_mask = DENALI_IRQ_ALL;
-
-	/* Clear all status bits */
-	for (i = 0; i < denali->max_banks; ++i)
-		iowrite32(0xFFFF, denali->flash_reg + INTR_STATUS(i));
-
-	denali_irq_enable(denali, int_mask);
+	for (i = 0; i < DENALI_NR_BANKS; i++)
+		iowrite32(0, denali->flash_reg + INTR_EN(i));
+	iowrite32(0, denali->flash_reg + GLOBAL_INT_ENABLE);
 }
 
-static void denali_irq_cleanup(int irqnum, struct denali_nand_info *denali)
+static void denali_clear_irq(struct denali_nand_info *denali,
+			     int bank, uint32_t irq_status)
 {
-	denali_set_intr_modes(denali, false);
+	/* write one to clear bits */
+	iowrite32(irq_status, denali->flash_reg + INTR_STATUS(bank));
 }
 
-static void denali_irq_enable(struct denali_nand_info *denali,
-							uint32_t int_mask)
+static void denali_clear_irq_all(struct denali_nand_info *denali)
 {
 	int i;
 
-	for (i = 0; i < denali->max_banks; ++i)
-		iowrite32(int_mask, denali->flash_reg + INTR_EN(i));
+	for (i = 0; i < DENALI_NR_BANKS; i++)
+		denali_clear_irq(denali, i, U32_MAX);
 }
 
-/*
- * This function only returns when an interrupt that this driver cares about
- * occurs. This is to reduce the overhead of servicing interrupts
- */
-static inline uint32_t denali_irq_detected(struct denali_nand_info *denali)
+static irqreturn_t denali_isr(int irq, void *dev_id)
 {
-	return read_interrupt_status(denali) & DENALI_IRQ_ALL;
-}
+	struct denali_nand_info *denali = dev_id;
+	irqreturn_t ret = IRQ_NONE;
+	uint32_t irq_status;
+	int i;
 
-/* Interrupts are cleared by writing a 1 to the appropriate status bit */
-static inline void clear_interrupt(struct denali_nand_info *denali,
-							uint32_t irq_mask)
-{
-	uint32_t intr_status_reg;
+	spin_lock(&denali->irq_lock);
 
-	intr_status_reg = INTR_STATUS(denali->flash_bank);
+	for (i = 0; i < DENALI_NR_BANKS; i++) {
+		irq_status = ioread32(denali->flash_reg + INTR_STATUS(i));
+		if (irq_status)
+			ret = IRQ_HANDLED;
 
-	iowrite32(irq_mask, denali->flash_reg + intr_status_reg);
-}
+		denali_clear_irq(denali, i, irq_status);
 
-static void clear_interrupts(struct denali_nand_info *denali)
-{
-	uint32_t status;
+		if (i != denali->flash_bank)
+			continue;
+
+		denali->irq_status |= irq_status;
 
-	spin_lock_irq(&denali->irq_lock);
+		if (denali->irq_status & denali->irq_mask)
+			complete(&denali->complete);
+	}
 
-	status = read_interrupt_status(denali);
-	clear_interrupt(denali, status);
+	spin_unlock(&denali->irq_lock);
 
-	denali->irq_status = 0x0;
-	spin_unlock_irq(&denali->irq_lock);
+	return ret;
 }
 
-static uint32_t read_interrupt_status(struct denali_nand_info *denali)
+static void denali_reset_irq(struct denali_nand_info *denali)
 {
-	uint32_t intr_status_reg;
-
-	intr_status_reg = INTR_STATUS(denali->flash_bank);
+	unsigned long flags;
 
-	return ioread32(denali->flash_reg + intr_status_reg);
+	spin_lock_irqsave(&denali->irq_lock, flags);
+	denali->irq_status = 0;
+	denali->irq_mask = 0;
+	spin_unlock_irqrestore(&denali->irq_lock, flags);
 }
 
-/*
- * This is the interrupt service routine. It handles all interrupts
- * sent to this device. Note that on CE4100, this is a shared interrupt.
- */
-static irqreturn_t denali_isr(int irq, void *dev_id)
+static uint32_t denali_wait_for_irq(struct denali_nand_info *denali,
+				    uint32_t irq_mask)
 {
-	struct denali_nand_info *denali = dev_id;
+	unsigned long time_left, flags;
 	uint32_t irq_status;
-	irqreturn_t result = IRQ_NONE;
 
-	spin_lock(&denali->irq_lock);
+	spin_lock_irqsave(&denali->irq_lock, flags);
 
-	/* check to see if a valid NAND chip has been selected. */
-	if (is_flash_bank_valid(denali->flash_bank)) {
-		/*
-		 * check to see if controller generated the interrupt,
-		 * since this is a shared interrupt
-		 */
-		irq_status = denali_irq_detected(denali);
-		if (irq_status != 0) {
-			/* handle interrupt */
-			/* first acknowledge it */
-			clear_interrupt(denali, irq_status);
-			/*
-			 * store the status in the device context for someone
-			 * to read
-			 */
-			denali->irq_status |= irq_status;
-			/* notify anyone who cares that it happened */
-			complete(&denali->complete);
-			/* tell the OS that we've handled this */
-			result = IRQ_HANDLED;
-		}
+	irq_status = denali->irq_status;
+
+	if (irq_mask & irq_status) {
+		/* return immediately if the IRQ has already happened. */
+		spin_unlock_irqrestore(&denali->irq_lock, flags);
+		return irq_status;
 	}
-	spin_unlock(&denali->irq_lock);
-	return result;
+
+	denali->irq_mask = irq_mask;
+	reinit_completion(&denali->complete);
+	spin_unlock_irqrestore(&denali->irq_lock, flags);
+
+	time_left = wait_for_completion_timeout(&denali->complete,
+						msecs_to_jiffies(1000));
+	if (!time_left) {
+		dev_err(denali->dev, "timeout while waiting for irq 0x%x\n",
+			denali->irq_mask);
+		return 0;
+	}
+
+	return denali->irq_status;
 }
 
-static uint32_t wait_for_irq(struct denali_nand_info *denali, uint32_t irq_mask)
+/* resets a specific device connected to the core */
+static void reset_bank(struct denali_nand_info *denali)
 {
-	unsigned long comp_res;
-	uint32_t intr_status;
-	unsigned long timeout = msecs_to_jiffies(1000);
+	uint32_t irq_status;
 
-	do {
-		comp_res =
-			wait_for_completion_timeout(&denali->complete, timeout);
-		spin_lock_irq(&denali->irq_lock);
-		intr_status = denali->irq_status;
-
-		if (intr_status & irq_mask) {
-			denali->irq_status &= ~irq_mask;
-			spin_unlock_irq(&denali->irq_lock);
-			/* our interrupt was detected */
-			break;
-		}
+	denali_reset_irq(denali);
 
-		/*
-		 * these are not the interrupts you are looking for -
-		 * need to wait again
-		 */
-		spin_unlock_irq(&denali->irq_lock);
-	} while (comp_res != 0);
+	iowrite32(1 << denali->flash_bank, denali->flash_reg + DEVICE_RESET);
 
-	if (comp_res == 0) {
-		/* timeout */
-		pr_err("timeout occurred, status = 0x%x, mask = 0x%x\n",
-				intr_status, irq_mask);
+	irq_status = denali_wait_for_irq(denali,
+					 INTR__RST_COMP | INTR__TIME_OUT);
 
-		intr_status = 0;
-	}
-	return intr_status;
+	if (!(irq_status & INTR__RST_COMP))
+		dev_err(denali->dev, "reset bank failed.\n");
 }
 
 /*
@@ -397,7 +315,7 @@ static int denali_send_pipeline_cmd(struct denali_nand_info *denali,
 
 	setup_ecc_for_xfer(denali, ecc_en, transfer_spare);
 
-	clear_interrupts(denali);
+	denali_reset_irq(denali);
 
 	addr = BANK(denali->flash_bank) | denali->page;
 
@@ -479,9 +397,9 @@ static int write_oob_data(struct mtd_info *mtd, uint8_t *buf, int page)
 		write_data_to_flash_mem(denali, buf, mtd->oobsize);
 
 		/* wait for operation to complete */
-		irq_status = wait_for_irq(denali, irq_mask);
+		irq_status = denali_wait_for_irq(denali, irq_mask);
 
-		if (irq_status == 0) {
+		if (!(irq_status & INTR__PROGRAM_COMP)) {
 			dev_err(denali->dev, "OOB write failed\n");
 			status = -EIO;
 		}
@@ -510,9 +428,9 @@ static void read_oob_data(struct mtd_info *mtd, uint8_t *buf, int page)
 		 * can always use status0 bit as the
 		 * mask is identical for each bank.
 		 */
-		irq_status = wait_for_irq(denali, irq_mask);
+		irq_status = denali_wait_for_irq(denali, irq_mask);
 
-		if (irq_status == 0)
+		if (!(irq_status & INTR__LOAD_COMP))
 			dev_err(denali->dev, "page on OOB timeout %d\n",
 					denali->page);
 
@@ -620,9 +538,9 @@ static int denali_sw_ecc_fixup(struct mtd_info *mtd,
 	unsigned int err_byte, err_sector, err_device;
 	uint8_t err_cor_value;
 	unsigned int prev_sector = 0;
+	uint32_t irq_status;
 
-	/* read the ECC errors. we'll ignore them for now */
-	denali_set_intr_modes(denali, false);
+	denali_reset_irq(denali);
 
 	do {
 		err_addr = ioread32(denali->flash_reg + ECC_ERROR_ADDRESS);
@@ -674,10 +592,9 @@ static int denali_sw_ecc_fixup(struct mtd_info *mtd,
 	 * ECC_TRANSACTION_DONE interrupt, so here just wait for
 	 * a while for this interrupt
 	 */
-	while (!(read_interrupt_status(denali) & INTR__ECC_TRANSACTION_DONE))
-		cpu_relax();
-	clear_interrupts(denali);
-	denali_set_intr_modes(denali, true);
+	irq_status = denali_wait_for_irq(denali, INTR__ECC_TRANSACTION_DONE);
+	if (!(irq_status & INTR__ECC_TRANSACTION_DONE))
+		return -EIO;
 
 	return max_bitflips;
 }
@@ -778,15 +695,14 @@ static int write_page(struct mtd_info *mtd, struct nand_chip *chip,
 
 	dma_sync_single_for_device(denali->dev, addr, size, DMA_TO_DEVICE);
 
-	clear_interrupts(denali);
+	denali_reset_irq(denali);
 	denali_enable_dma(denali, true);
 
 	denali_setup_dma(denali, DENALI_WRITE);
 
 	/* wait for operation to complete */
-	irq_status = wait_for_irq(denali, irq_mask);
-
-	if (irq_status == 0) {
+	irq_status = denali_wait_for_irq(denali, irq_mask);
+	if (!(irq_status & INTR__DMA_CMD_COMP)) {
 		dev_err(denali->dev, "timeout on write_page (type = %d)\n",
 			raw_xfer);
 		ret = -EIO;
@@ -865,11 +781,11 @@ static int denali_read_page(struct mtd_info *mtd, struct nand_chip *chip,
 	denali_enable_dma(denali, true);
 	dma_sync_single_for_device(denali->dev, addr, size, DMA_FROM_DEVICE);
 
-	clear_interrupts(denali);
+	denali_reset_irq(denali);
 	denali_setup_dma(denali, DENALI_READ);
 
 	/* wait for operation to complete */
-	irq_status = wait_for_irq(denali, irq_mask);
+	irq_status = denali_wait_for_irq(denali, irq_mask);
 
 	dma_sync_single_for_cpu(denali->dev, addr, size, DMA_FROM_DEVICE);
 
@@ -901,6 +817,7 @@ static int denali_read_page_raw(struct mtd_info *mtd, struct nand_chip *chip,
 	dma_addr_t addr = denali->buf.dma_buf;
 	size_t size = mtd->writesize + mtd->oobsize;
 	uint32_t irq_mask = INTR__DMA_CMD_COMP;
+	uint32_t irq_status;
 
 	denali->page = page;
 
@@ -909,11 +826,13 @@ static int denali_read_page_raw(struct mtd_info *mtd, struct nand_chip *chip,
 
 	dma_sync_single_for_device(denali->dev, addr, size, DMA_FROM_DEVICE);
 
-	clear_interrupts(denali);
+	denali_reset_irq(denali);
 	denali_setup_dma(denali, DENALI_READ);
 
 	/* wait for operation to complete */
-	wait_for_irq(denali, irq_mask);
+	irq_status = denali_wait_for_irq(denali, irq_mask);
+	if (irq_status & INTR__DMA_CMD_COMP)
+		return -ETIMEDOUT;
 
 	dma_sync_single_for_cpu(denali->dev, addr, size, DMA_FROM_DEVICE);
 
@@ -940,9 +859,7 @@ static void denali_select_chip(struct mtd_info *mtd, int chip)
 {
 	struct denali_nand_info *denali = mtd_to_denali(mtd);
 
-	spin_lock_irq(&denali->irq_lock);
 	denali->flash_bank = chip;
-	spin_unlock_irq(&denali->irq_lock);
 }
 
 static int denali_waitfunc(struct mtd_info *mtd, struct nand_chip *chip)
@@ -953,19 +870,19 @@ static int denali_waitfunc(struct mtd_info *mtd, struct nand_chip *chip)
 static int denali_erase(struct mtd_info *mtd, int page)
 {
 	struct denali_nand_info *denali = mtd_to_denali(mtd);
-
 	uint32_t cmd, irq_status;
 
-	clear_interrupts(denali);
+	denali_reset_irq(denali);
 
 	/* setup page read request for access type */
 	cmd = MODE_10 | BANK(denali->flash_bank) | page;
 	index_addr(denali, cmd, 0x1);
 
 	/* wait for erase to complete or failure to occur */
-	irq_status = wait_for_irq(denali, INTR__ERASE_COMP | INTR__ERASE_FAIL);
+	irq_status = denali_wait_for_irq(denali,
+					 INTR__ERASE_COMP | INTR__ERASE_FAIL);
 
-	return irq_status & INTR__ERASE_FAIL ? NAND_STATUS_FAIL : PASS;
+	return irq_status & INTR__ERASE_COMP ? 0 : NAND_STATUS_FAIL;
 }
 
 static void denali_cmdfunc(struct mtd_info *mtd, unsigned int cmd, int col,
@@ -1152,7 +1069,6 @@ static void denali_hw_init(struct denali_nand_info *denali)
 	/* Should set value for these registers when init */
 	iowrite32(0, denali->flash_reg + TWO_ROW_ADDR_CYCLES);
 	iowrite32(1, denali->flash_reg + ECC_ENABLE);
-	denali_irq_init(denali);
 }
 
 int denali_calc_ecc_bytes(int step_size, int strength)
@@ -1264,9 +1180,6 @@ static void denali_drv_init(struct denali_nand_info *denali)
 
 	/* indicate that MTD has not selected a valid bank yet */
 	denali->flash_bank = CHIP_SELECT_INVALID;
-
-	/* initialize our irq_status variable to indicate no interrupts */
-	denali->irq_status = 0;
 }
 
 static int denali_multidev_fixup(struct denali_nand_info *denali)
@@ -1336,6 +1249,8 @@ int denali_init(struct denali_nand_info *denali)
 	denali_hw_init(denali);
 	denali_drv_init(denali);
 
+	denali_clear_irq_all(denali);
+
 	/* Request IRQ after all the hardware initialization is finished */
 	ret = devm_request_irq(denali->dev, denali->irq, denali_isr,
 			       IRQF_SHARED, DENALI_NAND_NAME, denali);
@@ -1344,8 +1259,8 @@ int denali_init(struct denali_nand_info *denali)
 		return ret;
 	}
 
-	/* now that our ISR is registered, we can enable interrupts */
-	denali_set_intr_modes(denali, true);
+	denali_enable_irq(denali);
+
 	nand_set_flash_node(chip, denali->dev->of_node);
 	/* Fallback to the default name if DT did not give "label" property */
 	if (!mtd->name)
@@ -1370,7 +1285,7 @@ int denali_init(struct denali_nand_info *denali)
 	 */
 	ret = nand_scan_ident(mtd, denali->max_banks, NULL);
 	if (ret)
-		goto failed_req_irq;
+		goto disable_irq;
 
 	/* allocate the right size buffer now */
 	devm_kfree(denali->dev, denali->buf.buf);
@@ -1379,7 +1294,7 @@ int denali_init(struct denali_nand_info *denali)
 			     GFP_KERNEL);
 	if (!denali->buf.buf) {
 		ret = -ENOMEM;
-		goto failed_req_irq;
+		goto disable_irq;
 	}
 
 	ret = dma_set_mask(denali->dev,
@@ -1387,7 +1302,7 @@ int denali_init(struct denali_nand_info *denali)
 					64 : 32));
 	if (ret) {
 		dev_err(denali->dev, "No usable DMA configuration\n");
-		goto failed_req_irq;
+		goto disable_irq;
 	}
 
 	denali->buf.dma_buf = dma_map_single(denali->dev, denali->buf.buf,
@@ -1396,7 +1311,7 @@ int denali_init(struct denali_nand_info *denali)
 	if (dma_mapping_error(denali->dev, denali->buf.dma_buf)) {
 		dev_err(denali->dev, "Failed to map DMA buffer\n");
 		ret = -EIO;
-		goto failed_req_irq;
+		goto disable_irq;
 	}
 
 	/*
@@ -1420,7 +1335,7 @@ int denali_init(struct denali_nand_info *denali)
 	ret = denali_ecc_setup(mtd, chip, denali);
 	if (ret) {
 		dev_err(denali->dev, "Failed to setup ECC settings.\n");
-		goto failed_req_irq;
+		goto disable_irq;
 	}
 
 	dev_dbg(denali->dev,
@@ -1454,21 +1369,21 @@ int denali_init(struct denali_nand_info *denali)
 
 	ret = denali_multidev_fixup(denali);
 	if (ret)
-		goto failed_req_irq;
+		goto disable_irq;
 
 	ret = nand_scan_tail(mtd);
 	if (ret)
-		goto failed_req_irq;
+		goto disable_irq;
 
 	ret = mtd_device_register(mtd, NULL, 0);
 	if (ret) {
 		dev_err(denali->dev, "Failed to register MTD: %d\n", ret);
-		goto failed_req_irq;
+		goto disable_irq;
 	}
 	return 0;
 
-failed_req_irq:
-	denali_irq_cleanup(denali->irq, denali);
+disable_irq:
+	denali_disable_irq(denali);
 
 	return ret;
 }
@@ -1486,7 +1401,7 @@ void denali_remove(struct denali_nand_info *denali)
 	int bufsize = mtd->writesize + mtd->oobsize;
 
 	nand_release(mtd);
-	denali_irq_cleanup(denali->irq, denali);
+	denali_disable_irq(denali);
 	dma_unmap_single(denali->dev, denali->buf.dma_buf, bufsize,
 			 DMA_BIDIRECTIONAL);
 }
diff --git a/drivers/mtd/nand/denali.h b/drivers/mtd/nand/denali.h
index fb473895a79d..a0ac0f84f8b5 100644
--- a/drivers/mtd/nand/denali.h
+++ b/drivers/mtd/nand/denali.h
@@ -325,6 +325,7 @@ struct denali_nand_info {
 	/* elements used by ISR */
 	struct completion complete;
 	spinlock_t irq_lock;
+	uint32_t irq_mask;
 	uint32_t irq_status;
 	int irq;
 
-- 
2.7.4

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

* [PATCH v7 05/16] mtd: nand: denali: switch over to cmd_ctrl instead of cmdfunc
  2017-06-13 13:45 [PATCH v7 00/16] mtd: nand: denali: Denali NAND IP improvements Masahiro Yamada
                   ` (3 preceding siblings ...)
  2017-06-13 13:45 ` [PATCH v7 04/16] mtd: nand: denali: rework interrupt handling Masahiro Yamada
@ 2017-06-13 13:45 ` Masahiro Yamada
  2017-06-13 13:45 ` [PATCH v7 06/16] mtd: nand: denali: fix bank reset function to detect the number of chips Masahiro Yamada
                   ` (11 subsequent siblings)
  16 siblings, 0 replies; 19+ messages in thread
From: Masahiro Yamada @ 2017-06-13 13:45 UTC (permalink / raw)
  To: linux-mtd
  Cc: Enrico Jorns, Artem Bityutskiy, Dinh Nguyen, Boris Brezillon,
	Marek Vasut, David Woodhouse, Masami Hiramatsu, Chuanxiao Dong,
	Jassi Brar, Masahiro Yamada, Cyrille Pitchen, linux-kernel,
	Brian Norris, Richard Weinberger

The NAND_CMD_SET_FEATURES support is missing from denali_cmdfunc().
We also see /* TODO: Read OOB data */ comment.

It would be possible to add more commands along with the current
implementation, but having ->cmd_ctrl() seems a better approach from
the discussion with Boris [1].

Rely on the default ->cmdfunc() from the framework and implement the
driver's own ->cmd_ctrl().

This transition also fixes NAND_CMD_STATUS and NAND_CMD_PARAM handling.
NAND_CMD_STATUS was just faked by the register read, so the only valid
bit was the WP bit.  NAND_CMD_PARAM was completely broken; not only the
command sent on the bus was NAND_CMD_STATUS instead of NAND_CMD_PARAM,
but also the driver was only reading 8 bytes, while the parameter page
contains several hundreds of bytes.

Also add ->write_byte(), which is needed for write direction commands,
->read/write_buf(16), which will be used some commits later.
->read_word() is not used for now, but the core may call it in the
future.

Now, this driver can drop nand_onfi_get_set_features_notsupp().

[1] https://lkml.org/lkml/2017/3/15/97

Signed-off-by: Masahiro Yamada <yamada.masahiro@socionext.com>
---

Changes in v7:
  - Squash 3 commits
  - Implement ->read/write_buf(16), ->read_word hooks in this commit

Changes in v6:
  - Remove nand_onfi_get_set_features_notsupp()

Changes in v5: None
Changes in v4: None
Changes in v3: None
Changes in v2:
  - Newly added

 drivers/mtd/nand/denali.c | 232 ++++++++++++++++++++++++----------------------
 drivers/mtd/nand/denali.h |   2 -
 2 files changed, 123 insertions(+), 111 deletions(-)

diff --git a/drivers/mtd/nand/denali.c b/drivers/mtd/nand/denali.c
index 2acdc5f42a00..8091ba0916cc 100644
--- a/drivers/mtd/nand/denali.c
+++ b/drivers/mtd/nand/denali.c
@@ -85,43 +85,6 @@ static void index_addr(struct denali_nand_info *denali,
 	iowrite32(data, denali->flash_mem + 0x10);
 }
 
-/* Perform an indexed read of the device */
-static void index_addr_read_data(struct denali_nand_info *denali,
-				 uint32_t address, uint32_t *pdata)
-{
-	iowrite32(address, denali->flash_mem);
-	*pdata = ioread32(denali->flash_mem + 0x10);
-}
-
-/*
- * We need to buffer some data for some of the NAND core routines.
- * The operations manage buffering that data.
- */
-static void reset_buf(struct denali_nand_info *denali)
-{
-	denali->buf.head = denali->buf.tail = 0;
-}
-
-static void write_byte_to_buf(struct denali_nand_info *denali, uint8_t byte)
-{
-	denali->buf.buf[denali->buf.tail++] = byte;
-}
-
-/* reads the status of the device */
-static void read_status(struct denali_nand_info *denali)
-{
-	uint32_t cmd;
-
-	/* initialize the data buffer to store status */
-	reset_buf(denali);
-
-	cmd = ioread32(denali->flash_reg + WRITE_PROTECT);
-	if (cmd)
-		write_byte_to_buf(denali, NAND_STATUS_WP);
-	else
-		write_byte_to_buf(denali, 0);
-}
-
 /* Reset the flash controller */
 static uint16_t denali_nand_reset(struct denali_nand_info *denali)
 {
@@ -268,20 +231,16 @@ static uint32_t denali_wait_for_irq(struct denali_nand_info *denali,
 	return denali->irq_status;
 }
 
-/* resets a specific device connected to the core */
-static void reset_bank(struct denali_nand_info *denali)
+static uint32_t denali_check_irq(struct denali_nand_info *denali)
 {
+	unsigned long flags;
 	uint32_t irq_status;
 
-	denali_reset_irq(denali);
-
-	iowrite32(1 << denali->flash_bank, denali->flash_reg + DEVICE_RESET);
-
-	irq_status = denali_wait_for_irq(denali,
-					 INTR__RST_COMP | INTR__TIME_OUT);
+	spin_lock_irqsave(&denali->irq_lock, flags);
+	irq_status = denali->irq_status;
+	spin_unlock_irqrestore(&denali->irq_lock, flags);
 
-	if (!(irq_status & INTR__RST_COMP))
-		dev_err(denali->dev, "reset bank failed.\n");
+	return irq_status;
 }
 
 /*
@@ -302,6 +261,105 @@ static void setup_ecc_for_xfer(struct denali_nand_info *denali, bool ecc_en,
 	iowrite32(transfer_spare_flag, denali->flash_reg + TRANSFER_SPARE_REG);
 }
 
+static void denali_read_buf(struct mtd_info *mtd, uint8_t *buf, int len)
+{
+	struct denali_nand_info *denali = mtd_to_denali(mtd);
+	int i;
+
+	iowrite32(MODE_11 | BANK(denali->flash_bank) | 2, denali->flash_mem);
+
+	for (i = 0; i < len; i++)
+		buf[i] = ioread32(denali->flash_mem + 0x10);
+}
+
+static void denali_write_buf(struct mtd_info *mtd, const uint8_t *buf, int len)
+{
+	struct denali_nand_info *denali = mtd_to_denali(mtd);
+	int i;
+
+	iowrite32(MODE_11 | BANK(denali->flash_bank) | 2, denali->flash_mem);
+
+	for (i = 0; i < len; i++)
+		iowrite32(buf[i], denali->flash_mem + 0x10);
+}
+
+static void denali_read_buf16(struct mtd_info *mtd, uint8_t *buf, int len)
+{
+	struct denali_nand_info *denali = mtd_to_denali(mtd);
+	uint16_t *buf16 = (uint16_t *)buf;
+	int i;
+
+	iowrite32(MODE_11 | BANK(denali->flash_bank) | 2, denali->flash_mem);
+
+	for (i = 0; i < len / 2; i++)
+		buf16[i] = ioread32(denali->flash_mem + 0x10);
+}
+
+static void denali_write_buf16(struct mtd_info *mtd, const uint8_t *buf,
+			       int len)
+{
+	struct denali_nand_info *denali = mtd_to_denali(mtd);
+	const uint16_t *buf16 = (const uint16_t *)buf;
+	int i;
+
+	iowrite32(MODE_11 | BANK(denali->flash_bank) | 2, denali->flash_mem);
+
+	for (i = 0; i < len / 2; i++)
+		iowrite32(buf16[i], denali->flash_mem + 0x10);
+}
+
+static uint8_t denali_read_byte(struct mtd_info *mtd)
+{
+	uint8_t byte;
+
+	denali_read_buf(mtd, &byte, 1);
+
+	return byte;
+}
+
+static void denali_write_byte(struct mtd_info *mtd, uint8_t byte)
+{
+	denali_write_buf(mtd, &byte, 1);
+}
+
+static uint16_t denali_read_word(struct mtd_info *mtd)
+{
+	uint16_t word;
+
+	denali_read_buf16(mtd, (uint8_t *)&word, 2);
+
+	return word;
+}
+
+static void denali_cmd_ctrl(struct mtd_info *mtd, int dat, unsigned int ctrl)
+{
+	struct denali_nand_info *denali = mtd_to_denali(mtd);
+	uint32_t type;
+
+	if (ctrl & NAND_CLE)
+		type = 0;
+	else if (ctrl & NAND_ALE)
+		type = 1;
+	else
+		return;
+
+	/*
+	 * Some commands are followed by chip->dev_ready or chip->waitfunc.
+	 * irq_status must be cleared here to catch the R/B# interrupt later.
+	 */
+	if (ctrl & NAND_CTRL_CHANGE)
+		denali_reset_irq(denali);
+
+	index_addr(denali, MODE_11 | BANK(denali->flash_bank) | type, dat);
+}
+
+static int denali_dev_ready(struct mtd_info *mtd)
+{
+	struct denali_nand_info *denali = mtd_to_denali(mtd);
+
+	return !!(denali_check_irq(denali) & INTR__INT_ACT);
+}
+
 /*
  * sends a pipeline command operation to the controller. See the Denali NAND
  * controller's user guide for more information (section 4.2.3.6).
@@ -844,17 +902,6 @@ static int denali_read_page_raw(struct mtd_info *mtd, struct nand_chip *chip,
 	return 0;
 }
 
-static uint8_t denali_read_byte(struct mtd_info *mtd)
-{
-	struct denali_nand_info *denali = mtd_to_denali(mtd);
-	uint8_t result = 0xff;
-
-	if (denali->buf.head < denali->buf.tail)
-		result = denali->buf.buf[denali->buf.head++];
-
-	return result;
-}
-
 static void denali_select_chip(struct mtd_info *mtd, int chip)
 {
 	struct denali_nand_info *denali = mtd_to_denali(mtd);
@@ -864,7 +911,13 @@ static void denali_select_chip(struct mtd_info *mtd, int chip)
 
 static int denali_waitfunc(struct mtd_info *mtd, struct nand_chip *chip)
 {
-	return 0;
+	struct denali_nand_info *denali = mtd_to_denali(mtd);
+	uint32_t irq_status;
+
+	/* R/B# pin transitioned from low to high? */
+	irq_status = denali_wait_for_irq(denali, INTR__INT_ACT);
+
+	return irq_status & INTR__INT_ACT ? 0 : NAND_STATUS_FAIL;
 }
 
 static int denali_erase(struct mtd_info *mtd, int page)
@@ -885,45 +938,6 @@ static int denali_erase(struct mtd_info *mtd, int page)
 	return irq_status & INTR__ERASE_COMP ? 0 : NAND_STATUS_FAIL;
 }
 
-static void denali_cmdfunc(struct mtd_info *mtd, unsigned int cmd, int col,
-			   int page)
-{
-	struct denali_nand_info *denali = mtd_to_denali(mtd);
-	uint32_t addr, id;
-	int i;
-
-	switch (cmd) {
-	case NAND_CMD_STATUS:
-		read_status(denali);
-		break;
-	case NAND_CMD_READID:
-	case NAND_CMD_PARAM:
-		reset_buf(denali);
-		/*
-		 * sometimes ManufactureId read from register is not right
-		 * e.g. some of Micron MT29F32G08QAA MLC NAND chips
-		 * So here we send READID cmd to NAND insteand
-		 */
-		addr = MODE_11 | BANK(denali->flash_bank);
-		index_addr(denali, addr | 0, 0x90);
-		index_addr(denali, addr | 1, col);
-		for (i = 0; i < 8; i++) {
-			index_addr_read_data(denali, addr | 2, &id);
-			write_byte_to_buf(denali, id);
-		}
-		break;
-	case NAND_CMD_RESET:
-		reset_bank(denali);
-		break;
-	case NAND_CMD_READOOB:
-		/* TODO: Read OOB data */
-		break;
-	default:
-		pr_err(": unsupported command received 0x%x\n", cmd);
-		break;
-	}
-}
-
 #define DIV_ROUND_DOWN_ULL(ll, d) \
 	({ unsigned long long _tmp = (ll); do_div(_tmp, d); _tmp; })
 
@@ -1239,12 +1253,6 @@ int denali_init(struct denali_nand_info *denali)
 	struct mtd_info *mtd = nand_to_mtd(chip);
 	int ret;
 
-	/* allocate a temporary buffer for nand_scan_ident() */
-	denali->buf.buf = devm_kzalloc(denali->dev, PAGE_SIZE,
-					GFP_DMA | GFP_KERNEL);
-	if (!denali->buf.buf)
-		return -ENOMEM;
-
 	mtd->dev.parent = denali->dev;
 	denali_hw_init(denali);
 	denali_drv_init(denali);
@@ -1268,11 +1276,12 @@ int denali_init(struct denali_nand_info *denali)
 
 	/* register the driver with the NAND core subsystem */
 	chip->select_chip = denali_select_chip;
-	chip->cmdfunc = denali_cmdfunc;
 	chip->read_byte = denali_read_byte;
+	chip->write_byte = denali_write_byte;
+	chip->read_word = denali_read_word;
+	chip->cmd_ctrl = denali_cmd_ctrl;
+	chip->dev_ready = denali_dev_ready;
 	chip->waitfunc = denali_waitfunc;
-	chip->onfi_set_features = nand_onfi_get_set_features_notsupp;
-	chip->onfi_get_features = nand_onfi_get_set_features_notsupp;
 
 	/* clk rate info is needed for setup_data_interface */
 	if (denali->clk_x_rate)
@@ -1287,8 +1296,6 @@ int denali_init(struct denali_nand_info *denali)
 	if (ret)
 		goto disable_irq;
 
-	/* allocate the right size buffer now */
-	devm_kfree(denali->dev, denali->buf.buf);
 	denali->buf.buf = devm_kzalloc(denali->dev,
 			     mtd->writesize + mtd->oobsize,
 			     GFP_KERNEL);
@@ -1358,6 +1365,13 @@ int denali_init(struct denali_nand_info *denali)
 
 	mtd_set_ooblayout(mtd, &denali_ooblayout_ops);
 
+	if (chip->options & NAND_BUSWIDTH_16) {
+		chip->read_buf = denali_read_buf16;
+		chip->write_buf = denali_write_buf16;
+	} else {
+		chip->read_buf = denali_read_buf;
+		chip->write_buf = denali_write_buf;
+	}
 	chip->ecc.options |= NAND_ECC_CUSTOM_PAGE_ACCESS;
 	chip->ecc.read_page = denali_read_page;
 	chip->ecc.read_page_raw = denali_read_page_raw;
diff --git a/drivers/mtd/nand/denali.h b/drivers/mtd/nand/denali.h
index a0ac0f84f8b5..a84d8784ee98 100644
--- a/drivers/mtd/nand/denali.h
+++ b/drivers/mtd/nand/denali.h
@@ -306,8 +306,6 @@
 #define MODE_11    0x0C000000
 
 struct nand_buf {
-	int head;
-	int tail;
 	uint8_t *buf;
 	dma_addr_t dma_buf;
 };
-- 
2.7.4

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

* [PATCH v7 06/16] mtd: nand: denali: fix bank reset function to detect the number of chips
  2017-06-13 13:45 [PATCH v7 00/16] mtd: nand: denali: Denali NAND IP improvements Masahiro Yamada
                   ` (4 preceding siblings ...)
  2017-06-13 13:45 ` [PATCH v7 05/16] mtd: nand: denali: switch over to cmd_ctrl instead of cmdfunc Masahiro Yamada
@ 2017-06-13 13:45 ` Masahiro Yamada
  2017-06-13 13:45 ` [PATCH v7 07/16] mtd: nand: denali: use interrupt instead of polling for bank reset Masahiro Yamada
                   ` (10 subsequent siblings)
  16 siblings, 0 replies; 19+ messages in thread
From: Masahiro Yamada @ 2017-06-13 13:45 UTC (permalink / raw)
  To: linux-mtd
  Cc: Enrico Jorns, Artem Bityutskiy, Dinh Nguyen, Boris Brezillon,
	Marek Vasut, David Woodhouse, Masami Hiramatsu, Chuanxiao Dong,
	Jassi Brar, Masahiro Yamada, Cyrille Pitchen, linux-kernel,
	Brian Norris, Richard Weinberger

The nand_scan_ident() iterates over maxchips, and calls nand_reset()
for each.  This driver currently passes the maximum number of banks
(=chip selects) supported by the controller as maxchips.  So, maxchips
is typically 4 or 8.  Usually, less number of NAND chips are connected
to the controller.

This can be a problem for ONFi devices.  Now, this driver implements
->setup_data_interface() hook, so nand_setup_data_interface() issues
Set Features (0xEF) command, which waits until the chip returns R/B#
response.  If no chip there, we know it never happens, but the driver
still ends up with waiting for a long time.  It will finally bail-out
with timeout error and the driver will work with existing chips, but
unnecessary wait will give a bad user experience.

The denali_nand_reset() polls the INTR__RST_COMP and INTR__TIME_OUT
bits, but they are always set even if not NAND chip is connected to
that bank.  To know the chip existence, INTR__INT_ACT bit must be
checked; this flag is set only when R/B# is toggled.  Since the Reset
(0xFF) command toggles the R/B# pin, this can be used to know the
actual number of chips, and update denali->max_banks.

Signed-off-by: Masahiro Yamada <yamada.masahiro@socionext.com>
---

Boris mentioned this information can be retrieved from DT
(http://patchwork.ozlabs.org/patch/745118/), but I'd like to
take time for controller/chip decoupling.  I am tackling on
that, but not completed yet.

I believe this commit stands for denali_pci, at least I do not
know how to get the number of chips from PCI.


Changes in v7: None
Changes in v6: None
Changes in v5: None
Changes in v4:
  - Reword commit-log

Changes in v3: None
Changes in v2:
  - Newly added

 drivers/mtd/nand/denali.c | 52 +++++++++++++++++++++--------------------------
 1 file changed, 23 insertions(+), 29 deletions(-)

diff --git a/drivers/mtd/nand/denali.c b/drivers/mtd/nand/denali.c
index 8091ba0916cc..3169ba58c58a 100644
--- a/drivers/mtd/nand/denali.c
+++ b/drivers/mtd/nand/denali.c
@@ -85,33 +85,6 @@ static void index_addr(struct denali_nand_info *denali,
 	iowrite32(data, denali->flash_mem + 0x10);
 }
 
-/* Reset the flash controller */
-static uint16_t denali_nand_reset(struct denali_nand_info *denali)
-{
-	int i;
-
-	for (i = 0; i < denali->max_banks; i++)
-		iowrite32(INTR__RST_COMP | INTR__TIME_OUT,
-		denali->flash_reg + INTR_STATUS(i));
-
-	for (i = 0; i < denali->max_banks; i++) {
-		iowrite32(1 << i, denali->flash_reg + DEVICE_RESET);
-		while (!(ioread32(denali->flash_reg + INTR_STATUS(i)) &
-			(INTR__RST_COMP | INTR__TIME_OUT)))
-			cpu_relax();
-		if (ioread32(denali->flash_reg + INTR_STATUS(i)) &
-			INTR__TIME_OUT)
-			dev_dbg(denali->dev,
-			"NAND Reset operation timed out on bank %d\n", i);
-	}
-
-	for (i = 0; i < denali->max_banks; i++)
-		iowrite32(INTR__RST_COMP | INTR__TIME_OUT,
-			  denali->flash_reg + INTR_STATUS(i));
-
-	return PASS;
-}
-
 /*
  * Use the configuration feature register to determine the maximum number of
  * banks that the hardware supports.
@@ -1053,7 +1026,28 @@ static int denali_setup_data_interface(struct mtd_info *mtd, int chipnr,
 	return 0;
 }
 
-/* Initialization code to bring the device up to a known good state */
+static void denali_reset_banks(struct denali_nand_info *denali)
+{
+	int i;
+
+	denali_clear_irq_all(denali);
+
+	for (i = 0; i < denali->max_banks; i++) {
+		iowrite32(1 << i, denali->flash_reg + DEVICE_RESET);
+		while (!(ioread32(denali->flash_reg + INTR_STATUS(i)) &
+			(INTR__RST_COMP | INTR__TIME_OUT)))
+			cpu_relax();
+		if (!(ioread32(denali->flash_reg + INTR_STATUS(i)) &
+		      INTR__INT_ACT))
+			break;
+	}
+
+	dev_dbg(denali->dev, "%d chips connected\n", i);
+	denali->max_banks = i;
+
+	denali_clear_irq_all(denali);
+}
+
 static void denali_hw_init(struct denali_nand_info *denali)
 {
 	/*
@@ -1073,7 +1067,7 @@ static void denali_hw_init(struct denali_nand_info *denali)
 	denali->bbtskipbytes = ioread32(denali->flash_reg +
 						SPARE_AREA_SKIP_BYTES);
 	detect_max_banks(denali);
-	denali_nand_reset(denali);
+	denali_reset_banks(denali);
 	iowrite32(0x0F, denali->flash_reg + RB_PIN_ENABLED);
 	iowrite32(CHIP_EN_DONT_CARE__FLAG,
 			denali->flash_reg + CHIP_ENABLE_DONT_CARE);
-- 
2.7.4

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

* [PATCH v7 07/16] mtd: nand: denali: use interrupt instead of polling for bank reset
  2017-06-13 13:45 [PATCH v7 00/16] mtd: nand: denali: Denali NAND IP improvements Masahiro Yamada
                   ` (5 preceding siblings ...)
  2017-06-13 13:45 ` [PATCH v7 06/16] mtd: nand: denali: fix bank reset function to detect the number of chips Masahiro Yamada
@ 2017-06-13 13:45 ` Masahiro Yamada
  2017-06-13 13:45 ` [PATCH v7 08/16] mtd: nand: denali: propagate page to helpers via function argument Masahiro Yamada
                   ` (9 subsequent siblings)
  16 siblings, 0 replies; 19+ messages in thread
From: Masahiro Yamada @ 2017-06-13 13:45 UTC (permalink / raw)
  To: linux-mtd
  Cc: Enrico Jorns, Artem Bityutskiy, Dinh Nguyen, Boris Brezillon,
	Marek Vasut, David Woodhouse, Masami Hiramatsu, Chuanxiao Dong,
	Jassi Brar, Masahiro Yamada, Cyrille Pitchen, linux-kernel,
	Brian Norris, Richard Weinberger

The current bank reset implementation polls the INTR_STATUS register
until interested bits are set.  This is not good because:

- polling simply wastes time-slice of the thread

- The while() loop may continue eternally if no bit is set, for
  example, due to the controller problem.  The denali_wait_for_irq()
  uses wait_for_completion_timeout(), which is safer.

We can use interrupt by moving the denali_reset_bank() call below
the interrupt setup.

Signed-off-by: Masahiro Yamada <yamada.masahiro@socionext.com>
---

Changes in v7: None
Changes in v6: None
Changes in v5: None
Changes in v4: None
Changes in v3: None
Changes in v2:
  - Newly added

 drivers/mtd/nand/denali.c | 28 ++++++++++++++--------------
 1 file changed, 14 insertions(+), 14 deletions(-)

diff --git a/drivers/mtd/nand/denali.c b/drivers/mtd/nand/denali.c
index 3169ba58c58a..31d987d26e12 100644
--- a/drivers/mtd/nand/denali.c
+++ b/drivers/mtd/nand/denali.c
@@ -1028,24 +1028,25 @@ static int denali_setup_data_interface(struct mtd_info *mtd, int chipnr,
 
 static void denali_reset_banks(struct denali_nand_info *denali)
 {
+	u32 irq_status;
 	int i;
 
-	denali_clear_irq_all(denali);
-
 	for (i = 0; i < denali->max_banks; i++) {
-		iowrite32(1 << i, denali->flash_reg + DEVICE_RESET);
-		while (!(ioread32(denali->flash_reg + INTR_STATUS(i)) &
-			(INTR__RST_COMP | INTR__TIME_OUT)))
-			cpu_relax();
-		if (!(ioread32(denali->flash_reg + INTR_STATUS(i)) &
-		      INTR__INT_ACT))
+		denali->flash_bank = i;
+
+		denali_reset_irq(denali);
+
+		iowrite32(DEVICE_RESET__BANK(i),
+			  denali->flash_reg + DEVICE_RESET);
+
+		irq_status = denali_wait_for_irq(denali,
+			INTR__RST_COMP | INTR__INT_ACT | INTR__TIME_OUT);
+		if (!(irq_status & INTR__INT_ACT))
 			break;
 	}
 
 	dev_dbg(denali->dev, "%d chips connected\n", i);
 	denali->max_banks = i;
-
-	denali_clear_irq_all(denali);
 }
 
 static void denali_hw_init(struct denali_nand_info *denali)
@@ -1067,7 +1068,6 @@ static void denali_hw_init(struct denali_nand_info *denali)
 	denali->bbtskipbytes = ioread32(denali->flash_reg +
 						SPARE_AREA_SKIP_BYTES);
 	detect_max_banks(denali);
-	denali_reset_banks(denali);
 	iowrite32(0x0F, denali->flash_reg + RB_PIN_ENABLED);
 	iowrite32(CHIP_EN_DONT_CARE__FLAG,
 			denali->flash_reg + CHIP_ENABLE_DONT_CARE);
@@ -1185,9 +1185,6 @@ static void denali_drv_init(struct denali_nand_info *denali)
 	 * element that might be access shared data (interrupt status)
 	 */
 	spin_lock_init(&denali->irq_lock);
-
-	/* indicate that MTD has not selected a valid bank yet */
-	denali->flash_bank = CHIP_SELECT_INVALID;
 }
 
 static int denali_multidev_fixup(struct denali_nand_info *denali)
@@ -1262,6 +1259,9 @@ int denali_init(struct denali_nand_info *denali)
 	}
 
 	denali_enable_irq(denali);
+	denali_reset_banks(denali);
+
+	denali->flash_bank = CHIP_SELECT_INVALID;
 
 	nand_set_flash_node(chip, denali->dev->of_node);
 	/* Fallback to the default name if DT did not give "label" property */
-- 
2.7.4

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

* [PATCH v7 08/16] mtd: nand: denali: propagate page to helpers via function argument
  2017-06-13 13:45 [PATCH v7 00/16] mtd: nand: denali: Denali NAND IP improvements Masahiro Yamada
                   ` (6 preceding siblings ...)
  2017-06-13 13:45 ` [PATCH v7 07/16] mtd: nand: denali: use interrupt instead of polling for bank reset Masahiro Yamada
@ 2017-06-13 13:45 ` Masahiro Yamada
  2017-06-13 13:45 ` [PATCH v7 09/16] mtd: nand: denali: merge struct nand_buf into struct denali_nand_info Masahiro Yamada
                   ` (8 subsequent siblings)
  16 siblings, 0 replies; 19+ messages in thread
From: Masahiro Yamada @ 2017-06-13 13:45 UTC (permalink / raw)
  To: linux-mtd
  Cc: Enrico Jorns, Artem Bityutskiy, Dinh Nguyen, Boris Brezillon,
	Marek Vasut, David Woodhouse, Masami Hiramatsu, Chuanxiao Dong,
	Jassi Brar, Masahiro Yamada, Cyrille Pitchen, linux-kernel,
	Brian Norris, Richard Weinberger

This driver stores the currently addressed page into denali->page,
which is later read out by helper functions.  While I am tackling on
this driver, I often missed to insert "denali->page = page;" where
needed.  This makes page_read/write callbacks to get access to a
wrong page, which is a bug hard to figure out.

Instead, I'd rather pass the page via function argument because the
compiler's prototype checks will help to detect bugs.

For the same reason, propagate dma_addr to the DMA helpers instead
of denali->buf.dma_buf .

Signed-off-by: Masahiro Yamada <yamada.masahiro@socionext.com>
---

Changes in v7: None
Changes in v6: None
Changes in v5: None
Changes in v4: None
Changes in v3: None
Changes in v2:
  - Newly added

 drivers/mtd/nand/denali.c | 58 ++++++++++++++++++++---------------------------
 drivers/mtd/nand/denali.h |  1 -
 2 files changed, 24 insertions(+), 35 deletions(-)

diff --git a/drivers/mtd/nand/denali.c b/drivers/mtd/nand/denali.c
index 31d987d26e12..24849f6bb37b 100644
--- a/drivers/mtd/nand/denali.c
+++ b/drivers/mtd/nand/denali.c
@@ -337,7 +337,7 @@ static int denali_dev_ready(struct mtd_info *mtd)
  * sends a pipeline command operation to the controller. See the Denali NAND
  * controller's user guide for more information (section 4.2.3.6).
  */
-static int denali_send_pipeline_cmd(struct denali_nand_info *denali,
+static int denali_send_pipeline_cmd(struct denali_nand_info *denali, int page,
 				    bool ecc_en, bool transfer_spare,
 				    int access_type, int op)
 {
@@ -348,7 +348,7 @@ static int denali_send_pipeline_cmd(struct denali_nand_info *denali,
 
 	denali_reset_irq(denali);
 
-	addr = BANK(denali->flash_bank) | denali->page;
+	addr = BANK(denali->flash_bank) | page;
 
 	if (op == DENALI_WRITE && access_type != SPARE_ACCESS) {
 		cmd = MODE_01 | addr;
@@ -421,9 +421,7 @@ static int write_oob_data(struct mtd_info *mtd, uint8_t *buf, int page)
 	uint32_t irq_mask = INTR__PROGRAM_COMP | INTR__PROGRAM_FAIL;
 	int status = 0;
 
-	denali->page = page;
-
-	if (denali_send_pipeline_cmd(denali, false, false, SPARE_ACCESS,
+	if (denali_send_pipeline_cmd(denali, page, false, false, SPARE_ACCESS,
 							DENALI_WRITE) == PASS) {
 		write_data_to_flash_mem(denali, buf, mtd->oobsize);
 
@@ -448,9 +446,7 @@ static void read_oob_data(struct mtd_info *mtd, uint8_t *buf, int page)
 	uint32_t irq_mask = INTR__LOAD_COMP;
 	uint32_t irq_status, addr, cmd;
 
-	denali->page = page;
-
-	if (denali_send_pipeline_cmd(denali, false, true, SPARE_ACCESS,
+	if (denali_send_pipeline_cmd(denali, page, false, true, SPARE_ACCESS,
 							DENALI_READ) == PASS) {
 		read_data_from_flash_mem(denali, buf, mtd->oobsize);
 
@@ -462,8 +458,7 @@ static void read_oob_data(struct mtd_info *mtd, uint8_t *buf, int page)
 		irq_status = denali_wait_for_irq(denali, irq_mask);
 
 		if (!(irq_status & INTR__LOAD_COMP))
-			dev_err(denali->dev, "page on OOB timeout %d\n",
-					denali->page);
+			dev_err(denali->dev, "page on OOB timeout %d\n", page);
 
 		/*
 		 * We set the device back to MAIN_ACCESS here as I observed
@@ -472,7 +467,7 @@ static void read_oob_data(struct mtd_info *mtd, uint8_t *buf, int page)
 		 * is reliable (according to the MTD test infrastructure)
 		 * if you are in MAIN_ACCESS.
 		 */
-		addr = BANK(denali->flash_bank) | denali->page;
+		addr = BANK(denali->flash_bank) | page;
 		cmd = MODE_10 | addr;
 		index_addr(denali, cmd, MAIN_ACCESS);
 	}
@@ -637,13 +632,13 @@ static void denali_enable_dma(struct denali_nand_info *denali, bool en)
 	ioread32(denali->flash_reg + DMA_ENABLE);
 }
 
-static void denali_setup_dma64(struct denali_nand_info *denali, int op)
+static void denali_setup_dma64(struct denali_nand_info *denali,
+			       dma_addr_t dma_addr, int page, int op)
 {
 	uint32_t mode;
 	const int page_count = 1;
-	uint64_t addr = denali->buf.dma_buf;
 
-	mode = MODE_10 | BANK(denali->flash_bank) | denali->page;
+	mode = MODE_10 | BANK(denali->flash_bank) | page;
 
 	/* DMA is a three step process */
 
@@ -654,41 +649,42 @@ static void denali_setup_dma64(struct denali_nand_info *denali, int op)
 	index_addr(denali, mode, 0x01002000 | (64 << 16) | op | page_count);
 
 	/* 2. set memory low address */
-	index_addr(denali, mode, addr);
+	index_addr(denali, mode, dma_addr);
 
 	/* 3. set memory high address */
-	index_addr(denali, mode, addr >> 32);
+	index_addr(denali, mode, (uint64_t)dma_addr >> 32);
 }
 
-static void denali_setup_dma32(struct denali_nand_info *denali, int op)
+static void denali_setup_dma32(struct denali_nand_info *denali,
+			       dma_addr_t dma_addr, int page, int op)
 {
 	uint32_t mode;
 	const int page_count = 1;
-	uint32_t addr = denali->buf.dma_buf;
 
 	mode = MODE_10 | BANK(denali->flash_bank);
 
 	/* DMA is a four step process */
 
 	/* 1. setup transfer type and # of pages */
-	index_addr(denali, mode | denali->page, 0x2000 | op | page_count);
+	index_addr(denali, mode | page, 0x2000 | op | page_count);
 
 	/* 2. set memory high address bits 23:8 */
-	index_addr(denali, mode | ((addr >> 16) << 8), 0x2200);
+	index_addr(denali, mode | ((dma_addr >> 16) << 8), 0x2200);
 
 	/* 3. set memory low address bits 23:8 */
-	index_addr(denali, mode | ((addr & 0xffff) << 8), 0x2300);
+	index_addr(denali, mode | ((dma_addr & 0xffff) << 8), 0x2300);
 
 	/* 4. interrupt when complete, burst len = 64 bytes */
 	index_addr(denali, mode | 0x14000, 0x2400);
 }
 
-static void denali_setup_dma(struct denali_nand_info *denali, int op)
+static void denali_setup_dma(struct denali_nand_info *denali,
+			     dma_addr_t dma_addr, int page, int op)
 {
 	if (denali->caps & DENALI_CAP_DMA_64BIT)
-		denali_setup_dma64(denali, op);
+		denali_setup_dma64(denali, dma_addr, page, op);
 	else
-		denali_setup_dma32(denali, op);
+		denali_setup_dma32(denali, dma_addr, page, op);
 }
 
 /*
@@ -705,8 +701,6 @@ static int write_page(struct mtd_info *mtd, struct nand_chip *chip,
 	uint32_t irq_mask = INTR__DMA_CMD_COMP | INTR__PROGRAM_FAIL;
 	int ret = 0;
 
-	denali->page = page;
-
 	/*
 	 * if it is a raw xfer, we want to disable ecc and send the spare area.
 	 * !raw_xfer - enable ecc
@@ -729,7 +723,7 @@ static int write_page(struct mtd_info *mtd, struct nand_chip *chip,
 	denali_reset_irq(denali);
 	denali_enable_dma(denali, true);
 
-	denali_setup_dma(denali, DENALI_WRITE);
+	denali_setup_dma(denali, addr, page, DENALI_WRITE);
 
 	/* wait for operation to complete */
 	irq_status = denali_wait_for_irq(denali, irq_mask);
@@ -805,15 +799,13 @@ static int denali_read_page(struct mtd_info *mtd, struct nand_chip *chip,
 	unsigned long uncor_ecc_flags = 0;
 	int stat = 0;
 
-	denali->page = page;
-
 	setup_ecc_for_xfer(denali, true, false);
 
 	denali_enable_dma(denali, true);
 	dma_sync_single_for_device(denali->dev, addr, size, DMA_FROM_DEVICE);
 
 	denali_reset_irq(denali);
-	denali_setup_dma(denali, DENALI_READ);
+	denali_setup_dma(denali, addr, page, DENALI_READ);
 
 	/* wait for operation to complete */
 	irq_status = denali_wait_for_irq(denali, irq_mask);
@@ -832,7 +824,7 @@ static int denali_read_page(struct mtd_info *mtd, struct nand_chip *chip,
 		return stat;
 
 	if (uncor_ecc_flags) {
-		read_oob_data(mtd, chip->oob_poi, denali->page);
+		read_oob_data(mtd, chip->oob_poi, page);
 
 		stat = denali_check_erased_page(mtd, chip, buf,
 						uncor_ecc_flags, stat);
@@ -850,15 +842,13 @@ static int denali_read_page_raw(struct mtd_info *mtd, struct nand_chip *chip,
 	uint32_t irq_mask = INTR__DMA_CMD_COMP;
 	uint32_t irq_status;
 
-	denali->page = page;
-
 	setup_ecc_for_xfer(denali, false, true);
 	denali_enable_dma(denali, true);
 
 	dma_sync_single_for_device(denali->dev, addr, size, DMA_FROM_DEVICE);
 
 	denali_reset_irq(denali);
-	denali_setup_dma(denali, DENALI_READ);
+	denali_setup_dma(denali, addr, page, DENALI_READ);
 
 	/* wait for operation to complete */
 	irq_status = denali_wait_for_irq(denali, irq_mask);
diff --git a/drivers/mtd/nand/denali.h b/drivers/mtd/nand/denali.h
index a84d8784ee98..ad2223d179d0 100644
--- a/drivers/mtd/nand/denali.h
+++ b/drivers/mtd/nand/denali.h
@@ -316,7 +316,6 @@ struct denali_nand_info {
 	int flash_bank; /* currently selected chip */
 	struct nand_buf buf;
 	struct device *dev;
-	int page;
 	void __iomem *flash_reg;	/* Register Interface */
 	void __iomem *flash_mem;	/* Host Data/Command Interface */
 
-- 
2.7.4

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

* [PATCH v7 09/16] mtd: nand: denali: merge struct nand_buf into struct denali_nand_info
  2017-06-13 13:45 [PATCH v7 00/16] mtd: nand: denali: Denali NAND IP improvements Masahiro Yamada
                   ` (7 preceding siblings ...)
  2017-06-13 13:45 ` [PATCH v7 08/16] mtd: nand: denali: propagate page to helpers via function argument Masahiro Yamada
@ 2017-06-13 13:45 ` Masahiro Yamada
  2017-06-13 13:45 ` [PATCH v7 10/16] mtd: nand: denali: use flag instead of register macro for direction Masahiro Yamada
                   ` (7 subsequent siblings)
  16 siblings, 0 replies; 19+ messages in thread
From: Masahiro Yamada @ 2017-06-13 13:45 UTC (permalink / raw)
  To: linux-mtd
  Cc: Enrico Jorns, Artem Bityutskiy, Dinh Nguyen, Boris Brezillon,
	Marek Vasut, David Woodhouse, Masami Hiramatsu, Chuanxiao Dong,
	Jassi Brar, Masahiro Yamada, Cyrille Pitchen, linux-kernel,
	Brian Norris, Richard Weinberger

Now struct nand_buf has only two members, so I see no reason for the
separation.

Signed-off-by: Masahiro Yamada <yamada.masahiro@socionext.com>
---

Changes in v7: None
Changes in v6: None
Changes in v5: None
Changes in v4: None
Changes in v3: None
Changes in v2:
  - Newly added

 drivers/mtd/nand/denali.c | 29 ++++++++++++++---------------
 drivers/mtd/nand/denali.h |  8 ++------
 2 files changed, 16 insertions(+), 21 deletions(-)

diff --git a/drivers/mtd/nand/denali.c b/drivers/mtd/nand/denali.c
index 24849f6bb37b..4ba8ad610381 100644
--- a/drivers/mtd/nand/denali.c
+++ b/drivers/mtd/nand/denali.c
@@ -695,7 +695,7 @@ static int write_page(struct mtd_info *mtd, struct nand_chip *chip,
 			const uint8_t *buf, int page, bool raw_xfer)
 {
 	struct denali_nand_info *denali = mtd_to_denali(mtd);
-	dma_addr_t addr = denali->buf.dma_buf;
+	dma_addr_t addr = denali->dma_addr;
 	size_t size = mtd->writesize + mtd->oobsize;
 	uint32_t irq_status;
 	uint32_t irq_mask = INTR__DMA_CMD_COMP | INTR__PROGRAM_FAIL;
@@ -709,11 +709,11 @@ static int write_page(struct mtd_info *mtd, struct nand_chip *chip,
 	setup_ecc_for_xfer(denali, !raw_xfer, raw_xfer);
 
 	/* copy buffer into DMA buffer */
-	memcpy(denali->buf.buf, buf, mtd->writesize);
+	memcpy(denali->buf, buf, mtd->writesize);
 
 	if (raw_xfer) {
 		/* transfer the data to the spare area */
-		memcpy(denali->buf.buf + mtd->writesize,
+		memcpy(denali->buf + mtd->writesize,
 			chip->oob_poi,
 			mtd->oobsize);
 	}
@@ -790,7 +790,7 @@ static int denali_read_page(struct mtd_info *mtd, struct nand_chip *chip,
 			    uint8_t *buf, int oob_required, int page)
 {
 	struct denali_nand_info *denali = mtd_to_denali(mtd);
-	dma_addr_t addr = denali->buf.dma_buf;
+	dma_addr_t addr = denali->dma_addr;
 	size_t size = mtd->writesize + mtd->oobsize;
 	uint32_t irq_status;
 	uint32_t irq_mask = denali->caps & DENALI_CAP_HW_ECC_FIXUP ?
@@ -812,7 +812,7 @@ static int denali_read_page(struct mtd_info *mtd, struct nand_chip *chip,
 
 	dma_sync_single_for_cpu(denali->dev, addr, size, DMA_FROM_DEVICE);
 
-	memcpy(buf, denali->buf.buf, mtd->writesize);
+	memcpy(buf, denali->buf, mtd->writesize);
 
 	if (denali->caps & DENALI_CAP_HW_ECC_FIXUP)
 		stat = denali_hw_ecc_fixup(mtd, denali, &uncor_ecc_flags);
@@ -837,7 +837,7 @@ static int denali_read_page_raw(struct mtd_info *mtd, struct nand_chip *chip,
 				uint8_t *buf, int oob_required, int page)
 {
 	struct denali_nand_info *denali = mtd_to_denali(mtd);
-	dma_addr_t addr = denali->buf.dma_buf;
+	dma_addr_t addr = denali->dma_addr;
 	size_t size = mtd->writesize + mtd->oobsize;
 	uint32_t irq_mask = INTR__DMA_CMD_COMP;
 	uint32_t irq_status;
@@ -859,8 +859,8 @@ static int denali_read_page_raw(struct mtd_info *mtd, struct nand_chip *chip,
 
 	denali_enable_dma(denali, false);
 
-	memcpy(buf, denali->buf.buf, mtd->writesize);
-	memcpy(chip->oob_poi, denali->buf.buf + mtd->writesize, mtd->oobsize);
+	memcpy(buf, denali->buf, mtd->writesize);
+	memcpy(chip->oob_poi, denali->buf + mtd->writesize, mtd->oobsize);
 
 	return 0;
 }
@@ -1280,10 +1280,9 @@ int denali_init(struct denali_nand_info *denali)
 	if (ret)
 		goto disable_irq;
 
-	denali->buf.buf = devm_kzalloc(denali->dev,
-			     mtd->writesize + mtd->oobsize,
-			     GFP_KERNEL);
-	if (!denali->buf.buf) {
+	denali->buf = devm_kzalloc(denali->dev, mtd->writesize + mtd->oobsize,
+				   GFP_KERNEL);
+	if (!denali->buf) {
 		ret = -ENOMEM;
 		goto disable_irq;
 	}
@@ -1296,10 +1295,10 @@ int denali_init(struct denali_nand_info *denali)
 		goto disable_irq;
 	}
 
-	denali->buf.dma_buf = dma_map_single(denali->dev, denali->buf.buf,
+	denali->dma_addr = dma_map_single(denali->dev, denali->buf,
 			     mtd->writesize + mtd->oobsize,
 			     DMA_BIDIRECTIONAL);
-	if (dma_mapping_error(denali->dev, denali->buf.dma_buf)) {
+	if (dma_mapping_error(denali->dev, denali->dma_addr)) {
 		dev_err(denali->dev, "Failed to map DMA buffer\n");
 		ret = -EIO;
 		goto disable_irq;
@@ -1400,7 +1399,7 @@ void denali_remove(struct denali_nand_info *denali)
 
 	nand_release(mtd);
 	denali_disable_irq(denali);
-	dma_unmap_single(denali->dev, denali->buf.dma_buf, bufsize,
+	dma_unmap_single(denali->dev, denali->dma_addr, bufsize,
 			 DMA_BIDIRECTIONAL);
 }
 EXPORT_SYMBOL(denali_remove);
diff --git a/drivers/mtd/nand/denali.h b/drivers/mtd/nand/denali.h
index ad2223d179d0..1b991d3016f8 100644
--- a/drivers/mtd/nand/denali.h
+++ b/drivers/mtd/nand/denali.h
@@ -305,16 +305,10 @@
 #define MODE_10    0x08000000
 #define MODE_11    0x0C000000
 
-struct nand_buf {
-	uint8_t *buf;
-	dma_addr_t dma_buf;
-};
-
 struct denali_nand_info {
 	struct nand_chip nand;
 	unsigned long clk_x_rate;	/* bus interface clock rate */
 	int flash_bank; /* currently selected chip */
-	struct nand_buf buf;
 	struct device *dev;
 	void __iomem *flash_reg;	/* Register Interface */
 	void __iomem *flash_mem;	/* Host Data/Command Interface */
@@ -326,6 +320,8 @@ struct denali_nand_info {
 	uint32_t irq_status;
 	int irq;
 
+	void *buf;
+	dma_addr_t dma_addr;
 	int devnum;	/* represent how many nands connected */
 	int bbtskipbytes;
 	int max_banks;
-- 
2.7.4

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

* [PATCH v7 10/16] mtd: nand: denali: use flag instead of register macro for direction
  2017-06-13 13:45 [PATCH v7 00/16] mtd: nand: denali: Denali NAND IP improvements Masahiro Yamada
                   ` (8 preceding siblings ...)
  2017-06-13 13:45 ` [PATCH v7 09/16] mtd: nand: denali: merge struct nand_buf into struct denali_nand_info Masahiro Yamada
@ 2017-06-13 13:45 ` Masahiro Yamada
  2017-06-13 13:45 ` [PATCH v7 11/16] mtd: nand: denali: fix raw and oob accessors for syndrome page layout Masahiro Yamada
                   ` (6 subsequent siblings)
  16 siblings, 0 replies; 19+ messages in thread
From: Masahiro Yamada @ 2017-06-13 13:45 UTC (permalink / raw)
  To: linux-mtd
  Cc: Enrico Jorns, Artem Bityutskiy, Dinh Nguyen, Boris Brezillon,
	Marek Vasut, David Woodhouse, Masami Hiramatsu, Chuanxiao Dong,
	Jassi Brar, Masahiro Yamada, Cyrille Pitchen, linux-kernel,
	Brian Norris, Richard Weinberger

It is not a good idea to re-use macros that represent a specific
register bit field for the transfer direction.

It is true that bit 8 indicates the direction for the MAP10 pipeline
operation and the data DMA operation, but this is not valid across
the IP.

Use a simple flag (write: 1, read: 0) for the direction.

Signed-off-by: Masahiro Yamada <yamada.masahiro@socionext.com>
---

Changes in v7: None
Changes in v6:
  - Delete unused DENALI_{READ,WRITE}

Changes in v5: None
Changes in v4: None
Changes in v3: None
Changes in v2:
  - Newly added

 drivers/mtd/nand/denali.c | 36 +++++++++++++++++-------------------
 1 file changed, 17 insertions(+), 19 deletions(-)

diff --git a/drivers/mtd/nand/denali.c b/drivers/mtd/nand/denali.c
index 4ba8ad610381..3b7d2f81aa5b 100644
--- a/drivers/mtd/nand/denali.c
+++ b/drivers/mtd/nand/denali.c
@@ -63,9 +63,6 @@ static inline struct denali_nand_info *mtd_to_denali(struct mtd_info *mtd)
 #define MAIN_ACCESS		0x42
 #define MAIN_SPARE_ACCESS	0x43
 
-#define DENALI_READ	0
-#define DENALI_WRITE	0x100
-
 /*
  * this is a helper macro that allows us to
  * format the bank into the proper bits for the controller
@@ -339,7 +336,7 @@ static int denali_dev_ready(struct mtd_info *mtd)
  */
 static int denali_send_pipeline_cmd(struct denali_nand_info *denali, int page,
 				    bool ecc_en, bool transfer_spare,
-				    int access_type, int op)
+				    int access_type, int write)
 {
 	int status = PASS;
 	uint32_t addr, cmd;
@@ -350,17 +347,17 @@ static int denali_send_pipeline_cmd(struct denali_nand_info *denali, int page,
 
 	addr = BANK(denali->flash_bank) | page;
 
-	if (op == DENALI_WRITE && access_type != SPARE_ACCESS) {
+	if (write && access_type != SPARE_ACCESS) {
 		cmd = MODE_01 | addr;
 		iowrite32(cmd, denali->flash_mem);
-	} else if (op == DENALI_WRITE && access_type == SPARE_ACCESS) {
+	} else if (write && access_type == SPARE_ACCESS) {
 		/* read spare area */
 		cmd = MODE_10 | addr;
 		index_addr(denali, cmd, access_type);
 
 		cmd = MODE_01 | addr;
 		iowrite32(cmd, denali->flash_mem);
-	} else if (op == DENALI_READ) {
+	} else {
 		/* setup page read request for access type */
 		cmd = MODE_10 | addr;
 		index_addr(denali, cmd, access_type);
@@ -422,7 +419,7 @@ static int write_oob_data(struct mtd_info *mtd, uint8_t *buf, int page)
 	int status = 0;
 
 	if (denali_send_pipeline_cmd(denali, page, false, false, SPARE_ACCESS,
-							DENALI_WRITE) == PASS) {
+							1) == PASS) {
 		write_data_to_flash_mem(denali, buf, mtd->oobsize);
 
 		/* wait for operation to complete */
@@ -447,7 +444,7 @@ static void read_oob_data(struct mtd_info *mtd, uint8_t *buf, int page)
 	uint32_t irq_status, addr, cmd;
 
 	if (denali_send_pipeline_cmd(denali, page, false, true, SPARE_ACCESS,
-							DENALI_READ) == PASS) {
+							0) == PASS) {
 		read_data_from_flash_mem(denali, buf, mtd->oobsize);
 
 		/*
@@ -633,7 +630,7 @@ static void denali_enable_dma(struct denali_nand_info *denali, bool en)
 }
 
 static void denali_setup_dma64(struct denali_nand_info *denali,
-			       dma_addr_t dma_addr, int page, int op)
+			       dma_addr_t dma_addr, int page, int write)
 {
 	uint32_t mode;
 	const int page_count = 1;
@@ -646,7 +643,8 @@ static void denali_setup_dma64(struct denali_nand_info *denali,
 	 * 1. setup transfer type, interrupt when complete,
 	 *    burst len = 64 bytes, the number of pages
 	 */
-	index_addr(denali, mode, 0x01002000 | (64 << 16) | op | page_count);
+	index_addr(denali, mode,
+		   0x01002000 | (64 << 16) | (write << 8) | page_count);
 
 	/* 2. set memory low address */
 	index_addr(denali, mode, dma_addr);
@@ -656,7 +654,7 @@ static void denali_setup_dma64(struct denali_nand_info *denali,
 }
 
 static void denali_setup_dma32(struct denali_nand_info *denali,
-			       dma_addr_t dma_addr, int page, int op)
+			       dma_addr_t dma_addr, int page, int write)
 {
 	uint32_t mode;
 	const int page_count = 1;
@@ -666,7 +664,7 @@ static void denali_setup_dma32(struct denali_nand_info *denali,
 	/* DMA is a four step process */
 
 	/* 1. setup transfer type and # of pages */
-	index_addr(denali, mode | page, 0x2000 | op | page_count);
+	index_addr(denali, mode | page, 0x2000 | (write << 8) | page_count);
 
 	/* 2. set memory high address bits 23:8 */
 	index_addr(denali, mode | ((dma_addr >> 16) << 8), 0x2200);
@@ -679,12 +677,12 @@ static void denali_setup_dma32(struct denali_nand_info *denali,
 }
 
 static void denali_setup_dma(struct denali_nand_info *denali,
-			     dma_addr_t dma_addr, int page, int op)
+			     dma_addr_t dma_addr, int page, int write)
 {
 	if (denali->caps & DENALI_CAP_DMA_64BIT)
-		denali_setup_dma64(denali, dma_addr, page, op);
+		denali_setup_dma64(denali, dma_addr, page, write);
 	else
-		denali_setup_dma32(denali, dma_addr, page, op);
+		denali_setup_dma32(denali, dma_addr, page, write);
 }
 
 /*
@@ -723,7 +721,7 @@ static int write_page(struct mtd_info *mtd, struct nand_chip *chip,
 	denali_reset_irq(denali);
 	denali_enable_dma(denali, true);
 
-	denali_setup_dma(denali, addr, page, DENALI_WRITE);
+	denali_setup_dma(denali, addr, page, 1);
 
 	/* wait for operation to complete */
 	irq_status = denali_wait_for_irq(denali, irq_mask);
@@ -805,7 +803,7 @@ static int denali_read_page(struct mtd_info *mtd, struct nand_chip *chip,
 	dma_sync_single_for_device(denali->dev, addr, size, DMA_FROM_DEVICE);
 
 	denali_reset_irq(denali);
-	denali_setup_dma(denali, addr, page, DENALI_READ);
+	denali_setup_dma(denali, addr, page, 0);
 
 	/* wait for operation to complete */
 	irq_status = denali_wait_for_irq(denali, irq_mask);
@@ -848,7 +846,7 @@ static int denali_read_page_raw(struct mtd_info *mtd, struct nand_chip *chip,
 	dma_sync_single_for_device(denali->dev, addr, size, DMA_FROM_DEVICE);
 
 	denali_reset_irq(denali);
-	denali_setup_dma(denali, addr, page, DENALI_READ);
+	denali_setup_dma(denali, addr, page, 0);
 
 	/* wait for operation to complete */
 	irq_status = denali_wait_for_irq(denali, irq_mask);
-- 
2.7.4

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

* [PATCH v7 11/16] mtd: nand: denali: fix raw and oob accessors for syndrome page layout
  2017-06-13 13:45 [PATCH v7 00/16] mtd: nand: denali: Denali NAND IP improvements Masahiro Yamada
                   ` (9 preceding siblings ...)
  2017-06-13 13:45 ` [PATCH v7 10/16] mtd: nand: denali: use flag instead of register macro for direction Masahiro Yamada
@ 2017-06-13 13:45 ` Masahiro Yamada
  2017-06-13 13:45 ` [PATCH v7 12/16] mtd: nand: denali: support hardware-assisted erased page detection Masahiro Yamada
                   ` (5 subsequent siblings)
  16 siblings, 0 replies; 19+ messages in thread
From: Masahiro Yamada @ 2017-06-13 13:45 UTC (permalink / raw)
  To: linux-mtd
  Cc: Enrico Jorns, Artem Bityutskiy, Dinh Nguyen, Boris Brezillon,
	Marek Vasut, David Woodhouse, Masami Hiramatsu, Chuanxiao Dong,
	Jassi Brar, Masahiro Yamada, Cyrille Pitchen, linux-kernel,
	Brian Norris, Richard Weinberger

The Denali IP adopts the syndrome page layout; payload and ECC are
interleaved, with BBM area always placed at the beginning of OOB.

The figure below shows the page organization for ecc->steps == 2:

  |----------------|    |-----------|
  |                |    |           |
  |                |    |           |
  |    Payload0    |    |           |
  |                |    |           |
  |                |    |           |
  |                |    |           |
  |----------------|    |  in-band  |
  |      ECC0      |    |   area    |
  |----------------|    |           |
  |                |    |           |
  |                |    |           |
  |    Payload1    |    |           |
  |                |    |           |
  |                |    |           |
  |----------------|    |-----------|
  |      BBM       |    |           |
  |----------------|    |           |
  |Payload1 (cont.)|    |           |
  |----------------|    |out-of-band|
  |      ECC1      |    |    area   |
  |----------------|    |           |
  |    OOB free    |    |           |
  |----------------|    |-----------|

The current raw / oob accessors do not take that into consideration,
so in-band and out-of-band data are transferred as stored in the
device.  In the case above,

  in-band:      Payload0 + ECC0 + Payload1(partial)
  out-of-band:  BBM + Payload1(cont.) + ECC1 + OOB-free

This is wrong.  As the comment block of struct nand_ecc_ctrl says,
driver callbacks must hide the specific layout used by the hardware
and always return contiguous in-band and out-of-band data.

The current implementation is completely screwed-up, so read/write
callbacks must be re-worked.

Also, it is reasonable to support PIO transfer in case DMA may not
work for some reasons.  Actually, the Data DMA may not be equipped
depending on the configuration of the RTL.  This can be checked by
reading the bit 4 of the FEATURES register.  Even if the controller
has the DMA support, dma_set_mask() and dma_map_single() could fail.
In either case, the driver can fall back to the PIO transfer.  Slower
access would be better than giving up.

Signed-off-by: Masahiro Yamada <yamada.masahiro@socionext.com>
---

Changes in v7: None
Changes in v6:
  - Minor cleanup to deprecate {write,read}_data_{to,from}_flash_mem
  - Remove unnecessary MAP10 command for MAIN_(SPARE_)ACCESS

Changes in v5: None
Changes in v4: None
Changes in v3: None
Changes in v2:
  - Newly added

 drivers/mtd/nand/denali.c | 610 ++++++++++++++++++++++++++--------------------
 drivers/mtd/nand/denali.h |   3 +-
 2 files changed, 344 insertions(+), 269 deletions(-)

diff --git a/drivers/mtd/nand/denali.c b/drivers/mtd/nand/denali.c
index 3b7d2f81aa5b..ed0044c560e5 100644
--- a/drivers/mtd/nand/denali.c
+++ b/drivers/mtd/nand/denali.c
@@ -56,14 +56,6 @@ static inline struct denali_nand_info *mtd_to_denali(struct mtd_info *mtd)
 }
 
 /*
- * These constants are defined by the driver to enable common driver
- * configuration options.
- */
-#define SPARE_ACCESS		0x41
-#define MAIN_ACCESS		0x42
-#define MAIN_SPARE_ACCESS	0x43
-
-/*
  * this is a helper macro that allows us to
  * format the bank into the proper bits for the controller
  */
@@ -330,146 +322,6 @@ static int denali_dev_ready(struct mtd_info *mtd)
 	return !!(denali_check_irq(denali) & INTR__INT_ACT);
 }
 
-/*
- * sends a pipeline command operation to the controller. See the Denali NAND
- * controller's user guide for more information (section 4.2.3.6).
- */
-static int denali_send_pipeline_cmd(struct denali_nand_info *denali, int page,
-				    bool ecc_en, bool transfer_spare,
-				    int access_type, int write)
-{
-	int status = PASS;
-	uint32_t addr, cmd;
-
-	setup_ecc_for_xfer(denali, ecc_en, transfer_spare);
-
-	denali_reset_irq(denali);
-
-	addr = BANK(denali->flash_bank) | page;
-
-	if (write && access_type != SPARE_ACCESS) {
-		cmd = MODE_01 | addr;
-		iowrite32(cmd, denali->flash_mem);
-	} else if (write && access_type == SPARE_ACCESS) {
-		/* read spare area */
-		cmd = MODE_10 | addr;
-		index_addr(denali, cmd, access_type);
-
-		cmd = MODE_01 | addr;
-		iowrite32(cmd, denali->flash_mem);
-	} else {
-		/* setup page read request for access type */
-		cmd = MODE_10 | addr;
-		index_addr(denali, cmd, access_type);
-
-		cmd = MODE_01 | addr;
-		iowrite32(cmd, denali->flash_mem);
-	}
-	return status;
-}
-
-/* helper function that simply writes a buffer to the flash */
-static int write_data_to_flash_mem(struct denali_nand_info *denali,
-				   const uint8_t *buf, int len)
-{
-	uint32_t *buf32;
-	int i;
-
-	/*
-	 * verify that the len is a multiple of 4.
-	 * see comment in read_data_from_flash_mem()
-	 */
-	BUG_ON((len % 4) != 0);
-
-	/* write the data to the flash memory */
-	buf32 = (uint32_t *)buf;
-	for (i = 0; i < len / 4; i++)
-		iowrite32(*buf32++, denali->flash_mem + 0x10);
-	return i * 4; /* intent is to return the number of bytes read */
-}
-
-/* helper function that simply reads a buffer from the flash */
-static int read_data_from_flash_mem(struct denali_nand_info *denali,
-				    uint8_t *buf, int len)
-{
-	uint32_t *buf32;
-	int i;
-
-	/*
-	 * we assume that len will be a multiple of 4, if not it would be nice
-	 * to know about it ASAP rather than have random failures...
-	 * This assumption is based on the fact that this function is designed
-	 * to be used to read flash pages, which are typically multiples of 4.
-	 */
-	BUG_ON((len % 4) != 0);
-
-	/* transfer the data from the flash */
-	buf32 = (uint32_t *)buf;
-	for (i = 0; i < len / 4; i++)
-		*buf32++ = ioread32(denali->flash_mem + 0x10);
-	return i * 4; /* intent is to return the number of bytes read */
-}
-
-/* writes OOB data to the device */
-static int write_oob_data(struct mtd_info *mtd, uint8_t *buf, int page)
-{
-	struct denali_nand_info *denali = mtd_to_denali(mtd);
-	uint32_t irq_status;
-	uint32_t irq_mask = INTR__PROGRAM_COMP | INTR__PROGRAM_FAIL;
-	int status = 0;
-
-	if (denali_send_pipeline_cmd(denali, page, false, false, SPARE_ACCESS,
-							1) == PASS) {
-		write_data_to_flash_mem(denali, buf, mtd->oobsize);
-
-		/* wait for operation to complete */
-		irq_status = denali_wait_for_irq(denali, irq_mask);
-
-		if (!(irq_status & INTR__PROGRAM_COMP)) {
-			dev_err(denali->dev, "OOB write failed\n");
-			status = -EIO;
-		}
-	} else {
-		dev_err(denali->dev, "unable to send pipeline command\n");
-		status = -EIO;
-	}
-	return status;
-}
-
-/* reads OOB data from the device */
-static void read_oob_data(struct mtd_info *mtd, uint8_t *buf, int page)
-{
-	struct denali_nand_info *denali = mtd_to_denali(mtd);
-	uint32_t irq_mask = INTR__LOAD_COMP;
-	uint32_t irq_status, addr, cmd;
-
-	if (denali_send_pipeline_cmd(denali, page, false, true, SPARE_ACCESS,
-							0) == PASS) {
-		read_data_from_flash_mem(denali, buf, mtd->oobsize);
-
-		/*
-		 * wait for command to be accepted
-		 * can always use status0 bit as the
-		 * mask is identical for each bank.
-		 */
-		irq_status = denali_wait_for_irq(denali, irq_mask);
-
-		if (!(irq_status & INTR__LOAD_COMP))
-			dev_err(denali->dev, "page on OOB timeout %d\n", page);
-
-		/*
-		 * We set the device back to MAIN_ACCESS here as I observed
-		 * instability with the controller if you do a block erase
-		 * and the last transaction was a SPARE_ACCESS. Block erase
-		 * is reliable (according to the MTD test infrastructure)
-		 * if you are in MAIN_ACCESS.
-		 */
-		addr = BANK(denali->flash_bank) | page;
-		cmd = MODE_10 | addr;
-		index_addr(denali, cmd, MAIN_ACCESS);
-	}
-}
-
 static int denali_check_erased_page(struct mtd_info *mtd,
 				    struct nand_chip *chip, uint8_t *buf,
 				    unsigned long uncor_ecc_flags,
@@ -685,144 +537,303 @@ static void denali_setup_dma(struct denali_nand_info *denali,
 		denali_setup_dma32(denali, dma_addr, page, write);
 }
 
-/*
- * writes a page. user specifies type, and this function handles the
- * configuration details.
- */
-static int write_page(struct mtd_info *mtd, struct nand_chip *chip,
-			const uint8_t *buf, int page, bool raw_xfer)
+static int denali_pio_read(struct denali_nand_info *denali, void *buf,
+			   size_t size, int page, int raw)
 {
-	struct denali_nand_info *denali = mtd_to_denali(mtd);
-	dma_addr_t addr = denali->dma_addr;
-	size_t size = mtd->writesize + mtd->oobsize;
-	uint32_t irq_status;
-	uint32_t irq_mask = INTR__DMA_CMD_COMP | INTR__PROGRAM_FAIL;
-	int ret = 0;
+	uint32_t addr = BANK(denali->flash_bank) | page;
+	uint32_t *buf32 = (uint32_t *)buf;
+	uint32_t irq_status, ecc_err_mask;
+	int i;
 
-	/*
-	 * if it is a raw xfer, we want to disable ecc and send the spare area.
-	 * !raw_xfer - enable ecc
-	 * raw_xfer - transfer spare
-	 */
-	setup_ecc_for_xfer(denali, !raw_xfer, raw_xfer);
+	if (denali->caps & DENALI_CAP_HW_ECC_FIXUP)
+		ecc_err_mask = INTR__ECC_UNCOR_ERR;
+	else
+		ecc_err_mask = INTR__ECC_ERR;
 
-	/* copy buffer into DMA buffer */
-	memcpy(denali->buf, buf, mtd->writesize);
+	denali_reset_irq(denali);
 
-	if (raw_xfer) {
-		/* transfer the data to the spare area */
-		memcpy(denali->buf + mtd->writesize,
-			chip->oob_poi,
-			mtd->oobsize);
-	}
+	iowrite32(MODE_01 | addr, denali->flash_mem);
+	for (i = 0; i < size / 4; i++)
+		*buf32++ = ioread32(denali->flash_mem + 0x10);
+
+	irq_status = denali_wait_for_irq(denali, INTR__PAGE_XFER_INC);
+	if (!(irq_status & INTR__PAGE_XFER_INC))
+		return -EIO;
 
-	dma_sync_single_for_device(denali->dev, addr, size, DMA_TO_DEVICE);
+	return irq_status & ecc_err_mask ? -EBADMSG : 0;
+}
+
+static int denali_pio_write(struct denali_nand_info *denali,
+			    const void *buf, size_t size, int page, int raw)
+{
+	uint32_t addr = BANK(denali->flash_bank) | page;
+	const uint32_t *buf32 = (uint32_t *)buf;
+	uint32_t irq_status;
+	int i;
 
 	denali_reset_irq(denali);
+
+	iowrite32(MODE_01 | addr, denali->flash_mem);
+	for (i = 0; i < size / 4; i++)
+		iowrite32(*buf32++, denali->flash_mem + 0x10);
+
+	irq_status = denali_wait_for_irq(denali,
+				INTR__PROGRAM_COMP | INTR__PROGRAM_FAIL);
+	if (!(irq_status & INTR__PROGRAM_COMP))
+		return -EIO;
+
+	return 0;
+}
+
+static int denali_pio_xfer(struct denali_nand_info *denali, void *buf,
+			   size_t size, int page, int raw, int write)
+{
+	if (write)
+		return denali_pio_write(denali, buf, size, page, raw);
+	else
+		return denali_pio_read(denali, buf, size, page, raw);
+}
+
+static int denali_dma_xfer(struct denali_nand_info *denali, void *buf,
+			   size_t size, int page, int raw, int write)
+{
+	dma_addr_t dma_addr = denali->dma_addr;
+	uint32_t irq_mask, irq_status, ecc_err_mask;
+	enum dma_data_direction dir = write ? DMA_TO_DEVICE : DMA_FROM_DEVICE;
+	int ret = 0;
+
+	dma_sync_single_for_device(denali->dev, dma_addr, size, dir);
+
+	if (write) {
+		/*
+		 * INTR__PROGRAM_COMP is never asserted for the DMA transfer.
+		 * We can use INTR__DMA_CMD_COMP instead.  This flag is asserted
+		 * when the page program is completed.
+		 */
+		irq_mask = INTR__DMA_CMD_COMP | INTR__PROGRAM_FAIL;
+		ecc_err_mask = 0;
+	} else if (denali->caps & DENALI_CAP_HW_ECC_FIXUP) {
+		irq_mask = INTR__DMA_CMD_COMP;
+		ecc_err_mask = INTR__ECC_UNCOR_ERR;
+	} else {
+		irq_mask = INTR__DMA_CMD_COMP;
+		ecc_err_mask = INTR__ECC_ERR;
+	}
+
 	denali_enable_dma(denali, true);
 
-	denali_setup_dma(denali, addr, page, 1);
+	denali_reset_irq(denali);
+	denali_setup_dma(denali, dma_addr, page, write);
 
 	/* wait for operation to complete */
 	irq_status = denali_wait_for_irq(denali, irq_mask);
-	if (!(irq_status & INTR__DMA_CMD_COMP)) {
-		dev_err(denali->dev, "timeout on write_page (type = %d)\n",
-			raw_xfer);
+	if (!(irq_status & INTR__DMA_CMD_COMP))
 		ret = -EIO;
-	}
+	else if (irq_status & ecc_err_mask)
+		ret = -EBADMSG;
 
 	denali_enable_dma(denali, false);
-	dma_sync_single_for_cpu(denali->dev, addr, size, DMA_TO_DEVICE);
+	dma_sync_single_for_cpu(denali->dev, dma_addr, size, dir);
 
 	return ret;
 }
 
-/* NAND core entry points */
-
-/*
- * this is the callback that the NAND core calls to write a page. Since
- * writing a page with ECC or without is similar, all the work is done
- * by write_page above.
- */
-static int denali_write_page(struct mtd_info *mtd, struct nand_chip *chip,
-				const uint8_t *buf, int oob_required, int page)
+static int denali_data_xfer(struct denali_nand_info *denali, void *buf,
+			    size_t size, int page, int raw, int write)
 {
-	/*
-	 * for regular page writes, we let HW handle all the ECC
-	 * data written to the device.
-	 */
-	return write_page(mtd, chip, buf, page, false);
+	setup_ecc_for_xfer(denali, !raw, raw);
+
+	if (denali->dma_avail)
+		return denali_dma_xfer(denali, buf, size, page, raw, write);
+	else
+		return denali_pio_xfer(denali, buf, size, page, raw, write);
 }
 
-/*
- * This is the callback that the NAND core calls to write a page without ECC.
- * raw access is similar to ECC page writes, so all the work is done in the
- * write_page() function above.
- */
-static int denali_write_page_raw(struct mtd_info *mtd, struct nand_chip *chip,
-				 const uint8_t *buf, int oob_required,
-				 int page)
+static void denali_oob_xfer(struct mtd_info *mtd, struct nand_chip *chip,
+			    int page, int write)
 {
-	/*
-	 * for raw page writes, we want to disable ECC and simply write
-	 * whatever data is in the buffer.
-	 */
-	return write_page(mtd, chip, buf, page, true);
+	struct denali_nand_info *denali = mtd_to_denali(mtd);
+	unsigned int start_cmd = write ? NAND_CMD_SEQIN : NAND_CMD_READ0;
+	unsigned int rnd_cmd = write ? NAND_CMD_RNDIN : NAND_CMD_RNDOUT;
+	int writesize = mtd->writesize;
+	int oobsize = mtd->oobsize;
+	uint8_t *bufpoi = chip->oob_poi;
+	int ecc_steps = chip->ecc.steps;
+	int ecc_size = chip->ecc.size;
+	int ecc_bytes = chip->ecc.bytes;
+	int oob_skip = denali->bbtskipbytes;
+	size_t size = writesize + oobsize;
+	int i, pos, len;
+
+	/* BBM at the beginning of the OOB area */
+	chip->cmdfunc(mtd, start_cmd, writesize, page);
+	if (write)
+		chip->write_buf(mtd, bufpoi, oob_skip);
+	else
+		chip->read_buf(mtd, bufpoi, oob_skip);
+	bufpoi += oob_skip;
+
+	/* OOB ECC */
+	for (i = 0; i < ecc_steps; i++) {
+		pos = ecc_size + i * (ecc_size + ecc_bytes);
+		len = ecc_bytes;
+
+		if (pos >= writesize)
+			pos += oob_skip;
+		else if (pos + len > writesize)
+			len = writesize - pos;
+
+		chip->cmdfunc(mtd, rnd_cmd, pos, -1);
+		if (write)
+			chip->write_buf(mtd, bufpoi, len);
+		else
+			chip->read_buf(mtd, bufpoi, len);
+		bufpoi += len;
+		if (len < ecc_bytes) {
+			len = ecc_bytes - len;
+			chip->cmdfunc(mtd, rnd_cmd, writesize + oob_skip, -1);
+			if (write)
+				chip->write_buf(mtd, bufpoi, len);
+			else
+				chip->read_buf(mtd, bufpoi, len);
+			bufpoi += len;
+		}
+	}
+
+	/* OOB free */
+	len = oobsize - (bufpoi - chip->oob_poi);
+	chip->cmdfunc(mtd, rnd_cmd, size - len, -1);
+	if (write)
+		chip->write_buf(mtd, bufpoi, len);
+	else
+		chip->read_buf(mtd, bufpoi, len);
 }
 
-static int denali_write_oob(struct mtd_info *mtd, struct nand_chip *chip,
-			    int page)
+static int denali_read_page_raw(struct mtd_info *mtd, struct nand_chip *chip,
+				uint8_t *buf, int oob_required, int page)
 {
-	return write_oob_data(mtd, chip->oob_poi, page);
+	struct denali_nand_info *denali = mtd_to_denali(mtd);
+	int writesize = mtd->writesize;
+	int oobsize = mtd->oobsize;
+	int ecc_steps = chip->ecc.steps;
+	int ecc_size = chip->ecc.size;
+	int ecc_bytes = chip->ecc.bytes;
+	void *dma_buf = denali->buf;
+	int oob_skip = denali->bbtskipbytes;
+	size_t size = writesize + oobsize;
+	int ret, i, pos, len;
+
+	ret = denali_data_xfer(denali, dma_buf, size, page, 1, 0);
+	if (ret)
+		return ret;
+
+	/* Arrange the buffer for syndrome payload/ecc layout */
+	if (buf) {
+		for (i = 0; i < ecc_steps; i++) {
+			pos = i * (ecc_size + ecc_bytes);
+			len = ecc_size;
+
+			if (pos >= writesize)
+				pos += oob_skip;
+			else if (pos + len > writesize)
+				len = writesize - pos;
+
+			memcpy(buf, dma_buf + pos, len);
+			buf += len;
+			if (len < ecc_size) {
+				len = ecc_size - len;
+				memcpy(buf, dma_buf + writesize + oob_skip,
+				       len);
+				buf += len;
+			}
+		}
+	}
+
+	if (oob_required) {
+		uint8_t *oob = chip->oob_poi;
+
+		/* BBM at the beginning of the OOB area */
+		memcpy(oob, dma_buf + writesize, oob_skip);
+		oob += oob_skip;
+
+		/* OOB ECC */
+		for (i = 0; i < ecc_steps; i++) {
+			pos = ecc_size + i * (ecc_size + ecc_bytes);
+			len = ecc_bytes;
+
+			if (pos >= writesize)
+				pos += oob_skip;
+			else if (pos + len > writesize)
+				len = writesize - pos;
+
+			memcpy(oob, dma_buf + pos, len);
+			oob += len;
+			if (len < ecc_bytes) {
+				len = ecc_bytes - len;
+				memcpy(oob, dma_buf + writesize + oob_skip,
+				       len);
+				oob += len;
+			}
+		}
+
+		/* OOB free */
+		len = oobsize - (oob - chip->oob_poi);
+		memcpy(oob, dma_buf + size - len, len);
+	}
+
+	return 0;
 }
 
 static int denali_read_oob(struct mtd_info *mtd, struct nand_chip *chip,
 			   int page)
 {
-	read_oob_data(mtd, chip->oob_poi, page);
+	denali_oob_xfer(mtd, chip, page, 0);
 
 	return 0;
 }
 
-static int denali_read_page(struct mtd_info *mtd, struct nand_chip *chip,
-			    uint8_t *buf, int oob_required, int page)
+static int denali_write_oob(struct mtd_info *mtd, struct nand_chip *chip,
+			    int page)
 {
 	struct denali_nand_info *denali = mtd_to_denali(mtd);
-	dma_addr_t addr = denali->dma_addr;
-	size_t size = mtd->writesize + mtd->oobsize;
-	uint32_t irq_status;
-	uint32_t irq_mask = denali->caps & DENALI_CAP_HW_ECC_FIXUP ?
-				INTR__DMA_CMD_COMP | INTR__ECC_UNCOR_ERR :
-				INTR__ECC_TRANSACTION_DONE | INTR__ECC_ERR;
-	unsigned long uncor_ecc_flags = 0;
-	int stat = 0;
+	int status;
 
-	setup_ecc_for_xfer(denali, true, false);
+	denali_reset_irq(denali);
 
-	denali_enable_dma(denali, true);
-	dma_sync_single_for_device(denali->dev, addr, size, DMA_FROM_DEVICE);
+	denali_oob_xfer(mtd, chip, page, 1);
 
-	denali_reset_irq(denali);
-	denali_setup_dma(denali, addr, page, 0);
+	chip->cmdfunc(mtd, NAND_CMD_PAGEPROG, -1, -1);
+	status = chip->waitfunc(mtd, chip);
 
-	/* wait for operation to complete */
-	irq_status = denali_wait_for_irq(denali, irq_mask);
+	return status & NAND_STATUS_FAIL ? -EIO : 0;
+}
+
+static int denali_read_page(struct mtd_info *mtd, struct nand_chip *chip,
+			    uint8_t *buf, int oob_required, int page)
+{
+	struct denali_nand_info *denali = mtd_to_denali(mtd);
+	unsigned long uncor_ecc_flags = 0;
+	int stat = 0;
+	int ret;
 
-	dma_sync_single_for_cpu(denali->dev, addr, size, DMA_FROM_DEVICE);
+	ret = denali_data_xfer(denali, denali->buf, mtd->writesize, page, 0, 0);
+	if (ret && ret != -EBADMSG)
+		return ret;
 
 	memcpy(buf, denali->buf, mtd->writesize);
 
 	if (denali->caps & DENALI_CAP_HW_ECC_FIXUP)
 		stat = denali_hw_ecc_fixup(mtd, denali, &uncor_ecc_flags);
-	else if (irq_status & INTR__ECC_ERR)
+	else if (ret == -EBADMSG)
 		stat = denali_sw_ecc_fixup(mtd, denali, &uncor_ecc_flags, buf);
-	denali_enable_dma(denali, false);
 
 	if (stat < 0)
 		return stat;
 
 	if (uncor_ecc_flags) {
-		read_oob_data(mtd, chip->oob_poi, page);
+		ret = denali_read_oob(mtd, chip, page);
+		if (ret)
+			return ret;
 
 		stat = denali_check_erased_page(mtd, chip, buf,
 						uncor_ecc_flags, stat);
@@ -831,36 +842,93 @@ static int denali_read_page(struct mtd_info *mtd, struct nand_chip *chip,
 	return stat;
 }
 
-static int denali_read_page_raw(struct mtd_info *mtd, struct nand_chip *chip,
-				uint8_t *buf, int oob_required, int page)
+static int denali_write_page_raw(struct mtd_info *mtd, struct nand_chip *chip,
+				 const uint8_t *buf, int oob_required, int page)
 {
 	struct denali_nand_info *denali = mtd_to_denali(mtd);
-	dma_addr_t addr = denali->dma_addr;
-	size_t size = mtd->writesize + mtd->oobsize;
-	uint32_t irq_mask = INTR__DMA_CMD_COMP;
-	uint32_t irq_status;
-
-	setup_ecc_for_xfer(denali, false, true);
-	denali_enable_dma(denali, true);
+	int writesize = mtd->writesize;
+	int oobsize = mtd->oobsize;
+	int ecc_steps = chip->ecc.steps;
+	int ecc_size = chip->ecc.size;
+	int ecc_bytes = chip->ecc.bytes;
+	void *dma_buf = denali->buf;
+	int oob_skip = denali->bbtskipbytes;
+	size_t size = writesize + oobsize;
+	int i, pos, len;
 
-	dma_sync_single_for_device(denali->dev, addr, size, DMA_FROM_DEVICE);
+	/*
+	 * Fill the buffer with 0xff first except the full page transfer.
+	 * This simplifies the logic.
+	 */
+	if (!buf || !oob_required)
+		memset(dma_buf, 0xff, size);
+
+	/* Arrange the buffer for syndrome payload/ecc layout */
+	if (buf) {
+		for (i = 0; i < ecc_steps; i++) {
+			pos = i * (ecc_size + ecc_bytes);
+			len = ecc_size;
+
+			if (pos >= writesize)
+				pos += oob_skip;
+			else if (pos + len > writesize)
+				len = writesize - pos;
+
+			memcpy(dma_buf + pos, buf, len);
+			buf += len;
+			if (len < ecc_size) {
+				len = ecc_size - len;
+				memcpy(dma_buf + writesize + oob_skip, buf,
+				       len);
+				buf += len;
+			}
+		}
+	}
 
-	denali_reset_irq(denali);
-	denali_setup_dma(denali, addr, page, 0);
+	if (oob_required) {
+		const uint8_t *oob = chip->oob_poi;
+
+		/* BBM at the beginning of the OOB area */
+		memcpy(dma_buf + writesize, oob, oob_skip);
+		oob += oob_skip;
+
+		/* OOB ECC */
+		for (i = 0; i < ecc_steps; i++) {
+			pos = ecc_size + i * (ecc_size + ecc_bytes);
+			len = ecc_bytes;
+
+			if (pos >= writesize)
+				pos += oob_skip;
+			else if (pos + len > writesize)
+				len = writesize - pos;
+
+			memcpy(dma_buf + pos, oob, len);
+			oob += len;
+			if (len < ecc_bytes) {
+				len = ecc_bytes - len;
+				memcpy(dma_buf + writesize + oob_skip, oob,
+				       len);
+				oob += len;
+			}
+		}
 
-	/* wait for operation to complete */
-	irq_status = denali_wait_for_irq(denali, irq_mask);
-	if (irq_status & INTR__DMA_CMD_COMP)
-		return -ETIMEDOUT;
+		/* OOB free */
+		len = oobsize - (oob - chip->oob_poi);
+		memcpy(dma_buf + size - len, oob, len);
+	}
 
-	dma_sync_single_for_cpu(denali->dev, addr, size, DMA_FROM_DEVICE);
+	return denali_data_xfer(denali, dma_buf, size, page, 1, 1);
+}
 
-	denali_enable_dma(denali, false);
+static int denali_write_page(struct mtd_info *mtd, struct nand_chip *chip,
+			     const uint8_t *buf, int oob_required, int page)
+{
+	struct denali_nand_info *denali = mtd_to_denali(mtd);
 
-	memcpy(buf, denali->buf, mtd->writesize);
-	memcpy(chip->oob_poi, denali->buf + mtd->writesize, mtd->oobsize);
+	memcpy(denali->buf, buf, mtd->writesize);
 
-	return 0;
+	return denali_data_xfer(denali, denali->buf, mtd->writesize, page,
+				0, 1);
 }
 
 static void denali_select_chip(struct mtd_info *mtd, int chip)
@@ -1285,21 +1353,29 @@ int denali_init(struct denali_nand_info *denali)
 		goto disable_irq;
 	}
 
-	ret = dma_set_mask(denali->dev,
-			   DMA_BIT_MASK(denali->caps & DENALI_CAP_DMA_64BIT ?
-					64 : 32));
-	if (ret) {
-		dev_err(denali->dev, "No usable DMA configuration\n");
-		goto disable_irq;
+	if (ioread32(denali->flash_reg + FEATURES) & FEATURES__DMA)
+		denali->dma_avail = 1;
+
+	if (denali->dma_avail) {
+		int dma_bit = denali->caps & DENALI_CAP_DMA_64BIT ? 64 : 32;
+
+		ret = dma_set_mask(denali->dev, DMA_BIT_MASK(dma_bit));
+		if (ret) {
+			dev_info(denali->dev,
+				 "Failed to set DMA mask. Disabling DMA.\n");
+			denali->dma_avail = 0;
+		}
 	}
 
-	denali->dma_addr = dma_map_single(denali->dev, denali->buf,
-			     mtd->writesize + mtd->oobsize,
-			     DMA_BIDIRECTIONAL);
-	if (dma_mapping_error(denali->dev, denali->dma_addr)) {
-		dev_err(denali->dev, "Failed to map DMA buffer\n");
-		ret = -EIO;
-		goto disable_irq;
+	if (denali->dma_avail) {
+		denali->dma_addr = dma_map_single(denali->dev, denali->buf,
+						  mtd->writesize + mtd->oobsize,
+						  DMA_BIDIRECTIONAL);
+		if (dma_mapping_error(denali->dev, denali->dma_addr)) {
+			dev_info(denali->dev,
+				 "Failed to map DMA buffer. Disabling DMA.\n");
+			denali->dma_avail = 0;
+		};
 	}
 
 	/*
diff --git a/drivers/mtd/nand/denali.h b/drivers/mtd/nand/denali.h
index 1b991d3016f8..f5da52f09e34 100644
--- a/drivers/mtd/nand/denali.h
+++ b/drivers/mtd/nand/denali.h
@@ -298,8 +298,6 @@
 #define     CHNL_ACTIVE__CHANNEL2			BIT(2)
 #define     CHNL_ACTIVE__CHANNEL3			BIT(3)
 
-#define PASS 0                  /*success flag*/
-
 #define MODE_00    0x00000000
 #define MODE_01    0x04000000
 #define MODE_10    0x08000000
@@ -322,6 +320,7 @@ struct denali_nand_info {
 
 	void *buf;
 	dma_addr_t dma_addr;
+	int dma_avail;
 	int devnum;	/* represent how many nands connected */
 	int bbtskipbytes;
 	int max_banks;
-- 
2.7.4

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

* [PATCH v7 12/16] mtd: nand: denali: support hardware-assisted erased page detection
  2017-06-13 13:45 [PATCH v7 00/16] mtd: nand: denali: Denali NAND IP improvements Masahiro Yamada
                   ` (10 preceding siblings ...)
  2017-06-13 13:45 ` [PATCH v7 11/16] mtd: nand: denali: fix raw and oob accessors for syndrome page layout Masahiro Yamada
@ 2017-06-13 13:45 ` Masahiro Yamada
  2017-06-13 13:45 ` [PATCH v7 13/16] mtd: nand: denali: skip driver internal bounce buffer when possible Masahiro Yamada
                   ` (4 subsequent siblings)
  16 siblings, 0 replies; 19+ messages in thread
From: Masahiro Yamada @ 2017-06-13 13:45 UTC (permalink / raw)
  To: linux-mtd
  Cc: Enrico Jorns, Artem Bityutskiy, Dinh Nguyen, Boris Brezillon,
	Marek Vasut, David Woodhouse, Masami Hiramatsu, Chuanxiao Dong,
	Jassi Brar, Masahiro Yamada, Cyrille Pitchen, linux-kernel,
	Brian Norris, Richard Weinberger

Recent versions of this IP support automatic erased page detection.
If an erased page is detected on reads, the controller does not set
INTR__ECC_UNCOR_ERR, but INTR__ERASED_PAGE.

The detection of erased pages is based on the number of zeros in a
page; if the number of zeros is less than the value in the field
ERASED_THRESHOLD, the page is assumed as erased.

Please note ERASED_THRESHOLD specifies the number of zeros in a _page_
instead of an ECC chunk.  Moreover, the controller does not provide a
way to know the actual number of bitflips.

Actually, an erased page (all 0xff) is not an ECC correctable pattern
on the Denali ECC engine.  In other words, there may be overlap between
the following two:

[1] a bit pattern reachable from a valid payload + ECC pattern within
    ecc.strength bitflips
[2] a bit pattern reachable from an erased state (all 0xff) within
    ecc.strength bitflips

So, this feature may intercept ECC correctable patterns, then replace
[1] with [2].

After all, this feature can work safely only when ECC_THRESHOLD == 1,
i.e. detect erased pages without any bitflips.  This should be the
case most of the time.  If there is a bitflip or more, the driver will
fallback to the software method by using nand_check_erased_ecc_chunk().

Strangely enough, the driver still has to fill the buffer with 0xff
in case of INTR__ERASED_PAGE because the ECC correction engine has
already manipulated the data in the buffer before it judges erased
pages.

Signed-off-by: Masahiro Yamada <yamada.masahiro@socionext.com>
---

Changes in v7: None
Changes in v6:
  - memset(buf, 0xff, size) is necessary even if the page is completely
    erased (all 0xff), strangely.

Changes in v5:
  - Set ECC_THRESHOLD to 1

Changes in v4: None
Changes in v3: None
Changes in v2:
  - Newly added

 drivers/mtd/nand/denali.c | 9 ++++++++-
 drivers/mtd/nand/denali.h | 5 +++++
 2 files changed, 13 insertions(+), 1 deletion(-)

diff --git a/drivers/mtd/nand/denali.c b/drivers/mtd/nand/denali.c
index ed0044c560e5..e8d8e6c6f45e 100644
--- a/drivers/mtd/nand/denali.c
+++ b/drivers/mtd/nand/denali.c
@@ -560,6 +560,9 @@ static int denali_pio_read(struct denali_nand_info *denali, void *buf,
 	if (!(irq_status & INTR__PAGE_XFER_INC))
 		return -EIO;
 
+	if (irq_status & INTR__ERASED_PAGE)
+		memset(buf, 0xff, size);
+
 	return irq_status & ecc_err_mask ? -EBADMSG : 0;
 }
 
@@ -635,6 +638,9 @@ static int denali_dma_xfer(struct denali_nand_info *denali, void *buf,
 	denali_enable_dma(denali, false);
 	dma_sync_single_for_cpu(denali->dev, dma_addr, size, dir);
 
+	if (irq_status & INTR__ERASED_PAGE)
+		memset(buf, 0xff, size);
+
 	return ret;
 }
 
@@ -1406,7 +1412,8 @@ int denali_init(struct denali_nand_info *denali)
 		"chosen ECC settings: step=%d, strength=%d, bytes=%d\n",
 		chip->ecc.size, chip->ecc.strength, chip->ecc.bytes);
 
-	iowrite32(chip->ecc.strength, denali->flash_reg + ECC_CORRECTION);
+	iowrite32(MAKE_ECC_CORRECTION(chip->ecc.strength, 1),
+		  denali->flash_reg + ECC_CORRECTION);
 	iowrite32(mtd->erasesize / mtd->writesize,
 		  denali->flash_reg + PAGES_PER_BLOCK);
 	iowrite32(chip->options & NAND_BUSWIDTH_16 ? 1 : 0,
diff --git a/drivers/mtd/nand/denali.h b/drivers/mtd/nand/denali.h
index f5da52f09e34..657a794af695 100644
--- a/drivers/mtd/nand/denali.h
+++ b/drivers/mtd/nand/denali.h
@@ -110,6 +110,10 @@
 
 #define ECC_CORRECTION				0x1b0
 #define     ECC_CORRECTION__VALUE			GENMASK(4, 0)
+#define     ECC_CORRECTION__ERASE_THRESHOLD		GENMASK(31, 16)
+#define     MAKE_ECC_CORRECTION(val, thresh)		\
+			(((val) & (ECC_CORRECTION__VALUE)) | \
+			(((thresh) << 16) & (ECC_CORRECTION__ERASE_THRESHOLD)))
 
 #define READ_MODE				0x1c0
 #define     READ_MODE__VALUE				GENMASK(3, 0)
@@ -233,6 +237,7 @@
 #define     INTR__RST_COMP				BIT(13)
 #define     INTR__PIPE_CMD_ERR				BIT(14)
 #define     INTR__PAGE_XFER_INC				BIT(15)
+#define     INTR__ERASED_PAGE				BIT(16)
 
 #define PAGE_CNT(bank)				(0x430 + (bank) * 0x50)
 #define ERR_PAGE_ADDR(bank)			(0x440 + (bank) * 0x50)
-- 
2.7.4

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

* [PATCH v7 13/16] mtd: nand: denali: skip driver internal bounce buffer when possible
  2017-06-13 13:45 [PATCH v7 00/16] mtd: nand: denali: Denali NAND IP improvements Masahiro Yamada
                   ` (11 preceding siblings ...)
  2017-06-13 13:45 ` [PATCH v7 12/16] mtd: nand: denali: support hardware-assisted erased page detection Masahiro Yamada
@ 2017-06-13 13:45 ` Masahiro Yamada
  2017-06-13 13:45 ` [PATCH v7 14/16] mtd: nand: denali: use non-managed kmalloc() for DMA buffer Masahiro Yamada
                   ` (3 subsequent siblings)
  16 siblings, 0 replies; 19+ messages in thread
From: Masahiro Yamada @ 2017-06-13 13:45 UTC (permalink / raw)
  To: linux-mtd
  Cc: Enrico Jorns, Artem Bityutskiy, Dinh Nguyen, Boris Brezillon,
	Marek Vasut, David Woodhouse, Masami Hiramatsu, Chuanxiao Dong,
	Jassi Brar, Masahiro Yamada, Cyrille Pitchen, linux-kernel,
	Brian Norris, Richard Weinberger

For ecc->read_page() and ecc->write_page(), it is possible to call
dma_map_single() against the given buffer.  This bypasses the driver
internal bounce buffer and save the memcpy().

Signed-off-by: Masahiro Yamada <yamada.masahiro@socionext.com>
---

Changes in v7: None
Changes in v6: None
Changes in v5: None
Changes in v4:
  - Remove dma_unmap_single() from denali_remove()

Changes in v3:
  - Set chip->buf_align to 16

Changes in v2:
  - Newly added

 drivers/mtd/nand/denali.c | 38 ++++++++++++--------------------------
 1 file changed, 12 insertions(+), 26 deletions(-)

diff --git a/drivers/mtd/nand/denali.c b/drivers/mtd/nand/denali.c
index e8d8e6c6f45e..ec5fc8da5f9a 100644
--- a/drivers/mtd/nand/denali.c
+++ b/drivers/mtd/nand/denali.c
@@ -600,12 +600,16 @@ static int denali_pio_xfer(struct denali_nand_info *denali, void *buf,
 static int denali_dma_xfer(struct denali_nand_info *denali, void *buf,
 			   size_t size, int page, int raw, int write)
 {
-	dma_addr_t dma_addr = denali->dma_addr;
+	dma_addr_t dma_addr;
 	uint32_t irq_mask, irq_status, ecc_err_mask;
 	enum dma_data_direction dir = write ? DMA_TO_DEVICE : DMA_FROM_DEVICE;
 	int ret = 0;
 
-	dma_sync_single_for_device(denali->dev, dma_addr, size, dir);
+	dma_addr = dma_map_single(denali->dev, buf, size, dir);
+	if (dma_mapping_error(denali->dev, dma_addr)) {
+		dev_dbg(denali->dev, "Failed to DMA-map buffer. Trying PIO.\n");
+		return denali_pio_xfer(denali, buf, size, page, raw, write);
+	}
 
 	if (write) {
 		/*
@@ -636,7 +640,7 @@ static int denali_dma_xfer(struct denali_nand_info *denali, void *buf,
 		ret = -EBADMSG;
 
 	denali_enable_dma(denali, false);
-	dma_sync_single_for_cpu(denali->dev, dma_addr, size, dir);
+	dma_unmap_single(denali->dev, dma_addr, size, dir);
 
 	if (irq_status & INTR__ERASED_PAGE)
 		memset(buf, 0xff, size);
@@ -822,12 +826,10 @@ static int denali_read_page(struct mtd_info *mtd, struct nand_chip *chip,
 	int stat = 0;
 	int ret;
 
-	ret = denali_data_xfer(denali, denali->buf, mtd->writesize, page, 0, 0);
+	ret = denali_data_xfer(denali, buf, mtd->writesize, page, 0, 0);
 	if (ret && ret != -EBADMSG)
 		return ret;
 
-	memcpy(buf, denali->buf, mtd->writesize);
-
 	if (denali->caps & DENALI_CAP_HW_ECC_FIXUP)
 		stat = denali_hw_ecc_fixup(mtd, denali, &uncor_ecc_flags);
 	else if (ret == -EBADMSG)
@@ -931,10 +933,8 @@ static int denali_write_page(struct mtd_info *mtd, struct nand_chip *chip,
 {
 	struct denali_nand_info *denali = mtd_to_denali(mtd);
 
-	memcpy(denali->buf, buf, mtd->writesize);
-
-	return denali_data_xfer(denali, denali->buf, mtd->writesize, page,
-				0, 1);
+	return denali_data_xfer(denali, (void *)buf, mtd->writesize,
+				page, 0, 1);
 }
 
 static void denali_select_chip(struct mtd_info *mtd, int chip)
@@ -1374,14 +1374,8 @@ int denali_init(struct denali_nand_info *denali)
 	}
 
 	if (denali->dma_avail) {
-		denali->dma_addr = dma_map_single(denali->dev, denali->buf,
-						  mtd->writesize + mtd->oobsize,
-						  DMA_BIDIRECTIONAL);
-		if (dma_mapping_error(denali->dev, denali->dma_addr)) {
-			dev_info(denali->dev,
-				 "Failed to map DMA buffer. Disabling DMA.\n");
-			denali->dma_avail = 0;
-		};
+		chip->options |= NAND_USE_BOUNCE_BUFFER;
+		chip->buf_align = 16;
 	}
 
 	/*
@@ -1471,16 +1465,8 @@ EXPORT_SYMBOL(denali_init);
 void denali_remove(struct denali_nand_info *denali)
 {
 	struct mtd_info *mtd = nand_to_mtd(&denali->nand);
-	/*
-	 * Pre-compute DMA buffer size to avoid any problems in case
-	 * nand_release() ever changes in a way that mtd->writesize and
-	 * mtd->oobsize are not reliable after this call.
-	 */
-	int bufsize = mtd->writesize + mtd->oobsize;
 
 	nand_release(mtd);
 	denali_disable_irq(denali);
-	dma_unmap_single(denali->dev, denali->dma_addr, bufsize,
-			 DMA_BIDIRECTIONAL);
 }
 EXPORT_SYMBOL(denali_remove);
-- 
2.7.4

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

* [PATCH v7 14/16] mtd: nand: denali: use non-managed kmalloc() for DMA buffer
  2017-06-13 13:45 [PATCH v7 00/16] mtd: nand: denali: Denali NAND IP improvements Masahiro Yamada
                   ` (12 preceding siblings ...)
  2017-06-13 13:45 ` [PATCH v7 13/16] mtd: nand: denali: skip driver internal bounce buffer when possible Masahiro Yamada
@ 2017-06-13 13:45 ` Masahiro Yamada
  2017-06-13 13:45 ` [PATCH v7 15/16] mtd: nand: denali: enable bad block table scan Masahiro Yamada
                   ` (2 subsequent siblings)
  16 siblings, 0 replies; 19+ messages in thread
From: Masahiro Yamada @ 2017-06-13 13:45 UTC (permalink / raw)
  To: linux-mtd
  Cc: Enrico Jorns, Artem Bityutskiy, Dinh Nguyen, Boris Brezillon,
	Marek Vasut, David Woodhouse, Masami Hiramatsu, Chuanxiao Dong,
	Jassi Brar, Masahiro Yamada, Russell King, Lars-Peter Clausen,
	Cyrille Pitchen, linux-kernel, Brian Norris, Richard Weinberger

As Russell and Lars stated in the discussion [1], using
devm_k*alloc() with DMA is not a good idea.

Let's use kmalloc (not kzalloc because no need for zero-out).
Also, allocate the buffer as late as possible because it must be
freed for any error that follows.

[1] https://lkml.org/lkml/2017/3/8/693

Signed-off-by: Masahiro Yamada <yamada.masahiro@socionext.com>
Cc: Russell King <rmk+kernel@armlinux.org.uk>
Cc: Lars-Peter Clausen <lars@metafoo.de>
Acked-by: Robin Murphy <robin.murphy@arm.com>
---

Changes in v7: None
Changes in v6: None
Changes in v5: None
Changes in v4: None
Changes in v3: None
Changes in v2:
  - Newly added

 drivers/mtd/nand/denali.c | 26 +++++++++++++++++---------
 1 file changed, 17 insertions(+), 9 deletions(-)

diff --git a/drivers/mtd/nand/denali.c b/drivers/mtd/nand/denali.c
index ec5fc8da5f9a..bb2da2fd069e 100644
--- a/drivers/mtd/nand/denali.c
+++ b/drivers/mtd/nand/denali.c
@@ -23,6 +23,7 @@
 #include <linux/mutex.h>
 #include <linux/mtd/mtd.h>
 #include <linux/module.h>
+#include <linux/slab.h>
 
 #include "denali.h"
 
@@ -1352,13 +1353,6 @@ int denali_init(struct denali_nand_info *denali)
 	if (ret)
 		goto disable_irq;
 
-	denali->buf = devm_kzalloc(denali->dev, mtd->writesize + mtd->oobsize,
-				   GFP_KERNEL);
-	if (!denali->buf) {
-		ret = -ENOMEM;
-		goto disable_irq;
-	}
-
 	if (ioread32(denali->flash_reg + FEATURES) & FEATURES__DMA)
 		denali->dma_avail = 1;
 
@@ -1443,17 +1437,30 @@ int denali_init(struct denali_nand_info *denali)
 	if (ret)
 		goto disable_irq;
 
+	/*
+	 * This buffer is DMA-mapped by denali_{read,write}_page_raw.  Do not
+	 * use devm_kmalloc() because the memory allocated by devm_ does not
+	 * guarantee DMA-safe alignment.
+	 */
+	denali->buf = kmalloc(mtd->writesize + mtd->oobsize, GFP_KERNEL);
+	if (!denali->buf) {
+		ret = -ENOMEM;
+		goto disable_irq;
+	}
+
 	ret = nand_scan_tail(mtd);
 	if (ret)
-		goto disable_irq;
+		goto free_buf;
 
 	ret = mtd_device_register(mtd, NULL, 0);
 	if (ret) {
 		dev_err(denali->dev, "Failed to register MTD: %d\n", ret);
-		goto disable_irq;
+		goto free_buf;
 	}
 	return 0;
 
+free_buf:
+	kfree(denali->buf);
 disable_irq:
 	denali_disable_irq(denali);
 
@@ -1467,6 +1474,7 @@ void denali_remove(struct denali_nand_info *denali)
 	struct mtd_info *mtd = nand_to_mtd(&denali->nand);
 
 	nand_release(mtd);
+	kfree(denali->buf);
 	denali_disable_irq(denali);
 }
 EXPORT_SYMBOL(denali_remove);
-- 
2.7.4

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

* [PATCH v7 15/16] mtd: nand: denali: enable bad block table scan
  2017-06-13 13:45 [PATCH v7 00/16] mtd: nand: denali: Denali NAND IP improvements Masahiro Yamada
                   ` (13 preceding siblings ...)
  2017-06-13 13:45 ` [PATCH v7 14/16] mtd: nand: denali: use non-managed kmalloc() for DMA buffer Masahiro Yamada
@ 2017-06-13 13:45 ` Masahiro Yamada
  2017-06-13 13:45 ` [PATCH v7 16/16] mtd: nand: denali: avoid magic numbers and rename for clarification Masahiro Yamada
  2017-06-20  7:28 ` [PATCH v7 00/16] mtd: nand: denali: Denali NAND IP improvements Boris Brezillon
  16 siblings, 0 replies; 19+ messages in thread
From: Masahiro Yamada @ 2017-06-13 13:45 UTC (permalink / raw)
  To: linux-mtd
  Cc: Enrico Jorns, Artem Bityutskiy, Dinh Nguyen, Boris Brezillon,
	Marek Vasut, David Woodhouse, Masami Hiramatsu, Chuanxiao Dong,
	Jassi Brar, Masahiro Yamada, Cyrille Pitchen, linux-kernel,
	Brian Norris, Richard Weinberger

Now this driver is ready to remove NAND_SKIP_BBTSCAN.

The BBT descriptors in denali.c are equivalent to the ones in
nand_bbt.c.  There is no need to duplicate the equivalent structures.
The with-oob decriptors do not work for this driver anyway.

The bbt_pattern (offs = 8) and the version (veroffs = 12) area
overlaps the ECC area.  Set NAND_BBT_NO_OOB flag to use the no_oob
variant of the BBT descriptors.

Signed-off-by: Masahiro Yamada <yamada.masahiro@socionext.com>
---

Changes in v7: None
Changes in v6: None
Changes in v5: None
Changes in v4: None
Changes in v3: None
Changes in v2:
  - Newly added

 drivers/mtd/nand/denali.c | 31 ++-----------------------------
 1 file changed, 2 insertions(+), 29 deletions(-)

diff --git a/drivers/mtd/nand/denali.c b/drivers/mtd/nand/denali.c
index bb2da2fd069e..5ac3eb07c6d6 100644
--- a/drivers/mtd/nand/denali.c
+++ b/drivers/mtd/nand/denali.c
@@ -1211,29 +1211,6 @@ static const struct mtd_ooblayout_ops denali_ooblayout_ops = {
 	.free = denali_ooblayout_free,
 };
 
-static uint8_t bbt_pattern[] = {'B', 'b', 't', '0' };
-static uint8_t mirror_pattern[] = {'1', 't', 'b', 'B' };
-
-static struct nand_bbt_descr bbt_main_descr = {
-	.options = NAND_BBT_LASTBLOCK | NAND_BBT_CREATE | NAND_BBT_WRITE
-		| NAND_BBT_2BIT | NAND_BBT_VERSION | NAND_BBT_PERCHIP,
-	.offs =	8,
-	.len = 4,
-	.veroffs = 12,
-	.maxblocks = 4,
-	.pattern = bbt_pattern,
-};
-
-static struct nand_bbt_descr bbt_mirror_descr = {
-	.options = NAND_BBT_LASTBLOCK | NAND_BBT_CREATE | NAND_BBT_WRITE
-		| NAND_BBT_2BIT | NAND_BBT_VERSION | NAND_BBT_PERCHIP,
-	.offs =	8,
-	.len = 4,
-	.veroffs = 12,
-	.maxblocks = 4,
-	.pattern = mirror_pattern,
-};
-
 /* initialize driver data structures */
 static void denali_drv_init(struct denali_nand_info *denali)
 {
@@ -1378,13 +1355,9 @@ int denali_init(struct denali_nand_info *denali)
 	 * bad block management.
 	 */
 
-	/* Bad block management */
-	chip->bbt_td = &bbt_main_descr;
-	chip->bbt_md = &bbt_mirror_descr;
-
-	/* skip the scan for now until we have OOB read and write support */
 	chip->bbt_options |= NAND_BBT_USE_FLASH;
-	chip->options |= NAND_SKIP_BBTSCAN;
+	chip->bbt_options |= NAND_BBT_NO_OOB;
+
 	chip->ecc.mode = NAND_ECC_HW_SYNDROME;
 
 	/* no subpage writes on denali */
-- 
2.7.4

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

* [PATCH v7 16/16] mtd: nand: denali: avoid magic numbers and rename for clarification
  2017-06-13 13:45 [PATCH v7 00/16] mtd: nand: denali: Denali NAND IP improvements Masahiro Yamada
                   ` (14 preceding siblings ...)
  2017-06-13 13:45 ` [PATCH v7 15/16] mtd: nand: denali: enable bad block table scan Masahiro Yamada
@ 2017-06-13 13:45 ` Masahiro Yamada
  2017-06-16  5:38   ` Masahiro Yamada
  2017-06-20  7:28 ` [PATCH v7 00/16] mtd: nand: denali: Denali NAND IP improvements Boris Brezillon
  16 siblings, 1 reply; 19+ messages in thread
From: Masahiro Yamada @ 2017-06-13 13:45 UTC (permalink / raw)
  To: linux-mtd
  Cc: Enrico Jorns, Artem Bityutskiy, Dinh Nguyen, Boris Brezillon,
	Marek Vasut, David Woodhouse, Masami Hiramatsu, Chuanxiao Dong,
	Jassi Brar, Masahiro Yamada, Cyrille Pitchen, linux-kernel,
	Brian Norris, Richard Weinberger

Introduce some macros and helpers to avoid magic numbers and
rename macros/functions for clarification.

- We see '| 2' in several places.  This means Data Cycle in MAP11 mode.
  The Denali User's Guide says bit[1:0] of MAP11 is like follows:

  b'00 = Command Cycle
  b'01 = Address Cycle
  b'10 = Data Cycle

  So, this commit added DENALI_MAP11_{CMD,ADDR,DATA} macros.

- We see 'denali->flash_mem + 0x10' in several places, but 0x10 is a
  magic number.  Actually, this accesses the data port of the Host
  Data/Command Interface.  So, this commit added DENALI_HOST_DATA.
  On the other hand, 'denali->flash_mem' gets access to the address
  port, so DENALI_HOST_ADDR was also added.

- We see 'index_addr(denali, cmd, 0x1)' in denali_erase(), but 0x1
  is a magic number.  0x1 means the erase operation.  Replace 0x1
  with DENALI_ERASE.

- Rename index_addr() to denali_host_write() for clarification

- Denali User's Guide says MAP{00,01,10,11} for access mode.  Match
  the macros with terminology in the IP document.

- Rename struct members as follows:
  flash_bank   -> active_bank    (currently selected bank)
  flash_reg    -> reg            (base address of registers)
  flash_mem    -> host           (base address of host interface)
  devnum       -> devs_per_cs    (devices connected in parallel)
  bbtskipbytes -> oob_skip_bytes (number of bytes to skip in OOB)

Signed-off-by: Masahiro Yamada <yamada.masahiro@socionext.com>
---

Changes in v7: None
Changes in v6:
  - Newly added

Changes in v5: None
Changes in v4: None
Changes in v3: None
Changes in v2: None

 drivers/mtd/nand/denali.c     | 247 +++++++++++++++++++++---------------------
 drivers/mtd/nand/denali.h     |  15 +--
 drivers/mtd/nand/denali_dt.c  |  12 +-
 drivers/mtd/nand/denali_pci.c |  16 +--
 4 files changed, 144 insertions(+), 146 deletions(-)

diff --git a/drivers/mtd/nand/denali.c b/drivers/mtd/nand/denali.c
index 5ac3eb07c6d6..70b4c4476c69 100644
--- a/drivers/mtd/nand/denali.c
+++ b/drivers/mtd/nand/denali.c
@@ -31,12 +31,26 @@ MODULE_LICENSE("GPL");
 
 #define DENALI_NAND_NAME    "denali-nand"
 
-/*
- * indicates whether or not the internal value for the flash bank is
- * valid or not
- */
-#define CHIP_SELECT_INVALID	-1
+/* Host Data/Command Interface */
+#define DENALI_HOST_ADDR	0x00
+#define DENALI_HOST_DATA	0x10
+
+#define DENALI_MAP00		(0 << 26)	/* direct access to buffer */
+#define DENALI_MAP01		(1 << 26)	/* read/write pages in PIO */
+#define DENALI_MAP10		(2 << 26)	/* higher-level control plane */
+#define DENALI_MAP11		(3 << 26)	/* direct controller access */
+
+/* MAP11 access cycle type */
+#define DENALI_MAP11_CMD	((DENALI_MAP11) | 0)	/* command cycle */
+#define DENALI_MAP11_ADDR	((DENALI_MAP11) | 1)	/* address cycle */
+#define DENALI_MAP11_DATA	((DENALI_MAP11) | 2)	/* data cycle */
+
+/* MAP11 commands */
+#define DENALI_ERASE		0x01
+
+#define DENALI_BANK(denali)	((denali)->active_bank << 24)
 
+#define DENALI_INVALID_BANK	-1
 #define DENALI_NR_BANKS		4
 
 /*
@@ -56,23 +70,11 @@ static inline struct denali_nand_info *mtd_to_denali(struct mtd_info *mtd)
 	return container_of(mtd_to_nand(mtd), struct denali_nand_info, nand);
 }
 
-/*
- * this is a helper macro that allows us to
- * format the bank into the proper bits for the controller
- */
-#define BANK(x) ((x) << 24)
-
-/*
- * Certain operations for the denali NAND controller use an indexed mode to
- * read/write data. The operation is performed by writing the address value
- * of the command to the device memory followed by the data. This function
- * abstracts this common operation.
- */
-static void index_addr(struct denali_nand_info *denali,
-				uint32_t address, uint32_t data)
+static void denali_host_write(struct denali_nand_info *denali,
+			      uint32_t addr, uint32_t data)
 {
-	iowrite32(address, denali->flash_mem);
-	iowrite32(data, denali->flash_mem + 0x10);
+	iowrite32(addr, denali->host + DENALI_HOST_ADDR);
+	iowrite32(data, denali->host + DENALI_HOST_DATA);
 }
 
 /*
@@ -81,7 +83,7 @@ static void index_addr(struct denali_nand_info *denali,
  */
 static void detect_max_banks(struct denali_nand_info *denali)
 {
-	uint32_t features = ioread32(denali->flash_reg + FEATURES);
+	uint32_t features = ioread32(denali->reg + FEATURES);
 
 	denali->max_banks = 1 << (features & FEATURES__N_BANKS);
 
@@ -95,8 +97,8 @@ static void denali_enable_irq(struct denali_nand_info *denali)
 	int i;
 
 	for (i = 0; i < DENALI_NR_BANKS; i++)
-		iowrite32(U32_MAX, denali->flash_reg + INTR_EN(i));
-	iowrite32(GLOBAL_INT_EN_FLAG, denali->flash_reg + GLOBAL_INT_ENABLE);
+		iowrite32(U32_MAX, denali->reg + INTR_EN(i));
+	iowrite32(GLOBAL_INT_EN_FLAG, denali->reg + GLOBAL_INT_ENABLE);
 }
 
 static void denali_disable_irq(struct denali_nand_info *denali)
@@ -104,15 +106,15 @@ static void denali_disable_irq(struct denali_nand_info *denali)
 	int i;
 
 	for (i = 0; i < DENALI_NR_BANKS; i++)
-		iowrite32(0, denali->flash_reg + INTR_EN(i));
-	iowrite32(0, denali->flash_reg + GLOBAL_INT_ENABLE);
+		iowrite32(0, denali->reg + INTR_EN(i));
+	iowrite32(0, denali->reg + GLOBAL_INT_ENABLE);
 }
 
 static void denali_clear_irq(struct denali_nand_info *denali,
 			     int bank, uint32_t irq_status)
 {
 	/* write one to clear bits */
-	iowrite32(irq_status, denali->flash_reg + INTR_STATUS(bank));
+	iowrite32(irq_status, denali->reg + INTR_STATUS(bank));
 }
 
 static void denali_clear_irq_all(struct denali_nand_info *denali)
@@ -133,13 +135,13 @@ static irqreturn_t denali_isr(int irq, void *dev_id)
 	spin_lock(&denali->irq_lock);
 
 	for (i = 0; i < DENALI_NR_BANKS; i++) {
-		irq_status = ioread32(denali->flash_reg + INTR_STATUS(i));
+		irq_status = ioread32(denali->reg + INTR_STATUS(i));
 		if (irq_status)
 			ret = IRQ_HANDLED;
 
 		denali_clear_irq(denali, i, irq_status);
 
-		if (i != denali->flash_bank)
+		if (i != denali->active_bank)
 			continue;
 
 		denali->irq_status |= irq_status;
@@ -220,8 +222,8 @@ static void setup_ecc_for_xfer(struct denali_nand_info *denali, bool ecc_en,
 	transfer_spare_flag = transfer_spare ? TRANSFER_SPARE_REG__FLAG : 0;
 
 	/* Enable spare area/ECC per user's request. */
-	iowrite32(ecc_en_flag, denali->flash_reg + ECC_ENABLE);
-	iowrite32(transfer_spare_flag, denali->flash_reg + TRANSFER_SPARE_REG);
+	iowrite32(ecc_en_flag, denali->reg + ECC_ENABLE);
+	iowrite32(transfer_spare_flag, denali->reg + TRANSFER_SPARE_REG);
 }
 
 static void denali_read_buf(struct mtd_info *mtd, uint8_t *buf, int len)
@@ -229,10 +231,11 @@ static void denali_read_buf(struct mtd_info *mtd, uint8_t *buf, int len)
 	struct denali_nand_info *denali = mtd_to_denali(mtd);
 	int i;
 
-	iowrite32(MODE_11 | BANK(denali->flash_bank) | 2, denali->flash_mem);
+	iowrite32(DENALI_MAP11_DATA | DENALI_BANK(denali),
+		  denali->host + DENALI_HOST_ADDR);
 
 	for (i = 0; i < len; i++)
-		buf[i] = ioread32(denali->flash_mem + 0x10);
+		buf[i] = ioread32(denali->host + DENALI_HOST_DATA);
 }
 
 static void denali_write_buf(struct mtd_info *mtd, const uint8_t *buf, int len)
@@ -240,10 +243,11 @@ static void denali_write_buf(struct mtd_info *mtd, const uint8_t *buf, int len)
 	struct denali_nand_info *denali = mtd_to_denali(mtd);
 	int i;
 
-	iowrite32(MODE_11 | BANK(denali->flash_bank) | 2, denali->flash_mem);
+	iowrite32(DENALI_MAP11_DATA | DENALI_BANK(denali),
+		  denali->host + DENALI_HOST_ADDR);
 
 	for (i = 0; i < len; i++)
-		iowrite32(buf[i], denali->flash_mem + 0x10);
+		iowrite32(buf[i], denali->host + DENALI_HOST_DATA);
 }
 
 static void denali_read_buf16(struct mtd_info *mtd, uint8_t *buf, int len)
@@ -252,10 +256,11 @@ static void denali_read_buf16(struct mtd_info *mtd, uint8_t *buf, int len)
 	uint16_t *buf16 = (uint16_t *)buf;
 	int i;
 
-	iowrite32(MODE_11 | BANK(denali->flash_bank) | 2, denali->flash_mem);
+	iowrite32(DENALI_MAP11_DATA | DENALI_BANK(denali),
+		  denali->host + DENALI_HOST_ADDR);
 
 	for (i = 0; i < len / 2; i++)
-		buf16[i] = ioread32(denali->flash_mem + 0x10);
+		buf16[i] = ioread32(denali->host + DENALI_HOST_DATA);
 }
 
 static void denali_write_buf16(struct mtd_info *mtd, const uint8_t *buf,
@@ -265,10 +270,11 @@ static void denali_write_buf16(struct mtd_info *mtd, const uint8_t *buf,
 	const uint16_t *buf16 = (const uint16_t *)buf;
 	int i;
 
-	iowrite32(MODE_11 | BANK(denali->flash_bank) | 2, denali->flash_mem);
+	iowrite32(DENALI_MAP11_DATA | DENALI_BANK(denali),
+		  denali->host + DENALI_HOST_ADDR);
 
 	for (i = 0; i < len / 2; i++)
-		iowrite32(buf16[i], denali->flash_mem + 0x10);
+		iowrite32(buf16[i], denali->host + DENALI_HOST_DATA);
 }
 
 static uint8_t denali_read_byte(struct mtd_info *mtd)
@@ -300,9 +306,9 @@ static void denali_cmd_ctrl(struct mtd_info *mtd, int dat, unsigned int ctrl)
 	uint32_t type;
 
 	if (ctrl & NAND_CLE)
-		type = 0;
+		type = DENALI_MAP11_CMD;
 	else if (ctrl & NAND_ALE)
-		type = 1;
+		type = DENALI_MAP11_ADDR;
 	else
 		return;
 
@@ -313,7 +319,7 @@ static void denali_cmd_ctrl(struct mtd_info *mtd, int dat, unsigned int ctrl)
 	if (ctrl & NAND_CTRL_CHANGE)
 		denali_reset_irq(denali);
 
-	index_addr(denali, MODE_11 | BANK(denali->flash_bank) | type, dat);
+	denali_host_write(denali, DENALI_BANK(denali) | type, dat);
 }
 
 static int denali_dev_ready(struct mtd_info *mtd)
@@ -366,11 +372,11 @@ static int denali_hw_ecc_fixup(struct mtd_info *mtd,
 			       unsigned long *uncor_ecc_flags)
 {
 	struct nand_chip *chip = mtd_to_nand(mtd);
-	int bank = denali->flash_bank;
+	int bank = denali->active_bank;
 	uint32_t ecc_cor;
 	unsigned int max_bitflips;
 
-	ecc_cor = ioread32(denali->flash_reg + ECC_COR_INFO(bank));
+	ecc_cor = ioread32(denali->reg + ECC_COR_INFO(bank));
 	ecc_cor >>= ECC_COR_INFO__SHIFT(bank);
 
 	if (ecc_cor & ECC_COR_INFO__UNCOR_ERR) {
@@ -419,11 +425,11 @@ static int denali_sw_ecc_fixup(struct mtd_info *mtd,
 	denali_reset_irq(denali);
 
 	do {
-		err_addr = ioread32(denali->flash_reg + ECC_ERROR_ADDRESS);
+		err_addr = ioread32(denali->reg + ECC_ERROR_ADDRESS);
 		err_sector = ECC_SECTOR(err_addr);
 		err_byte = ECC_BYTE(err_addr);
 
-		err_cor_info = ioread32(denali->flash_reg + ERR_CORRECTION_INFO);
+		err_cor_info = ioread32(denali->reg + ERR_CORRECTION_INFO);
 		err_cor_value = ECC_CORRECTION_VALUE(err_cor_info);
 		err_device = ECC_ERR_DEVICE(err_cor_info);
 
@@ -449,7 +455,7 @@ static int denali_sw_ecc_fixup(struct mtd_info *mtd,
 			unsigned int flips_in_byte;
 
 			offset = (err_sector * ecc_size + err_byte) *
-						denali->devnum + err_device;
+					denali->devs_per_cs + err_device;
 
 			/* correct the ECC error */
 			flips_in_byte = hweight8(buf[offset] ^ err_cor_value);
@@ -478,8 +484,8 @@ static int denali_sw_ecc_fixup(struct mtd_info *mtd,
 /* programs the controller to either enable/disable DMA transfers */
 static void denali_enable_dma(struct denali_nand_info *denali, bool en)
 {
-	iowrite32(en ? DMA_ENABLE__FLAG : 0, denali->flash_reg + DMA_ENABLE);
-	ioread32(denali->flash_reg + DMA_ENABLE);
+	iowrite32(en ? DMA_ENABLE__FLAG : 0, denali->reg + DMA_ENABLE);
+	ioread32(denali->reg + DMA_ENABLE);
 }
 
 static void denali_setup_dma64(struct denali_nand_info *denali,
@@ -488,7 +494,7 @@ static void denali_setup_dma64(struct denali_nand_info *denali,
 	uint32_t mode;
 	const int page_count = 1;
 
-	mode = MODE_10 | BANK(denali->flash_bank) | page;
+	mode = DENALI_MAP10 | DENALI_BANK(denali) | page;
 
 	/* DMA is a three step process */
 
@@ -496,14 +502,14 @@ static void denali_setup_dma64(struct denali_nand_info *denali,
 	 * 1. setup transfer type, interrupt when complete,
 	 *    burst len = 64 bytes, the number of pages
 	 */
-	index_addr(denali, mode,
-		   0x01002000 | (64 << 16) | (write << 8) | page_count);
+	denali_host_write(denali, mode,
+			  0x01002000 | (64 << 16) | (write << 8) | page_count);
 
 	/* 2. set memory low address */
-	index_addr(denali, mode, dma_addr);
+	denali_host_write(denali, mode, dma_addr);
 
 	/* 3. set memory high address */
-	index_addr(denali, mode, (uint64_t)dma_addr >> 32);
+	denali_host_write(denali, mode, (uint64_t)dma_addr >> 32);
 }
 
 static void denali_setup_dma32(struct denali_nand_info *denali,
@@ -512,21 +518,22 @@ static void denali_setup_dma32(struct denali_nand_info *denali,
 	uint32_t mode;
 	const int page_count = 1;
 
-	mode = MODE_10 | BANK(denali->flash_bank);
+	mode = DENALI_MAP10 | DENALI_BANK(denali);
 
 	/* DMA is a four step process */
 
 	/* 1. setup transfer type and # of pages */
-	index_addr(denali, mode | page, 0x2000 | (write << 8) | page_count);
+	denali_host_write(denali, mode | page,
+			  0x2000 | (write << 8) | page_count);
 
 	/* 2. set memory high address bits 23:8 */
-	index_addr(denali, mode | ((dma_addr >> 16) << 8), 0x2200);
+	denali_host_write(denali, mode | ((dma_addr >> 16) << 8), 0x2200);
 
 	/* 3. set memory low address bits 23:8 */
-	index_addr(denali, mode | ((dma_addr & 0xffff) << 8), 0x2300);
+	denali_host_write(denali, mode | ((dma_addr & 0xffff) << 8), 0x2300);
 
 	/* 4. interrupt when complete, burst len = 64 bytes */
-	index_addr(denali, mode | 0x14000, 0x2400);
+	denali_host_write(denali, mode | 0x14000, 0x2400);
 }
 
 static void denali_setup_dma(struct denali_nand_info *denali,
@@ -541,7 +548,7 @@ static void denali_setup_dma(struct denali_nand_info *denali,
 static int denali_pio_read(struct denali_nand_info *denali, void *buf,
 			   size_t size, int page, int raw)
 {
-	uint32_t addr = BANK(denali->flash_bank) | page;
+	uint32_t addr = DENALI_BANK(denali) | page;
 	uint32_t *buf32 = (uint32_t *)buf;
 	uint32_t irq_status, ecc_err_mask;
 	int i;
@@ -553,9 +560,9 @@ static int denali_pio_read(struct denali_nand_info *denali, void *buf,
 
 	denali_reset_irq(denali);
 
-	iowrite32(MODE_01 | addr, denali->flash_mem);
+	iowrite32(DENALI_MAP01 | addr, denali->host + DENALI_HOST_ADDR);
 	for (i = 0; i < size / 4; i++)
-		*buf32++ = ioread32(denali->flash_mem + 0x10);
+		*buf32++ = ioread32(denali->host + DENALI_HOST_DATA);
 
 	irq_status = denali_wait_for_irq(denali, INTR__PAGE_XFER_INC);
 	if (!(irq_status & INTR__PAGE_XFER_INC))
@@ -570,16 +577,16 @@ static int denali_pio_read(struct denali_nand_info *denali, void *buf,
 static int denali_pio_write(struct denali_nand_info *denali,
 			    const void *buf, size_t size, int page, int raw)
 {
-	uint32_t addr = BANK(denali->flash_bank) | page;
+	uint32_t addr = DENALI_BANK(denali) | page;
 	const uint32_t *buf32 = (uint32_t *)buf;
 	uint32_t irq_status;
 	int i;
 
 	denali_reset_irq(denali);
 
-	iowrite32(MODE_01 | addr, denali->flash_mem);
+	iowrite32(DENALI_MAP01 | addr, denali->host + DENALI_HOST_ADDR);
 	for (i = 0; i < size / 4; i++)
-		iowrite32(*buf32++, denali->flash_mem + 0x10);
+		iowrite32(*buf32++, denali->host + DENALI_HOST_DATA);
 
 	irq_status = denali_wait_for_irq(denali,
 				INTR__PROGRAM_COMP | INTR__PROGRAM_FAIL);
@@ -672,7 +679,7 @@ static void denali_oob_xfer(struct mtd_info *mtd, struct nand_chip *chip,
 	int ecc_steps = chip->ecc.steps;
 	int ecc_size = chip->ecc.size;
 	int ecc_bytes = chip->ecc.bytes;
-	int oob_skip = denali->bbtskipbytes;
+	int oob_skip = denali->oob_skip_bytes;
 	size_t size = writesize + oobsize;
 	int i, pos, len;
 
@@ -730,7 +737,7 @@ static int denali_read_page_raw(struct mtd_info *mtd, struct nand_chip *chip,
 	int ecc_size = chip->ecc.size;
 	int ecc_bytes = chip->ecc.bytes;
 	void *dma_buf = denali->buf;
-	int oob_skip = denali->bbtskipbytes;
+	int oob_skip = denali->oob_skip_bytes;
 	size_t size = writesize + oobsize;
 	int ret, i, pos, len;
 
@@ -861,7 +868,7 @@ static int denali_write_page_raw(struct mtd_info *mtd, struct nand_chip *chip,
 	int ecc_size = chip->ecc.size;
 	int ecc_bytes = chip->ecc.bytes;
 	void *dma_buf = denali->buf;
-	int oob_skip = denali->bbtskipbytes;
+	int oob_skip = denali->oob_skip_bytes;
 	size_t size = writesize + oobsize;
 	int i, pos, len;
 
@@ -942,7 +949,7 @@ static void denali_select_chip(struct mtd_info *mtd, int chip)
 {
 	struct denali_nand_info *denali = mtd_to_denali(mtd);
 
-	denali->flash_bank = chip;
+	denali->active_bank = chip;
 }
 
 static int denali_waitfunc(struct mtd_info *mtd, struct nand_chip *chip)
@@ -959,13 +966,12 @@ static int denali_waitfunc(struct mtd_info *mtd, struct nand_chip *chip)
 static int denali_erase(struct mtd_info *mtd, int page)
 {
 	struct denali_nand_info *denali = mtd_to_denali(mtd);
-	uint32_t cmd, irq_status;
+	uint32_t irq_status;
 
 	denali_reset_irq(denali);
 
-	/* setup page read request for access type */
-	cmd = MODE_10 | BANK(denali->flash_bank) | page;
-	index_addr(denali, cmd, 0x1);
+	denali_host_write(denali, DENALI_MAP10 | DENALI_BANK(denali) | page,
+			  DENALI_ERASE);
 
 	/* wait for erase to complete or failure to occur */
 	irq_status = denali_wait_for_irq(denali,
@@ -1004,37 +1010,37 @@ static int denali_setup_data_interface(struct mtd_info *mtd, int chipnr,
 	acc_clks = DIV_ROUND_UP(timings->tREA_max, t_clk);
 	acc_clks = min_t(int, acc_clks, ACC_CLKS__VALUE);
 
-	tmp = ioread32(denali->flash_reg + ACC_CLKS);
+	tmp = ioread32(denali->reg + ACC_CLKS);
 	tmp &= ~ACC_CLKS__VALUE;
 	tmp |= acc_clks;
-	iowrite32(tmp, denali->flash_reg + ACC_CLKS);
+	iowrite32(tmp, denali->reg + ACC_CLKS);
 
 	/* tRWH -> RE_2_WE */
 	re_2_we = DIV_ROUND_UP(timings->tRHW_min, t_clk);
 	re_2_we = min_t(int, re_2_we, RE_2_WE__VALUE);
 
-	tmp = ioread32(denali->flash_reg + RE_2_WE);
+	tmp = ioread32(denali->reg + RE_2_WE);
 	tmp &= ~RE_2_WE__VALUE;
 	tmp |= re_2_we;
-	iowrite32(tmp, denali->flash_reg + RE_2_WE);
+	iowrite32(tmp, denali->reg + RE_2_WE);
 
 	/* tRHZ -> RE_2_RE */
 	re_2_re = DIV_ROUND_UP(timings->tRHZ_max, t_clk);
 	re_2_re = min_t(int, re_2_re, RE_2_RE__VALUE);
 
-	tmp = ioread32(denali->flash_reg + RE_2_RE);
+	tmp = ioread32(denali->reg + RE_2_RE);
 	tmp &= ~RE_2_RE__VALUE;
 	tmp |= re_2_re;
-	iowrite32(tmp, denali->flash_reg + RE_2_RE);
+	iowrite32(tmp, denali->reg + RE_2_RE);
 
 	/* tWHR -> WE_2_RE */
 	we_2_re = DIV_ROUND_UP(timings->tWHR_min, t_clk);
 	we_2_re = min_t(int, we_2_re, TWHR2_AND_WE_2_RE__WE_2_RE);
 
-	tmp = ioread32(denali->flash_reg + TWHR2_AND_WE_2_RE);
+	tmp = ioread32(denali->reg + TWHR2_AND_WE_2_RE);
 	tmp &= ~TWHR2_AND_WE_2_RE__WE_2_RE;
 	tmp |= we_2_re;
-	iowrite32(tmp, denali->flash_reg + TWHR2_AND_WE_2_RE);
+	iowrite32(tmp, denali->reg + TWHR2_AND_WE_2_RE);
 
 	/* tADL -> ADDR_2_DATA */
 
@@ -1046,20 +1052,20 @@ static int denali_setup_data_interface(struct mtd_info *mtd, int chipnr,
 	addr_2_data = DIV_ROUND_UP(timings->tADL_min, t_clk);
 	addr_2_data = min_t(int, addr_2_data, addr_2_data_mask);
 
-	tmp = ioread32(denali->flash_reg + TCWAW_AND_ADDR_2_DATA);
+	tmp = ioread32(denali->reg + TCWAW_AND_ADDR_2_DATA);
 	tmp &= ~addr_2_data_mask;
 	tmp |= addr_2_data;
-	iowrite32(tmp, denali->flash_reg + TCWAW_AND_ADDR_2_DATA);
+	iowrite32(tmp, denali->reg + TCWAW_AND_ADDR_2_DATA);
 
 	/* tREH, tWH -> RDWR_EN_HI_CNT */
 	rdwr_en_hi = DIV_ROUND_UP(max(timings->tREH_min, timings->tWH_min),
 				  t_clk);
 	rdwr_en_hi = min_t(int, rdwr_en_hi, RDWR_EN_HI_CNT__VALUE);
 
-	tmp = ioread32(denali->flash_reg + RDWR_EN_HI_CNT);
+	tmp = ioread32(denali->reg + RDWR_EN_HI_CNT);
 	tmp &= ~RDWR_EN_HI_CNT__VALUE;
 	tmp |= rdwr_en_hi;
-	iowrite32(tmp, denali->flash_reg + RDWR_EN_HI_CNT);
+	iowrite32(tmp, denali->reg + RDWR_EN_HI_CNT);
 
 	/* tRP, tWP -> RDWR_EN_LO_CNT */
 	rdwr_en_lo = DIV_ROUND_UP(max(timings->tRP_min, timings->tWP_min),
@@ -1070,10 +1076,10 @@ static int denali_setup_data_interface(struct mtd_info *mtd, int chipnr,
 	rdwr_en_lo = max(rdwr_en_lo, rdwr_en_lo_hi - rdwr_en_hi);
 	rdwr_en_lo = min_t(int, rdwr_en_lo, RDWR_EN_LO_CNT__VALUE);
 
-	tmp = ioread32(denali->flash_reg + RDWR_EN_LO_CNT);
+	tmp = ioread32(denali->reg + RDWR_EN_LO_CNT);
 	tmp &= ~RDWR_EN_LO_CNT__VALUE;
 	tmp |= rdwr_en_lo;
-	iowrite32(tmp, denali->flash_reg + RDWR_EN_LO_CNT);
+	iowrite32(tmp, denali->reg + RDWR_EN_LO_CNT);
 
 	/* tCS, tCEA -> CS_SETUP_CNT */
 	cs_setup = max3((int)DIV_ROUND_UP(timings->tCS_min, t_clk) - rdwr_en_lo,
@@ -1081,10 +1087,10 @@ static int denali_setup_data_interface(struct mtd_info *mtd, int chipnr,
 			0);
 	cs_setup = min_t(int, cs_setup, CS_SETUP_CNT__VALUE);
 
-	tmp = ioread32(denali->flash_reg + CS_SETUP_CNT);
+	tmp = ioread32(denali->reg + CS_SETUP_CNT);
 	tmp &= ~CS_SETUP_CNT__VALUE;
 	tmp |= cs_setup;
-	iowrite32(tmp, denali->flash_reg + CS_SETUP_CNT);
+	iowrite32(tmp, denali->reg + CS_SETUP_CNT);
 
 	return 0;
 }
@@ -1095,12 +1101,12 @@ static void denali_reset_banks(struct denali_nand_info *denali)
 	int i;
 
 	for (i = 0; i < denali->max_banks; i++) {
-		denali->flash_bank = i;
+		denali->active_bank = i;
 
 		denali_reset_irq(denali);
 
 		iowrite32(DEVICE_RESET__BANK(i),
-			  denali->flash_reg + DEVICE_RESET);
+			  denali->reg + DEVICE_RESET);
 
 		irq_status = denali_wait_for_irq(denali,
 			INTR__RST_COMP | INTR__INT_ACT | INTR__TIME_OUT);
@@ -1119,8 +1125,7 @@ static void denali_hw_init(struct denali_nand_info *denali)
 	 * override it.
 	 */
 	if (!denali->revision)
-		denali->revision =
-				swab16(ioread32(denali->flash_reg + REVISION));
+		denali->revision = swab16(ioread32(denali->reg + REVISION));
 
 	/*
 	 * tell driver how many bit controller will skip before
@@ -1128,18 +1133,16 @@ static void denali_hw_init(struct denali_nand_info *denali)
 	 * set by firmware. So we read this value out.
 	 * if this value is 0, just let it be.
 	 */
-	denali->bbtskipbytes = ioread32(denali->flash_reg +
-						SPARE_AREA_SKIP_BYTES);
+	denali->oob_skip_bytes = ioread32(denali->reg + SPARE_AREA_SKIP_BYTES);
 	detect_max_banks(denali);
-	iowrite32(0x0F, denali->flash_reg + RB_PIN_ENABLED);
-	iowrite32(CHIP_EN_DONT_CARE__FLAG,
-			denali->flash_reg + CHIP_ENABLE_DONT_CARE);
+	iowrite32(0x0F, denali->reg + RB_PIN_ENABLED);
+	iowrite32(CHIP_EN_DONT_CARE__FLAG, denali->reg + CHIP_ENABLE_DONT_CARE);
 
-	iowrite32(0xffff, denali->flash_reg + SPARE_AREA_MARKER);
+	iowrite32(0xffff, denali->reg + SPARE_AREA_MARKER);
 
 	/* Should set value for these registers when init */
-	iowrite32(0, denali->flash_reg + TWO_ROW_ADDR_CYCLES);
-	iowrite32(1, denali->flash_reg + ECC_ENABLE);
+	iowrite32(0, denali->reg + TWO_ROW_ADDR_CYCLES);
+	iowrite32(1, denali->reg + ECC_ENABLE);
 }
 
 int denali_calc_ecc_bytes(int step_size, int strength)
@@ -1152,7 +1155,7 @@ EXPORT_SYMBOL(denali_calc_ecc_bytes);
 static int denali_ecc_setup(struct mtd_info *mtd, struct nand_chip *chip,
 			    struct denali_nand_info *denali)
 {
-	int oobavail = mtd->oobsize - denali->bbtskipbytes;
+	int oobavail = mtd->oobsize - denali->oob_skip_bytes;
 	int ret;
 
 	/*
@@ -1185,7 +1188,7 @@ static int denali_ooblayout_ecc(struct mtd_info *mtd, int section,
 	if (section)
 		return -ERANGE;
 
-	oobregion->offset = denali->bbtskipbytes;
+	oobregion->offset = denali->oob_skip_bytes;
 	oobregion->length = chip->ecc.total;
 
 	return 0;
@@ -1200,7 +1203,7 @@ static int denali_ooblayout_free(struct mtd_info *mtd, int section,
 	if (section)
 		return -ERANGE;
 
-	oobregion->offset = chip->ecc.total + denali->bbtskipbytes;
+	oobregion->offset = chip->ecc.total + denali->oob_skip_bytes;
 	oobregion->length = mtd->oobsize - oobregion->offset;
 
 	return 0;
@@ -1239,23 +1242,23 @@ static int denali_multidev_fixup(struct denali_nand_info *denali)
 	 * In this case, the core framework knows nothing about this fact,
 	 * so we should tell it the _logical_ pagesize and anything necessary.
 	 */
-	denali->devnum = ioread32(denali->flash_reg + DEVICES_CONNECTED);
+	denali->devs_per_cs = ioread32(denali->reg + DEVICES_CONNECTED);
 
 	/*
 	 * On some SoCs, DEVICES_CONNECTED is not auto-detected.
 	 * For those, DEVICES_CONNECTED is left to 0.  Set 1 if it is the case.
 	 */
-	if (denali->devnum == 0) {
-		denali->devnum = 1;
-		iowrite32(1, denali->flash_reg + DEVICES_CONNECTED);
+	if (denali->devs_per_cs == 0) {
+		denali->devs_per_cs = 1;
+		iowrite32(1, denali->reg + DEVICES_CONNECTED);
 	}
 
-	if (denali->devnum == 1)
+	if (denali->devs_per_cs == 1)
 		return 0;
 
-	if (denali->devnum != 2) {
+	if (denali->devs_per_cs != 2) {
 		dev_err(denali->dev, "unsupported number of devices %d\n",
-			denali->devnum);
+			denali->devs_per_cs);
 		return -EINVAL;
 	}
 
@@ -1273,7 +1276,7 @@ static int denali_multidev_fixup(struct denali_nand_info *denali)
 	chip->ecc.size <<= 1;
 	chip->ecc.bytes <<= 1;
 	chip->ecc.strength <<= 1;
-	denali->bbtskipbytes <<= 1;
+	denali->oob_skip_bytes <<= 1;
 
 	return 0;
 }
@@ -1301,7 +1304,7 @@ int denali_init(struct denali_nand_info *denali)
 	denali_enable_irq(denali);
 	denali_reset_banks(denali);
 
-	denali->flash_bank = CHIP_SELECT_INVALID;
+	denali->active_bank = DENALI_INVALID_BANK;
 
 	nand_set_flash_node(chip, denali->dev->of_node);
 	/* Fallback to the default name if DT did not give "label" property */
@@ -1330,7 +1333,7 @@ int denali_init(struct denali_nand_info *denali)
 	if (ret)
 		goto disable_irq;
 
-	if (ioread32(denali->flash_reg + FEATURES) & FEATURES__DMA)
+	if (ioread32(denali->reg + FEATURES) & FEATURES__DMA)
 		denali->dma_avail = 1;
 
 	if (denali->dma_avail) {
@@ -1374,19 +1377,19 @@ int denali_init(struct denali_nand_info *denali)
 		chip->ecc.size, chip->ecc.strength, chip->ecc.bytes);
 
 	iowrite32(MAKE_ECC_CORRECTION(chip->ecc.strength, 1),
-		  denali->flash_reg + ECC_CORRECTION);
+		  denali->reg + ECC_CORRECTION);
 	iowrite32(mtd->erasesize / mtd->writesize,
-		  denali->flash_reg + PAGES_PER_BLOCK);
+		  denali->reg + PAGES_PER_BLOCK);
 	iowrite32(chip->options & NAND_BUSWIDTH_16 ? 1 : 0,
-		  denali->flash_reg + DEVICE_WIDTH);
-	iowrite32(mtd->writesize, denali->flash_reg + DEVICE_MAIN_AREA_SIZE);
-	iowrite32(mtd->oobsize, denali->flash_reg + DEVICE_SPARE_AREA_SIZE);
+		  denali->reg + DEVICE_WIDTH);
+	iowrite32(mtd->writesize, denali->reg + DEVICE_MAIN_AREA_SIZE);
+	iowrite32(mtd->oobsize, denali->reg + DEVICE_SPARE_AREA_SIZE);
 
-	iowrite32(chip->ecc.size, denali->flash_reg + CFG_DATA_BLOCK_SIZE);
-	iowrite32(chip->ecc.size, denali->flash_reg + CFG_LAST_DATA_BLOCK_SIZE);
+	iowrite32(chip->ecc.size, denali->reg + CFG_DATA_BLOCK_SIZE);
+	iowrite32(chip->ecc.size, denali->reg + CFG_LAST_DATA_BLOCK_SIZE);
 	/* chip->ecc.steps is set by nand_scan_tail(); not available here */
 	iowrite32(mtd->writesize / chip->ecc.size,
-		  denali->flash_reg + CFG_NUM_DATA_BLOCKS);
+		  denali->reg + CFG_NUM_DATA_BLOCKS);
 
 	mtd_set_ooblayout(mtd, &denali_ooblayout_ops);
 
diff --git a/drivers/mtd/nand/denali.h b/drivers/mtd/nand/denali.h
index 657a794af695..237cc706b0fb 100644
--- a/drivers/mtd/nand/denali.h
+++ b/drivers/mtd/nand/denali.h
@@ -303,18 +303,13 @@
 #define     CHNL_ACTIVE__CHANNEL2			BIT(2)
 #define     CHNL_ACTIVE__CHANNEL3			BIT(3)
 
-#define MODE_00    0x00000000
-#define MODE_01    0x04000000
-#define MODE_10    0x08000000
-#define MODE_11    0x0C000000
-
 struct denali_nand_info {
 	struct nand_chip nand;
 	unsigned long clk_x_rate;	/* bus interface clock rate */
-	int flash_bank; /* currently selected chip */
+	int active_bank;		/* currently selected bank */
 	struct device *dev;
-	void __iomem *flash_reg;	/* Register Interface */
-	void __iomem *flash_mem;	/* Host Data/Command Interface */
+	void __iomem *reg;		/* Register Interface */
+	void __iomem *host;		/* Host Data/Command Interface */
 
 	/* elements used by ISR */
 	struct completion complete;
@@ -326,8 +321,8 @@ struct denali_nand_info {
 	void *buf;
 	dma_addr_t dma_addr;
 	int dma_avail;
-	int devnum;	/* represent how many nands connected */
-	int bbtskipbytes;
+	int devs_per_cs;		/* devices connected in parallel */
+	int oob_skip_bytes;
 	int max_banks;
 	unsigned int revision;
 	unsigned int caps;
diff --git a/drivers/mtd/nand/denali_dt.c b/drivers/mtd/nand/denali_dt.c
index ebcce50f4005..47f398edf18f 100644
--- a/drivers/mtd/nand/denali_dt.c
+++ b/drivers/mtd/nand/denali_dt.c
@@ -104,14 +104,14 @@ static int denali_dt_probe(struct platform_device *pdev)
 	}
 
 	res = platform_get_resource_byname(pdev, IORESOURCE_MEM, "denali_reg");
-	denali->flash_reg = devm_ioremap_resource(&pdev->dev, res);
-	if (IS_ERR(denali->flash_reg))
-		return PTR_ERR(denali->flash_reg);
+	denali->reg = devm_ioremap_resource(&pdev->dev, res);
+	if (IS_ERR(denali->reg))
+		return PTR_ERR(denali->reg);
 
 	res = platform_get_resource_byname(pdev, IORESOURCE_MEM, "nand_data");
-	denali->flash_mem = devm_ioremap_resource(&pdev->dev, res);
-	if (IS_ERR(denali->flash_mem))
-		return PTR_ERR(denali->flash_mem);
+	denali->host = devm_ioremap_resource(&pdev->dev, res);
+	if (IS_ERR(denali->host))
+		return PTR_ERR(denali->host);
 
 	dt->clk = devm_clk_get(&pdev->dev, NULL);
 	if (IS_ERR(dt->clk)) {
diff --git a/drivers/mtd/nand/denali_pci.c b/drivers/mtd/nand/denali_pci.c
index 6217525c1000..81370c79aa48 100644
--- a/drivers/mtd/nand/denali_pci.c
+++ b/drivers/mtd/nand/denali_pci.c
@@ -78,14 +78,14 @@ static int denali_pci_probe(struct pci_dev *dev, const struct pci_device_id *id)
 		return ret;
 	}
 
-	denali->flash_reg = ioremap_nocache(csr_base, csr_len);
-	if (!denali->flash_reg) {
+	denali->reg = ioremap_nocache(csr_base, csr_len);
+	if (!denali->reg) {
 		dev_err(&dev->dev, "Spectra: Unable to remap memory region\n");
 		return -ENOMEM;
 	}
 
-	denali->flash_mem = ioremap_nocache(mem_base, mem_len);
-	if (!denali->flash_mem) {
+	denali->host = ioremap_nocache(mem_base, mem_len);
+	if (!denali->host) {
 		dev_err(&dev->dev, "Spectra: ioremap_nocache failed!");
 		ret = -ENOMEM;
 		goto failed_remap_reg;
@@ -100,9 +100,9 @@ static int denali_pci_probe(struct pci_dev *dev, const struct pci_device_id *id)
 	return 0;
 
 failed_remap_mem:
-	iounmap(denali->flash_mem);
+	iounmap(denali->host);
 failed_remap_reg:
-	iounmap(denali->flash_reg);
+	iounmap(denali->reg);
 	return ret;
 }
 
@@ -112,8 +112,8 @@ static void denali_pci_remove(struct pci_dev *dev)
 	struct denali_nand_info *denali = pci_get_drvdata(dev);
 
 	denali_remove(denali);
-	iounmap(denali->flash_reg);
-	iounmap(denali->flash_mem);
+	iounmap(denali->reg);
+	iounmap(denali->host);
 }
 
 static struct pci_driver denali_pci_driver = {
-- 
2.7.4

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

* Re: [PATCH v7 16/16] mtd: nand: denali: avoid magic numbers and rename for clarification
  2017-06-13 13:45 ` [PATCH v7 16/16] mtd: nand: denali: avoid magic numbers and rename for clarification Masahiro Yamada
@ 2017-06-16  5:38   ` Masahiro Yamada
  0 siblings, 0 replies; 19+ messages in thread
From: Masahiro Yamada @ 2017-06-16  5:38 UTC (permalink / raw)
  To: linux-mtd
  Cc: Boris Brezillon, Cyrille Pitchen, Richard Weinberger,
	Marek Vasut, Masahiro Yamada, Artem Bityutskiy, Dinh Nguyen,
	Linux Kernel Mailing List, Enrico Jorns, Masami Hiramatsu,
	Chuanxiao Dong, Jassi Brar, Brian Norris, David Woodhouse

2017-06-13 22:45 GMT+09:00 Masahiro Yamada <yamada.masahiro@socionext.com>:
> Introduce some macros and helpers to avoid magic numbers and
> rename macros/functions for clarification.
>
> - We see '| 2' in several places.  This means Data Cycle in MAP11 mode.
>   The Denali User's Guide says bit[1:0] of MAP11 is like follows:
>
>   b'00 = Command Cycle
>   b'01 = Address Cycle
>   b'10 = Data Cycle
>
>   So, this commit added DENALI_MAP11_{CMD,ADDR,DATA} macros.
>
> - We see 'denali->flash_mem + 0x10' in several places, but 0x10 is a
>   magic number.  Actually, this accesses the data port of the Host
>   Data/Command Interface.  So, this commit added DENALI_HOST_DATA.
>   On the other hand, 'denali->flash_mem' gets access to the address
>   port, so DENALI_HOST_ADDR was also added.
>
> - We see 'index_addr(denali, cmd, 0x1)' in denali_erase(), but 0x1
>   is a magic number.  0x1 means the erase operation.  Replace 0x1
>   with DENALI_ERASE.
>
> - Rename index_addr() to denali_host_write() for clarification
>
> - Denali User's Guide says MAP{00,01,10,11} for access mode.  Match
>   the macros with terminology in the IP document.
>
> - Rename struct members as follows:
>   flash_bank   -> active_bank    (currently selected bank)
>   flash_reg    -> reg            (base address of registers)
>   flash_mem    -> host           (base address of host interface)
>   devnum       -> devs_per_cs    (devices connected in parallel)
>   bbtskipbytes -> oob_skip_bytes (number of bytes to skip in OOB)
>
> Signed-off-by: Masahiro Yamada <yamada.masahiro@socionext.com>
> ---
>
> Changes in v7: None
> Changes in v6:
>   - Newly added
>
> Changes in v5: None
> Changes in v4: None
> Changes in v3: None
> Changes in v2: None


Please let me update 18.  (only typo fix)

I sent v8 for this one.


-- 
Best Regards
Masahiro Yamada

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

* Re: [PATCH v7 00/16] mtd: nand: denali: Denali NAND IP improvements
  2017-06-13 13:45 [PATCH v7 00/16] mtd: nand: denali: Denali NAND IP improvements Masahiro Yamada
                   ` (15 preceding siblings ...)
  2017-06-13 13:45 ` [PATCH v7 16/16] mtd: nand: denali: avoid magic numbers and rename for clarification Masahiro Yamada
@ 2017-06-20  7:28 ` Boris Brezillon
  16 siblings, 0 replies; 19+ messages in thread
From: Boris Brezillon @ 2017-06-20  7:28 UTC (permalink / raw)
  To: Masahiro Yamada
  Cc: linux-mtd, Enrico Jorns, Artem Bityutskiy, Dinh Nguyen,
	Marek Vasut, David Woodhouse, Masami Hiramatsu, Chuanxiao Dong,
	Jassi Brar, Cyrille Pitchen, linux-kernel, Brian Norris,
	Richard Weinberger

On Tue, 13 Jun 2017 22:45:34 +0900
Masahiro Yamada <yamada.masahiro@socionext.com> wrote:

> This patch series intends to solve various problems.
> 
> [1] The driver just retrieves the OOB area as-is
>     whereas the controller uses syndrome page layout.
> [2] ONFi devices are not working
> [3] It can not read Bad Block Marker
> 
> Outstanding changes are:
>  - Fix raw/oob callbacks for syndrome page layout
>  - Implement setup_data_interface() callback
>  - Fix/implement more commands for ONFi devices
>  - Allow to skip the driver internal bounce buffer
>  - Support PIO in case DMA is not supported
>  - Switch from ->cmdfunc over to ->cmd_ctrl
> 
> 18 patches were merged by v2.
> 11 patches were merged by v3.
> 2 patches were merged by v4.
> 5 patches were merged by v5.
> Here is the rest of the series.
> 
> v1: https://lkml.org/lkml/2016/11/26/144
> v2: https://lkml.org/lkml/2017/3/22/804
> v3: https://lkml.org/lkml/2017/3/30/90
> v4: https://lkml.org/lkml/2017/6/5/1005
> 
> 
> Masahiro Yamada (16):
>   mtd: nand: denali: set NAND_ECC_CUSTOM_PAGE_ACCESS
>   mtd: nand: denali: remove unneeded find_valid_banks()
>   mtd: nand: denali: handle timing parameters by setup_data_interface()
>   mtd: nand: denali: rework interrupt handling
>   mtd: nand: denali: switch over to cmd_ctrl instead of cmdfunc
>   mtd: nand: denali: fix bank reset function to detect the number of
>     chips
>   mtd: nand: denali: use interrupt instead of polling for bank reset
>   mtd: nand: denali: propagate page to helpers via function argument
>   mtd: nand: denali: merge struct nand_buf into struct denali_nand_info
>   mtd: nand: denali: use flag instead of register macro for direction
>   mtd: nand: denali: fix raw and oob accessors for syndrome page layout
>   mtd: nand: denali: support hardware-assisted erased page detection
>   mtd: nand: denali: skip driver internal bounce buffer when possible
>   mtd: nand: denali: use non-managed kmalloc() for DMA buffer
>   mtd: nand: denali: enable bad block table scan
>   mtd: nand: denali: avoid magic numbers and rename for clarification

Applied patches 1 to 15 and v8 of patch 16.

Thanks a lot for your patience and the cleanup/rework you've done.

BTW, I'd be happy to have you flagged as the maintainer of the NAND
denali driver. If you like the idea, just send a patch adding an entry
in MAINTAINERS.

Regards,

Boris

> 
>  drivers/mtd/nand/denali.c     | 1724 ++++++++++++++++++-----------------------
>  drivers/mtd/nand/denali.h     |   63 +-
>  drivers/mtd/nand/denali_dt.c  |   15 +-
>  drivers/mtd/nand/denali_pci.c |   22 +-
>  4 files changed, 806 insertions(+), 1018 deletions(-)
> 

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

end of thread, other threads:[~2017-06-20  7:28 UTC | newest]

Thread overview: 19+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2017-06-13 13:45 [PATCH v7 00/16] mtd: nand: denali: Denali NAND IP improvements Masahiro Yamada
2017-06-13 13:45 ` [PATCH v7 01/16] mtd: nand: denali: set NAND_ECC_CUSTOM_PAGE_ACCESS Masahiro Yamada
2017-06-13 13:45 ` [PATCH v7 02/16] mtd: nand: denali: remove unneeded find_valid_banks() Masahiro Yamada
2017-06-13 13:45 ` [PATCH v7 03/16] mtd: nand: denali: handle timing parameters by setup_data_interface() Masahiro Yamada
2017-06-13 13:45 ` [PATCH v7 04/16] mtd: nand: denali: rework interrupt handling Masahiro Yamada
2017-06-13 13:45 ` [PATCH v7 05/16] mtd: nand: denali: switch over to cmd_ctrl instead of cmdfunc Masahiro Yamada
2017-06-13 13:45 ` [PATCH v7 06/16] mtd: nand: denali: fix bank reset function to detect the number of chips Masahiro Yamada
2017-06-13 13:45 ` [PATCH v7 07/16] mtd: nand: denali: use interrupt instead of polling for bank reset Masahiro Yamada
2017-06-13 13:45 ` [PATCH v7 08/16] mtd: nand: denali: propagate page to helpers via function argument Masahiro Yamada
2017-06-13 13:45 ` [PATCH v7 09/16] mtd: nand: denali: merge struct nand_buf into struct denali_nand_info Masahiro Yamada
2017-06-13 13:45 ` [PATCH v7 10/16] mtd: nand: denali: use flag instead of register macro for direction Masahiro Yamada
2017-06-13 13:45 ` [PATCH v7 11/16] mtd: nand: denali: fix raw and oob accessors for syndrome page layout Masahiro Yamada
2017-06-13 13:45 ` [PATCH v7 12/16] mtd: nand: denali: support hardware-assisted erased page detection Masahiro Yamada
2017-06-13 13:45 ` [PATCH v7 13/16] mtd: nand: denali: skip driver internal bounce buffer when possible Masahiro Yamada
2017-06-13 13:45 ` [PATCH v7 14/16] mtd: nand: denali: use non-managed kmalloc() for DMA buffer Masahiro Yamada
2017-06-13 13:45 ` [PATCH v7 15/16] mtd: nand: denali: enable bad block table scan Masahiro Yamada
2017-06-13 13:45 ` [PATCH v7 16/16] mtd: nand: denali: avoid magic numbers and rename for clarification Masahiro Yamada
2017-06-16  5:38   ` Masahiro Yamada
2017-06-20  7:28 ` [PATCH v7 00/16] mtd: nand: denali: Denali NAND IP improvements Boris Brezillon

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).