[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]
Re: [Qemu-devel] [PATCH v5 11/24] hw/nand.c: bug fix to BUSY/READY statu
From: |
Peter Maydell |
Subject: |
Re: [Qemu-devel] [PATCH v5 11/24] hw/nand.c: bug fix to BUSY/READY status bit |
Date: |
Thu, 28 Feb 2013 16:16:09 +0000 |
On 27 February 2013 07:15, Kuo-Jung Su <address@hidden> wrote:
> From: Kuo-Jung Su <address@hidden>
Your subject line could be made a little more specific, like this:
"hw/nand.c: correct the sense of the BUSY/READY status bit".
> The BIT6 of Status Register(SR):
>
> SR[6] behaves the same as R/B# pin
> SR[6] = 0 indicates the device is busy;
> SR[6] = 1 means the device is ready
>
> Some NAND flash controller (i.e. ftnandc021) relies on the SR[6]
> to determine if the NAND flash erase/program is success or error timeout.
>
> P.S:
> The exmaple NAND flash datasheet could be found at following link:
> http://www.mxic.com.tw/QuickPlace/hq/PageLibrary4825740B00298A3B.nsf/h_Index/8FEA549237D2F7674825795800104C26/$File/MX30LF1G08AA,%203V,%201Gb,%20v1.1.pdf
>
> Signed-off-by: Kuo-Jung Su <address@hidden>
> ---
> hw/nand.c | 20 +++++++++++++++++---
> 1 file changed, 17 insertions(+), 3 deletions(-)
>
> diff --git a/hw/nand.c b/hw/nand.c
> index 4a71265..7f40ebf 100644
> --- a/hw/nand.c
> +++ b/hw/nand.c
> @@ -46,7 +46,7 @@
> # define NAND_IOSTATUS_PLANE1 (1 << 2)
> # define NAND_IOSTATUS_PLANE2 (1 << 3)
> # define NAND_IOSTATUS_PLANE3 (1 << 4)
> -# define NAND_IOSTATUS_BUSY (1 << 6)
> +# define NAND_IOSTATUS_READY (1 << 6)
> # define NAND_IOSTATUS_UNPROTCT (1 << 7)
>
> # define MAX_PAGE 0x800
> @@ -231,6 +231,7 @@ static void nand_reset(DeviceState *dev)
> s->iolen = 0;
> s->offset = 0;
> s->status &= NAND_IOSTATUS_UNPROTCT;
> + s->status |= NAND_IOSTATUS_READY;
> }
These two parts look good.
>
> static inline void nand_pushio_byte(NANDFlashState *s, uint8_t value)
> @@ -647,6 +648,8 @@ static void glue(nand_blk_write_,
> PAGE_SIZE)(NANDFlashState *s)
> if (PAGE(s->addr) >= s->pages)
> return;
>
> + s->status &= ~NAND_IOSTATUS_READY;
> +
> if (!s->bdrv) {
> mem_and(s->storage + PAGE_START(s->addr) + (s->addr & PAGE_MASK) +
> s->offset, s->io, s->iolen);
> @@ -656,7 +659,7 @@ static void glue(nand_blk_write_,
> PAGE_SIZE)(NANDFlashState *s)
> soff = SECTOR_OFFSET(s->addr);
> if (bdrv_read(s->bdrv, sector, iobuf, PAGE_SECTORS) < 0) {
> printf("%s: read error in sector %" PRIu64 "\n", __func__,
> sector);
> - return;
> + goto blkw_out;
> }
>
> mem_and(iobuf + (soff | off), s->io, MIN(s->iolen, PAGE_SIZE - off));
> @@ -675,7 +678,7 @@ static void glue(nand_blk_write_,
> PAGE_SIZE)(NANDFlashState *s)
> soff = off & 0x1ff;
> if (bdrv_read(s->bdrv, sector, iobuf, PAGE_SECTORS + 2) < 0) {
> printf("%s: read error in sector %" PRIu64 "\n", __func__,
> sector);
> - return;
> + goto blkw_out;
> }
>
> mem_and(iobuf + soff, s->io, s->iolen);
> @@ -685,6 +688,9 @@ static void glue(nand_blk_write_,
> PAGE_SIZE)(NANDFlashState *s)
> }
> }
> s->offset = 0;
> +
> +blkw_out:
> + s->status |= NAND_IOSTATUS_READY;
> }
How is it ever possible for a guest to observe the status register
in the state where the READY bit is cleared? There's no point in
clearing the bit on entry to this function and setting it again
on every exit path when nobody will ever read the status register
while we're inside the function. (Same comments apply for read
and erase.)
thanks
-- PMM
- [Qemu-devel] [PATCH v5 05/24] hw/arm: add Faraday FTDDRII030 support, (continued)
- [Qemu-devel] [PATCH v5 05/24] hw/arm: add Faraday FTDDRII030 support, Kuo-Jung Su, 2013/02/27
- [Qemu-devel] [PATCH v5 06/24] hw/arm: add Faraday FTPWMTMR010 timer support, Kuo-Jung Su, 2013/02/27
- [Qemu-devel] [PATCH v5 07/24] hw/arm: add Faraday FTWDT010 watchdog timer support, Kuo-Jung Su, 2013/02/27
- [Qemu-devel] [PATCH v5 08/24] hw/arm: add Faraday FTRTC011 RTC timer support, Kuo-Jung Su, 2013/02/27
- [Qemu-devel] [PATCH v5 09/24] hw/arm: add Faraday FTDMAC020 AHB DMA support, Kuo-Jung Su, 2013/02/27
- [Qemu-devel] [PATCH v5 10/24] hw/arm: add Faraday FTAPBBRG020 APB DMA support, Kuo-Jung Su, 2013/02/27
- [Qemu-devel] [PATCH v5 13/24] hw/arm: add Faraday FTNANDC021 nand flash controller support, Kuo-Jung Su, 2013/02/27
- [Qemu-devel] [PATCH v5 12/24] hw/nand.c: bug fix to erase operation, Kuo-Jung Su, 2013/02/27
- [Qemu-devel] [PATCH v5 11/24] hw/nand.c: bug fix to BUSY/READY status bit, Kuo-Jung Su, 2013/02/27
- Re: [Qemu-devel] [PATCH v5 11/24] hw/nand.c: bug fix to BUSY/READY status bit,
Peter Maydell <=
- [Qemu-devel] [PATCH v5 14/24] hw/arm: add Faraday FTI2C010 I2C controller support, Kuo-Jung Su, 2013/02/27
- [Qemu-devel] [PATCH v5 18/24] hw/arm: add Faraday FTGMAC100 1Gbps ethernet support, Kuo-Jung Su, 2013/02/27
- [Qemu-devel] [PATCH v5 24/24] hw/arm: add Faraday FTSPI020 SPI flash controller support, Kuo-Jung Su, 2013/02/27
- [Qemu-devel] [PATCH v5 17/24] util: add linux bit ordering reversal functions, Kuo-Jung Su, 2013/02/27
- [Qemu-devel] [PATCH v5 22/24] hw/arm: add Faraday FTMAC110 10/100Mbps ethernet support, Kuo-Jung Su, 2013/02/27
- [Qemu-devel] [PATCH v5 21/24] hw/arm: add Faraday FTSDC010 MMC/SD controller support, Kuo-Jung Su, 2013/02/27
- [Qemu-devel] [PATCH v5 23/24] hw/arm: add Faraday FTTMR010 timer support, Kuo-Jung Su, 2013/02/27