[PATCH v5 0/4] hw/block/m25p80: Numonyx: Fix dummy cycles and check for SPI mode on cmds

[PATCH v5 0/4] hw/block/m25p80: Numonyx: Fix dummy cycles and check for SPI mode on cmds

[Joe Komlodi]

Changelog:
v4 -> v5
 - 3/4: Simplify logic when changing state and checking mode.
 - 3/4: numonyx_get_mode -> numonyx_mode
 - 4/4: Reword commit message to include QIO mode.

v3 -> v4
 - 1/4: Patch changed to change names of register fields to be more accurate.
 - 1/4: Revert polarity change from v3.
 - 2/4: Added, fixes polarity of VCFG XIP mode when copied from NVCFG.
 - 3/4: Removed check_cmd_mode function, each command check is done in decode_new_cmd instead.
 - 3/4: Add guest error print if JEDEC read is executed in QIO or DIO mode.
 - 3/4: Don't check PP and PP4, they work regardless of mode. PP4_4 is left as is.
 - 3/4: Simplify get_mode function.
 - 4/4: Simplify extract_cfg_num_dummies function.
 - 4/4: Use switch statement instead of table for cycle retrieving.

v2 -> v3
 - 1/3: Added, Fixes NVCFG polarity for DIO/QIO.
 - 2/3: Added, Checks if we can execute the current command in standard/DIO/QIO mode.
 - 3/3: Was 1/1 in v2.  Added cycle counts for DIO/QIO mode.

v1 -> v2
 - 1/2: Change function name to be more accurate
 - 2/2: Dropped

Hi all,

The series fixes the behavior of the dummy cycle register for Numonyx flashes so
it's closer to how hardware behaves.
It also checks if a command can be executed in the current SPI mode
(standard, DIO, or QIO) before extracting dummy cycles for the command.

On hardware, the dummy cycles for fast read commands are set to a specific value
(8 or 10) if the register is all 0s or 1s.
If the register value isn't all 0s or 1s, then the flash expects the amount of
cycles sent to be equal to the count in the register.

Thanks!
Joe

Joe Komlodi (4):
  hw/block/m25p80: Make Numonyx config field names more accurate
  hw/block/m25p80: Fix when VCFG XIP bit is set for Numonyx
  hw/block/m25p80: Check SPI mode before running some Numonyx commands
  hw/block/m25p80: Fix Numonyx fast read dummy cycle count

 hw/block/m25p80.c | 158 ++++++++++++++++++++++++++++++++++++++++++++----------
 1 file changed, 129 insertions(+), 29 deletions(-)

-- 
2.7.4

[PATCH v5 1/4] hw/block/m25p80: Make Numonyx config field names more accurate

[Joe Komlodi]

The previous naming of the configuration registers made it sound like that if
the bits were set the settings would be enabled, while the opposite is true.

Signed-off-by: Joe Komlodi <komlodi@xilinx.com>
Reviewed-by: Francisco Iglesias <francisco.iglesias@xilinx.com>
---
 hw/block/m25p80.c | 12 ++++++------
 1 file changed, 6 insertions(+), 6 deletions(-)

