All of lore.kernel.org
 help / color / mirror / Atom feed
* [PATCH v2 0/5] MTK NAND driver improvements and fixes
@ 2019-04-30 10:02 ` Xiaolei Li
  0 siblings, 0 replies; 33+ messages in thread
From: Xiaolei Li @ 2019-04-30 10:02 UTC (permalink / raw)
  To: miquel.raynal, richard
  Cc: linux-mediatek, xiaolei.li, linux-mtd, srv_heupstream

The following patch set mainly contains:
* Fix low level time calculation of read/write cycle to meet tRC_min
  and tWC_min requirements.
* Refine RE# pulse width calculation and data sampling to improve read
  performance.
* Add CS validity check.
* Fix oob buffer pointer wrongly setting and empty page threshold setting.

Changes on v2 relative to:
--------------------

tree    : git://git.infradead.org/linux-mtd.git
branch  : master
commit  :
        'commit 3e35730dd754 ("mtd: powernv_flash: Fix device
         registration error")

Patch v2:
---------
- Fix type
- Reference correct faulty patch
- Refine code to do calculation and condition in separate steps
- Fix empty threshold calculation

Tests:
------

* ubifs and jffs2 are validated on NAND device MT29F16G08ADBCA by
  'dd' command.
* all drivers/mtd/tests/* pass.
* speed test:
  eraseblock write speed is 11087 KiB/s
  eraseblock read speed is 19986 KiB/s
  page write speed is 10689 KiB/s
  page read speed is 18724 KiB/s
  2 page write speed is 10611 KiB/s
  2 page read speed is 18713 KiB/s
  erase speed is 103696 KiB/s
  2x multi-block erase speed is 354248 KiB/s
  4x multi-block erase speed is 350459 KiB/s
  8x multi-block erase speed is 356173 KiB/s
  16x multi-block erase speed is 356173 KiB/s
  32x multi-block erase speed is 358120 KiB/s
  64x multi-block erase speed is 356173 KiB/s

Xiaolei Li (5):
  mtd: rawnand: mtk: Correct low level time calculation of r/w cycle
  mtd: rawnand: mtk: Improve data sampling timing for read cycle
  mtd: rawnand: mtk: Add validity check for CE# pin setting
  mtd: rawnand: mtk: Fix wrongly assigned OOB buffer pointer issue
  mtd: rawnand: mtk: Setup empty page threshold correctly

 drivers/mtd/nand/raw/mtk_nand.c | 93 ++++++++++++++++++++++++++++-----
 1 file changed, 79 insertions(+), 14 deletions(-)

-- 
2.18.0



______________________________________________________
Linux MTD discussion mailing list
http://lists.infradead.org/mailman/listinfo/linux-mtd/

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

* [PATCH v2 0/5] MTK NAND driver improvements and fixes
@ 2019-04-30 10:02 ` Xiaolei Li
  0 siblings, 0 replies; 33+ messages in thread
From: Xiaolei Li @ 2019-04-30 10:02 UTC (permalink / raw)
  To: miquel.raynal, richard
  Cc: linux-mediatek, xiaolei.li, linux-mtd, srv_heupstream

The following patch set mainly contains:
* Fix low level time calculation of read/write cycle to meet tRC_min
  and tWC_min requirements.
* Refine RE# pulse width calculation and data sampling to improve read
  performance.
* Add CS validity check.
* Fix oob buffer pointer wrongly setting and empty page threshold setting.

Changes on v2 relative to:
--------------------

tree    : git://git.infradead.org/linux-mtd.git
branch  : master
commit  :
        'commit 3e35730dd754 ("mtd: powernv_flash: Fix device
         registration error")

Patch v2:
---------
- Fix type
- Reference correct faulty patch
- Refine code to do calculation and condition in separate steps
- Fix empty threshold calculation

Tests:
------

* ubifs and jffs2 are validated on NAND device MT29F16G08ADBCA by
  'dd' command.
* all drivers/mtd/tests/* pass.
* speed test:
  eraseblock write speed is 11087 KiB/s
  eraseblock read speed is 19986 KiB/s
  page write speed is 10689 KiB/s
  page read speed is 18724 KiB/s
  2 page write speed is 10611 KiB/s
  2 page read speed is 18713 KiB/s
  erase speed is 103696 KiB/s
  2x multi-block erase speed is 354248 KiB/s
  4x multi-block erase speed is 350459 KiB/s
  8x multi-block erase speed is 356173 KiB/s
  16x multi-block erase speed is 356173 KiB/s
  32x multi-block erase speed is 358120 KiB/s
  64x multi-block erase speed is 356173 KiB/s

Xiaolei Li (5):
  mtd: rawnand: mtk: Correct low level time calculation of r/w cycle
  mtd: rawnand: mtk: Improve data sampling timing for read cycle
  mtd: rawnand: mtk: Add validity check for CE# pin setting
  mtd: rawnand: mtk: Fix wrongly assigned OOB buffer pointer issue
  mtd: rawnand: mtk: Setup empty page threshold correctly

 drivers/mtd/nand/raw/mtk_nand.c | 93 ++++++++++++++++++++++++++++-----
 1 file changed, 79 insertions(+), 14 deletions(-)

-- 
2.18.0



______________________________________________________
Linux MTD discussion mailing list
http://lists.infradead.org/mailman/listinfo/linux-mtd/

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

* [PATCH v2 1/5] mtd: rawnand: mtk: Correct low level time calculation of r/w cycle
  2019-04-30 10:02 ` Xiaolei Li
  (?)
@ 2019-04-30 10:02   ` Xiaolei Li
  -1 siblings, 0 replies; 33+ messages in thread
From: Xiaolei Li @ 2019-04-30 10:02 UTC (permalink / raw)
  To: miquel.raynal, richard
  Cc: linux-mtd, linux-mediatek, srv_heupstream, xiaolei.li, stable

At present, the flow of calculating AC timing of read/write cycle in SDR
mode is that:
At first, calculate high hold time which is valid for both read and write
cycle using the max value between tREH_min and tWH_min.
Secondly, calculate WE# pulse width using tWP_min.
Thridly, calculate RE# pulse width using the bigger one between tREA_max
and tRP_min.

But NAND SPEC shows that Controller should also meet write/read cycle time.
That is write cycle time should be more than tWC_min and read cycle should
be more than tRC_min. Obviously, we do not achieve that now.

This patch corrects the low level time calculation to meet minimum
read/write cycle time required. After getting the high hold time, WE# low
level time will be promised to meet tWP_min and tWC_min requirement,
and RE# low level time will be promised to meet tREA_max, tRP_min and
tRC_min requirement.

Fixes: edfee3619c49 ("mtd: nand: mtk: add ->setup_data_interface() hook")
Cc: stable@vger.kernel.org
Signed-off-by: Xiaolei Li <xiaolei.li@mediatek.com>
---
 drivers/mtd/nand/raw/mtk_nand.c | 24 +++++++++++++++++++++---
 1 file changed, 21 insertions(+), 3 deletions(-)

diff --git a/drivers/mtd/nand/raw/mtk_nand.c b/drivers/mtd/nand/raw/mtk_nand.c
index b6b4602f5132..4fbb0c6ecae3 100644
--- a/drivers/mtd/nand/raw/mtk_nand.c
+++ b/drivers/mtd/nand/raw/mtk_nand.c
@@ -508,7 +508,8 @@ static int mtk_nfc_setup_data_interface(struct nand_chip *chip, int csline,
 {
 	struct mtk_nfc *nfc = nand_get_controller_data(chip);
 	const struct nand_sdr_timings *timings;
-	u32 rate, tpoecs, tprecs, tc2r, tw2r, twh, twst, trlt;
+	u32 rate, tpoecs, tprecs, tc2r, tw2r, twh, twst = 0, trlt = 0;
+	u32 thold;
 
 	timings = nand_get_sdr_timings(conf);
 	if (IS_ERR(timings))
@@ -544,11 +545,28 @@ static int mtk_nfc_setup_data_interface(struct nand_chip *chip, int csline,
 	twh = DIV_ROUND_UP(twh * rate, 1000000) - 1;
 	twh &= 0xf;
 
-	twst = timings->tWP_min / 1000;
+	/* Calculate real WE#/RE# hold time in nanosecond */
+	thold = (twh + 1) * 1000000 / rate;
+	/* nanosecond to picosecond */
+	thold *= 1000;
+
+	/**
+	 * WE# low level time should be expaned to meet WE# pulse time
+	 * and WE# cycle time at the same time.
+	 */
+	if (thold < timings->tWC_min)
+		twst = timings->tWC_min - thold;
+	twst = max(timings->tWP_min, twst) / 1000;
 	twst = DIV_ROUND_UP(twst * rate, 1000000) - 1;
 	twst &= 0xf;
 
-	trlt = max(timings->tREA_max, timings->tRP_min) / 1000;
+	/**
+	 * RE# low level time should be expaned to meet RE# pulse time,
+	 * RE# access time and RE# cycle time at the same time.
+	 */
+	if (thold < timings->tRC_min)
+		trlt = timings->tRC_min - thold;
+	trlt = max3(trlt, timings->tREA_max, timings->tRP_min) / 1000;
 	trlt = DIV_ROUND_UP(trlt * rate, 1000000) - 1;
 	trlt &= 0xf;
 
-- 
2.18.0


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

* [PATCH v2 1/5] mtd: rawnand: mtk: Correct low level time calculation of r/w cycle
@ 2019-04-30 10:02   ` Xiaolei Li
  0 siblings, 0 replies; 33+ messages in thread
From: Xiaolei Li @ 2019-04-30 10:02 UTC (permalink / raw)
  To: miquel.raynal, richard
  Cc: linux-mediatek, xiaolei.li, linux-mtd, stable, srv_heupstream

At present, the flow of calculating AC timing of read/write cycle in SDR
mode is that:
At first, calculate high hold time which is valid for both read and write
cycle using the max value between tREH_min and tWH_min.
Secondly, calculate WE# pulse width using tWP_min.
Thridly, calculate RE# pulse width using the bigger one between tREA_max
and tRP_min.

But NAND SPEC shows that Controller should also meet write/read cycle time.
That is write cycle time should be more than tWC_min and read cycle should
be more than tRC_min. Obviously, we do not achieve that now.

This patch corrects the low level time calculation to meet minimum
read/write cycle time required. After getting the high hold time, WE# low
level time will be promised to meet tWP_min and tWC_min requirement,
and RE# low level time will be promised to meet tREA_max, tRP_min and
tRC_min requirement.

Fixes: edfee3619c49 ("mtd: nand: mtk: add ->setup_data_interface() hook")
Cc: stable@vger.kernel.org
Signed-off-by: Xiaolei Li <xiaolei.li@mediatek.com>
---
 drivers/mtd/nand/raw/mtk_nand.c | 24 +++++++++++++++++++++---
 1 file changed, 21 insertions(+), 3 deletions(-)

diff --git a/drivers/mtd/nand/raw/mtk_nand.c b/drivers/mtd/nand/raw/mtk_nand.c
index b6b4602f5132..4fbb0c6ecae3 100644
--- a/drivers/mtd/nand/raw/mtk_nand.c
+++ b/drivers/mtd/nand/raw/mtk_nand.c
@@ -508,7 +508,8 @@ static int mtk_nfc_setup_data_interface(struct nand_chip *chip, int csline,
 {
 	struct mtk_nfc *nfc = nand_get_controller_data(chip);
 	const struct nand_sdr_timings *timings;
-	u32 rate, tpoecs, tprecs, tc2r, tw2r, twh, twst, trlt;
+	u32 rate, tpoecs, tprecs, tc2r, tw2r, twh, twst = 0, trlt = 0;
+	u32 thold;
 
 	timings = nand_get_sdr_timings(conf);
 	if (IS_ERR(timings))
@@ -544,11 +545,28 @@ static int mtk_nfc_setup_data_interface(struct nand_chip *chip, int csline,
 	twh = DIV_ROUND_UP(twh * rate, 1000000) - 1;
 	twh &= 0xf;
 
-	twst = timings->tWP_min / 1000;
+	/* Calculate real WE#/RE# hold time in nanosecond */
+	thold = (twh + 1) * 1000000 / rate;
+	/* nanosecond to picosecond */
+	thold *= 1000;
+
+	/**
+	 * WE# low level time should be expaned to meet WE# pulse time
+	 * and WE# cycle time at the same time.
+	 */
+	if (thold < timings->tWC_min)
+		twst = timings->tWC_min - thold;
+	twst = max(timings->tWP_min, twst) / 1000;
 	twst = DIV_ROUND_UP(twst * rate, 1000000) - 1;
 	twst &= 0xf;
 
-	trlt = max(timings->tREA_max, timings->tRP_min) / 1000;
+	/**
+	 * RE# low level time should be expaned to meet RE# pulse time,
+	 * RE# access time and RE# cycle time at the same time.
+	 */
+	if (thold < timings->tRC_min)
+		trlt = timings->tRC_min - thold;
+	trlt = max3(trlt, timings->tREA_max, timings->tRP_min) / 1000;
 	trlt = DIV_ROUND_UP(trlt * rate, 1000000) - 1;
 	trlt &= 0xf;
 
-- 
2.18.0


______________________________________________________
Linux MTD discussion mailing list
http://lists.infradead.org/mailman/listinfo/linux-mtd/

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

* [PATCH v2 1/5] mtd: rawnand: mtk: Correct low level time calculation of r/w cycle
@ 2019-04-30 10:02   ` Xiaolei Li
  0 siblings, 0 replies; 33+ messages in thread
From: Xiaolei Li @ 2019-04-30 10:02 UTC (permalink / raw)
  To: miquel.raynal, richard
  Cc: linux-mtd, linux-mediatek, srv_heupstream, xiaolei.li, stable

At present, the flow of calculating AC timing of read/write cycle in SDR
mode is that:
At first, calculate high hold time which is valid for both read and write
cycle using the max value between tREH_min and tWH_min.
Secondly, calculate WE# pulse width using tWP_min.
Thridly, calculate RE# pulse width using the bigger one between tREA_max
and tRP_min.

But NAND SPEC shows that Controller should also meet write/read cycle time.
That is write cycle time should be more than tWC_min and read cycle should
be more than tRC_min. Obviously, we do not achieve that now.

This patch corrects the low level time calculation to meet minimum
read/write cycle time required. After getting the high hold time, WE# low
level time will be promised to meet tWP_min and tWC_min requirement,
and RE# low level time will be promised to meet tREA_max, tRP_min and
tRC_min requirement.

Fixes: edfee3619c49 ("mtd: nand: mtk: add ->setup_data_interface() hook")
Cc: stable@vger.kernel.org
Signed-off-by: Xiaolei Li <xiaolei.li@mediatek.com>
---
 drivers/mtd/nand/raw/mtk_nand.c | 24 +++++++++++++++++++++---
 1 file changed, 21 insertions(+), 3 deletions(-)

