Some Numonyx flash commands cannot be executed in DIO and QIO mode, such as
trying to do DPP or DOR when in QIO mode.

Signed-off-by: Joe Komlodi <komlodi@xilinx.com>
---
 hw/block/m25p80.c | 134 +++++++++++++++++++++++++++++++++++++++++++++---------
 1 file changed, 112 insertions(+), 22 deletions(-)

diff --git a/hw/block/m25p80.c b/hw/block/m25p80.c
index eb6539f..2552f2c 100644
--- a/hw/block/m25p80.c
+++ b/hw/block/m25p80.c
@@ -413,6 +413,12 @@ typedef enum {
     MAN_GENERIC,
 } Manufacturer;
 
+typedef enum {
+    MODE_STD = 0,
+    MODE_DIO = 1,
+    MODE_QIO = 2
+} SPIMode;
+
 #define M25P80_INTERNAL_DATA_BUFFER_SZ 16
 
 struct Flash {
@@ -820,6 +826,17 @@ static void reset_memory(Flash *s)
     trace_m25p80_reset_done(s);
 }
 
+static uint8_t numonyx_get_mode(Flash *s)
+{
+    if (!(s->enh_volatile_cfg & EVCFG_QUAD_IO_DISABLED)) {
+        return MODE_QIO;
+    } else if (!(s->enh_volatile_cfg & EVCFG_DUAL_IO_DISABLED)) {
+        return MODE_DIO;
+    } else {
+        return MODE_STD;
+    }
+}
+
 static void decode_fast_read_cmd(Flash *s)
 {
     s->needed_bytes = get_addr_length(s);
@@ -827,9 +844,11 @@ static void decode_fast_read_cmd(Flash *s)
     /* Dummy cycles - modeled with bytes writes instead of bits */
     case MAN_WINBOND:
         s->needed_bytes += 8;
+        s->state = STATE_COLLECTING_DATA;
         break;
     case MAN_NUMONYX:
         s->needed_bytes += extract32(s->volatile_cfg, 4, 4);
+        s->state = STATE_COLLECTING_DATA;
         break;
     case MAN_MACRONIX:
         if (extract32(s->volatile_cfg, 6, 2) == 1) {
@@ -837,19 +856,21 @@ static void decode_fast_read_cmd(Flash *s)
         } else {
             s->needed_bytes += 8;
         }
+        s->state = STATE_COLLECTING_DATA;
         break;
     case MAN_SPANSION:
         s->needed_bytes += extract32(s->spansion_cr2v,
                                     SPANSION_DUMMY_CLK_POS,
                                     SPANSION_DUMMY_CLK_LEN
                                     );
+        s->state = STATE_COLLECTING_DATA;
         break;
     default:
+        s->state = STATE_COLLECTING_DATA;
         break;
     }
     s->pos = 0;
     s->len = 0;
-    s->state = STATE_COLLECTING_DATA;
 }
 
 static void decode_dio_read_cmd(Flash *s)
@@ -859,6 +880,7 @@ static void decode_dio_read_cmd(Flash *s)
     switch (get_man(s)) {
     case MAN_WINBOND:
         s->needed_bytes += WINBOND_CONTINUOUS_READ_MODE_CMD_LEN;
+        s->state = STATE_COLLECTING_DATA;
         break;
     case MAN_SPANSION:
         s->needed_bytes += SPANSION_CONTINUOUS_READ_MODE_CMD_LEN;
@@ -866,9 +888,11 @@ static void decode_dio_read_cmd(Flash *s)
                                     SPANSION_DUMMY_CLK_POS,
                                     SPANSION_DUMMY_CLK_LEN
                                     );
+        s->state = STATE_COLLECTING_DATA;
         break;
     case MAN_NUMONYX:
         s->needed_bytes += extract32(s->volatile_cfg, 4, 4);
+        s->state = STATE_COLLECTING_DATA;
         break;
     case MAN_MACRONIX:
         switch (extract32(s->volatile_cfg, 6, 2)) {
@@ -882,13 +906,14 @@ static void decode_dio_read_cmd(Flash *s)
             s->needed_bytes += 4;
             break;
         }
+        s->state = STATE_COLLECTING_DATA;
         break;
     default:
+        s->state = STATE_COLLECTING_DATA;
         break;
     }
     s->pos = 0;
     s->len = 0;
-    s->state = STATE_COLLECTING_DATA;
 }
 
 static void decode_qio_read_cmd(Flash *s)
@@ -899,6 +924,7 @@ static void decode_qio_read_cmd(Flash *s)
     case MAN_WINBOND:
         s->needed_bytes += WINBOND_CONTINUOUS_READ_MODE_CMD_LEN;
         s->needed_bytes += 4;
