Re: [Qemu-devel] [PATCH] NAND: Correct random data reads.

[Edgar E. Iglesias]

On Mon, Dec 15, 2008 at 03:31:55AM +0100, andrzej zaborowski wrote:
> 2008/12/12 Edgar E. Iglesias <address@hidden>:
> > I'm having problems with machine that uses a NAND_MFR_STMICRO 0xf1,
> > recent Linux versions cannot properly read from the flash.
> >
> > Turns out that Linux MTD recently learned howto do randomly accessed reads
> > from NAND flashes. QEMU has problemns emulating these.
> >
> > This patch fixes the problem for me but I've only tested on one flash
> > model and only with Linux. I'd appreciate help with testing more machines
> > with NANDs and ofcourse any comments people may have on the patch.
> >
> > Thanks
> >
> > commit 68a19f4348b12f85bd5fbfd8cf7b19033e1fd784
> > Author: Edgar E. Iglesias <address@hidden>
> > Date:   Fri Dec 12 12:48:58 2008 +0100
> >
> >    NAND: Correct random data reads.
> >
> >    Random reading depends on having the last row/page latched and not beeing
> >    clobbered between read and any following random reads.
> >
> >    Also, s->iolen must be updated when loading the io/data register with 
> > randomly
> >    accessed flash data.
> >
> >    Signed-off-by: Edgar E. Iglesias <address@hidden>
> >
> > diff --git a/hw/nand.c b/hw/nand.c
> > index 7c9f0aa..a6f67c6 100644
> > --- a/hw/nand.c
> > +++ b/hw/nand.c
> > @@ -212,6 +212,7 @@ void nand_reset(struct nand_flash_s *s)
> >
> >  static void nand_command(struct nand_flash_s *s)
> >  {
> > +    unsigned int offset;
> >     switch (s->cmd) {
> >     case NAND_CMD_READ0:
> >         s->iolen = 0;
> > @@ -233,8 +234,12 @@ static void nand_command(struct nand_flash_s *s)
> >     case NAND_CMD_NOSERIALREAD2:
> >         if (!(nand_flash_ids[s->chip_id].options & NAND_SAMSUNG_LP))
> >             break;
> > -
> > -        s->blk_load(s, s->addr, s->addr & ((1 << s->addr_shift) - 1));
> > +        offset = s->addr & ((1 << s->addr_shift) - 1);
> > +        s->blk_load(s, s->addr, offset);
> > +        if (s->gnd)
> > +            s->iolen = (1 << s->page_shift) - offset;
> > +        else
> > +            s->iolen = (1 << s->page_shift) + (1 << s->oob_shift) - offset;
> >         break;
> >
> >     case NAND_CMD_RESET:
> > @@ -380,12 +385,15 @@ void nand_setio(struct nand_flash_s *s, uint8_t value)
> >
> >         if (s->cmd != NAND_CMD_RANDOMREAD2) {
> >             s->addrlen = 0;
> > -            s->addr = 0;
> >         }
> >     }
> >
> >     if (s->ale) {
> > -        s->addr |= value << (s->addrlen * 8);
> > +        unsigned int shift = s->addrlen * 8;
> > +        unsigned int mask = ~(0xff << shift);
> > +        unsigned int v = value << shift;
> > +
> > +        s->addr = (s->addr & mask) | v;
> >         s->addrlen ++;
> >
> >         if (s->addrlen == 1 && s->cmd == NAND_CMD_READID)
> > @@ -680,9 +688,6 @@ static void glue(nand_blk_load_, PAGE_SIZE)(struct 
> > nand_flash_s *s,
> >                         offset, PAGE_SIZE + OOB_SIZE - offset);
> >         s->ioaddr = s->io;
> >     }
> > -
> > -    s->addr &= PAGE_SIZE - 1;
> > -    s->addr += PAGE_SIZE;
> 
> Won't that break sequential reading?  I think the other modifications

I found specs for a flash with sequential page-crossing reads. My
code does not handle that but neither does current svn AFAICT. The
masking is wrong, we'll always end up at page 1.

> should also be conditional on the command being RANDOMREAD.
> 
> BTW, do you happen to know what the difference is between RANDOMREAD2
> and NOSERIALREAD2?  I basically added those commands because various

AFAIK noserial reads are for preparing copyback writes. Random reads are
for random access within the page register.

I'll try to fix and test the page-crossing seqreads.

Thanks alot for the comments.