diff --git a/drivers/mtd/nand/raw/mtk_nand.c b/drivers/mtd/nand/raw/mtk_nand.c
index b6b4602f5132..4fbb0c6ecae3 100644
--- a/drivers/mtd/nand/raw/mtk_nand.c
+++ b/drivers/mtd/nand/raw/mtk_nand.c
@@ -508,7 +508,8 @@ static int mtk_nfc_setup_data_interface(struct nand_chip *chip, int csline,
 {
 	struct mtk_nfc *nfc = nand_get_controller_data(chip);
 	const struct nand_sdr_timings *timings;
-	u32 rate, tpoecs, tprecs, tc2r, tw2r, twh, twst, trlt;
+	u32 rate, tpoecs, tprecs, tc2r, tw2r, twh, twst = 0, trlt = 0;
+	u32 thold;
 
 	timings = nand_get_sdr_timings(conf);
 	if (IS_ERR(timings))
@@ -544,11 +545,28 @@ static int mtk_nfc_setup_data_interface(struct nand_chip *chip, int csline,
 	twh = DIV_ROUND_UP(twh * rate, 1000000) - 1;
 	twh &= 0xf;
 
-	twst = timings->tWP_min / 1000;
+	/* Calculate real WE#/RE# hold time in nanosecond */
+	thold = (twh + 1) * 1000000 / rate;
+	/* nanosecond to picosecond */
+	thold *= 1000;
+
+	/**
+	 * WE# low level time should be expaned to meet WE# pulse time
+	 * and WE# cycle time at the same time.
+	 */
+	if (thold < timings->tWC_min)
+		twst = timings->tWC_min - thold;
+	twst = max(timings->tWP_min, twst) / 1000;
 	twst = DIV_ROUND_UP(twst * rate, 1000000) - 1;
 	twst &= 0xf;
 
-	trlt = max(timings->tREA_max, timings->tRP_min) / 1000;
+	/**
+	 * RE# low level time should be expaned to meet RE# pulse time,
+	 * RE# access time and RE# cycle time at the same time.
+	 */
+	if (thold < timings->tRC_min)
+		trlt = timings->tRC_min - thold;
+	trlt = max3(trlt, timings->tREA_max, timings->tRP_min) / 1000;
 	trlt = DIV_ROUND_UP(trlt * rate, 1000000) - 1;
 	trlt &= 0xf;
 
-- 
2.18.0

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

* [PATCH v2 2/5] mtd: rawnand: mtk: Improve data sampling timing for read cycle
  2019-04-30 10:02 ` Xiaolei Li
@ 2019-04-30 10:02   ` Xiaolei Li
  -1 siblings, 0 replies; 33+ messages in thread
From: Xiaolei Li @ 2019-04-30 10:02 UTC (permalink / raw)
  To: miquel.raynal, richard
  Cc: linux-mediatek, xiaolei.li, linux-mtd, srv_heupstream

Currently, we expand RE# low level time by choosing the max value
between RE# pulse width and RE# access time, and sample data at the
rising edge of RE#.

Then, if RE# access time is bigger than RE# pulse width, the real
read cycle time may be more than NAND SPEC required. This makes
read performance be worse than that expected.

This patch improves data sampling timing by calculating RE# low level
time according to RE# pulse width. If RE# access time is bigger than
RE# pulse width, then delay sampling data timing.

The result of contrast test base on MT2712 evaluat board is as follow.

nand: Micron MT29F16G08ADBCAH4
nand: 2048 MiB, SLC, erase size: 256 KiB, page size: 4096, OOB size: 224
NFI 2x clock rate: 124800000 HZ.

Read speed without this patch:
mtd_speedtest: page read speed is 14012 KiB/s
mtd_speedtest: 2 page read speed is 14860 KiB/s

Read speed with this patch:
mtd_speedtest: page read speed is 18724 KiB/s
mtd_speedtest: 2 page read speed is 18713 KiB/s

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

diff --git a/drivers/mtd/nand/raw/mtk_nand.c b/drivers/mtd/nand/raw/mtk_nand.c
index 4fbb0c6ecae3..e90c38c6f835 100644
--- a/drivers/mtd/nand/raw/mtk_nand.c
+++ b/drivers/mtd/nand/raw/mtk_nand.c
@@ -87,6 +87,10 @@
 #define NFI_FDMM(x)		(0xA4 + (x) * sizeof(u32) * 2)
 #define NFI_FDM_MAX_SIZE	(8)
 #define NFI_FDM_MIN_SIZE	(1)
+#define NFI_DEBUG_CON1		(0x220)
+#define		STROBE_MASK		GENMASK(4, 3)
+#define		STROBE_SHIFT		(3)
+#define		MAX_STROBE_DLY		(3)
 #define NFI_MASTER_STA		(0x224)
 #define		MASTER_STA_MASK		(0x0FFF)
 #define NFI_EMPTY_THRESH	(0x23C)
@@ -509,7 +513,7 @@ static int mtk_nfc_setup_data_interface(struct nand_chip *chip, int csline,
 	struct mtk_nfc *nfc = nand_get_controller_data(chip);
 	const struct nand_sdr_timings *timings;
 	u32 rate, tpoecs, tprecs, tc2r, tw2r, twh, twst = 0, trlt = 0;
-	u32 thold;
+	u32 temp, tsel = 0;
 
 	timings = nand_get_sdr_timings(conf);
 	if (IS_ERR(timings))
@@ -546,30 +550,52 @@ static int mtk_nfc_setup_data_interface(struct nand_chip *chip, int csline,
 	twh &= 0xf;
 
 	/* Calculate real WE#/RE# hold time in nanosecond */
-	thold = (twh + 1) * 1000000 / rate;
+	temp = (twh + 1) * 1000000 / rate;
 	/* nanosecond to picosecond */
-	thold *= 1000;
+	temp *= 1000;
 
 	/**
 	 * WE# low level time should be expaned to meet WE# pulse time
 	 * and WE# cycle time at the same time.
 	 */
-	if (thold < timings->tWC_min)
-		twst = timings->tWC_min - thold;
+	if (temp < timings->tWC_min)
+		twst = timings->tWC_min - temp;
 	twst = max(timings->tWP_min, twst) / 1000;
 	twst = DIV_ROUND_UP(twst * rate, 1000000) - 1;
 	twst &= 0xf;
 
 	/**
-	 * RE# low level time should be expaned to meet RE# pulse time,
-	 * RE# access time and RE# cycle time at the same time.
+	 * RE# low level time should be expaned to meet RE# pulse time
+	 * and RE# cycle time at the same time.
 	 */
-	if (thold < timings->tRC_min)
-		trlt = timings->tRC_min - thold;
-	trlt = max3(trlt, timings->tREA_max, timings->tRP_min) / 1000;
+	if (temp < timings->tRC_min)
+		trlt = timings->tRC_min - temp;
+	trlt = max(trlt, timings->tRP_min) / 1000;
 	trlt = DIV_ROUND_UP(trlt * rate, 1000000) - 1;
 	trlt &= 0xf;
 
+	/**
+	 * Calculate strobe select timing.
+	 * If RE# access time is bigger than RE# pulse time,
+	 * delay sampling data timing.
+	 */
+	temp = (trlt + 1) * 1000000 / rate;
+	/* nanosecond to picosecond */
+	temp *= 1000;
+	if (temp < timings->tREA_max) {
+		tsel = timings->tREA_max / 1000;
+		tsel = DIV_ROUND_UP(tsel * rate, 1000000);
+		tsel -= (trlt + 1);
+		if (tsel > MAX_STROBE_DLY) {
+			trlt += tsel - MAX_STROBE_DLY;
+			tsel = MAX_STROBE_DLY;
+		}
+	}
+	temp = nfi_readl(nfc, NFI_DEBUG_CON1);
+	temp &= ~STROBE_MASK;
+	temp |= tsel << STROBE_SHIFT;
+	nfi_writel(nfc, temp, NFI_DEBUG_CON1);
+
 	/*
 	 * ACCON: access timing control register
 	 * -------------------------------------
-- 
2.18.0


______________________________________________________
Linux MTD discussion mailing list
http://lists.infradead.org/mailman/listinfo/linux-mtd/

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

* [PATCH v2 2/5] mtd: rawnand: mtk: Improve data sampling timing for read cycle
@ 2019-04-30 10:02   ` Xiaolei Li
  0 siblings, 0 replies; 33+ messages in thread
From: Xiaolei Li @ 2019-04-30 10:02 UTC (permalink / raw)
  To: miquel.raynal, richard
  Cc: linux-mediatek, xiaolei.li, linux-mtd, srv_heupstream

Currently, we expand RE# low level time by choosing the max value
between RE# pulse width and RE# access time, and sample data at the
rising edge of RE#.

Then, if RE# access time is bigger than RE# pulse width, the real
read cycle time may be more than NAND SPEC required. This makes
read performance be worse than that expected.

This patch improves data sampling timing by calculating RE# low level
time according to RE# pulse width. If RE# access time is bigger than
RE# pulse width, then delay sampling data timing.

The result of contrast test base on MT2712 evaluat board is as follow.

nand: Micron MT29F16G08ADBCAH4
nand: 2048 MiB, SLC, erase size: 256 KiB, page size: 4096, OOB size: 224
NFI 2x clock rate: 124800000 HZ.

Read speed without this patch:
mtd_speedtest: page read speed is 14012 KiB/s
mtd_speedtest: 2 page read speed is 14860 KiB/s

Read speed with this patch:
mtd_speedtest: page read speed is 18724 KiB/s
mtd_speedtest: 2 page read speed is 18713 KiB/s

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

diff --git a/drivers/mtd/nand/raw/mtk_nand.c b/drivers/mtd/nand/raw/mtk_nand.c
index 4fbb0c6ecae3..e90c38c6f835 100644
--- a/drivers/mtd/nand/raw/mtk_nand.c
+++ b/drivers/mtd/nand/raw/mtk_nand.c
@@ -87,6 +87,10 @@
 #define NFI_FDMM(x)		(0xA4 + (x) * sizeof(u32) * 2)
 #define NFI_FDM_MAX_SIZE	(8)
 #define NFI_FDM_MIN_SIZE	(1)
+#define NFI_DEBUG_CON1		(0x220)
+#define		STROBE_MASK		GENMASK(4, 3)
+#define		STROBE_SHIFT		(3)
+#define		MAX_STROBE_DLY		(3)
 #define NFI_MASTER_STA		(0x224)
 #define		MASTER_STA_MASK		(0x0FFF)
 #define NFI_EMPTY_THRESH	(0x23C)
@@ -509,7 +513,7 @@ static int mtk_nfc_setup_data_interface(struct nand_chip *chip, int csline,
 	struct mtk_nfc *nfc = nand_get_controller_data(chip);
 	const struct nand_sdr_timings *timings;
 	u32 rate, tpoecs, tprecs, tc2r, tw2r, twh, twst = 0, trlt = 0;
-	u32 thold;
+	u32 temp, tsel = 0;
 
 	timings = nand_get_sdr_timings(conf);
 	if (IS_ERR(timings))
@@ -546,30 +550,52 @@ static int mtk_nfc_setup_data_interface(struct nand_chip *chip, int csline,
 	twh &= 0xf;
 
 	/* Calculate real WE#/RE# hold time in nanosecond */
-	thold = (twh + 1) * 1000000 / rate;
+	temp = (twh + 1) * 1000000 / rate;
 	/* nanosecond to picosecond */
-	thold *= 1000;
+	temp *= 1000;
 
 	/**
 	 * WE# low level time should be expaned to meet WE# pulse time
 	 * and WE# cycle time at the same time.
 	 */
-	if (thold < timings->tWC_min)
-		twst = timings->tWC_min - thold;
+	if (temp < timings->tWC_min)
+		twst = timings->tWC_min - temp;
 	twst = max(timings->tWP_min, twst) / 1000;
 	twst = DIV_ROUND_UP(twst * rate, 1000000) - 1;
 	twst &= 0xf;
 
 	/**
-	 * RE# low level time should be expaned to meet RE# pulse time,
-	 * RE# access time and RE# cycle time at the same time.
+	 * RE# low level time should be expaned to meet RE# pulse time
+	 * and RE# cycle time at the same time.
 	 */
-	if (thold < timings->tRC_min)
-		trlt = timings->tRC_min - thold;
-	trlt = max3(trlt, timings->tREA_max, timings->tRP_min) / 1000;
+	if (temp < timings->tRC_min)
+		trlt = timings->tRC_min - temp;
+	trlt = max(trlt, timings->tRP_min) / 1000;
 	trlt = DIV_ROUND_UP(trlt * rate, 1000000) - 1;
 	trlt &= 0xf;
 
+	/**
+	 * Calculate strobe select timing.
+	 * If RE# access time is bigger than RE# pulse time,
+	 * delay sampling data timing.
+	 */
+	temp = (trlt + 1) * 1000000 / rate;
+	/* nanosecond to picosecond */
+	temp *= 1000;
+	if (temp < timings->tREA_max) {
+		tsel = timings->tREA_max / 1000;
+		tsel = DIV_ROUND_UP(tsel * rate, 1000000);
+		tsel -= (trlt + 1);
+		if (tsel > MAX_STROBE_DLY) {
+			trlt += tsel - MAX_STROBE_DLY;
+			tsel = MAX_STROBE_DLY;
+		}
+	}
+	temp = nfi_readl(nfc, NFI_DEBUG_CON1);
+	temp &= ~STROBE_MASK;
+	temp |= tsel << STROBE_SHIFT;
+	nfi_writel(nfc, temp, NFI_DEBUG_CON1);
+
 	/*
 	 * ACCON: access timing control register
 	 * -------------------------------------
-- 
2.18.0


______________________________________________________
Linux MTD discussion mailing list
http://lists.infradead.org/mailman/listinfo/linux-mtd/

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

* [PATCH v2 3/5] mtd: rawnand: mtk: Add validity check for CE# pin setting
@ 2019-04-30 10:02   ` Xiaolei Li
  0 siblings, 0 replies; 33+ messages in thread
From: Xiaolei Li @ 2019-04-30 10:02 UTC (permalink / raw)
  To: miquel.raynal, richard
  Cc: linux-mediatek, xiaolei.li, linux-mtd, srv_heupstream

Currently, we only check how many CE# pins are set in device tree.
But it should be necessary to check whether CE# pin setting is
duplicated or if CE# pin index exceeds the maximum CE# number that
controller supports.

So, add validity check to avoid these invalid settings.

Signed-off-by: Xiaolei Li <xiaolei.li@mediatek.com>
Reviewed-by: Miquel Raynal <miquel.raynal@bootlin.com>
---
 drivers/mtd/nand/raw/mtk_nand.c | 13 +++++++++++++
 1 file changed, 13 insertions(+)

diff --git a/drivers/mtd/nand/raw/mtk_nand.c b/drivers/mtd/nand/raw/mtk_nand.c
index e90c38c6f835..23ebd668bbcd 100644
--- a/drivers/mtd/nand/raw/mtk_nand.c
+++ b/drivers/mtd/nand/raw/mtk_nand.c
@@ -162,6 +162,8 @@ struct mtk_nfc {
 	struct list_head chips;
 
 	u8 *buffer;
+
+	unsigned long assigned_cs;
 };
 
 /*
@@ -1367,6 +1369,17 @@ static int mtk_nfc_nand_chip_init(struct device *dev, struct mtk_nfc *nfc,
 			dev_err(dev, "reg property failure : %d\n", ret);
 			return ret;
 		}
+
+		if (tmp >= MTK_NAND_MAX_NSELS) {
+			dev_err(dev, "invalid CS: %u\n", tmp);
+			return -EINVAL;
+		}
+
+		if (test_and_set_bit(tmp, &nfc->assigned_cs)) {
+			dev_err(dev, "CS %u already assigned\n", tmp);
+			return -EINVAL;
+		}
+
 		chip->sels[i] = tmp;
 	}
 
-- 
2.18.0


______________________________________________________
Linux MTD discussion mailing list
http://lists.infradead.org/mailman/listinfo/linux-mtd/

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

* [PATCH v2 3/5] mtd: rawnand: mtk: Add validity check for CE# pin setting
@ 2019-04-30 10:02   ` Xiaolei Li
  0 siblings, 0 replies; 33+ messages in thread
From: Xiaolei Li @ 2019-04-30 10:02 UTC (permalink / raw)
  To: miquel.raynal-LDxbnhwyfcJBDgjK7y7TUQ, richard-/L3Ra7n9ekc
  Cc: linux-mediatek-IAPFreCvJWM7uuMidbF8XUB+6BGkLq7r,
	xiaolei.li-NuS5LvNUpcJWk0Htik3J/w,
	linux-mtd-IAPFreCvJWM7uuMidbF8XUB+6BGkLq7r,
	srv_heupstream-NuS5LvNUpcJWk0Htik3J/w

Currently, we only check how many CE# pins are set in device tree.
But it should be necessary to check whether CE# pin setting is
duplicated or if CE# pin index exceeds the maximum CE# number that
controller supports.

So, add validity check to avoid these invalid settings.

Signed-off-by: Xiaolei Li <xiaolei.li-NuS5LvNUpcJWk0Htik3J/w@public.gmane.org>
Reviewed-by: Miquel Raynal <miquel.raynal-LDxbnhwyfcJBDgjK7y7TUQ@public.gmane.org>
---
 drivers/mtd/nand/raw/mtk_nand.c | 13 +++++++++++++
 1 file changed, 13 insertions(+)

diff --git a/drivers/mtd/nand/raw/mtk_nand.c b/drivers/mtd/nand/raw/mtk_nand.c
index e90c38c6f835..23ebd668bbcd 100644
--- a/drivers/mtd/nand/raw/mtk_nand.c
+++ b/drivers/mtd/nand/raw/mtk_nand.c
@@ -162,6 +162,8 @@ struct mtk_nfc {
 	struct list_head chips;
 
 	u8 *buffer;
+
+	unsigned long assigned_cs;
 };
 
 /*
@@ -1367,6 +1369,17 @@ static int mtk_nfc_nand_chip_init(struct device *dev, struct mtk_nfc *nfc,
 			dev_err(dev, "reg property failure : %d\n", ret);
 			return ret;
 		}
+
+		if (tmp >= MTK_NAND_MAX_NSELS) {
+			dev_err(dev, "invalid CS: %u\n", tmp);
+			return -EINVAL;
+		}
+
+		if (test_and_set_bit(tmp, &nfc->assigned_cs)) {
+			dev_err(dev, "CS %u already assigned\n", tmp);
+			return -EINVAL;
+		}
+
 		chip->sels[i] = tmp;
 	}
 
-- 
2.18.0

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

* [PATCH v2 4/5] mtd: rawnand: mtk: Fix wrongly assigned OOB buffer pointer issue
  2019-04-30 10:02 ` Xiaolei Li
@ 2019-04-30 10:02   ` Xiaolei Li
  -1 siblings, 0 replies; 33+ messages in thread
From: Xiaolei Li @ 2019-04-30 10:02 UTC (permalink / raw)
  To: miquel.raynal, richard
  Cc: linux-mediatek, xiaolei.li, linux-mtd, srv_heupstream

One main goal of the function mtk_nfc_update_ecc_stats is to check
whether sectors are all empty. If they are empty, set these sectors's
data buffer and OOB buffer as 0xff.

But now, the sector OOB buffer pointer is wrongly assigned. We always
do memset from sector 0.

To fix this issue, pass start sector number to make OOB buffer pointer
be properly assigned.

Fixes: 1d6b1e464950 ("mtd: mediatek: driver for MTK Smart Device")
Signed-off-by: Xiaolei Li <xiaolei.li@mediatek.com>
Reviewed-by: Miquel Raynal <miquel.raynal@bootlin.com>
---
 drivers/mtd/nand/raw/mtk_nand.c | 21 ++++++++++-----------
 1 file changed, 10 insertions(+), 11 deletions(-)

diff --git a/drivers/mtd/nand/raw/mtk_nand.c b/drivers/mtd/nand/raw/mtk_nand.c
index 23ebd668bbcd..48759af5c058 100644
--- a/drivers/mtd/nand/raw/mtk_nand.c
+++ b/drivers/mtd/nand/raw/mtk_nand.c
@@ -889,19 +889,21 @@ static int mtk_nfc_write_oob_std(struct nand_chip *chip, int page)
 	return mtk_nfc_write_page_raw(chip, NULL, 1, page);
 }
 
-static int mtk_nfc_update_ecc_stats(struct mtd_info *mtd, u8 *buf, u32 sectors)
+static int mtk_nfc_update_ecc_stats(struct mtd_info *mtd, u8 *buf, u32 start,
+				    u32 sectors)
 {
 	struct nand_chip *chip = mtd_to_nand(mtd);
 	struct mtk_nfc *nfc = nand_get_controller_data(chip);
 	struct mtk_nfc_nand_chip *mtk_nand = to_mtk_nand(chip);
 	struct mtk_ecc_stats stats;
+	u32 reg_size = mtk_nand->fdm.reg_size;
 	int rc, i;
 
 	rc = nfi_readl(nfc, NFI_STA) & STA_EMP_PAGE;
 	if (rc) {
 		memset(buf, 0xff, sectors * chip->ecc.size);
 		for (i = 0; i < sectors; i++)
-			memset(oob_ptr(chip, i), 0xff, mtk_nand->fdm.reg_size);
+			memset(oob_ptr(chip, start + i), 0xff, reg_size);
 		return 0;
 	}
 
@@ -921,7 +923,7 @@ static int mtk_nfc_read_subpage(struct mtd_info *mtd, struct nand_chip *chip,
 	u32 spare = mtk_nand->spare_per_sector;
 	u32 column, sectors, start, end, reg;
 	dma_addr_t addr;
-	int bitflips;
+	int bitflips = 0;
 	size_t len;
 	u8 *buf;
 	int rc;
@@ -988,14 +990,11 @@ static int mtk_nfc_read_subpage(struct mtd_info *mtd, struct nand_chip *chip,
 	if (rc < 0) {
 		dev_err(nfc->dev, "subpage done timeout\n");
 		bitflips = -EIO;
-	} else {
-		bitflips = 0;
-		if (!raw) {
-			rc = mtk_ecc_wait_done(nfc->ecc, ECC_DECODE);
-			bitflips = rc < 0 ? -ETIMEDOUT :
-				mtk_nfc_update_ecc_stats(mtd, buf, sectors);
-			mtk_nfc_read_fdm(chip, start, sectors);
-		}
+	} else if (!raw) {
+		rc = mtk_ecc_wait_done(nfc->ecc, ECC_DECODE);
+		bitflips = rc < 0 ? -ETIMEDOUT :
+			mtk_nfc_update_ecc_stats(mtd, buf, start, sectors);
+		mtk_nfc_read_fdm(chip, start, sectors);
 	}
 
 	dma_unmap_single(nfc->dev, addr, len, DMA_FROM_DEVICE);
-- 
2.18.0


______________________________________________________
Linux MTD discussion mailing list
http://lists.infradead.org/mailman/listinfo/linux-mtd/

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

* [PATCH v2 4/5] mtd: rawnand: mtk: Fix wrongly assigned OOB buffer pointer issue
@ 2019-04-30 10:02   ` Xiaolei Li
  0 siblings, 0 replies; 33+ messages in thread
From: Xiaolei Li @ 2019-04-30 10:02 UTC (permalink / raw)
  To: miquel.raynal, richard
  Cc: linux-mediatek, xiaolei.li, linux-mtd, srv_heupstream

One main goal of the function mtk_nfc_update_ecc_stats is to check
whether sectors are all empty. If they are empty, set these sectors's
data buffer and OOB buffer as 0xff.

But now, the sector OOB buffer pointer is wrongly assigned. We always
do memset from sector 0.

To fix this issue, pass start sector number to make OOB buffer pointer
be properly assigned.

Fixes: 1d6b1e464950 ("mtd: mediatek: driver for MTK Smart Device")
Signed-off-by: Xiaolei Li <xiaolei.li@mediatek.com>
Reviewed-by: Miquel Raynal <miquel.raynal@bootlin.com>
---
 drivers/mtd/nand/raw/mtk_nand.c | 21 ++++++++++-----------
 1 file changed, 10 insertions(+), 11 deletions(-)

diff --git a/drivers/mtd/nand/raw/mtk_nand.c b/drivers/mtd/nand/raw/mtk_nand.c
index 23ebd668bbcd..48759af5c058 100644
--- a/drivers/mtd/nand/raw/mtk_nand.c
+++ b/drivers/mtd/nand/raw/mtk_nand.c
@@ -889,19 +889,21 @@ static int mtk_nfc_write_oob_std(struct nand_chip *chip, int page)
 	return mtk_nfc_write_page_raw(chip, NULL, 1, page);
 }
 
-static int mtk_nfc_update_ecc_stats(struct mtd_info *mtd, u8 *buf, u32 sectors)
+static int mtk_nfc_update_ecc_stats(struct mtd_info *mtd, u8 *buf, u32 start,
+				    u32 sectors)
 {
 	struct nand_chip *chip = mtd_to_nand(mtd);
 	struct mtk_nfc *nfc = nand_get_controller_data(chip);
 	struct mtk_nfc_nand_chip *mtk_nand = to_mtk_nand(chip);
 	struct mtk_ecc_stats stats;
+	u32 reg_size = mtk_nand->fdm.reg_size;
 	int rc, i;
 
 	rc = nfi_readl(nfc, NFI_STA) & STA_EMP_PAGE;
 	if (rc) {
 		memset(buf, 0xff, sectors * chip->ecc.size);
 		for (i = 0; i < sectors; i++)
-			memset(oob_ptr(chip, i), 0xff, mtk_nand->fdm.reg_size);
+			memset(oob_ptr(chip, start + i), 0xff, reg_size);
 		return 0;
 	}
 
@@ -921,7 +923,7 @@ static int mtk_nfc_read_subpage(struct mtd_info *mtd, struct nand_chip *chip,
 	u32 spare = mtk_nand->spare_per_sector;
 	u32 column, sectors, start, end, reg;
 	dma_addr_t addr;
-	int bitflips;
+	int bitflips = 0;
 	size_t len;
 	u8 *buf;
 	int rc;
@@ -988,14 +990,11 @@ static int mtk_nfc_read_subpage(struct mtd_info *mtd, struct nand_chip *chip,
 	if (rc < 0) {
 		dev_err(nfc->dev, "subpage done timeout\n");
 		bitflips = -EIO;
-	} else {
-		bitflips = 0;
-		if (!raw) {
-			rc = mtk_ecc_wait_done(nfc->ecc, ECC_DECODE);
-			bitflips = rc < 0 ? -ETIMEDOUT :
-				mtk_nfc_update_ecc_stats(mtd, buf, sectors);
-			mtk_nfc_read_fdm(chip, start, sectors);
-		}
+	} else if (!raw) {
+		rc = mtk_ecc_wait_done(nfc->ecc, ECC_DECODE);
+		bitflips = rc < 0 ? -ETIMEDOUT :
+			mtk_nfc_update_ecc_stats(mtd, buf, start, sectors);
+		mtk_nfc_read_fdm(chip, start, sectors);
 	}
 
 	dma_unmap_single(nfc->dev, addr, len, DMA_FROM_DEVICE);
-- 
2.18.0


______________________________________________________
Linux MTD discussion mailing list
http://lists.infradead.org/mailman/listinfo/linux-mtd/

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

* [PATCH v2 5/5] mtd: rawnand: mtk: Setup empty page threshold correctly
  2019-04-30 10:02 ` Xiaolei Li
@ 2019-04-30 10:02   ` Xiaolei Li
  -1 siblings, 0 replies; 33+ messages in thread
From: Xiaolei Li @ 2019-04-30 10:02 UTC (permalink / raw)
  To: miquel.raynal, richard
  Cc: linux-mediatek, xiaolei.li, linux-mtd, srv_heupstream

MTK NAND Controller has the ability to check whether read data are
mostly 0xff by comparing zero bit count of read data with empty
threshold automatically.

But now we never set this threshold and always make it be default value
which is 10.

This patch fixes this problem by setting empty threshold as the product
of read sector count and ECC strength.

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

diff --git a/drivers/mtd/nand/raw/mtk_nand.c b/drivers/mtd/nand/raw/mtk_nand.c
index 48759af5c058..b56965328771 100644
--- a/drivers/mtd/nand/raw/mtk_nand.c
+++ b/drivers/mtd/nand/raw/mtk_nand.c
@@ -94,6 +94,7 @@
 #define NFI_MASTER_STA		(0x224)
 #define		MASTER_STA_MASK		(0x0FFF)
 #define NFI_EMPTY_THRESH	(0x23C)
+#define		EMPTY_THRESH_MASK	GENMASK(7, 0)
 
 #define MTK_NAME		"mtk-nand"
 #define KB(x)			((x) * 1024UL)
@@ -947,6 +948,14 @@ static int mtk_nfc_read_subpage(struct mtd_info *mtd, struct nand_chip *chip,
 		return -EINVAL;
 	}
 
+	/**
+	 * Setup empty threshold as the product of sector count
+	 * and ECC strength
+	 */
+	reg = sectors * chip->ecc.strength;
+	reg = min_t(unsigned int, reg, EMPTY_THRESH_MASK);
+	nfi_writel(nfc, reg, NFI_EMPTY_THRESH);
+
 	reg = nfi_readw(nfc, NFI_CNFG);
 	reg |= CNFG_READ_EN | CNFG_DMA_BURST_EN | CNFG_AHB;
 	if (!raw) {
-- 
2.18.0


______________________________________________________
Linux MTD discussion mailing list
http://lists.infradead.org/mailman/listinfo/linux-mtd/

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

* [PATCH v2 5/5] mtd: rawnand: mtk: Setup empty page threshold correctly
@ 2019-04-30 10:02   ` Xiaolei Li
  0 siblings, 0 replies; 33+ messages in thread
From: Xiaolei Li @ 2019-04-30 10:02 UTC (permalink / raw)
  To: miquel.raynal, richard
  Cc: linux-mediatek, xiaolei.li, linux-mtd, srv_heupstream

MTK NAND Controller has the ability to check whether read data are
mostly 0xff by comparing zero bit count of read data with empty
threshold automatically.

But now we never set this threshold and always make it be default value
which is 10.

This patch fixes this problem by setting empty threshold as the product
of read sector count and ECC strength.

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

diff --git a/drivers/mtd/nand/raw/mtk_nand.c b/drivers/mtd/nand/raw/mtk_nand.c
index 48759af5c058..b56965328771 100644
--- a/drivers/mtd/nand/raw/mtk_nand.c
+++ b/drivers/mtd/nand/raw/mtk_nand.c
@@ -94,6 +94,7 @@
 #define NFI_MASTER_STA		(0x224)
 #define		MASTER_STA_MASK		(0x0FFF)
 #define NFI_EMPTY_THRESH	(0x23C)
+#define		EMPTY_THRESH_MASK	GENMASK(7, 0)
 
 #define MTK_NAME		"mtk-nand"
 #define KB(x)			((x) * 1024UL)
@@ -947,6 +948,14 @@ static int mtk_nfc_read_subpage(struct mtd_info *mtd, struct nand_chip *chip,
 		return -EINVAL;
 	}
 
+	/**
+	 * Setup empty threshold as the product of sector count
+	 * and ECC strength
+	 */
+	reg = sectors * chip->ecc.strength;
+	reg = min_t(unsigned int, reg, EMPTY_THRESH_MASK);
+	nfi_writel(nfc, reg, NFI_EMPTY_THRESH);
+
 	reg = nfi_readw(nfc, NFI_CNFG);
 	reg |= CNFG_READ_EN | CNFG_DMA_BURST_EN | CNFG_AHB;
 	if (!raw) {
-- 
2.18.0


______________________________________________________
Linux MTD discussion mailing list
http://lists.infradead.org/mailman/listinfo/linux-mtd/

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

* Re: [PATCH v2 1/5] mtd: rawnand: mtk: Correct low level time calculation of r/w cycle
  2019-04-30 10:02   ` Xiaolei Li
  (?)
  (?)
@ 2019-04-30 10:32   ` Sasha Levin
  2019-05-05  7:06       ` xiaolei li
  -1 siblings, 1 reply; 33+ messages in thread
From: Sasha Levin @ 2019-04-30 10:32 UTC (permalink / raw)
  To: Sasha Levin, Xiaolei Li, miquel.raynal, richard; +Cc: , linux-mtd, stable

Hi,

[This is an automated email]

This commit has been processed because it contains a "Fixes:" tag,
fixing commit: edfee3619c49 mtd: nand: mtk: add ->setup_data_interface() hook.

The bot has tested the following trees: v5.0.10, v4.19.37, v4.14.114.

v5.0.10: Build OK!
v4.19.37: Build OK!
v4.14.114: Failed to apply! Possible dependencies:
    Unable to calculate


How should we proceed with this patch?

--
Thanks,
Sasha

______________________________________________________
Linux MTD discussion mailing list
http://lists.infradead.org/mailman/listinfo/linux-mtd/

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

* Re: [PATCH v2 1/5] mtd: rawnand: mtk: Correct low level time calculation of r/w cycle
  2019-04-30 10:02   ` Xiaolei Li
  (?)
@ 2019-04-30 11:59     ` Miquel Raynal
  -1 siblings, 0 replies; 33+ messages in thread
From: Miquel Raynal @ 2019-04-30 11:59 UTC (permalink / raw)
  To: Xiaolei Li; +Cc: richard, linux-mtd, linux-mediatek, srv_heupstream, stable

Hi Xiaolei,

Xiaolei Li <xiaolei.li@mediatek.com> wrote on Tue, 30 Apr 2019 18:02:46
+0800:

> At present, the flow of calculating AC timing of read/write cycle in SDR
> mode is that:
> At first, calculate high hold time which is valid for both read and write
> cycle using the max value between tREH_min and tWH_min.
> Secondly, calculate WE# pulse width using tWP_min.
> Thridly, calculate RE# pulse width using the bigger one between tREA_max
> and tRP_min.
> 
> But NAND SPEC shows that Controller should also meet write/read cycle time.
> That is write cycle time should be more than tWC_min and read cycle should
> be more than tRC_min. Obviously, we do not achieve that now.
> 
> This patch corrects the low level time calculation to meet minimum
> read/write cycle time required. After getting the high hold time, WE# low
> level time will be promised to meet tWP_min and tWC_min requirement,
> and RE# low level time will be promised to meet tREA_max, tRP_min and
> tRC_min requirement.
> 
> Fixes: edfee3619c49 ("mtd: nand: mtk: add ->setup_data_interface() hook")
> Cc: stable@vger.kernel.org
> Signed-off-by: Xiaolei Li <xiaolei.li@mediatek.com>
> ---
>  drivers/mtd/nand/raw/mtk_nand.c | 24 +++++++++++++++++++++---
>  1 file changed, 21 insertions(+), 3 deletions(-)
> 
> diff --git a/drivers/mtd/nand/raw/mtk_nand.c b/drivers/mtd/nand/raw/mtk_nand.c
> index b6b4602f5132..4fbb0c6ecae3 100644
> --- a/drivers/mtd/nand/raw/mtk_nand.c
> +++ b/drivers/mtd/nand/raw/mtk_nand.c
> @@ -508,7 +508,8 @@ static int mtk_nfc_setup_data_interface(struct nand_chip *chip, int csline,
>  {
>  	struct mtk_nfc *nfc = nand_get_controller_data(chip);
>  	const struct nand_sdr_timings *timings;
> -	u32 rate, tpoecs, tprecs, tc2r, tw2r, twh, twst, trlt;
> +	u32 rate, tpoecs, tprecs, tc2r, tw2r, twh, twst = 0, trlt = 0;
> +	u32 thold;
>  
>  	timings = nand_get_sdr_timings(conf);
>  	if (IS_ERR(timings))
> @@ -544,11 +545,28 @@ static int mtk_nfc_setup_data_interface(struct nand_chip *chip, int csline,
>  	twh = DIV_ROUND_UP(twh * rate, 1000000) - 1;
>  	twh &= 0xf;
>  
> -	twst = timings->tWP_min / 1000;
> +	/* Calculate real WE#/RE# hold time in nanosecond */
> +	thold = (twh + 1) * 1000000 / rate;
> +	/* nanosecond to picosecond */
> +	thold *= 1000;
> +
> +	/**

        /*

> +	 * WE# low level time should be expaned to meet WE# pulse time
> +	 * and WE# cycle time at the same time.
> +	 */
> +	if (thold < timings->tWC_min)
> +		twst = timings->tWC_min - thold;
> +	twst = max(timings->tWP_min, twst) / 1000;
>  	twst = DIV_ROUND_UP(twst * rate, 1000000) - 1;
>  	twst &= 0xf;
>  
> -	trlt = max(timings->tREA_max, timings->tRP_min) / 1000;
> +	/**

Ditto

> +	 * RE# low level time should be expaned to meet RE# pulse time,
> +	 * RE# access time and RE# cycle time at the same time.
> +	 */
> +	if (thold < timings->tRC_min)
> +		trlt = timings->tRC_min - thold;
> +	trlt = max3(trlt, timings->tREA_max, timings->tRP_min) / 1000;
>  	trlt = DIV_ROUND_UP(trlt * rate, 1000000) - 1;
>  	trlt &= 0xf;
>  

With this fixed:

Reviewed-by: Miquel Raynal <miquel.raynal@bootlin.com>


Thanks,
Miquèl

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

* Re: [PATCH v2 1/5] mtd: rawnand: mtk: Correct low level time calculation of r/w cycle
@ 2019-04-30 11:59     ` Miquel Raynal
  0 siblings, 0 replies; 33+ messages in thread
From: Miquel Raynal @ 2019-04-30 11:59 UTC (permalink / raw)
  To: Xiaolei Li; +Cc: richard, linux-mediatek, linux-mtd, stable, srv_heupstream

Hi Xiaolei,

Xiaolei Li <xiaolei.li@mediatek.com> wrote on Tue, 30 Apr 2019 18:02:46
+0800:

> At present, the flow of calculating AC timing of read/write cycle in SDR
> mode is that:
> At first, calculate high hold time which is valid for both read and write
> cycle using the max value between tREH_min and tWH_min.
> Secondly, calculate WE# pulse width using tWP_min.
> Thridly, calculate RE# pulse width using the bigger one between tREA_max
> and tRP_min.
> 
> But NAND SPEC shows that Controller should also meet write/read cycle time.
> That is write cycle time should be more than tWC_min and read cycle should
> be more than tRC_min. Obviously, we do not achieve that now.
> 
> This patch corrects the low level time calculation to meet minimum
> read/write cycle time required. After getting the high hold time, WE# low
> level time will be promised to meet tWP_min and tWC_min requirement,
> and RE# low level time will be promised to meet tREA_max, tRP_min and
> tRC_min requirement.
> 
> Fixes: edfee3619c49 ("mtd: nand: mtk: add ->setup_data_interface() hook")
> Cc: stable@vger.kernel.org
> Signed-off-by: Xiaolei Li <xiaolei.li@mediatek.com>
> ---
>  drivers/mtd/nand/raw/mtk_nand.c | 24 +++++++++++++++++++++---
>  1 file changed, 21 insertions(+), 3 deletions(-)
> 
> diff --git a/drivers/mtd/nand/raw/mtk_nand.c b/drivers/mtd/nand/raw/mtk_nand.c
> index b6b4602f5132..4fbb0c6ecae3 100644
> --- a/drivers/mtd/nand/raw/mtk_nand.c
> +++ b/drivers/mtd/nand/raw/mtk_nand.c
> @@ -508,7 +508,8 @@ static int mtk_nfc_setup_data_interface(struct nand_chip *chip, int csline,
>  {
>  	struct mtk_nfc *nfc = nand_get_controller_data(chip);
>  	const struct nand_sdr_timings *timings;
> -	u32 rate, tpoecs, tprecs, tc2r, tw2r, twh, twst, trlt;
> +	u32 rate, tpoecs, tprecs, tc2r, tw2r, twh, twst = 0, trlt = 0;
> +	u32 thold;
>  
>  	timings = nand_get_sdr_timings(conf);
>  	if (IS_ERR(timings))
> @@ -544,11 +545,28 @@ static int mtk_nfc_setup_data_interface(struct nand_chip *chip, int csline,
>  	twh = DIV_ROUND_UP(twh * rate, 1000000) - 1;
>  	twh &= 0xf;
>  
> -	twst = timings->tWP_min / 1000;
> +	/* Calculate real WE#/RE# hold time in nanosecond */
> +	thold = (twh + 1) * 1000000 / rate;
> +	/* nanosecond to picosecond */
> +	thold *= 1000;
> +
> +	/**

        /*

> +	 * WE# low level time should be expaned to meet WE# pulse time
> +	 * and WE# cycle time at the same time.
> +	 */
> +	if (thold < timings->tWC_min)
> +		twst = timings->tWC_min - thold;
> +	twst = max(timings->tWP_min, twst) / 1000;
>  	twst = DIV_ROUND_UP(twst * rate, 1000000) - 1;
>  	twst &= 0xf;
>  
> -	trlt = max(timings->tREA_max, timings->tRP_min) / 1000;
> +	/**

Ditto

> +	 * RE# low level time should be expaned to meet RE# pulse time,
> +	 * RE# access time and RE# cycle time at the same time.
> +	 */
> +	if (thold < timings->tRC_min)
> +		trlt = timings->tRC_min - thold;
> +	trlt = max3(trlt, timings->tREA_max, timings->tRP_min) / 1000;
>  	trlt = DIV_ROUND_UP(trlt * rate, 1000000) - 1;
>  	trlt &= 0xf;
>  

With this fixed:

Reviewed-by: Miquel Raynal <miquel.raynal@bootlin.com>


Thanks,
Miquèl

______________________________________________________
Linux MTD discussion mailing list
http://lists.infradead.org/mailman/listinfo/linux-mtd/

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

* Re: [PATCH v2 1/5] mtd: rawnand: mtk: Correct low level time calculation of r/w cycle
@ 2019-04-30 11:59     ` Miquel Raynal
  0 siblings, 0 replies; 33+ messages in thread
From: Miquel Raynal @ 2019-04-30 11:59 UTC (permalink / raw)
  To: Xiaolei Li; +Cc: richard, linux-mtd, linux-mediatek, srv_heupstream, stable

Hi Xiaolei,

Xiaolei Li <xiaolei.li@mediatek.com> wrote on Tue, 30 Apr 2019 18:02:46
+0800:

> At present, the flow of calculating AC timing of read/write cycle in SDR
> mode is that:
> At first, calculate high hold time which is valid for both read and write
> cycle using the max value between tREH_min and tWH_min.
> Secondly, calculate WE# pulse width using tWP_min.
> Thridly, calculate RE# pulse width using the bigger one between tREA_max
> and tRP_min.
> 
> But NAND SPEC shows that Controller should also meet write/read cycle time.
> That is write cycle time should be more than tWC_min and read cycle should
> be more than tRC_min. Obviously, we do not achieve that now.
> 
> This patch corrects the low level time calculation to meet minimum
> read/write cycle time required. After getting the high hold time, WE# low
> level time will be promised to meet tWP_min and tWC_min requirement,
> and RE# low level time will be promised to meet tREA_max, tRP_min and
> tRC_min requirement.
> 
> Fixes: edfee3619c49 ("mtd: nand: mtk: add ->setup_data_interface() hook")
> Cc: stable@vger.kernel.org
> Signed-off-by: Xiaolei Li <xiaolei.li@mediatek.com>
> ---
>  drivers/mtd/nand/raw/mtk_nand.c | 24 +++++++++++++++++++++---
>  1 file changed, 21 insertions(+), 3 deletions(-)
> 
> diff --git a/drivers/mtd/nand/raw/mtk_nand.c b/drivers/mtd/nand/raw/mtk_nand.c
> index b6b4602f5132..4fbb0c6ecae3 100644
> --- a/drivers/mtd/nand/raw/mtk_nand.c
> +++ b/drivers/mtd/nand/raw/mtk_nand.c
> @@ -508,7 +508,8 @@ static int mtk_nfc_setup_data_interface(struct nand_chip *chip, int csline,
>  {
>  	struct mtk_nfc *nfc = nand_get_controller_data(chip);
>  	const struct nand_sdr_timings *timings;
> -	u32 rate, tpoecs, tprecs, tc2r, tw2r, twh, twst, trlt;
> +	u32 rate, tpoecs, tprecs, tc2r, tw2r, twh, twst = 0, trlt = 0;
> +	u32 thold;
>  
>  	timings = nand_get_sdr_timings(conf);
>  	if (IS_ERR(timings))
> @@ -544,11 +545,28 @@ static int mtk_nfc_setup_data_interface(struct nand_chip *chip, int csline,
>  	twh = DIV_ROUND_UP(twh * rate, 1000000) - 1;
>  	twh &= 0xf;
>  
> -	twst = timings->tWP_min / 1000;
> +	/* Calculate real WE#/RE# hold time in nanosecond */
> +	thold = (twh + 1) * 1000000 / rate;
> +	/* nanosecond to picosecond */
> +	thold *= 1000;
> +
> +	/**

        /*

> +	 * WE# low level time should be expaned to meet WE# pulse time
> +	 * and WE# cycle time at the same time.
> +	 */
> +	if (thold < timings->tWC_min)
> +		twst = timings->tWC_min - thold;
> +	twst = max(timings->tWP_min, twst) / 1000;
>  	twst = DIV_ROUND_UP(twst * rate, 1000000) - 1;
>  	twst &= 0xf;
>  
> -	trlt = max(timings->tREA_max, timings->tRP_min) / 1000;
> +	/**

Ditto

> +	 * RE# low level time should be expaned to meet RE# pulse time,
> +	 * RE# access time and RE# cycle time at the same time.
> +	 */
> +	if (thold < timings->tRC_min)
> +		trlt = timings->tRC_min - thold;
> +	trlt = max3(trlt, timings->tREA_max, timings->tRP_min) / 1000;
>  	trlt = DIV_ROUND_UP(trlt * rate, 1000000) - 1;
>  	trlt &= 0xf;
>  

With this fixed:

Reviewed-by: Miquel Raynal <miquel.raynal@bootlin.com>


Thanks,
Miquèl

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

* Re: [PATCH v2 0/5] MTK NAND driver improvements and fixes
@ 2019-04-30 12:08   ` Miquel Raynal
  0 siblings, 0 replies; 33+ messages in thread
From: Miquel Raynal @ 2019-04-30 12:08 UTC (permalink / raw)
  To: Xiaolei Li; +Cc: richard, linux-mediatek, linux-mtd, srv_heupstream

Hi Xiaolei,

Xiaolei Li <xiaolei.li@mediatek.com> wrote on Tue, 30 Apr 2019 18:02:45
+0800:

> The following patch set mainly contains:
> * Fix low level time calculation of read/write cycle to meet tRC_min
>   and tWC_min requirements.
> * Refine RE# pulse width calculation and data sampling to improve read
>   performance.
> * Add CS validity check.
> * Fix oob buffer pointer wrongly setting and empty page threshold setting.
> 
> Changes on v2 relative to:
> --------------------
> 
> tree    : git://git.infradead.org/linux-mtd.git
> branch  : master

We just switched to a repository hosted on kernel.org:
https://git.kernel.org/pub/scm/linux/kernel/git/mtd/linux.git/
nand/next is the branch for this kind of series.

Thanks,
Miquèl

______________________________________________________
Linux MTD discussion mailing list
http://lists.infradead.org/mailman/listinfo/linux-mtd/

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

* Re: [PATCH v2 0/5] MTK NAND driver improvements and fixes
@ 2019-04-30 12:08   ` Miquel Raynal
  0 siblings, 0 replies; 33+ messages in thread
From: Miquel Raynal @ 2019-04-30 12:08 UTC (permalink / raw)
  To: Xiaolei Li
  Cc: richard-/L3Ra7n9ekc,
	linux-mediatek-IAPFreCvJWM7uuMidbF8XUB+6BGkLq7r,
	linux-mtd-IAPFreCvJWM7uuMidbF8XUB+6BGkLq7r,
	srv_heupstream-NuS5LvNUpcJWk0Htik3J/w

Hi Xiaolei,

Xiaolei Li <xiaolei.li@mediatek.com> wrote on Tue, 30 Apr 2019 18:02:45
+0800:

> The following patch set mainly contains:
> * Fix low level time calculation of read/write cycle to meet tRC_min
>   and tWC_min requirements.
> * Refine RE# pulse width calculation and data sampling to improve read
>   performance.
> * Add CS validity check.
> * Fix oob buffer pointer wrongly setting and empty page threshold setting.
> 
> Changes on v2 relative to:
> --------------------
> 
> tree    : git://git.infradead.org/linux-mtd.git
> branch  : master

We just switched to a repository hosted on kernel.org:
https://git.kernel.org/pub/scm/linux/kernel/git/mtd/linux.git/
nand/next is the branch for this kind of series.

Thanks,
Miquèl

_______________________________________________
Linux-mediatek mailing list
Linux-mediatek@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-mediatek

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

* Re: [PATCH v2 2/5] mtd: rawnand: mtk: Improve data sampling timing for read cycle
@ 2019-04-30 12:11     ` Miquel Raynal
  0 siblings, 0 replies; 33+ messages in thread
From: Miquel Raynal @ 2019-04-30 12:11 UTC (permalink / raw)
  To: Xiaolei Li; +Cc: richard, linux-mediatek, linux-mtd, srv_heupstream

Hi Xiaolei,

Xiaolei Li <xiaolei.li@mediatek.com> wrote on Tue, 30 Apr 2019 18:02:47
+0800:

> Currently, we expand RE# low level time by choosing the max value
> between RE# pulse width and RE# access time, and sample data at the
> rising edge of RE#.
> 
> Then, if RE# access time is bigger than RE# pulse width, the real
> read cycle time may be more than NAND SPEC required. This makes
> read performance be worse than that expected.
> 
> This patch improves data sampling timing by calculating RE# low level
> time according to RE# pulse width. If RE# access time is bigger than
> RE# pulse width, then delay sampling data timing.
> 
> The result of contrast test base on MT2712 evaluat board is as follow.
> 
> nand: Micron MT29F16G08ADBCAH4
> nand: 2048 MiB, SLC, erase size: 256 KiB, page size: 4096, OOB size: 224
> NFI 2x clock rate: 124800000 HZ.
> 
> Read speed without this patch:
> mtd_speedtest: page read speed is 14012 KiB/s
> mtd_speedtest: 2 page read speed is 14860 KiB/s
> 
> Read speed with this patch:
> mtd_speedtest: page read speed is 18724 KiB/s
> mtd_speedtest: 2 page read speed is 18713 KiB/s
> 
> Signed-off-by: Xiaolei Li <xiaolei.li@mediatek.com>
> ---
>  drivers/mtd/nand/raw/mtk_nand.c | 46 ++++++++++++++++++++++++++-------
>  1 file changed, 36 insertions(+), 10 deletions(-)
> 
> diff --git a/drivers/mtd/nand/raw/mtk_nand.c b/drivers/mtd/nand/raw/mtk_nand.c
> index 4fbb0c6ecae3..e90c38c6f835 100644
> --- a/drivers/mtd/nand/raw/mtk_nand.c
> +++ b/drivers/mtd/nand/raw/mtk_nand.c
> @@ -87,6 +87,10 @@
>  #define NFI_FDMM(x)		(0xA4 + (x) * sizeof(u32) * 2)
>  #define NFI_FDM_MAX_SIZE	(8)
>  #define NFI_FDM_MIN_SIZE	(1)
> +#define NFI_DEBUG_CON1		(0x220)
> +#define		STROBE_MASK		GENMASK(4, 3)
> +#define		STROBE_SHIFT		(3)
> +#define		MAX_STROBE_DLY		(3)
>  #define NFI_MASTER_STA		(0x224)
>  #define		MASTER_STA_MASK		(0x0FFF)
>  #define NFI_EMPTY_THRESH	(0x23C)
> @@ -509,7 +513,7 @@ static int mtk_nfc_setup_data_interface(struct nand_chip *chip, int csline,
>  	struct mtk_nfc *nfc = nand_get_controller_data(chip);
>  	const struct nand_sdr_timings *timings;
>  	u32 rate, tpoecs, tprecs, tc2r, tw2r, twh, twst = 0, trlt = 0;
> -	u32 thold;
> +	u32 temp, tsel = 0;
>  
>  	timings = nand_get_sdr_timings(conf);
>  	if (IS_ERR(timings))
> @@ -546,30 +550,52 @@ static int mtk_nfc_setup_data_interface(struct nand_chip *chip, int csline,
>  	twh &= 0xf;
>  
>  	/* Calculate real WE#/RE# hold time in nanosecond */
> -	thold = (twh + 1) * 1000000 / rate;
> +	temp = (twh + 1) * 1000000 / rate;
>  	/* nanosecond to picosecond */
> -	thold *= 1000;
> +	temp *= 1000;
>  
>  	/**
>  	 * WE# low level time should be expaned to meet WE# pulse time
>  	 * and WE# cycle time at the same time.
>  	 */
> -	if (thold < timings->tWC_min)
> -		twst = timings->tWC_min - thold;
> +	if (temp < timings->tWC_min)
> +		twst = timings->tWC_min - temp;
>  	twst = max(timings->tWP_min, twst) / 1000;
>  	twst = DIV_ROUND_UP(twst * rate, 1000000) - 1;
>  	twst &= 0xf;
>  
>  	/**
> -	 * RE# low level time should be expaned to meet RE# pulse time,
> -	 * RE# access time and RE# cycle time at the same time.
> +	 * RE# low level time should be expaned to meet RE# pulse time
> +	 * and RE# cycle time at the same time.
>  	 */
> -	if (thold < timings->tRC_min)
> -		trlt = timings->tRC_min - thold;
> -	trlt = max3(trlt, timings->tREA_max, timings->tRP_min) / 1000;
> +	if (temp < timings->tRC_min)
> +		trlt = timings->tRC_min - temp;
> +	trlt = max(trlt, timings->tRP_min) / 1000;
>  	trlt = DIV_ROUND_UP(trlt * rate, 1000000) - 1;
>  	trlt &= 0xf;
>  
> +	/**

        /*

> +	 * Calculate strobe select timing.
> +	 * If RE# access time is bigger than RE# pulse time,
> +	 * delay sampling data timing.
> +	 */
> +	temp = (trlt + 1) * 1000000 / rate;

You could precise what unit conversion you do here.

> +	/* nanosecond to picosecond */
> +	temp *= 1000;
> +	if (temp < timings->tREA_max) {
> +		tsel = timings->tREA_max / 1000;
> +		tsel = DIV_ROUND_UP(tsel * rate, 1000000);
> +		tsel -= (trlt + 1);
> +		if (tsel > MAX_STROBE_DLY) {
> +			trlt += tsel - MAX_STROBE_DLY;
> +			tsel = MAX_STROBE_DLY;
> +		}
> +	}
> +	temp = nfi_readl(nfc, NFI_DEBUG_CON1);
> +	temp &= ~STROBE_MASK;
> +	temp |= tsel << STROBE_SHIFT;
> +	nfi_writel(nfc, temp, NFI_DEBUG_CON1);
> +
>  	/*
>  	 * ACCON: access timing control register
>  	 * -------------------------------------

With this:

Reviewed-by: Miquel Raynal <miquel.raynal@bootlin.com>


Thanks,
Miquèl

______________________________________________________
Linux MTD discussion mailing list
http://lists.infradead.org/mailman/listinfo/linux-mtd/

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

* Re: [PATCH v2 2/5] mtd: rawnand: mtk: Improve data sampling timing for read cycle
@ 2019-04-30 12:11     ` Miquel Raynal
  0 siblings, 0 replies; 33+ messages in thread
From: Miquel Raynal @ 2019-04-30 12:11 UTC (permalink / raw)
  To: Xiaolei Li
  Cc: richard-/L3Ra7n9ekc,
	linux-mediatek-IAPFreCvJWM7uuMidbF8XUB+6BGkLq7r,
	linux-mtd-IAPFreCvJWM7uuMidbF8XUB+6BGkLq7r,
	srv_heupstream-NuS5LvNUpcJWk0Htik3J/w

Hi Xiaolei,

Xiaolei Li <xiaolei.li@mediatek.com> wrote on Tue, 30 Apr 2019 18:02:47
+0800:

> Currently, we expand RE# low level time by choosing the max value
> between RE# pulse width and RE# access time, and sample data at the
> rising edge of RE#.
> 
> Then, if RE# access time is bigger than RE# pulse width, the real
> read cycle time may be more than NAND SPEC required. This makes
> read performance be worse than that expected.
> 
> This patch improves data sampling timing by calculating RE# low level
> time according to RE# pulse width. If RE# access time is bigger than
> RE# pulse width, then delay sampling data timing.
> 
> The result of contrast test base on MT2712 evaluat board is as follow.
> 
> nand: Micron MT29F16G08ADBCAH4
> nand: 2048 MiB, SLC, erase size: 256 KiB, page size: 4096, OOB size: 224
> NFI 2x clock rate: 124800000 HZ.
> 
> Read speed without this patch:
> mtd_speedtest: page read speed is 14012 KiB/s
> mtd_speedtest: 2 page read speed is 14860 KiB/s
> 
> Read speed with this patch:
> mtd_speedtest: page read speed is 18724 KiB/s
> mtd_speedtest: 2 page read speed is 18713 KiB/s
> 
> Signed-off-by: Xiaolei Li <xiaolei.li@mediatek.com>
> ---
>  drivers/mtd/nand/raw/mtk_nand.c | 46 ++++++++++++++++++++++++++-------
>  1 file changed, 36 insertions(+), 10 deletions(-)
> 
> diff --git a/drivers/mtd/nand/raw/mtk_nand.c b/drivers/mtd/nand/raw/mtk_nand.c
> index 4fbb0c6ecae3..e90c38c6f835 100644
> --- a/drivers/mtd/nand/raw/mtk_nand.c
> +++ b/drivers/mtd/nand/raw/mtk_nand.c
> @@ -87,6 +87,10 @@
>  #define NFI_FDMM(x)		(0xA4 + (x) * sizeof(u32) * 2)
>  #define NFI_FDM_MAX_SIZE	(8)
>  #define NFI_FDM_MIN_SIZE	(1)
> +#define NFI_DEBUG_CON1		(0x220)
> +#define		STROBE_MASK		GENMASK(4, 3)
> +#define		STROBE_SHIFT		(3)
> +#define		MAX_STROBE_DLY		(3)
>  #define NFI_MASTER_STA		(0x224)
>  #define		MASTER_STA_MASK		(0x0FFF)
>  #define NFI_EMPTY_THRESH	(0x23C)
> @@ -509,7 +513,7 @@ static int mtk_nfc_setup_data_interface(struct nand_chip *chip, int csline,
>  	struct mtk_nfc *nfc = nand_get_controller_data(chip);
>  	const struct nand_sdr_timings *timings;
>  	u32 rate, tpoecs, tprecs, tc2r, tw2r, twh, twst = 0, trlt = 0;
> -	u32 thold;
> +	u32 temp, tsel = 0;
>  
>  	timings = nand_get_sdr_timings(conf);
>  	if (IS_ERR(timings))
> @@ -546,30 +550,52 @@ static int mtk_nfc_setup_data_interface(struct nand_chip *chip, int csline,
>  	twh &= 0xf;
>  
>  	/* Calculate real WE#/RE# hold time in nanosecond */
> -	thold = (twh + 1) * 1000000 / rate;
> +	temp = (twh + 1) * 1000000 / rate;
>  	/* nanosecond to picosecond */
> -	thold *= 1000;
> +	temp *= 1000;
>  
>  	/**
>  	 * WE# low level time should be expaned to meet WE# pulse time
>  	 * and WE# cycle time at the same time.
>  	 */
> -	if (thold < timings->tWC_min)
> -		twst = timings->tWC_min - thold;
> +	if (temp < timings->tWC_min)
> +		twst = timings->tWC_min - temp;
>  	twst = max(timings->tWP_min, twst) / 1000;
>  	twst = DIV_ROUND_UP(twst * rate, 1000000) - 1;
>  	twst &= 0xf;
>  
>  	/**
> -	 * RE# low level time should be expaned to meet RE# pulse time,
> -	 * RE# access time and RE# cycle time at the same time.
> +	 * RE# low level time should be expaned to meet RE# pulse time
> +	 * and RE# cycle time at the same time.
>  	 */
> -	if (thold < timings->tRC_min)
> -		trlt = timings->tRC_min - thold;
> -	trlt = max3(trlt, timings->tREA_max, timings->tRP_min) / 1000;
> +	if (temp < timings->tRC_min)
> +		trlt = timings->tRC_min - temp;
> +	trlt = max(trlt, timings->tRP_min) / 1000;
>  	trlt = DIV_ROUND_UP(trlt * rate, 1000000) - 1;
>  	trlt &= 0xf;
>  
> +	/**

        /*

> +	 * Calculate strobe select timing.
> +	 * If RE# access time is bigger than RE# pulse time,
> +	 * delay sampling data timing.
> +	 */
> +	temp = (trlt + 1) * 1000000 / rate;

You could precise what unit conversion you do here.

> +	/* nanosecond to picosecond */
> +	temp *= 1000;
> +	if (temp < timings->tREA_max) {
> +		tsel = timings->tREA_max / 1000;
> +		tsel = DIV_ROUND_UP(tsel * rate, 1000000);
> +		tsel -= (trlt + 1);
> +		if (tsel > MAX_STROBE_DLY) {
> +			trlt += tsel - MAX_STROBE_DLY;
> +			tsel = MAX_STROBE_DLY;
> +		}
> +	}
> +	temp = nfi_readl(nfc, NFI_DEBUG_CON1);
> +	temp &= ~STROBE_MASK;
> +	temp |= tsel << STROBE_SHIFT;
> +	nfi_writel(nfc, temp, NFI_DEBUG_CON1);
> +
>  	/*
>  	 * ACCON: access timing control register
>  	 * -------------------------------------

With this:

Reviewed-by: Miquel Raynal <miquel.raynal@bootlin.com>


Thanks,
Miquèl

_______________________________________________
Linux-mediatek mailing list
Linux-mediatek@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-mediatek

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

* Re: [PATCH v2 5/5] mtd: rawnand: mtk: Setup empty page threshold correctly
  2019-04-30 10:02   ` Xiaolei Li
  (?)
@ 2019-04-30 12:17   ` Miquel Raynal
  2019-05-05  7:50       ` xiaolei li
  -1 siblings, 1 reply; 33+ messages in thread
From: Miquel Raynal @ 2019-04-30 12:17 UTC (permalink / raw)
  To: Xiaolei Li; +Cc: richard, linux-mediatek, linux-mtd, srv_heupstream

Hi Xiaolei,

Xiaolei Li <xiaolei.li@mediatek.com> wrote on Tue, 30 Apr 2019 18:02:50
+0800:

> MTK NAND Controller has the ability to check whether read data are
> mostly 0xff by comparing zero bit count of read data with empty
> threshold automatically.
> 
> But now we never set this threshold and always make it be default value
> which is 10.
> 
> This patch fixes this problem by setting empty threshold as the product
> of read sector count and ECC strength.

Are you sure it is not a per-sector value?

Did you use nandbiterrs to validate?

> 
> Fixes: 1d6b1e464950 ("mtd: mediatek: driver for MTK Smart Device")
> Signed-off-by: Xiaolei Li <xiaolei.li@mediatek.com>
> ---
>  drivers/mtd/nand/raw/mtk_nand.c | 9 +++++++++
>  1 file changed, 9 insertions(+)
> 
> diff --git a/drivers/mtd/nand/raw/mtk_nand.c b/drivers/mtd/nand/raw/mtk_nand.c
> index 48759af5c058..b56965328771 100644
> --- a/drivers/mtd/nand/raw/mtk_nand.c
> +++ b/drivers/mtd/nand/raw/mtk_nand.c
> @@ -94,6 +94,7 @@
>  #define NFI_MASTER_STA		(0x224)
>  #define		MASTER_STA_MASK		(0x0FFF)
>  #define NFI_EMPTY_THRESH	(0x23C)
> +#define		EMPTY_THRESH_MASK	GENMASK(7, 0)
>  
>  #define MTK_NAME		"mtk-nand"
>  #define KB(x)			((x) * 1024UL)
> @@ -947,6 +948,14 @@ static int mtk_nfc_read_subpage(struct mtd_info *mtd, struct nand_chip *chip,
>  		return -EINVAL;
>  	}
>  
> +	/**

        /*

> +	 * Setup empty threshold as the product of sector count
> +	 * and ECC strength
> +	 */
> +	reg = sectors * chip->ecc.strength;
> +	reg = min_t(unsigned int, reg, EMPTY_THRESH_MASK);
> +	nfi_writel(nfc, reg, NFI_EMPTY_THRESH);
> +
>  	reg = nfi_readw(nfc, NFI_CNFG);
>  	reg |= CNFG_READ_EN | CNFG_DMA_BURST_EN | CNFG_AHB;
>  	if (!raw) {


Thanks,
Miquèl

______________________________________________________
Linux MTD discussion mailing list
http://lists.infradead.org/mailman/listinfo/linux-mtd/

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

* Re: [PATCH v2 1/5] mtd: rawnand: mtk: Correct low level time calculation of r/w cycle
  2019-04-30 10:32   ` Sasha Levin
@ 2019-05-05  7:06       ` xiaolei li
  0 siblings, 0 replies; 33+ messages in thread
From: xiaolei li @ 2019-05-05  7:06 UTC (permalink / raw)
  To: Sasha Levin; +Cc: miquel.raynal, richard, linux-mtd, stable

Hi Sasha,

I am not sure if it is caused by raw NAND code path change.

Raw NAND code was moved from mtd/nand to mtd/nand/raw subdirectory since
kernel v4.17.

The fixing commit: edfee3619c49 mtd: nand: mtk:
add->setup_data_interface() hook exists before kernel v4.17.

@Miquel, do you know if some other raw NAND driver ever encountered this
case? Thanks.

On Tue, 2019-04-30 at 10:32 +0000, Sasha Levin wrote:
> Hi,
> 
> [This is an automated email]
> 
> This commit has been processed because it contains a "Fixes:" tag,
> fixing commit: edfee3619c49 mtd: nand: mtk: add ->setup_data_interface() hook.
> 
> The bot has tested the following trees: v5.0.10, v4.19.37, v4.14.114.
> 
> v5.0.10: Build OK!
> v4.19.37: Build OK!
> v4.14.114: Failed to apply! Possible dependencies:
>     Unable to calculate
> 
> 
> How should we proceed with this patch?
> 
> --
> Thanks,
> Sasha

Thanks,
Xiaolei


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

* Re: [PATCH v2 1/5] mtd: rawnand: mtk: Correct low level time calculation of r/w cycle
@ 2019-05-05  7:06       ` xiaolei li
  0 siblings, 0 replies; 33+ messages in thread
From: xiaolei li @ 2019-05-05  7:06 UTC (permalink / raw)
  To: Sasha Levin; +Cc: richard, linux-mtd, stable, miquel.raynal

Hi Sasha,

I am not sure if it is caused by raw NAND code path change.

Raw NAND code was moved from mtd/nand to mtd/nand/raw subdirectory since
kernel v4.17.

The fixing commit: edfee3619c49 mtd: nand: mtk:
add->setup_data_interface() hook exists before kernel v4.17.

@Miquel, do you know if some other raw NAND driver ever encountered this
case? Thanks.

On Tue, 2019-04-30 at 10:32 +0000, Sasha Levin wrote:
> Hi,
> 
> [This is an automated email]
> 
> This commit has been processed because it contains a "Fixes:" tag,
> fixing commit: edfee3619c49 mtd: nand: mtk: add ->setup_data_interface() hook.
> 
> The bot has tested the following trees: v5.0.10, v4.19.37, v4.14.114.
> 
> v5.0.10: Build OK!
> v4.19.37: Build OK!
> v4.14.114: Failed to apply! Possible dependencies:
>     Unable to calculate
> 
> 
> How should we proceed with this patch?
> 
> --
> Thanks,
> Sasha

Thanks,
Xiaolei


______________________________________________________
Linux MTD discussion mailing list
http://lists.infradead.org/mailman/listinfo/linux-mtd/

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

* Re: [PATCH v2 0/5] MTK NAND driver improvements and fixes
  2019-04-30 12:08   ` Miquel Raynal
  (?)
@ 2019-05-05  7:08   ` xiaolei li
  -1 siblings, 0 replies; 33+ messages in thread
From: xiaolei li @ 2019-05-05  7:08 UTC (permalink / raw)
  To: Miquel Raynal; +Cc: richard, linux-mediatek, linux-mtd, srv_heupstream

Hi Miquel,

On Tue, 2019-04-30 at 14:08 +0200, Miquel Raynal wrote:
> Hi Xiaolei,
> 
> Xiaolei Li <xiaolei.li@mediatek.com> wrote on Tue, 30 Apr 2019 18:02:45
> +0800:
> 
> > The following patch set mainly contains:
> > * Fix low level time calculation of read/write cycle to meet tRC_min
> >   and tWC_min requirements.
> > * Refine RE# pulse width calculation and data sampling to improve read
> >   performance.
> > * Add CS validity check.
> > * Fix oob buffer pointer wrongly setting and empty page threshold setting.
> > 
> > Changes on v2 relative to:
> > --------------------
> > 
> > tree    : git://git.infradead.org/linux-mtd.git
> > branch  : master
> 
> We just switched to a repository hosted on kernel.org:
> https://git.kernel.org/pub/scm/linux/kernel/git/mtd/linux.git/
> nand/next is the branch for this kind of series.
OK. Good.
Thanks for your sharing.
> 
> Thanks,
> Miquèl

Thanks,
Xiaolei



______________________________________________________
Linux MTD discussion mailing list
http://lists.infradead.org/mailman/listinfo/linux-mtd/

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

* Re: [PATCH v2 1/5] mtd: rawnand: mtk: Correct low level time calculation of r/w cycle
  2019-04-30 11:59     ` Miquel Raynal
  (?)
@ 2019-05-05  7:12       ` xiaolei li
  -1 siblings, 0 replies; 33+ messages in thread
From: xiaolei li @ 2019-05-05  7:12 UTC (permalink / raw)
  To: Miquel Raynal; +Cc: richard, linux-mtd, linux-mediatek, srv_heupstream, stable

On Tue, 2019-04-30 at 13:59 +0200, Miquel Raynal wrote:
> Hi Xiaolei,
> 
> Xiaolei Li <xiaolei.li@mediatek.com> wrote on Tue, 30 Apr 2019 18:02:46
> +0800:
> 
> > At present, the flow of calculating AC timing of read/write cycle in SDR
> > mode is that:
> > At first, calculate high hold time which is valid for both read and write
> > cycle using the max value between tREH_min and tWH_min.
> > Secondly, calculate WE# pulse width using tWP_min.
> > Thridly, calculate RE# pulse width using the bigger one between tREA_max
> > and tRP_min.
> > 
> > But NAND SPEC shows that Controller should also meet write/read cycle time.
> > That is write cycle time should be more than tWC_min and read cycle should
> > be more than tRC_min. Obviously, we do not achieve that now.
> > 
> > This patch corrects the low level time calculation to meet minimum
> > read/write cycle time required. After getting the high hold time, WE# low
> > level time will be promised to meet tWP_min and tWC_min requirement,
> > and RE# low level time will be promised to meet tREA_max, tRP_min and
> > tRC_min requirement.
> > 
> > Fixes: edfee3619c49 ("mtd: nand: mtk: add ->setup_data_interface() hook")
> > Cc: stable@vger.kernel.org
> > Signed-off-by: Xiaolei Li <xiaolei.li@mediatek.com>
> > ---
> >  drivers/mtd/nand/raw/mtk_nand.c | 24 +++++++++++++++++++++---
> >  1 file changed, 21 insertions(+), 3 deletions(-)
> > 
> > diff --git a/drivers/mtd/nand/raw/mtk_nand.c b/drivers/mtd/nand/raw/mtk_nand.c
> > index b6b4602f5132..4fbb0c6ecae3 100644
> > --- a/drivers/mtd/nand/raw/mtk_nand.c
> > +++ b/drivers/mtd/nand/raw/mtk_nand.c
> > @@ -508,7 +508,8 @@ static int mtk_nfc_setup_data_interface(struct nand_chip *chip, int csline,
> >  {
> >  	struct mtk_nfc *nfc = nand_get_controller_data(chip);
> >  	const struct nand_sdr_timings *timings;
> > -	u32 rate, tpoecs, tprecs, tc2r, tw2r, twh, twst, trlt;
> > +	u32 rate, tpoecs, tprecs, tc2r, tw2r, twh, twst = 0, trlt = 0;
> > +	u32 thold;
> >  
> >  	timings = nand_get_sdr_timings(conf);
> >  	if (IS_ERR(timings))
> > @@ -544,11 +545,28 @@ static int mtk_nfc_setup_data_interface(struct nand_chip *chip, int csline,
> >  	twh = DIV_ROUND_UP(twh * rate, 1000000) - 1;
> >  	twh &= 0xf;
> >  
> > -	twst = timings->tWP_min / 1000;
> > +	/* Calculate real WE#/RE# hold time in nanosecond */
> > +	thold = (twh + 1) * 1000000 / rate;
> > +	/* nanosecond to picosecond */
> > +	thold *= 1000;
> > +
> > +	/**
> 
>         /*
> 
> > +	 * WE# low level time should be expaned to meet WE# pulse time
> > +	 * and WE# cycle time at the same time.
> > +	 */
> > +	if (thold < timings->tWC_min)
> > +		twst = timings->tWC_min - thold;
> > +	twst = max(timings->tWP_min, twst) / 1000;
> >  	twst = DIV_ROUND_UP(twst * rate, 1000000) - 1;
> >  	twst &= 0xf;
> >  
> > -	trlt = max(timings->tREA_max, timings->tRP_min) / 1000;
> > +	/**
> 
> Ditto
OK.

> 
> > +	 * RE# low level time should be expaned to meet RE# pulse time,
> > +	 * RE# access time and RE# cycle time at the same time.
> > +	 */
> > +	if (thold < timings->tRC_min)
> > +		trlt = timings->tRC_min - thold;
> > +	trlt = max3(trlt, timings->tREA_max, timings->tRP_min) / 1000;
> >  	trlt = DIV_ROUND_UP(trlt * rate, 1000000) - 1;
> >  	trlt &= 0xf;
> >  
> 
> With this fixed:
> 
> Reviewed-by: Miquel Raynal <miquel.raynal@bootlin.com>
> 
> 
> Thanks,
> Miquèl

Thanks,
Xiaolei


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

* Re: [PATCH v2 1/5] mtd: rawnand: mtk: Correct low level time calculation of r/w cycle
@ 2019-05-05  7:12       ` xiaolei li
  0 siblings, 0 replies; 33+ messages in thread
From: xiaolei li @ 2019-05-05  7:12 UTC (permalink / raw)
  To: Miquel Raynal; +Cc: richard, linux-mediatek, linux-mtd, stable, srv_heupstream

On Tue, 2019-04-30 at 13:59 +0200, Miquel Raynal wrote:
> Hi Xiaolei,
> 
> Xiaolei Li <xiaolei.li@mediatek.com> wrote on Tue, 30 Apr 2019 18:02:46
> +0800:
> 
> > At present, the flow of calculating AC timing of read/write cycle in SDR
> > mode is that:
> > At first, calculate high hold time which is valid for both read and write
> > cycle using the max value between tREH_min and tWH_min.
> > Secondly, calculate WE# pulse width using tWP_min.
> > Thridly, calculate RE# pulse width using the bigger one between tREA_max
> > and tRP_min.
> > 
> > But NAND SPEC shows that Controller should also meet write/read cycle time.
> > That is write cycle time should be more than tWC_min and read cycle should
> > be more than tRC_min. Obviously, we do not achieve that now.
> > 
> > This patch corrects the low level time calculation to meet minimum
> > read/write cycle time required. After getting the high hold time, WE# low
> > level time will be promised to meet tWP_min and tWC_min requirement,
> > and RE# low level time will be promised to meet tREA_max, tRP_min and
> > tRC_min requirement.
> > 
> > Fixes: edfee3619c49 ("mtd: nand: mtk: add ->setup_data_interface() hook")
> > Cc: stable@vger.kernel.org
> > Signed-off-by: Xiaolei Li <xiaolei.li@mediatek.com>
> > ---
> >  drivers/mtd/nand/raw/mtk_nand.c | 24 +++++++++++++++++++++---
> >  1 file changed, 21 insertions(+), 3 deletions(-)
> > 
> > diff --git a/drivers/mtd/nand/raw/mtk_nand.c b/drivers/mtd/nand/raw/mtk_nand.c
> > index b6b4602f5132..4fbb0c6ecae3 100644
> > --- a/drivers/mtd/nand/raw/mtk_nand.c
> > +++ b/drivers/mtd/nand/raw/mtk_nand.c
> > @@ -508,7 +508,8 @@ static int mtk_nfc_setup_data_interface(struct nand_chip *chip, int csline,
> >  {
> >  	struct mtk_nfc *nfc = nand_get_controller_data(chip);
> >  	const struct nand_sdr_timings *timings;
> > -	u32 rate, tpoecs, tprecs, tc2r, tw2r, twh, twst, trlt;
> > +	u32 rate, tpoecs, tprecs, tc2r, tw2r, twh, twst = 0, trlt = 0;
> > +	u32 thold;
> >  
> >  	timings = nand_get_sdr_timings(conf);
> >  	if (IS_ERR(timings))
> > @@ -544,11 +545,28 @@ static int mtk_nfc_setup_data_interface(struct nand_chip *chip, int csline,
> >  	twh = DIV_ROUND_UP(twh * rate, 1000000) - 1;
> >  	twh &= 0xf;
> >  
> > -	twst = timings->tWP_min / 1000;
> > +	/* Calculate real WE#/RE# hold time in nanosecond */
> > +	thold = (twh + 1) * 1000000 / rate;
> > +	/* nanosecond to picosecond */
> > +	thold *= 1000;
> > +
> > +	/**
> 
>         /*
> 
> > +	 * WE# low level time should be expaned to meet WE# pulse time
> > +	 * and WE# cycle time at the same time.
> > +	 */
> > +	if (thold < timings->tWC_min)
> > +		twst = timings->tWC_min - thold;
> > +	twst = max(timings->tWP_min, twst) / 1000;
> >  	twst = DIV_ROUND_UP(twst * rate, 1000000) - 1;
> >  	twst &= 0xf;
> >  
> > -	trlt = max(timings->tREA_max, timings->tRP_min) / 1000;
> > +	/**
> 
> Ditto
OK.

> 
> > +	 * RE# low level time should be expaned to meet RE# pulse time,
> > +	 * RE# access time and RE# cycle time at the same time.
> > +	 */
> > +	if (thold < timings->tRC_min)
> > +		trlt = timings->tRC_min - thold;
> > +	trlt = max3(trlt, timings->tREA_max, timings->tRP_min) / 1000;
> >  	trlt = DIV_ROUND_UP(trlt * rate, 1000000) - 1;
> >  	trlt &= 0xf;
> >  
> 
> With this fixed:
> 
> Reviewed-by: Miquel Raynal <miquel.raynal@bootlin.com>
> 
> 
> Thanks,
> Miquèl

Thanks,
Xiaolei


______________________________________________________
Linux MTD discussion mailing list
http://lists.infradead.org/mailman/listinfo/linux-mtd/

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

* Re: [PATCH v2 1/5] mtd: rawnand: mtk: Correct low level time calculation of r/w cycle
@ 2019-05-05  7:12       ` xiaolei li
  0 siblings, 0 replies; 33+ messages in thread
From: xiaolei li @ 2019-05-05  7:12 UTC (permalink / raw)
  To: Miquel Raynal; +Cc: richard, linux-mtd, linux-mediatek, srv_heupstream, stable

On Tue, 2019-04-30 at 13:59 +0200, Miquel Raynal wrote:
> Hi Xiaolei,
> 
> Xiaolei Li <xiaolei.li@mediatek.com> wrote on Tue, 30 Apr 2019 18:02:46
> +0800:
> 
> > At present, the flow of calculating AC timing of read/write cycle in SDR
> > mode is that:
> > At first, calculate high hold time which is valid for both read and write
> > cycle using the max value between tREH_min and tWH_min.
> > Secondly, calculate WE# pulse width using tWP_min.
> > Thridly, calculate RE# pulse width using the bigger one between tREA_max
> > and tRP_min.
> > 
> > But NAND SPEC shows that Controller should also meet write/read cycle time.
> > That is write cycle time should be more than tWC_min and read cycle should
> > be more than tRC_min. Obviously, we do not achieve that now.
> > 
> > This patch corrects the low level time calculation to meet minimum
> > read/write cycle time required. After getting the high hold time, WE# low
> > level time will be promised to meet tWP_min and tWC_min requirement,
> > and RE# low level time will be promised to meet tREA_max, tRP_min and
> > tRC_min requirement.
> > 
> > Fixes: edfee3619c49 ("mtd: nand: mtk: add ->setup_data_interface() hook")
> > Cc: stable@vger.kernel.org
> > Signed-off-by: Xiaolei Li <xiaolei.li@mediatek.com>
> > ---
> >  drivers/mtd/nand/raw/mtk_nand.c | 24 +++++++++++++++++++++---
> >  1 file changed, 21 insertions(+), 3 deletions(-)
> > 
> > diff --git a/drivers/mtd/nand/raw/mtk_nand.c b/drivers/mtd/nand/raw/mtk_nand.c
> > index b6b4602f5132..4fbb0c6ecae3 100644
> > --- a/drivers/mtd/nand/raw/mtk_nand.c
> > +++ b/drivers/mtd/nand/raw/mtk_nand.c
> > @@ -508,7 +508,8 @@ static int mtk_nfc_setup_data_interface(struct nand_chip *chip, int csline,
> >  {
> >  	struct mtk_nfc *nfc = nand_get_controller_data(chip);
> >  	const struct nand_sdr_timings *timings;
> > -	u32 rate, tpoecs, tprecs, tc2r, tw2r, twh, twst, trlt;
> > +	u32 rate, tpoecs, tprecs, tc2r, tw2r, twh, twst = 0, trlt = 0;
> > +	u32 thold;
> >  
> >  	timings = nand_get_sdr_timings(conf);
> >  	if (IS_ERR(timings))
> > @@ -544,11 +545,28 @@ static int mtk_nfc_setup_data_interface(struct nand_chip *chip, int csline,
> >  	twh = DIV_ROUND_UP(twh * rate, 1000000) - 1;
> >  	twh &= 0xf;
> >  
> > -	twst = timings->tWP_min / 1000;
> > +	/* Calculate real WE#/RE# hold time in nanosecond */
> > +	thold = (twh + 1) * 1000000 / rate;
> > +	/* nanosecond to picosecond */
> > +	thold *= 1000;
> > +
> > +	/**
> 
>         /*
> 
> > +	 * WE# low level time should be expaned to meet WE# pulse time
> > +	 * and WE# cycle time at the same time.
> > +	 */
> > +	if (thold < timings->tWC_min)
> > +		twst = timings->tWC_min - thold;
> > +	twst = max(timings->tWP_min, twst) / 1000;
> >  	twst = DIV_ROUND_UP(twst * rate, 1000000) - 1;
> >  	twst &= 0xf;
> >  
> > -	trlt = max(timings->tREA_max, timings->tRP_min) / 1000;
> > +	/**
> 
> Ditto
OK.

> 
> > +	 * RE# low level time should be expaned to meet RE# pulse time,
> > +	 * RE# access time and RE# cycle time at the same time.
> > +	 */
> > +	if (thold < timings->tRC_min)
> > +		trlt = timings->tRC_min - thold;
> > +	trlt = max3(trlt, timings->tREA_max, timings->tRP_min) / 1000;
> >  	trlt = DIV_ROUND_UP(trlt * rate, 1000000) - 1;
> >  	trlt &= 0xf;
> >  
> 
> With this fixed:
> 
> Reviewed-by: Miquel Raynal <miquel.raynal@bootlin.com>
> 
> 
> Thanks,
> Miquèl

Thanks,
Xiaolei

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

* Re: [PATCH v2 2/5] mtd: rawnand: mtk: Improve data sampling timing for read cycle
  2019-04-30 12:11     ` Miquel Raynal
  (?)
@ 2019-05-05  7:29     ` xiaolei li
  -1 siblings, 0 replies; 33+ messages in thread
From: xiaolei li @ 2019-05-05  7:29 UTC (permalink / raw)
  To: Miquel Raynal; +Cc: richard, linux-mediatek, linux-mtd, srv_heupstream

On Tue, 2019-04-30 at 14:11 +0200, Miquel Raynal wrote:
> Hi Xiaolei,
> 
> Xiaolei Li <xiaolei.li@mediatek.com> wrote on Tue, 30 Apr 2019 18:02:47
> +0800:
> 
> > Currently, we expand RE# low level time by choosing the max value
> > between RE# pulse width and RE# access time, and sample data at the
> > rising edge of RE#.
> > 
> > Then, if RE# access time is bigger than RE# pulse width, the real
> > read cycle time may be more than NAND SPEC required. This makes
> > read performance be worse than that expected.
> > 
> > This patch improves data sampling timing by calculating RE# low level
> > time according to RE# pulse width. If RE# access time is bigger than
> > RE# pulse width, then delay sampling data timing.
> > 
> > The result of contrast test base on MT2712 evaluat board is as follow.
> > 
> > nand: Micron MT29F16G08ADBCAH4
> > nand: 2048 MiB, SLC, erase size: 256 KiB, page size: 4096, OOB size: 224
> > NFI 2x clock rate: 124800000 HZ.
> > 
> > Read speed without this patch:
> > mtd_speedtest: page read speed is 14012 KiB/s
> > mtd_speedtest: 2 page read speed is 14860 KiB/s
> > 
> > Read speed with this patch:
> > mtd_speedtest: page read speed is 18724 KiB/s
> > mtd_speedtest: 2 page read speed is 18713 KiB/s
> > 
> > Signed-off-by: Xiaolei Li <xiaolei.li@mediatek.com>
> > ---
> >  drivers/mtd/nand/raw/mtk_nand.c | 46 ++++++++++++++++++++++++++-------
> >  1 file changed, 36 insertions(+), 10 deletions(-)
> > 
> > diff --git a/drivers/mtd/nand/raw/mtk_nand.c b/drivers/mtd/nand/raw/mtk_nand.c
> > index 4fbb0c6ecae3..e90c38c6f835 100644
> > --- a/drivers/mtd/nand/raw/mtk_nand.c
> > +++ b/drivers/mtd/nand/raw/mtk_nand.c
> > @@ -87,6 +87,10 @@
> >  #define NFI_FDMM(x)		(0xA4 + (x) * sizeof(u32) * 2)
> >  #define NFI_FDM_MAX_SIZE	(8)
> >  #define NFI_FDM_MIN_SIZE	(1)
> > +#define NFI_DEBUG_CON1		(0x220)
> > +#define		STROBE_MASK		GENMASK(4, 3)
> > +#define		STROBE_SHIFT		(3)
> > +#define		MAX_STROBE_DLY		(3)
> >  #define NFI_MASTER_STA		(0x224)
> >  #define		MASTER_STA_MASK		(0x0FFF)
> >  #define NFI_EMPTY_THRESH	(0x23C)
> > @@ -509,7 +513,7 @@ static int mtk_nfc_setup_data_interface(struct nand_chip *chip, int csline,
> >  	struct mtk_nfc *nfc = nand_get_controller_data(chip);
> >  	const struct nand_sdr_timings *timings;
> >  	u32 rate, tpoecs, tprecs, tc2r, tw2r, twh, twst = 0, trlt = 0;
> > -	u32 thold;
> > +	u32 temp, tsel = 0;
> >  
> >  	timings = nand_get_sdr_timings(conf);
> >  	if (IS_ERR(timings))
> > @@ -546,30 +550,52 @@ static int mtk_nfc_setup_data_interface(struct nand_chip *chip, int csline,
> >  	twh &= 0xf;
> >  
> >  	/* Calculate real WE#/RE# hold time in nanosecond */
> > -	thold = (twh + 1) * 1000000 / rate;
> > +	temp = (twh + 1) * 1000000 / rate;
> >  	/* nanosecond to picosecond */
> > -	thold *= 1000;
> > +	temp *= 1000;
> >  
> >  	/**
> >  	 * WE# low level time should be expaned to meet WE# pulse time
> >  	 * and WE# cycle time at the same time.
> >  	 */
> > -	if (thold < timings->tWC_min)
> > -		twst = timings->tWC_min - thold;
> > +	if (temp < timings->tWC_min)
> > +		twst = timings->tWC_min - temp;
> >  	twst = max(timings->tWP_min, twst) / 1000;
> >  	twst = DIV_ROUND_UP(twst * rate, 1000000) - 1;
> >  	twst &= 0xf;
> >  
> >  	/**
> > -	 * RE# low level time should be expaned to meet RE# pulse time,
> > -	 * RE# access time and RE# cycle time at the same time.
> > +	 * RE# low level time should be expaned to meet RE# pulse time
> > +	 * and RE# cycle time at the same time.
> >  	 */
> > -	if (thold < timings->tRC_min)
> > -		trlt = timings->tRC_min - thold;
> > -	trlt = max3(trlt, timings->tREA_max, timings->tRP_min) / 1000;
> > +	if (temp < timings->tRC_min)
> > +		trlt = timings->tRC_min - temp;
> > +	trlt = max(trlt, timings->tRP_min) / 1000;
> >  	trlt = DIV_ROUND_UP(trlt * rate, 1000000) - 1;
> >  	trlt &= 0xf;
> >  
> > +	/**
> 
>         /*
> 
> > +	 * Calculate strobe select timing.
> > +	 * If RE# access time is bigger than RE# pulse time,
> > +	 * delay sampling data timing.
> > +	 */
> > +	temp = (trlt + 1) * 1000000 / rate;
> 
> You could precise what unit conversion you do here.
OK. Unit here is nanosecond. I will describe it in next patch version.

> 
> > +	/* nanosecond to picosecond */
> > +	temp *= 1000;
> > +	if (temp < timings->tREA_max) {
> > +		tsel = timings->tREA_max / 1000;
> > +		tsel = DIV_ROUND_UP(tsel * rate, 1000000);
> > +		tsel -= (trlt + 1);
> > +		if (tsel > MAX_STROBE_DLY) {
> > +			trlt += tsel - MAX_STROBE_DLY;
> > +			tsel = MAX_STROBE_DLY;
> > +		}
> > +	}
> > +	temp = nfi_readl(nfc, NFI_DEBUG_CON1);
> > +	temp &= ~STROBE_MASK;
> > +	temp |= tsel << STROBE_SHIFT;
> > +	nfi_writel(nfc, temp, NFI_DEBUG_CON1);
> > +
> >  	/*
> >  	 * ACCON: access timing control register
> >  	 * -------------------------------------
> 
> With this:
> 
> Reviewed-by: Miquel Raynal <miquel.raynal@bootlin.com>
> 
> 
> Thanks,
> Miquèl

Thanks,
Xiaolei



______________________________________________________
Linux MTD discussion mailing list
http://lists.infradead.org/mailman/listinfo/linux-mtd/

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

* Re: [PATCH v2 5/5] mtd: rawnand: mtk: Setup empty page threshold correctly
  2019-04-30 12:17   ` Miquel Raynal
@ 2019-05-05  7:50       ` xiaolei li
  0 siblings, 0 replies; 33+ messages in thread
From: xiaolei li @ 2019-05-05  7:50 UTC (permalink / raw)
  To: Miquel Raynal; +Cc: richard, linux-mediatek, linux-mtd, srv_heupstream

Hi Miquel,

On Tue, 2019-04-30 at 14:17 +0200, Miquel Raynal wrote:
> Hi Xiaolei,
> 
> Xiaolei Li <xiaolei.li@mediatek.com> wrote on Tue, 30 Apr 2019 18:02:50
> +0800:
> 
> > MTK NAND Controller has the ability to check whether read data are
> > mostly 0xff by comparing zero bit count of read data with empty
> > threshold automatically.
> > 
> > But now we never set this threshold and always make it be default value
> > which is 10.
> > 
> > This patch fixes this problem by setting empty threshold as the product
> > of read sector count and ECC strength.
> 
> Are you sure it is not a per-sector value?
Frankly, I also think it should be a per-sector value.

But MTK NAND Controller cannot check 0xff by sector. It counts zero bit
through all read data. So, I set the empty threshold as the product of
read sector count and ECC strength.

> 
> Did you use nandbiterrs to validate?
Yes. I ever did nandbiterrs test, and it passed.

> 
> > 
> > Fixes: 1d6b1e464950 ("mtd: mediatek: driver for MTK Smart Device")
> > Signed-off-by: Xiaolei Li <xiaolei.li@mediatek.com>
> > ---
> >  drivers/mtd/nand/raw/mtk_nand.c | 9 +++++++++
> >  1 file changed, 9 insertions(+)
> > 
> > diff --git a/drivers/mtd/nand/raw/mtk_nand.c b/drivers/mtd/nand/raw/mtk_nand.c
> > index 48759af5c058..b56965328771 100644
> > --- a/drivers/mtd/nand/raw/mtk_nand.c
> > +++ b/drivers/mtd/nand/raw/mtk_nand.c
> > @@ -94,6 +94,7 @@
> >  #define NFI_MASTER_STA		(0x224)
> >  #define		MASTER_STA_MASK		(0x0FFF)
> >  #define NFI_EMPTY_THRESH	(0x23C)
> > +#define		EMPTY_THRESH_MASK	GENMASK(7, 0)
> >  
> >  #define MTK_NAME		"mtk-nand"
> >  #define KB(x)			((x) * 1024UL)
> > @@ -947,6 +948,14 @@ static int mtk_nfc_read_subpage(struct mtd_info *mtd, struct nand_chip *chip,
> >  		return -EINVAL;
> >  	}
> >  
> > +	/**
> 
>         /*
> 
> > +	 * Setup empty threshold as the product of sector count
> > +	 * and ECC strength
> > +	 */
> > +	reg = sectors * chip->ecc.strength;
> > +	reg = min_t(unsigned int, reg, EMPTY_THRESH_MASK);
> > +	nfi_writel(nfc, reg, NFI_EMPTY_THRESH);
> > +
> >  	reg = nfi_readw(nfc, NFI_CNFG);
> >  	reg |= CNFG_READ_EN | CNFG_DMA_BURST_EN | CNFG_AHB;
> >  	if (!raw) {
> 
> 
> Thanks,
> Miquèl

Thanks,
Xiaolei


______________________________________________________
Linux MTD discussion mailing list
http://lists.infradead.org/mailman/listinfo/linux-mtd/

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

* Re: [PATCH v2 5/5] mtd: rawnand: mtk: Setup empty page threshold correctly
@ 2019-05-05  7:50       ` xiaolei li
  0 siblings, 0 replies; 33+ messages in thread
From: xiaolei li @ 2019-05-05  7:50 UTC (permalink / raw)
  To: Miquel Raynal
  Cc: richard-/L3Ra7n9ekc,
	linux-mediatek-IAPFreCvJWM7uuMidbF8XUB+6BGkLq7r,
	linux-mtd-IAPFreCvJWM7uuMidbF8XUB+6BGkLq7r,
	srv_heupstream-NuS5LvNUpcJWk0Htik3J/w

Hi Miquel,

On Tue, 2019-04-30 at 14:17 +0200, Miquel Raynal wrote:
> Hi Xiaolei,
> 
> Xiaolei Li <xiaolei.li@mediatek.com> wrote on Tue, 30 Apr 2019 18:02:50
> +0800:
> 
> > MTK NAND Controller has the ability to check whether read data are
> > mostly 0xff by comparing zero bit count of read data with empty
> > threshold automatically.
> > 
> > But now we never set this threshold and always make it be default value
> > which is 10.
> > 
> > This patch fixes this problem by setting empty threshold as the product
> > of read sector count and ECC strength.
> 
> Are you sure it is not a per-sector value?
Frankly, I also think it should be a per-sector value.

But MTK NAND Controller cannot check 0xff by sector. It counts zero bit
through all read data. So, I set the empty threshold as the product of
read sector count and ECC strength.

> 
> Did you use nandbiterrs to validate?
Yes. I ever did nandbiterrs test, and it passed.

> 
> > 
> > Fixes: 1d6b1e464950 ("mtd: mediatek: driver for MTK Smart Device")
> > Signed-off-by: Xiaolei Li <xiaolei.li@mediatek.com>
> > ---
> >  drivers/mtd/nand/raw/mtk_nand.c | 9 +++++++++
> >  1 file changed, 9 insertions(+)
> > 
> > diff --git a/drivers/mtd/nand/raw/mtk_nand.c b/drivers/mtd/nand/raw/mtk_nand.c
> > index 48759af5c058..b56965328771 100644
> > --- a/drivers/mtd/nand/raw/mtk_nand.c
> > +++ b/drivers/mtd/nand/raw/mtk_nand.c
> > @@ -94,6 +94,7 @@
> >  #define NFI_MASTER_STA		(0x224)
> >  #define		MASTER_STA_MASK		(0x0FFF)
> >  #define NFI_EMPTY_THRESH	(0x23C)
> > +#define		EMPTY_THRESH_MASK	GENMASK(7, 0)
> >  
> >  #define MTK_NAME		"mtk-nand"
> >  #define KB(x)			((x) * 1024UL)
> > @@ -947,6 +948,14 @@ static int mtk_nfc_read_subpage(struct mtd_info *mtd, struct nand_chip *chip,
> >  		return -EINVAL;
> >  	}
> >  
> > +	/**
> 
>         /*
> 
> > +	 * Setup empty threshold as the product of sector count
> > +	 * and ECC strength
> > +	 */
> > +	reg = sectors * chip->ecc.strength;
> > +	reg = min_t(unsigned int, reg, EMPTY_THRESH_MASK);
> > +	nfi_writel(nfc, reg, NFI_EMPTY_THRESH);
> > +
> >  	reg = nfi_readw(nfc, NFI_CNFG);
> >  	reg |= CNFG_READ_EN | CNFG_DMA_BURST_EN | CNFG_AHB;
> >  	if (!raw) {
> 
> 
> Thanks,
> Miquèl

Thanks,
Xiaolei


_______________________________________________
Linux-mediatek mailing list
Linux-mediatek@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-mediatek

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

* Re: [PATCH v2 1/5] mtd: rawnand: mtk: Correct low level time calculation of r/w cycle
  2019-05-05  7:06       ` xiaolei li
@ 2019-05-06  8:11         ` Miquel Raynal
  -1 siblings, 0 replies; 33+ messages in thread
From: Miquel Raynal @ 2019-05-06  8:11 UTC (permalink / raw)
  To: xiaolei li; +Cc: Sasha Levin, richard, linux-mtd, stable

Hi xiaolei,

xiaolei li <xiaolei.li@mediatek.com> wrote on Sun, 5 May 2019 15:06:40
+0800:

> Hi Sasha,
> 
> I am not sure if it is caused by raw NAND code path change.
> 
> Raw NAND code was moved from mtd/nand to mtd/nand/raw subdirectory since
> kernel v4.17.
> 
> The fixing commit: edfee3619c49 mtd: nand: mtk:
> add->setup_data_interface() hook exists before kernel v4.17.
> 
> @Miquel, do you know if some other raw NAND driver ever encountered this
> case? Thanks.

Don't know. Just checkout a 4.14.114 and try to apply the patch :)

> 
> On Tue, 2019-04-30 at 10:32 +0000, Sasha Levin wrote:
> > Hi,
> > 
> > [This is an automated email]
> > 
> > This commit has been processed because it contains a "Fixes:" tag,
> > fixing commit: edfee3619c49 mtd: nand: mtk: add ->setup_data_interface() hook.


Thanks,
Miquèl

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

* Re: [PATCH v2 1/5] mtd: rawnand: mtk: Correct low level time calculation of r/w cycle
@ 2019-05-06  8:11         ` Miquel Raynal
  0 siblings, 0 replies; 33+ messages in thread
From: Miquel Raynal @ 2019-05-06  8:11 UTC (permalink / raw)
  To: xiaolei li; +Cc: Sasha Levin, richard, linux-mtd, stable

Hi xiaolei,

xiaolei li <xiaolei.li@mediatek.com> wrote on Sun, 5 May 2019 15:06:40
+0800:

> Hi Sasha,
> 
> I am not sure if it is caused by raw NAND code path change.
> 
> Raw NAND code was moved from mtd/nand to mtd/nand/raw subdirectory since
> kernel v4.17.
> 
> The fixing commit: edfee3619c49 mtd: nand: mtk:
> add->setup_data_interface() hook exists before kernel v4.17.
> 
> @Miquel, do you know if some other raw NAND driver ever encountered this
> case? Thanks.

Don't know. Just checkout a 4.14.114 and try to apply the patch :)

> 
> On Tue, 2019-04-30 at 10:32 +0000, Sasha Levin wrote:
> > Hi,
> > 
> > [This is an automated email]
> > 
> > This commit has been processed because it contains a "Fixes:" tag,
> > fixing commit: edfee3619c49 mtd: nand: mtk: add ->setup_data_interface() hook.


Thanks,
Miquèl

______________________________________________________
Linux MTD discussion mailing list
http://lists.infradead.org/mailman/listinfo/linux-mtd/

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

end of thread, other threads:[~2019-05-06  8:12 UTC | newest]

Thread overview: 33+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2019-04-30 10:02 [PATCH v2 0/5] MTK NAND driver improvements and fixes Xiaolei Li
2019-04-30 10:02 ` Xiaolei Li
2019-04-30 10:02 ` [PATCH v2 1/5] mtd: rawnand: mtk: Correct low level time calculation of r/w cycle Xiaolei Li
2019-04-30 10:02   ` Xiaolei Li
2019-04-30 10:02   ` Xiaolei Li
2019-04-30 10:32   ` Sasha Levin
2019-05-05  7:06     ` xiaolei li
2019-05-05  7:06       ` xiaolei li
2019-05-06  8:11       ` Miquel Raynal
2019-05-06  8:11         ` Miquel Raynal
2019-04-30 11:59   ` Miquel Raynal
2019-04-30 11:59     ` Miquel Raynal
2019-04-30 11:59     ` Miquel Raynal
2019-05-05  7:12     ` xiaolei li
2019-05-05  7:12       ` xiaolei li
2019-05-05  7:12       ` xiaolei li
2019-04-30 10:02 ` [PATCH v2 2/5] mtd: rawnand: mtk: Improve data sampling timing for read cycle Xiaolei Li
2019-04-30 10:02   ` Xiaolei Li
2019-04-30 12:11   ` Miquel Raynal
2019-04-30 12:11     ` Miquel Raynal
2019-05-05  7:29     ` xiaolei li
2019-04-30 10:02 ` [PATCH v2 3/5] mtd: rawnand: mtk: Add validity check for CE# pin setting Xiaolei Li
2019-04-30 10:02   ` Xiaolei Li
2019-04-30 10:02 ` [PATCH v2 4/5] mtd: rawnand: mtk: Fix wrongly assigned OOB buffer pointer issue Xiaolei Li
2019-04-30 10:02   ` Xiaolei Li
2019-04-30 10:02 ` [PATCH v2 5/5] mtd: rawnand: mtk: Setup empty page threshold correctly Xiaolei Li
2019-04-30 10:02   ` Xiaolei Li
2019-04-30 12:17   ` Miquel Raynal
2019-05-05  7:50     ` xiaolei li
2019-05-05  7:50       ` xiaolei li
2019-04-30 12:08 ` [PATCH v2 0/5] MTK NAND driver improvements and fixes Miquel Raynal
2019-04-30 12:08   ` Miquel Raynal
2019-05-05  7:08   ` xiaolei li

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