Re: [Qemu-devel] [PATCH v2 08/10] m25p80: Fast read commands family changes.

[Cédric Le Goater]

On 06/17/2016 12:28 PM, address@hidden wrote:
> From: Marcin Krzeminski <address@hidden>
> 
> Support for Spansion and Macronix flashes.
> Additionally Numonyx(Micron) moved from default
> in fast read commands family. Also moved fast read
> command decoding to functions.
> 
> Signed-off-by: Marcin Krzeminski <address@hidden>

Reviewed-by: Cédric Le Goater <address@hidden>

Thanks, it looks better.

C.

> ---
>  hw/block/m25p80.c | 145 
> ++++++++++++++++++++++++++++++++++++++++--------------
>  1 file changed, 108 insertions(+), 37 deletions(-)
> 
> diff --git a/hw/block/m25p80.c b/hw/block/m25p80.c
> index 21998db..f40fa99 100644
> --- a/hw/block/m25p80.c
> +++ b/hw/block/m25p80.c
> @@ -719,6 +719,111 @@ static void reset_memory(Flash *s)
>      DB_PRINT_L(0, "Reset done.\n");
>  }
>  
> +static void decode_fast_read_cmd(Flash *s)
> +{
> +    s->needed_bytes = get_addr_length(s);
> +    switch (get_man(s)) {
> +    /* Dummy cycles - modeled with bytes writes instead of bits */
> +    case MAN_NUMONYX:
> +        s->needed_bytes += extract32(s->volatile_cfg, 4, 4);
> +        break;
> +    case MAN_MACRONIX:
> +        if (extract32(s->volatile_cfg, 6, 2) == 1) {
> +            s->needed_bytes += 6;
> +        } else {
> +            s->needed_bytes += 8;
> +        }
> +        break;
> +    case MAN_SPANSION:
> +        s->needed_bytes += extract32(s->spansion_cr2v,
> +                                    SPANSION_DUMMY_CLK_POS,
> +                                    SPANSION_DUMMY_CLK_LEN
> +                                    );
> +        break;
> +    default:
> +        break;
> +    }
> +    s->pos = 0;
> +    s->len = 0;
> +    s->state = STATE_COLLECTING_DATA;
> +}
> +
> +static void decode_dio_read_cmd(Flash *s)
> +{
> +    s->needed_bytes = get_addr_length(s);
> +    /* Dummy cycles modeled with bytes writes instead of bits */
> +    switch (get_man(s)) {
> +    case MAN_WINBOND:
> +        s->needed_bytes = 4;
> +        break;
> +    case MAN_SPANSION:
> +        s->needed_bytes += extract32(s->spansion_cr2v,
> +                                    SPANSION_DUMMY_CLK_POS,
> +                                    SPANSION_DUMMY_CLK_LEN
> +                                    );
> +        break;
> +    case MAN_NUMONYX:
> +        s->needed_bytes += extract32(s->volatile_cfg, 4, 4);
> +        break;
> +    case MAN_MACRONIX:
> +        switch (extract32(s->volatile_cfg, 6, 2)) {
> +        case 1:
> +            s->needed_bytes += 6;
> +            break;
> +        case 2:
> +            s->needed_bytes += 8;
> +            break;
> +        default:
> +            s->needed_bytes += 4;
> +            break;
> +        }
> +        break;
> +    default:
> +        break;
> +    }
> +    s->pos = 0;
> +    s->len = 0;
> +    s->state = STATE_COLLECTING_DATA;
> +}
> +
> +static void decode_qio_read_cmd(Flash *s)
> +{
> +    s->needed_bytes = get_addr_length(s);
> +    /* Dummy cycles modeled with bytes writes instead of bits */
> +    switch (get_man(s)) {
> +    case MAN_WINBOND:
> +        s->needed_bytes = 6;
> +        break;
> +    case MAN_SPANSION:
> +        s->needed_bytes += extract32(s->spansion_cr2v,
> +                                    SPANSION_DUMMY_CLK_POS,
> +                                    SPANSION_DUMMY_CLK_LEN
> +                                    );
> +        break;
> +    case MAN_NUMONYX:
> +        s->needed_bytes += extract32(s->volatile_cfg, 4, 4);
> +        break;
> +    case MAN_MACRONIX:
> +        switch (extract32(s->volatile_cfg, 6, 2)) {
> +        case 1:
> +            s->needed_bytes += 4;
> +            break;
> +        case 2:
> +            s->needed_bytes += 8;
> +            break;
> +        default:
> +            s->needed_bytes += 6;
> +            break;
> +        }
> +        break;
> +    default:
> +        break;
> +    }
> +    s->pos = 0;
> +    s->len = 0;
> +    s->state = STATE_COLLECTING_DATA;
> +}
> +
>  static void decode_new_cmd(Flash *s, uint32_t value)
>  {
>      s->cmd_in_progress = value;
> @@ -756,51 +861,17 @@ static void decode_new_cmd(Flash *s, uint32_t value)
>      case DOR4:
>      case QOR:
>      case QOR4:
> -        s->needed_bytes = get_addr_length(s);
> -        switch (get_man(s)) {
> -        case MAN_NUMONYX:
> -            s->needed_bytes += extract32(s->volatile_cfg, 4, 4);
> -            break;
> -        default:
> -            break;
> -        }
> -        s->pos = 0;
> -        s->len = 0;
> -        s->state = STATE_COLLECTING_DATA;
> +        decode_fast_read_cmd(s);
>          break;
>  
>      case DIOR:
>      case DIOR4:
> -        switch (get_man(s)) {
> -        case MAN_WINBOND:
> -        case MAN_SPANSION:
> -            s->needed_bytes = 4;
> -            break;
> -        default:
> -            s->needed_bytes = get_addr_length(s);
> -            /* Dummy cycles modeled with bytes writes instead of bits */
> -            s->needed_bytes += extract32(s->volatile_cfg, 4, 4);
> -        }
> -        s->pos = 0;
> -        s->len = 0;
> -        s->state = STATE_COLLECTING_DATA;
> +        decode_dio_read_cmd(s);
>          break;
>  
>      case QIOR:
>      case QIOR4:
> -        switch (get_man(s)) {
> -        case MAN_WINBOND:
> -        case MAN_SPANSION:
> -            s->needed_bytes = 6;
> -            break;
> -        default:
> -            s->needed_bytes = get_addr_length(s);
> -            /* Dummy cycles modeled with bytes writes instead of bits */
> -            s->needed_bytes += extract32(s->volatile_cfg, 4, 4);
> -        }
> -        s->pos = 0;
> -        s->len = 0;
> -        s->state = STATE_COLLECTING_DATA;
> +        decode_qio_read_cmd(s);
>          break;
>  
>      case WRSR:
>