* [PATCH 0/3] mtd: onenand: add cache program feature for 4kb page onenand
@ 2010-11-02 11:25 Roman Tereshonkov
2010-11-02 11:25 ` [PATCH 1/3] mtd: onenand: add option and variable for cache program feature Roman Tereshonkov
` (3 more replies)
0 siblings, 4 replies; 11+ messages in thread
From: Roman Tereshonkov @ 2010-11-02 11:25 UTC (permalink / raw)
To: linux-mtd; +Cc: kyungmin.park, Roman Tereshonkov
Implement cache program feature for 4KB page onenand.
This feature improves the write data performance.
The observed 128KB data program speed change is
from 8827KB/s to 14156 KB/s when the feature is enabled.
Roman Tereshonkov (3):
mtd: onenand: add option and variable for cache program feature
mtd: onenand: introduce not "real" commands for cache program
feature.
mtd: onenand: implement cache program feature for 4kb page onenand
drivers/mtd/onenand/omap2.c | 12 ++++++++--
drivers/mtd/onenand/onenand_base.c | 39 ++++++++++++++++++++++++++++++++++-
include/linux/mtd/onenand.h | 12 +++++++++++
include/linux/mtd/onenand_regs.h | 2 +
4 files changed, 60 insertions(+), 5 deletions(-)
^ permalink raw reply [flat|nested] 11+ messages in thread* [PATCH 1/3] mtd: onenand: add option and variable for cache program feature 2010-11-02 11:25 [PATCH 0/3] mtd: onenand: add cache program feature for 4kb page onenand Roman Tereshonkov @ 2010-11-02 11:25 ` Roman Tereshonkov 2010-11-02 11:25 ` [PATCH 2/3] mtd: onenand: introduce not "real" commands " Roman Tereshonkov ` (2 subsequent siblings) 3 siblings, 0 replies; 11+ messages in thread From: Roman Tereshonkov @ 2010-11-02 11:25 UTC (permalink / raw) To: linux-mtd; +Cc: kyungmin.park, Roman Tereshonkov A new option is added for cache program feature. A new variable ongoing is introduced for onenand_chip to handle the multi-command cache program operation. Signed-off-by: Roman Tereshonkov <roman.tereshonkov@nokia.com> --- include/linux/mtd/onenand.h | 12 ++++++++++++ 1 files changed, 12 insertions(+), 0 deletions(-) diff --git a/include/linux/mtd/onenand.h b/include/linux/mtd/onenand.h index 0c8815b..6da3fe3 100644 --- a/include/linux/mtd/onenand.h +++ b/include/linux/mtd/onenand.h @@ -137,6 +137,14 @@ struct onenand_chip { void *bbm; void *priv; + + /* + * Shows that the current operation is composed + * of sequence of commands. For example, cache program. + * Such command status OnGo bit is checked at the end of + * sequence. + */ + unsigned int ongoing; }; /* @@ -171,6 +179,9 @@ struct onenand_chip { #define ONENAND_IS_2PLANE(this) (0) #endif +#define ONENAND_IS_CACHE_PROGRAM(this) \ + (this->options & ONENAND_HAS_CACHE_PROGRAM) + /* Check byte access in OneNAND */ #define ONENAND_CHECK_BYTE_ACCESS(addr) (addr & 0x1) @@ -181,6 +192,7 @@ struct onenand_chip { #define ONENAND_HAS_UNLOCK_ALL (0x0002) #define ONENAND_HAS_2PLANE (0x0004) #define ONENAND_HAS_4KB_PAGE (0x0008) +#define ONENAND_HAS_CACHE_PROGRAM (0x0010) #define ONENAND_SKIP_UNLOCK_CHECK (0x0100) #define ONENAND_PAGEBUF_ALLOC (0x1000) #define ONENAND_OOBBUF_ALLOC (0x2000) -- 1.7.0.4 ^ permalink raw reply related [flat|nested] 11+ messages in thread
* [PATCH 2/3] mtd: onenand: introduce not "real" commands for cache program feature. 2010-11-02 11:25 [PATCH 0/3] mtd: onenand: add cache program feature for 4kb page onenand Roman Tereshonkov 2010-11-02 11:25 ` [PATCH 1/3] mtd: onenand: add option and variable for cache program feature Roman Tereshonkov @ 2010-11-02 11:25 ` Roman Tereshonkov 2010-11-02 11:25 ` [PATCH 3/3] mtd: onenand: implement cache program feature for 4kb page onenand Roman Tereshonkov 2010-11-02 14:01 ` [PATCH 0/3] mtd: onenand: add " Kyungmin Park 3 siblings, 0 replies; 11+ messages in thread From: Roman Tereshonkov @ 2010-11-02 11:25 UTC (permalink / raw) To: linux-mtd; +Cc: kyungmin.park, Roman Tereshonkov Introduce not "real" commands for start/continue and stop the cache program operation. Signed-off-by: Roman Tereshonkov <roman.tereshonkov@nokia.com> --- include/linux/mtd/onenand_regs.h | 2 ++ 1 files changed, 2 insertions(+), 0 deletions(-) diff --git a/include/linux/mtd/onenand_regs.h b/include/linux/mtd/onenand_regs.h index cd6f3b4..ce61e9c 100644 --- a/include/linux/mtd/onenand_regs.h +++ b/include/linux/mtd/onenand_regs.h @@ -143,6 +143,8 @@ /* NOTE: Those are not *REAL* commands */ #define ONENAND_CMD_BUFFERRAM (0x1978) #define FLEXONENAND_CMD_READ_PI (0x1985) +#define ONENAND_CMD_CACHE_PROG (0x1986) +#define ONENAND_CMD_CACHE_PROG_LAST (0x1987) /* * System Configuration 1 Register F221h (R, R/W) -- 1.7.0.4 ^ permalink raw reply related [flat|nested] 11+ messages in thread
* [PATCH 3/3] mtd: onenand: implement cache program feature for 4kb page onenand 2010-11-02 11:25 [PATCH 0/3] mtd: onenand: add cache program feature for 4kb page onenand Roman Tereshonkov 2010-11-02 11:25 ` [PATCH 1/3] mtd: onenand: add option and variable for cache program feature Roman Tereshonkov 2010-11-02 11:25 ` [PATCH 2/3] mtd: onenand: introduce not "real" commands " Roman Tereshonkov @ 2010-11-02 11:25 ` Roman Tereshonkov 2010-11-02 14:00 ` Kyungmin Park 2010-11-02 14:01 ` [PATCH 0/3] mtd: onenand: add " Kyungmin Park 3 siblings, 1 reply; 11+ messages in thread From: Roman Tereshonkov @ 2010-11-02 11:25 UTC (permalink / raw) To: linux-mtd; +Cc: kyungmin.park, Roman Tereshonkov Implement cache program feature for 4KB page onenand. This feature improves the write data performance. The observed 128KB data program speed change is from 8827KB/s to 14156 KB/s when the feature is enabled. Signed-off-by: Roman Tereshonkov <roman.tereshonkov@nokia.com> --- drivers/mtd/onenand/omap2.c | 12 ++++++++-- drivers/mtd/onenand/onenand_base.c | 39 ++++++++++++++++++++++++++++++++++- 2 files changed, 46 insertions(+), 5 deletions(-) diff --git a/drivers/mtd/onenand/omap2.c b/drivers/mtd/onenand/omap2.c index 9f322f1..da25a90 100644 --- a/drivers/mtd/onenand/omap2.c +++ b/drivers/mtd/onenand/omap2.c @@ -108,8 +108,9 @@ static void wait_warn(char *msg, int state, unsigned int ctrl, static int omap2_onenand_wait(struct mtd_info *mtd, int state) { struct omap2_onenand *c = container_of(mtd, struct omap2_onenand, mtd); + struct onenand_chip *this = mtd->priv; unsigned int intr = 0; - unsigned int ctrl; + unsigned int ctrl, ctrl_mask; unsigned long timeout; u32 syscfg; @@ -180,7 +181,8 @@ retry: if (result == 0) { /* Timeout after 20ms */ ctrl = read_reg(c, ONENAND_REG_CTRL_STATUS); - if (ctrl & ONENAND_CTRL_ONGO) { + if (ctrl & ONENAND_CTRL_ONGO && + !this->ongoing) { /* * The operation seems to be still going * so give it some more time. @@ -269,7 +271,11 @@ retry: return -EIO; } - if (ctrl & 0xFE9F) + ctrl_mask = 0xFE9F; + if (this->ongoing) + ctrl_mask &= ~0x8000; + + if (ctrl & ctrl_mask) wait_warn("unexpected controller status", state, ctrl, intr); return 0; diff --git a/drivers/mtd/onenand/onenand_base.c b/drivers/mtd/onenand/onenand_base.c index 6b3a875..b2c3e97 100644 --- a/drivers/mtd/onenand/onenand_base.c +++ b/drivers/mtd/onenand/onenand_base.c @@ -440,6 +440,19 @@ static int onenand_command(struct mtd_info *mtd, int cmd, loff_t addr, size_t le default: if (ONENAND_IS_2PLANE(this) && cmd == ONENAND_CMD_PROG) cmd = ONENAND_CMD_2X_PROG; + + /* Exclude 1st OTP and OTP blocks */ + if (ONENAND_IS_CACHE_PROGRAM(this) && + likely(onenand_block(this, addr) != 0) && + ONENAND_IS_4KB_PAGE(this)) { + + if (cmd == ONENAND_CMD_CACHE_PROG) + cmd = ONENAND_CMD_2X_CACHE_PROG; + else if (cmd == ONENAND_CMD_CACHE_PROG_LAST) + cmd = ONENAND_CMD_PROG; + + } + dataram = ONENAND_CURRENT_BUFFERRAM(this); break; } @@ -1845,7 +1858,7 @@ static int onenand_write_ops_nolock(struct mtd_info *mtd, loff_t to, const u_char *buf = ops->datbuf; const u_char *oob = ops->oobbuf; u_char *oobbuf; - int ret = 0; + int ret = 0, cmd; DEBUG(MTD_DEBUG_LEVEL3, "%s: to = 0x%08x, len = %i\n", __func__, (unsigned int) to, (int) len); @@ -1954,7 +1967,25 @@ static int onenand_write_ops_nolock(struct mtd_info *mtd, loff_t to, ONENAND_SET_NEXT_BUFFERRAM(this); } - this->command(mtd, ONENAND_CMD_PROG, to, mtd->writesize); + this->ongoing = 0; + + /* Exclude 1st OTP and OTP blocks */ + if (ONENAND_IS_CACHE_PROGRAM(this) && + likely(onenand_block(this, to) != 0) && + ONENAND_IS_4KB_PAGE(this)) { + + if ((written + thislen) < len) { + cmd = ONENAND_CMD_CACHE_PROG; + this->ongoing = 1; + } else { + cmd = ONENAND_CMD_CACHE_PROG_LAST; + } + + } else { + cmd = ONENAND_CMD_PROG; + } + + this->command(mtd, cmd, to, mtd->writesize); /* * 2 PLANE, MLC, and Flex-OneNAND wait here @@ -3380,6 +3411,8 @@ static void onenand_check_features(struct mtd_info *mtd) else if (numbufs == 1) this->options |= ONENAND_HAS_4KB_PAGE; + this->options |= ONENAND_HAS_CACHE_PROGRAM; + case ONENAND_DEVICE_DENSITY_2Gb: /* 2Gb DDP does not have 2 plane */ if (!ONENAND_IS_DDP(this)) @@ -3415,6 +3448,8 @@ static void onenand_check_features(struct mtd_info *mtd) printk(KERN_DEBUG "Chip has 2 plane\n"); if (this->options & ONENAND_HAS_4KB_PAGE) printk(KERN_DEBUG "Chip has 4KiB pagesize\n"); + if (this->options & ONENAND_HAS_CACHE_PROGRAM) + printk(KERN_DEBUG "Chip has cache program feature\n"); } /** -- 1.7.0.4 ^ permalink raw reply related [flat|nested] 11+ messages in thread
* Re: [PATCH 3/3] mtd: onenand: implement cache program feature for 4kb page onenand 2010-11-02 11:25 ` [PATCH 3/3] mtd: onenand: implement cache program feature for 4kb page onenand Roman Tereshonkov @ 2010-11-02 14:00 ` Kyungmin Park 2010-11-02 14:37 ` Roman Tereshonkov 0 siblings, 1 reply; 11+ messages in thread From: Kyungmin Park @ 2010-11-02 14:00 UTC (permalink / raw) To: Roman Tereshonkov; +Cc: kyungmin.park, linux-mtd Hi, On Tue, Nov 2, 2010 at 8:25 PM, Roman Tereshonkov <roman.tereshonkov@nokia.com> wrote: > Implement cache program feature for 4KB page onenand. > This feature improves the write data performance. > The observed 128KB data program speed change is > from 8827KB/s to 14156 KB/s when the feature is enabled. > > Signed-off-by: Roman Tereshonkov <roman.tereshonkov@nokia.com> > --- > drivers/mtd/onenand/omap2.c | 12 ++++++++-- > drivers/mtd/onenand/onenand_base.c | 39 ++++++++++++++++++++++++++++++++++- > 2 files changed, 46 insertions(+), 5 deletions(-) Please make separate the two patches, one for OMAP, another is for onenand_base.c > > diff --git a/drivers/mtd/onenand/omap2.c b/drivers/mtd/onenand/omap2.c > index 9f322f1..da25a90 100644 > --- a/drivers/mtd/onenand/omap2.c > +++ b/drivers/mtd/onenand/omap2.c > @@ -108,8 +108,9 @@ static void wait_warn(char *msg, int state, unsigned int ctrl, > static int omap2_onenand_wait(struct mtd_info *mtd, int state) > { > struct omap2_onenand *c = container_of(mtd, struct omap2_onenand, mtd); > + struct onenand_chip *this = mtd->priv; > unsigned int intr = 0; > - unsigned int ctrl; > + unsigned int ctrl, ctrl_mask; > unsigned long timeout; > u32 syscfg; > > @@ -180,7 +181,8 @@ retry: > if (result == 0) { > /* Timeout after 20ms */ > ctrl = read_reg(c, ONENAND_REG_CTRL_STATUS); > - if (ctrl & ONENAND_CTRL_ONGO) { > + if (ctrl & ONENAND_CTRL_ONGO && > + !this->ongoing) { > /* > * The operation seems to be still going > * so give it some more time. > @@ -269,7 +271,11 @@ retry: > return -EIO; > } > > - if (ctrl & 0xFE9F) > + ctrl_mask = 0xFE9F; > + if (this->ongoing) > + ctrl_mask &= ~0x8000; > + > + if (ctrl & ctrl_mask) > wait_warn("unexpected controller status", state, ctrl, intr); > > return 0; > diff --git a/drivers/mtd/onenand/onenand_base.c b/drivers/mtd/onenand/onenand_base.c > index 6b3a875..b2c3e97 100644 > --- a/drivers/mtd/onenand/onenand_base.c > +++ b/drivers/mtd/onenand/onenand_base.c > @@ -440,6 +440,19 @@ static int onenand_command(struct mtd_info *mtd, int cmd, loff_t addr, size_t le > default: > if (ONENAND_IS_2PLANE(this) && cmd == ONENAND_CMD_PROG) > cmd = ONENAND_CMD_2X_PROG; > + > + /* Exclude 1st OTP and OTP blocks */ Are there any reason? I think 1st OTP block is just block 0 so no need to consider block 0 case. > + if (ONENAND_IS_CACHE_PROGRAM(this) && > + likely(onenand_block(this, addr) != 0) && > + ONENAND_IS_4KB_PAGE(this)) { and I think no need to check 4KB_PAGE. > + > + if (cmd == ONENAND_CMD_CACHE_PROG) > + cmd = ONENAND_CMD_2X_CACHE_PROG; > + else if (cmd == ONENAND_CMD_CACHE_PROG_LAST) > + cmd = ONENAND_CMD_PROG; > + > + } > + > dataram = ONENAND_CURRENT_BUFFERRAM(this); > break; > } > @@ -1845,7 +1858,7 @@ static int onenand_write_ops_nolock(struct mtd_info *mtd, loff_t to, > const u_char *buf = ops->datbuf; > const u_char *oob = ops->oobbuf; > u_char *oobbuf; > - int ret = 0; > + int ret = 0, cmd; > > DEBUG(MTD_DEBUG_LEVEL3, "%s: to = 0x%08x, len = %i\n", > __func__, (unsigned int) to, (int) len); > @@ -1954,7 +1967,25 @@ static int onenand_write_ops_nolock(struct mtd_info *mtd, loff_t to, > ONENAND_SET_NEXT_BUFFERRAM(this); > } > > - this->command(mtd, ONENAND_CMD_PROG, to, mtd->writesize); > + this->ongoing = 0; > + > + /* Exclude 1st OTP and OTP blocks */ > + if (ONENAND_IS_CACHE_PROGRAM(this) && > + likely(onenand_block(this, to) != 0) && > + ONENAND_IS_4KB_PAGE(this)) { check the length here instead of 4KB_PAGE. > + > + if ((written + thislen) < len) { > + cmd = ONENAND_CMD_CACHE_PROG; > + this->ongoing = 1; > + } else { > + cmd = ONENAND_CMD_CACHE_PROG_LAST; > + } Need to check how to pass the cmd properly. How/When send the CACHE_PROG_LAST? > + > + } else { > + cmd = ONENAND_CMD_PROG; > + } > + > + this->command(mtd, cmd, to, mtd->writesize); > > /* > * 2 PLANE, MLC, and Flex-OneNAND wait here > @@ -3380,6 +3411,8 @@ static void onenand_check_features(struct mtd_info *mtd) > else if (numbufs == 1) > this->options |= ONENAND_HAS_4KB_PAGE; > > + this->options |= ONENAND_HAS_CACHE_PROGRAM; It also needs to set CACHE_PROGRAM option. > + > case ONENAND_DEVICE_DENSITY_2Gb: > /* 2Gb DDP does not have 2 plane */ > if (!ONENAND_IS_DDP(this)) > @@ -3415,6 +3448,8 @@ static void onenand_check_features(struct mtd_info *mtd) > printk(KERN_DEBUG "Chip has 2 plane\n"); > if (this->options & ONENAND_HAS_4KB_PAGE) > printk(KERN_DEBUG "Chip has 4KiB pagesize\n"); > + if (this->options & ONENAND_HAS_CACHE_PROGRAM) > + printk(KERN_DEBUG "Chip has cache program feature\n"); > } Thank you, Kyungmin Park ^ permalink raw reply [flat|nested] 11+ messages in thread
* Re: [PATCH 3/3] mtd: onenand: implement cache program feature for 4kb page onenand 2010-11-02 14:00 ` Kyungmin Park @ 2010-11-02 14:37 ` Roman Tereshonkov 2010-11-02 14:50 ` Kyungmin Park 0 siblings, 1 reply; 11+ messages in thread From: Roman Tereshonkov @ 2010-11-02 14:37 UTC (permalink / raw) To: ext Kyungmin Park; +Cc: linux-mtd On Tue, Nov 02, 2010 at 03:00:41PM +0100, ext Kyungmin Park wrote: > Hi, > > On Tue, Nov 2, 2010 at 8:25 PM, Roman Tereshonkov > <roman.tereshonkov@nokia.com> wrote: > > Implement cache program feature for 4KB page onenand. > > This feature improves the write data performance. > > The observed 128KB data program speed change is > > from 8827KB/s to 14156 KB/s when the feature is enabled. > > > > Signed-off-by: Roman Tereshonkov <roman.tereshonkov@nokia.com> > > --- > > drivers/mtd/onenand/omap2.c | 12 ++++++++-- > > drivers/mtd/onenand/onenand_base.c | 39 ++++++++++++++++++++++++++++++++++- > > 2 files changed, 46 insertions(+), 5 deletions(-) > > Please make separate the two patches, one for OMAP, another is for > onenand_base.c > > > > > diff --git a/drivers/mtd/onenand/omap2.c b/drivers/mtd/onenand/omap2.c > > index 9f322f1..da25a90 100644 > > --- a/drivers/mtd/onenand/omap2.c > > +++ b/drivers/mtd/onenand/omap2.c > > @@ -108,8 +108,9 @@ static void wait_warn(char *msg, int state, unsigned int ctrl, > > static int omap2_onenand_wait(struct mtd_info *mtd, int state) > > { > > struct omap2_onenand *c = container_of(mtd, struct omap2_onenand, mtd); > > + struct onenand_chip *this = mtd->priv; > > unsigned int intr = 0; > > - unsigned int ctrl; > > + unsigned int ctrl, ctrl_mask; > > unsigned long timeout; > > u32 syscfg; > > > > @@ -180,7 +181,8 @@ retry: > > if (result == 0) { > > /* Timeout after 20ms */ > > ctrl = read_reg(c, ONENAND_REG_CTRL_STATUS); > > - if (ctrl & ONENAND_CTRL_ONGO) { > > + if (ctrl & ONENAND_CTRL_ONGO && > > + !this->ongoing) { > > /* > > * The operation seems to be still going > > * so give it some more time. > > @@ -269,7 +271,11 @@ retry: > > return -EIO; > > } > > > > - if (ctrl & 0xFE9F) > > + ctrl_mask = 0xFE9F; > > + if (this->ongoing) > > + ctrl_mask &= ~0x8000; > > + > > + if (ctrl & ctrl_mask) > > wait_warn("unexpected controller status", state, ctrl, intr); > > > > return 0; > > diff --git a/drivers/mtd/onenand/onenand_base.c b/drivers/mtd/onenand/onenand_base.c > > index 6b3a875..b2c3e97 100644 > > --- a/drivers/mtd/onenand/onenand_base.c > > +++ b/drivers/mtd/onenand/onenand_base.c > > @@ -440,6 +440,19 @@ static int onenand_command(struct mtd_info *mtd, int cmd, loff_t addr, size_t le > > default: > > if (ONENAND_IS_2PLANE(this) && cmd == ONENAND_CMD_PROG) > > cmd = ONENAND_CMD_2X_PROG; > > + > > + /* Exclude 1st OTP and OTP blocks */ > Are there any reason? I think 1st OTP block is just block 0 so no need > to consider block 0 case. Specification tells: 3.9.1 Cache Program Operation "Note that Cache Program command cannot be performed on OTP block and 1st block OTP" > > + if (ONENAND_IS_CACHE_PROGRAM(this) && > > + likely(onenand_block(this, addr) != 0) && > > + ONENAND_IS_4KB_PAGE(this)) { > and I think no need to check 4KB_PAGE. This patch is for 4kb page onenand only. The 2kb page onenand uses different commands for cache program. Also 2kb page case cache program depends on CONFIG_MTD_ONENAND_2X_PROGRAM. > > + > > + if (cmd == ONENAND_CMD_CACHE_PROG) > > + cmd = ONENAND_CMD_2X_CACHE_PROG; > > + else if (cmd == ONENAND_CMD_CACHE_PROG_LAST) > > + cmd = ONENAND_CMD_PROG; > > + > > + } > > + > > dataram = ONENAND_CURRENT_BUFFERRAM(this); > > break; > > } > > @@ -1845,7 +1858,7 @@ static int onenand_write_ops_nolock(struct mtd_info *mtd, loff_t to, > > const u_char *buf = ops->datbuf; > > const u_char *oob = ops->oobbuf; > > u_char *oobbuf; > > - int ret = 0; > > + int ret = 0, cmd; > > > > DEBUG(MTD_DEBUG_LEVEL3, "%s: to = 0x%08x, len = %i\n", > > __func__, (unsigned int) to, (int) len); > > @@ -1954,7 +1967,25 @@ static int onenand_write_ops_nolock(struct mtd_info *mtd, loff_t to, > > ONENAND_SET_NEXT_BUFFERRAM(this); > > } > > > > - this->command(mtd, ONENAND_CMD_PROG, to, mtd->writesize); > > + this->ongoing = 0; > > + > > + /* Exclude 1st OTP and OTP blocks */ > > + if (ONENAND_IS_CACHE_PROGRAM(this) && > > + likely(onenand_block(this, to) != 0) && > > + ONENAND_IS_4KB_PAGE(this)) { > check the length here instead of 4KB_PAGE. This patch is for 4kb page onenand only. The 2kb page onenand uses different commands for cache program. > > + > > + if ((written + thislen) < len) { > > + cmd = ONENAND_CMD_CACHE_PROG; > > + this->ongoing = 1; > > + } else { > > + cmd = ONENAND_CMD_CACHE_PROG_LAST; > > + } > Need to check how to pass the cmd properly. How/When send the CACHE_PROG_LAST? CACHE_PROG_LAST command must be sent for last programmed piece of data, that is (written + thislen) >= len. > > + > > + } else { > > + cmd = ONENAND_CMD_PROG; > > + } > > + > > + this->command(mtd, cmd, to, mtd->writesize); > > > > /* > > * 2 PLANE, MLC, and Flex-OneNAND wait here > > @@ -3380,6 +3411,8 @@ static void onenand_check_features(struct mtd_info *mtd) > > else if (numbufs == 1) > > this->options |= ONENAND_HAS_4KB_PAGE; > > > > + this->options |= ONENAND_HAS_CACHE_PROGRAM; > It also needs to set CACHE_PROGRAM option. Sorry. What do mean here? > > + > > case ONENAND_DEVICE_DENSITY_2Gb: > > /* 2Gb DDP does not have 2 plane */ > > if (!ONENAND_IS_DDP(this)) > > @@ -3415,6 +3448,8 @@ static void onenand_check_features(struct mtd_info *mtd) > > printk(KERN_DEBUG "Chip has 2 plane\n"); > > if (this->options & ONENAND_HAS_4KB_PAGE) > > printk(KERN_DEBUG "Chip has 4KiB pagesize\n"); > > + if (this->options & ONENAND_HAS_CACHE_PROGRAM) > > + printk(KERN_DEBUG "Chip has cache program feature\n"); > > } > > Thank you, > Kyungmin Park ^ permalink raw reply [flat|nested] 11+ messages in thread
* Re: [PATCH 3/3] mtd: onenand: implement cache program feature for 4kb page onenand 2010-11-02 14:37 ` Roman Tereshonkov @ 2010-11-02 14:50 ` Kyungmin Park 2010-11-02 16:53 ` Roman Tereshonkov 0 siblings, 1 reply; 11+ messages in thread From: Kyungmin Park @ 2010-11-02 14:50 UTC (permalink / raw) To: Roman Tereshonkov; +Cc: linux-mtd On Tue, Nov 2, 2010 at 11:37 PM, Roman Tereshonkov <roman.tereshonkov@nokia.com> wrote: > On Tue, Nov 02, 2010 at 03:00:41PM +0100, ext Kyungmin Park wrote: >> Hi, >> >> On Tue, Nov 2, 2010 at 8:25 PM, Roman Tereshonkov >> <roman.tereshonkov@nokia.com> wrote: >> > Implement cache program feature for 4KB page onenand. >> > This feature improves the write data performance. >> > The observed 128KB data program speed change is >> > from 8827KB/s to 14156 KB/s when the feature is enabled. >> > >> > Signed-off-by: Roman Tereshonkov <roman.tereshonkov@nokia.com> >> > --- >> > drivers/mtd/onenand/omap2.c | 12 ++++++++-- >> > drivers/mtd/onenand/onenand_base.c | 39 ++++++++++++++++++++++++++++++++++- >> > 2 files changed, 46 insertions(+), 5 deletions(-) >> >> Please make separate the two patches, one for OMAP, another is for >> onenand_base.c >> >> > >> > diff --git a/drivers/mtd/onenand/omap2.c b/drivers/mtd/onenand/omap2.c >> > index 9f322f1..da25a90 100644 >> > --- a/drivers/mtd/onenand/omap2.c >> > +++ b/drivers/mtd/onenand/omap2.c >> > @@ -108,8 +108,9 @@ static void wait_warn(char *msg, int state, unsigned int ctrl, >> > static int omap2_onenand_wait(struct mtd_info *mtd, int state) >> > { >> > struct omap2_onenand *c = container_of(mtd, struct omap2_onenand, mtd); >> > + struct onenand_chip *this = mtd->priv; >> > unsigned int intr = 0; >> > - unsigned int ctrl; >> > + unsigned int ctrl, ctrl_mask; >> > unsigned long timeout; >> > u32 syscfg; >> > >> > @@ -180,7 +181,8 @@ retry: >> > if (result == 0) { >> > /* Timeout after 20ms */ >> > ctrl = read_reg(c, ONENAND_REG_CTRL_STATUS); >> > - if (ctrl & ONENAND_CTRL_ONGO) { >> > + if (ctrl & ONENAND_CTRL_ONGO && >> > + !this->ongoing) { >> > /* >> > * The operation seems to be still going >> > * so give it some more time. >> > @@ -269,7 +271,11 @@ retry: >> > return -EIO; >> > } >> > >> > - if (ctrl & 0xFE9F) >> > + ctrl_mask = 0xFE9F; >> > + if (this->ongoing) >> > + ctrl_mask &= ~0x8000; >> > + >> > + if (ctrl & ctrl_mask) >> > wait_warn("unexpected controller status", state, ctrl, intr); >> > >> > return 0; >> > diff --git a/drivers/mtd/onenand/onenand_base.c b/drivers/mtd/onenand/onenand_base.c >> > index 6b3a875..b2c3e97 100644 >> > --- a/drivers/mtd/onenand/onenand_base.c >> > +++ b/drivers/mtd/onenand/onenand_base.c >> > @@ -440,6 +440,19 @@ static int onenand_command(struct mtd_info *mtd, int cmd, loff_t addr, size_t le >> > default: >> > if (ONENAND_IS_2PLANE(this) && cmd == ONENAND_CMD_PROG) >> > cmd = ONENAND_CMD_2X_PROG; >> > + >> > + /* Exclude 1st OTP and OTP blocks */ >> Are there any reason? I think 1st OTP block is just block 0 so no need >> to consider block 0 case. > > > Specification tells: > 3.9.1 Cache Program Operation > "Note that Cache Program command cannot be performed on OTP block and 1st block OTP" Okay I'm not yet check the Spec. I will check it at office. > > >> > + if (ONENAND_IS_CACHE_PROGRAM(this) && >> > + likely(onenand_block(this, addr) != 0) && >> > + ONENAND_IS_4KB_PAGE(this)) { >> and I think no need to check 4KB_PAGE. > > > This patch is for 4kb page onenand only. The 2kb page onenand uses different > commands for cache program. Also 2kb page case cache program depends on > CONFIG_MTD_ONENAND_2X_PROGRAM. > > >> > + >> > + if (cmd == ONENAND_CMD_CACHE_PROG) >> > + cmd = ONENAND_CMD_2X_CACHE_PROG; >> > + else if (cmd == ONENAND_CMD_CACHE_PROG_LAST) >> > + cmd = ONENAND_CMD_PROG; >> > + >> > + } >> > + >> > dataram = ONENAND_CURRENT_BUFFERRAM(this); >> > break; >> > } >> > @@ -1845,7 +1858,7 @@ static int onenand_write_ops_nolock(struct mtd_info *mtd, loff_t to, >> > const u_char *buf = ops->datbuf; >> > const u_char *oob = ops->oobbuf; >> > u_char *oobbuf; >> > - int ret = 0; >> > + int ret = 0, cmd; >> > >> > DEBUG(MTD_DEBUG_LEVEL3, "%s: to = 0x%08x, len = %i\n", >> > __func__, (unsigned int) to, (int) len); >> > @@ -1954,7 +1967,25 @@ static int onenand_write_ops_nolock(struct mtd_info *mtd, loff_t to, >> > ONENAND_SET_NEXT_BUFFERRAM(this); >> > } >> > >> > - this->command(mtd, ONENAND_CMD_PROG, to, mtd->writesize); >> > + this->ongoing = 0; >> > + >> > + /* Exclude 1st OTP and OTP blocks */ >> > + if (ONENAND_IS_CACHE_PROGRAM(this) && >> > + likely(onenand_block(this, to) != 0) && >> > + ONENAND_IS_4KB_PAGE(this)) { >> check the length here instead of 4KB_PAGE. > > > This patch is for 4kb page onenand only. The 2kb page onenand uses different > commands for cache program. I see, check it both 4KiB and 2KiB at office. > > >> > + >> > + if ((written + thislen) < len) { >> > + cmd = ONENAND_CMD_CACHE_PROG; >> > + this->ongoing = 1; >> > + } else { >> > + cmd = ONENAND_CMD_CACHE_PROG_LAST; >> > + } >> Need to check how to pass the cmd properly. How/When send the CACHE_PROG_LAST? > > > CACHE_PROG_LAST command must be sent for last programmed piece of data, that is > (written + thislen) >= len. Yes I know, I mean to use the CACHE program I think write size should be more than 4KiB. e.g., 8KiB write size, > > >> > + >> > + } else { >> > + cmd = ONENAND_CMD_PROG; >> > + } >> > + >> > + this->command(mtd, cmd, to, mtd->writesize); >> > >> > /* >> > * 2 PLANE, MLC, and Flex-OneNAND wait here >> > @@ -3380,6 +3411,8 @@ static void onenand_check_features(struct mtd_info *mtd) >> > else if (numbufs == 1) >> > this->options |= ONENAND_HAS_4KB_PAGE; >> > >> > + this->options |= ONENAND_HAS_CACHE_PROGRAM; >> It also needs to set CACHE_PROGRAM option. > > > Sorry. What do mean here? Ah, Now it always set the CACHE_PROGRAM, so it should be set on each chip spec. > > >> > + >> > case ONENAND_DEVICE_DENSITY_2Gb: >> > /* 2Gb DDP does not have 2 plane */ >> > if (!ONENAND_IS_DDP(this)) >> > @@ -3415,6 +3448,8 @@ static void onenand_check_features(struct mtd_info *mtd) >> > printk(KERN_DEBUG "Chip has 2 plane\n"); >> > if (this->options & ONENAND_HAS_4KB_PAGE) >> > printk(KERN_DEBUG "Chip has 4KiB pagesize\n"); >> > + if (this->options & ONENAND_HAS_CACHE_PROGRAM) >> > + printk(KERN_DEBUG "Chip has cache program feature\n"); >> > } >> >> Thank you, >> Kyungmin Park > ^ permalink raw reply [flat|nested] 11+ messages in thread
* Re: [PATCH 3/3] mtd: onenand: implement cache program feature for 4kb page onenand 2010-11-02 14:50 ` Kyungmin Park @ 2010-11-02 16:53 ` Roman Tereshonkov 2010-11-03 7:34 ` Kyungmin Park 0 siblings, 1 reply; 11+ messages in thread From: Roman Tereshonkov @ 2010-11-02 16:53 UTC (permalink / raw) To: ext Kyungmin Park; +Cc: linux-mtd On Tue, Nov 02, 2010 at 03:50:51PM +0100, ext Kyungmin Park wrote: > On Tue, Nov 2, 2010 at 11:37 PM, Roman Tereshonkov > <roman.tereshonkov@nokia.com> wrote: > > On Tue, Nov 02, 2010 at 03:00:41PM +0100, ext Kyungmin Park wrote: > >> Hi, > >> > >> On Tue, Nov 2, 2010 at 8:25 PM, Roman Tereshonkov > >> <roman.tereshonkov@nokia.com> wrote: > >> > Implement cache program feature for 4KB page onenand. > >> > This feature improves the write data performance. > >> > The observed 128KB data program speed change is > >> > from 8827KB/s to 14156 KB/s when the feature is enabled. > >> > > >> > Signed-off-by: Roman Tereshonkov <roman.tereshonkov@nokia.com> > >> > --- > >> > drivers/mtd/onenand/omap2.c | 12 ++++++++-- > >> > drivers/mtd/onenand/onenand_base.c | 39 ++++++++++++++++++++++++++++++++++- > >> > 2 files changed, 46 insertions(+), 5 deletions(-) > >> > >> Please make separate the two patches, one for OMAP, another is for > >> onenand_base.c > >> > >> > > >> > diff --git a/drivers/mtd/onenand/omap2.c b/drivers/mtd/onenand/omap2.c > >> > index 9f322f1..da25a90 100644 > >> > --- a/drivers/mtd/onenand/omap2.c > >> > +++ b/drivers/mtd/onenand/omap2.c > >> > @@ -108,8 +108,9 @@ static void wait_warn(char *msg, int state, unsigned int ctrl, > >> > static int omap2_onenand_wait(struct mtd_info *mtd, int state) > >> > { > >> > struct omap2_onenand *c = container_of(mtd, struct omap2_onenand, mtd); > >> > + struct onenand_chip *this = mtd->priv; > >> > unsigned int intr = 0; > >> > - unsigned int ctrl; > >> > + unsigned int ctrl, ctrl_mask; > >> > unsigned long timeout; > >> > u32 syscfg; > >> > > >> > @@ -180,7 +181,8 @@ retry: > >> > if (result == 0) { > >> > /* Timeout after 20ms */ > >> > ctrl = read_reg(c, ONENAND_REG_CTRL_STATUS); > >> > - if (ctrl & ONENAND_CTRL_ONGO) { > >> > + if (ctrl & ONENAND_CTRL_ONGO && > >> > + !this->ongoing) { > >> > /* > >> > * The operation seems to be still going > >> > * so give it some more time. > >> > @@ -269,7 +271,11 @@ retry: > >> > return -EIO; > >> > } > >> > > >> > - if (ctrl & 0xFE9F) > >> > + ctrl_mask = 0xFE9F; > >> > + if (this->ongoing) > >> > + ctrl_mask &= ~0x8000; > >> > + > >> > + if (ctrl & ctrl_mask) > >> > wait_warn("unexpected controller status", state, ctrl, intr); > >> > > >> > return 0; > >> > diff --git a/drivers/mtd/onenand/onenand_base.c b/drivers/mtd/onenand/onenand_base.c > >> > index 6b3a875..b2c3e97 100644 > >> > --- a/drivers/mtd/onenand/onenand_base.c > >> > +++ b/drivers/mtd/onenand/onenand_base.c > >> > @@ -440,6 +440,19 @@ static int onenand_command(struct mtd_info *mtd, int cmd, loff_t addr, size_t le > >> > default: > >> > if (ONENAND_IS_2PLANE(this) && cmd == ONENAND_CMD_PROG) > >> > cmd = ONENAND_CMD_2X_PROG; > >> > + > >> > + /* Exclude 1st OTP and OTP blocks */ > >> Are there any reason? I think 1st OTP block is just block 0 so no need > >> to consider block 0 case. > > > > > > Specification tells: > > 3.9.1 Cache Program Operation > > "Note that Cache Program command cannot be performed on OTP block and 1st block OTP" > > Okay I'm not yet check the Spec. I will check it at office. > > > > > >> > + if (ONENAND_IS_CACHE_PROGRAM(this) && > >> > + likely(onenand_block(this, addr) != 0) && > >> > + ONENAND_IS_4KB_PAGE(this)) { > >> and I think no need to check 4KB_PAGE. > > > > > > This patch is for 4kb page onenand only. The 2kb page onenand uses different > > commands for cache program. Also 2kb page case cache program depends on > > CONFIG_MTD_ONENAND_2X_PROGRAM. > > > > > >> > + > >> > + if (cmd == ONENAND_CMD_CACHE_PROG) > >> > + cmd = ONENAND_CMD_2X_CACHE_PROG; > >> > + else if (cmd == ONENAND_CMD_CACHE_PROG_LAST) > >> > + cmd = ONENAND_CMD_PROG; > >> > + > >> > + } > >> > + > >> > dataram = ONENAND_CURRENT_BUFFERRAM(this); > >> > break; > >> > } > >> > @@ -1845,7 +1858,7 @@ static int onenand_write_ops_nolock(struct mtd_info *mtd, loff_t to, > >> > const u_char *buf = ops->datbuf; > >> > const u_char *oob = ops->oobbuf; > >> > u_char *oobbuf; > >> > - int ret = 0; > >> > + int ret = 0, cmd; > >> > > >> > DEBUG(MTD_DEBUG_LEVEL3, "%s: to = 0x%08x, len = %i\n", > >> > __func__, (unsigned int) to, (int) len); > >> > @@ -1954,7 +1967,25 @@ static int onenand_write_ops_nolock(struct mtd_info *mtd, loff_t to, > >> > ONENAND_SET_NEXT_BUFFERRAM(this); > >> > } > >> > > >> > - this->command(mtd, ONENAND_CMD_PROG, to, mtd->writesize); > >> > + this->ongoing = 0; > >> > + > >> > + /* Exclude 1st OTP and OTP blocks */ > >> > + if (ONENAND_IS_CACHE_PROGRAM(this) && > >> > + likely(onenand_block(this, to) != 0) && > >> > + ONENAND_IS_4KB_PAGE(this)) { > >> check the length here instead of 4KB_PAGE. > > > > > > This patch is for 4kb page onenand only. The 2kb page onenand uses different > > commands for cache program. > I see, check it both 4KiB and 2KiB at office. > > > > > >> > + > >> > + if ((written + thislen) < len) { > >> > + cmd = ONENAND_CMD_CACHE_PROG; > >> > + this->ongoing = 1; > >> > + } else { > >> > + cmd = ONENAND_CMD_CACHE_PROG_LAST; > >> > + } > >> Need to check how to pass the cmd properly. How/When send the CACHE_PROG_LAST? > > > > > > CACHE_PROG_LAST command must be sent for last programmed piece of data, that is > > (written + thislen) >= len. > Yes I know, I mean to use the CACHE program I think write size should > be more than 4KiB. e.g., 8KiB write size, Considering 4KB page onenand, for each program command the 4KB data is written at once. If the condition ((written + thislen) < len) is true it means that there is more than 4KB data to written. That means at least one more data program command. Otherwise the ONENAND_CMD_CACHE_PROG_LAST is used which is normal program command. > > > > > >> > + > >> > + } else { > >> > + cmd = ONENAND_CMD_PROG; > >> > + } > >> > + > >> > + this->command(mtd, cmd, to, mtd->writesize); > >> > > >> > /* > >> > * 2 PLANE, MLC, and Flex-OneNAND wait here > >> > @@ -3380,6 +3411,8 @@ static void onenand_check_features(struct mtd_info *mtd) > >> > else if (numbufs == 1) > >> > this->options |= ONENAND_HAS_4KB_PAGE; > >> > > >> > + this->options |= ONENAND_HAS_CACHE_PROGRAM; > >> It also needs to set CACHE_PROGRAM option. > > > > > > Sorry. What do mean here? > Ah, Now it always set the CACHE_PROGRAM, so it should be set on each chip spec. Is there any problem that CACHE_PROGRAM feature is set for each 4Gb chip? I suppose each of them supports cache program either as 4KB page cache or 2KB page 2x cache program feature. > > > > > >> > + > >> > case ONENAND_DEVICE_DENSITY_2Gb: > >> > /* 2Gb DDP does not have 2 plane */ > >> > if (!ONENAND_IS_DDP(this)) > >> > @@ -3415,6 +3448,8 @@ static void onenand_check_features(struct mtd_info *mtd) > >> > printk(KERN_DEBUG "Chip has 2 plane\n"); > >> > if (this->options & ONENAND_HAS_4KB_PAGE) > >> > printk(KERN_DEBUG "Chip has 4KiB pagesize\n"); > >> > + if (this->options & ONENAND_HAS_CACHE_PROGRAM) > >> > + printk(KERN_DEBUG "Chip has cache program feature\n"); > >> > } > >> > >> Thank you, > >> Kyungmin Park > > ^ permalink raw reply [flat|nested] 11+ messages in thread
* Re: [PATCH 3/3] mtd: onenand: implement cache program feature for 4kb page onenand 2010-11-02 16:53 ` Roman Tereshonkov @ 2010-11-03 7:34 ` Kyungmin Park 2010-11-03 10:06 ` Kyungmin Park 0 siblings, 1 reply; 11+ messages in thread From: Kyungmin Park @ 2010-11-03 7:34 UTC (permalink / raw) To: Roman Tereshonkov; +Cc: linux-mtd On Wed, Nov 3, 2010 at 1:53 AM, Roman Tereshonkov <roman.tereshonkov@nokia.com> wrote: > On Tue, Nov 02, 2010 at 03:50:51PM +0100, ext Kyungmin Park wrote: >> On Tue, Nov 2, 2010 at 11:37 PM, Roman Tereshonkov >> <roman.tereshonkov@nokia.com> wrote: >> > On Tue, Nov 02, 2010 at 03:00:41PM +0100, ext Kyungmin Park wrote: >> >> Hi, >> >> >> >> On Tue, Nov 2, 2010 at 8:25 PM, Roman Tereshonkov >> >> <roman.tereshonkov@nokia.com> wrote: >> >> > Implement cache program feature for 4KB page onenand. >> >> > This feature improves the write data performance. >> >> > The observed 128KB data program speed change is >> >> > from 8827KB/s to 14156 KB/s when the feature is enabled. >> >> > >> >> > Signed-off-by: Roman Tereshonkov <roman.tereshonkov@nokia.com> >> >> > --- >> >> > drivers/mtd/onenand/omap2.c | 12 ++++++++-- >> >> > drivers/mtd/onenand/onenand_base.c | 39 ++++++++++++++++++++++++++++++++++- >> >> > 2 files changed, 46 insertions(+), 5 deletions(-) >> >> >> >> Please make separate the two patches, one for OMAP, another is for >> >> onenand_base.c >> >> >> >> > >> >> > diff --git a/drivers/mtd/onenand/omap2.c b/drivers/mtd/onenand/omap2.c >> >> > index 9f322f1..da25a90 100644 >> >> > --- a/drivers/mtd/onenand/omap2.c >> >> > +++ b/drivers/mtd/onenand/omap2.c >> >> > @@ -108,8 +108,9 @@ static void wait_warn(char *msg, int state, unsigned int ctrl, >> >> > static int omap2_onenand_wait(struct mtd_info *mtd, int state) >> >> > { >> >> > struct omap2_onenand *c = container_of(mtd, struct omap2_onenand, mtd); >> >> > + struct onenand_chip *this = mtd->priv; >> >> > unsigned int intr = 0; >> >> > - unsigned int ctrl; >> >> > + unsigned int ctrl, ctrl_mask; >> >> > unsigned long timeout; >> >> > u32 syscfg; >> >> > >> >> > @@ -180,7 +181,8 @@ retry: >> >> > if (result == 0) { >> >> > /* Timeout after 20ms */ >> >> > ctrl = read_reg(c, ONENAND_REG_CTRL_STATUS); >> >> > - if (ctrl & ONENAND_CTRL_ONGO) { >> >> > + if (ctrl & ONENAND_CTRL_ONGO && >> >> > + !this->ongoing) { >> >> > /* >> >> > * The operation seems to be still going >> >> > * so give it some more time. >> >> > @@ -269,7 +271,11 @@ retry: >> >> > return -EIO; >> >> > } >> >> > >> >> > - if (ctrl & 0xFE9F) >> >> > + ctrl_mask = 0xFE9F; >> >> > + if (this->ongoing) >> >> > + ctrl_mask &= ~0x8000; >> >> > + >> >> > + if (ctrl & ctrl_mask) >> >> > wait_warn("unexpected controller status", state, ctrl, intr); We have to implement the onenand_wait also. it's only for working for OMAP. >> >> > >> >> > return 0; >> >> > diff --git a/drivers/mtd/onenand/onenand_base.c b/drivers/mtd/onenand/onenand_base.c >> >> > index 6b3a875..b2c3e97 100644 >> >> > --- a/drivers/mtd/onenand/onenand_base.c >> >> > +++ b/drivers/mtd/onenand/onenand_base.c >> >> > @@ -440,6 +440,19 @@ static int onenand_command(struct mtd_info *mtd, int cmd, loff_t addr, size_t le >> >> > default: >> >> > if (ONENAND_IS_2PLANE(this) && cmd == ONENAND_CMD_PROG) >> >> > cmd = ONENAND_CMD_2X_PROG; >> >> > + >> >> > + /* Exclude 1st OTP and OTP blocks */ >> >> Are there any reason? I think 1st OTP block is just block 0 so no need >> >> to consider block 0 case. >> > >> > >> > Specification tells: >> > 3.9.1 Cache Program Operation >> > "Note that Cache Program command cannot be performed on OTP block and 1st block OTP" >> >> Okay I'm not yet check the Spec. I will check it at office. >> > >> > >> >> > + if (ONENAND_IS_CACHE_PROGRAM(this) && >> >> > + likely(onenand_block(this, addr) != 0) && >> >> > + ONENAND_IS_4KB_PAGE(this)) { >> >> and I think no need to check 4KB_PAGE. >> > >> > >> > This patch is for 4kb page onenand only. The 2kb page onenand uses different >> > commands for cache program. Also 2kb page case cache program depends on >> > CONFIG_MTD_ONENAND_2X_PROGRAM. >> > >> > >> >> > + >> >> > + if (cmd == ONENAND_CMD_CACHE_PROG) >> >> > + cmd = ONENAND_CMD_2X_CACHE_PROG; >> >> > + else if (cmd == ONENAND_CMD_CACHE_PROG_LAST) >> >> > + cmd = ONENAND_CMD_PROG; >> >> > + >> >> > + } >> >> > + >> >> > dataram = ONENAND_CURRENT_BUFFERRAM(this); >> >> > break; >> >> > } >> >> > @@ -1845,7 +1858,7 @@ static int onenand_write_ops_nolock(struct mtd_info *mtd, loff_t to, >> >> > const u_char *buf = ops->datbuf; >> >> > const u_char *oob = ops->oobbuf; >> >> > u_char *oobbuf; >> >> > - int ret = 0; >> >> > + int ret = 0, cmd; >> >> > >> >> > DEBUG(MTD_DEBUG_LEVEL3, "%s: to = 0x%08x, len = %i\n", >> >> > __func__, (unsigned int) to, (int) len); >> >> > @@ -1954,7 +1967,25 @@ static int onenand_write_ops_nolock(struct mtd_info *mtd, loff_t to, >> >> > ONENAND_SET_NEXT_BUFFERRAM(this); >> >> > } >> >> > >> >> > - this->command(mtd, ONENAND_CMD_PROG, to, mtd->writesize); >> >> > + this->ongoing = 0; >> >> > + >> >> > + /* Exclude 1st OTP and OTP blocks */ >> >> > + if (ONENAND_IS_CACHE_PROGRAM(this) && >> >> > + likely(onenand_block(this, to) != 0) && >> >> > + ONENAND_IS_4KB_PAGE(this)) { >> >> check the length here instead of 4KB_PAGE. >> > >> > >> > This patch is for 4kb page onenand only. The 2kb page onenand uses different >> > commands for cache program. >> I see, check it both 4KiB and 2KiB at office. >> > >> > >> >> > + >> >> > + if ((written + thislen) < len) { >> >> > + cmd = ONENAND_CMD_CACHE_PROG; >> >> > + this->ongoing = 1; >> >> > + } else { >> >> > + cmd = ONENAND_CMD_CACHE_PROG_LAST; >> >> > + } >> >> Need to check how to pass the cmd properly. How/When send the CACHE_PROG_LAST? >> > >> > >> > CACHE_PROG_LAST command must be sent for last programmed piece of data, that is >> > (written + thislen) >= len. >> Yes I know, I mean to use the CACHE program I think write size should >> be more than 4KiB. e.g., 8KiB write size, > > Considering 4KB page onenand, for each program command the 4KB data is written at once. > If the condition ((written + thislen) < len) is true it means that there is more > than 4KB data to written. That means at least one more data program command. > Otherwise the ONENAND_CMD_CACHE_PROG_LAST is used which is normal program command. Right, then do you need the pseudo command? how about to set the real command at write function? > > >> > >> > >> >> > + >> >> > + } else { >> >> > + cmd = ONENAND_CMD_PROG; >> >> > + } >> >> > + >> >> > + this->command(mtd, cmd, to, mtd->writesize); >> >> > >> >> > /* >> >> > * 2 PLANE, MLC, and Flex-OneNAND wait here >> >> > @@ -3380,6 +3411,8 @@ static void onenand_check_features(struct mtd_info *mtd) >> >> > else if (numbufs == 1) >> >> > this->options |= ONENAND_HAS_4KB_PAGE; >> >> > >> >> > + this->options |= ONENAND_HAS_CACHE_PROGRAM; >> >> It also needs to set CACHE_PROGRAM option. >> > >> > >> > Sorry. What do mean here? >> Ah, Now it always set the CACHE_PROGRAM, so it should be set on each chip spec. > > Is there any problem that CACHE_PROGRAM feature is set for each 4Gb chip? > I suppose each of them supports cache program either as 4KB page cache or > 2KB page 2x cache program feature. For clear, how about to set the 4KiB pagesize only? I'm not yet check the 4Gib DDP chip. Thank you, Kyungmin Park > >> > >> > >> >> > + >> >> > case ONENAND_DEVICE_DENSITY_2Gb: >> >> > /* 2Gb DDP does not have 2 plane */ >> >> > if (!ONENAND_IS_DDP(this)) >> >> > @@ -3415,6 +3448,8 @@ static void onenand_check_features(struct mtd_info *mtd) >> >> > printk(KERN_DEBUG "Chip has 2 plane\n"); >> >> > if (this->options & ONENAND_HAS_4KB_PAGE) >> >> > printk(KERN_DEBUG "Chip has 4KiB pagesize\n"); >> >> > + if (this->options & ONENAND_HAS_CACHE_PROGRAM) >> >> > + printk(KERN_DEBUG "Chip has cache program feature\n"); >> >> > } >> >> >> >> Thank you, >> >> Kyungmin Park >> > > ^ permalink raw reply [flat|nested] 11+ messages in thread
* Re: [PATCH 3/3] mtd: onenand: implement cache program feature for 4kb page onenand 2010-11-03 7:34 ` Kyungmin Park @ 2010-11-03 10:06 ` Kyungmin Park 0 siblings, 0 replies; 11+ messages in thread From: Kyungmin Park @ 2010-11-03 10:06 UTC (permalink / raw) To: Roman Tereshonkov; +Cc: linux-mtd On Wed, Nov 3, 2010 at 4:34 PM, Kyungmin Park <kmpark@infradead.org> wrote: > On Wed, Nov 3, 2010 at 1:53 AM, Roman Tereshonkov > <roman.tereshonkov@nokia.com> wrote: >> On Tue, Nov 02, 2010 at 03:50:51PM +0100, ext Kyungmin Park wrote: >>> On Tue, Nov 2, 2010 at 11:37 PM, Roman Tereshonkov >>> <roman.tereshonkov@nokia.com> wrote: >>> > On Tue, Nov 02, 2010 at 03:00:41PM +0100, ext Kyungmin Park wrote: >>> >> Hi, >>> >> >>> >> On Tue, Nov 2, 2010 at 8:25 PM, Roman Tereshonkov >>> >> <roman.tereshonkov@nokia.com> wrote: >>> >> > Implement cache program feature for 4KB page onenand. >>> >> > This feature improves the write data performance. >>> >> > The observed 128KB data program speed change is >>> >> > from 8827KB/s to 14156 KB/s when the feature is enabled. >>> >> > >>> >> > Signed-off-by: Roman Tereshonkov <roman.tereshonkov@nokia.com> >>> >> > --- >>> >> > drivers/mtd/onenand/omap2.c | 12 ++++++++-- >>> >> > drivers/mtd/onenand/onenand_base.c | 39 ++++++++++++++++++++++++++++++++++- >>> >> > 2 files changed, 46 insertions(+), 5 deletions(-) >>> >> >>> >> Please make separate the two patches, one for OMAP, another is for >>> >> onenand_base.c >>> >> >>> >> > >>> >> > diff --git a/drivers/mtd/onenand/omap2.c b/drivers/mtd/onenand/omap2.c >>> >> > index 9f322f1..da25a90 100644 >>> >> > --- a/drivers/mtd/onenand/omap2.c >>> >> > +++ b/drivers/mtd/onenand/omap2.c >>> >> > @@ -108,8 +108,9 @@ static void wait_warn(char *msg, int state, unsigned int ctrl, >>> >> > static int omap2_onenand_wait(struct mtd_info *mtd, int state) >>> >> > { >>> >> > struct omap2_onenand *c = container_of(mtd, struct omap2_onenand, mtd); >>> >> > + struct onenand_chip *this = mtd->priv; >>> >> > unsigned int intr = 0; >>> >> > - unsigned int ctrl; >>> >> > + unsigned int ctrl, ctrl_mask; >>> >> > unsigned long timeout; >>> >> > u32 syscfg; >>> >> > >>> >> > @@ -180,7 +181,8 @@ retry: >>> >> > if (result == 0) { >>> >> > /* Timeout after 20ms */ >>> >> > ctrl = read_reg(c, ONENAND_REG_CTRL_STATUS); >>> >> > - if (ctrl & ONENAND_CTRL_ONGO) { >>> >> > + if (ctrl & ONENAND_CTRL_ONGO && >>> >> > + !this->ongoing) { >>> >> > /* >>> >> > * The operation seems to be still going >>> >> > * so give it some more time. >>> >> > @@ -269,7 +271,11 @@ retry: >>> >> > return -EIO; >>> >> > } >>> >> > >>> >> > - if (ctrl & 0xFE9F) >>> >> > + ctrl_mask = 0xFE9F; >>> >> > + if (this->ongoing) >>> >> > + ctrl_mask &= ~0x8000; >>> >> > + >>> >> > + if (ctrl & ctrl_mask) >>> >> > wait_warn("unexpected controller status", state, ctrl, intr); > > We have to implement the onenand_wait also. it's only for working for OMAP. > >>> >> > >>> >> > return 0; >>> >> > diff --git a/drivers/mtd/onenand/onenand_base.c b/drivers/mtd/onenand/onenand_base.c >>> >> > index 6b3a875..b2c3e97 100644 >>> >> > --- a/drivers/mtd/onenand/onenand_base.c >>> >> > +++ b/drivers/mtd/onenand/onenand_base.c >>> >> > @@ -440,6 +440,19 @@ static int onenand_command(struct mtd_info *mtd, int cmd, loff_t addr, size_t le >>> >> > default: >>> >> > if (ONENAND_IS_2PLANE(this) && cmd == ONENAND_CMD_PROG) >>> >> > cmd = ONENAND_CMD_2X_PROG; >>> >> > + >>> >> > + /* Exclude 1st OTP and OTP blocks */ >>> >> Are there any reason? I think 1st OTP block is just block 0 so no need >>> >> to consider block 0 case. >>> > >>> > >>> > Specification tells: >>> > 3.9.1 Cache Program Operation >>> > "Note that Cache Program command cannot be performed on OTP block and 1st block OTP" >>> >>> Okay I'm not yet check the Spec. I will check it at office. >>> > >>> > >>> >> > + if (ONENAND_IS_CACHE_PROGRAM(this) && >>> >> > + likely(onenand_block(this, addr) != 0) && >>> >> > + ONENAND_IS_4KB_PAGE(this)) { >>> >> and I think no need to check 4KB_PAGE. >>> > >>> > >>> > This patch is for 4kb page onenand only. The 2kb page onenand uses different >>> > commands for cache program. Also 2kb page case cache program depends on >>> > CONFIG_MTD_ONENAND_2X_PROGRAM. >>> > >>> > >>> >> > + >>> >> > + if (cmd == ONENAND_CMD_CACHE_PROG) >>> >> > + cmd = ONENAND_CMD_2X_CACHE_PROG; >>> >> > + else if (cmd == ONENAND_CMD_CACHE_PROG_LAST) >>> >> > + cmd = ONENAND_CMD_PROG; >>> >> > + >>> >> > + } >>> >> > + >>> >> > dataram = ONENAND_CURRENT_BUFFERRAM(this); >>> >> > break; >>> >> > } >>> >> > @@ -1845,7 +1858,7 @@ static int onenand_write_ops_nolock(struct mtd_info *mtd, loff_t to, >>> >> > const u_char *buf = ops->datbuf; >>> >> > const u_char *oob = ops->oobbuf; >>> >> > u_char *oobbuf; >>> >> > - int ret = 0; >>> >> > + int ret = 0, cmd; >>> >> > >>> >> > DEBUG(MTD_DEBUG_LEVEL3, "%s: to = 0x%08x, len = %i\n", >>> >> > __func__, (unsigned int) to, (int) len); >>> >> > @@ -1954,7 +1967,25 @@ static int onenand_write_ops_nolock(struct mtd_info *mtd, loff_t to, >>> >> > ONENAND_SET_NEXT_BUFFERRAM(this); >>> >> > } >>> >> > >>> >> > - this->command(mtd, ONENAND_CMD_PROG, to, mtd->writesize); >>> >> > + this->ongoing = 0; >>> >> > + >>> >> > + /* Exclude 1st OTP and OTP blocks */ >>> >> > + if (ONENAND_IS_CACHE_PROGRAM(this) && >>> >> > + likely(onenand_block(this, to) != 0) && >>> >> > + ONENAND_IS_4KB_PAGE(this)) { >>> >> check the length here instead of 4KB_PAGE. >>> > >>> > >>> > This patch is for 4kb page onenand only. The 2kb page onenand uses different >>> > commands for cache program. >>> I see, check it both 4KiB and 2KiB at office. >>> > >>> > >>> >> > + >>> >> > + if ((written + thislen) < len) { >>> >> > + cmd = ONENAND_CMD_CACHE_PROG; >>> >> > + this->ongoing = 1; >>> >> > + } else { >>> >> > + cmd = ONENAND_CMD_CACHE_PROG_LAST; >>> >> > + } >>> >> Need to check how to pass the cmd properly. How/When send the CACHE_PROG_LAST? >>> > >>> > >>> > CACHE_PROG_LAST command must be sent for last programmed piece of data, that is >>> > (written + thislen) >= len. >>> Yes I know, I mean to use the CACHE program I think write size should >>> be more than 4KiB. e.g., 8KiB write size, >> >> Considering 4KB page onenand, for each program command the 4KB data is written at once. >> If the condition ((written + thislen) < len) is true it means that there is more >> than 4KB data to written. That means at least one more data program command. >> Otherwise the ONENAND_CMD_CACHE_PROG_LAST is used which is normal program command. > > Right, then do you need the pseudo command? how about to set the real > command at write function? >> >> >>> > >>> > >>> >> > + >>> >> > + } else { >>> >> > + cmd = ONENAND_CMD_PROG; >>> >> > + } >>> >> > + >>> >> > + this->command(mtd, cmd, to, mtd->writesize); >>> >> > >>> >> > /* >>> >> > * 2 PLANE, MLC, and Flex-OneNAND wait here >>> >> > @@ -3380,6 +3411,8 @@ static void onenand_check_features(struct mtd_info *mtd) >>> >> > else if (numbufs == 1) >>> >> > this->options |= ONENAND_HAS_4KB_PAGE; >>> >> > >>> >> > + this->options |= ONENAND_HAS_CACHE_PROGRAM; >>> >> It also needs to set CACHE_PROGRAM option. >>> > >>> > >>> > Sorry. What do mean here? >>> Ah, Now it always set the CACHE_PROGRAM, so it should be set on each chip spec. >> >> Is there any problem that CACHE_PROGRAM feature is set for each 4Gb chip? >> I suppose each of them supports cache program either as 4KB page cache or >> 2KB page 2x cache program feature. > For clear, how about to set the 4KiB pagesize only? I'm not yet check > the 4Gib DDP chip. > > Thank you, > Kyungmin Park > >> >>> > >>> > >>> >> > + >>> >> > case ONENAND_DEVICE_DENSITY_2Gb: >>> >> > /* 2Gb DDP does not have 2 plane */ >>> >> > if (!ONENAND_IS_DDP(this)) >>> >> > @@ -3415,6 +3448,8 @@ static void onenand_check_features(struct mtd_info *mtd) >>> >> > printk(KERN_DEBUG "Chip has 2 plane\n"); >>> >> > if (this->options & ONENAND_HAS_4KB_PAGE) >>> >> > printk(KERN_DEBUG "Chip has 4KiB pagesize\n"); >>> >> > + if (this->options & ONENAND_HAS_CACHE_PROGRAM) >>> >> > + printk(KERN_DEBUG "Chip has cache program feature\n"); >>> >> > } >>> >> One more can you show your speedtest result? Here's my results. * Before [ 2.490000] mtd_speedtest: testing eraseblock write speed [ 10.005000] mtd_speedtest: eraseblock write speed is 8147 KiB/s [ 10.010000] mtd_speedtest: testing eraseblock read speed [ 11.660000] mtd_speedtest: eraseblock read speed is 37171 KiB/s [ 11.920000] mtd_speedtest: testing page write speed [ 19.440000] mtd_speedtest: page write speed is 8138 KiB/s [ 19.445000] mtd_speedtest: testing page read speed [ 21.115000] mtd_speedtest: page read speed is 36725 KiB/s [ 21.375000] mtd_speedtest: testing 2 page write speed [ 28.885000] mtd_speedtest: 2 page write speed is 8151 KiB/s [ 28.890000] mtd_speedtest: testing 2 page read speed [ 30.550000] mtd_speedtest: 2 page read speed is 36924 KiB/s [ 30.555000] mtd_speedtest: Testing erase speed [ 30.815000] mtd_speedtest: erase speed is 240881 KiB/s * After [ 73.385000] mtd_speedtest: scanned 240 eraseblocks, 1 are bad [ 73.645000] mtd_speedtest: testing eraseblock write speed [ 78.820000] mtd_speedtest: eraseblock write speed is 11834 KiB/s [ 78.825000] mtd_speedtest: testing eraseblock read speed [ 80.475000] mtd_speedtest: eraseblock read speed is 37193 KiB/s [ 80.735000] mtd_speedtest: testing page write speed [ 88.255000] mtd_speedtest: page write speed is 8138 KiB/s [ 88.260000] mtd_speedtest: testing page read speed [ 89.930000] mtd_speedtest: page read speed is 36725 KiB/s [ 90.190000] mtd_speedtest: testing 2 page write speed [ 96.515000] mtd_speedtest: 2 page write speed is 9676 KiB/s [ 96.520000] mtd_speedtest: testing 2 page read speed [ 98.180000] mtd_speedtest: 2 page read speed is 36924 KiB/s [ 98.185000] mtd_speedtest: Testing erase speed [ 98.445000] mtd_speedtest: erase speed is 240881 KiB/s [ 98.445000] mtd_speedtest: finished Thank you, Kyungmin Park ^ permalink raw reply [flat|nested] 11+ messages in thread
* Re: [PATCH 0/3] mtd: onenand: add cache program feature for 4kb page onenand 2010-11-02 11:25 [PATCH 0/3] mtd: onenand: add cache program feature for 4kb page onenand Roman Tereshonkov ` (2 preceding siblings ...) 2010-11-02 11:25 ` [PATCH 3/3] mtd: onenand: implement cache program feature for 4kb page onenand Roman Tereshonkov @ 2010-11-02 14:01 ` Kyungmin Park 3 siblings, 0 replies; 11+ messages in thread From: Kyungmin Park @ 2010-11-02 14:01 UTC (permalink / raw) To: Roman Tereshonkov; +Cc: linux-mtd Hi, Thank you for your works. I'll check your patch in my board, 2KiB & 4KiB pagesize. Thank you, Kyungmin Park On Tue, Nov 2, 2010 at 8:25 PM, Roman Tereshonkov <roman.tereshonkov@nokia.com> wrote: > Implement cache program feature for 4KB page onenand. > This feature improves the write data performance. > The observed 128KB data program speed change is > from 8827KB/s to 14156 KB/s when the feature is enabled. > > Roman Tereshonkov (3): > mtd: onenand: add option and variable for cache program feature > mtd: onenand: introduce not "real" commands for cache program > feature. > mtd: onenand: implement cache program feature for 4kb page onenand > > drivers/mtd/onenand/omap2.c | 12 ++++++++-- > drivers/mtd/onenand/onenand_base.c | 39 ++++++++++++++++++++++++++++++++++- > include/linux/mtd/onenand.h | 12 +++++++++++ > include/linux/mtd/onenand_regs.h | 2 + > 4 files changed, 60 insertions(+), 5 deletions(-) > > > ______________________________________________________ > Linux MTD discussion mailing list > http://lists.infradead.org/mailman/listinfo/linux-mtd/ > ^ permalink raw reply [flat|nested] 11+ messages in thread
end of thread, other threads:[~2010-11-03 10:06 UTC | newest] Thread overview: 11+ messages (download: mbox.gz follow: Atom feed -- links below jump to the message on this page -- 2010-11-02 11:25 [PATCH 0/3] mtd: onenand: add cache program feature for 4kb page onenand Roman Tereshonkov 2010-11-02 11:25 ` [PATCH 1/3] mtd: onenand: add option and variable for cache program feature Roman Tereshonkov 2010-11-02 11:25 ` [PATCH 2/3] mtd: onenand: introduce not "real" commands " Roman Tereshonkov 2010-11-02 11:25 ` [PATCH 3/3] mtd: onenand: implement cache program feature for 4kb page onenand Roman Tereshonkov 2010-11-02 14:00 ` Kyungmin Park 2010-11-02 14:37 ` Roman Tereshonkov 2010-11-02 14:50 ` Kyungmin Park 2010-11-02 16:53 ` Roman Tereshonkov 2010-11-03 7:34 ` Kyungmin Park 2010-11-03 10:06 ` Kyungmin Park 2010-11-02 14:01 ` [PATCH 0/3] mtd: onenand: add " Kyungmin Park
This is an external index of several public inboxes, see mirroring instructions on how to clone and mirror all data and code used by this external index.