+        s->state = STATE_COLLECTING_DATA;
         break;
     case MAN_SPANSION:
         s->needed_bytes += SPANSION_CONTINUOUS_READ_MODE_CMD_LEN;
@@ -906,9 +932,11 @@ static void decode_qio_read_cmd(Flash *s)
                                     SPANSION_DUMMY_CLK_POS,
                                     SPANSION_DUMMY_CLK_LEN
                                     );
+        s->state = STATE_COLLECTING_DATA;
         break;
     case MAN_NUMONYX:
         s->needed_bytes += extract32(s->volatile_cfg, 4, 4);
+        s->state = STATE_COLLECTING_DATA;
         break;
     case MAN_MACRONIX:
         switch (extract32(s->volatile_cfg, 6, 2)) {
@@ -922,13 +950,14 @@ static void decode_qio_read_cmd(Flash *s)
             s->needed_bytes += 6;
             break;
         }
+        s->state = STATE_COLLECTING_DATA;
         break;
     default:
+        s->state = STATE_COLLECTING_DATA;
         break;
     }
     s->pos = 0;
     s->len = 0;
-    s->state = STATE_COLLECTING_DATA;
 }
 
 static void decode_new_cmd(Flash *s, uint32_t value)
@@ -950,14 +979,8 @@ static void decode_new_cmd(Flash *s, uint32_t value)
     case ERASE4_32K:
     case ERASE_SECTOR:
     case ERASE4_SECTOR:
-    case READ:
-    case READ4:
-    case DPP:
-    case QPP:
-    case QPP_4:
     case PP:
     case PP4:
-    case PP4_4:
     case DIE_ERASE:
     case RDID_90:
     case RDID_AB:
@@ -966,24 +989,85 @@ static void decode_new_cmd(Flash *s, uint32_t value)
         s->len = 0;
         s->state = STATE_COLLECTING_DATA;
         break;
+    case READ:
+    case READ4:
+        if (!(get_man(s) == MAN_NUMONYX) || (numonyx_get_mode(s) != MODE_DIO &&
+            numonyx_get_mode(s) != MODE_QIO)) {
+            s->needed_bytes = get_addr_length(s);
+            s->pos = 0;
+            s->len = 0;
+            s->state = STATE_COLLECTING_DATA;
+        } else {
+            qemu_log_mask(LOG_GUEST_ERROR, "M25P80: Cannot execute cmd %x in "
+                          "DIO or QIO mode\n", s->cmd_in_progress);
+        }
+        break;
+    case DPP:
+        if (!(get_man(s) == MAN_NUMONYX) || numonyx_get_mode(s) != MODE_QIO) {
+            s->needed_bytes = get_addr_length(s);
+            s->pos = 0;
+            s->len = 0;
+            s->state = STATE_COLLECTING_DATA;
+        } else {
+            qemu_log_mask(LOG_GUEST_ERROR, "M25P80: Cannot execute cmd %x in "
+                          "QIO mode\n", s->cmd_in_progress);
+        }
+        break;
+    case QPP:
+    case QPP_4:
+    case PP4_4:
+        if (!(get_man(s) == MAN_NUMONYX) || numonyx_get_mode(s) != MODE_DIO) {
+            s->needed_bytes = get_addr_length(s);
+            s->pos = 0;
+            s->len = 0;
+            s->state = STATE_COLLECTING_DATA;
+        } else {
+            qemu_log_mask(LOG_GUEST_ERROR, "M25P80: Cannot execute cmd %x in "
+                          "DIO mode\n", s->cmd_in_progress);
+        }
+        break;
 
     case FAST_READ:
     case FAST_READ4:
+        decode_fast_read_cmd(s);
+        break;
     case DOR:
     case DOR4:
+        if (!(get_man(s) == MAN_NUMONYX) || numonyx_get_mode(s) != MODE_QIO) {
+            decode_fast_read_cmd(s);
+        } else {
+            qemu_log_mask(LOG_GUEST_ERROR, "M25P80: Cannot execute cmd %x in "
+                          "QIO mode\n", s->cmd_in_progress);
+        }
+        break;
     case QOR:
     case QOR4:
-        decode_fast_read_cmd(s);
+        if (!(get_man(s) == MAN_NUMONYX) || numonyx_get_mode(s) != MODE_DIO) {
+            decode_fast_read_cmd(s);
+        } else {
+            qemu_log_mask(LOG_GUEST_ERROR, "M25P80: Cannot execute cmd %x in "
+                          "DIO mode\n", s->cmd_in_progress);
+        }
         break;
 
     case DIOR:
     case DIOR4:
-        decode_dio_read_cmd(s);
+        if (!(get_man(s) == MAN_NUMONYX) || numonyx_get_mode(s) != MODE_QIO) {
+            decode_dio_read_cmd(s);
+        } else {
+            qemu_log_mask(LOG_GUEST_ERROR, "M25P80: Cannot execute cmd %x in "
+                          "QIO mode\n", s->cmd_in_progress);
+        }
         break;
 
     case QIOR:
     case QIOR4:
