[PATCH 0/5] Improve MTK NAND driver

[PATCH 0/5] Improve MTK NAND driver

[Xiaolei Li]

The following patch set mainly contains:
* optimize ecc irq setting
* remove redundant code in nfi/ecc resume functions
* support ->setup_data_interface() hook, to optimize driver performance

Changes relative to:
--------------------

tree    : https://github.com/bbrezillon/linux-0day
branch  : nand/next
commit  :
        'commit c25030acf920 ("mtd: nand: denali: avoid magic numbers and
         rename for clarification")'

Tests:
------

* suspend/resume function check through commands:
  echo mem > /sys/power/state
  cat /sys/kernel/debug/suspend_stats
* ubifs and jffs2 are validated on NAND device MT29F16G08ADBCA
  by 'dd' command and iozone.
* all drivers/mtd/tests/* pass.
* speed test:
  eraseblock write speed is 10520 KiB/s
  eraseblock read speed is 17714 KiB/s
  page write speed is 10365 KiB/s
  page read speed is 17506 KiB/s
  2 page write speed is 10439 KiB/s
  2 page read speed is 17626 KiB/s
  erase speed is 64427 KiB/s
  2x multi-block erase speed is 324435 KiB/s
  4x multi-block erase speed is 331659 KiB/s
  8x multi-block erase speed is 332332 KiB/s
  16x multi-block erase speed is 333008 KiB/s
  32x multi-block erase speed is 334367 KiB/s
  64x multi-block erase speed is 333008 KiB/s

Xiaolei Li (5):
  mtd: nand: mtk: fix incorrect register setting order about ecc irq
  mtd: nand: mtk: disable ecc irq when writing page with hwecc
  mtd: nand: mtk: remove unneeded mtk_nfc_hw_init from mtk_nfc_resume
  mtd: nand: mtk: remove unneeded mtk_ecc_hw_init from mtk_ecc_resume
  mtd: nand: mtk: add ->setup_data_interface() hook

 drivers/mtd/nand/mtk_ecc.c  | 26 ++++++-------
 drivers/mtd/nand/mtk_nand.c | 92 ++++++++++++++++++++++++++++++++++++---------
 2 files changed, 88 insertions(+), 30 deletions(-)

--
1.9.1

[PATCH 1/5] mtd: nand: mtk: fix incorrect register setting order about ecc irq

[Xiaolei Li]

Currently, we trigger ECC HW before setting ecc irq. It is incorrect.
Because ECC starts working once the register ECC_CTL_REG is set as
ECC_OP_ENABLE. And this may lead an abnormal behavior of ecc irq.
So, should enable ecc irq at first, then trigger ECC.

Fixes: 1d6b1e464950 ("mtd: mediatek: driver for MTK Smart Device")
Signed-off-by: Xiaolei Li <xiaolei.li@mediatek.com>
---
 drivers/mtd/nand/mtk_ecc.c | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/drivers/mtd/nand/mtk_ecc.c b/drivers/mtd/nand/mtk_ecc.c
index 4958121..a855a4e 100644
--- a/drivers/mtd/nand/mtk_ecc.c
+++ b/drivers/mtd/nand/mtk_ecc.c
@@ -276,8 +276,6 @@ int mtk_ecc_enable(struct mtk_ecc *ecc, struct mtk_ecc_config *config)
 	if (ret)
 		return ret;
 
-	writew(ECC_OP_ENABLE, ecc->regs + ECC_CTL_REG(op));
-
 	init_completion(&ecc->done);
 	reg_val = ECC_IRQ_EN;
 	/*
@@ -289,6 +287,8 @@ int mtk_ecc_enable(struct mtk_ecc *ecc, struct mtk_ecc_config *config)
 		reg_val |= ECC_PG_IRQ_SEL;
 	writew(reg_val, ecc->regs + ECC_IRQ_REG(op));
 
+	writew(ECC_OP_ENABLE, ecc->regs + ECC_CTL_REG(op));
+
 	return 0;
 }
 EXPORT_SYMBOL(mtk_ecc_enable);
-- 
1.9.1

[PATCH 2/5] mtd: nand: mtk: disable ecc irq when writing page with hwecc

[Xiaolei Li]

Currently, ecc encode irq is enabled when writing page with hwecc, but
we actually do not wait for this irq done. Because NFI and ECC work in
parallel, nfi irq and ecc irq almost come together.

Now, there are two steps to check whether page data are totally written.
First, wait for nfi irq INTR_AHB_DONE. This is to ensure all data
in RAM are received by NFI.
Second, polling the register NFI_ADDRCNTR till all data include ecc
parity data runtime generated by ECC are sent to NAND device.

So, it is redunant to enable ecc irq without waiting for it.

Signed-off-by: Xiaolei Li <xiaolei.li@mediatek.com>
---
 drivers/mtd/nand/mtk_ecc.c | 22 ++++++++++++----------
 1 file changed, 12 insertions(+), 10 deletions(-)

diff --git a/drivers/mtd/nand/mtk_ecc.c b/drivers/mtd/nand/mtk_ecc.c
index a855a4e..00ce22e 100644
--- a/drivers/mtd/nand/mtk_ecc.c
+++ b/drivers/mtd/nand/mtk_ecc.c
@@ -276,16 +276,18 @@ int mtk_ecc_enable(struct mtk_ecc *ecc, struct mtk_ecc_config *config)
 	if (ret)
 		return ret;
 
-	init_completion(&ecc->done);
-	reg_val = ECC_IRQ_EN;
-	/*
-	 * For ECC_NFI_MODE, if ecc->caps->pg_irq_sel is 1, then it
-	 * means this chip can only generate one ecc irq during page
-	 * read / write. If is 0, generate one ecc irq each ecc step.
-	 */
-	if ((ecc->caps->pg_irq_sel) && (config->mode == ECC_NFI_MODE))
-		reg_val |= ECC_PG_IRQ_SEL;
-	writew(reg_val, ecc->regs + ECC_IRQ_REG(op));
+	if (config->mode != ECC_NFI_MODE || op != ECC_ENCODE) {
+		init_completion(&ecc->done);
+		reg_val = ECC_IRQ_EN;
+		/*
+		 * For ECC_NFI_MODE, if ecc->caps->pg_irq_sel is 1, then it
+		 * means this chip can only generate one ecc irq during page
+		 * read / write. If is 0, generate one ecc irq each ecc step.
+		 */
+		if (ecc->caps->pg_irq_sel && config->mode == ECC_NFI_MODE)
+			reg_val |= ECC_PG_IRQ_SEL;
+		writew(reg_val, ecc->regs + ECC_IRQ_REG(op));
+	}
 
 	writew(ECC_OP_ENABLE, ecc->regs + ECC_CTL_REG(op));
 
