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

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

[Edgar E. Iglesias]

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 <edgar.iglesias@axis.com>
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 <edgar.iglesias@gmail.com>

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;
 }
 
 static void glue(nand_init_, PAGE_SIZE)(struct nand_flash_s *s)

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

[andrzej zaborowski]

2008/12/12 Edgar E. Iglesias <edgar.iglesias@axis.com>:
> 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 <edgar.iglesias@axis.com>
> 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 <edgar.iglesias@gmail.com>
>
> 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
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
drivers I found had them declared or some specs had them but there
seems to be no standard or any place where they would be defined.

Cheers

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 <edgar.iglesias@axis.com>:
> > 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 <edgar.iglesias@axis.com>
> > 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 <edgar.iglesias@gmail.com>
> >
> > 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.

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

[Edgar E. Iglesias]

On Thu, Jan 07, 2010 at 03:19:15PM -0500, Jean-Hugues Deschenes wrote:
> > 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>/
> > >/ > +    unsigned int offset;/

...

> > >/ 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.
> Hi,
> 
> 	I'm just wondering what is the status of this patch; It has definitely solved one of my problems, where a NAND flash controller I'm emulating sends random read commands to an emulated large page device.

Hi,

I forgot about this one.. It's in one of my trees, I'll try to
find it & repost and see if there is more feedback.

My main concern was that I only had linux guests to test with.

Cheers

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

[Jean-Hugues Deschenes]

> 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.
Hi,

	I'm just wondering what is the status of this patch; It has definitely solved one of my problems, where a NAND flash controller I'm emulating sends random read commands to an emulated large page device.

jh