add EDO feature for gpmi-nand driver
Subsystems (2)
cover add EDO feature for gpmi-nand driver
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: 2mtd_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 speedmtd_speedtest: eraseblock read speed is 3632 KiB/s
.......................................mtd_speedtest: testing page read speedmtd_speedtest: page read speed is 3554 KiB/s
.......................................mtd_speedtest: testing 2 page read speedmtd_speedtest: 2 page read speed is 3592 KiB/s
.......................................
=================================================
2) The test result AFTER we add the EDO feature:
=================================================mtd_speedtest: MTD device: 2mtd_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 speedmtd_speedtest: eraseblock read speed is 19555 KiB/s
.......................................mtd_speedtest: testing page read speedmtd_speedtest: page read speed is 17319 KiB/s
.......................................mtd_speedtest: testing 2 page read speedmtd_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
Add the set-features(0xef)/get-features(0xee) helpers for ONFI nand. Also add the necessary macros.
--- drivers/mtd/nand/nand_base.c | 50 ++++++++++++++++++++++++++++++++++++++++++ include/linux/mtd/nand.h | 14 +++++++++++ 2 files changed, 64 insertions(+), 0 deletions(-)
drivers/mtd/nand/nand_base.c +50 -0
@@ -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
include/linux/mtd/nand.h +14 -0
@@ -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;
Signature · 1 line
1.7.0.4
PATCH 2/9 mtd: add helpers to get the supportted ONFI timing mode
In reply to Huang Shijieadd 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.
--- include/linux/mtd/nand.h | 25 +++++++++++++++++++++++++ 1 files changed, 25 insertions(+), 0 deletions(-)
include/linux/mtd/nand.h +25 -0
@@ -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 */
Signature · 1 line
1.7.0.4
PATCH 3/9 mtd: gpmi: add a new field for HW_GPMI_TIMING1
In reply to Huang ShijieThe 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.
--- 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(-)
drivers/mtd/nand/gpmi-nand/gpmi-lib.c +9 -8
@@ -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. */
drivers/mtd/nand/gpmi-nand/gpmi-nand.h +9 -0
@@ -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; };
drivers/mtd/nand/gpmi-nand/gpmi-regs.h +3 -0
@@ -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
Signature · 1 line
1.7.0.4
PATCH 3/9 mtd: gpmi: add a new field for HW_GPMI_TIMING1
Hello Huang Shijie,
Quoted 21 lines · 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_TIMING0andHW_GPMI_CTRL1.So it is better to add a new field setting for
HW_GPMI_TIMING1in 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<[email protected]>--- 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(-)drivers/mtd/nand/gpmi-nand/gpmi-lib.c +0
-0
drivers/mtd/nand/gpmi-nand/gpmi-lib.c return_results: · line 731
Quoted 4 lines
drivers/mtd/nand/gpmi-nand/gpmi-lib.c +1
-0@@ -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?
Quoted 25 lines
drivers/mtd/nand/gpmi-nand/gpmi-lib.c +1
-6/* 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 ); -/*
drivers/mtd/nand/gpmi-nand/gpmi-lib.c void gpmi_begin(struct gpmi_nand_data *this) · line 776
Quoted 10 lines
drivers/mtd/nand/gpmi-nand/gpmi-lib.c +7
-2-* 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?
Quoted 34 lines
drivers/mtd/nand/gpmi-nand/gpmi-lib.c +0
-0/* Clear out the DLL control fields. */ drivers/mtd/nand/gpmi-nand/gpmi-nand.h +9
-0@@ -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; };drivers/mtd/nand/gpmi-nand/gpmi-regs.h +0
-0
drivers/mtd/nand/gpmi-nand/gpmi-regs.h line 157
Quoted 4 lines
drivers/mtd/nand/gpmi-nand/gpmi-regs.h +1
-0@@ -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!
drivers/mtd/nand/gpmi-nand/gpmi-regs.h line 162
Quoted 5 lines
drivers/mtd/nand/gpmi-nand/gpmi-regs.h +2
-0+#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
? 2012?09?12? 02:00, Vikram Narayanan ??:
Quoted 19 lines
Hello Huang Shijie,
Quoted 16 lines · 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_TIMING0andHW_GPMI_CTRL1.So it is better to add a new field setting for
HW_GPMI_TIMING1in 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<[email protected]>---
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(-)
Quoted 12 lines
drivers/mtd/nand/gpmi-nand/gpmi-lib.c return_results: · line 731
Quoted 10 lines
drivers/mtd/nand/gpmi-nand/gpmi-lib.c +1
-0@@ -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.
Quoted 25 lines
Quoted 25 lines
drivers/mtd/nand/gpmi-nand/gpmi-lib.c +1
-5/* 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);
Quoted 15 lines
Quoted 10 lines
- /*
- *DLL_ENABLEmust be set to 0 when settingRDN_DELAYorHALF_PERIOD.
- */
+ /* [2] SetHW_GPMI_TIMING1*/
+ writel(BF_GPMI_TIMING1_BUSY_TIMEOUT(hw.device_busy_timeout),
+gpmi_regs+HW_GPMI_TIMING1);drivers/mtd/nand/gpmi-nand/gpmi-lib.c +4
-0+ +/* [3] The following code is to set the HW_GPMI_CTRL1. */ + +/* DLL_ENABLE must be set to 0 when setting RDN_DELAY or Quoted 2 lines
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?
Quoted 38 lines
Quoted 38 lines
/* Clear out the DLL control fields. */
drivers/mtd/nand/gpmi-nand/gpmi-nand.h +9
-0@@ -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; };drivers/mtd/nand/gpmi-nand/gpmi-regs.h +0
-0@@ -154,6 +154,9 @@
Quoted 8 lines
drivers/mtd/nand/gpmi-nand/gpmi-regs.h
Quoted 4 lines
drivers/mtd/nand/gpmi-nand/gpmi-regs.h +1
-0#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
Re: [PATCH 3/9] mtd: gpmi: add a new field for HW_GPMI_TIMING1
Hello Huang Shijie,
<Dropped Cc's>
Quoted 20 lines · Huang Shijie
Quoted 19 lines · Vikram Narayanan
Hello Huang Shijie,
Quoted 16 lines · 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_TIMING0andHW_GPMI_CTRL1.So it is better to add a new field setting for
HW_GPMI_TIMING1in 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<[email protected]>---
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(-)
<snip>
Quoted 22 lines
Quoted 7 lines
Quoted 7 lines
+ /* [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);Quoted 15 lines
Quoted 10 lines
- /*
- *DLL_ENABLEmust be set to 0 when settingRDN_DELAYorHALF_PERIOD.
- */
+ /* [2] SetHW_GPMI_TIMING1*/
+ writel(BF_GPMI_TIMING1_BUSY_TIMEOUT(hw.device_busy_timeout),
+gpmi_regs+HW_GPMI_TIMING1);drivers/mtd/nand/gpmi-nand/gpmi-lib.c +4
-0+ +/* [3] The following code is to set the HW_GPMI_CTRL1. */ + +/* DLL_ENABLE must be set to 0 when setting RDN_DELAY or Quoted 2 lines
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?
Quoted 4 lines
no.
These notations just help to comment the steps of setting timing registers. Are you confused at this?
Not much. But this is the notation usually used for references. you can keep this as it is. Not an issue.
Regards,
Vikram
PATCH 4/9 mtd: gpmi: do not get the clock frequency in gpmi_begin()
In reply to Huang ShijieThe 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.
--- drivers/mtd/nand/gpmi-nand/gpmi-lib.c | 8 +++----- 1 files changed, 3 insertions(+), 5 deletions(-)
drivers/mtd/nand/gpmi-nand/gpmi-lib.c +3 -5
@@ -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)
Signature · 1 line
1.7.0.4
PATCH 5/9 mtd: gpmi: add a new field for HW_GPMI_CTRL1
In reply to Huang Shijieadd the WRN_DLY_SEL field for HW_GPMI_CTRL1. This field is used as delay for gpmi write strobe.
--- 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(-)
drivers/mtd/nand/gpmi-nand/gpmi-lib.c +6 -0
@@ -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 );
drivers/mtd/nand/gpmi-nand/gpmi-nand.h +2 -0
@@ -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; };/**
drivers/mtd/nand/gpmi-nand/gpmi-regs.h +5 -0
@@ -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
Signature · 1 line
1.7.0.4
PATCH 5/9 mtd: gpmi: add a new field for HW_GPMI_CTRL1
Hello Huang Shijie,
Quoted 59 lines · Huang Shijie
add the
WRN_DLY_SELfield forHW_GPMI_CTRL1. This field is used as delay for gpmi write strobe.Signed-off-by Huang Shijie<[email protected]>--- 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(-)drivers/mtd/nand/gpmi-nand/gpmi-lib.c +6
-0@@ -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 );drivers/mtd/nand/gpmi-nand/gpmi-nand.h +2
-0@@ -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; };/** drivers/mtd/nand/gpmi-nand/gpmi-regs.h +0
-0
drivers/mtd/nand/gpmi-nand/gpmi-regs.h line 112
Quoted 5 lines
drivers/mtd/nand/gpmi-nand/gpmi-regs.h +2
-0@@ -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.
drivers/mtd/nand/gpmi-nand/gpmi-regs.h line 118
Quoted 6 lines
drivers/mtd/nand/gpmi-nand/gpmi-regs.h +3
-0+#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
? 2012?09?12? 02:04, Vikram Narayanan ??:
Quoted 74 lines
Hello Huang Shijie,
Quoted 62 lines · Huang Shijie
add the
WRN_DLY_SELfield forHW_GPMI_CTRL1. This field is used as delay for gpmi write strobe.Signed-off-by Huang Shijie<[email protected]>---
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(-)drivers/mtd/nand/gpmi-nand/gpmi-lib.c +0
-0@@ -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;drivers/mtd/nand/gpmi-nand/gpmi-lib.c +5
-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 );drivers/mtd/nand/gpmi-nand/gpmi-nand.h +2
-0@@ -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; };/** drivers/mtd/nand/gpmi-nand/gpmi-regs.h +0
-0drivers/mtd/nand/gpmi-nand/gpmi-regs.h line 112
Quoted 5 lines
drivers/mtd/nand/gpmi-nand/gpmi-regs.h +2
-0@@ -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 0x00C00000In [PATCH 3/9] mtd: gpmi: add a new field for
HW_GPMI_TIMING1, the above is done in a different way.
Quoted 6 lines
#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
In reply to Huang ShijieThe setting DLL code is a little mess.
Just simplify the code and the comments.
--- drivers/mtd/nand/gpmi-nand/gpmi-lib.c | 22 +++++++++------------- 1 files changed, 9 insertions(+), 13 deletions(-)
drivers/mtd/nand/gpmi-nand/gpmi-lib.c +9 -13
@@ -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 ;
Signature · 1 line
1.7.0.4
PATCH 6/9 mtd: gpmi: simplify the setting DLL code
Hello Huang Shijie,
Quoted 41 lines · Huang Shijie
The setting DLL code is a little mess.
Just simplify the code and the comments.Signed-off-by Huang Shijie<[email protected]>--- drivers/mtd/nand/gpmi-nand/gpmi-lib.c | 22 +++++++++------------- 1 files changed, 9 insertions(+), 13 deletions(-)drivers/mtd/nand/gpmi-nand/gpmi-lib.c +7
-10@@ -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 );
drivers/mtd/nand/gpmi-nand/gpmi-lib.c void gpmi_begin(struct gpmi_nand_data *this) · line 803
Quoted 10 lines
drivers/mtd/nand/gpmi-nand/gpmi-lib.c +2
-3/* * 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.
drivers/mtd/nand/gpmi-nand/gpmi-lib.c void gpmi_begin(struct gpmi_nand_data *this) · line 804
Quoted 1 line
drivers/mtd/nand/gpmi-nand/gpmi-lib.c +0
-0dll_wait_time_in_us = (clock_period_in_ns *64 ) /1000 ;
Regards,
Vikram
PATCH 6/9 mtd: gpmi: simplify the setting DLL code
? 2012?09?12? 02:08, Vikram Narayanan ??:
Quoted 46 lines
Hello Huang Shijie,
Quoted 43 lines · Huang Shijie
The setting DLL code is a little mess.
Just simplify the code and the comments.Signed-off-by Huang Shijie<[email protected]>---
drivers/mtd/nand/gpmi-nand/gpmi-lib.c| 22 +++++++++-------------1 files changed, 9 insertions(+), 13 deletions(-)drivers/mtd/nand/gpmi-nand/gpmi-lib.c +7
-10@@ -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 );
Quoted 11 lines
drivers/mtd/nand/gpmi-nand/gpmi-lib.c void gpmi_begin(struct gpmi_nand_data *this) · line 803
Quoted 9 lines
drivers/mtd/nand/gpmi-nand/gpmi-lib.c +2
-3/* * 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_SECmacro in the above statement?
thanks. I like this macro.
Quoted 4 lines
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
Quoted 6 lines
drivers/mtd/nand/gpmi-nand/gpmi-lib.c void gpmi_begin(struct gpmi_nand_data *this) · line 804
Quoted 2 lines
drivers/mtd/nand/gpmi-nand/gpmi-lib.c +0
-0dll_wait_time_in_us = (clock_period_in_ns *64 ) /1000 ;Regards,
Vikram
PATCH 6/9 mtd: gpmi: simplify the setting DLL code
In reply to Vikram Narayanan? 2012?09?12? 02:08, Vikram Narayanan ??:
Quoted 11 lines
Quoted 10 lines
/*
* 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]);
Quoted 4 lines
NSEC_PER_SECmacro 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
In reply to Huang ShijieThe 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.
--- drivers/mtd/nand/gpmi-nand/gpmi-nand.c | 15 +++++---------- 1 files changed, 5 insertions(+), 10 deletions(-)
drivers/mtd/nand/gpmi-nand/gpmi-nand.c +5 -10
@@ -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:
Signature · 1 line
1.7.0.4
PATCH 8/9 mtd: gpmi: add EDO feature for imx6q
In reply to Huang ShijieWhen 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: 2mtd_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 speedmtd_speedtest: eraseblock read speed is 3632 KiB/s
.......................................mtd_speedtest: testing page read speedmtd_speedtest: page read speed is 3554 KiB/s
.......................................mtd_speedtest: testing 2 page read speedmtd_speedtest: 2 page read speed is 3592 KiB/s
.......................................
=================================================
2) The test result AFTER we add the EDO feature:
=================================================mtd_speedtest: MTD device: 2mtd_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 speedmtd_speedtest: eraseblock read speed is 19555 KiB/s
.......................................mtd_speedtest: testing page read speedmtd_speedtest: page read speed is 17319 KiB/s
.......................................mtd_speedtest: testing 2 page read speedmtd_speedtest: 2 page read speed is 18339 KiB/s
.......................................
=================================================
The read data performance is much improved by more then 5 times.
--- 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(-)
drivers/mtd/nand/gpmi-nand/gpmi-lib.c +213 -1
@@ -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) |
drivers/mtd/nand/gpmi-nand/gpmi-nand.c +8 -0
@@ -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); }
drivers/mtd/nand/gpmi-nand/gpmi-nand.h +6 -0
@@ -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 *);
Signature · 1 line
1.7.0.4
PATCH 9/9 mtd: gpmi: initialize the timing registers only one time
In reply to Huang ShijieThe 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.
--- 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(-)
drivers/mtd/nand/gpmi-nand/gpmi-lib.c +8 -0
@@ -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
drivers/mtd/nand/gpmi-nand/gpmi-nand.h +1 -0
@@ -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 */
Signature · 1 line
1.7.0.4