-- 
1.9.1

[PATCH 3/5] mtd: nand: mtk: remove unneeded mtk_nfc_hw_init from mtk_nfc_resume

[Xiaolei Li]

chip->select_chip will do nfc runtime configuration. There is no need to
do mtk_nfc_hw_init before it.

Signed-off-by: Xiaolei Li <xiaolei.li@mediatek.com>
---
 drivers/mtd/nand/mtk_nand.c | 2 --
 1 file changed, 2 deletions(-)

diff --git a/drivers/mtd/nand/mtk_nand.c b/drivers/mtd/nand/mtk_nand.c
index 4d9ee27..d51c58e 100644
--- a/drivers/mtd/nand/mtk_nand.c
+++ b/drivers/mtd/nand/mtk_nand.c
@@ -1490,8 +1490,6 @@ static int mtk_nfc_resume(struct device *dev)
 	if (ret)
 		return ret;
 
-	mtk_nfc_hw_init(nfc);
-
 	/* reset NAND chip if VCC was powered off */
 	list_for_each_entry(chip, &nfc->chips, node) {
 		nand = &chip->nand;
-- 
1.9.1

[PATCH 4/5] mtd: nand: mtk: remove unneeded mtk_ecc_hw_init from mtk_ecc_resume

[Xiaolei Li]

There is no need to add mtk_ecc_hw_init during ecc resume, because there
always takes mtk_ecc_wait_idle in the function mtk_ecc_enable.

Signed-off-by: Xiaolei Li <xiaolei.li@mediatek.com>
---
 drivers/mtd/nand/mtk_ecc.c | 2 --
 1 file changed, 2 deletions(-)

diff --git a/drivers/mtd/nand/mtk_ecc.c b/drivers/mtd/nand/mtk_ecc.c
index 00ce22e..f38e2bb 100644
--- a/drivers/mtd/nand/mtk_ecc.c
+++ b/drivers/mtd/nand/mtk_ecc.c
@@ -507,8 +507,6 @@ static int mtk_ecc_resume(struct device *dev)
 		return ret;
 	}
 
-	mtk_ecc_hw_init(ecc);
-
 	return 0;
 }
 
-- 
1.9.1

[PATCH 5/5] mtd: nand: mtk: add ->setup_data_interface() hook

[Xiaolei Li]

Currently, we use the fixed ACC timing 0x10804211. This is not the best
setting for each case. Actually, MTK NAND controller can adapt ACC timings
dynamically according to nfi clock frequence.
Implement the ->setup_data_interface() hook to optimize driver performance.

