* [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 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
* 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
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.