[PATCH 3/3] mtd: onenand: implement cache program feature for 4kb page onenand

Kyungmin Park kmpark at infradead.org
Wed Nov 3 03:34:59 EDT 2010


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");
>> >> >  }
>> >>
>> >> Thank you,
>> >> Kyungmin Park
>> >
>



More information about the linux-mtd mailing list