diff mbox series

[2/5] mtd: rawnand: mtk: Improve data sampling timing for read cycle

Message ID 20190429063834.45967-3-xiaolei.li@mediatek.com (mailing list archive)
State New, archived
Headers show
Series MTK NAND driver improvements and fixes | expand

Commit Message

xiaolei li April 29, 2019, 6:38 a.m. UTC
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.

Test tool: mtd_speedtest.ko

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 | 22 +++++++++++++++++++++-
 1 file changed, 21 insertions(+), 1 deletion(-)

Comments

Miquel Raynal April 29, 2019, 9:10 a.m. UTC | #1
Hi Xiaolei,

Xiaolei Li <xiaolei.li@mediatek.com> wrote on Mon, 29 Apr 2019 14:38:31
+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.
> 
> Test tool: mtd_speedtest.ko
> 
> 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 | 22 +++++++++++++++++++++-
>  1 file changed, 21 insertions(+), 1 deletion(-)
> 
> diff --git a/drivers/mtd/nand/raw/mtk_nand.c b/drivers/mtd/nand/raw/mtk_nand.c
> index dd855f860a4b..a2f7af536380 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,6 +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 tsel, reg;
>  
>  	timings = nand_get_sdr_timings(conf);
>  	if (IS_ERR(timings))
> @@ -556,10 +561,25 @@ static int mtk_nfc_setup_data_interface(struct nand_chip *chip, int csline,
>  	if ((twh + 1) * 1000000 / rate < timings->tRC_min / 1000)
>  		trlt = (timings->tRC_min / 1000 - (twh + 1) * 1000000 / rate)
>  			* 1000;
> -	trlt = max3(trlt, timings->tREA_max, timings->tRP_min) / 1000;
> +	trlt = max(trlt, timings->tRP_min) / 1000;
>  	trlt = DIV_ROUND_UP(trlt * rate, 1000000) - 1;
>  	trlt &= 0xf;
>  
> +	/* Calculate strobe sel */
> +	reg = nfi_readl(nfc, NFI_DEBUG_CON1);
> +	reg &= ~STROBE_MASK;
> +	if ((trlt + 1) * 1000000 / rate < timings->tREA_max / 1000) {

Please do the calculation and condition in separate step, this is
hardly readable. Maybe you can explain it with a comment as well.

> +		tsel = timings->tREA_max / 1000;
> +		tsel = DIV_ROUND_UP(tsel * rate, 1000000);

Are you sure tsel * rate cannot overflow?

> +		tsel -= (trlt + 1);
> +		if (tsel > MAX_STROBE_DLY) {
> +			trlt += tsel - MAX_STROBE_DLY;
> +			tsel = MAX_STROBE_DLY;
> +		}
> +		reg |= tsel << STROBE_SHIFT;
> +	}
> +	nfi_writel(nfc, reg, NFI_DEBUG_CON1);
> +
>  	/*
>  	 * ACCON: access timing control register
>  	 * -------------------------------------




Thanks,
Miquèl
xiaolei li April 29, 2019, 9:49 a.m. UTC | #2
On Mon, 2019-04-29 at 11:10 +0200, Miquel Raynal wrote:
> Hi Xiaolei,
> 
> Xiaolei Li <xiaolei.li@mediatek.com> wrote on Mon, 29 Apr 2019 14:38:31
> +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.
> > 
> > Test tool: mtd_speedtest.ko
> > 
> > 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 | 22 +++++++++++++++++++++-
> >  1 file changed, 21 insertions(+), 1 deletion(-)
> > 
> > diff --git a/drivers/mtd/nand/raw/mtk_nand.c b/drivers/mtd/nand/raw/mtk_nand.c
> > index dd855f860a4b..a2f7af536380 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,6 +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 tsel, reg;
> >  
> >  	timings = nand_get_sdr_timings(conf);
> >  	if (IS_ERR(timings))
> > @@ -556,10 +561,25 @@ static int mtk_nfc_setup_data_interface(struct nand_chip *chip, int csline,
> >  	if ((twh + 1) * 1000000 / rate < timings->tRC_min / 1000)
> >  		trlt = (timings->tRC_min / 1000 - (twh + 1) * 1000000 / rate)
> >  			* 1000;
> > -	trlt = max3(trlt, timings->tREA_max, timings->tRP_min) / 1000;
> > +	trlt = max(trlt, timings->tRP_min) / 1000;
> >  	trlt = DIV_ROUND_UP(trlt * rate, 1000000) - 1;
> >  	trlt &= 0xf;
> >  
> > +	/* Calculate strobe sel */
> > +	reg = nfi_readl(nfc, NFI_DEBUG_CON1);
> > +	reg &= ~STROBE_MASK;
> > +	if ((trlt + 1) * 1000000 / rate < timings->tREA_max / 1000) {
> 
> Please do the calculation and condition in separate step, this is
> hardly readable. Maybe you can explain it with a comment as well.
Sorry to trouble you.
I will change it as your advice in next patch version.

> 
> > +		tsel = timings->tREA_max / 1000;
> > +		tsel = DIV_ROUND_UP(tsel * rate, 1000000);
> 
> Are you sure tsel * rate cannot overflow?
Thanks for your reminding.
The unit of tsel here is nano second, and the unit of rate is KHZ.
There should be no overflowing here.

> 
> > +		tsel -= (trlt + 1);
> > +		if (tsel > MAX_STROBE_DLY) {
> > +			trlt += tsel - MAX_STROBE_DLY;
> > +			tsel = MAX_STROBE_DLY;
> > +		}
> > +		reg |= tsel << STROBE_SHIFT;
> > +	}
> > +	nfi_writel(nfc, reg, NFI_DEBUG_CON1);
> > +
> >  	/*
> >  	 * ACCON: access timing control register
> >  	 * -------------------------------------
> 
> 
> 
> 
> Thanks,
> Miquèl
diff mbox series

Patch

diff --git a/drivers/mtd/nand/raw/mtk_nand.c b/drivers/mtd/nand/raw/mtk_nand.c
index dd855f860a4b..a2f7af536380 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,6 +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 tsel, reg;
 
 	timings = nand_get_sdr_timings(conf);
 	if (IS_ERR(timings))
@@ -556,10 +561,25 @@  static int mtk_nfc_setup_data_interface(struct nand_chip *chip, int csline,
 	if ((twh + 1) * 1000000 / rate < timings->tRC_min / 1000)
 		trlt = (timings->tRC_min / 1000 - (twh + 1) * 1000000 / rate)
 			* 1000;
-	trlt = max3(trlt, timings->tREA_max, timings->tRP_min) / 1000;
+	trlt = max(trlt, timings->tRP_min) / 1000;
 	trlt = DIV_ROUND_UP(trlt * rate, 1000000) - 1;
 	trlt &= 0xf;
 
+	/* Calculate strobe sel */
+	reg = nfi_readl(nfc, NFI_DEBUG_CON1);
+	reg &= ~STROBE_MASK;
+	if ((trlt + 1) * 1000000 / rate < timings->tREA_max / 1000) {
+		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;
+		}
+		reg |= tsel << STROBE_SHIFT;
+	}
+	nfi_writel(nfc, reg, NFI_DEBUG_CON1);
+
 	/*
 	 * ACCON: access timing control register
 	 * -------------------------------------