-        decode_qio_read_cmd(s);
+        if (!(get_man(s) == MAN_NUMONYX) || numonyx_get_mode(s) != MODE_DIO) {
+            decode_qio_read_cmd(s);
+        } else {
+            qemu_log_mask(LOG_GUEST_ERROR, "M25P80: Cannot execute cmd %x in "
+                          "DIO mode\n", s->cmd_in_progress);
+        }
         break;
 
     case WRSR:
@@ -1035,17 +1119,23 @@ static void decode_new_cmd(Flash *s, uint32_t value)
         break;
 
     case JEDEC_READ:
-        trace_m25p80_populated_jedec(s);
-        for (i = 0; i < s->pi->id_len; i++) {
-            s->data[i] = s->pi->id[i];
-        }
-        for (; i < SPI_NOR_MAX_ID_LEN; i++) {
-            s->data[i] = 0;
-        }
+        if (!(get_man(s) == MAN_NUMONYX) || (numonyx_get_mode(s) != MODE_DIO &&
+            numonyx_get_mode(s) != MODE_QIO)) {
+            trace_m25p80_populated_jedec(s);
+            for (i = 0; i < s->pi->id_len; i++) {
+                s->data[i] = s->pi->id[i];
+            }
+            for (; i < SPI_NOR_MAX_ID_LEN; i++) {
+                s->data[i] = 0;
+            }
 
-        s->len = SPI_NOR_MAX_ID_LEN;
-        s->pos = 0;
-        s->state = STATE_READING_DATA;
+            s->len = SPI_NOR_MAX_ID_LEN;
+            s->pos = 0;
+            s->state = STATE_READING_DATA;
+        } else {
+            qemu_log_mask(LOG_GUEST_ERROR, "M25P80: Cannot execute JEDEC read "
+                          "in DIO or QIO mode\n");
+        }
         break;
 
     case RDCR:
-- 
2.7.4

Hi Joe,

On Thu, Nov 12, 2020 at 07:10:54PM -0800, Joe Komlodi wrote:
> Some Numonyx flash commands cannot be executed in DIO and QIO mode, such as
> trying to do DPP or DOR when in QIO mode.
> 
> Signed-off-by: Joe Komlodi <komlodi@xilinx.com>
> ---
>  hw/block/m25p80.c | 134 +++++++++++++++++++++++++++++++++++++++++++++---------
>  1 file changed, 112 insertions(+), 22 deletions(-)
> 
> diff --git a/hw/block/m25p80.c b/hw/block/m25p80.c
> index eb6539f..2552f2c 100644
> --- a/hw/block/m25p80.c
> +++ b/hw/block/m25p80.c
> @@ -413,6 +413,12 @@ typedef enum {
>      MAN_GENERIC,
>  } Manufacturer;
>  
> +typedef enum {
> +    MODE_STD = 0,
> +    MODE_DIO = 1,
> +    MODE_QIO = 2
> +} SPIMode;
> +
>  #define M25P80_INTERNAL_DATA_BUFFER_SZ 16
>  
>  struct Flash {
> @@ -820,6 +826,17 @@ static void reset_memory(Flash *s)
>      trace_m25p80_reset_done(s);
>  }
>  
> +static uint8_t numonyx_get_mode(Flash *s)
> +{
> +    if (!(s->enh_volatile_cfg & EVCFG_QUAD_IO_DISABLED)) {
> +        return MODE_QIO;
> +    } else if (!(s->enh_volatile_cfg & EVCFG_DUAL_IO_DISABLED)) {
> +        return MODE_DIO;
> +    } else {
> +        return MODE_STD;
> +    }
> +}
> +
>  static void decode_fast_read_cmd(Flash *s)
>  {
>      s->needed_bytes = get_addr_length(s);
> @@ -827,9 +844,11 @@ static void decode_fast_read_cmd(Flash *s)
>      /* Dummy cycles - modeled with bytes writes instead of bits */
>      case MAN_WINBOND:
>          s->needed_bytes += 8;
> +        s->state = STATE_COLLECTING_DATA;
>          break;
>      case MAN_NUMONYX:
>          s->needed_bytes += extract32(s->volatile_cfg, 4, 4);
> +        s->state = STATE_COLLECTING_DATA;
>          break;
>      case MAN_MACRONIX:
>          if (extract32(s->volatile_cfg, 6, 2) == 1) {
> @@ -837,19 +856,21 @@ static void decode_fast_read_cmd(Flash *s)
>          } else {
>              s->needed_bytes += 8;
>          }
> +        s->state = STATE_COLLECTING_DATA;
>          break;
>      case MAN_SPANSION:
>          s->needed_bytes += extract32(s->spansion_cr2v,
>                                      SPANSION_DUMMY_CLK_POS,
>                                      SPANSION_DUMMY_CLK_LEN
>                                      );
> +        s->state = STATE_COLLECTING_DATA;
>          break;
>      default:
> +        s->state = STATE_COLLECTING_DATA;
>          break;
>      }
>      s->pos = 0;
>      s->len = 0;
> -    s->state = STATE_COLLECTING_DATA;