Signed-off-by: Xiaolei Li <xiaolei.li@mediatek.com>
---
 drivers/mtd/nand/mtk_nand.c | 90 +++++++++++++++++++++++++++++++++++++--------
 1 file changed, 75 insertions(+), 15 deletions(-)

diff --git a/drivers/mtd/nand/mtk_nand.c b/drivers/mtd/nand/mtk_nand.c
index d51c58e..f7ae994 100644
--- a/drivers/mtd/nand/mtk_nand.c
+++ b/drivers/mtd/nand/mtk_nand.c
@@ -100,11 +100,15 @@
 #define MTK_MAX_SECTOR		(16)
 #define MTK_NAND_MAX_NSELS	(2)
 #define MTK_NFC_MIN_SPARE	(16)
+#define ACCTIMING(tpoecs, tprecs, tc2r, tw2r, twh, twst, trlt) \
+	((tpoecs) << 28 | (tprecs) << 22 | (tc2r) << 16 | \
+	(tw2r) << 12 | (twh) << 8 | (twst) << 4 | (trlt))
 
 struct mtk_nfc_caps {
 	const u8 *spare_size;
 	u8 num_spare_size;
 	u8 pageformat_spare_shift;
+	u8 nfi_clk_div;
 };
 
 struct mtk_nfc_bad_mark_ctl {
@@ -495,6 +499,74 @@ static void mtk_nfc_write_buf(struct mtd_info *mtd, const u8 *buf, int len)
 		mtk_nfc_write_byte(mtd, buf[i]);
 }
 
