[PATCH 3/3] mtd: onenand: implement cache program feature for 4kb page onenand
Kyungmin Park
kmpark at infradead.org
Wed Nov 3 06:06:13 EDT 2010
On Wed, Nov 3, 2010 at 4:34 PM, Kyungmin Park <kmpark at infradead.org> wrote:
> On Wed, Nov 3, 2010 at 1:53 AM, Roman Tereshonkov
> <roman.tereshonkov at 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 at 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 at 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 at 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
More information about the linux-mtd
mailing list