[PATCH 0/9] add EDO feature for gpmi-nand driver

[PATCH 0/9] add EDO feature for gpmi-nand driver

[Huang Shijie]

The ONFI nand supports the EDO (extended data out) feature in 
asynchronous mode when the host controller(such as gpmi-nand)
uses a tRC of less then 30ns.

The gpmi can supports this EDO feature.
This patch set adds the EDO support the gpmi-nand driver.

patch 1 ~ patch 2: 
	 These two patches provide the infrastructure for the EDO feature.
         They add necessary MTD helpers for the ONFI nand set/get features,
	 and the help to get the supportted timing mode.
	 These two patches are new version. The init version has been
	 reviewed by Vikram & Florian.

patch 3 ~ patch 7:
         These patches are clean-ups for the gpmi-nand's timing code.
	 Also they make the some preparations for the EDO patch.

patch 8: add the EDO feature to the gpmi-nand.


patch 9: a small optimization for the timing. 
         only set the timing registers one time.

I tested this patch set on the IMX6Q-arm2 board with several Micron's
ONFI nand chips. Some chips only can supports to mode 4, some chips
can supports to mode 5.

The performance is much improved. Take Micron MT29F32G08MAA for example
(in mode 5, 100MHz):

1) The test result BEFORE we add the EDO feature:
=================================================
mtd_speedtest: MTD device: 2
mtd_speedtest: MTD device size 209715200, eraseblock size 524288,
			page size 4096, count of eraseblocks 400,
			pages per eraseblock 128, OOB size 218
.......................................
mtd_speedtest: testing eraseblock read speed
mtd_speedtest: eraseblock read speed is 3632 KiB/s
.......................................
mtd_speedtest: testing page read speed
mtd_speedtest: page read speed is 3554 KiB/s
.......................................
mtd_speedtest: testing 2 page read speed
mtd_speedtest: 2 page read speed is 3592 KiB/s
.......................................
=================================================

2) The test result AFTER we add the EDO feature:
=================================================
mtd_speedtest: MTD device: 2
mtd_speedtest: MTD device size 209715200, eraseblock size 524288,
			page size 4096, count of eraseblocks 400,
			pages per eraseblock 128, OOB size 218
.......................................
mtd_speedtest: testing eraseblock read speed
mtd_speedtest: eraseblock read speed is 19555 KiB/s
.......................................
mtd_speedtest: testing page read speed
mtd_speedtest: page read speed is 17319 KiB/s
.......................................
mtd_speedtest: testing 2 page read speed
mtd_speedtest: 2 page read speed is 18339 KiB/s
.......................................
=================================================

The read data performance is much improved by more then 5 times.

Huang Shijie (9):
  mtd: add helpers to set/get features for ONFI nand
  mtd: add helpers to get the supportted ONFI timing mode
  mtd: gpmi: add a new field for HW_GPMI_TIMING1
  mtd: gpmi: do not get the clock frequency in gpmi_begin()
  mtd: gpmi: add a new field for HW_GPMI_CTRL1
  mtd: gpmi: simplify the setting DLL code
  mtd: gpmi: do not set the default values for the extra clocks
  mtd: gpmi: add EDO feature for imx6q
  mtd: gpmi: initialize the timing registers only one time

 drivers/mtd/nand/gpmi-nand/gpmi-lib.c  |  273 +++++++++++++++++++++++++++++---
 drivers/mtd/nand/gpmi-nand/gpmi-nand.c |   23 ++--
 drivers/mtd/nand/gpmi-nand/gpmi-nand.h |   18 ++
 drivers/mtd/nand/gpmi-nand/gpmi-regs.h |    8 +
 drivers/mtd/nand/nand_base.c           |   50 ++++++
 include/linux/mtd/nand.h               |   39 +++++
 6 files changed, 375 insertions(+), 36 deletions(-)

[PATCH 1/9] mtd: add helpers to set/get features for ONFI nand

[Huang Shijie]

Add the set-features(0xef)/get-features(0xee) helpers for ONFI nand.
Also add the necessary macros.

Signed-off-by: Huang Shijie <b32955@freescale.com>
---
 drivers/mtd/nand/nand_base.c |   50 ++++++++++++++++++++++++++++++++++++++++++
 include/linux/mtd/nand.h     |   14 +++++++++++
 2 files changed, 64 insertions(+), 0 deletions(-)

diff --git a/drivers/mtd/nand/nand_base.c b/drivers/mtd/nand/nand_base.c
index 88f671c..d06a80d 100644
--- a/drivers/mtd/nand/nand_base.c
+++ b/drivers/mtd/nand/nand_base.c
@@ -2700,6 +2700,50 @@ static int nand_block_markbad(struct mtd_info *mtd, loff_t ofs)
 }
 
 /**
+ * nand_onfi_set_features- [REPLACEABLE] set features for ONFI nand
+ * @mtd: MTD device structure
+ * @chip: nand chip info structure
+ * @addr: feature address.
+ * @subfeature_param: the subfeature parameters, a four bytes array.
+ */
+static int nand_onfi_set_features(struct mtd_info *mtd, struct nand_chip *chip,
+			int addr, uint8_t *subfeature_param)
+{
+	int status;
+
+	if (!chip->onfi_version)
+		return -EINVAL;
+
+	chip->cmdfunc(mtd, NAND_CMD_SET_FEATURES, addr, -1);
+	chip->write_buf(mtd, subfeature_param, ONFI_SUBFEATURE_PARAM_LEN);
+	status = chip->waitfunc(mtd, chip);
+	if (status & NAND_STATUS_FAIL)
+		return -EIO;
+	return 0;
+}
+
+/**
+ * nand_onfi_get_features- [REPLACEABLE] get features for ONFI nand
+ * @mtd: MTD device structure
+ * @chip: nand chip info structure
+ * @addr: feature address.
+ * @subfeature_param: the subfeature parameters, a four bytes array.
+ */
+static int nand_onfi_get_features(struct mtd_info *mtd, struct nand_chip *chip,
+			int addr, uint8_t *subfeature_param)
+{
+	if (!chip->onfi_version)
+		return -EINVAL;
+
+	/* clear the sub feature parameters */
+	memset(subfeature_param, 0, ONFI_SUBFEATURE_PARAM_LEN);
+
+	chip->cmdfunc(mtd, NAND_CMD_GET_FEATURES, addr, -1);
+	chip->read_buf(mtd, subfeature_param, ONFI_SUBFEATURE_PARAM_LEN);
+	return 0;
+}
+
+/**
  * nand_suspend - [MTD Interface] Suspend the NAND flash
  * @mtd: MTD device structure
  */
@@ -3223,6 +3267,12 @@ int nand_scan_tail(struct mtd_info *mtd)
 	if (!chip->write_page)
 		chip->write_page = nand_write_page;
 
+	/* set for ONFI nand */
+	if (!chip->onfi_set_features)
+		chip->onfi_set_features = nand_onfi_set_features;
+	if (!chip->onfi_get_features)
+		chip->onfi_get_features = nand_onfi_get_features;
+
 	/*
 	 * Check ECC mode, default to software if 3byte/512byte hardware ECC is
 	 * selected and we have 256 byte pagesize fallback to software ECC
diff --git a/include/linux/mtd/nand.h b/include/linux/mtd/nand.h
index 8f99d36..537f59c 100644
--- a/include/linux/mtd/nand.h
+++ b/include/linux/mtd/nand.h
@@ -92,6 +92,8 @@ extern int nand_unlock(struct mtd_info *mtd, loff_t ofs, uint64_t len);
 #define NAND_CMD_READID		0x90
 #define NAND_CMD_ERASE2		0xd0
 #define NAND_CMD_PARAM		0xec
+#define NAND_CMD_GET_FEATURES	0xee
+#define NAND_CMD_SET_FEATURES	0xef
 #define NAND_CMD_RESET		0xff
 
 #define NAND_CMD_LOCK		0x2a
@@ -229,6 +231,12 @@ typedef enum {
 /* Keep gcc happy */
 struct nand_chip;
 
