Re: [Qemu-devel] [PATCH 15/15] hw/onenand: program actions can only clear bits

[andrzej zaborowski]

On 29 July 2011 17:35, Peter Maydell <address@hidden> wrote:
> From: Juha Riihimäki <address@hidden>
>
> The program actions onenand_prog_main() and onenand_prog_spare()
> can only set bits.
>
> This implies a rewrite of onenand_erase() to not use the program
> functions, since erase does need to set bits.
>
> Signed-off-by: Juha Riihimäki <address@hidden>
> [Riku Voipio: Fixes and restructuring patchset]
> Signed-off-by: Riku Voipio <address@hidden>
> [Peter Maydell: More fixes and cleanups for upstream submission]
> Signed-off-by:  Peter Maydell <address@hidden>
> ---
>  hw/onenand.c |  135 +++++++++++++++++++++++++++++++++++++++++++++------------
>  1 files changed, 106 insertions(+), 29 deletions(-)
>
> diff --git a/hw/onenand.c b/hw/onenand.c
...
>  static inline int onenand_erase(OneNANDState *s, int sec, int num)
>  {
> -    /* TODO: optimise */
> -    uint8_t buf[512];
> -
> -    memset(buf, 0xff, sizeof(buf));
> -    for (; num > 0; num --, sec ++) {
> -        if (onenand_prog_main(s, sec, 1, buf))
> -            return 1;
> -        if (onenand_prog_spare(s, sec, 1, buf))
> -            return 1;
> +    uint8_t *blankbuf, *tmpbuf;
> +    blankbuf = qemu_malloc(512);
> +    if (!blankbuf) {
> +        return 1;
> +    }
> +    tmpbuf = qemu_malloc(512);
> +    if (!tmpbuf) {
> +        qemu_free(blankbuf);
> +        return 1;
> +    }
> +    memset(blankbuf, 0xff, 512);
> +    for (; num > 0; num--, sec++) {
> +        if (s->bdrv_cur) {
> +            int erasesec = s->secs_cur + (sec >> 5);
> +            if (bdrv_write(s->bdrv_cur, sec, blankbuf, 1)) {
> +                goto fail;
> +            }
> +            if (bdrv_read(s->bdrv_cur, erasesec, tmpbuf, 1) < 0) {
> +                goto fail;
> +            }
> +            memcpy(tmpbuf + ((sec & 31) << 4), blankbuf, 1 << 4);
> +            if (bdrv_write(s->bdrv_cur, erasesec, tmpbuf, 1) < 0) {
> +                goto fail;
> +            }

By the way it seems that this can be optimised to require just one
read for the entire erase.

Cheers