diff --git a/hw/block/m25p80.c b/hw/block/m25p80.c
index 483925f..452d252 100644
--- a/hw/block/m25p80.c
+++ b/hw/block/m25p80.c
@@ -136,7 +136,7 @@ typedef struct FlashPartInfo {
 #define VCFG_WRAP_SEQUENTIAL 0x2
 #define NVCFG_XIP_MODE_DISABLED (7 << 9)
 #define NVCFG_XIP_MODE_MASK (7 << 9)
-#define VCFG_XIP_MODE_ENABLED (1 << 3)
+#define VCFG_XIP_MODE_DISABLED (1 << 3)
 #define CFG_DUMMY_CLK_LEN 4
 #define NVCFG_DUMMY_CLK_POS 12
 #define VCFG_DUMMY_CLK_POS 4
@@ -144,9 +144,9 @@ typedef struct FlashPartInfo {
 #define EVCFG_VPP_ACCELERATOR (1 << 3)
 #define EVCFG_RESET_HOLD_ENABLED (1 << 4)
 #define NVCFG_DUAL_IO_MASK (1 << 2)
-#define EVCFG_DUAL_IO_ENABLED (1 << 6)
+#define EVCFG_DUAL_IO_DISABLED (1 << 6)
 #define NVCFG_QUAD_IO_MASK (1 << 3)
-#define EVCFG_QUAD_IO_ENABLED (1 << 7)
+#define EVCFG_QUAD_IO_DISABLED (1 << 7)
 #define NVCFG_4BYTE_ADDR_MASK (1 << 0)
 #define NVCFG_LOWER_SEGMENT_MASK (1 << 1)
 
@@ -769,7 +769,7 @@ static void reset_memory(Flash *s)
         s->volatile_cfg |= VCFG_WRAP_SEQUENTIAL;
         if ((s->nonvolatile_cfg & NVCFG_XIP_MODE_MASK)
                                 != NVCFG_XIP_MODE_DISABLED) {
-            s->volatile_cfg |= VCFG_XIP_MODE_ENABLED;
+            s->volatile_cfg |= VCFG_XIP_MODE_DISABLED;
         }
         s->volatile_cfg |= deposit32(s->volatile_cfg,
                             VCFG_DUMMY_CLK_POS,
@@ -784,10 +784,10 @@ static void reset_memory(Flash *s)
         s->enh_volatile_cfg |= EVCFG_VPP_ACCELERATOR;
         s->enh_volatile_cfg |= EVCFG_RESET_HOLD_ENABLED;
         if (s->nonvolatile_cfg & NVCFG_DUAL_IO_MASK) {
-            s->enh_volatile_cfg |= EVCFG_DUAL_IO_ENABLED;
+            s->enh_volatile_cfg |= EVCFG_DUAL_IO_DISABLED;
         }
         if (s->nonvolatile_cfg & NVCFG_QUAD_IO_MASK) {
-            s->enh_volatile_cfg |= EVCFG_QUAD_IO_ENABLED;
+            s->enh_volatile_cfg |= EVCFG_QUAD_IO_DISABLED;
         }
         if (!(s->nonvolatile_cfg & NVCFG_4BYTE_ADDR_MASK)) {
             s->four_bytes_address_mode = true;
-- 
2.7.4

[PATCH v5 2/4] hw/block/m25p80: Fix when VCFG XIP bit is set for Numonyx

[Joe Komlodi]

VCFG XIP is set (disabled) when the NVCFG XIP bits are all set (disabled).

Signed-off-by: Joe Komlodi <komlodi@xilinx.com>
Reviewed-by: Francisco Iglesias <francisco.iglesias@xilinx.com>
---
 hw/block/m25p80.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/hw/block/m25p80.c b/hw/block/m25p80.c
index 452d252..eb6539f 100644
--- a/hw/block/m25p80.c
+++ b/hw/block/m25p80.c
@@ -768,7 +768,7 @@ static void reset_memory(Flash *s)
         s->volatile_cfg |= VCFG_DUMMY;
         s->volatile_cfg |= VCFG_WRAP_SEQUENTIAL;
         if ((s->nonvolatile_cfg & NVCFG_XIP_MODE_MASK)
-                                != NVCFG_XIP_MODE_DISABLED) {
+                                == NVCFG_XIP_MODE_DISABLED) {
             s->volatile_cfg |= VCFG_XIP_MODE_DISABLED;
         }
         s->volatile_cfg |= deposit32(s->volatile_cfg,
-- 
2.7.4

[PATCH v5 3/4] hw/block/m25p80: Check SPI mode before running some Numonyx commands

[Joe Komlodi]

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 | 114 +++++++++++++++++++++++++++++++++++++++++++++---------
 1 file changed, 95 insertions(+), 19 deletions(-)

diff --git a/hw/block/m25p80.c b/hw/block/m25p80.c
index eb6539f..56bd5bc 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_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);
@@ -950,14 +967,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 +977,84 @@ 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_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_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_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_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_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_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_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 +1106,22 @@ 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_mode(s) == MODE_STD) {
+            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

[PATCH v5 4/4] hw/block/m25p80: Fix Numonyx fast read dummy cycle count

[Joe Komlodi]

Numonyx chips determine the number of cycles to wait based on bits 7:4
in the volatile configuration register.

However, if these bits are 0x0 or 0xF, the number of dummy cycles to
wait is 10 for QIOR and QIOR4 commands or when in QIO mode, and otherwise 8 for
the currently supported fast read commands. [1]

[1]
https://www.micron.com/-/media/client/global/documents/products/data-sheet/nor-flash/serial-nor/mt25q/die-rev-b/mt25q_qlkt_u_02g_cbb_0.pdf?rev=9b167fbf2b3645efba6385949a72e453

Signed-off-by: Joe Komlodi <komlodi@xilinx.com>
Reviewed-by: Francisco Iglesias <francisco.iglesias@xilinx.com>
---
 hw/block/m25p80.c | 30 +++++++++++++++++++++++++++---
 1 file changed, 27 insertions(+), 3 deletions(-)

diff --git a/hw/block/m25p80.c b/hw/block/m25p80.c
index 56bd5bc..a67dc53 100644
--- a/hw/block/m25p80.c
+++ b/hw/block/m25p80.c
@@ -837,6 +837,30 @@ static uint8_t numonyx_mode(Flash *s)
     }
 }
 
+static uint8_t numonyx_extract_cfg_num_dummies(Flash *s)
+{
+    uint8_t num_dummies;
+    uint8_t mode;
+    assert(get_man(s) == MAN_NUMONYX);
+
+    mode = numonyx_mode(s);
+    num_dummies = extract32(s->volatile_cfg, 4, 4);
+
+    if (num_dummies == 0x0 || num_dummies == 0xf) {
+        switch (s->cmd_in_progress) {
+        case QIOR:
+        case QIOR4:
+            num_dummies = 10;
+            break;
+        default:
+            num_dummies = (mode == MODE_QIO) ? 10 : 8;
+            break;
+        }
+    }
+
+    return num_dummies;
+}
+
 static void decode_fast_read_cmd(Flash *s)
 {
     s->needed_bytes = get_addr_length(s);
@@ -846,7 +870,7 @@ static void decode_fast_read_cmd(Flash *s)
         s->needed_bytes += 8;
         break;
     case MAN_NUMONYX:
-        s->needed_bytes += extract32(s->volatile_cfg, 4, 4);
+        s->needed_bytes += numonyx_extract_cfg_num_dummies(s);
         break;
     case MAN_MACRONIX:
         if (extract32(s->volatile_cfg, 6, 2) == 1) {
@@ -885,7 +909,7 @@ static void decode_dio_read_cmd(Flash *s)
                                     );
         break;
     case MAN_NUMONYX:
-        s->needed_bytes += extract32(s->volatile_cfg, 4, 4);
+        s->needed_bytes += numonyx_extract_cfg_num_dummies(s);
         break;
     case MAN_MACRONIX:
         switch (extract32(s->volatile_cfg, 6, 2)) {
@@ -925,7 +949,7 @@ static void decode_qio_read_cmd(Flash *s)
                                     );
         break;
     case MAN_NUMONYX:
-        s->needed_bytes += extract32(s->volatile_cfg, 4, 4);
+        s->needed_bytes += numonyx_extract_cfg_num_dummies(s);
         break;
     case MAN_MACRONIX:
         switch (extract32(s->volatile_cfg, 6, 2)) {
-- 
2.7.4

Re: [PATCH v5 3/4] hw/block/m25p80: Check SPI mode before running some Numonyx commands

[Francisco Iglesias]

On Mon, Nov 16, 2020 at 03:11:03PM -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>

Reviewed-by: Francisco Iglesias <francisco.iglesias@xilinx.com>

> ---
>  hw/block/m25p80.c | 114 +++++++++++++++++++++++++++++++++++++++++++++---------
>  1 file changed, 95 insertions(+), 19 deletions(-)
> 
> diff --git a/hw/block/m25p80.c b/hw/block/m25p80.c
> index eb6539f..56bd5bc 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_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);
> @@ -950,14 +967,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 +977,84 @@ 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_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_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_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_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_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_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_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 +1106,22 @@ 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_mode(s) == MODE_STD) {
> +            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
> 

RE: [PATCH v5 0/4] hw/block/m25p80: Numonyx: Fix dummy cycles and check for SPI mode on cmds

[Joe Komlodi]

Hi Peter,

This series has been reviewed, but it looks like it slipped through the cracks.
Is it possible it could be merged through your tree, assuming it looks good?

Thanks!
Joe

-----Original Message-----
From: Qemu-devel <qemu-devel-bounces+komlodi=xilinx.com@nongnu.org> On Behalf Of Joe Komlodi
Sent: Monday, November 16, 2020 3:11 PM
To: qemu-devel@nongnu.org
Cc: Francisco Eduardo Iglesias <figlesia@xilinx.com>; alistair@alistair23.me; philippe.mathieu.daude@gmail.com; qemu-block@nongnu.org; mreitz@redhat.com
Subject: [PATCH v5 0/4] hw/block/m25p80: Numonyx: Fix dummy cycles and check for SPI mode on cmds

Changelog:
v4 -> v5
 - 3/4: Simplify logic when changing state and checking mode.
 - 3/4: numonyx_get_mode -> numonyx_mode
 - 4/4: Reword commit message to include QIO mode.

v3 -> v4
 - 1/4: Patch changed to change names of register fields to be more accurate.
 - 1/4: Revert polarity change from v3.
 - 2/4: Added, fixes polarity of VCFG XIP mode when copied from NVCFG.
 - 3/4: Removed check_cmd_mode function, each command check is done in decode_new_cmd instead.
 - 3/4: Add guest error print if JEDEC read is executed in QIO or DIO mode.
 - 3/4: Don't check PP and PP4, they work regardless of mode. PP4_4 is left as is.
 - 3/4: Simplify get_mode function.
 - 4/4: Simplify extract_cfg_num_dummies function.
 - 4/4: Use switch statement instead of table for cycle retrieving.

v2 -> v3
 - 1/3: Added, Fixes NVCFG polarity for DIO/QIO.
 - 2/3: Added, Checks if we can execute the current command in standard/DIO/QIO mode.
 - 3/3: Was 1/1 in v2.  Added cycle counts for DIO/QIO mode.

v1 -> v2
 - 1/2: Change function name to be more accurate
 - 2/2: Dropped

Hi all,

The series fixes the behavior of the dummy cycle register for Numonyx flashes so it's closer to how hardware behaves.
It also checks if a command can be executed in the current SPI mode (standard, DIO, or QIO) before extracting dummy cycles for the command.

On hardware, the dummy cycles for fast read commands are set to a specific value
(8 or 10) if the register is all 0s or 1s.
If the register value isn't all 0s or 1s, then the flash expects the amount of cycles sent to be equal to the count in the register.

Thanks!
Joe

Joe Komlodi (4):
  hw/block/m25p80: Make Numonyx config field names more accurate
  hw/block/m25p80: Fix when VCFG XIP bit is set for Numonyx
  hw/block/m25p80: Check SPI mode before running some Numonyx commands
  hw/block/m25p80: Fix Numonyx fast read dummy cycle count

 hw/block/m25p80.c | 158 ++++++++++++++++++++++++++++++++++++++++++++----------
 1 file changed, 129 insertions(+), 29 deletions(-)

--
2.7.4

Re: [PATCH v5 0/4] hw/block/m25p80: Numonyx: Fix dummy cycles and check for SPI mode on cmds

[Peter Maydell]

On Mon, 16 Nov 2020 at 23:13, Joe Komlodi <joe.komlodi@xilinx.com> wrote:
>
> Changelog:
> v4 -> v5
>  - 3/4: Simplify logic when changing state and checking mode.
>  - 3/4: numonyx_get_mode -> numonyx_mode
>  - 4/4: Reword commit message to include QIO mode.
>
> v3 -> v4
>  - 1/4: Patch changed to change names of register fields to be more accurate.
>  - 1/4: Revert polarity change from v3.
>  - 2/4: Added, fixes polarity of VCFG XIP mode when copied from NVCFG.
>  - 3/4: Removed check_cmd_mode function, each command check is done in decode_new_cmd instead.
>  - 3/4: Add guest error print if JEDEC read is executed in QIO or DIO mode.
>  - 3/4: Don't check PP and PP4, they work regardless of mode. PP4_4 is left as is.
>  - 3/4: Simplify get_mode function.
>  - 4/4: Simplify extract_cfg_num_dummies function.
>  - 4/4: Use switch statement instead of table for cycle retrieving.
>
> v2 -> v3
>  - 1/3: Added, Fixes NVCFG polarity for DIO/QIO.
>  - 2/3: Added, Checks if we can execute the current command in standard/DIO/QIO mode.
>  - 3/3: Was 1/1 in v2.  Added cycle counts for DIO/QIO mode.
>
> v1 -> v2
>  - 1/2: Change function name to be more accurate
>  - 2/2: Dropped
>
> Hi all,
>
> The series fixes the behavior of the dummy cycle register for Numonyx flashes so
> it's closer to how hardware behaves.
> It also checks if a command can be executed in the current SPI mode
> (standard, DIO, or QIO) before extracting dummy cycles for the command.
>
> On hardware, the dummy cycles for fast read commands are set to a specific value
> (8 or 10) if the register is all 0s or 1s.
> If the register value isn't all 0s or 1s, then the flash expects the amount of
> cycles sent to be equal to the count in the register.



Applied to target-arm.next, thanks.

-- PMM