Re: [Qemu-devel] [PATCH 10/12] Support for quad commands.

[Peter Crosthwaite]

On Wed, Dec 16, 2015 at 4:57 AM,  <address@hidden> wrote:
> From: Marcin Krzeminski <address@hidden>
>
> Signed-off-by: Pawel Lenkow <address@hidden>
> ---
>  hw/block/m25p80.c | 38 ++++++++++++++++++++++++++++++++++++--
>  1 file changed, 36 insertions(+), 2 deletions(-)
>
> diff --git a/hw/block/m25p80.c b/hw/block/m25p80.c
> index 6fc55a3..25ec666 100644
> --- a/hw/block/m25p80.c
> +++ b/hw/block/m25p80.c
> @@ -255,19 +255,24 @@ typedef enum {
>      BULK_ERASE = 0xc7,
>
>      READ = 0x3,
> +    READ4 = 0x13,
>      FAST_READ = 0xb,
>      DOR = 0x3b,
>      QOR = 0x6b,
>      DIOR = 0xbb,
>      QIOR = 0xeb,
> +    QIOR4 = 0xec,
>
>      PP = 0x2,
> +    PP4 = 0x12,
>      DPP = 0xa2,
>      QPP = 0x32,
>
>      ERASE_4K = 0x20,
> +    ERASE4_4K = 0x21,
>      ERASE_32K = 0x52,
>      ERASE_SECTOR = 0xd8,
> +    ERASE4_SECTOR = 0xdc,
>
>      EN_4BYTE_ADDR = 0xB7,
>      EX_4BYTE_ADDR = 0xE9,
> @@ -307,6 +312,7 @@ typedef struct Flash {
>      bool write_enable;
>      bool four_bytes_address_mode;
>      bool reset_enable;
> +    bool quad_enable;
>      bool initialized;
>      uint8_t reset_pin;
>      uint8_t ear;
> @@ -381,6 +387,7 @@ static void flash_erase(Flash *s, int offset, FlashCMD 
> cmd)
>
>      switch (cmd) {
>      case ERASE_4K:
> +    case ERASE4_4K:
>          len = 4 << 10;
>          capa_to_assert = ER_4K;
>          break;
> @@ -389,6 +396,7 @@ static void flash_erase(Flash *s, int offset, FlashCMD 
> cmd)
>          capa_to_assert = ER_32K;
>          break;
>      case ERASE_SECTOR:
> +    case ERASE4_SECTOR:
>          len = s->pi->sector_size;
>          break;
>      case BULK_ERASE:
> @@ -447,6 +455,14 @@ void flash_write8(Flash *s, uint64_t addr, uint8_t data)
>
>  static inline int is_4bytes(Flash *s)
>  {
> +   switch (s->cmd_in_progress) {
> +   case PP4:
> +   case READ4:
> +   case QIOR4:
> +   case ERASE4_4K:
> +   case ERASE4_SECTOR:
> +       return 1;
> +   default:
>         return s->four_bytes_address_mode;
>     }
>  }
> @@ -472,19 +488,24 @@ static void complete_collecting_data(Flash *s)
>      case DPP:
>      case QPP:
>      case PP:
> +    case PP4:
>          s->state = STATE_PAGE_PROGRAM;
>          break;
>      case READ:
> +    case READ4:
>      case FAST_READ:
>      case DOR:
>      case QOR:
>      case DIOR:
>      case QIOR:
> +    case QIOR4:
>          s->state = STATE_READ;
>          break;
>      case ERASE_4K:
> +    case ERASE4_4K:
>      case ERASE_32K:
>      case ERASE_SECTOR:
> +    case ERASE4_SECTOR:
>          flash_erase(s, s->cur_addr, s->cmd_in_progress);
>          break;
>      case WRSR:
> @@ -512,6 +533,7 @@ static void reset_memory(Flash *s)
>      s->state = STATE_IDLE;
>      s->write_enable = false;
>      s->reset_enable = false;
> +    s->quad_enable = false;
>
>      DB_PRINT_L(0, "Reset done.\n");
>  }
> @@ -519,18 +541,23 @@ static void reset_memory(Flash *s)
>  static void decode_new_cmd(Flash *s, uint32_t value)
>  {
>      s->cmd_in_progress = value;
> +    int i;
>      DB_PRINT_L(0, "decoded new command:%x\n", value);
>
>      switch (value) {
>
>      case ERASE_4K:
> +    case ERASE4_4K:
>      case ERASE_32K:
>      case ERASE_SECTOR:
> +    case ERASE4_SECTOR:
>      case READ:
> +    case READ4:
>      case DPP:
>      case QPP:
>      case PP:
> -        s->needed_bytes = 3;
> +    case PP4:
> +        s->needed_bytes = is_4bytes(s) ? 4 : 3;
>          s->pos = 0;
>          s->len = 0;
>          s->state = STATE_COLLECTING_DATA;
> @@ -574,7 +601,13 @@ static void decode_new_cmd(Flash *s, uint32_t value)
>          s->len = 0;
>          s->state = STATE_COLLECTING_DATA;
>          break;
> -
> +    case QIOR4:
> +        /* 4 address + 1 dummy */
> +        s->needed_bytes = 5;

So looking at the current QIOR handling, the number of "dummy bytes"
is calculated as:

num_dummy_cycles X 4.

This makes sense, as both the address and data-phase on either side of
the dummy phase is done on four-wires, so it is cleaner that the dummy
phase is also on 4 wires. Ideally we implement Dual and Quad mode
awareness and cycle accuracy on the SSI layer itself to handle these
cases, but until then we stuck with this strange fixed policy.

For numonyx, this gives 1 command + 3 address + 4 dummies ( == 8
cycles) = 8 needed_bytes.

So this would be 9 for the extra address byte in 4b mode.

> +        s->pos = 0;
> +        s->len = 0;
> +        s->state = STATE_COLLECTING_DATA;
> +        break;

Following on from, Cedric's spot, should this be a fallthrough to
regular QIOR, which then uses is_4bytes to implement the +1 on needed
bytes?

Regards,
Peter

>      case WRSR:
>          if (s->write_enable) {
>              s->needed_bytes = 1;
> @@ -792,6 +825,7 @@ static const VMStateDescription vmstate_m25p80 = {
>          VMSTATE_BOOL(four_bytes_address_mode, Flash),
>          VMSTATE_UINT8(ear, Flash),
>          VMSTATE_BOOL(reset_enable, Flash),
> +        VMSTATE_BOOL(quad_enable, Flash),
>          VMSTATE_BOOL(initialized, Flash),
>          VMSTATE_UINT8(reset_pin, Flash),
>          VMSTATE_END_OF_LIST()
> --
> 2.5.0
>
>