+static int mtk_nfc_setup_data_interface(struct mtd_info *mtd, int csline,
+					const struct nand_data_interface *conf)
+{
+	struct mtk_nfc *nfc = nand_get_controller_data(mtd_to_nand(mtd));
+	const struct nand_sdr_timings *timings;
+	u32 rate, tpoecs, tprecs, tc2r, tw2r, twh, twst, trlt;
+
+	timings = nand_get_sdr_timings(conf);
+	if (IS_ERR(timings))
+		return -ENOTSUPP;
+
+	if (csline == NAND_DATA_IFACE_CHECK_ONLY)
+		return 0;
+
+	rate = clk_get_rate(nfc->clk.nfi_clk);
+	/* There is a frequency divider in some IPs */
+	rate /= nfc->caps->nfi_clk_div;
+
+	/* turn clock rate into KHZ */
+	rate /= 1000;
+
+	tpoecs = max(timings->tALH_min, timings->tCLH_min) / 1000;
+	tpoecs = DIV_ROUND_UP(tpoecs * rate, 1000000);
+	tpoecs &= 0xf;
+
+	tprecs = max(timings->tCLS_min, timings->tALS_min) / 1000;
+	tprecs = DIV_ROUND_UP(tprecs * rate, 1000000);
+	tprecs &= 0x3f;
+
+	/* sdr interface has no tCR which means CE# low to RE# low */
+	tc2r = 0;
+
+	tw2r = timings->tWHR_min / 1000;
+	tw2r = DIV_ROUND_UP(tw2r * rate, 1000000);
+	tw2r = DIV_ROUND_UP(tw2r - 1, 2);
+	tw2r &= 0xf;
+
+	twh = max(timings->tREH_min, timings->tWH_min) / 1000;
+	twh = DIV_ROUND_UP(twh * rate, 1000000) - 1;
+	twh &= 0xf;
+
+	twst = timings->tWP_min / 1000;
+	twst = DIV_ROUND_UP(twst * rate, 1000000) - 1;
+	twst &= 0xf;
+
+	trlt = max(timings->tREA_max, timings->tRP_min) / 1000;
+	trlt = DIV_ROUND_UP(trlt * rate, 1000000) - 1;
+	trlt &= 0xf;
+
+	/*
+	 * ACCON: access timing control register
+	 * -------------------------------------
+	 * 31:28: tpoecs, minimum required time for CS post pulling down after
+	 *        accessing the device
+	 * 27:22: tprecs, minimum required time for CS pre pulling down before
+	 *        accessing the device
+	 * 21:16: tc2r, minimum required time from NCEB low to NREB low
+	 * 15:12: tw2r, minimum required time from NWEB high to NREB low.
+	 * 11:08: twh, write enable hold time
+	 * 07:04: twst, write wait states
+	 * 03:00: trlt, read wait states
+	 */
+	trlt = ACCTIMING(tpoecs, tprecs, tc2r, tw2r, twh, twst, trlt);
+	nfi_writel(nfc, trlt, NFI_ACCCON);
+
+	return 0;
+}
+
 static int mtk_nfc_sector_encode(struct nand_chip *chip, u8 *data)
 {
 	struct mtk_nfc *nfc = nand_get_controller_data(chip);
@@ -952,21 +1024,6 @@ static int mtk_nfc_read_oob_std(struct mtd_info *mtd, struct nand_chip *chip,
 static inline void mtk_nfc_hw_init(struct mtk_nfc *nfc)
 {
 	/*
-	 * ACCON: access timing control register
-	 * -------------------------------------
-	 * 31:28: minimum required time for CS post pulling down after accessing
-	 *	the device
-	 * 27:22: minimum required time for CS pre pulling down before accessing
-	 *	the device
-	 * 21:16: minimum required time from NCEB low to NREB low
-	 * 15:12: minimum required time from NWEB high to NREB low.
-	 * 11:08: write enable hold time
-	 * 07:04: write wait states
-	 * 03:00: read wait states
-	 */
-	nfi_writel(nfc, 0x10804211, NFI_ACCCON);
-
-	/*
 	 * CNRNB: nand ready/busy register
 	 * -------------------------------
 	 * 7:4: timeout register for polling the NAND busy/ready signal
@@ -1240,6 +1297,7 @@ static int mtk_nfc_nand_chip_init(struct device *dev, struct mtk_nfc *nfc,
 	nand->read_byte = mtk_nfc_read_byte;
 	nand->read_buf = mtk_nfc_read_buf;
 	nand->cmd_ctrl = mtk_nfc_cmd_ctrl;
+	nand->setup_data_interface = mtk_nfc_setup_data_interface;
 
 	/* set default mode in case dt entry is missing */
 	nand->ecc.mode = NAND_ECC_HW;
@@ -1330,12 +1388,14 @@ static int mtk_nfc_nand_chips_init(struct device *dev, struct mtk_nfc *nfc)
 	.spare_size = spare_size_mt2701,
 	.num_spare_size = 16,
 	.pageformat_spare_shift = 4,
+	.nfi_clk_div = 1,
 };
 
 static const struct mtk_nfc_caps mtk_nfc_caps_mt2712 = {
 	.spare_size = spare_size_mt2712,
 	.num_spare_size = 19,
 	.pageformat_spare_shift = 16,
+	.nfi_clk_div = 2,
 };
 
 static const struct of_device_id mtk_nfc_id_table[] = {
-- 
1.9.1

Re: [PATCH 0/5] Improve MTK NAND driver

[Boris Brezillon]

Le Fri, 23 Jun 2017 15:12:23 +0800,
Xiaolei Li <xiaolei.li@mediatek.com> a écrit :

> The following patch set mainly contains:
> * optimize ecc irq setting
> * remove redundant code in nfi/ecc resume functions
> * support ->setup_data_interface() hook, to optimize driver performance

Applied the whole series.

Thanks,

Boris

> 
> Changes relative to:
> --------------------
> 
> tree    : https://github.com/bbrezillon/linux-0day
> branch  : nand/next
> commit  :
>         'commit c25030acf920 ("mtd: nand: denali: avoid magic numbers and
>          rename for clarification")'
> 
> Tests:
> ------
> 
> * suspend/resume function check through commands:
>   echo mem > /sys/power/state
>   cat /sys/kernel/debug/suspend_stats
> * ubifs and jffs2 are validated on NAND device MT29F16G08ADBCA
>   by 'dd' command and iozone.
> * all drivers/mtd/tests/* pass.
> * speed test:
>   eraseblock write speed is 10520 KiB/s
>   eraseblock read speed is 17714 KiB/s
>   page write speed is 10365 KiB/s
>   page read speed is 17506 KiB/s
>   2 page write speed is 10439 KiB/s
>   2 page read speed is 17626 KiB/s
>   erase speed is 64427 KiB/s
>   2x multi-block erase speed is 324435 KiB/s
>   4x multi-block erase speed is 331659 KiB/s
>   8x multi-block erase speed is 332332 KiB/s
>   16x multi-block erase speed is 333008 KiB/s
>   32x multi-block erase speed is 334367 KiB/s
>   64x multi-block erase speed is 333008 KiB/s
> 
> Xiaolei Li (5):
>   mtd: nand: mtk: fix incorrect register setting order about ecc irq
>   mtd: nand: mtk: disable ecc irq when writing page with hwecc
>   mtd: nand: mtk: remove unneeded mtk_nfc_hw_init from mtk_nfc_resume
>   mtd: nand: mtk: remove unneeded mtk_ecc_hw_init from mtk_ecc_resume
>   mtd: nand: mtk: add ->setup_data_interface() hook
> 
>  drivers/mtd/nand/mtk_ecc.c  | 26 ++++++-------
>  drivers/mtd/nand/mtk_nand.c | 92 ++++++++++++++++++++++++++++++++++++---------
>  2 files changed, 88 insertions(+), 30 deletions(-)
> 
> --
> 1.9.1