+/* ONFI feature address */
+#define ONFI_FEATURE_ADDR_TIMING_MODE	0x1
+
+/* ONFI subfeature parameters length */
+#define ONFI_SUBFEATURE_PARAM_LEN	4
+
 struct nand_onfi_params {
 	/* rev info and features block */
 	/* 'O' 'N' 'F' 'I'  */
@@ -452,6 +460,8 @@ struct nand_buffers {
  *			non 0 if ONFI supported.
  * @onfi_params:	[INTERN] holds the ONFI page parameter when ONFI is
  *			supported, 0 otherwise.
+ * @onfi_set_features	[REPLACEABLE] set the features for ONFI nand
+ * @onfi_get_features	[REPLACEABLE] get the features for ONFI nand
  * @ecclayout:		[REPLACEABLE] the default ECC placement scheme
  * @bbt:		[INTERN] bad block table pointer
  * @bbt_td:		[REPLACEABLE] bad block table descriptor for flash
@@ -494,6 +504,10 @@ struct nand_chip {
 	int (*write_page)(struct mtd_info *mtd, struct nand_chip *chip,
 			const uint8_t *buf, int oob_required, int page,
 			int cached, int raw);
+	int (*onfi_set_features)(struct mtd_info *mtd, struct nand_chip *chip,
+			int feature_addr, uint8_t *subfeature_para);
+	int (*onfi_get_features)(struct mtd_info *mtd, struct nand_chip *chip,
+			int feature_addr, uint8_t *subfeature_para);
 
 	int chip_delay;
 	unsigned int options;
-- 
1.7.0.4

[PATCH 2/9] mtd: add helpers to get the supportted ONFI timing mode

[Huang Shijie]

add onfi_get_async_timing_mode() to get the supportted asynchronous
timing mode.

add onfi_get_sync_timing_mode() to get the supportted synchronous
timing mode.

Also add the neccessary macros : the timing modes.

Signed-off-by: Huang Shijie <b32955@freescale.com>
---
 include/linux/mtd/nand.h |   25 +++++++++++++++++++++++++
 1 files changed, 25 insertions(+), 0 deletions(-)

diff --git a/include/linux/mtd/nand.h b/include/linux/mtd/nand.h
index 537f59c..7a7b246 100644
--- a/include/linux/mtd/nand.h
+++ b/include/linux/mtd/nand.h
@@ -231,6 +231,15 @@ typedef enum {
 /* Keep gcc happy */
 struct nand_chip;
 
+/* ONFI timing mode, used in both asynchronous and synchronous mode */
+#define ONFI_TIMING_MODE_0		(1 << 0)
+#define ONFI_TIMING_MODE_1		(1 << 1)
+#define ONFI_TIMING_MODE_2		(1 << 2)
+#define ONFI_TIMING_MODE_3		(1 << 3)
+#define ONFI_TIMING_MODE_4		(1 << 4)
+#define ONFI_TIMING_MODE_5		(1 << 5)
+#define ONFI_TIMING_MODE_UNKNOWN	(1 << 6)
+
 /* ONFI feature address */
 #define ONFI_FEATURE_ADDR_TIMING_MODE	0x1
 
@@ -682,4 +691,20 @@ struct platform_nand_chip *get_platform_nandchip(struct mtd_info *mtd)
 	return chip->priv;
 }
 
+/* return the supported asynchronous timing mode. */
+static inline int onfi_get_async_timing_mode(struct nand_chip *chip)
+{
+	if (!chip->onfi_version)
+		return ONFI_TIMING_MODE_UNKNOWN;
+	return le16_to_cpu(chip->onfi_params.async_timing_mode);
+}
+
+/* return the supported synchronous timing mode. */
+static inline int onfi_get_sync_timing_mode(struct nand_chip *chip)
+{
+	if (!chip->onfi_version)
+		return ONFI_TIMING_MODE_UNKNOWN;
+	return le16_to_cpu(chip->onfi_params.src_sync_timing_mode);
+}
+
 #endif /* __LINUX_MTD_NAND_H */
-- 
1.7.0.4

[PATCH 3/9] mtd: gpmi: add a new field for HW_GPMI_TIMING1

[Huang Shijie]

The gpmi_nfc_compute_hardware_timing{} should contains all the
fields setting for gpmi timing registers. It already contains the fields
for HW_GPMI_TIMING0 and HW_GPMI_CTRL1.

So it is better to add a new field setting for HW_GPMI_TIMING1 in
this data structure. This makes the code more clear in logic.

This patch also changes some comments to make the code more readable.

Signed-off-by: Huang Shijie <b32955@freescale.com>
---
 drivers/mtd/nand/gpmi-nand/gpmi-lib.c  |   17 +++++++++--------
 drivers/mtd/nand/gpmi-nand/gpmi-nand.h |    9 +++++++++
 drivers/mtd/nand/gpmi-nand/gpmi-regs.h |    3 +++
 3 files changed, 21 insertions(+), 8 deletions(-)

diff --git a/drivers/mtd/nand/gpmi-nand/gpmi-lib.c b/drivers/mtd/nand/gpmi-nand/gpmi-lib.c
index 2289cf8..fb9b3c1 100644
--- a/drivers/mtd/nand/gpmi-nand/gpmi-lib.c
+++ b/drivers/mtd/nand/gpmi-nand/gpmi-lib.c
@@ -728,6 +728,7 @@ return_results:
 	hw->address_setup_in_cycles = address_setup_in_cycles;
 	hw->use_half_periods        = dll_use_half_periods;
 	hw->sample_delay_factor     = sample_delay_factor;
+	hw->device_busy_timeout     = 0x500; /* default busy timeout value. */
 
 	/* Return success. */
 	return 0;
@@ -752,26 +753,26 @@ void gpmi_begin(struct gpmi_nand_data *this)
 		goto err_out;
 	}
 
-	/* set ready/busy timeout */
-	writel(0x500 << BP_GPMI_TIMING1_BUSY_TIMEOUT,
-		gpmi_regs + HW_GPMI_TIMING1);
-
 	/* Get the timing information we need. */
 	nfc->clock_frequency_in_hz = clk_get_rate(r->clock[0]);
 	clock_period_in_ns = 1000000000 / nfc->clock_frequency_in_hz;
 
 	gpmi_nfc_compute_hardware_timing(this, &hw);
 
-	/* Set up all the simple timing parameters. */
+	/* [1] Set HW_GPMI_TIMING0 */
 	reg = BF_GPMI_TIMING0_ADDRESS_SETUP(hw.address_setup_in_cycles) |
 		BF_GPMI_TIMING0_DATA_HOLD(hw.data_hold_in_cycles)         |
 		BF_GPMI_TIMING0_DATA_SETUP(hw.data_setup_in_cycles)       ;
 
 	writel(reg, gpmi_regs + HW_GPMI_TIMING0);
 
-	/*
-	 * DLL_ENABLE must be set to 0 when setting RDN_DELAY or HALF_PERIOD.
-	 */
+	/* [2] Set HW_GPMI_TIMING1 */
+	writel(BF_GPMI_TIMING1_BUSY_TIMEOUT(hw.device_busy_timeout),
+		gpmi_regs + HW_GPMI_TIMING1);
+
+	/* [3] The following code is to set the HW_GPMI_CTRL1. */
+
+	/* DLL_ENABLE must be set to 0 when setting RDN_DELAY or HALF_PERIOD. */
 	writel(BM_GPMI_CTRL1_DLL_ENABLE, gpmi_regs + HW_GPMI_CTRL1_CLR);
 
 	/* Clear out the DLL control fields. */
diff --git a/drivers/mtd/nand/gpmi-nand/gpmi-nand.h b/drivers/mtd/nand/gpmi-nand/gpmi-nand.h
index 1f61217..e68bbac 100644
--- a/drivers/mtd/nand/gpmi-nand/gpmi-nand.h
+++ b/drivers/mtd/nand/gpmi-nand/gpmi-nand.h
@@ -189,14 +189,23 @@ struct gpmi_nand_data {
  * @data_setup_in_cycles:      The data setup time, in cycles.
  * @data_hold_in_cycles:       The data hold time, in cycles.
  * @address_setup_in_cycles:   The address setup time, in cycles.
+ * @device_busy_timeout:       The timeout waiting for NAND Ready/Busy,
+ *                             this value is the number of cycles multiplied
+ *                             by 4096.
  * @use_half_periods:          Indicates the clock is running slowly, so the
  *                             NFC DLL should use half-periods.
  * @sample_delay_factor:       The sample delay factor.
  */
 struct gpmi_nfc_hardware_timing {
+	/* for HW_GPMI_TIMING0 */
 	uint8_t  data_setup_in_cycles;
 	uint8_t  data_hold_in_cycles;
 	uint8_t  address_setup_in_cycles;
+
+	/* for HW_GPMI_TIMING1 */
+	uint16_t device_busy_timeout;
+
+	/* for HW_GPMI_CTRL1 */
 	bool     use_half_periods;
 	uint8_t  sample_delay_factor;
 };
diff --git a/drivers/mtd/nand/gpmi-nand/gpmi-regs.h b/drivers/mtd/nand/gpmi-nand/gpmi-regs.h
index 8343124..7961c14 100644
--- a/drivers/mtd/nand/gpmi-nand/gpmi-regs.h
+++ b/drivers/mtd/nand/gpmi-nand/gpmi-regs.h
@@ -154,6 +154,9 @@
 
 #define HW_GPMI_TIMING1					0x00000080
 #define BP_GPMI_TIMING1_BUSY_TIMEOUT			16
+#define BM_GPMI_TIMING1_BUSY_TIMEOUT	(0xff << BP_GPMI_TIMING1_BUSY_TIMEOUT)
+#define BF_GPMI_TIMING1_BUSY_TIMEOUT(v)		\
+	(((v) << BP_GPMI_TIMING1_BUSY_TIMEOUT) & BM_GPMI_TIMING1_BUSY_TIMEOUT)
 
 #define HW_GPMI_TIMING2					0x00000090
 #define HW_GPMI_DATA					0x000000a0
-- 
1.7.0.4

[PATCH 3/9] mtd: gpmi: add a new field for HW_GPMI_TIMING1

[Vikram Narayanan]

Hello Huang Shijie,

On 9/11/2012 11:47 AM, Huang Shijie wrote:
> The gpmi_nfc_compute_hardware_timing{} should contains all the
> fields setting for gpmi timing registers. It already contains the fields
> for HW_GPMI_TIMING0 and HW_GPMI_CTRL1.
>
> So it is better to add a new field setting for HW_GPMI_TIMING1 in
> this data structure. This makes the code more clear in logic.
>
> This patch also changes some comments to make the code more readable.
>
> Signed-off-by: Huang Shijie<b32955@freescale.com>
> ---
>   drivers/mtd/nand/gpmi-nand/gpmi-lib.c  |   17 +++++++++--------
>   drivers/mtd/nand/gpmi-nand/gpmi-nand.h |    9 +++++++++
>   drivers/mtd/nand/gpmi-nand/gpmi-regs.h |    3 +++
>   3 files changed, 21 insertions(+), 8 deletions(-)
>
> diff --git a/drivers/mtd/nand/gpmi-nand/gpmi-lib.c b/drivers/mtd/nand/gpmi-nand/gpmi-lib.c
> index 2289cf8..fb9b3c1 100644
> --- a/drivers/mtd/nand/gpmi-nand/gpmi-lib.c
> +++ b/drivers/mtd/nand/gpmi-nand/gpmi-lib.c
> @@ -728,6 +728,7 @@ return_results:
>   	hw->address_setup_in_cycles = address_setup_in_cycles;
>   	hw->use_half_periods        = dll_use_half_periods;
>   	hw->sample_delay_factor     = sample_delay_factor;
> +	hw->device_busy_timeout     = 0x500; /* default busy timeout value. */

Can we make this value a macro and put the comment there?

>   	/* Return success. */
>   	return 0;
> @@ -752,26 +753,26 @@ void gpmi_begin(struct gpmi_nand_data *this)
>   		goto err_out;
>   	}
>
> -	/* set ready/busy timeout */
> -	writel(0x500<<  BP_GPMI_TIMING1_BUSY_TIMEOUT,
> -		gpmi_regs + HW_GPMI_TIMING1);
> -
>   	/* Get the timing information we need. */
>   	nfc->clock_frequency_in_hz = clk_get_rate(r->clock[0]);
>   	clock_period_in_ns = 1000000000 / nfc->clock_frequency_in_hz;
>
>   	gpmi_nfc_compute_hardware_timing(this,&hw);
>
> -	/* Set up all the simple timing parameters. */
> +	/* [1] Set HW_GPMI_TIMING0 */
>   	reg = BF_GPMI_TIMING0_ADDRESS_SETUP(hw.address_setup_in_cycles) |
>   		BF_GPMI_TIMING0_DATA_HOLD(hw.data_hold_in_cycles)         |
>   		BF_GPMI_TIMING0_DATA_SETUP(hw.data_setup_in_cycles)       ;
>
>   	writel(reg, gpmi_regs + HW_GPMI_TIMING0);
>
> -	/*
> -	 * DLL_ENABLE must be set to 0 when setting RDN_DELAY or HALF_PERIOD.
> -	 */
> +	/* [2] Set HW_GPMI_TIMING1 */
> +	writel(BF_GPMI_TIMING1_BUSY_TIMEOUT(hw.device_busy_timeout),
> +		gpmi_regs + HW_GPMI_TIMING1);
> +
> +	/* [3] The following code is to set the HW_GPMI_CTRL1. */
> +
> +	/* DLL_ENABLE must be set to 0 when setting RDN_DELAY or HALF_PERIOD. */
>   	writel(BM_GPMI_CTRL1_DLL_ENABLE, gpmi_regs + HW_GPMI_CTRL1_CLR);


Are these [1], [2], [3] notation references to some other comments?

>   	/* Clear out the DLL control fields. */
> diff --git a/drivers/mtd/nand/gpmi-nand/gpmi-nand.h b/drivers/mtd/nand/gpmi-nand/gpmi-nand.h
> index 1f61217..e68bbac 100644
> --- a/drivers/mtd/nand/gpmi-nand/gpmi-nand.h
> +++ b/drivers/mtd/nand/gpmi-nand/gpmi-nand.h
> @@ -189,14 +189,23 @@ struct gpmi_nand_data {
>    * @data_setup_in_cycles:      The data setup time, in cycles.
>    * @data_hold_in_cycles:       The data hold time, in cycles.
>    * @address_setup_in_cycles:   The address setup time, in cycles.
> + * @device_busy_timeout:       The timeout waiting for NAND Ready/Busy,
> + *                             this value is the number of cycles multiplied
> + *                             by 4096.
>    * @use_half_periods:          Indicates the clock is running slowly, so the
>    *                             NFC DLL should use half-periods.
>    * @sample_delay_factor:       The sample delay factor.
>    */
>   struct gpmi_nfc_hardware_timing {
> +	/* for HW_GPMI_TIMING0 */
>   	uint8_t  data_setup_in_cycles;
>   	uint8_t  data_hold_in_cycles;
>   	uint8_t  address_setup_in_cycles;
> +
> +	/* for HW_GPMI_TIMING1 */
> +	uint16_t device_busy_timeout;
> +
> +	/* for HW_GPMI_CTRL1 */
>   	bool     use_half_periods;
>   	uint8_t  sample_delay_factor;
>   };
> diff --git a/drivers/mtd/nand/gpmi-nand/gpmi-regs.h b/drivers/mtd/nand/gpmi-nand/gpmi-regs.h
> index 8343124..7961c14 100644
> --- a/drivers/mtd/nand/gpmi-nand/gpmi-regs.h
> +++ b/drivers/mtd/nand/gpmi-nand/gpmi-regs.h
> @@ -154,6 +154,9 @@
>
>   #define HW_GPMI_TIMING1					0x00000080
>   #define BP_GPMI_TIMING1_BUSY_TIMEOUT			16
> +#define BM_GPMI_TIMING1_BUSY_TIMEOUT	(0xff<<  BP_GPMI_TIMING1_BUSY_TIMEOUT)

Isn't this 0xffff?
My Rev C manual states so. I might be wrong if I'm referring to a older one!

> +#define BF_GPMI_TIMING1_BUSY_TIMEOUT(v)		\
> +	(((v)<<  BP_GPMI_TIMING1_BUSY_TIMEOUT)&  BM_GPMI_TIMING1_BUSY_TIMEOUT)
>
>   #define HW_GPMI_TIMING2					0x00000090
>   #define HW_GPMI_DATA					0x000000a0

Regards,
Vikram

[PATCH 3/9] mtd: gpmi: add a new field for HW_GPMI_TIMING1

[Huang Shijie]

? 2012?09?12? 02:00, Vikram Narayanan ??:
> Hello Huang Shijie,
>
> On 9/11/2012 11:47 AM, Huang Shijie wrote:
>> The gpmi_nfc_compute_hardware_timing{} should contains all the
>> fields setting for gpmi timing registers. It already contains the fields
>> for HW_GPMI_TIMING0 and HW_GPMI_CTRL1.
>>
>> So it is better to add a new field setting for HW_GPMI_TIMING1 in
>> this data structure. This makes the code more clear in logic.
>>
>> This patch also changes some comments to make the code more readable.
>>
>> Signed-off-by: Huang Shijie<b32955@freescale.com>
>> ---
>> drivers/mtd/nand/gpmi-nand/gpmi-lib.c | 17 +++++++++--------
>> drivers/mtd/nand/gpmi-nand/gpmi-nand.h | 9 +++++++++
>> drivers/mtd/nand/gpmi-nand/gpmi-regs.h | 3 +++
>> 3 files changed, 21 insertions(+), 8 deletions(-)
>>
>> diff --git a/drivers/mtd/nand/gpmi-nand/gpmi-lib.c 
>> b/drivers/mtd/nand/gpmi-nand/gpmi-lib.c
>> index 2289cf8..fb9b3c1 100644
>> --- a/drivers/mtd/nand/gpmi-nand/gpmi-lib.c
>> +++ b/drivers/mtd/nand/gpmi-nand/gpmi-lib.c
>> @@ -728,6 +728,7 @@ return_results:
>> hw->address_setup_in_cycles = address_setup_in_cycles;
>> hw->use_half_periods = dll_use_half_periods;
>> hw->sample_delay_factor = sample_delay_factor;
>> + hw->device_busy_timeout = 0x500; /* default busy timeout value. */
>
> Can we make this value a macro and put the comment there?
ok.

thanks.
>
>> /* Return success. */
>> return 0;
>> @@ -752,26 +753,26 @@ void gpmi_begin(struct gpmi_nand_data *this)
>> goto err_out;
>> }
>>
>> - /* set ready/busy timeout */
>> - writel(0x500<< BP_GPMI_TIMING1_BUSY_TIMEOUT,
>> - gpmi_regs + HW_GPMI_TIMING1);
>> -
>> /* Get the timing information we need. */
>> nfc->clock_frequency_in_hz = clk_get_rate(r->clock[0]);
>> clock_period_in_ns = 1000000000 / nfc->clock_frequency_in_hz;
>>
>> gpmi_nfc_compute_hardware_timing(this,&hw);
>>
>> - /* Set up all the simple timing parameters. */
>> + /* [1] Set HW_GPMI_TIMING0 */
>> reg = BF_GPMI_TIMING0_ADDRESS_SETUP(hw.address_setup_in_cycles) |
>> BF_GPMI_TIMING0_DATA_HOLD(hw.data_hold_in_cycles) |
>> BF_GPMI_TIMING0_DATA_SETUP(hw.data_setup_in_cycles) ;
>>
>> writel(reg, gpmi_regs + HW_GPMI_TIMING0);
>>
>> - /*
>> - * DLL_ENABLE must be set to 0 when setting RDN_DELAY or HALF_PERIOD.
>> - */
>> + /* [2] Set HW_GPMI_TIMING1 */
>> + writel(BF_GPMI_TIMING1_BUSY_TIMEOUT(hw.device_busy_timeout),
>> + gpmi_regs + HW_GPMI_TIMING1);
>> +
>> + /* [3] The following code is to set the HW_GPMI_CTRL1. */
>> +
>> + /* DLL_ENABLE must be set to 0 when setting RDN_DELAY or 
>> HALF_PERIOD. */
>> writel(BM_GPMI_CTRL1_DLL_ENABLE, gpmi_regs + HW_GPMI_CTRL1_CLR);
>
>
> Are these [1], [2], [3] notation references to some other comments?
no.

These notations just help to comment the steps of setting timing registers.
Are you confused at this?


>
>> /* Clear out the DLL control fields. */
>> diff --git a/drivers/mtd/nand/gpmi-nand/gpmi-nand.h 
>> b/drivers/mtd/nand/gpmi-nand/gpmi-nand.h
>> index 1f61217..e68bbac 100644
>> --- a/drivers/mtd/nand/gpmi-nand/gpmi-nand.h
>> +++ b/drivers/mtd/nand/gpmi-nand/gpmi-nand.h
>> @@ -189,14 +189,23 @@ struct gpmi_nand_data {
>> * @data_setup_in_cycles: The data setup time, in cycles.
>> * @data_hold_in_cycles: The data hold time, in cycles.
>> * @address_setup_in_cycles: The address setup time, in cycles.
>> + * @device_busy_timeout: The timeout waiting for NAND Ready/Busy,
>> + * this value is the number of cycles multiplied
>> + * by 4096.
>> * @use_half_periods: Indicates the clock is running slowly, so the
>> * NFC DLL should use half-periods.
>> * @sample_delay_factor: The sample delay factor.
>> */
>> struct gpmi_nfc_hardware_timing {
>> + /* for HW_GPMI_TIMING0 */
>> uint8_t data_setup_in_cycles;
>> uint8_t data_hold_in_cycles;
>> uint8_t address_setup_in_cycles;
>> +
>> + /* for HW_GPMI_TIMING1 */
>> + uint16_t device_busy_timeout;
>> +
>> + /* for HW_GPMI_CTRL1 */
>> bool use_half_periods;
>> uint8_t sample_delay_factor;
>> };
>> diff --git a/drivers/mtd/nand/gpmi-nand/gpmi-regs.h 
>> b/drivers/mtd/nand/gpmi-nand/gpmi-regs.h
>> index 8343124..7961c14 100644
>> --- a/drivers/mtd/nand/gpmi-nand/gpmi-regs.h
>> +++ b/drivers/mtd/nand/gpmi-nand/gpmi-regs.h
>> @@ -154,6 +154,9 @@
>>
>> #define HW_GPMI_TIMING1 0x00000080
>> #define BP_GPMI_TIMING1_BUSY_TIMEOUT 16
>> +#define BM_GPMI_TIMING1_BUSY_TIMEOUT (0xff<< 
>> BP_GPMI_TIMING1_BUSY_TIMEOUT)
>
> Isn't this 0xffff?
> My Rev C manual states so. I might be wrong if I'm referring to a 
> older one!
this is my mistake.
It should be 0xffff.

thanks a lot !

Best Regards
Huang Shijie

[PATCH 4/9] mtd: gpmi: do not get the clock frequency in gpmi_begin()

[Huang Shijie]

The current code will gets the clock frequency which is used by
gpmi_nfc_compute_hardware_timing(). It makes the code a little mess.

So move the `get clock frequency` code to the
gpmi_nfc_compute_hardware_timing() itself. This makes the code tidy
and clean.
Signed-off-by: Huang Shijie <b32955@freescale.com>
---
 drivers/mtd/nand/gpmi-nand/gpmi-lib.c |    8 +++-----
 1 files changed, 3 insertions(+), 5 deletions(-)

diff --git a/drivers/mtd/nand/gpmi-nand/gpmi-lib.c b/drivers/mtd/nand/gpmi-nand/gpmi-lib.c
index fb9b3c1..eee0159 100644
--- a/drivers/mtd/nand/gpmi-nand/gpmi-lib.c
+++ b/drivers/mtd/nand/gpmi-nand/gpmi-lib.c
@@ -293,6 +293,7 @@ static int gpmi_nfc_compute_hardware_timing(struct gpmi_nand_data *this,
 					struct gpmi_nfc_hardware_timing *hw)
 {
 	struct timing_threshod *nfc = &timing_default_threshold;
+	struct resources *r = &this->resources;
 	struct nand_chip *nand = &this->nand;
 	struct nand_timing target = this->timing;
 	bool improved_timing_is_available;
@@ -332,6 +333,7 @@ static int gpmi_nfc_compute_hardware_timing(struct gpmi_nand_data *this,
 		(target.tRHOH_in_ns >= 0) ;
 
 	/* Inspect the clock. */
+	nfc->clock_frequency_in_hz = clk_get_rate(r->clock[0]);
 	clock_frequency_in_hz = nfc->clock_frequency_in_hz;
 	clock_period_in_ns    = 1000000000 / clock_frequency_in_hz;
 
@@ -738,7 +740,6 @@ return_results:
 void gpmi_begin(struct gpmi_nand_data *this)
 {
 	struct resources *r = &this->resources;
-	struct timing_threshod *nfc = &timing_default_threshold;
 	void __iomem *gpmi_regs = r->gpmi_regs;
 	unsigned int   clock_period_in_ns;
 	uint32_t       reg;
@@ -753,10 +754,6 @@ void gpmi_begin(struct gpmi_nand_data *this)
 		goto err_out;
 	}
 
-	/* Get the timing information we need. */
-	nfc->clock_frequency_in_hz = clk_get_rate(r->clock[0]);
-	clock_period_in_ns = 1000000000 / nfc->clock_frequency_in_hz;
-
 	gpmi_nfc_compute_hardware_timing(this, &hw);
 
 	/* [1] Set HW_GPMI_TIMING0 */
@@ -801,6 +798,7 @@ void gpmi_begin(struct gpmi_nand_data *this)
 	 *
 	 * Calculate the amount of time we need to wait, in microseconds.
 	 */
+	clock_period_in_ns = 1000000000 / clk_get_rate(r->clock[0]);
 	dll_wait_time_in_us = (clock_period_in_ns * 64) / 1000;
 
 	if (!dll_wait_time_in_us)
-- 
1.7.0.4

[PATCH 5/9] mtd: gpmi: add a new field for HW_GPMI_CTRL1

[Huang Shijie]

add the WRN_DLY_SEL field for HW_GPMI_CTRL1.
This field is used as delay for gpmi write strobe.

Signed-off-by: Huang Shijie <b32955@freescale.com>
---
 drivers/mtd/nand/gpmi-nand/gpmi-lib.c  |    6 ++++++
 drivers/mtd/nand/gpmi-nand/gpmi-nand.h |    2 ++
 drivers/mtd/nand/gpmi-nand/gpmi-regs.h |    5 +++++
 3 files changed, 13 insertions(+), 0 deletions(-)

diff --git a/drivers/mtd/nand/gpmi-nand/gpmi-lib.c b/drivers/mtd/nand/gpmi-nand/gpmi-lib.c
index eee0159..037438a 100644
--- a/drivers/mtd/nand/gpmi-nand/gpmi-lib.c
+++ b/drivers/mtd/nand/gpmi-nand/gpmi-lib.c
@@ -731,6 +731,7 @@ return_results:
 	hw->use_half_periods        = dll_use_half_periods;
 	hw->sample_delay_factor     = sample_delay_factor;
 	hw->device_busy_timeout     = 0x500; /* default busy timeout value. */
+	hw->wrn_dly_sel             = 0;
 
 	/* Return success. */
 	return 0;
@@ -769,6 +770,11 @@ void gpmi_begin(struct gpmi_nand_data *this)
 
 	/* [3] The following code is to set the HW_GPMI_CTRL1. */
 
+	/* Set the WRN_DLY_SEL */
+	writel(BM_GPMI_CTRL1_WRN_DLY_SEL, gpmi_regs + HW_GPMI_CTRL1_CLR);
+	writel(BF_GPMI_CTRL1_WRN_DLY_SEL(hw.wrn_dly_sel),
+					gpmi_regs + HW_GPMI_CTRL1_SET);
+
 	/* DLL_ENABLE must be set to 0 when setting RDN_DELAY or HALF_PERIOD. */
 	writel(BM_GPMI_CTRL1_DLL_ENABLE, gpmi_regs + HW_GPMI_CTRL1_CLR);
 
diff --git a/drivers/mtd/nand/gpmi-nand/gpmi-nand.h b/drivers/mtd/nand/gpmi-nand/gpmi-nand.h
index e68bbac..39f1f76 100644
--- a/drivers/mtd/nand/gpmi-nand/gpmi-nand.h
+++ b/drivers/mtd/nand/gpmi-nand/gpmi-nand.h
@@ -195,6 +195,7 @@ struct gpmi_nand_data {
  * @use_half_periods:          Indicates the clock is running slowly, so the
  *                             NFC DLL should use half-periods.
  * @sample_delay_factor:       The sample delay factor.
+ * @wrn_dly_sel:               The delay on the GPMI write strobe.
  */
 struct gpmi_nfc_hardware_timing {
 	/* for HW_GPMI_TIMING0 */
@@ -208,6 +209,7 @@ struct gpmi_nfc_hardware_timing {
 	/* for HW_GPMI_CTRL1 */
 	bool     use_half_periods;
 	uint8_t  sample_delay_factor;
+	uint8_t  wrn_dly_sel;
 };
 
 /**
diff --git a/drivers/mtd/nand/gpmi-nand/gpmi-regs.h b/drivers/mtd/nand/gpmi-nand/gpmi-regs.h
index 7961c14..ab428d6 100644
--- a/drivers/mtd/nand/gpmi-nand/gpmi-regs.h
+++ b/drivers/mtd/nand/gpmi-nand/gpmi-regs.h
@@ -108,6 +108,11 @@
 #define HW_GPMI_CTRL1_CLR				0x00000068
 #define HW_GPMI_CTRL1_TOG				0x0000006c
 
+#define BP_GPMI_CTRL1_WRN_DLY_SEL			22
+#define BM_GPMI_CTRL1_WRN_DLY_SEL			0x00C00000
+#define BF_GPMI_CTRL1_WRN_DLY_SEL(v)  \
+	(((v) << BP_GPMI_CTRL1_WRN_DLY_SEL) & BM_GPMI_CTRL1_WRN_DLY_SEL)
+
 #define BM_GPMI_CTRL1_BCH_MODE				(1 << 18)
 
 #define BP_GPMI_CTRL1_DLL_ENABLE			17
-- 
1.7.0.4

[PATCH 5/9] mtd: gpmi: add a new field for HW_GPMI_CTRL1

[Vikram Narayanan]

Hello Huang Shijie,

On 9/11/2012 11:47 AM, Huang Shijie wrote:
> add the WRN_DLY_SEL field for HW_GPMI_CTRL1.
> This field is used as delay for gpmi write strobe.
>
> Signed-off-by: Huang Shijie<b32955@freescale.com>
> ---
>   drivers/mtd/nand/gpmi-nand/gpmi-lib.c  |    6 ++++++
>   drivers/mtd/nand/gpmi-nand/gpmi-nand.h |    2 ++
>   drivers/mtd/nand/gpmi-nand/gpmi-regs.h |    5 +++++
>   3 files changed, 13 insertions(+), 0 deletions(-)
>
> diff --git a/drivers/mtd/nand/gpmi-nand/gpmi-lib.c b/drivers/mtd/nand/gpmi-nand/gpmi-lib.c
> index eee0159..037438a 100644
> --- a/drivers/mtd/nand/gpmi-nand/gpmi-lib.c
> +++ b/drivers/mtd/nand/gpmi-nand/gpmi-lib.c
> @@ -731,6 +731,7 @@ return_results:
>   	hw->use_half_periods        = dll_use_half_periods;
>   	hw->sample_delay_factor     = sample_delay_factor;
>   	hw->device_busy_timeout     = 0x500; /* default busy timeout value. */
> +	hw->wrn_dly_sel             = 0;
>
>   	/* Return success. */
>   	return 0;
> @@ -769,6 +770,11 @@ void gpmi_begin(struct gpmi_nand_data *this)
>
>   	/* [3] The following code is to set the HW_GPMI_CTRL1. */
>
> +	/* Set the WRN_DLY_SEL */
> +	writel(BM_GPMI_CTRL1_WRN_DLY_SEL, gpmi_regs + HW_GPMI_CTRL1_CLR);
> +	writel(BF_GPMI_CTRL1_WRN_DLY_SEL(hw.wrn_dly_sel),
> +					gpmi_regs + HW_GPMI_CTRL1_SET);
> +
>   	/* DLL_ENABLE must be set to 0 when setting RDN_DELAY or HALF_PERIOD. */
>   	writel(BM_GPMI_CTRL1_DLL_ENABLE, gpmi_regs + HW_GPMI_CTRL1_CLR);
>
> diff --git a/drivers/mtd/nand/gpmi-nand/gpmi-nand.h b/drivers/mtd/nand/gpmi-nand/gpmi-nand.h
> index e68bbac..39f1f76 100644
> --- a/drivers/mtd/nand/gpmi-nand/gpmi-nand.h
> +++ b/drivers/mtd/nand/gpmi-nand/gpmi-nand.h
> @@ -195,6 +195,7 @@ struct gpmi_nand_data {
>    * @use_half_periods:          Indicates the clock is running slowly, so the
>    *                             NFC DLL should use half-periods.
>    * @sample_delay_factor:       The sample delay factor.
> + * @wrn_dly_sel:               The delay on the GPMI write strobe.
>    */
>   struct gpmi_nfc_hardware_timing {
>   	/* for HW_GPMI_TIMING0 */
> @@ -208,6 +209,7 @@ struct gpmi_nfc_hardware_timing {
>   	/* for HW_GPMI_CTRL1 */
>   	bool     use_half_periods;
>   	uint8_t  sample_delay_factor;
> +	uint8_t  wrn_dly_sel;
>   };
>
>   /**
> diff --git a/drivers/mtd/nand/gpmi-nand/gpmi-regs.h b/drivers/mtd/nand/gpmi-nand/gpmi-regs.h
> index 7961c14..ab428d6 100644
> --- a/drivers/mtd/nand/gpmi-nand/gpmi-regs.h
> +++ b/drivers/mtd/nand/gpmi-nand/gpmi-regs.h
> @@ -108,6 +108,11 @@
>   #define HW_GPMI_CTRL1_CLR				0x00000068
>   #define HW_GPMI_CTRL1_TOG				0x0000006c
>
> +#define BP_GPMI_CTRL1_WRN_DLY_SEL			22
> +#define BM_GPMI_CTRL1_WRN_DLY_SEL			0x00C00000

In [PATCH 3/9] mtd: gpmi: add a new field for HW_GPMI_TIMING1,
the above is done in a different way.

#define BM_GPMI_TIMING1_BUSY_TIMEOUT	
		(0xff << BP_GPMI_TIMING1_BUSY_TIMEOUT)

I think it would be good if we follow the same conventions in all the 
places.

> +#define BF_GPMI_CTRL1_WRN_DLY_SEL(v)  \
> +	(((v)<<  BP_GPMI_CTRL1_WRN_DLY_SEL)&  BM_GPMI_CTRL1_WRN_DLY_SEL)
> +
>   #define BM_GPMI_CTRL1_BCH_MODE				(1<<  18)
>
>   #define BP_GPMI_CTRL1_DLL_ENABLE			17

Regards,
Vikram

[PATCH 5/9] mtd: gpmi: add a new field for HW_GPMI_CTRL1

[Huang Shijie]

? 2012?09?12? 02:04, Vikram Narayanan ??:
> Hello Huang Shijie,
>
> On 9/11/2012 11:47 AM, Huang Shijie wrote:
>> add the WRN_DLY_SEL field for HW_GPMI_CTRL1.
>> This field is used as delay for gpmi write strobe.
>>
>> Signed-off-by: Huang Shijie<b32955@freescale.com>
>> ---
>> drivers/mtd/nand/gpmi-nand/gpmi-lib.c | 6 ++++++
>> drivers/mtd/nand/gpmi-nand/gpmi-nand.h | 2 ++
>> drivers/mtd/nand/gpmi-nand/gpmi-regs.h | 5 +++++
>> 3 files changed, 13 insertions(+), 0 deletions(-)
>>
>> diff --git a/drivers/mtd/nand/gpmi-nand/gpmi-lib.c 
>> b/drivers/mtd/nand/gpmi-nand/gpmi-lib.c
>> index eee0159..037438a 100644
>> --- a/drivers/mtd/nand/gpmi-nand/gpmi-lib.c
>> +++ b/drivers/mtd/nand/gpmi-nand/gpmi-lib.c
>> @@ -731,6 +731,7 @@ return_results:
>> hw->use_half_periods = dll_use_half_periods;
>> hw->sample_delay_factor = sample_delay_factor;
>> hw->device_busy_timeout = 0x500; /* default busy timeout value. */
>> + hw->wrn_dly_sel = 0;
>>
>> /* Return success. */
>> return 0;
>> @@ -769,6 +770,11 @@ void gpmi_begin(struct gpmi_nand_data *this)
>>
>> /* [3] The following code is to set the HW_GPMI_CTRL1. */
>>
>> + /* Set the WRN_DLY_SEL */
>> + writel(BM_GPMI_CTRL1_WRN_DLY_SEL, gpmi_regs + HW_GPMI_CTRL1_CLR);
>> + writel(BF_GPMI_CTRL1_WRN_DLY_SEL(hw.wrn_dly_sel),
>> + gpmi_regs + HW_GPMI_CTRL1_SET);
>> +
>> /* DLL_ENABLE must be set to 0 when setting RDN_DELAY or HALF_PERIOD. */
>> writel(BM_GPMI_CTRL1_DLL_ENABLE, gpmi_regs + HW_GPMI_CTRL1_CLR);
>>
>> diff --git a/drivers/mtd/nand/gpmi-nand/gpmi-nand.h 
>> b/drivers/mtd/nand/gpmi-nand/gpmi-nand.h
>> index e68bbac..39f1f76 100644
>> --- a/drivers/mtd/nand/gpmi-nand/gpmi-nand.h
>> +++ b/drivers/mtd/nand/gpmi-nand/gpmi-nand.h
>> @@ -195,6 +195,7 @@ struct gpmi_nand_data {
>> * @use_half_periods: Indicates the clock is running slowly, so the
>> * NFC DLL should use half-periods.
>> * @sample_delay_factor: The sample delay factor.
>> + * @wrn_dly_sel: The delay on the GPMI write strobe.
>> */
>> struct gpmi_nfc_hardware_timing {
>> /* for HW_GPMI_TIMING0 */
>> @@ -208,6 +209,7 @@ struct gpmi_nfc_hardware_timing {
>> /* for HW_GPMI_CTRL1 */
>> bool use_half_periods;
>> uint8_t sample_delay_factor;
>> + uint8_t wrn_dly_sel;
>> };
>>
>> /**
>> diff --git a/drivers/mtd/nand/gpmi-nand/gpmi-regs.h 
>> b/drivers/mtd/nand/gpmi-nand/gpmi-regs.h
>> index 7961c14..ab428d6 100644
>> --- a/drivers/mtd/nand/gpmi-nand/gpmi-regs.h
>> +++ b/drivers/mtd/nand/gpmi-nand/gpmi-regs.h
>> @@ -108,6 +108,11 @@
>> #define HW_GPMI_CTRL1_CLR 0x00000068
>> #define HW_GPMI_CTRL1_TOG 0x0000006c
>>
>> +#define BP_GPMI_CTRL1_WRN_DLY_SEL 22
>> +#define BM_GPMI_CTRL1_WRN_DLY_SEL 0x00C00000
>
> In [PATCH 3/9] mtd: gpmi: add a new field for HW_GPMI_TIMING1,
> the above is done in a different way.
>
> #define BM_GPMI_TIMING1_BUSY_TIMEOUT
> (0xff << BP_GPMI_TIMING1_BUSY_TIMEOUT)
>
> I think it would be good if we follow the same conventions in all the 
> places.
>
yes. we should.

thanks
Huang Shijie

[PATCH 6/9] mtd: gpmi: simplify the setting DLL code

[Huang Shijie]

The setting DLL code is a little mess.
Just simplify the code and the comments.

Signed-off-by: Huang Shijie <b32955@freescale.com>
---
 drivers/mtd/nand/gpmi-nand/gpmi-lib.c |   22 +++++++++-------------
 1 files changed, 9 insertions(+), 13 deletions(-)

diff --git a/drivers/mtd/nand/gpmi-nand/gpmi-lib.c b/drivers/mtd/nand/gpmi-nand/gpmi-lib.c
index 037438a..83c5573 100644
--- a/drivers/mtd/nand/gpmi-nand/gpmi-lib.c
+++ b/drivers/mtd/nand/gpmi-nand/gpmi-lib.c
@@ -779,30 +779,26 @@ void gpmi_begin(struct gpmi_nand_data *this)
 	writel(BM_GPMI_CTRL1_DLL_ENABLE, gpmi_regs + HW_GPMI_CTRL1_CLR);
 
 	/* Clear out the DLL control fields. */
-	writel(BM_GPMI_CTRL1_RDN_DELAY,   gpmi_regs + HW_GPMI_CTRL1_CLR);
-	writel(BM_GPMI_CTRL1_HALF_PERIOD, gpmi_regs + HW_GPMI_CTRL1_CLR);
+	reg = BM_GPMI_CTRL1_RDN_DELAY | BM_GPMI_CTRL1_HALF_PERIOD;
+	writel(reg, gpmi_regs + HW_GPMI_CTRL1_CLR);
 
 	/* If no sample delay is called for, return immediately. */
 	if (!hw.sample_delay_factor)
 		return;
 
-	/* Configure the HALF_PERIOD flag. */
-	if (hw.use_half_periods)
-		writel(BM_GPMI_CTRL1_HALF_PERIOD,
-						gpmi_regs + HW_GPMI_CTRL1_SET);
+	/* Set RDN_DELAY or HALF_PERIOD. */
+	reg = ((hw.use_half_periods) ? BM_GPMI_CTRL1_HALF_PERIOD : 0)
+		| BF_GPMI_CTRL1_RDN_DELAY(hw.sample_delay_factor);
 
-	/* Set the delay factor. */
-	writel(BF_GPMI_CTRL1_RDN_DELAY(hw.sample_delay_factor),
-						gpmi_regs + HW_GPMI_CTRL1_SET);
+	writel(reg, gpmi_regs + HW_GPMI_CTRL1_SET);
 
-	/* Enable the DLL. */
+	/* At last, we enable the DLL. */
 	writel(BM_GPMI_CTRL1_DLL_ENABLE, gpmi_regs + HW_GPMI_CTRL1_SET);
 
 	/*
 	 * After we enable the GPMI DLL, we have to wait 64 clock cycles before
-	 * we can use the GPMI.
-	 *
-	 * Calculate the amount of time we need to wait, in microseconds.
+	 * we can use the GPMI. Calculate the amount of time we need to wait,
+	 * in microseconds.
 	 */
 	clock_period_in_ns = 1000000000 / clk_get_rate(r->clock[0]);
 	dll_wait_time_in_us = (clock_period_in_ns * 64) / 1000;
-- 
1.7.0.4

[PATCH 6/9] mtd: gpmi: simplify the setting DLL code

[Vikram Narayanan]

Hello Huang Shijie,

On 9/11/2012 11:47 AM, Huang Shijie wrote:
> The setting DLL code is a little mess.
> Just simplify the code and the comments.
>
> Signed-off-by: Huang Shijie<b32955@freescale.com>
> ---
>   drivers/mtd/nand/gpmi-nand/gpmi-lib.c |   22 +++++++++-------------
>   1 files changed, 9 insertions(+), 13 deletions(-)
>
> diff --git a/drivers/mtd/nand/gpmi-nand/gpmi-lib.c b/drivers/mtd/nand/gpmi-nand/gpmi-lib.c
> index 037438a..83c5573 100644
> --- a/drivers/mtd/nand/gpmi-nand/gpmi-lib.c
> +++ b/drivers/mtd/nand/gpmi-nand/gpmi-lib.c
> @@ -779,30 +779,26 @@ void gpmi_begin(struct gpmi_nand_data *this)
>   	writel(BM_GPMI_CTRL1_DLL_ENABLE, gpmi_regs + HW_GPMI_CTRL1_CLR);
>
>   	/* Clear out the DLL control fields. */
> -	writel(BM_GPMI_CTRL1_RDN_DELAY,   gpmi_regs + HW_GPMI_CTRL1_CLR);
> -	writel(BM_GPMI_CTRL1_HALF_PERIOD, gpmi_regs + HW_GPMI_CTRL1_CLR);
> +	reg = BM_GPMI_CTRL1_RDN_DELAY | BM_GPMI_CTRL1_HALF_PERIOD;
> +	writel(reg, gpmi_regs + HW_GPMI_CTRL1_CLR);
>
>   	/* If no sample delay is called for, return immediately. */
>   	if (!hw.sample_delay_factor)
>   		return;
>
> -	/* Configure the HALF_PERIOD flag. */
> -	if (hw.use_half_periods)
> -		writel(BM_GPMI_CTRL1_HALF_PERIOD,
> -						gpmi_regs + HW_GPMI_CTRL1_SET);
> +	/* Set RDN_DELAY or HALF_PERIOD. */
> +	reg = ((hw.use_half_periods) ? BM_GPMI_CTRL1_HALF_PERIOD : 0)
> +		| BF_GPMI_CTRL1_RDN_DELAY(hw.sample_delay_factor);
>
> -	/* Set the delay factor. */
> -	writel(BF_GPMI_CTRL1_RDN_DELAY(hw.sample_delay_factor),
> -						gpmi_regs + HW_GPMI_CTRL1_SET);
> +	writel(reg, gpmi_regs + HW_GPMI_CTRL1_SET);
>
> -	/* Enable the DLL. */
> +	/* At last, we enable the DLL. */
>   	writel(BM_GPMI_CTRL1_DLL_ENABLE, gpmi_regs + HW_GPMI_CTRL1_SET);
>
>   	/*
>   	 * After we enable the GPMI DLL, we have to wait 64 clock cycles before
> -	 * we can use the GPMI.
> -	 *
> -	 * Calculate the amount of time we need to wait, in microseconds.
> +	 * we can use the GPMI. Calculate the amount of time we need to wait,
> +	 * in microseconds.
>   	 */
>   	clock_period_in_ns = 1000000000 / clk_get_rate(r->clock[0]);

NSEC_PER_SEC macro in the above statement?

Don't curse me for commenting about the code that you've not written. As 
this patch does some cleanups, I'm suggesting this.


>   	dll_wait_time_in_us = (clock_period_in_ns * 64) / 1000;

Regards,
Vikram

[PATCH 6/9] mtd: gpmi: simplify the setting DLL code

[Huang Shijie]

? 2012?09?12? 02:08, Vikram Narayanan ??:
> Hello Huang Shijie,
>
> On 9/11/2012 11:47 AM, Huang Shijie wrote:
>> The setting DLL code is a little mess.
>> Just simplify the code and the comments.
>>
>> Signed-off-by: Huang Shijie<b32955@freescale.com>
>> ---
>> drivers/mtd/nand/gpmi-nand/gpmi-lib.c | 22 +++++++++-------------
>> 1 files changed, 9 insertions(+), 13 deletions(-)
>>
>> diff --git a/drivers/mtd/nand/gpmi-nand/gpmi-lib.c 
>> b/drivers/mtd/nand/gpmi-nand/gpmi-lib.c
>> index 037438a..83c5573 100644
>> --- a/drivers/mtd/nand/gpmi-nand/gpmi-lib.c
>> +++ b/drivers/mtd/nand/gpmi-nand/gpmi-lib.c
>> @@ -779,30 +779,26 @@ void gpmi_begin(struct gpmi_nand_data *this)
>> writel(BM_GPMI_CTRL1_DLL_ENABLE, gpmi_regs + HW_GPMI_CTRL1_CLR);
>>
>> /* Clear out the DLL control fields. */
>> - writel(BM_GPMI_CTRL1_RDN_DELAY, gpmi_regs + HW_GPMI_CTRL1_CLR);
>> - writel(BM_GPMI_CTRL1_HALF_PERIOD, gpmi_regs + HW_GPMI_CTRL1_CLR);
>> + reg = BM_GPMI_CTRL1_RDN_DELAY | BM_GPMI_CTRL1_HALF_PERIOD;
>> + writel(reg, gpmi_regs + HW_GPMI_CTRL1_CLR);
>>
>> /* If no sample delay is called for, return immediately. */
>> if (!hw.sample_delay_factor)
>> return;
>>
>> - /* Configure the HALF_PERIOD flag. */
>> - if (hw.use_half_periods)
>> - writel(BM_GPMI_CTRL1_HALF_PERIOD,
>> - gpmi_regs + HW_GPMI_CTRL1_SET);
>> + /* Set RDN_DELAY or HALF_PERIOD. */
>> + reg = ((hw.use_half_periods) ? BM_GPMI_CTRL1_HALF_PERIOD : 0)
>> + | BF_GPMI_CTRL1_RDN_DELAY(hw.sample_delay_factor);
>>
>> - /* Set the delay factor. */
>> - writel(BF_GPMI_CTRL1_RDN_DELAY(hw.sample_delay_factor),
>> - gpmi_regs + HW_GPMI_CTRL1_SET);
>> + writel(reg, gpmi_regs + HW_GPMI_CTRL1_SET);
>>
>> - /* Enable the DLL. */
>> + /* At last, we enable the DLL. */
>> writel(BM_GPMI_CTRL1_DLL_ENABLE, gpmi_regs + HW_GPMI_CTRL1_SET);
>>
>> /*
>> * After we enable the GPMI DLL, we have to wait 64 clock cycles before
>> - * we can use the GPMI.
>> - *
>> - * Calculate the amount of time we need to wait, in microseconds.
>> + * we can use the GPMI. Calculate the amount of time we need to wait,
>> + * in microseconds.
>> */
>> clock_period_in_ns = 1000000000 / clk_get_rate(r->clock[0]);
>
> NSEC_PER_SEC macro in the above statement?
thanks. I like this macro.

>
> Don't curse me for commenting about the code that you've not written. 
> As this patch does some cleanups, I'm suggesting this.
>
sorry, I do not understand it very well.
could you tell me in detail what's the "commenting about the code that 
you've not written" meaning?

Which comments do you think is not proper?

Best Regards
Huang Shijie



>
>> dll_wait_time_in_us = (clock_period_in_ns * 64) / 1000;
>
> Regards,
> Vikram
>

[PATCH 6/9] mtd: gpmi: simplify the setting DLL code

[Huang Shijie]

? 2012?09?12? 02:08, Vikram Narayanan ??:
>>
>> /*
>> * After we enable the GPMI DLL, we have to wait 64 clock cycles before
>> - * we can use the GPMI.
>> - *
>> - * Calculate the amount of time we need to wait, in microseconds.
>> + * we can use the GPMI. Calculate the amount of time we need to wait,
>> + * in microseconds.
>> */
>> clock_period_in_ns = 1000000000 / clk_get_rate(r->clock[0]);
>
> NSEC_PER_SEC macro in the above statement?
>
> Don't curse me for commenting about the code that you've not written. 
> As this patch does some cleanups, I'm suggesting this. 
I finally get what's your meaning. thanks for you review :)

I really appreciate it.


Best Regards
Huang Shijie

[PATCH 7/9] mtd: gpmi: do not set the default values for the extra clocks

[Huang Shijie]

The default frequencies of the extra clocks are 200MHz.
The current code sets the extra clocks to 44.5MHz.

When i add the EDO feature to gpmi, i have to revert the extra clocks
to 200MHz.

So it is better that we do not set the default values for the extra
clocks. The driver runs well even when we do not set the default values for
extra clocks.

Signed-off-by: Huang Shijie <b32955@freescale.com>
---
 drivers/mtd/nand/gpmi-nand/gpmi-nand.c |   15 +++++----------
 1 files changed, 5 insertions(+), 10 deletions(-)

diff --git a/drivers/mtd/nand/gpmi-nand/gpmi-nand.c b/drivers/mtd/nand/gpmi-nand/gpmi-nand.c
index c46be6c..cc74ebf 100644
--- a/drivers/mtd/nand/gpmi-nand/gpmi-nand.c
+++ b/drivers/mtd/nand/gpmi-nand/gpmi-nand.c
@@ -513,20 +513,15 @@ static int __devinit gpmi_get_clks(struct gpmi_nand_data *this)
 		r->clock[i] = clk;
 	}
 
-	if (GPMI_IS_MX6Q(this)) {
+	if (GPMI_IS_MX6Q(this))
 		/*
-		 * Set the default values for the clocks in mx6q:
-		 *    The main clock(enfc) : 22MHz
-		 *    The others           : 44.5MHz
+		 * Set the default value for the gpmi clock in mx6q:
 		 *
-		 * These are just the default values. If you want to use
-		 * the ONFI nand which is in the Synchronous Mode, you should
-		 * change the clocks's frequencies as you need.
+		 * If you want to use the ONFI nand which is in the
+		 * Synchronous Mode, you should change the clock as you need.
 		 */
 		clk_set_rate(r->clock[0], 22000000);
-		for (i = 1; i < GPMI_CLK_MAX && r->clock[i]; i++)
-			clk_set_rate(r->clock[i], 44500000);
-	}
+
 	return 0;
 
 err_clock:
-- 
1.7.0.4

[PATCH 8/9] mtd: gpmi: add EDO feature for imx6q

[Huang Shijie]

When the frequency on the nand chip pins is above 33MHz,
the nand EDO(extended Data Out) timing could be applied.
The GPMI implements a Feedback read strobe to sample the read data in
the EDO timing mode.

This patch adds the EDO feature for the gpmi-nand driver.

For some onfi nand chips, the mode 4 is the fastest;
while for other onfi nand chips, the mode 5 is the fastest.
This patch only adds the support for the fastest asynchronous timing mode.
So this patch only supports the mode 4 and mode 5.

I tested several Micron's ONFI nand chips with EDO enabled,
take Micron MT29F32G08MAA for example (in mode 5, 100MHz):

1) The test result BEFORE we add the EDO feature:
=================================================
mtd_speedtest: MTD device: 2
mtd_speedtest: MTD device size 209715200, eraseblock size 524288,
			page size 4096, count of eraseblocks 400,
			pages per eraseblock 128, OOB size 218
.......................................
mtd_speedtest: testing eraseblock read speed
mtd_speedtest: eraseblock read speed is 3632 KiB/s
.......................................
mtd_speedtest: testing page read speed
mtd_speedtest: page read speed is 3554 KiB/s
.......................................
mtd_speedtest: testing 2 page read speed
mtd_speedtest: 2 page read speed is 3592 KiB/s
.......................................
=================================================

2) The test result AFTER we add the EDO feature:
=================================================
mtd_speedtest: MTD device: 2
mtd_speedtest: MTD device size 209715200, eraseblock size 524288,
			page size 4096, count of eraseblocks 400,
			pages per eraseblock 128, OOB size 218
.......................................
mtd_speedtest: testing eraseblock read speed
mtd_speedtest: eraseblock read speed is 19555 KiB/s
.......................................
mtd_speedtest: testing page read speed
mtd_speedtest: page read speed is 17319 KiB/s
.......................................
mtd_speedtest: testing 2 page read speed
mtd_speedtest: 2 page read speed is 18339 KiB/s
.......................................
=================================================

The read data performance is much improved by more then 5 times.

Signed-off-by: Huang Shijie <b32955@freescale.com>
---
 drivers/mtd/nand/gpmi-nand/gpmi-lib.c  |  214 +++++++++++++++++++++++++++++++-
 drivers/mtd/nand/gpmi-nand/gpmi-nand.c |    8 ++
 drivers/mtd/nand/gpmi-nand/gpmi-nand.h |    6 +
 3 files changed, 227 insertions(+), 1 deletions(-)

diff --git a/drivers/mtd/nand/gpmi-nand/gpmi-lib.c b/drivers/mtd/nand/gpmi-nand/gpmi-lib.c
index 83c5573..7efd326 100644
--- a/drivers/mtd/nand/gpmi-nand/gpmi-lib.c
+++ b/drivers/mtd/nand/gpmi-nand/gpmi-lib.c
@@ -737,6 +737,215 @@ return_results:
 	return 0;
 }
 
+/*
+ * [1] Firstly, we should know what's the GPMI-clock means.
+ *     The GPMI-clock is the internal clock in the gpmi nand controller.
+ *     If you set 100MHz to gpmi nand controller, the GPMI-clock's period
+ *     is 10ns. Mark the GPMI-clock's period as GPMI-clock-period.
+ *
+ * [2] Secondly, we should know what's the frequency on the nand chip pins.
+ *     The frequency on the nand chip pins is derived from the GPMI-clock.
+ *     We can get it from the following equation:
+ *
+ *         F = G / (DS + DH)
+ *
+ *         F  : the frequency on the nand chip pins.
+ *         G  : the GPMI clock, such as 100MHz.
+ *         DS : GPMI_HW_GPMI_TIMING0:DATA_SETUP
+ *         DH : GPMI_HW_GPMI_TIMING0:DATA_HOLD
+ *
+ * [3] Thirdly, when the frequency on the nand chip pins is above 33MHz,
+ *     the nand EDO(extended Data Out) timing could be applied.
+ *     The GPMI implements a feedback read strobe to sample the read data.
+ *     The feedback read strobe can be delayed to support the nand EDO timing
+ *     where the read strobe may deasserts before the read data is valid, and
+ *     read data is valid for some time after read strobe.
+ *
+ *     The following figure illustrates some aspects of a NAND Flash read:
+ *
+ *                   |<---tREA---->|
+ *                   |             |
+ *                   |         |   |
+ *                   |<--tRP-->|   |
+ *                   |         |   |
+ *                  __          ___|__________________________________
+ *     RDN            \________/   |
+ *                                 |
+ *                                 /---------\
+ *     Read Data    --------------<           >---------
+ *                                 \---------/
+ *                                |     |
+ *                                |<-D->|
+ *     FeedbackRDN  ________             ____________
+ *                          \___________/
+ *
+ *          D stands for delay, set in the HW_GPMI_CTRL1:RDN_DELAY.
+ *
+ *
+ * [4] Now, we begin to describe how to compute the right RDN_DELAY.
+ *
+ *  4.1) From the aspect of the nand chip pins:
+ *        Delay = (tREA + C - tRP)               [1]
+ *
+ *        tREA : the maximum read access time. From the ONFI nand standards,
+ *               we know that tREA is 16ns in mode 5, tREA is 20ns is mode 4.
+ *               Please check it in : www.onfi.org
+ *        C    : a constant for adjust the delay. default is 4.
+ *        tRP  : the read pulse width.
+ *               Specified by the HW_GPMI_TIMING0:DATA_SETUP:
+ *                    tRP = (GPMI-clock-period) * DATA_SETUP
+ *
+ *  4.2) From the aspect of the GPMI nand controller:
+ *         Delay = RDN_DELAY * 0.125 * RP        [2]
+ *
+ *         RP   : the DLL reference period.
+ *            if (GPMI-clock-period > DLL_THRETHOLD)
+ *                   RP = GPMI-clock-period / 2;
+ *            else
+ *                   RP = GPMI-clock-period;
+ *
+ *            Set the HW_GPMI_CTRL1:HALF_PERIOD if GPMI-clock-period
+ *            is greater DLL_THRETHOLD. In other Soc, the thethold is 16ns,
+ *            but in mx6q, we use 12ns.
+ *
+ *  4.3) since [1] equals [2], we get:
+ *
+ *                    (tREA + 4 - tRP) * 8
+ *         RDN_DELAY = ---------------------     [3]
+ *                           RP
+ *
+ *  4.4) We only support the fastest asynchronous mode of ONFI nand.
+ *       For some ONFI nand, the mode 4 is the fastest mode;
+ *       while for some ONFI nand, the mode 5 is the fastest mode.
+ *       So we only support the mode 4 and mode 5. It is no need to
+ *       support other modes.
+ */
+static void gpmi_compute_edo_timing(struct gpmi_nand_data *this,
+			struct gpmi_nfc_hardware_timing *hw)
+{
+	struct resources *r = &this->resources;
+	unsigned long rate = clk_get_rate(r->clock[0]);
+	int mode = this->timing_mode;
+	int dll_threshold = 16; /* in ns */
+	unsigned long delay;
+	unsigned long clk_period;
+	int t_rea;
+	int c = 4;
+	int t_rp;
+	int rp;
+
+	/*
+	 * 1) for GPMI_HW_GPMI_TIMING0:
+	 *    The async mode requires 40MHz for mode 4, 50MHz for mode 5.
+	 *    The GPMI can support 100MHz at most. But the if we want to
+	 *    get the 40MHz or 50MHz, we have to set DS=1, DH=1.
+	 *    It is okay to set ADDRESS_SETUP to 1.
+	 */
+	hw->data_setup_in_cycles = 1;
+	hw->data_hold_in_cycles = 1;
+	hw->address_setup_in_cycles = 1;
+
+	/* 2) for GPMI_HW_GPMI_TIMING1 */
+	hw->device_busy_timeout = 0x9000;
+
+	/* 3) for GPMI_HW_GPMI_CTRL1 */
+	hw->wrn_dly_sel = 3; /* no delay for write strobe. */
+
+	if (GPMI_IS_MX6Q(this))
+		dll_threshold = 12;
+
+	/*
+	 * Enlarge 10 times for the numerator and denominator in [3].
+	 * This make us to get more accurate result.
+	 */
+	clk_period = 1000000000 / (rate / 10);
+	dll_threshold *= 10;
+	t_rea = ((mode == 5) ? 16 : 20) * 10;
+	c *= 10;
+
+	t_rp = clk_period * 1; /* DATA_SETUP is 1 */
+
+	if (clk_period > dll_threshold) {
+		hw->use_half_periods = 1;
+		rp = clk_period / 2;
+	} else {
+		hw->use_half_periods = 0;
+		rp = clk_period;
+	}
+
+	/*
+	 * Multiply the numerator with 10, we could do a round off:
+	 *      7.8 round up to 8; 7.4 round down to 7.
+	 */
+	delay  = (((t_rea + c - t_rp) * 8) * 10) / rp;
+	delay = (delay + 5) / 10;
+
+	hw->sample_delay_factor = delay;
+}
+
+static int enable_edo_mode(struct gpmi_nand_data *this, int mode)
+{
+	struct resources  *r = &this->resources;
+	struct nand_chip *nand = &this->nand;
+	struct mtd_info	 *mtd = &this->mtd;
+	uint8_t feature[ONFI_SUBFEATURE_PARAM_LEN] = {};
+	unsigned long rate;
+	int ret;
+
+	nand->select_chip(mtd, 0);
+
+	/* [1] send SET FEATURE commond to NAND */
+	feature[0] = mode;
+	ret = nand->onfi_set_features(mtd, nand,
+				ONFI_FEATURE_ADDR_TIMING_MODE, feature);
+	if (ret)
+		goto err_out;
+
+	/* [2] send GET FEATURE command to double-check the timing mode */
+	memset(feature, 0, ONFI_SUBFEATURE_PARAM_LEN);
+	ret = nand->onfi_get_features(mtd, nand,
+				ONFI_FEATURE_ADDR_TIMING_MODE, feature);
+	if (ret)
+		goto err_out;
+
+	nand->select_chip(mtd, -1);
+
+	/* [3] set the main IO clock, 100MHz for mode 5, 80MHz for mode 4. */
+	rate = (mode == 5) ? 100000000 : 80000000;
+	clk_set_rate(r->clock[0], rate);
+
+	this->flags |= GPMI_ASYNC_EDO_ENABLED;
+	this->timing_mode = mode;
+	pr_info("gpmi-nand enters asynchronous EDO mode %d\n", mode);
+	return 0;
+
+err_out:
+	nand->select_chip(mtd, -1);
+	dev_err(this->dev, "mode:%d ,failed in set feature.\n", mode);
+	return -EINVAL;
+}
+
+int gpmi_extra_init(struct gpmi_nand_data *this)
+{
+	struct nand_chip *chip = &this->nand;
+
+	/* Enable the asynchronous EDO feature. */
+	if (GPMI_IS_MX6Q(this) && chip->onfi_version) {
+		int mode = onfi_get_async_timing_mode(chip);
+
+		/* We only support the timing mode 4 and mode 5. */
+		if (mode & ONFI_TIMING_MODE_5)
+			mode = 5;
+		else if (mode & ONFI_TIMING_MODE_4)
+			mode = 4;
+		else
+			return 0;
+
+		return enable_edo_mode(this, mode);
+	}
+	return 0;
+}
+
 /* Begin the I/O */
 void gpmi_begin(struct gpmi_nand_data *this)
 {
@@ -755,7 +964,10 @@ void gpmi_begin(struct gpmi_nand_data *this)
 		goto err_out;
 	}
 
-	gpmi_nfc_compute_hardware_timing(this, &hw);
+	if (this->flags & GPMI_ASYNC_EDO_ENABLED)
+		gpmi_compute_edo_timing(this, &hw);
+	else
+		gpmi_nfc_compute_hardware_timing(this, &hw);
 
 	/* [1] Set HW_GPMI_TIMING0 */
 	reg = BF_GPMI_TIMING0_ADDRESS_SETUP(hw.address_setup_in_cycles) |
diff --git a/drivers/mtd/nand/gpmi-nand/gpmi-nand.c b/drivers/mtd/nand/gpmi-nand/gpmi-nand.c
index cc74ebf..684c456 100644
--- a/drivers/mtd/nand/gpmi-nand/gpmi-nand.c
+++ b/drivers/mtd/nand/gpmi-nand/gpmi-nand.c
@@ -1514,6 +1514,14 @@ static int gpmi_scan_bbt(struct mtd_info *mtd)
 	if (ret)
 		return ret;
 
+	/*
+	 * Can we enable the extra features? such as EDO or Sync mode.
+	 *
+	 * We do not check the return value now. That's means if we fail in
+	 * enable the extra features, we still can run in the normal way.
+	 */
+	gpmi_extra_init(this);
+
 	/* use the default BBT implementation */
 	return nand_default_bbt(mtd);
 }
diff --git a/drivers/mtd/nand/gpmi-nand/gpmi-nand.h b/drivers/mtd/nand/gpmi-nand/gpmi-nand.h
index 39f1f76..fe52a3b 100644
--- a/drivers/mtd/nand/gpmi-nand/gpmi-nand.h
+++ b/drivers/mtd/nand/gpmi-nand/gpmi-nand.h
@@ -122,6 +122,10 @@ struct nand_timing {
 };
 
 struct gpmi_nand_data {
+	/* flags */
+#define GPMI_ASYNC_EDO_ENABLED	(1 << 0)
+	int			flags;
+
 	/* System Interface */
 	struct device		*dev;
 	struct platform_device	*pdev;
@@ -132,6 +136,7 @@ struct gpmi_nand_data {
 
 	/* Flash Hardware */
 	struct nand_timing	timing;
+	int			timing_mode;
 
 	/* BCH */
 	struct bch_geometry	bch_geometry;
@@ -258,6 +263,7 @@ extern int start_dma_with_bch_irq(struct gpmi_nand_data *,
 
 /* GPMI-NAND helper function library */
 extern int gpmi_init(struct gpmi_nand_data *);
+extern int gpmi_extra_init(struct gpmi_nand_data *);
 extern void gpmi_clear_bch(struct gpmi_nand_data *);
 extern void gpmi_dump_info(struct gpmi_nand_data *);
 extern int bch_set_geometry(struct gpmi_nand_data *);
-- 
1.7.0.4

[PATCH 9/9] mtd: gpmi: initialize the timing registers only one time

[Huang Shijie]

The current code initialize the timing registers at very time
we call the gpmi_begin(). This really wastes the cpu cycles.

Add a flag to let the gpmi driver initializes the timing registers
only one time.

Signed-off-by: Huang Shijie <b32955@freescale.com>
---
 drivers/mtd/nand/gpmi-nand/gpmi-lib.c  |    8 ++++++++
 drivers/mtd/nand/gpmi-nand/gpmi-nand.h |    1 +
 2 files changed, 9 insertions(+), 0 deletions(-)

diff --git a/drivers/mtd/nand/gpmi-nand/gpmi-lib.c b/drivers/mtd/nand/gpmi-nand/gpmi-lib.c
index 7efd326..c374236 100644
--- a/drivers/mtd/nand/gpmi-nand/gpmi-lib.c
+++ b/drivers/mtd/nand/gpmi-nand/gpmi-lib.c
@@ -914,6 +914,9 @@ static int enable_edo_mode(struct gpmi_nand_data *this, int mode)
 	rate = (mode == 5) ? 100000000 : 80000000;
 	clk_set_rate(r->clock[0], rate);
 
+	/* Let the gpmi_begin() re-compute the timing again. */
+	this->flags &= ~GPMI_TIMING_INIT_OK;
+
 	this->flags |= GPMI_ASYNC_EDO_ENABLED;
 	this->timing_mode = mode;
 	pr_info("gpmi-nand enters asynchronous EDO mode %d\n", mode);
@@ -964,6 +967,11 @@ void gpmi_begin(struct gpmi_nand_data *this)
 		goto err_out;
 	}
 
+	/* Only initialize the timing once */
+	if (this->flags & GPMI_TIMING_INIT_OK)
+		return;
+	this->flags |= GPMI_TIMING_INIT_OK;
+
 	if (this->flags & GPMI_ASYNC_EDO_ENABLED)
 		gpmi_compute_edo_timing(this, &hw);
 	else
diff --git a/drivers/mtd/nand/gpmi-nand/gpmi-nand.h b/drivers/mtd/nand/gpmi-nand/gpmi-nand.h
index fe52a3b..1e01a03 100644
--- a/drivers/mtd/nand/gpmi-nand/gpmi-nand.h
+++ b/drivers/mtd/nand/gpmi-nand/gpmi-nand.h
@@ -124,6 +124,7 @@ struct nand_timing {
 struct gpmi_nand_data {
 	/* flags */
 #define GPMI_ASYNC_EDO_ENABLED	(1 << 0)
+#define GPMI_TIMING_INIT_OK	(1 << 1)
 	int			flags;
 
 	/* System Interface */
-- 
1.7.0.4