[Qemu-devel] [PATCH v4 00/11] Support for N25Q256/512 and AT25128/256

[Qemu-devel] [PATCH v4 00/11] Support for N25Q256/512 and AT25128/256

[marcin.krzeminski]

From: Marcin Krzeminski <marcin.krzeminski@nokia.com>

V4:
- Fixed RNVCR command (needed bytes set to 2 instead of 1)
- Config registers are configured only for micron flash devices
- Move config registers initialization to reset_memory function
- Removed clearing reset_enable flag when chip was selcted by CS signal
V3:
- Checkpatch run on patches
- Renamed function
V2:
- Removed support for mx66u51235 and s25fl512s from this series
- Corrected/implemented dummy cycles
- rebased to master

Marcin Krzeminski (11):
  block: m25p80: Removed unused variable
  block: m25p80: RESET_ENABLE and RESET_MEMORY commnads
  block: m25p80: Widen flags variable
  block: m25p80: Extend address mode
  block: m25p80: 4byte address mode
  block: m25p80: Add configuration registers
  block: m25p80: Dummy cycles for N25Q256/512
  block: m25p80: Fast read and 4bytes commands
  block: m25p80: Implemented FSR register
  block: m25p80: n25q256a/n25q512a models
  block: m25p80: at25128a/at25256a models

 hw/block/m25p80.c | 292 ++++++++++++++++++++++++++++++++++++++++++++++++++++--
 1 file changed, 281 insertions(+), 11 deletions(-)

-- 
2.5.0

[Qemu-devel] [PATCH v4 01/11] block: m25p80: Removed unused variable

[marcin.krzeminski]

From: Marcin Krzeminski <marcin.krzeminski@nokia.com>

Signed-off-by: Marcin Krzeminski <marcin.krzeminski@nokia.com>
Reviewed-by: Peter Crosthwaite <crosthwaite.peter@gmail.com>
---
 hw/block/m25p80.c | 2 --
 1 file changed, 2 deletions(-)