Above change in this function and the similar ones in below two functions
don't seem to be needed anymore (s->state = STATE_COLLECTING_DATA is being done
in all cases).

>  }
>  
>  static void decode_dio_read_cmd(Flash *s)
> @@ -859,6 +880,7 @@ static void decode_dio_read_cmd(Flash *s)
>      switch (get_man(s)) {
>      case MAN_WINBOND:
>          s->needed_bytes += WINBOND_CONTINUOUS_READ_MODE_CMD_LEN;
> +        s->state = STATE_COLLECTING_DATA;
>          break;
>      case MAN_SPANSION:
>          s->needed_bytes += SPANSION_CONTINUOUS_READ_MODE_CMD_LEN;
> @@ -866,9 +888,11 @@ static void decode_dio_read_cmd(Flash *s)
>                                      SPANSION_DUMMY_CLK_POS,
>                                      SPANSION_DUMMY_CLK_LEN
>                                      );
> +        s->state = STATE_COLLECTING_DATA;
>          break;
>      case MAN_NUMONYX:
>          s->needed_bytes += extract32(s->volatile_cfg, 4, 4);
> +        s->state = STATE_COLLECTING_DATA;
>          break;
>      case MAN_MACRONIX:
>          switch (extract32(s->volatile_cfg, 6, 2)) {
> @@ -882,13 +906,14 @@ static void decode_dio_read_cmd(Flash *s)
>              s->needed_bytes += 4;
>              break;
>          }
> +        s->state = STATE_COLLECTING_DATA;
>          break;
>      default:
> +        s->state = STATE_COLLECTING_DATA;
>          break;
>      }
>      s->pos = 0;
>      s->len = 0;
> -    s->state = STATE_COLLECTING_DATA;
>  }
>  
>  static void decode_qio_read_cmd(Flash *s)
> @@ -899,6 +924,7 @@ static void decode_qio_read_cmd(Flash *s)
>      case MAN_WINBOND:
>          s->needed_bytes += WINBOND_CONTINUOUS_READ_MODE_CMD_LEN;
>          s->needed_bytes += 4;
> +        s->state = STATE_COLLECTING_DATA;
>          break;
>      case MAN_SPANSION:
>          s->needed_bytes += SPANSION_CONTINUOUS_READ_MODE_CMD_LEN;
> @@ -906,9 +932,11 @@ static void decode_qio_read_cmd(Flash *s)
>                                      SPANSION_DUMMY_CLK_POS,
>                                      SPANSION_DUMMY_CLK_LEN
>                                      );
> +        s->state = STATE_COLLECTING_DATA;
>          break;
>      case MAN_NUMONYX:
>          s->needed_bytes += extract32(s->volatile_cfg, 4, 4);
> +        s->state = STATE_COLLECTING_DATA;
>          break;
>      case MAN_MACRONIX:
>          switch (extract32(s->volatile_cfg, 6, 2)) {
> @@ -922,13 +950,14 @@ static void decode_qio_read_cmd(Flash *s)
>              s->needed_bytes += 6;
>              break;
>          }
> +        s->state = STATE_COLLECTING_DATA;
>          break;
>      default:
> +        s->state = STATE_COLLECTING_DATA;
>          break;
>      }
>      s->pos = 0;
>      s->len = 0;
> -    s->state = STATE_COLLECTING_DATA;
>  }
>  
>  static void decode_new_cmd(Flash *s, uint32_t value)
> @@ -950,14 +979,8 @@ static void decode_new_cmd(Flash *s, uint32_t value)
>      case ERASE4_32K:
>      case ERASE_SECTOR:
>      case ERASE4_SECTOR:
> -    case READ:
> -    case READ4:
> -    case DPP:
> -    case QPP:
> -    case QPP_4:
>      case PP:
>      case PP4:
> -    case PP4_4:
>      case DIE_ERASE:
>      case RDID_90:
>      case RDID_AB:
> @@ -966,24 +989,85 @@ static void decode_new_cmd(Flash *s, uint32_t value)
>          s->len = 0;
>          s->state = STATE_COLLECTING_DATA;
>          break;
> +    case READ:
> +    case READ4:
> +        if (!(get_man(s) == MAN_NUMONYX) || (numonyx_get_mode(s) != MODE_DIO &&
> +            numonyx_get_mode(s) != MODE_QIO)) {

Above seems easier to check if we are in the correct mode here instead:

if (get_man(s) != MAN_NUMONYX || numonyx_get_mode(s) == MODE_STD) {

> +            s->needed_bytes = get_addr_length(s);
> +            s->pos = 0;
> +            s->len = 0;
> +            s->state = STATE_COLLECTING_DATA;
> +        } else {
> +            qemu_log_mask(LOG_GUEST_ERROR, "M25P80: Cannot execute cmd %x in "
> +                          "DIO or QIO mode\n", s->cmd_in_progress);
> +        }
> +        break;
> +    case DPP:
> +        if (!(get_man(s) == MAN_NUMONYX) || numonyx_get_mode(s) != MODE_QIO) {
> +            s->needed_bytes = get_addr_length(s);
> +            s->pos = 0;
> +            s->len = 0;
> +            s->state = STATE_COLLECTING_DATA;
> +        } else {
> +            qemu_log_mask(LOG_GUEST_ERROR, "M25P80: Cannot execute cmd %x in "
> +                          "QIO mode\n", s->cmd_in_progress);
> +        }
> +        break;
> +    case QPP:
> +    case QPP_4:
> +    case PP4_4:
> +        if (!(get_man(s) == MAN_NUMONYX) || numonyx_get_mode(s) != MODE_DIO) {
> +            s->needed_bytes = get_addr_length(s);
> +            s->pos = 0;
> +            s->len = 0;
> +            s->state = STATE_COLLECTING_DATA;
> +        } else {
> +            qemu_log_mask(LOG_GUEST_ERROR, "M25P80: Cannot execute cmd %x in "
> +                          "DIO mode\n", s->cmd_in_progress);
> +        }
> +        break;
>  
>      case FAST_READ:
>      case FAST_READ4:
> +        decode_fast_read_cmd(s);
> +        break;
>      case DOR:
>      case DOR4:
> +        if (!(get_man(s) == MAN_NUMONYX) || numonyx_get_mode(s) != MODE_QIO) {
> +            decode_fast_read_cmd(s);
> +        } else {
> +            qemu_log_mask(LOG_GUEST_ERROR, "M25P80: Cannot execute cmd %x in "
> +                          "QIO mode\n", s->cmd_in_progress);
> +        }
> +        break;
>      case QOR:
>      case QOR4:
> -        decode_fast_read_cmd(s);
> +        if (!(get_man(s) == MAN_NUMONYX) || numonyx_get_mode(s) != MODE_DIO) {
> +            decode_fast_read_cmd(s);
> +        } else {
> +            qemu_log_mask(LOG_GUEST_ERROR, "M25P80: Cannot execute cmd %x in "
> +                          "DIO mode\n", s->cmd_in_progress);
> +        }
>          break;
>  
>      case DIOR:
>      case DIOR4:
> -        decode_dio_read_cmd(s);
> +        if (!(get_man(s) == MAN_NUMONYX) || numonyx_get_mode(s) != MODE_QIO) {
> +            decode_dio_read_cmd(s);
> +        } else {
> +            qemu_log_mask(LOG_GUEST_ERROR, "M25P80: Cannot execute cmd %x in "
> +                          "QIO mode\n", s->cmd_in_progress);
> +        }
>          break;
>  
>      case QIOR:
>      case QIOR4:
> -        decode_qio_read_cmd(s);
> +        if (!(get_man(s) == MAN_NUMONYX) || numonyx_get_mode(s) != MODE_DIO) {
> +            decode_qio_read_cmd(s);
> +        } else {
> +            qemu_log_mask(LOG_GUEST_ERROR, "M25P80: Cannot execute cmd %x in "
> +                          "DIO mode\n", s->cmd_in_progress);
> +        }
>          break;
>  
>      case WRSR:
> @@ -1035,17 +1119,23 @@ static void decode_new_cmd(Flash *s, uint32_t value)
>          break;
>  
>      case JEDEC_READ:
> -        trace_m25p80_populated_jedec(s);
> -        for (i = 0; i < s->pi->id_len; i++) {
> -            s->data[i] = s->pi->id[i];
> -        }
> -        for (; i < SPI_NOR_MAX_ID_LEN; i++) {
> -            s->data[i] = 0;
> -        }
> +        if (!(get_man(s) == MAN_NUMONYX) || (numonyx_get_mode(s) != MODE_DIO &&
> +            numonyx_get_mode(s) != MODE_QIO)) {

The same here as above:
if (get_man(s) != MAN_NUMONYX || numonyx_get_mode(s) == MODE_STD) {

Best regards,
Francisco Iglesias


> +            trace_m25p80_populated_jedec(s);
> +            for (i = 0; i < s->pi->id_len; i++) {
> +                s->data[i] = s->pi->id[i];
> +            }
> +            for (; i < SPI_NOR_MAX_ID_LEN; i++) {
> +                s->data[i] = 0;
> +            }
>  
> -        s->len = SPI_NOR_MAX_ID_LEN;
> -        s->pos = 0;
> -        s->state = STATE_READING_DATA;
> +            s->len = SPI_NOR_MAX_ID_LEN;
> +            s->pos = 0;
> +            s->state = STATE_READING_DATA;
> +        } else {
> +            qemu_log_mask(LOG_GUEST_ERROR, "M25P80: Cannot execute JEDEC read "
> +                          "in DIO or QIO mode\n");
> +        }
>          break;
>  
>      case RDCR:
> -- 
> 2.7.4
> 

Hi Francisco,

-----Original Message-----
From: Francisco Iglesias <francisco.iglesias@xilinx.com> 
Sent: Monday, November 16, 2020 7:59 AM
To: Joe Komlodi <komlodi@xilinx.com>
Cc: qemu-devel@nongnu.org; philippe.mathieu.daude@gmail.com; Francisco Eduardo Iglesias <figlesia@xilinx.com>; alistair@alistair23.me; qemu-block@nongnu.org; mreitz@redhat.com
Subject: Re: [PATCH v4 3/4] hw/block/m25p80: Check SPI mode before running some Numonyx commands

Hi Joe,

On Thu, Nov 12, 2020 at 07:10:54PM -0800, Joe Komlodi wrote:
> Some Numonyx flash commands cannot be executed in DIO and QIO mode, 
> such as trying to do DPP or DOR when in QIO mode.
> 
> Signed-off-by: Joe Komlodi <komlodi@xilinx.com>
> ---
>  hw/block/m25p80.c | 134 
> +++++++++++++++++++++++++++++++++++++++++++++---------
>  1 file changed, 112 insertions(+), 22 deletions(-)
> 
> diff --git a/hw/block/m25p80.c b/hw/block/m25p80.c index 
> eb6539f..2552f2c 100644
> --- a/hw/block/m25p80.c
> +++ b/hw/block/m25p80.c
> @@ -413,6 +413,12 @@ typedef enum {
>      MAN_GENERIC,
>  } Manufacturer;
>  
> +typedef enum {
> +    MODE_STD = 0,
> +    MODE_DIO = 1,
> +    MODE_QIO = 2
> +} SPIMode;
> +
>  #define M25P80_INTERNAL_DATA_BUFFER_SZ 16
>  
>  struct Flash {
> @@ -820,6 +826,17 @@ static void reset_memory(Flash *s)
>      trace_m25p80_reset_done(s);
>  }
>  
> +static uint8_t numonyx_get_mode(Flash *s) {
> +    if (!(s->enh_volatile_cfg & EVCFG_QUAD_IO_DISABLED)) {
> +        return MODE_QIO;
> +    } else if (!(s->enh_volatile_cfg & EVCFG_DUAL_IO_DISABLED)) {
> +        return MODE_DIO;
> +    } else {
> +        return MODE_STD;
> +    }
> +}
> +
>  static void decode_fast_read_cmd(Flash *s)  {
>      s->needed_bytes = get_addr_length(s); @@ -827,9 +844,11 @@ static 
> void decode_fast_read_cmd(Flash *s)
>      /* Dummy cycles - modeled with bytes writes instead of bits */
>      case MAN_WINBOND:
>          s->needed_bytes += 8;
> +        s->state = STATE_COLLECTING_DATA;
>          break;
>      case MAN_NUMONYX:
>          s->needed_bytes += extract32(s->volatile_cfg, 4, 4);
> +        s->state = STATE_COLLECTING_DATA;
>          break;
>      case MAN_MACRONIX:
>          if (extract32(s->volatile_cfg, 6, 2) == 1) { @@ -837,19 
> +856,21 @@ static void decode_fast_read_cmd(Flash *s)
>          } else {
>              s->needed_bytes += 8;
>          }
> +        s->state = STATE_COLLECTING_DATA;
>          break;
>      case MAN_SPANSION:
>          s->needed_bytes += extract32(s->spansion_cr2v,
>                                      SPANSION_DUMMY_CLK_POS,
>                                      SPANSION_DUMMY_CLK_LEN
>                                      );
> +        s->state = STATE_COLLECTING_DATA;
>          break;
>      default:
> +        s->state = STATE_COLLECTING_DATA;
>          break;
>      }
>      s->pos = 0;
>      s->len = 0;
> -    s->state = STATE_COLLECTING_DATA;

Above change in this function and the similar ones in below two functions don't seem to be needed anymore (s->state = STATE_COLLECTING_DATA is being done in all cases).
[Joe] Oops, I'll simplify that.

>  }
>  
>  static void decode_dio_read_cmd(Flash *s) @@ -859,6 +880,7 @@ static 
> void decode_dio_read_cmd(Flash *s)
>      switch (get_man(s)) {
>      case MAN_WINBOND:
>          s->needed_bytes += WINBOND_CONTINUOUS_READ_MODE_CMD_LEN;
> +        s->state = STATE_COLLECTING_DATA;
>          break;
>      case MAN_SPANSION:
>          s->needed_bytes += SPANSION_CONTINUOUS_READ_MODE_CMD_LEN;
> @@ -866,9 +888,11 @@ static void decode_dio_read_cmd(Flash *s)
>                                      SPANSION_DUMMY_CLK_POS,
>                                      SPANSION_DUMMY_CLK_LEN
>                                      );
> +        s->state = STATE_COLLECTING_DATA;
>          break;
>      case MAN_NUMONYX:
>          s->needed_bytes += extract32(s->volatile_cfg, 4, 4);
> +        s->state = STATE_COLLECTING_DATA;
>          break;
>      case MAN_MACRONIX:
>          switch (extract32(s->volatile_cfg, 6, 2)) { @@ -882,13 
> +906,14 @@ static void decode_dio_read_cmd(Flash *s)
>              s->needed_bytes += 4;
>              break;
>          }
> +        s->state = STATE_COLLECTING_DATA;
>          break;
>      default:
> +        s->state = STATE_COLLECTING_DATA;
>          break;
>      }
>      s->pos = 0;
>      s->len = 0;
> -    s->state = STATE_COLLECTING_DATA;
>  }
>  
>  static void decode_qio_read_cmd(Flash *s) @@ -899,6 +924,7 @@ static 
> void decode_qio_read_cmd(Flash *s)
>      case MAN_WINBOND:
>          s->needed_bytes += WINBOND_CONTINUOUS_READ_MODE_CMD_LEN;
>          s->needed_bytes += 4;
> +        s->state = STATE_COLLECTING_DATA;
>          break;
>      case MAN_SPANSION:
>          s->needed_bytes += SPANSION_CONTINUOUS_READ_MODE_CMD_LEN;
> @@ -906,9 +932,11 @@ static void decode_qio_read_cmd(Flash *s)
>                                      SPANSION_DUMMY_CLK_POS,
>                                      SPANSION_DUMMY_CLK_LEN
>                                      );
> +        s->state = STATE_COLLECTING_DATA;
>          break;
>      case MAN_NUMONYX:
>          s->needed_bytes += extract32(s->volatile_cfg, 4, 4);
> +        s->state = STATE_COLLECTING_DATA;
>          break;
>      case MAN_MACRONIX:
>          switch (extract32(s->volatile_cfg, 6, 2)) { @@ -922,13 
> +950,14 @@ static void decode_qio_read_cmd(Flash *s)
>              s->needed_bytes += 6;
>              break;
>          }
> +        s->state = STATE_COLLECTING_DATA;
>          break;
>      default:
> +        s->state = STATE_COLLECTING_DATA;
>          break;
>      }
>      s->pos = 0;
>      s->len = 0;
> -    s->state = STATE_COLLECTING_DATA;
>  }
>  
>  static void decode_new_cmd(Flash *s, uint32_t value) @@ -950,14 
> +979,8 @@ static void decode_new_cmd(Flash *s, uint32_t value)
>      case ERASE4_32K:
>      case ERASE_SECTOR:
>      case ERASE4_SECTOR:
> -    case READ:
> -    case READ4:
> -    case DPP:
> -    case QPP:
> -    case QPP_4:
>      case PP:
>      case PP4:
> -    case PP4_4:
>      case DIE_ERASE:
>      case RDID_90:
>      case RDID_AB:
> @@ -966,24 +989,85 @@ static void decode_new_cmd(Flash *s, uint32_t value)
>          s->len = 0;
>          s->state = STATE_COLLECTING_DATA;
>          break;
> +    case READ:
> +    case READ4:
> +        if (!(get_man(s) == MAN_NUMONYX) || (numonyx_get_mode(s) != MODE_DIO &&
> +            numonyx_get_mode(s) != MODE_QIO)) {

Above seems easier to check if we are in the correct mode here instead:

if (get_man(s) != MAN_NUMONYX || numonyx_get_mode(s) == MODE_STD) {

> +            s->needed_bytes = get_addr_length(s);
> +            s->pos = 0;
> +            s->len = 0;
> +            s->state = STATE_COLLECTING_DATA;
> +        } else {
> +            qemu_log_mask(LOG_GUEST_ERROR, "M25P80: Cannot execute cmd %x in "
> +                          "DIO or QIO mode\n", s->cmd_in_progress);
> +        }
> +        break;
> +    case DPP:
> +        if (!(get_man(s) == MAN_NUMONYX) || numonyx_get_mode(s) != MODE_QIO) {
> +            s->needed_bytes = get_addr_length(s);
> +            s->pos = 0;
> +            s->len = 0;
> +            s->state = STATE_COLLECTING_DATA;
> +        } else {
> +            qemu_log_mask(LOG_GUEST_ERROR, "M25P80: Cannot execute cmd %x in "
> +                          "QIO mode\n", s->cmd_in_progress);
> +        }
> +        break;
> +    case QPP:
> +    case QPP_4:
> +    case PP4_4:
> +        if (!(get_man(s) == MAN_NUMONYX) || numonyx_get_mode(s) != MODE_DIO) {
> +            s->needed_bytes = get_addr_length(s);
> +            s->pos = 0;
> +            s->len = 0;
> +            s->state = STATE_COLLECTING_DATA;
> +        } else {
> +            qemu_log_mask(LOG_GUEST_ERROR, "M25P80: Cannot execute cmd %x in "
> +                          "DIO mode\n", s->cmd_in_progress);
> +        }
> +        break;
>  
>      case FAST_READ:
>      case FAST_READ4:
> +        decode_fast_read_cmd(s);
> +        break;
>      case DOR:
>      case DOR4:
> +        if (!(get_man(s) == MAN_NUMONYX) || numonyx_get_mode(s) != MODE_QIO) {
> +            decode_fast_read_cmd(s);
> +        } else {
> +            qemu_log_mask(LOG_GUEST_ERROR, "M25P80: Cannot execute cmd %x in "
> +                          "QIO mode\n", s->cmd_in_progress);
> +        }
> +        break;
>      case QOR:
>      case QOR4:
> -        decode_fast_read_cmd(s);
> +        if (!(get_man(s) == MAN_NUMONYX) || numonyx_get_mode(s) != MODE_DIO) {
> +            decode_fast_read_cmd(s);
> +        } else {
> +            qemu_log_mask(LOG_GUEST_ERROR, "M25P80: Cannot execute cmd %x in "
> +                          "DIO mode\n", s->cmd_in_progress);
> +        }
>          break;
>  
>      case DIOR:
>      case DIOR4:
> -        decode_dio_read_cmd(s);
> +        if (!(get_man(s) == MAN_NUMONYX) || numonyx_get_mode(s) != MODE_QIO) {
> +            decode_dio_read_cmd(s);
> +        } else {
> +            qemu_log_mask(LOG_GUEST_ERROR, "M25P80: Cannot execute cmd %x in "
> +                          "QIO mode\n", s->cmd_in_progress);
> +        }
>          break;
>  
>      case QIOR:
>      case QIOR4:
> -        decode_qio_read_cmd(s);
> +        if (!(get_man(s) == MAN_NUMONYX) || numonyx_get_mode(s) != MODE_DIO) {
> +            decode_qio_read_cmd(s);
> +        } else {
> +            qemu_log_mask(LOG_GUEST_ERROR, "M25P80: Cannot execute cmd %x in "
> +                          "DIO mode\n", s->cmd_in_progress);
> +        }
>          break;
>  
>      case WRSR:
> @@ -1035,17 +1119,23 @@ static void decode_new_cmd(Flash *s, uint32_t value)
>          break;
>  
>      case JEDEC_READ:
> -        trace_m25p80_populated_jedec(s);
> -        for (i = 0; i < s->pi->id_len; i++) {
> -            s->data[i] = s->pi->id[i];
> -        }
> -        for (; i < SPI_NOR_MAX_ID_LEN; i++) {
> -            s->data[i] = 0;
> -        }
> +        if (!(get_man(s) == MAN_NUMONYX) || (numonyx_get_mode(s) != MODE_DIO &&
> +            numonyx_get_mode(s) != MODE_QIO)) {

The same here as above:
if (get_man(s) != MAN_NUMONYX || numonyx_get_mode(s) == MODE_STD) {

[Joe] Agreed, that's much easier and keeps it on one line, not sure how I missed that...
Thanks!
Joe

Best regards,
Francisco Iglesias


> +            trace_m25p80_populated_jedec(s);
> +            for (i = 0; i < s->pi->id_len; i++) {
> +                s->data[i] = s->pi->id[i];
> +            }
> +            for (; i < SPI_NOR_MAX_ID_LEN; i++) {
> +                s->data[i] = 0;
> +            }
>  
> -        s->len = SPI_NOR_MAX_ID_LEN;
> -        s->pos = 0;
> -        s->state = STATE_READING_DATA;
> +            s->len = SPI_NOR_MAX_ID_LEN;
> +            s->pos = 0;
> +            s->state = STATE_READING_DATA;
> +        } else {
> +            qemu_log_mask(LOG_GUEST_ERROR, "M25P80: Cannot execute JEDEC read "
> +                          "in DIO or QIO mode\n");
> +        }
>          break;
>  
>      case RDCR:
> --
> 2.7.4
>