diff --git a/hw/block/m25p80.c b/hw/block/m25p80.c
index de24f42..2222124 100644
--- a/hw/block/m25p80.c
+++ b/hw/block/m25p80.c
@@ -246,8 +246,6 @@ typedef enum {
 typedef struct Flash {
     SSISlave parent_obj;
 
-    uint32_t r;
-
     BlockBackend *blk;
 
     uint8_t *storage;
-- 
2.5.0

[Qemu-devel] [PATCH v4 02/11] block: m25p80: RESET_ENABLE and RESET_MEMORY commnads

[marcin.krzeminski]

From: Marcin Krzeminski <marcin.krzeminski@nokia.com>

Signed-off-by: Marcin Krzeminski <marcin.krzeminski@nokia.com>
---
 hw/block/m25p80.c | 35 ++++++++++++++++++++++++++++++++++-
 1 file changed, 34 insertions(+), 1 deletion(-)

diff --git a/hw/block/m25p80.c b/hw/block/m25p80.c
index 2222124..06b0af3 100644
--- a/hw/block/m25p80.c
+++ b/hw/block/m25p80.c
@@ -233,6 +233,9 @@ typedef enum {
     ERASE_4K = 0x20,
     ERASE_32K = 0x52,
     ERASE_SECTOR = 0xd8,
+
+    RESET_ENABLE = 0x66,
+    RESET_MEMORY = 0x99,
 } FlashCMD;
 
 typedef enum {
@@ -260,6 +263,7 @@ typedef struct Flash {
     uint8_t cmd_in_progress;
     uint64_t cur_addr;
     bool write_enable;
+    bool reset_enable;
 
     int64_t dirty_page;
 
@@ -432,11 +436,29 @@ static void complete_collecting_data(Flash *s)
     }
 }
 
+static void reset_memory(Flash *s)
+{
+    s->cmd_in_progress = NOP;
+    s->cur_addr = 0;
+    s->len = 0;
+    s->needed_bytes = 0;
+    s->pos = 0;
+    s->state = STATE_IDLE;
+    s->write_enable = false;
+    s->reset_enable = false;
+
+    DB_PRINT_L(0, "Reset done.\n");
+}
+
 static void decode_new_cmd(Flash *s, uint32_t value)
 {
     s->cmd_in_progress = value;
     DB_PRINT_L(0, "decoded new command:%x\n", value);
 
+    if (value != RESET_MEMORY) {
+        s->reset_enable = false;
+    }
+
     switch (value) {
 
     case ERASE_4K:
@@ -541,6 +563,14 @@ static void decode_new_cmd(Flash *s, uint32_t value)
         break;
     case NOP:
         break;
+    case RESET_ENABLE:
+        s->reset_enable = true;
+        break;
+    case RESET_MEMORY:
+        if (s->reset_enable) {
+            reset_memory(s);
+        }
+        break;
     default:
         qemu_log_mask(LOG_GUEST_ERROR, "M25P80: Unknown cmd %x\n", value);
         break;
@@ -622,6 +652,8 @@ static int m25p80_init(SSISlave *ss)
     s->size = s->pi->sector_size * s->pi->n_sectors;
     s->dirty_page = -1;
 
+    reset_memory(s);
+
     /* FIXME use a qdev drive property instead of drive_get_next() */
     dinfo = drive_get_next(IF_MTD);
 
@@ -654,7 +686,7 @@ static void m25p80_pre_save(void *opaque)
 
 static const VMStateDescription vmstate_m25p80 = {
     .name = "xilinx_spi",
-    .version_id = 1,
+    .version_id = 2,
     .minimum_version_id = 1,
     .pre_save = m25p80_pre_save,
     .fields = (VMStateField[]) {
@@ -666,6 +698,7 @@ static const VMStateDescription vmstate_m25p80 = {
         VMSTATE_UINT8(cmd_in_progress, Flash),
         VMSTATE_UINT64(cur_addr, Flash),
         VMSTATE_BOOL(write_enable, Flash),
+        VMSTATE_BOOL(reset_enable, Flash),
         VMSTATE_END_OF_LIST()
     }
 };
-- 
2.5.0

[Qemu-devel] [PATCH v4 03/11] block: m25p80: Widen flags variable

[marcin.krzeminski]

From: Marcin Krzeminski <marcin.krzeminski@nokia.com>

Extend the width of the flags variable to support the already existing
(but unused) WR_1 flag, which is above the range of 8 bits.
This allows support of EEPROM emulation which requires the WR_1 feature.

Signed-off-by: Marcin Krzeminski <marcin.krzeminski@nokia.com>
Reviewed-by: Peter Crosthwaite <crosthwaite.peter@gmail.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 06b0af3..a3ea994 100644
--- a/hw/block/m25p80.c
+++ b/hw/block/m25p80.c
@@ -61,7 +61,7 @@ typedef struct FlashPartInfo {
     uint32_t sector_size;
     uint32_t n_sectors;
     uint32_t page_size;
-    uint8_t flags;
+    uint16_t flags;
 } FlashPartInfo;
 
 /* adapted from linux */
-- 
2.5.0

[Qemu-devel] [PATCH v4 04/11] block: m25p80: Extend address mode

[marcin.krzeminski]

From: Marcin Krzeminski <marcin.krzeminski@nokia.com>

Extend address mode allows to switch flash 16 MiB banks,
allowing user to access all flash sectors.
This access mode is used by u-boot.

Signed-off-by: Marcin Krzeminski <marcin.krzeminski@nokia.com>
Reviewed-by: Peter Crosthwaite <crosthwaite.peter@gmail.com>
---
 hw/block/m25p80.c | 27 +++++++++++++++++++++++++++
 1 file changed, 27 insertions(+)

diff --git a/hw/block/m25p80.c b/hw/block/m25p80.c
index a3ea994..0540dde 100644
--- a/hw/block/m25p80.c
+++ b/hw/block/m25p80.c
@@ -48,6 +48,9 @@
  */
 #define WR_1 0x100
 
+/* 16 MiB max in 3 byte address mode */
+#define MAX_3BYTES_SIZE 0x1000000
+
 typedef struct FlashPartInfo {
     const char *part_name;
     /* jedec code. (jedec >> 16) & 0xff is the 1st byte, >> 8 the 2nd etc */
@@ -234,6 +237,9 @@ typedef enum {
     ERASE_32K = 0x52,
     ERASE_SECTOR = 0xd8,
 
+    EXTEND_ADDR_READ = 0xC8,
+    EXTEND_ADDR_WRITE = 0xC5,
+
     RESET_ENABLE = 0x66,
     RESET_MEMORY = 0x99,
 } FlashCMD;
@@ -264,6 +270,7 @@ typedef struct Flash {
     uint64_t cur_addr;
     bool write_enable;
     bool reset_enable;
+    uint8_t ear;
 
     int64_t dirty_page;
 
@@ -404,6 +411,7 @@ static void complete_collecting_data(Flash *s)
     s->cur_addr = s->data[0] << 16;
     s->cur_addr |= s->data[1] << 8;
     s->cur_addr |= s->data[2];
+    s->cur_addr += (s->ear & 0x3) * MAX_3BYTES_SIZE;
 
     s->state = STATE_IDLE;
 
@@ -431,6 +439,9 @@ static void complete_collecting_data(Flash *s)
             s->write_enable = false;
         }
         break;
+    case EXTEND_ADDR_WRITE:
+        s->ear = s->data[0];
+        break;
     default:
         break;
     }
@@ -440,6 +451,7 @@ static void reset_memory(Flash *s)
 {
     s->cmd_in_progress = NOP;
     s->cur_addr = 0;
+    s->ear = 0;
     s->len = 0;
     s->needed_bytes = 0;
     s->pos = 0;
@@ -563,6 +575,20 @@ static void decode_new_cmd(Flash *s, uint32_t value)
         break;
     case NOP:
         break;
+    case EXTEND_ADDR_READ:
+        s->data[0] = s->ear;
+        s->pos = 0;
+        s->len = 1;
+        s->state = STATE_READING_DATA;
+        break;
+    case EXTEND_ADDR_WRITE:
+        if (s->write_enable) {
+            s->needed_bytes = 1;
+            s->pos = 0;
+            s->len = 0;
+            s->state = STATE_COLLECTING_DATA;
+        }
+        break;
     case RESET_ENABLE:
         s->reset_enable = true;
         break;
@@ -698,6 +724,7 @@ static const VMStateDescription vmstate_m25p80 = {
         VMSTATE_UINT8(cmd_in_progress, Flash),
         VMSTATE_UINT64(cur_addr, Flash),
         VMSTATE_BOOL(write_enable, Flash),
+        VMSTATE_UINT8(ear, Flash),
         VMSTATE_BOOL(reset_enable, Flash),
         VMSTATE_END_OF_LIST()
     }
-- 
2.5.0

[Qemu-devel] [PATCH v4 05/11] block: m25p80: 4byte address mode

[marcin.krzeminski]

From: Marcin Krzeminski <marcin.krzeminski@nokia.com>

This patch adds only 4byte address mode (does not cover dummy cycles).
This mode is needed to access more than 16 MiB of flash.

Signed-off-by: Marcin Krzeminski <marcin.krzeminski@nokia.com>
---
 hw/block/m25p80.c | 41 +++++++++++++++++++++++++++++++++++------
 1 file changed, 35 insertions(+), 6 deletions(-)

diff --git a/hw/block/m25p80.c b/hw/block/m25p80.c
index 0540dde..0698e7b 100644
--- a/hw/block/m25p80.c
+++ b/hw/block/m25p80.c
@@ -237,6 +237,9 @@ typedef enum {
     ERASE_32K = 0x52,
     ERASE_SECTOR = 0xd8,
 
+    EN_4BYTE_ADDR = 0xB7,
+    EX_4BYTE_ADDR = 0xE9,
+
     EXTEND_ADDR_READ = 0xC8,
     EXTEND_ADDR_WRITE = 0xC5,
 
@@ -269,6 +272,7 @@ typedef struct Flash {
     uint8_t cmd_in_progress;
     uint64_t cur_addr;
     bool write_enable;
+    bool four_bytes_address_mode;
     bool reset_enable;
     uint8_t ear;
 
@@ -406,12 +410,25 @@ void flash_write8(Flash *s, uint64_t addr, uint8_t data)
     s->dirty_page = page;
 }
 
+static inline int get_addr_length(Flash *s)
+{
+    return s->four_bytes_address_mode ? 4 : 3;
+}
+
 static void complete_collecting_data(Flash *s)
 {
-    s->cur_addr = s->data[0] << 16;
-    s->cur_addr |= s->data[1] << 8;
-    s->cur_addr |= s->data[2];
-    s->cur_addr += (s->ear & 0x3) * MAX_3BYTES_SIZE;
+    int i;
+
+    s->cur_addr = 0;
+
+    for (i = 0; i < get_addr_length(s); ++i) {
+        s->cur_addr <<= 8;
+        s->cur_addr |= s->data[i];
+    }
+
+    if (get_addr_length(s) == 3) {
+        s->cur_addr += (s->ear & 0x3) * MAX_3BYTES_SIZE;
+    }
 
     s->state = STATE_IDLE;
 
@@ -452,6 +469,7 @@ static void reset_memory(Flash *s)
     s->cmd_in_progress = NOP;
     s->cur_addr = 0;
     s->ear = 0;
+    s->four_bytes_address_mode = false;
     s->len = 0;
     s->needed_bytes = 0;
     s->pos = 0;
@@ -480,7 +498,7 @@ static void decode_new_cmd(Flash *s, uint32_t value)
     case DPP:
     case QPP:
     case PP:
-        s->needed_bytes = 3;
+        s->needed_bytes = get_addr_length(s);
         s->pos = 0;
         s->len = 0;
         s->state = STATE_COLLECTING_DATA;
@@ -489,7 +507,7 @@ static void decode_new_cmd(Flash *s, uint32_t value)
     case FAST_READ:
     case DOR:
     case QOR:
-        s->needed_bytes = 4;
+        s->needed_bytes = get_addr_length(s);
         s->pos = 0;
         s->len = 0;
         s->state = STATE_COLLECTING_DATA;
@@ -502,6 +520,8 @@ static void decode_new_cmd(Flash *s, uint32_t value)
             s->needed_bytes = 4;
             break;
         case JEDEC_NUMONYX:
+            s->needed_bytes = get_addr_length(s);
+            break;
         default:
             s->needed_bytes = 5;
         }
@@ -517,6 +537,8 @@ static void decode_new_cmd(Flash *s, uint32_t value)
             s->needed_bytes = 6;
             break;
         case JEDEC_NUMONYX:
+            s->needed_bytes = get_addr_length(s);
+            break;
         default:
             s->needed_bytes = 8;
         }
@@ -575,6 +597,12 @@ static void decode_new_cmd(Flash *s, uint32_t value)
         break;
     case NOP:
         break;
+    case EN_4BYTE_ADDR:
+        s->four_bytes_address_mode = true;
+        break;
+    case EX_4BYTE_ADDR:
+        s->four_bytes_address_mode = false;
+        break;
     case EXTEND_ADDR_READ:
         s->data[0] = s->ear;
         s->pos = 0;
@@ -724,6 +752,7 @@ static const VMStateDescription vmstate_m25p80 = {
         VMSTATE_UINT8(cmd_in_progress, Flash),
         VMSTATE_UINT64(cur_addr, Flash),
         VMSTATE_BOOL(write_enable, Flash),
+        VMSTATE_BOOL(four_bytes_address_mode, Flash),
         VMSTATE_UINT8(ear, Flash),
         VMSTATE_BOOL(reset_enable, Flash),
         VMSTATE_END_OF_LIST()
-- 
2.5.0

[Qemu-devel] [PATCH v4 06/11] block: m25p80: Add configuration registers

[marcin.krzeminski]

From: Marcin Krzeminski <marcin.krzeminski@nokia.com>

This patch adds both volatile and non volatile configuration registers
and commands to allow modify them. It is needed for proper handling
dummy cycles. Initialization of those registers and flash state
has been included as well.
Some of this registers are used by kernel.

Signed-off-by: Marcin Krzeminski <marcin.krzeminski@nokia.com>
---
 hw/block/m25p80.c | 110 ++++++++++++++++++++++++++++++++++++++++++++++++++++++
 1 file changed, 110 insertions(+)

diff --git a/hw/block/m25p80.c b/hw/block/m25p80.c
index 0698e7b..9d5a071 100644
--- a/hw/block/m25p80.c
+++ b/hw/block/m25p80.c
@@ -26,6 +26,7 @@
 #include "sysemu/block-backend.h"
 #include "sysemu/blockdev.h"
 #include "hw/ssi/ssi.h"
+#include "qemu/bitops.h"
 
 #ifndef M25P80_ERR_DEBUG
 #define M25P80_ERR_DEBUG 0
@@ -245,6 +246,15 @@ typedef enum {
 
     RESET_ENABLE = 0x66,
     RESET_MEMORY = 0x99,
+
+    RNVCR = 0xB5,
+    WNVCR = 0xB1,
+
+    RVCR = 0x85,
+    WVCR = 0x81,
+
+    REVCR = 0x65,
+    WEVCR = 0x61,
 } FlashCMD;
 
 typedef enum {
@@ -271,6 +281,9 @@ typedef struct Flash {
     uint8_t needed_bytes;
     uint8_t cmd_in_progress;
     uint64_t cur_addr;
+    uint32_t nonvolatile_cfg;
+    uint32_t volatile_cfg;
+    uint32_t enh_volatile_cfg;
     bool write_enable;
     bool four_bytes_address_mode;
     bool reset_enable;
@@ -459,6 +472,15 @@ static void complete_collecting_data(Flash *s)
     case EXTEND_ADDR_WRITE:
         s->ear = s->data[0];
         break;
+    case WNVCR:
+        s->nonvolatile_cfg = s->data[0] | (s->data[1] << 8);
+        break;
+    case WVCR:
+        s->volatile_cfg = s->data[0];
+        break;
+    case WEVCR:
+        s->enh_volatile_cfg = s->data[0];
+        break;
     default:
         break;
     }
@@ -477,6 +499,42 @@ static void reset_memory(Flash *s)
     s->write_enable = false;
     s->reset_enable = false;
 
+    if (((s->pi->jedec >> 16) & 0xFF) == JEDEC_NUMONYX) {
+        s->volatile_cfg = 0;
+        /* WRAP & reserved*/
+        s->volatile_cfg |= 0x3;
+        /* XIP */
+        if (extract32(s->nonvolatile_cfg, 9, 3) != 0x7) {
+            s->volatile_cfg |= (1 << 3);
+        }
+        /* Number of dummy cycles */
+        s->volatile_cfg |= deposit32(s->volatile_cfg,
+                            4, 4, extract32(s->nonvolatile_cfg, 12, 4));
+        s->enh_volatile_cfg = 0;
+        /* Output driver strength */
+        s->enh_volatile_cfg |= 0x7;
+        /* Vpp accelerator */
+        s->enh_volatile_cfg |= (1 << 3);
+        /* Reset/hold & reserved */
+        s->enh_volatile_cfg |= (1 << 4);
+        /* Dual I/O protocol */
+        if ((s->nonvolatile_cfg >> 1) & 0x1) {
+            s->enh_volatile_cfg |= (1 << 6);
+        }
+        /* Quad I/O protocol */
+        if ((s->nonvolatile_cfg >> 3) & 0x1) {
+            s->enh_volatile_cfg |= (1 << 7);
+        }
+
+        if (!(s->nonvolatile_cfg & 0x1)) {
+            s->four_bytes_address_mode = true;
+        }
+
+        if (!((s->nonvolatile_cfg >> 1) & 0x1)) {
+            s->ear = 0x3;
+        }
+    }
+
     DB_PRINT_L(0, "Reset done.\n");
 }
 
@@ -617,6 +675,49 @@ static void decode_new_cmd(Flash *s, uint32_t value)
             s->state = STATE_COLLECTING_DATA;
         }
         break;
+    case RNVCR:
+        s->data[0] = s->nonvolatile_cfg & 0xFF;
+        s->data[1] = (s->nonvolatile_cfg >> 8) & 0xFF;
+        s->pos = 0;
+        s->len = 2;
+        s->state = STATE_READING_DATA;
+        break;
+    case WNVCR:
+        if (s->write_enable) {
+            s->needed_bytes = 2;
+            s->pos = 0;
+            s->len = 0;
+            s->state = STATE_COLLECTING_DATA;
+        }
+        break;
+    case RVCR:
+        s->data[0] = s->volatile_cfg & 0xFF;
+        s->pos = 0;
+        s->len = 1;
+        s->state = STATE_READING_DATA;
+        break;
+    case WVCR:
+        if (s->write_enable) {
+            s->needed_bytes = 1;
+            s->pos = 0;
+            s->len = 0;
+            s->state = STATE_COLLECTING_DATA;
+        }
+        break;
+    case REVCR:
+        s->data[0] = s->enh_volatile_cfg & 0xFF;
+        s->pos = 0;
+        s->len = 1;
+        s->state = STATE_READING_DATA;
+        break;
+    case WEVCR:
+        if (s->write_enable) {
+            s->needed_bytes = 1;
+            s->pos = 0;
+            s->len = 0;
+            s->state = STATE_COLLECTING_DATA;
+        }
+        break;
     case RESET_ENABLE:
         s->reset_enable = true;
         break;
@@ -738,6 +839,11 @@ static void m25p80_pre_save(void *opaque)
     flash_sync_dirty((Flash *)opaque, -1);
 }
 
+static Property m25p80_properties[] = {
+    DEFINE_PROP_UINT32("nonvolatile-cfg", Flash, nonvolatile_cfg, 0x8FFF),
+    DEFINE_PROP_END_OF_LIST(),
+};
+
 static const VMStateDescription vmstate_m25p80 = {
     .name = "xilinx_spi",
     .version_id = 2,
@@ -755,6 +861,9 @@ static const VMStateDescription vmstate_m25p80 = {
         VMSTATE_BOOL(four_bytes_address_mode, Flash),
         VMSTATE_UINT8(ear, Flash),
         VMSTATE_BOOL(reset_enable, Flash),
+        VMSTATE_UINT32(nonvolatile_cfg, Flash),
+        VMSTATE_UINT32(volatile_cfg, Flash),
+        VMSTATE_UINT32(enh_volatile_cfg, Flash),
         VMSTATE_END_OF_LIST()
     }
 };
@@ -770,6 +879,7 @@ static void m25p80_class_init(ObjectClass *klass, void *data)
     k->set_cs = m25p80_cs;
     k->cs_polarity = SSI_CS_LOW;
     dc->vmsd = &vmstate_m25p80;
+    dc->props = m25p80_properties;
     mc->pi = data;
 }
 
-- 
2.5.0

[Qemu-devel] [PATCH v4 07/11] block: m25p80: Dummy cycles for N25Q256/512

[marcin.krzeminski]

From: Marcin Krzeminski <marcin.krzeminski@nokia.com>

This patch handles dummy cycles.

Signed-off-by: Marcin Krzeminski <marcin.krzeminski@nokia.com>
---
 hw/block/m25p80.c | 8 ++++++++
 1 file changed, 8 insertions(+)

diff --git a/hw/block/m25p80.c b/hw/block/m25p80.c
index 9d5a071..aff28f3 100644
--- a/hw/block/m25p80.c
+++ b/hw/block/m25p80.c
@@ -566,6 +566,10 @@ static void decode_new_cmd(Flash *s, uint32_t value)
     case DOR:
     case QOR:
         s->needed_bytes = get_addr_length(s);
+        if (((s->pi->jedec >> 16) & 0xFF) == JEDEC_NUMONYX) {
+            /* 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;
@@ -579,6 +583,8 @@ static void decode_new_cmd(Flash *s, uint32_t value)
             break;
         case JEDEC_NUMONYX:
             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);
             break;
         default:
             s->needed_bytes = 5;
@@ -596,6 +602,8 @@ static void decode_new_cmd(Flash *s, uint32_t value)
             break;
         case JEDEC_NUMONYX:
             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);
             break;
         default:
             s->needed_bytes = 8;
-- 
2.5.0

[Qemu-devel] [PATCH v4 08/11] block: m25p80: Fast read and 4bytes commands

[marcin.krzeminski]

From: Marcin Krzeminski <marcin.krzeminski@nokia.com>

Adds fast read and 4bytes commands family.
This work is based on Pawel Lenkow patch from v1.

Signed-off-by: Marcin Krzeminski <marcin.krzeminski@nokia.com>
---
 hw/block/m25p80.c | 48 +++++++++++++++++++++++++++++++++++++++++++++---
 1 file changed, 45 insertions(+), 3 deletions(-)

diff --git a/hw/block/m25p80.c b/hw/block/m25p80.c
index aff28f3..4acc79a 100644
--- a/hw/block/m25p80.c
+++ b/hw/block/m25p80.c
@@ -224,19 +224,28 @@ typedef enum {
     BULK_ERASE = 0xc7,
 
     READ = 0x3,
-    FAST_READ = 0xb,
+    READ4 = 0x13,
+    FAST_READ = 0x0b,
+    FAST_READ4 = 0x0c,
     DOR = 0x3b,
+    DOR4 = 0x3c,
     QOR = 0x6b,
+    QOR4 = 0x6c,
     DIOR = 0xbb,
+    DIOR4 = 0xbc,
     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,
@@ -359,6 +368,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;
@@ -367,6 +377,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:
@@ -425,7 +436,20 @@ void flash_write8(Flash *s, uint64_t addr, uint8_t data)
 
 static inline int get_addr_length(Flash *s)
 {
-    return s->four_bytes_address_mode ? 4 : 3;
+   switch (s->cmd_in_progress) {
+   case PP4:
+   case READ4:
+   case QIOR4:
+   case ERASE4_4K:
+   case ERASE4_SECTOR:
+   case FAST_READ4:
+   case DOR4:
+   case QOR4:
+   case DIOR4:
+       return 4;
+   default:
+       return s->four_bytes_address_mode ? 4 : 3;
+   }
 }
 
 static void complete_collecting_data(Flash *s)
@@ -449,19 +473,28 @@ 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 FAST_READ4:
     case DOR:
+    case DOR4:
     case QOR:
+    case QOR4:
     case DIOR:
+    case DIOR4:
     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:
@@ -550,12 +583,16 @@ static void decode_new_cmd(Flash *s, uint32_t 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:
+    case PP4:
         s->needed_bytes = get_addr_length(s);
         s->pos = 0;
         s->len = 0;
@@ -563,11 +600,14 @@ static void decode_new_cmd(Flash *s, uint32_t value)
         break;
 
     case FAST_READ:
+    case FAST_READ4:
     case DOR:
+    case DOR4:
     case QOR:
+    case QOR4:
         s->needed_bytes = get_addr_length(s);
         if (((s->pi->jedec >> 16) & 0xFF) == JEDEC_NUMONYX) {
-            /* Dummy cycles modeled with bytes writes instead of bits */
+            /* Dummy cycles - modeled with bytes writes instead of bits */
             s->needed_bytes += extract32(s->volatile_cfg, 4, 4);
         }
         s->pos = 0;
@@ -576,6 +616,7 @@ static void decode_new_cmd(Flash *s, uint32_t value)
         break;
 
     case DIOR:
+    case DIOR4:
         switch ((s->pi->jedec >> 16) & 0xFF) {
         case JEDEC_WINBOND:
         case JEDEC_SPANSION:
@@ -595,6 +636,7 @@ static void decode_new_cmd(Flash *s, uint32_t value)
         break;
 
     case QIOR:
+    case QIOR4:
         switch ((s->pi->jedec >> 16) & 0xFF) {
         case JEDEC_WINBOND:
         case JEDEC_SPANSION:
-- 
2.5.0

[Qemu-devel] [PATCH v4 09/11] block: m25p80: Implemented FSR register

[marcin.krzeminski]

From: Marcin Krzeminski <marcin.krzeminski@nokia.com>

Implements FSR register, it is used for busy waits.

Signed-off-by: Marcin Krzeminski <marcin.krzeminski@nokia.com>
---
 hw/block/m25p80.c | 11 +++++++++++
 1 file changed, 11 insertions(+)

diff --git a/hw/block/m25p80.c b/hw/block/m25p80.c
index 4acc79a..bc0dadb 100644
--- a/hw/block/m25p80.c
+++ b/hw/block/m25p80.c
@@ -222,6 +222,7 @@ typedef enum {
     WREN = 0x6,
     JEDEC_READ = 0x9f,
     BULK_ERASE = 0xc7,
+    READ_FSR = 0x70,
 
     READ = 0x3,
     READ4 = 0x13,
@@ -678,6 +679,16 @@ static void decode_new_cmd(Flash *s, uint32_t value)
         s->state = STATE_READING_DATA;
         break;
 
+    case READ_FSR:
+        s->data[0] = (1 << 7); /*Indicates flash is ready */
+        if (s->four_bytes_address_mode) {
+            s->data[0] |= 0x1;
+        }
+        s->pos = 0;
+        s->len = 1;
+        s->state = STATE_READING_DATA;
+        break;
+
     case JEDEC_READ:
         DB_PRINT_L(0, "populated jedec code\n");
         s->data[0] = (s->pi->jedec >> 16) & 0xff;
-- 
2.5.0

[Qemu-devel] [PATCH v4 10/11] block: m25p80: n25q256a/n25q512a models

[marcin.krzeminski]

From: Marcin Krzeminski <marcin.krzeminski@nokia.com>

Signed-off-by: Marcin Krzeminski <marcin.krzeminski@nokia.com>
Reviewed-by: Peter Crosthwaite <crosthwaite.peter@gmail.com>
---
 hw/block/m25p80.c | 3 ++-
 1 file changed, 2 insertions(+), 1 deletion(-)

diff --git a/hw/block/m25p80.c b/hw/block/m25p80.c
index bc0dadb..2b7d19f 100644
--- a/hw/block/m25p80.c
+++ b/hw/block/m25p80.c
@@ -210,8 +210,9 @@ static const FlashPartInfo known_devices[] = {
     { INFO("w25q80bl",    0xef4014,      0,  64 << 10,  16, ER_4K) },
     { INFO("w25q256",     0xef4019,      0,  64 << 10, 512, ER_4K) },
 
-    /* Numonyx -- n25q128 */
     { INFO("n25q128",      0x20ba18,      0,  64 << 10, 256, 0) },
+    { INFO("n25q256a",     0x20ba19,      0,  64 << 10, 512, ER_4K) },
+    { INFO("n25q512a",     0x20ba20,      0,  64 << 10, 1024, ER_4K) },
 };
 
 typedef enum {
-- 
2.5.0

[Qemu-devel] [PATCH v4 11/11] block: m25p80: at25128a/at25256a models

[marcin.krzeminski]

From: Marcin Krzeminski <marcin.krzeminski@nokia.com>

Signed-off-by: Marcin Krzeminski <marcin.krzeminski@nokia.com>
---
 hw/block/m25p80.c | 11 +++++++++++
 1 file changed, 11 insertions(+)

diff --git a/hw/block/m25p80.c b/hw/block/m25p80.c
index 2b7d19f..987fe07 100644
--- a/hw/block/m25p80.c
+++ b/hw/block/m25p80.c
@@ -99,6 +99,12 @@ static const FlashPartInfo known_devices[] = {
 
     { INFO("at45db081d",  0x1f2500,      0,  64 << 10,  16, ER_4K) },
 
+    /* Atmel EEPROMS - it is assumed, that don't care bit in command
+     * is set to 0. Block protection is not supported.
+     */
+    { INFO("at25128a-nonjedec", 0x0,     0,         1, 131072, WR_1) },
+    { INFO("at25256a-nonjedec", 0x0,     0,         1, 262144, WR_1) },
+
     /* EON -- en25xxx */
     { INFO("en25f32",     0x1c3116,      0,  64 << 10,  64, ER_4K) },
     { INFO("en25p32",     0x1c2016,      0,  64 << 10,  64, 0) },
@@ -438,6 +444,11 @@ void flash_write8(Flash *s, uint64_t addr, uint8_t data)
 
 static inline int get_addr_length(Flash *s)
 {
+   /* check if eeprom is in use */
+    if (s->pi->flags == WR_1) {
+        return 2;
+    }
+
    switch (s->cmd_in_progress) {
    case PP4:
    case READ4:
-- 
2.5.0

Re: [Qemu-devel] [PATCH v4 06/11] block: m25p80: Add configuration registers

[Peter Crosthwaite]

On Mon, Feb 22, 2016 at 12:03 AM,  <marcin.krzeminski@nokia.com> wrote:
> From: Marcin Krzeminski <marcin.krzeminski@nokia.com>
>
> This patch adds both volatile and non volatile configuration registers
> and commands to allow modify them. It is needed for proper handling
> dummy cycles. Initialization of those registers and flash state
> has been included as well.
> Some of this registers are used by kernel.
>
> Signed-off-by: Marcin Krzeminski <marcin.krzeminski@nokia.com>
> ---
>  hw/block/m25p80.c | 110 ++++++++++++++++++++++++++++++++++++++++++++++++++++++
>  1 file changed, 110 insertions(+)
>
> diff --git a/hw/block/m25p80.c b/hw/block/m25p80.c
> index 0698e7b..9d5a071 100644
> --- a/hw/block/m25p80.c
> +++ b/hw/block/m25p80.c
> @@ -26,6 +26,7 @@
>  #include "sysemu/block-backend.h"
>  #include "sysemu/blockdev.h"
>  #include "hw/ssi/ssi.h"
> +#include "qemu/bitops.h"
>
>  #ifndef M25P80_ERR_DEBUG
>  #define M25P80_ERR_DEBUG 0
> @@ -245,6 +246,15 @@ typedef enum {
>
>      RESET_ENABLE = 0x66,
>      RESET_MEMORY = 0x99,
> +
> +    RNVCR = 0xB5,
> +    WNVCR = 0xB1,
> +
> +    RVCR = 0x85,
> +    WVCR = 0x81,
> +
> +    REVCR = 0x65,
> +    WEVCR = 0x61,
>  } FlashCMD;
>
>  typedef enum {
> @@ -271,6 +281,9 @@ typedef struct Flash {
>      uint8_t needed_bytes;
>      uint8_t cmd_in_progress;
>      uint64_t cur_addr;
> +    uint32_t nonvolatile_cfg;
> +    uint32_t volatile_cfg;
> +    uint32_t enh_volatile_cfg;
>      bool write_enable;
>      bool four_bytes_address_mode;
>      bool reset_enable;
> @@ -459,6 +472,15 @@ static void complete_collecting_data(Flash *s)
>      case EXTEND_ADDR_WRITE:
>          s->ear = s->data[0];
>          break;
> +    case WNVCR:
> +        s->nonvolatile_cfg = s->data[0] | (s->data[1] << 8);
> +        break;
> +    case WVCR:
> +        s->volatile_cfg = s->data[0];
> +        break;
> +    case WEVCR:
> +        s->enh_volatile_cfg = s->data[0];
> +        break;
>      default:
>          break;
>      }
> @@ -477,6 +499,42 @@ static void reset_memory(Flash *s)
>      s->write_enable = false;
>      s->reset_enable = false;
>
> +    if (((s->pi->jedec >> 16) & 0xFF) == JEDEC_NUMONYX) {
> +        s->volatile_cfg = 0;
> +        /* WRAP & reserved*/

These bitfield masks and their values need some macrofication. Then
the comments explaining what bits are what can be dropped.

> +        s->volatile_cfg |= 0x3;

Why you set a reserved bit?

> +        /* XIP */
> +        if (extract32(s->nonvolatile_cfg, 9, 3) != 0x7) {

An example,
#define NVCFG_XIP_MODE_DISABLED 0x7

> +            s->volatile_cfg |= (1 << 3);
> +        }
> +        /* Number of dummy cycles */
> +        s->volatile_cfg |= deposit32(s->volatile_cfg,
> +                            4, 4, extract32(s->nonvolatile_cfg, 12, 4));
> +        s->enh_volatile_cfg = 0;
> +        /* Output driver strength */
> +        s->enh_volatile_cfg |= 0x7;
> +        /* Vpp accelerator */
> +        s->enh_volatile_cfg |= (1 << 3);

#define EVCFG_VPP_ACCELERATOR (1 << 3) ...

I am aware of my unreliability to get the review timely but the patch
intent looks good,

so with the macroification changes (globally to this function):

Acked-by: Peter Crosthwaite <crosthwaite.peter@gmail.com>

Regards,
Peter

> +        /* Reset/hold & reserved */
> +        s->enh_volatile_cfg |= (1 << 4);
> +        /* Dual I/O protocol */
> +        if ((s->nonvolatile_cfg >> 1) & 0x1) {
> +            s->enh_volatile_cfg |= (1 << 6);
> +        }
> +        /* Quad I/O protocol */
> +        if ((s->nonvolatile_cfg >> 3) & 0x1) {
> +            s->enh_volatile_cfg |= (1 << 7);
> +        }
> +
> +        if (!(s->nonvolatile_cfg & 0x1)) {
> +            s->four_bytes_address_mode = true;
> +        }
> +
> +        if (!((s->nonvolatile_cfg >> 1) & 0x1)) {
> +            s->ear = 0x3;
> +        }
> +    }
> +
>      DB_PRINT_L(0, "Reset done.\n");
>  }
>
> @@ -617,6 +675,49 @@ static void decode_new_cmd(Flash *s, uint32_t value)
>              s->state = STATE_COLLECTING_DATA;
>          }
>          break;
> +    case RNVCR:
> +        s->data[0] = s->nonvolatile_cfg & 0xFF;
> +        s->data[1] = (s->nonvolatile_cfg >> 8) & 0xFF;
> +        s->pos = 0;
> +        s->len = 2;
> +        s->state = STATE_READING_DATA;
> +        break;
> +    case WNVCR:
> +        if (s->write_enable) {
> +            s->needed_bytes = 2;
> +            s->pos = 0;
> +            s->len = 0;
> +            s->state = STATE_COLLECTING_DATA;
> +        }
> +        break;
> +    case RVCR:
> +        s->data[0] = s->volatile_cfg & 0xFF;
> +        s->pos = 0;
> +        s->len = 1;
> +        s->state = STATE_READING_DATA;
> +        break;
> +    case WVCR:
> +        if (s->write_enable) {
> +            s->needed_bytes = 1;
> +            s->pos = 0;
> +            s->len = 0;
> +            s->state = STATE_COLLECTING_DATA;
> +        }
> +        break;
> +    case REVCR:
> +        s->data[0] = s->enh_volatile_cfg & 0xFF;
> +        s->pos = 0;
> +        s->len = 1;
> +        s->state = STATE_READING_DATA;
> +        break;
> +    case WEVCR:
> +        if (s->write_enable) {
> +            s->needed_bytes = 1;
> +            s->pos = 0;
> +            s->len = 0;
> +            s->state = STATE_COLLECTING_DATA;
> +        }
> +        break;
>      case RESET_ENABLE:
>          s->reset_enable = true;
>          break;
> @@ -738,6 +839,11 @@ static void m25p80_pre_save(void *opaque)
>      flash_sync_dirty((Flash *)opaque, -1);
>  }
>
> +static Property m25p80_properties[] = {
> +    DEFINE_PROP_UINT32("nonvolatile-cfg", Flash, nonvolatile_cfg, 0x8FFF),
> +    DEFINE_PROP_END_OF_LIST(),
> +};
> +
>  static const VMStateDescription vmstate_m25p80 = {
>      .name = "xilinx_spi",
>      .version_id = 2,
> @@ -755,6 +861,9 @@ static const VMStateDescription vmstate_m25p80 = {
>          VMSTATE_BOOL(four_bytes_address_mode, Flash),
>          VMSTATE_UINT8(ear, Flash),
>          VMSTATE_BOOL(reset_enable, Flash),
> +        VMSTATE_UINT32(nonvolatile_cfg, Flash),
> +        VMSTATE_UINT32(volatile_cfg, Flash),
> +        VMSTATE_UINT32(enh_volatile_cfg, Flash),
>          VMSTATE_END_OF_LIST()
>      }
>  };
> @@ -770,6 +879,7 @@ static void m25p80_class_init(ObjectClass *klass, void *data)
>      k->set_cs = m25p80_cs;
>      k->cs_polarity = SSI_CS_LOW;
>      dc->vmsd = &vmstate_m25p80;
> +    dc->props = m25p80_properties;
>      mc->pi = data;
>  }
>
> --
> 2.5.0
>

Re: [Qemu-devel] [PATCH v4 05/11] block: m25p80: 4byte address mode

[Peter Crosthwaite]

On Mon, Feb 22, 2016 at 12:03 AM,  <marcin.krzeminski@nokia.com> wrote:
> From: Marcin Krzeminski <marcin.krzeminski@nokia.com>
>
> This patch adds only 4byte address mode (does not cover dummy cycles).
> This mode is needed to access more than 16 MiB of flash.
>
> Signed-off-by: Marcin Krzeminski <marcin.krzeminski@nokia.com>
> ---
>  hw/block/m25p80.c | 41 +++++++++++++++++++++++++++++++++++------
>  1 file changed, 35 insertions(+), 6 deletions(-)
>
> diff --git a/hw/block/m25p80.c b/hw/block/m25p80.c
> index 0540dde..0698e7b 100644
> --- a/hw/block/m25p80.c
> +++ b/hw/block/m25p80.c
> @@ -237,6 +237,9 @@ typedef enum {
>      ERASE_32K = 0x52,
>      ERASE_SECTOR = 0xd8,
>
> +    EN_4BYTE_ADDR = 0xB7,
> +    EX_4BYTE_ADDR = 0xE9,
> +
>      EXTEND_ADDR_READ = 0xC8,
>      EXTEND_ADDR_WRITE = 0xC5,
>
> @@ -269,6 +272,7 @@ typedef struct Flash {
>      uint8_t cmd_in_progress;
>      uint64_t cur_addr;
>      bool write_enable;
> +    bool four_bytes_address_mode;
>      bool reset_enable;
>      uint8_t ear;
>
> @@ -406,12 +410,25 @@ void flash_write8(Flash *s, uint64_t addr, uint8_t data)
>      s->dirty_page = page;
>  }
>
> +static inline int get_addr_length(Flash *s)
> +{
> +    return s->four_bytes_address_mode ? 4 : 3;
> +}
> +
>  static void complete_collecting_data(Flash *s)
>  {
> -    s->cur_addr = s->data[0] << 16;
> -    s->cur_addr |= s->data[1] << 8;
> -    s->cur_addr |= s->data[2];
> -    s->cur_addr += (s->ear & 0x3) * MAX_3BYTES_SIZE;
> +    int i;
> +
> +    s->cur_addr = 0;
> +
> +    for (i = 0; i < get_addr_length(s); ++i) {
> +        s->cur_addr <<= 8;
> +        s->cur_addr |= s->data[i];
> +    }
> +
> +    if (get_addr_length(s) == 3) {
> +        s->cur_addr += (s->ear & 0x3) * MAX_3BYTES_SIZE;
> +    }
>
>      s->state = STATE_IDLE;
>
> @@ -452,6 +469,7 @@ static void reset_memory(Flash *s)
>      s->cmd_in_progress = NOP;
>      s->cur_addr = 0;
>      s->ear = 0;
> +    s->four_bytes_address_mode = false;
>      s->len = 0;
>      s->needed_bytes = 0;
>      s->pos = 0;
> @@ -480,7 +498,7 @@ static void decode_new_cmd(Flash *s, uint32_t value)
>      case DPP:
>      case QPP:
>      case PP:
> -        s->needed_bytes = 3;
> +        s->needed_bytes = get_addr_length(s);
>          s->pos = 0;
>          s->len = 0;
>          s->state = STATE_COLLECTING_DATA;
> @@ -489,7 +507,7 @@ static void decode_new_cmd(Flash *s, uint32_t value)
>      case FAST_READ:
>      case DOR:
>      case QOR:
> -        s->needed_bytes = 4;
> +        s->needed_bytes = get_addr_length(s);

You fix this later with the configuration of dummy cycles, but you
should preserve the existing behaviour until your fix lands. This
means that you should have +1 here.

>          s->pos = 0;
>          s->len = 0;
>          s->state = STATE_COLLECTING_DATA;
> @@ -502,6 +520,8 @@ static void decode_new_cmd(Flash *s, uint32_t value)
>              s->needed_bytes = 4;
>              break;
>          case JEDEC_NUMONYX:
> +            s->needed_bytes = get_addr_length(s);
> +            break;

This change ...

>          default:
>              s->needed_bytes = 5;

Should be here, with a +2 (I think?).

>          }
> @@ -517,6 +537,8 @@ static void decode_new_cmd(Flash *s, uint32_t value)
>              s->needed_bytes = 6;
>              break;
>          case JEDEC_NUMONYX:
> +            s->needed_bytes = get_addr_length(s);
> +            break;

Similar.

Otherwise,

Reviewed-by: Peter Crosthwaite <crosthwaite.peter@gmail.com>

Regards,
Peter

>          default:
>              s->needed_bytes = 8;
>          }
> @@ -575,6 +597,12 @@ static void decode_new_cmd(Flash *s, uint32_t value)
>          break;
>      case NOP:
>          break;
> +    case EN_4BYTE_ADDR:
> +        s->four_bytes_address_mode = true;
> +        break;
> +    case EX_4BYTE_ADDR:
> +        s->four_bytes_address_mode = false;
> +        break;
>      case EXTEND_ADDR_READ:
>          s->data[0] = s->ear;
>          s->pos = 0;
> @@ -724,6 +752,7 @@ static const VMStateDescription vmstate_m25p80 = {
>          VMSTATE_UINT8(cmd_in_progress, Flash),
>          VMSTATE_UINT64(cur_addr, Flash),
>          VMSTATE_BOOL(write_enable, Flash),
> +        VMSTATE_BOOL(four_bytes_address_mode, Flash),
>          VMSTATE_UINT8(ear, Flash),
>          VMSTATE_BOOL(reset_enable, Flash),
>          VMSTATE_END_OF_LIST()
> --
> 2.5.0
>

Re: [Qemu-devel] [PATCH v4 07/11] block: m25p80: Dummy cycles for N25Q256/512

[Peter Crosthwaite]

On Mon, Feb 22, 2016 at 12:03 AM,  <marcin.krzeminski@nokia.com> wrote:
> From: Marcin Krzeminski <marcin.krzeminski@nokia.com>
>
> This patch handles dummy cycles.
>

More commit message needed:

"Use the setting in the volatile cfg register to correctly set the
number of dummy bytes"

> Signed-off-by: Marcin Krzeminski <marcin.krzeminski@nokia.com>
> ---
>  hw/block/m25p80.c | 8 ++++++++
>  1 file changed, 8 insertions(+)
>
> diff --git a/hw/block/m25p80.c b/hw/block/m25p80.c
> index 9d5a071..aff28f3 100644
> --- a/hw/block/m25p80.c
> +++ b/hw/block/m25p80.c
> @@ -566,6 +566,10 @@ static void decode_new_cmd(Flash *s, uint32_t value)
>      case DOR:
>      case QOR:
>          s->needed_bytes = get_addr_length(s);
> +        if (((s->pi->jedec >> 16) & 0xFF) == JEDEC_NUMONYX) {
> +            /* 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;
> @@ -579,6 +583,8 @@ static void decode_new_cmd(Flash *s, uint32_t value)
>              break;
>          case JEDEC_NUMONYX:
>              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);
>              break;
>          default:
>              s->needed_bytes = 5;

Following on from before, these defaults are NUMONYX policy based, so
I think your patch is to the default.

Otherwise: Reviewed-by: Peter Crosthwaite <crosthwaite.peter@gmail.com>

Regards,
Peter

> @@ -596,6 +602,8 @@ static void decode_new_cmd(Flash *s, uint32_t value)
>              break;
>          case JEDEC_NUMONYX:
>              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);
>              break;
>          default:
>              s->needed_bytes = 8;
> --
> 2.5.0
>

Re: [Qemu-devel] [PATCH v4 08/11] block: m25p80: Fast read and 4bytes commands

[Peter Crosthwaite]

On Mon, Feb 22, 2016 at 12:03 AM,  <marcin.krzeminski@nokia.com> wrote:
> From: Marcin Krzeminski <marcin.krzeminski@nokia.com>
>
> Adds fast read and 4bytes commands family.
> This work is based on Pawel Lenkow patch from v1.
>
> Signed-off-by: Marcin Krzeminski <marcin.krzeminski@nokia.com>
> ---
>  hw/block/m25p80.c | 48 +++++++++++++++++++++++++++++++++++++++++++++---
>  1 file changed, 45 insertions(+), 3 deletions(-)
>
> diff --git a/hw/block/m25p80.c b/hw/block/m25p80.c
> index aff28f3..4acc79a 100644
> --- a/hw/block/m25p80.c
> +++ b/hw/block/m25p80.c
> @@ -224,19 +224,28 @@ typedef enum {
>      BULK_ERASE = 0xc7,
>
>      READ = 0x3,

You should fix READ as "0x03"

> -    FAST_READ = 0xb,
> +    READ4 = 0x13,
> +    FAST_READ = 0x0b,

To match this context change

> +    FAST_READ4 = 0x0c,
>      DOR = 0x3b,
> +    DOR4 = 0x3c,
>      QOR = 0x6b,
> +    QOR4 = 0x6c,
>      DIOR = 0xbb,
> +    DIOR4 = 0xbc,
>      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,
> @@ -359,6 +368,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;
> @@ -367,6 +377,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:
> @@ -425,7 +436,20 @@ void flash_write8(Flash *s, uint64_t addr, uint8_t data)
>
>  static inline int get_addr_length(Flash *s)
>  {
> -    return s->four_bytes_address_mode ? 4 : 3;
> +   switch (s->cmd_in_progress) {
> +   case PP4:
> +   case READ4:
> +   case QIOR4:
> +   case ERASE4_4K:
> +   case ERASE4_SECTOR:
> +   case FAST_READ4:
> +   case DOR4:
> +   case QOR4:
> +   case DIOR4:
> +       return 4;
> +   default:
> +       return s->four_bytes_address_mode ? 4 : 3;
> +   }
>  }
>
>  static void complete_collecting_data(Flash *s)
> @@ -449,19 +473,28 @@ 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 FAST_READ4:
>      case DOR:
> +    case DOR4:
>      case QOR:
> +    case QOR4:
>      case DIOR:
> +    case DIOR4:
>      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:
> @@ -550,12 +583,16 @@ static void decode_new_cmd(Flash *s, uint32_t 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:
> +    case PP4:
>          s->needed_bytes = get_addr_length(s);
>          s->pos = 0;
>          s->len = 0;
> @@ -563,11 +600,14 @@ static void decode_new_cmd(Flash *s, uint32_t value)
>          break;
>
>      case FAST_READ:
> +    case FAST_READ4:
>      case DOR:
> +    case DOR4:
>      case QOR:
> +    case QOR4:
>          s->needed_bytes = get_addr_length(s);
>          if (((s->pi->jedec >> 16) & 0xFF) == JEDEC_NUMONYX) {
> -            /* Dummy cycles modeled with bytes writes instead of bits */
> +            /* Dummy cycles - modeled with bytes writes instead of bits */

White change shouldn't be here.

Otherwise:

Reviewed-by: Peter Crosthwaite <crosthwaite.peter@gmail.com>

>              s->needed_bytes += extract32(s->volatile_cfg, 4, 4);
>          }
>          s->pos = 0;
> @@ -576,6 +616,7 @@ static void decode_new_cmd(Flash *s, uint32_t value)
>          break;
>
>      case DIOR:
> +    case DIOR4:
>          switch ((s->pi->jedec >> 16) & 0xFF) {
>          case JEDEC_WINBOND:
>          case JEDEC_SPANSION:
> @@ -595,6 +636,7 @@ static void decode_new_cmd(Flash *s, uint32_t value)
>          break;
>
>      case QIOR:
> +    case QIOR4:
>          switch ((s->pi->jedec >> 16) & 0xFF) {
>          case JEDEC_WINBOND:
>          case JEDEC_SPANSION:
> --
> 2.5.0
>

Re: [Qemu-devel] [PATCH v4 09/11] block: m25p80: Implemented FSR register

[Peter Crosthwaite]

On Mon, Feb 22, 2016 at 12:03 AM,  <marcin.krzeminski@nokia.com> wrote:
> From: Marcin Krzeminski <marcin.krzeminski@nokia.com>
>
> Implements FSR register, it is used for busy waits.
>
> Signed-off-by: Marcin Krzeminski <marcin.krzeminski@nokia.com>
> ---
>  hw/block/m25p80.c | 11 +++++++++++
>  1 file changed, 11 insertions(+)
>
> diff --git a/hw/block/m25p80.c b/hw/block/m25p80.c
> index 4acc79a..bc0dadb 100644
> --- a/hw/block/m25p80.c
> +++ b/hw/block/m25p80.c
> @@ -222,6 +222,7 @@ typedef enum {
>      WREN = 0x6,
>      JEDEC_READ = 0x9f,
>      BULK_ERASE = 0xc7,
> +    READ_FSR = 0x70,
>
>      READ = 0x3,
>      READ4 = 0x13,
> @@ -678,6 +679,16 @@ static void decode_new_cmd(Flash *s, uint32_t value)
>          s->state = STATE_READING_DATA;
>          break;
>
> +    case READ_FSR:
> +        s->data[0] = (1 << 7); /*Indicates flash is ready */
> +        if (s->four_bytes_address_mode) {
> +            s->data[0] |= 0x1;
> +        }
> +        s->pos = 0;
> +        s->len = 1;
> +        s->state = STATE_READING_DATA;
> +        break;
> +

To be consistent with recommendation on CFG register macros it should
be done here too.

Otherwise:

Reviewed-by: Peter Crosthwaite <crosthwaite.peter@gmail.com>

>      case JEDEC_READ:
>          DB_PRINT_L(0, "populated jedec code\n");
>          s->data[0] = (s->pi->jedec >> 16) & 0xff;
> --
> 2.5.0
>

Re: [Qemu-devel] [PATCH v4 11/11] block: m25p80: at25128a/at25256a models

[Peter Crosthwaite]

On Mon, Feb 22, 2016 at 12:03 AM,  <marcin.krzeminski@nokia.com> wrote:
> From: Marcin Krzeminski <marcin.krzeminski@nokia.com>
>
> Signed-off-by: Marcin Krzeminski <marcin.krzeminski@nokia.com>
> ---
>  hw/block/m25p80.c | 11 +++++++++++
>  1 file changed, 11 insertions(+)
>
> diff --git a/hw/block/m25p80.c b/hw/block/m25p80.c
> index 2b7d19f..987fe07 100644
> --- a/hw/block/m25p80.c
> +++ b/hw/block/m25p80.c
> @@ -99,6 +99,12 @@ static const FlashPartInfo known_devices[] = {
>
>      { INFO("at45db081d",  0x1f2500,      0,  64 << 10,  16, ER_4K) },
>
> +    /* Atmel EEPROMS - it is assumed, that don't care bit in command
> +     * is set to 0. Block protection is not supported.
> +     */
> +    { INFO("at25128a-nonjedec", 0x0,     0,         1, 131072, WR_1) },
> +    { INFO("at25256a-nonjedec", 0x0,     0,         1, 262144, WR_1) },
> +
>      /* EON -- en25xxx */
>      { INFO("en25f32",     0x1c3116,      0,  64 << 10,  64, ER_4K) },
>      { INFO("en25p32",     0x1c2016,      0,  64 << 10,  64, 0) },
> @@ -438,6 +444,11 @@ void flash_write8(Flash *s, uint64_t addr, uint8_t data)
>
>  static inline int get_addr_length(Flash *s)
>  {
> +   /* check if eeprom is in use */
> +    if (s->pi->flags == WR_1) {
> +        return 2;
> +    }
> +

Neat!

But I think this indicates the flag is incorrectly named. Should be
renamed to EEPROM or something like.

Otherwise:

Reviewed-by: Peter Crosthwaite <crosthwaite.peter@gmail.com>

>     switch (s->cmd_in_progress) {
>     case PP4:
>     case READ4:
> --
> 2.5.0
>

Re: [Qemu-devel] [PATCH v4 02/11] block: m25p80: RESET_ENABLE and RESET_MEMORY commnads

[Peter Crosthwaite]

"commands" in commit msg

On Mon, Feb 22, 2016 at 12:03 AM,  <marcin.krzeminski@nokia.com> wrote:
> From: Marcin Krzeminski <marcin.krzeminski@nokia.com>
>
> Signed-off-by: Marcin Krzeminski <marcin.krzeminski@nokia.com>
> ---
>  hw/block/m25p80.c | 35 ++++++++++++++++++++++++++++++++++-
>  1 file changed, 34 insertions(+), 1 deletion(-)
>
> diff --git a/hw/block/m25p80.c b/hw/block/m25p80.c
> index 2222124..06b0af3 100644
> --- a/hw/block/m25p80.c
> +++ b/hw/block/m25p80.c
> @@ -233,6 +233,9 @@ typedef enum {
>      ERASE_4K = 0x20,
>      ERASE_32K = 0x52,
>      ERASE_SECTOR = 0xd8,
> +
> +    RESET_ENABLE = 0x66,
> +    RESET_MEMORY = 0x99,
>  } FlashCMD;
>
>  typedef enum {
> @@ -260,6 +263,7 @@ typedef struct Flash {
>      uint8_t cmd_in_progress;
>      uint64_t cur_addr;
>      bool write_enable;
> +    bool reset_enable;
>
>      int64_t dirty_page;
>
> @@ -432,11 +436,29 @@ static void complete_collecting_data(Flash *s)
>      }
>  }
>
> +static void reset_memory(Flash *s)
> +{
> +    s->cmd_in_progress = NOP;
> +    s->cur_addr = 0;
> +    s->len = 0;
> +    s->needed_bytes = 0;
> +    s->pos = 0;
> +    s->state = STATE_IDLE;
> +    s->write_enable = false;
> +    s->reset_enable = false;
> +
> +    DB_PRINT_L(0, "Reset done.\n");
> +}
> +
>  static void decode_new_cmd(Flash *s, uint32_t value)
>  {
>      s->cmd_in_progress = value;
>      DB_PRINT_L(0, "decoded new command:%x\n", value);
>
> +    if (value != RESET_MEMORY) {
> +        s->reset_enable = false;
> +    }
> +
>      switch (value) {
>
>      case ERASE_4K:
> @@ -541,6 +563,14 @@ static void decode_new_cmd(Flash *s, uint32_t value)
>          break;
>      case NOP:
>          break;
> +    case RESET_ENABLE:
> +        s->reset_enable = true;
> +        break;
> +    case RESET_MEMORY:
> +        if (s->reset_enable) {
> +            reset_memory(s);
> +        }
> +        break;
>      default:
>          qemu_log_mask(LOG_GUEST_ERROR, "M25P80: Unknown cmd %x\n", value);
>          break;
> @@ -622,6 +652,8 @@ static int m25p80_init(SSISlave *ss)
>      s->size = s->pi->sector_size * s->pi->n_sectors;
>      s->dirty_page = -1;
>
> +    reset_memory(s);
> +

This shouldn't be here, you need to add a Device::reset function. Your
use case of persisting data through a warn system reset (that we
discussed previously) is difficult to support with QEMUs current reset
semantics and your board is out-of-tree. So I think this should be in
the Device::reset and we need to revisit your unique use case along
with the addition of your board.

So with the move to device::reset,

Reviewed-by: Peter Crosthwaite <crosthwaite.peter@gmail.com>

>      /* FIXME use a qdev drive property instead of drive_get_next() */
>      dinfo = drive_get_next(IF_MTD);
>
> @@ -654,7 +686,7 @@ static void m25p80_pre_save(void *opaque)
>
>  static const VMStateDescription vmstate_m25p80 = {
>      .name = "xilinx_spi",
> -    .version_id = 1,
> +    .version_id = 2,
>      .minimum_version_id = 1,
>      .pre_save = m25p80_pre_save,
>      .fields = (VMStateField[]) {
> @@ -666,6 +698,7 @@ static const VMStateDescription vmstate_m25p80 = {
>          VMSTATE_UINT8(cmd_in_progress, Flash),
>          VMSTATE_UINT64(cur_addr, Flash),
>          VMSTATE_BOOL(write_enable, Flash),
> +        VMSTATE_BOOL(reset_enable, Flash),
>          VMSTATE_END_OF_LIST()
>      }
>  };
> --
> 2.5.0
>

[Qemu-devel] ODP: [PATCH v4 06/11] block: m25p80: Add configuration registers

[Krzeminski, Marcin (Nokia - PL/Wroclaw)]

W dniu 17.03.2016 o 18:24, Peter Crosthwaite pisze:
> On Mon, Feb 22, 2016 at 12:03 AM,  <marcin.krzeminski@nokia.com> wrote:
>> From: Marcin Krzeminski <marcin.krzeminski@nokia.com>
>>
>> This patch adds both volatile and non volatile configuration registers
>> and commands to allow modify them. It is needed for proper handling
>> dummy cycles. Initialization of those registers and flash state
>> has been included as well.
>> Some of this registers are used by kernel.
>>
>> Signed-off-by: Marcin Krzeminski <marcin.krzeminski@nokia.com>
>> ---
>>  hw/block/m25p80.c | 110 ++++++++++++++++++++++++++++++++++++++++++++++++++++++
>>  1 file changed, 110 insertions(+)
>>
>> diff --git a/hw/block/m25p80.c b/hw/block/m25p80.c
>> index 0698e7b..9d5a071 100644
>> --- a/hw/block/m25p80.c
>> +++ b/hw/block/m25p80.c
>> @@ -26,6 +26,7 @@
>>  #include "sysemu/block-backend.h"
>>  #include "sysemu/blockdev.h"
>>  #include "hw/ssi/ssi.h"
>> +#include "qemu/bitops.h"
>>
>>  #ifndef M25P80_ERR_DEBUG
>>  #define M25P80_ERR_DEBUG 0
>> @@ -245,6 +246,15 @@ typedef enum {
>>
>>      RESET_ENABLE = 0x66,
>>      RESET_MEMORY = 0x99,
>> +
>> +    RNVCR = 0xB5,
>> +    WNVCR = 0xB1,
>> +
>> +    RVCR = 0x85,
>> +    WVCR = 0x81,
>> +
>> +    REVCR = 0x65,
>> +    WEVCR = 0x61,
>>  } FlashCMD;
>>
>>  typedef enum {
>> @@ -271,6 +281,9 @@ typedef struct Flash {
>>      uint8_t needed_bytes;
>>      uint8_t cmd_in_progress;
>>      uint64_t cur_addr;
>> +    uint32_t nonvolatile_cfg;
>> +    uint32_t volatile_cfg;
>> +    uint32_t enh_volatile_cfg;
>>      bool write_enable;
>>      bool four_bytes_address_mode;
>>      bool reset_enable;
>> @@ -459,6 +472,15 @@ static void complete_collecting_data(Flash *s)
>>      case EXTEND_ADDR_WRITE:
>>          s->ear = s->data[0];
>>          break;
>> +    case WNVCR:
>> +        s->nonvolatile_cfg = s->data[0] | (s->data[1] << 8);
>> +        break;
>> +    case WVCR:
>> +        s->volatile_cfg = s->data[0];
>> +        break;
>> +    case WEVCR:
>> +        s->enh_volatile_cfg = s->data[0];
>> +        break;
>>      default:
>>          break;
>>      }
>> @@ -477,6 +499,42 @@ static void reset_memory(Flash *s)
>>      s->write_enable = false;
>>      s->reset_enable = false;
>>
>> +    if (((s->pi->jedec >> 16) & 0xFF) == JEDEC_NUMONYX) {
>> +        s->volatile_cfg = 0;
>> +        /* WRAP & reserved*/
>
> These bitfield masks and their values need some macrofication. Then
> the comments explaining what bits are what can be dropped.
>
>> +        s->volatile_cfg |= 0x3;
>
> Why you set a reserved bit?
>From datasheet: The device ships from
the factory with all bits erased to 1 (FFFFh).
>
>
>> +        /* XIP */
>> +        if (extract32(s->nonvolatile_cfg, 9, 3) != 0x7) {
>
> An example,
> #define NVCFG_XIP_MODE_DISABLED 0x7
>
>> +            s->volatile_cfg |= (1 << 3);
>> +        }
>> +        /* Number of dummy cycles */
>> +        s->volatile_cfg |= deposit32(s->volatile_cfg,
>> +                            4, 4, extract32(s->nonvolatile_cfg, 12, 4));
>> +        s->enh_volatile_cfg = 0;
>> +        /* Output driver strength */
>> +        s->enh_volatile_cfg |= 0x7;
>> +        /* Vpp accelerator */
>> +        s->enh_volatile_cfg |= (1 << 3);
>
> #define EVCFG_VPP_ACCELERATOR (1 << 3) ...
Macros here is a good idea. I will add them in v5.

Thanks,
Marcin
>
>
> I am aware of my unreliability to get the review timely but the patch
> intent looks good,
>
> so with the macroification changes (globally to this function):
>
> Acked-by: Peter Crosthwaite <crosthwaite.peter@gmail.com>
>
> Regards,
> Peter

[Qemu-devel] ODP: [PATCH v4 11/11] block: m25p80: at25128a/at25256a models

[Krzeminski, Marcin (Nokia - PL/Wroclaw)]

W dniu 17.03.2016 o 18:39, Peter Crosthwaite pisze:
> On Mon, Feb 22, 2016 at 12:03 AM,  <marcin.krzeminski@nokia.com> wrote:
>> From: Marcin Krzeminski <marcin.krzeminski@nokia.com>
>>
>> Signed-off-by: Marcin Krzeminski <marcin.krzeminski@nokia.com>
>> ---
>>  hw/block/m25p80.c | 11 +++++++++++
>>  1 file changed, 11 insertions(+)
>>
>> diff --git a/hw/block/m25p80.c b/hw/block/m25p80.c
>> index 2b7d19f..987fe07 100644
>> --- a/hw/block/m25p80.c
>> +++ b/hw/block/m25p80.c
>> @@ -99,6 +99,12 @@ static const FlashPartInfo known_devices[] = {
>>
>>      { INFO("at45db081d",  0x1f2500,      0,  64 << 10,  16, ER_4K) },
>>
>> +    /* Atmel EEPROMS - it is assumed, that don't care bit in command
>> +     * is set to 0. Block protection is not supported.
>> +     */
>> +    { INFO("at25128a-nonjedec", 0x0,     0,         1, 131072, WR_1) },
>> +    { INFO("at25256a-nonjedec", 0x0,     0,         1, 262144, WR_1) },
>> +
>>      /* EON -- en25xxx */
>>      { INFO("en25f32",     0x1c3116,      0,  64 << 10,  64, ER_4K) },
>>      { INFO("en25p32",     0x1c2016,      0,  64 << 10,  64, 0) },
>> @@ -438,6 +444,11 @@ void flash_write8(Flash *s, uint64_t addr, uint8_t data)
>>
>>  static inline int get_addr_length(Flash *s)
>>  {
>> +   /* check if eeprom is in use */
>> +    if (s->pi->flags == WR_1) {
>> +        return 2;
>> +    }
>> +
>
> Neat!
>
> But I think this indicates the flag is incorrectly named. Should be
> renamed to EEPROM or something like.
Yes, EEPROM sound much better. Will be changed in v5.

Thanks,
Marcin
>
>
> Otherwise:
>
> Reviewed-by: Peter Crosthwaite <crosthwaite.peter@gmail.com>
>
>>     switch (s->cmd_in_progress) {
>>     case PP4:
>>     case READ4:
>> --
>> 2.5.0
>>
>
>

[Qemu-devel] ODP: [PATCH v4 02/11] block: m25p80: RESET_ENABLE and RESET_MEMORY commnads

[Krzeminski, Marcin (Nokia - PL/Wroclaw)]

W dniu 17.03.2016 o 18:42, Peter Crosthwaite pisze:
> "commands" in commit msg
>
> On Mon, Feb 22, 2016 at 12:03 AM,  <marcin.krzeminski@nokia.com> wrote:
>> From: Marcin Krzeminski <marcin.krzeminski@nokia.com>
>>
>> Signed-off-by: Marcin Krzeminski <marcin.krzeminski@nokia.com>
>> ---
>>  hw/block/m25p80.c | 35 ++++++++++++++++++++++++++++++++++-
>>  1 file changed, 34 insertions(+), 1 deletion(-)
>>
>> diff --git a/hw/block/m25p80.c b/hw/block/m25p80.c
>> index 2222124..06b0af3 100644
>> --- a/hw/block/m25p80.c
>> +++ b/hw/block/m25p80.c
>> @@ -233,6 +233,9 @@ typedef enum {
>>      ERASE_4K = 0x20,
>>      ERASE_32K = 0x52,
>>      ERASE_SECTOR = 0xd8,
>> +
>> +    RESET_ENABLE = 0x66,
>> +    RESET_MEMORY = 0x99,
>>  } FlashCMD;
>>
>>  typedef enum {
>> @@ -260,6 +263,7 @@ typedef struct Flash {
>>      uint8_t cmd_in_progress;
>>      uint64_t cur_addr;
>>      bool write_enable;
>> +    bool reset_enable;
>>
>>      int64_t dirty_page;
>>
>> @@ -432,11 +436,29 @@ static void complete_collecting_data(Flash *s)
>>      }
>>  }
>>
>> +static void reset_memory(Flash *s)
>> +{
>> +    s->cmd_in_progress = NOP;
>> +    s->cur_addr = 0;
>> +    s->len = 0;
>> +    s->needed_bytes = 0;
>> +    s->pos = 0;
>> +    s->state = STATE_IDLE;
>> +    s->write_enable = false;
>> +    s->reset_enable = false;
>> +
>> +    DB_PRINT_L(0, "Reset done.\n");
>> +}
>> +
>>  static void decode_new_cmd(Flash *s, uint32_t value)
>>  {
>>      s->cmd_in_progress = value;
>>      DB_PRINT_L(0, "decoded new command:%x\n", value);
>>
>> +    if (value != RESET_MEMORY) {
>> +        s->reset_enable = false;
>> +    }
>> +
>>      switch (value) {
>>
>>      case ERASE_4K:
>> @@ -541,6 +563,14 @@ static void decode_new_cmd(Flash *s, uint32_t value)
>>          break;
>>      case NOP:
>>          break;
>> +    case RESET_ENABLE:
>> +        s->reset_enable = true;
>> +        break;
>> +    case RESET_MEMORY:
>> +        if (s->reset_enable) {
>> +            reset_memory(s);
>> +        }
>> +        break;
>>      default:
>>          qemu_log_mask(LOG_GUEST_ERROR, "M25P80: Unknown cmd %x\n", value);
>>          break;
>> @@ -622,6 +652,8 @@ static int m25p80_init(SSISlave *ss)
>>      s->size = s->pi->sector_size * s->pi->n_sectors;
>>      s->dirty_page = -1;
>>
>> +    reset_memory(s);
>> +
>
> This shouldn't be here, you need to add a Device::reset function. Your
> use case of persisting data through a warn system reset (that we
> discussed previously) is difficult to support with QEMUs current reset
> semantics and your board is out-of-tree. So I think this should be in
> the Device::reset and we need to revisit your unique use case along
> with the addition of your board.
My impression was, that I should remove all "warm reset" additions I made
(including dc::reset) and move reset_memory call to init that is why it is here.
I will move it to dc:reset in v5.

Thanks,
Marcin
>
>
> So with the move to device::reset,
>
> Reviewed-by: Peter Crosthwaite <crosthwaite.peter@gmail.com>
>
>>      /* FIXME use a qdev drive property instead of drive_get_next() */
>>      dinfo = drive_get_next(IF_MTD);
>>
>> @@ -654,7 +686,7 @@ static void m25p80_pre_save(void *opaque)
>>
>>  static const VMStateDescription vmstate_m25p80 = {
>>      .name = "xilinx_spi",
>> -    .version_id = 1,
>> +    .version_id = 2,
>>      .minimum_version_id = 1,
>>      .pre_save = m25p80_pre_save,
>>      .fields = (VMStateField[]) {
>> @@ -666,6 +698,7 @@ static const VMStateDescription vmstate_m25p80 = {
>>          VMSTATE_UINT8(cmd_in_progress, Flash),
>>          VMSTATE_UINT64(cur_addr, Flash),
>>          VMSTATE_BOOL(write_enable, Flash),
>> +        VMSTATE_BOOL(reset_enable, Flash),
>>          VMSTATE_END_OF_LIST()
>>      }
>>  };
>> --
>> 2.5.0
>>
>
>

Re: [Qemu-devel] [PATCH v4 05/11] block: m25p80: 4byte address mode

[Krzeminski, Marcin (Nokia - PL/Wroclaw)]

> -----Original Message-----
> From: EXT Peter Crosthwaite [mailto:crosthwaitepeter@gmail.com]
> Sent: Thursday, March 17, 2016 6:27 PM
> To: Krzeminski, Marcin (Nokia - PL/Wroclaw)
> <marcin.krzeminski@nokia.com>
> Cc: qemu-devel@nongnu.org Developers <qemu-devel@nongnu.org>;
> Cédric Le Goater <clg@fr.ibm.com>; pawel.lenkow@itlen.com
> Subject: Re: [PATCH v4 05/11] block: m25p80: 4byte address mode
> 
> On Mon, Feb 22, 2016 at 12:03 AM,  <marcin.krzeminski@nokia.com> wrote:
> > From: Marcin Krzeminski <marcin.krzeminski@nokia.com>
> >
> > This patch adds only 4byte address mode (does not cover dummy cycles).
> > This mode is needed to access more than 16 MiB of flash.
> >
> > Signed-off-by: Marcin Krzeminski <marcin.krzeminski@nokia.com>
> > ---
> >  hw/block/m25p80.c | 41 +++++++++++++++++++++++++++++++++++-----
> -
> >  1 file changed, 35 insertions(+), 6 deletions(-)
> >
> > diff --git a/hw/block/m25p80.c b/hw/block/m25p80.c index
> > 0540dde..0698e7b 100644
> > --- a/hw/block/m25p80.c
> > +++ b/hw/block/m25p80.c
> > @@ -237,6 +237,9 @@ typedef enum {
> >      ERASE_32K = 0x52,
> >      ERASE_SECTOR = 0xd8,
> >
> > +    EN_4BYTE_ADDR = 0xB7,
> > +    EX_4BYTE_ADDR = 0xE9,
> > +
> >      EXTEND_ADDR_READ = 0xC8,
> >      EXTEND_ADDR_WRITE = 0xC5,
> >
> > @@ -269,6 +272,7 @@ typedef struct Flash {
> >      uint8_t cmd_in_progress;
> >      uint64_t cur_addr;
> >      bool write_enable;
> > +    bool four_bytes_address_mode;
> >      bool reset_enable;
> >      uint8_t ear;
> >
> > @@ -406,12 +410,25 @@ void flash_write8(Flash *s, uint64_t addr, uint8_t
> data)
> >      s->dirty_page = page;
> >  }
> >
> > +static inline int get_addr_length(Flash *s) {
> > +    return s->four_bytes_address_mode ? 4 : 3; }
> > +
> >  static void complete_collecting_data(Flash *s)  {
> > -    s->cur_addr = s->data[0] << 16;
> > -    s->cur_addr |= s->data[1] << 8;
> > -    s->cur_addr |= s->data[2];
> > -    s->cur_addr += (s->ear & 0x3) * MAX_3BYTES_SIZE;
> > +    int i;
> > +
> > +    s->cur_addr = 0;
> > +
> > +    for (i = 0; i < get_addr_length(s); ++i) {
> > +        s->cur_addr <<= 8;
> > +        s->cur_addr |= s->data[i];
> > +    }
> > +
> > +    if (get_addr_length(s) == 3) {
> > +        s->cur_addr += (s->ear & 0x3) * MAX_3BYTES_SIZE;
> > +    }
> >
> >      s->state = STATE_IDLE;
> >
> > @@ -452,6 +469,7 @@ static void reset_memory(Flash *s)
> >      s->cmd_in_progress = NOP;
> >      s->cur_addr = 0;
> >      s->ear = 0;
> > +    s->four_bytes_address_mode = false;
> >      s->len = 0;
> >      s->needed_bytes = 0;
> >      s->pos = 0;
> > @@ -480,7 +498,7 @@ static void decode_new_cmd(Flash *s, uint32_t
> value)
> >      case DPP:
> >      case QPP:
> >      case PP:
> > -        s->needed_bytes = 3;
> > +        s->needed_bytes = get_addr_length(s);
> >          s->pos = 0;
> >          s->len = 0;
> >          s->state = STATE_COLLECTING_DATA; @@ -489,7 +507,7 @@ static
> > void decode_new_cmd(Flash *s, uint32_t value)
> >      case FAST_READ:
> >      case DOR:
> >      case QOR:
> > -        s->needed_bytes = 4;
> > +        s->needed_bytes = get_addr_length(s);
> 
> You fix this later with the configuration of dummy cycles, but you should
> preserve the existing behaviour until your fix lands. This means that you
> should have +1 here.
True, all is that because there is a 11 patches for one file. From logical point of view
this make sense, but from device emulation point of view applying not whole
series it does not.
> 
> >          s->pos = 0;
> >          s->len = 0;
> >          s->state = STATE_COLLECTING_DATA; @@ -502,6 +520,8 @@ static
> > void decode_new_cmd(Flash *s, uint32_t value)
> >              s->needed_bytes = 4;
> >              break;
> >          case JEDEC_NUMONYX:
> > +            s->needed_bytes = get_addr_length(s);
> > +            break;
> 
> This change ...
> 
> >          default:
> >              s->needed_bytes = 5;
> 
> Should be here, with a +2 (I think?).
Yes, but I would not prefer this in default, but since there is not much time and it
does not change functionality I will put all in default, but change it with Macronix
and Spansion future patch series (for those I reworked a bit all that switches so it is
already changed there).

Thanks,
Marcin
> 
> >          }
> > @@ -517,6 +537,8 @@ static void decode_new_cmd(Flash *s, uint32_t
> value)
> >              s->needed_bytes = 6;
> >              break;
> >          case JEDEC_NUMONYX:
> > +            s->needed_bytes = get_addr_length(s);
> > +            break;
> 
> Similar.
> 
> Otherwise,
> 
> Reviewed-by: Peter Crosthwaite <crosthwaite.peter@gmail.com>
> 
> Regards,
> Peter
> 
> >          default:
> >              s->needed_bytes = 8;
> >          }
> > @@ -575,6 +597,12 @@ static void decode_new_cmd(Flash *s, uint32_t
> value)
> >          break;
> >      case NOP:
> >          break;
> > +    case EN_4BYTE_ADDR:
> > +        s->four_bytes_address_mode = true;
> > +        break;
> > +    case EX_4BYTE_ADDR:
> > +        s->four_bytes_address_mode = false;
> > +        break;
> >      case EXTEND_ADDR_READ:
> >          s->data[0] = s->ear;
> >          s->pos = 0;
> > @@ -724,6 +752,7 @@ static const VMStateDescription vmstate_m25p80 =
> {
> >          VMSTATE_UINT8(cmd_in_progress, Flash),
> >          VMSTATE_UINT64(cur_addr, Flash),
> >          VMSTATE_BOOL(write_enable, Flash),
> > +        VMSTATE_BOOL(four_bytes_address_mode, Flash),
> >          VMSTATE_UINT8(ear, Flash),
> >          VMSTATE_BOOL(reset_enable, Flash),
> >          VMSTATE_END_OF_LIST()
> > --
> > 2.5.0
> >