qemu-devel.nongnu.org archive mirror
 help / color / mirror / Atom feed
* [Qemu-devel] [PATCH 0/9] m25p80: Add new 512Mbit and 1Gbit devices.
@ 2016-06-15 13:41 marcin.krzeminski
  2016-06-15 13:41 ` [Qemu-devel] [PATCH 1/9] m25p80: Replace JEDEC ID masking with function marcin.krzeminski
                   ` (8 more replies)
  0 siblings, 9 replies; 28+ messages in thread
From: marcin.krzeminski @ 2016-06-15 13:41 UTC (permalink / raw)
  To: qemu-devel
  Cc: crosthwaitepeter, pawel.lenkow, rfsw-patches, peter.maydell, clg

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

This series adds support for new flash devices.

Marcin Krzeminski (9):
  m25p80: Replace JEDEC ID masking with function.
  m25p80: Make a table for JEDEC ID.
  m25p80: Allow more than four banks.
  m25p80: Introduce COLLECTING_VAR_LEN_DATA state.
  m25p80: Add additional flash commands:
  m25p80: Introduce quad and equad modes.
  m25p80: Introduce configuration registers.
  m25p80: Fast read commands family changes.
  m25p80: New flash devices.

 hw/block/m25p80.c | 335 ++++++++++++++++++++++++++++++++++++++++++++++--------
 1 file changed, 286 insertions(+), 49 deletions(-)

-- 
2.7.4

^ permalink raw reply	[flat|nested] 28+ messages in thread

* [Qemu-devel] [PATCH 1/9] m25p80: Replace JEDEC ID masking with function.
  2016-06-15 13:41 [Qemu-devel] [PATCH 0/9] m25p80: Add new 512Mbit and 1Gbit devices marcin.krzeminski
@ 2016-06-15 13:41 ` marcin.krzeminski
  2016-06-15 14:05   ` Cédric Le Goater
  2016-06-15 13:41 ` [Qemu-devel] [PATCH 2/9] m25p80: Make a table for JEDEC ID marcin.krzeminski
                   ` (7 subsequent siblings)
  8 siblings, 1 reply; 28+ messages in thread
From: marcin.krzeminski @ 2016-06-15 13:41 UTC (permalink / raw)
  To: qemu-devel
  Cc: crosthwaitepeter, pawel.lenkow, rfsw-patches, peter.maydell, clg

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

Instead of always reading and comparing jededc ID,
replace it by function.

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

diff --git a/hw/block/m25p80.c b/hw/block/m25p80.c
index 4c856f5..15765f5 100644
--- a/hw/block/m25p80.c
+++ b/hw/block/m25p80.c
@@ -307,6 +307,14 @@ typedef enum {
     STATE_READING_DATA,
 } CMDState;
 
+typedef enum {
+    MAN_SPANSION,
+    MAN_MACRONIX,
+    MAN_NUMONYX,
+    MAN_WINBOND,
+    MAN_GENERIC,
+} Manufacturer;
+
 typedef struct Flash {
     SSISlave parent_obj;
 
@@ -350,6 +358,22 @@ typedef struct M25P80Class {
 #define M25P80_GET_CLASS(obj) \
      OBJECT_GET_CLASS(M25P80Class, (obj), TYPE_M25P80)
 
+static inline Manufacturer get_man(Flash *s)
+{
+    switch (((s->pi->jedec >> 16) & 0xFF)) {
+    case 0x20:
+        return MAN_NUMONYX;
+    case 0xEF:
+        return MAN_WINBOND;
+    case 0x01:
+        return MAN_SPANSION;
+    case 0xC2:
+        return MAN_MACRONIX;
+    default:
+        return MAN_GENERIC;
+    }
+}
+
 static void blk_sync_complete(void *opaque, int ret)
 {
     /* do nothing. Masters do not directly interact with the backing store,
@@ -562,7 +586,8 @@ static void reset_memory(Flash *s)
     s->write_enable = false;
     s->reset_enable = false;
 
-    if (((s->pi->jedec >> 16) & 0xFF) == JEDEC_NUMONYX) {
+    switch (get_man(s)) {
+    case MAN_NUMONYX:
         s->volatile_cfg = 0;
         s->volatile_cfg |= VCFG_DUMMY;
         s->volatile_cfg |= VCFG_WRAP_SEQUENTIAL;
@@ -594,6 +619,9 @@ static void reset_memory(Flash *s)
         if (!(s->nonvolatile_cfg & NVCFG_LOWER_SEGMENT_MASK)) {
             s->ear = CFG_UPPER_128MB_SEG_ENABLED;
         }
+        break;
+    default:
+        break;
     }
 
     DB_PRINT_L(0, "Reset done.\n");
@@ -634,9 +662,12 @@ static void decode_new_cmd(Flash *s, uint32_t value)
     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 */
+        switch (get_man(s)) {
+        case MAN_NUMONYX:
             s->needed_bytes += extract32(s->volatile_cfg, 4, 4);
+            break;
+        default:
+            break;
         }
         s->pos = 0;
         s->len = 0;
@@ -645,9 +676,9 @@ static void decode_new_cmd(Flash *s, uint32_t value)
 
     case DIOR:
     case DIOR4:
-        switch ((s->pi->jedec >> 16) & 0xFF) {
-        case JEDEC_WINBOND:
-        case JEDEC_SPANSION:
+        switch (get_man(s)) {
+        case MAN_WINBOND:
+        case MAN_SPANSION:
             s->needed_bytes = 4;
             break;
         default:
@@ -662,9 +693,9 @@ static void decode_new_cmd(Flash *s, uint32_t value)
 
     case QIOR:
     case QIOR4:
-        switch ((s->pi->jedec >> 16) & 0xFF) {
-        case JEDEC_WINBOND:
-        case JEDEC_SPANSION:
+        switch (get_man(s)) {
+        case MAN_WINBOND:
+        case MAN_SPANSION:
             s->needed_bytes = 6;
             break;
         default:
-- 
2.7.4

^ permalink raw reply related	[flat|nested] 28+ messages in thread

* [Qemu-devel] [PATCH 2/9] m25p80: Make a table for JEDEC ID.
  2016-06-15 13:41 [Qemu-devel] [PATCH 0/9] m25p80: Add new 512Mbit and 1Gbit devices marcin.krzeminski
  2016-06-15 13:41 ` [Qemu-devel] [PATCH 1/9] m25p80: Replace JEDEC ID masking with function marcin.krzeminski
@ 2016-06-15 13:41 ` marcin.krzeminski
  2016-06-15 14:17   ` Cédric Le Goater
  2016-06-15 13:41 ` [Qemu-devel] [PATCH 3/9] m25p80: Allow more than four banks marcin.krzeminski
                   ` (6 subsequent siblings)
  8 siblings, 1 reply; 28+ messages in thread
From: marcin.krzeminski @ 2016-06-15 13:41 UTC (permalink / raw)
  To: qemu-devel
  Cc: crosthwaitepeter, pawel.lenkow, rfsw-patches, peter.maydell, clg

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

Since it is now longer than 4. This work based on Pawel Lenkow
changes and the kernel SPI framework.

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

diff --git a/hw/block/m25p80.c b/hw/block/m25p80.c
index 15765f5..342f7c9 100644
--- a/hw/block/m25p80.c
+++ b/hw/block/m25p80.c
@@ -53,12 +53,17 @@
 /* 16 MiB max in 3 byte address mode */
 #define MAX_3BYTES_SIZE 0x1000000
 
+#define SPI_NOR_MAX_ID_LEN 6
+
 typedef struct FlashPartInfo {
     const char *part_name;
-    /* jedec code. (jedec >> 16) & 0xff is the 1st byte, >> 8 the 2nd etc */
-    uint32_t jedec;
-    /* extended jedec code */
-    uint16_t ext_jedec;
+    /*
+     * This array stores the ID bytes.
+     * The first three bytes are the JEDIC ID.
+     * JEDEC ID zero means "no ID" (mostly older chips).
+     */
+    uint8_t id[SPI_NOR_MAX_ID_LEN];
+    uint8_t id_len;
     /* there is confusion between manufacturers as to what a sector is. In this
      * device model, a "sector" is the size that is erased by the ERASE_SECTOR
      * command (opcode 0xd8).
@@ -70,11 +75,33 @@ typedef struct FlashPartInfo {
 } FlashPartInfo;
 
 /* adapted from linux */
-
-#define INFO(_part_name, _jedec, _ext_jedec, _sector_size, _n_sectors, _flags)\
-    .part_name = (_part_name),\
-    .jedec = (_jedec),\
-    .ext_jedec = (_ext_jedec),\
+/* Used when the "_ext_id" is two bytes at most */
+#define INFO(_part_name, _jedec_id, _ext_id, _sector_size, _n_sectors, _flags)\
+    .part_name = _part_name,\
+    .id = {\
+        ((_jedec_id) >> 16) & 0xff,\
+        ((_jedec_id) >> 8) & 0xff,\
+        (_jedec_id) & 0xff,\
+        ((_ext_id) >> 8) & 0xff,\
+        (_ext_id) & 0xff,\
+          },\
+    .id_len = (!(_jedec_id) ? 0 : (3 + ((_ext_id) ? 2 : 0))),\
+    .sector_size = (_sector_size),\
+    .n_sectors = (_n_sectors),\
+    .page_size = 256,\
+    .flags = (_flags),
+
+#define INFO6(_part_name, _jedec_id, _ext_id, _sector_size, _n_sectors, _flags)\
+    .part_name = _part_name,\
+    .id = {\
+        ((_jedec_id) >> 16) & 0xff,\
+        ((_jedec_id) >> 8) & 0xff,\
+        (_jedec_id) & 0xff,\
+        ((_ext_id) >> 16) & 0xff,\
+        ((_ext_id) >> 8) & 0xff,\
+        (_ext_id) & 0xff,\
+          },\
+    .id_len = 6,\
     .sector_size = (_sector_size),\
     .n_sectors = (_n_sectors),\
     .page_size = 256,\
@@ -360,7 +387,7 @@ typedef struct M25P80Class {
 
 static inline Manufacturer get_man(Flash *s)
 {
-    switch (((s->pi->jedec >> 16) & 0xFF)) {
+    switch (s->pi->id[0]) {
     case 0x20:
         return MAN_NUMONYX;
     case 0xEF:
@@ -630,6 +657,7 @@ static void reset_memory(Flash *s)
 static void decode_new_cmd(Flash *s, uint32_t value)
 {
     s->cmd_in_progress = value;
+    int i;
     DB_PRINT_L(0, "decoded new command:%x\n", value);
 
     if (value != RESET_MEMORY) {
@@ -743,16 +771,11 @@ static void decode_new_cmd(Flash *s, uint32_t value)
 
     case JEDEC_READ:
         DB_PRINT_L(0, "populated jedec code\n");
-        s->data[0] = (s->pi->jedec >> 16) & 0xff;
-        s->data[1] = (s->pi->jedec >> 8) & 0xff;
-        s->data[2] = s->pi->jedec & 0xff;
-        if (s->pi->ext_jedec) {
-            s->data[3] = (s->pi->ext_jedec >> 8) & 0xff;
-            s->data[4] = s->pi->ext_jedec & 0xff;
-            s->len = 5;
-        } else {
-            s->len = 3;
+        for (i = 0; i < s->pi->id_len; i++) {
+            s->data[i] = s->pi->id[i];
         }
+
+        s->len = s->pi->id_len;
         s->pos = 0;
         s->state = STATE_READING_DATA;
         break;
-- 
2.7.4

^ permalink raw reply related	[flat|nested] 28+ messages in thread

* [Qemu-devel] [PATCH 3/9] m25p80: Allow more than four banks.
  2016-06-15 13:41 [Qemu-devel] [PATCH 0/9] m25p80: Add new 512Mbit and 1Gbit devices marcin.krzeminski
  2016-06-15 13:41 ` [Qemu-devel] [PATCH 1/9] m25p80: Replace JEDEC ID masking with function marcin.krzeminski
  2016-06-15 13:41 ` [Qemu-devel] [PATCH 2/9] m25p80: Make a table for JEDEC ID marcin.krzeminski
@ 2016-06-15 13:41 ` marcin.krzeminski
  2016-06-16  7:09   ` Cédric Le Goater
  2016-06-15 13:41 ` [Qemu-devel] [PATCH 4/9] m25p80: Introduce COLLECTING_VAR_LEN_DATA state marcin.krzeminski
                   ` (5 subsequent siblings)
  8 siblings, 1 reply; 28+ messages in thread
From: marcin.krzeminski @ 2016-06-15 13:41 UTC (permalink / raw)
  To: qemu-devel
  Cc: crosthwaitepeter, pawel.lenkow, rfsw-patches, peter.maydell, clg

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

Allow to have more than four 16MiB regions for bigger flash devices.

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

diff --git a/hw/block/m25p80.c b/hw/block/m25p80.c
index 342f7c9..6910c52 100644
--- a/hw/block/m25p80.c
+++ b/hw/block/m25p80.c
@@ -129,7 +129,6 @@ typedef struct FlashPartInfo {
 #define EVCFG_QUAD_IO_ENABLED (1 << 7)
 #define NVCFG_4BYTE_ADDR_MASK (1 << 0)
 #define NVCFG_LOWER_SEGMENT_MASK (1 << 1)
-#define CFG_UPPER_128MB_SEG_ENABLED 0x3
 
 /* Numonyx (Micron) Flag Status Register macros */
 #define FSR_4BYTE_ADDR_MODE_ENABLED 0x1
@@ -545,7 +544,7 @@ static void complete_collecting_data(Flash *s)
     }
 
     if (get_addr_length(s) == 3) {
-        s->cur_addr += (s->ear & 0x3) * MAX_3BYTES_SIZE;
+        s->cur_addr += s->ear * MAX_3BYTES_SIZE;
     }
 
     s->state = STATE_IDLE;
@@ -644,7 +643,7 @@ static void reset_memory(Flash *s)
             s->four_bytes_address_mode = true;
         }
         if (!(s->nonvolatile_cfg & NVCFG_LOWER_SEGMENT_MASK)) {
-            s->ear = CFG_UPPER_128MB_SEG_ENABLED;
+            s->ear = s->size / MAX_3BYTES_SIZE - 1;
         }
         break;
     default:
-- 
2.7.4

^ permalink raw reply related	[flat|nested] 28+ messages in thread

* [Qemu-devel] [PATCH 4/9] m25p80: Introduce COLLECTING_VAR_LEN_DATA state.
  2016-06-15 13:41 [Qemu-devel] [PATCH 0/9] m25p80: Add new 512Mbit and 1Gbit devices marcin.krzeminski
                   ` (2 preceding siblings ...)
  2016-06-15 13:41 ` [Qemu-devel] [PATCH 3/9] m25p80: Allow more than four banks marcin.krzeminski
@ 2016-06-15 13:41 ` marcin.krzeminski
  2016-06-16  7:13   ` Cédric Le Goater
  2016-06-15 13:41 ` [Qemu-devel] [PATCH 5/9] m25p80: Add additional flash commands: marcin.krzeminski
                   ` (4 subsequent siblings)
  8 siblings, 1 reply; 28+ messages in thread
From: marcin.krzeminski @ 2016-06-15 13:41 UTC (permalink / raw)
  To: qemu-devel
  Cc: crosthwaitepeter, pawel.lenkow, rfsw-patches, peter.maydell, clg

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

Some flash allows to stop read at any time.
Allow framework to support this.

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

diff --git a/hw/block/m25p80.c b/hw/block/m25p80.c
index 6910c52..ca1f882 100644
--- a/hw/block/m25p80.c
+++ b/hw/block/m25p80.c
@@ -330,6 +330,7 @@ typedef enum {
     STATE_PAGE_PROGRAM,
     STATE_READ,
     STATE_COLLECTING_DATA,
+    STATE_COLLECTING_VAR_LEN_DATA,
     STATE_READING_DATA,
 } CMDState;
 
@@ -872,6 +873,9 @@ static int m25p80_cs(SSISlave *ss, bool select)
     Flash *s = M25P80(ss);
 
     if (select) {
+        if (s->state == STATE_COLLECTING_VAR_LEN_DATA) {
+            complete_collecting_data(s);
+        }
         s->len = 0;
         s->pos = 0;
         s->state = STATE_IDLE;
@@ -905,6 +909,7 @@ static uint32_t m25p80_transfer8(SSISlave *ss, uint32_t tx)
         break;
 
     case STATE_COLLECTING_DATA:
+    case STATE_COLLECTING_VAR_LEN_DATA:
         s->data[s->len] = (uint8_t)tx;
         s->len++;
 
-- 
2.7.4

^ permalink raw reply related	[flat|nested] 28+ messages in thread

* [Qemu-devel] [PATCH 5/9] m25p80: Add additional flash commands:
  2016-06-15 13:41 [Qemu-devel] [PATCH 0/9] m25p80: Add new 512Mbit and 1Gbit devices marcin.krzeminski
                   ` (3 preceding siblings ...)
  2016-06-15 13:41 ` [Qemu-devel] [PATCH 4/9] m25p80: Introduce COLLECTING_VAR_LEN_DATA state marcin.krzeminski
@ 2016-06-15 13:41 ` marcin.krzeminski
  2016-06-16  7:14   ` Cédric Le Goater
  2016-06-15 13:41 ` [Qemu-devel] [PATCH 6/9] m25p80: Introduce quad and equad modes marcin.krzeminski
                   ` (3 subsequent siblings)
  8 siblings, 1 reply; 28+ messages in thread
From: marcin.krzeminski @ 2016-06-15 13:41 UTC (permalink / raw)
  To: qemu-devel
  Cc: crosthwaitepeter, pawel.lenkow, rfsw-patches, peter.maydell, clg

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

Page program 4byte/quad and erase 32K sectors 4 bytes.

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

diff --git a/hw/block/m25p80.c b/hw/block/m25p80.c
index ca1f882..55b4377 100644
--- a/hw/block/m25p80.c
+++ b/hw/block/m25p80.c
@@ -297,12 +297,14 @@ typedef enum {
 
     PP = 0x02,
     PP4 = 0x12,
+    PP4_4 = 0x3e,
     DPP = 0xa2,
     QPP = 0x32,
 
     ERASE_4K = 0x20,
     ERASE4_4K = 0x21,
     ERASE_32K = 0x52,
+    ERASE4_32K = 0x5c,
     ERASE_SECTOR = 0xd8,
     ERASE4_SECTOR = 0xdc,
 
@@ -449,6 +451,7 @@ static void flash_erase(Flash *s, int offset, FlashCMD cmd)
         capa_to_assert = ER_4K;
         break;
     case ERASE_32K:
+    case ERASE4_32K:
         len = 32 << 10;
         capa_to_assert = ER_32K;
         break;
@@ -519,9 +522,11 @@ static inline int get_addr_length(Flash *s)
 
    switch (s->cmd_in_progress) {
    case PP4:
+   case PP4_4:
    case READ4:
    case QIOR4:
    case ERASE4_4K:
+   case ERASE4_32K:
    case ERASE4_SECTOR:
    case FAST_READ4:
    case DOR4:
@@ -555,6 +560,7 @@ static void complete_collecting_data(Flash *s)
     case QPP:
     case PP:
     case PP4:
+    case PP4_4:
         s->state = STATE_PAGE_PROGRAM;
         break;
     case READ:
@@ -574,6 +580,7 @@ static void complete_collecting_data(Flash *s)
     case ERASE_4K:
     case ERASE4_4K:
     case ERASE_32K:
+    case ERASE4_32K:
     case ERASE_SECTOR:
     case ERASE4_SECTOR:
         flash_erase(s, s->cur_addr, s->cmd_in_progress);
@@ -669,6 +676,7 @@ static void decode_new_cmd(Flash *s, uint32_t value)
     case ERASE_4K:
     case ERASE4_4K:
     case ERASE_32K:
+    case ERASE4_32K:
     case ERASE_SECTOR:
     case ERASE4_SECTOR:
     case READ:
@@ -677,6 +685,7 @@ static void decode_new_cmd(Flash *s, uint32_t value)
     case QPP:
     case PP:
     case PP4:
+    case PP4_4:
         s->needed_bytes = get_addr_length(s);
         s->pos = 0;
         s->len = 0;
-- 
2.7.4

^ permalink raw reply related	[flat|nested] 28+ messages in thread

* [Qemu-devel] [PATCH 6/9] m25p80: Introduce quad and equad modes.
  2016-06-15 13:41 [Qemu-devel] [PATCH 0/9] m25p80: Add new 512Mbit and 1Gbit devices marcin.krzeminski
                   ` (4 preceding siblings ...)
  2016-06-15 13:41 ` [Qemu-devel] [PATCH 5/9] m25p80: Add additional flash commands: marcin.krzeminski
@ 2016-06-15 13:41 ` marcin.krzeminski
  2016-06-15 14:25   ` Cédric Le Goater
  2016-06-15 13:41 ` [Qemu-devel] [PATCH 7/9] m25p80: Introduce configuration registers marcin.krzeminski
                   ` (2 subsequent siblings)
  8 siblings, 1 reply; 28+ messages in thread
From: marcin.krzeminski @ 2016-06-15 13:41 UTC (permalink / raw)
  To: qemu-devel
  Cc: crosthwaitepeter, pawel.lenkow, rfsw-patches, peter.maydell, clg

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

Quad and Equad modes for Spansion and Macronix flash devices.
This commit also includes modification and new command to manipulate
quad mode (status registers and dedicated commands).
This work is based on Pawel Lenkow work.

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

diff --git a/hw/block/m25p80.c b/hw/block/m25p80.c
index 55b4377..d1c4d46 100644
--- a/hw/block/m25p80.c
+++ b/hw/block/m25p80.c
@@ -281,6 +281,7 @@ typedef enum {
     JEDEC_READ = 0x9f,
     BULK_ERASE = 0xc7,
     READ_FSR = 0x70,
+    RDCR = 0x15,
 
     READ = 0x03,
     READ4 = 0x13,
@@ -317,6 +318,13 @@ typedef enum {
     RESET_ENABLE = 0x66,
     RESET_MEMORY = 0x99,
 
+    /*
+     * Micron: 0x35 - enable QPI
+     * Spansion: 0x35 - read control register
+     */
+    RDCR_EQIO = 0x35,
+    RSTQIO = 0xf5,
+
     RNVCR = 0xB5,
     WNVCR = 0xB1,
 
@@ -366,6 +374,7 @@ typedef struct Flash {
     bool write_enable;
     bool four_bytes_address_mode;
     bool reset_enable;
+    bool quad_enable;
     uint8_t ear;
 
     int64_t dirty_page;
@@ -586,6 +595,16 @@ static void complete_collecting_data(Flash *s)
         flash_erase(s, s->cur_addr, s->cmd_in_progress);
         break;
     case WRSR:
+        switch (get_man(s)) {
+        case MAN_SPANSION:
+            s->quad_enable = !!(s->data[1] & 0x02);
+            break;
+        case MAN_MACRONIX:
+            s->quad_enable = extract32(s->data[0], 6, 1);
+            break;
+        default:
+            break;
+        }
         if (s->write_enable) {
             s->write_enable = false;
         }
@@ -619,6 +638,7 @@ static void reset_memory(Flash *s)
     s->state = STATE_IDLE;
     s->write_enable = false;
     s->reset_enable = false;
+    s->quad_enable = false;
 
     switch (get_man(s)) {
     case MAN_NUMONYX:
@@ -747,10 +767,20 @@ static void decode_new_cmd(Flash *s, uint32_t value)
 
     case WRSR:
         if (s->write_enable) {
-            s->needed_bytes = 1;
+            switch (get_man(s)) {
+            case MAN_SPANSION:
+                s->needed_bytes = 2;
+                s->state = STATE_COLLECTING_DATA;
+                break;
+            case MAN_MACRONIX:
+                s->needed_bytes = 2;
+                s->state = STATE_COLLECTING_VAR_LEN_DATA;
+                break;
+            default:
+                s->needed_bytes = 1;
+                s->state = STATE_COLLECTING_DATA;
+            }
             s->pos = 0;
-            s->len = 0;
-            s->state = STATE_COLLECTING_DATA;
         }
         break;
 
@@ -763,6 +793,9 @@ static void decode_new_cmd(Flash *s, uint32_t value)
 
     case RDSR:
         s->data[0] = (!!s->write_enable) << 1;
+        if (get_man(s) == MAN_MACRONIX) {
+            s->data[0] |= (!!s->quad_enable) << 6;
+        }
         s->pos = 0;
         s->len = 1;
         s->state = STATE_READING_DATA;
@@ -789,6 +822,14 @@ static void decode_new_cmd(Flash *s, uint32_t value)
         s->state = STATE_READING_DATA;
         break;
 
+    case RDCR:
+        s->data[0] = s->volatile_cfg & 0xFF;
+        s->data[0] |= (!!s->four_bytes_address_mode) << 5;
+        s->pos = 0;
+        s->len = 1;
+        s->state = STATE_READING_DATA;
+        break;
+
     case BULK_ERASE:
         if (s->write_enable) {
             DB_PRINT_L(0, "chip erase\n");
@@ -828,7 +869,7 @@ static void decode_new_cmd(Flash *s, uint32_t value)
         s->state = STATE_READING_DATA;
         break;
     case WNVCR:
-        if (s->write_enable) {
+        if (s->write_enable && get_man(s) == MAN_NUMONYX) {
             s->needed_bytes = 2;
             s->pos = 0;
             s->len = 0;
@@ -871,6 +912,24 @@ static void decode_new_cmd(Flash *s, uint32_t value)
             reset_memory(s);
         }
         break;
+    case RDCR_EQIO:
+        switch (get_man(s)) {
+        case MAN_SPANSION:
+            s->data[0] = (!!s->quad_enable) << 1;
+            s->pos = 0;
+            s->len = 1;
+            s->state = STATE_READING_DATA;
+            break;
+        case MAN_MACRONIX:
+            s->quad_enable = true;
+            break;
+        default:
+            break;
+        }
+        break;
+    case RSTQIO:
+        s->quad_enable = false;
+        break;
     default:
         qemu_log_mask(LOG_GUEST_ERROR, "M25P80: Unknown cmd %x\n", value);
         break;
@@ -999,7 +1058,7 @@ static Property m25p80_properties[] = {
 
 static const VMStateDescription vmstate_m25p80 = {
     .name = "xilinx_spi",
-    .version_id = 2,
+    .version_id = 3,
     .minimum_version_id = 1,
     .pre_save = m25p80_pre_save,
     .fields = (VMStateField[]) {
@@ -1017,6 +1076,7 @@ static const VMStateDescription vmstate_m25p80 = {
         VMSTATE_UINT32_V(nonvolatile_cfg, Flash, 2),
         VMSTATE_UINT32_V(volatile_cfg, Flash, 2),
         VMSTATE_UINT32_V(enh_volatile_cfg, Flash, 2),
+        VMSTATE_BOOL_V(quad_enable, Flash, 3),
         VMSTATE_END_OF_LIST()
     }
 };
-- 
2.7.4

^ permalink raw reply related	[flat|nested] 28+ messages in thread

* [Qemu-devel] [PATCH 7/9] m25p80: Introduce configuration registers.
  2016-06-15 13:41 [Qemu-devel] [PATCH 0/9] m25p80: Add new 512Mbit and 1Gbit devices marcin.krzeminski
                   ` (5 preceding siblings ...)
  2016-06-15 13:41 ` [Qemu-devel] [PATCH 6/9] m25p80: Introduce quad and equad modes marcin.krzeminski
@ 2016-06-15 13:41 ` marcin.krzeminski
  2016-06-16  7:24   ` Cédric Le Goater
  2016-06-15 13:41 ` [Qemu-devel] [PATCH 8/9] m25p80: Fast read commands family changes marcin.krzeminski
  2016-06-15 13:41 ` [Qemu-devel] [PATCH 9/9] m25p80: New flash devices marcin.krzeminski
  8 siblings, 1 reply; 28+ messages in thread
From: marcin.krzeminski @ 2016-06-15 13:41 UTC (permalink / raw)
  To: qemu-devel
  Cc: crosthwaitepeter, pawel.lenkow, rfsw-patches, peter.maydell, clg

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

Configuration registers for Spansion and Macronix devices.

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

diff --git a/hw/block/m25p80.c b/hw/block/m25p80.c
index d1c4d46..21998db 100644
--- a/hw/block/m25p80.c
+++ b/hw/block/m25p80.c
@@ -134,6 +134,14 @@ typedef struct FlashPartInfo {
 #define FSR_4BYTE_ADDR_MODE_ENABLED 0x1
 #define FSR_FLASH_READY (1 << 7)
 
+/* Spansion configuration registers macros. */
+#define SPANSION_QUAD_CFG_POS 0
+#define SPANSION_QUAD_CFG_LEN 1
+#define SPANSION_DUMMY_CLK_POS 0
+#define SPANSION_DUMMY_CLK_LEN 4
+#define SPANSION_ADDR_LEN_POS 7
+#define SPANSION_ADDR_LEN_LEN 1
+
 static const FlashPartInfo known_devices[] = {
     /* Atmel -- some are (confusingly) marketed as "DataFlash" */
     { INFO("at25fs010",   0x1f6601,      0,  32 << 10,   4, ER_4K) },
@@ -369,8 +377,18 @@ typedef struct Flash {
     uint8_t cmd_in_progress;
     uint64_t cur_addr;
     uint32_t nonvolatile_cfg;
+    /* Configuration register for Macronix */
     uint32_t volatile_cfg;
     uint32_t enh_volatile_cfg;
+    /* Spansion cfg registers. */
+    uint8_t spansion_cr1nv;
+    uint8_t spansion_cr2nv;
+    uint8_t spansion_cr3nv;
+    uint8_t spansion_cr4nv;
+    uint8_t spansion_cr1v;
+    uint8_t spansion_cr2v;
+    uint8_t spansion_cr3v;
+    uint8_t spansion_cr4v;
     bool write_enable;
     bool four_bytes_address_mode;
     bool reset_enable;
@@ -601,6 +619,9 @@ static void complete_collecting_data(Flash *s)
             break;
         case MAN_MACRONIX:
             s->quad_enable = extract32(s->data[0], 6, 1);
+            if (s->len > 1) {
+                s->four_bytes_address_mode = extract32(s->data[1], 5, 1);
+            }
             break;
         default:
             break;
@@ -674,6 +695,23 @@ static void reset_memory(Flash *s)
             s->ear = s->size / MAX_3BYTES_SIZE - 1;
         }
         break;
+    case MAN_MACRONIX:
+        s->volatile_cfg = 0x7;
+        break;
+    case MAN_SPANSION:
+        s->spansion_cr1v = s->spansion_cr1nv;
+        s->spansion_cr2v = s->spansion_cr2nv;
+        s->spansion_cr3v = s->spansion_cr3nv;
+        s->spansion_cr4v = s->spansion_cr4nv;
+        s->quad_enable = extract32(s->spansion_cr1v,
+                                   SPANSION_QUAD_CFG_POS,
+                                   SPANSION_QUAD_CFG_LEN
+                                   );
+        s->four_bytes_address_mode = extract32(s->spansion_cr2v,
+                SPANSION_ADDR_LEN_POS,
+                SPANSION_ADDR_LEN_LEN
+                );
+        break;
     default:
         break;
     }
@@ -1052,7 +1090,12 @@ static void m25p80_pre_save(void *opaque)
 }
 
 static Property m25p80_properties[] = {
+    /* This is default value for Micron flash */
     DEFINE_PROP_UINT32("nonvolatile-cfg", Flash, nonvolatile_cfg, 0x8FFF),
+    DEFINE_PROP_UINT8("spansion-cr1nv", Flash, spansion_cr1nv, 0x0),
+    DEFINE_PROP_UINT8("spansion-cr2nv", Flash, spansion_cr2nv, 0x8),
+    DEFINE_PROP_UINT8("spansion-cr3nv", Flash, spansion_cr3nv, 0x2),
+    DEFINE_PROP_UINT8("spansion-cr4nv", Flash, spansion_cr4nv, 0x10),
     DEFINE_PROP_END_OF_LIST(),
 };
 
@@ -1077,6 +1120,10 @@ static const VMStateDescription vmstate_m25p80 = {
         VMSTATE_UINT32_V(volatile_cfg, Flash, 2),
         VMSTATE_UINT32_V(enh_volatile_cfg, Flash, 2),
         VMSTATE_BOOL_V(quad_enable, Flash, 3),
+        VMSTATE_UINT8_V(spansion_cr1nv, Flash, 3),
+        VMSTATE_UINT8_V(spansion_cr2nv, Flash, 3),
+        VMSTATE_UINT8_V(spansion_cr3nv, Flash, 3),
+        VMSTATE_UINT8_V(spansion_cr4nv, Flash, 3),
         VMSTATE_END_OF_LIST()
     }
 };
-- 
2.7.4

^ permalink raw reply related	[flat|nested] 28+ messages in thread

* [Qemu-devel] [PATCH 8/9] m25p80: Fast read commands family changes.
  2016-06-15 13:41 [Qemu-devel] [PATCH 0/9] m25p80: Add new 512Mbit and 1Gbit devices marcin.krzeminski
                   ` (6 preceding siblings ...)
  2016-06-15 13:41 ` [Qemu-devel] [PATCH 7/9] m25p80: Introduce configuration registers marcin.krzeminski
@ 2016-06-15 13:41 ` marcin.krzeminski
  2016-06-16  7:19   ` Cédric Le Goater
  2016-06-15 13:41 ` [Qemu-devel] [PATCH 9/9] m25p80: New flash devices marcin.krzeminski
  8 siblings, 1 reply; 28+ messages in thread
From: marcin.krzeminski @ 2016-06-15 13:41 UTC (permalink / raw)
  To: qemu-devel
  Cc: crosthwaitepeter, pawel.lenkow, rfsw-patches, peter.maydell, clg

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

Add support for Spansion and Macronix flashes.
Additionally Numonyx(Micron) move from default
in fast read commands family.

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

diff --git a/hw/block/m25p80.c b/hw/block/m25p80.c
index 21998db..7bc0e03 100644
--- a/hw/block/m25p80.c
+++ b/hw/block/m25p80.c
@@ -758,9 +758,23 @@ static void decode_new_cmd(Flash *s, uint32_t value)
     case QOR4:
         s->needed_bytes = get_addr_length(s);
         switch (get_man(s)) {
+        /* Dummy cycles - modeled with bytes writes instead of bits */
         case MAN_NUMONYX:
             s->needed_bytes += extract32(s->volatile_cfg, 4, 4);
             break;
+        case MAN_MACRONIX:
+            if (extract32(s->volatile_cfg, 6, 2) == 1) {
+                s->needed_bytes += 6;
+            } else {
+                s->needed_bytes += 8;
+            }
+            break;
+        case MAN_SPANSION:
+            s->needed_bytes += extract32(s->spansion_cr2v,
+                                        SPANSION_DUMMY_CLK_POS,
+                                        SPANSION_DUMMY_CLK_LEN
+                                        );
+            break;
         default:
             break;
         }
@@ -771,15 +785,36 @@ static void decode_new_cmd(Flash *s, uint32_t value)
 
     case DIOR:
     case DIOR4:
+        s->needed_bytes = get_addr_length(s);
+        /* Dummy cycles modeled with bytes writes instead of bits */
         switch (get_man(s)) {
         case MAN_WINBOND:
+            s->needed_bytes += 4;
+            break;
         case MAN_SPANSION:
-            s->needed_bytes = 4;
+            s->needed_bytes += extract32(s->spansion_cr2v,
+                                        SPANSION_DUMMY_CLK_POS,
+                                        SPANSION_DUMMY_CLK_LEN
+                                        );
             break;
-        default:
-            s->needed_bytes = get_addr_length(s);
-            /* Dummy cycles modeled with bytes writes instead of bits */
+        case MAN_NUMONYX:
             s->needed_bytes += extract32(s->volatile_cfg, 4, 4);
+            break;
+        case MAN_MACRONIX:
+            switch (extract32(s->volatile_cfg, 6, 2)) {
+            case 1:
+                s->needed_bytes += 6;
+                break;
+            case 2:
+                s->needed_bytes += 8;
+                break;
+            default:
+                s->needed_bytes += 4;
+                break;
+            }
+            break;
+        default:
+            break;
         }
         s->pos = 0;
         s->len = 0;
@@ -788,15 +823,36 @@ static void decode_new_cmd(Flash *s, uint32_t value)
 
     case QIOR:
     case QIOR4:
+        s->needed_bytes = get_addr_length(s);
+        /* Dummy cycles modeled with bytes writes instead of bits */
         switch (get_man(s)) {
         case MAN_WINBOND:
+            s->needed_bytes += 4;
+            break;
         case MAN_SPANSION:
-            s->needed_bytes = 6;
+            s->needed_bytes += extract32(s->spansion_cr2v,
+                                        SPANSION_DUMMY_CLK_POS,
+                                        SPANSION_DUMMY_CLK_LEN
+                                        );
             break;
-        default:
-            s->needed_bytes = get_addr_length(s);
-            /* Dummy cycles modeled with bytes writes instead of bits */
+        case MAN_NUMONYX:
             s->needed_bytes += extract32(s->volatile_cfg, 4, 4);
+            break;
+        case MAN_MACRONIX:
+            switch (extract32(s->volatile_cfg, 6, 2)) {
+            case 1:
+                s->needed_bytes += 4;
+                break;
+            case 2:
+                s->needed_bytes += 8;
+                break;
+            default:
+                s->needed_bytes += 6;
+                break;
+            }
+            break;
+        default:
+            break;
         }
         s->pos = 0;
         s->len = 0;
-- 
2.7.4

^ permalink raw reply related	[flat|nested] 28+ messages in thread

* [Qemu-devel] [PATCH 9/9] m25p80: New flash devices.
  2016-06-15 13:41 [Qemu-devel] [PATCH 0/9] m25p80: Add new 512Mbit and 1Gbit devices marcin.krzeminski
                   ` (7 preceding siblings ...)
  2016-06-15 13:41 ` [Qemu-devel] [PATCH 8/9] m25p80: Fast read commands family changes marcin.krzeminski
@ 2016-06-15 13:41 ` marcin.krzeminski
  2016-06-16  7:20   ` Cédric Le Goater
  8 siblings, 1 reply; 28+ messages in thread
From: marcin.krzeminski @ 2016-06-15 13:41 UTC (permalink / raw)
  To: qemu-devel
  Cc: crosthwaitepeter, pawel.lenkow, rfsw-patches, peter.maydell, clg

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

Macronix: mx66u51235f and mx66u1g45g
Micron: mt25ql01g and mt25qu01g
Spansion: s25fs512s and s70fs01gs

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

diff --git a/hw/block/m25p80.c b/hw/block/m25p80.c
index 7bc0e03..e1a7698 100644
--- a/hw/block/m25p80.c
+++ b/hw/block/m25p80.c
@@ -192,6 +192,8 @@ static const FlashPartInfo known_devices[] = {
     { INFO("mx25l12855e", 0xc22618,      0,  64 << 10, 256, 0) },
     { INFO("mx25l25635e", 0xc22019,      0,  64 << 10, 512, 0) },
     { INFO("mx25l25655e", 0xc22619,      0,  64 << 10, 512, 0) },
+    { INFO("mx66u51235f", 0xc2253a,      0,  64 << 10, 1024, ER_4K | ER_32K) },
+    { INFO("mx66u1g45g",  0xc2253b,      0,  64 << 10, 2048, ER_4K | ER_32K) },
 
     /* Micron */
     { INFO("n25q032a11",  0x20bb16,      0,  64 << 10,  64, ER_4K) },
@@ -202,6 +204,11 @@ static const FlashPartInfo known_devices[] = {
     { INFO("n25q128a13",  0x20ba18,      0,  64 << 10, 256, ER_4K) },
     { INFO("n25q256a11",  0x20bb19,      0,  64 << 10, 512, ER_4K) },
     { INFO("n25q256a13",  0x20ba19,      0,  64 << 10, 512, ER_4K) },
+    { 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) },
+    { INFO("mt25ql01g",   0x20ba21,      0,  64 << 10, 2048, ER_4K) },
+    { INFO("mt25qu01g",   0x20bb21,      0,  64 << 10, 2048, ER_4K) },
 
     /* Spansion -- single (large) sector size only, at least
      * for the chips listed here (without boot sectors).
@@ -210,8 +217,8 @@ static const FlashPartInfo known_devices[] = {
     { INFO("s25sl064p",   0x010216, 0x4d00,  64 << 10, 128, ER_4K) },
     { INFO("s25fl256s0",  0x010219, 0x4d00, 256 << 10, 128, 0) },
     { INFO("s25fl256s1",  0x010219, 0x4d01,  64 << 10, 512, 0) },
-    { INFO("s25fl512s",   0x010220, 0x4d00, 256 << 10, 256, 0) },
-    { INFO("s70fl01gs",   0x010221, 0x4d00, 256 << 10, 256, 0) },
+    { INFO6("s25fl512s",  0x010220, 0x4d0080, 256 << 10, 256, 0) },
+    { INFO6("s70fl01gs",  0x010221, 0x4d0080, 256 << 10, 512, 0) },
     { INFO("s25sl12800",  0x012018, 0x0300, 256 << 10,  64, 0) },
     { INFO("s25sl12801",  0x012018, 0x0301,  64 << 10, 256, 0) },
     { INFO("s25fl129p0",  0x012018, 0x4d00, 256 << 10,  64, 0) },
@@ -224,6 +231,10 @@ static const FlashPartInfo known_devices[] = {
     { INFO("s25fl016k",   0xef4015,      0,  64 << 10,  32, ER_4K | ER_32K) },
     { INFO("s25fl064k",   0xef4017,      0,  64 << 10, 128, ER_4K | ER_32K) },
 
+    /* Spansion --  boot sectors support  */
+    { INFO6("s25fs512s",    0x010220, 0x4d0081, 256 << 10, 256, 0) },
+    { INFO6("s70fs01gs",    0x010221, 0x4d0081, 256 << 10, 512, 0) },
+
     /* SST -- large erase sizes are "overlays", "sectors" are 4<< 10 */
     { INFO("sst25vf040b", 0xbf258d,      0,  64 << 10,   8, ER_4K) },
     { INFO("sst25vf080b", 0xbf258e,      0,  64 << 10,  16, ER_4K) },
@@ -274,10 +285,6 @@ static const FlashPartInfo known_devices[] = {
     { INFO("w25q80",      0xef5014,      0,  64 << 10,  16, ER_4K) },
     { INFO("w25q80bl",    0xef4014,      0,  64 << 10,  16, ER_4K) },
     { INFO("w25q256",     0xef4019,      0,  64 << 10, 512, ER_4K) },
-
-    { 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.7.4

^ permalink raw reply related	[flat|nested] 28+ messages in thread

* Re: [Qemu-devel] [PATCH 1/9] m25p80: Replace JEDEC ID masking with function.
  2016-06-15 13:41 ` [Qemu-devel] [PATCH 1/9] m25p80: Replace JEDEC ID masking with function marcin.krzeminski
@ 2016-06-15 14:05   ` Cédric Le Goater
  2016-06-15 17:09     ` [Qemu-devel] Odp.: " Krzeminski, Marcin (Nokia - PL/Wroclaw)
  0 siblings, 1 reply; 28+ messages in thread
From: Cédric Le Goater @ 2016-06-15 14:05 UTC (permalink / raw)
  To: marcin.krzeminski, qemu-devel
  Cc: crosthwaitepeter, pawel.lenkow, peter.maydell

On 06/15/2016 03:41 PM, marcin.krzeminski@nokia.com wrote:
> From: Marcin Krzeminski <marcin.krzeminski@nokia.com>
> 
> Instead of always reading and comparing jededc ID,
> replace it by function.
> 
> Signed-off-by: Marcin Krzeminski <marcin.krzeminski@nokia.com>

Looks good to me. Some minor comments below.

Thanks,

C.


> ---
>  hw/block/m25p80.c | 49 ++++++++++++++++++++++++++++++++++++++++---------
>  1 file changed, 40 insertions(+), 9 deletions(-)
> 
> diff --git a/hw/block/m25p80.c b/hw/block/m25p80.c
> index 4c856f5..15765f5 100644
> --- a/hw/block/m25p80.c
> +++ b/hw/block/m25p80.c
> @@ -307,6 +307,14 @@ typedef enum {
>      STATE_READING_DATA,
>  } CMDState;
> 
> +typedef enum {
> +    MAN_SPANSION,
> +    MAN_MACRONIX,
> +    MAN_NUMONYX,
> +    MAN_WINBOND,
> +    MAN_GENERIC,
> +} Manufacturer;
> +
>  typedef struct Flash {
>      SSISlave parent_obj;
> 
> @@ -350,6 +358,22 @@ typedef struct M25P80Class {
>  #define M25P80_GET_CLASS(obj) \
>       OBJECT_GET_CLASS(M25P80Class, (obj), TYPE_M25P80)
> 
> +static inline Manufacturer get_man(Flash *s)
> +{
> +    switch (((s->pi->jedec >> 16) & 0xFF)) {
> +    case 0x20:
> +        return MAN_NUMONYX;
> +    case 0xEF:
> +        return MAN_WINBOND;
> +    case 0x01:
> +        return MAN_SPANSION;
> +    case 0xC2:
> +        return MAN_MACRONIX;
> +    default:
> +        return MAN_GENERIC;
> +    }
> +}
> +
>  static void blk_sync_complete(void *opaque, int ret)
>  {
>      /* do nothing. Masters do not directly interact with the backing store,
> @@ -562,7 +586,8 @@ static void reset_memory(Flash *s)
>      s->write_enable = false;
>      s->reset_enable = false;
> 
> -    if (((s->pi->jedec >> 16) & 0xFF) == JEDEC_NUMONYX) {

We could have kept the if ()

> +    switch (get_man(s)) {
> +    case MAN_NUMONYX:
>          s->volatile_cfg = 0;
>          s->volatile_cfg |= VCFG_DUMMY;
>          s->volatile_cfg |= VCFG_WRAP_SEQUENTIAL;
> @@ -594,6 +619,9 @@ static void reset_memory(Flash *s)
>          if (!(s->nonvolatile_cfg & NVCFG_LOWER_SEGMENT_MASK)) {
>              s->ear = CFG_UPPER_128MB_SEG_ENABLED;
>          }
> +        break;
> +    default:
> +        break;
>      }
> 
>      DB_PRINT_L(0, "Reset done.\n");
> @@ -634,9 +662,12 @@ static void decode_new_cmd(Flash *s, uint32_t value)
>      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 */

why remove the comment ? 

> +        switch (get_man(s)) {
> +        case MAN_NUMONYX:
>              s->needed_bytes += extract32(s->volatile_cfg, 4, 4);
> +            break;
> +        default:
> +            break;
>          }
>          s->pos = 0;
>          s->len = 0;
> @@ -645,9 +676,9 @@ static void decode_new_cmd(Flash *s, uint32_t value)
> 
>      case DIOR:
>      case DIOR4:
> -        switch ((s->pi->jedec >> 16) & 0xFF) {
> -        case JEDEC_WINBOND:
> -        case JEDEC_SPANSION:
> +        switch (get_man(s)) {
> +        case MAN_WINBOND:
> +        case MAN_SPANSION:
>              s->needed_bytes = 4;
>              break;
>          default:
> @@ -662,9 +693,9 @@ static void decode_new_cmd(Flash *s, uint32_t value)
> 
>      case QIOR:
>      case QIOR4:
> -        switch ((s->pi->jedec >> 16) & 0xFF) {
> -        case JEDEC_WINBOND:
> -        case JEDEC_SPANSION:
> +        switch (get_man(s)) {
> +        case MAN_WINBOND:
> +        case MAN_SPANSION:
>              s->needed_bytes = 6;
>              break;
>          default:
> 

^ permalink raw reply	[flat|nested] 28+ messages in thread

* Re: [Qemu-devel] [PATCH 2/9] m25p80: Make a table for JEDEC ID.
  2016-06-15 13:41 ` [Qemu-devel] [PATCH 2/9] m25p80: Make a table for JEDEC ID marcin.krzeminski
@ 2016-06-15 14:17   ` Cédric Le Goater
  0 siblings, 0 replies; 28+ messages in thread
From: Cédric Le Goater @ 2016-06-15 14:17 UTC (permalink / raw)
  To: marcin.krzeminski, qemu-devel
  Cc: crosthwaitepeter, pawel.lenkow, peter.maydell

On 06/15/2016 03:41 PM, marcin.krzeminski@nokia.com wrote:
> From: Marcin Krzeminski <marcin.krzeminski@nokia.com>
> 
> Since it is now longer than 4. This work based on Pawel Lenkow
> changes and the kernel SPI framework.
>
> Signed-off-by: Marcin Krzeminski <marcin.krzeminski@nokia.com>

Reviewed-by: Cédric Le Goater <clg@kaod.org>

> ---
>  hw/block/m25p80.c | 61 ++++++++++++++++++++++++++++++++++++++-----------------
>  1 file changed, 42 insertions(+), 19 deletions(-)
> 
> diff --git a/hw/block/m25p80.c b/hw/block/m25p80.c
> index 15765f5..342f7c9 100644
> --- a/hw/block/m25p80.c
> +++ b/hw/block/m25p80.c
> @@ -53,12 +53,17 @@
>  /* 16 MiB max in 3 byte address mode */
>  #define MAX_3BYTES_SIZE 0x1000000
> 
> +#define SPI_NOR_MAX_ID_LEN 6
> +
>  typedef struct FlashPartInfo {
>      const char *part_name;
> -    /* jedec code. (jedec >> 16) & 0xff is the 1st byte, >> 8 the 2nd etc */
> -    uint32_t jedec;
> -    /* extended jedec code */
> -    uint16_t ext_jedec;
> +    /*
> +     * This array stores the ID bytes.
> +     * The first three bytes are the JEDIC ID.
> +     * JEDEC ID zero means "no ID" (mostly older chips).
> +     */
> +    uint8_t id[SPI_NOR_MAX_ID_LEN];
> +    uint8_t id_len;
>      /* there is confusion between manufacturers as to what a sector is. In this
>       * device model, a "sector" is the size that is erased by the ERASE_SECTOR
>       * command (opcode 0xd8).
> @@ -70,11 +75,33 @@ typedef struct FlashPartInfo {
>  } FlashPartInfo;
> 
>  /* adapted from linux */
> -
> -#define INFO(_part_name, _jedec, _ext_jedec, _sector_size, _n_sectors, _flags)\
> -    .part_name = (_part_name),\
> -    .jedec = (_jedec),\
> -    .ext_jedec = (_ext_jedec),\
> +/* Used when the "_ext_id" is two bytes at most */
> +#define INFO(_part_name, _jedec_id, _ext_id, _sector_size, _n_sectors, _flags)\
> +    .part_name = _part_name,\
> +    .id = {\
> +        ((_jedec_id) >> 16) & 0xff,\
> +        ((_jedec_id) >> 8) & 0xff,\
> +        (_jedec_id) & 0xff,\
> +        ((_ext_id) >> 8) & 0xff,\
> +        (_ext_id) & 0xff,\
> +          },\
> +    .id_len = (!(_jedec_id) ? 0 : (3 + ((_ext_id) ? 2 : 0))),\
> +    .sector_size = (_sector_size),\
> +    .n_sectors = (_n_sectors),\
> +    .page_size = 256,\
> +    .flags = (_flags),
> +
> +#define INFO6(_part_name, _jedec_id, _ext_id, _sector_size, _n_sectors, _flags)\
> +    .part_name = _part_name,\
> +    .id = {\
> +        ((_jedec_id) >> 16) & 0xff,\
> +        ((_jedec_id) >> 8) & 0xff,\
> +        (_jedec_id) & 0xff,\
> +        ((_ext_id) >> 16) & 0xff,\
> +        ((_ext_id) >> 8) & 0xff,\
> +        (_ext_id) & 0xff,\
> +          },\
> +    .id_len = 6,\
>      .sector_size = (_sector_size),\
>      .n_sectors = (_n_sectors),\
>      .page_size = 256,\
> @@ -360,7 +387,7 @@ typedef struct M25P80Class {
> 
>  static inline Manufacturer get_man(Flash *s)
>  {
> -    switch (((s->pi->jedec >> 16) & 0xFF)) {
> +    switch (s->pi->id[0]) {
>      case 0x20:
>          return MAN_NUMONYX;
>      case 0xEF:
> @@ -630,6 +657,7 @@ static void reset_memory(Flash *s)
>  static void decode_new_cmd(Flash *s, uint32_t value)
>  {
>      s->cmd_in_progress = value;
> +    int i;
>      DB_PRINT_L(0, "decoded new command:%x\n", value);
> 
>      if (value != RESET_MEMORY) {
> @@ -743,16 +771,11 @@ static void decode_new_cmd(Flash *s, uint32_t value)
> 
>      case JEDEC_READ:
>          DB_PRINT_L(0, "populated jedec code\n");
> -        s->data[0] = (s->pi->jedec >> 16) & 0xff;
> -        s->data[1] = (s->pi->jedec >> 8) & 0xff;
> -        s->data[2] = s->pi->jedec & 0xff;
> -        if (s->pi->ext_jedec) {
> -            s->data[3] = (s->pi->ext_jedec >> 8) & 0xff;
> -            s->data[4] = s->pi->ext_jedec & 0xff;
> -            s->len = 5;
> -        } else {
> -            s->len = 3;
> +        for (i = 0; i < s->pi->id_len; i++) {
> +            s->data[i] = s->pi->id[i];
>          }
> +
> +        s->len = s->pi->id_len;
>          s->pos = 0;
>          s->state = STATE_READING_DATA;
>          break;
> 

^ permalink raw reply	[flat|nested] 28+ messages in thread

* Re: [Qemu-devel] [PATCH 6/9] m25p80: Introduce quad and equad modes.
  2016-06-15 13:41 ` [Qemu-devel] [PATCH 6/9] m25p80: Introduce quad and equad modes marcin.krzeminski
@ 2016-06-15 14:25   ` Cédric Le Goater
  2016-06-15 17:40     ` [Qemu-devel] Odp.: " Krzeminski, Marcin (Nokia - PL/Wroclaw)
  0 siblings, 1 reply; 28+ messages in thread
From: Cédric Le Goater @ 2016-06-15 14:25 UTC (permalink / raw)
  To: marcin.krzeminski, qemu-devel
  Cc: crosthwaitepeter, pawel.lenkow, peter.maydell

On 06/15/2016 03:41 PM, marcin.krzeminski@nokia.com wrote:
> From: Marcin Krzeminski <marcin.krzeminski@nokia.com>
> 
> Quad and Equad modes for Spansion and Macronix flash devices.
> This commit also includes modification and new command to manipulate
> quad mode (status registers and dedicated commands).
> This work is based on Pawel Lenkow work.
> 
> Signed-off-by: Marcin Krzeminski <marcin.krzeminski@nokia.com>
> ---
>  hw/block/m25p80.c | 70 +++++++++++++++++++++++++++++++++++++++++++++++++++----
>  1 file changed, 65 insertions(+), 5 deletions(-)
> 
> diff --git a/hw/block/m25p80.c b/hw/block/m25p80.c
> index 55b4377..d1c4d46 100644
> --- a/hw/block/m25p80.c
> +++ b/hw/block/m25p80.c
> @@ -281,6 +281,7 @@ typedef enum {
>      JEDEC_READ = 0x9f,
>      BULK_ERASE = 0xc7,
>      READ_FSR = 0x70,
> +    RDCR = 0x15,
> 
>      READ = 0x03,
>      READ4 = 0x13,
> @@ -317,6 +318,13 @@ typedef enum {
>      RESET_ENABLE = 0x66,
>      RESET_MEMORY = 0x99,
> 
> +    /*
> +     * Micron: 0x35 - enable QPI
> +     * Spansion: 0x35 - read control register
> +     */
> +    RDCR_EQIO = 0x35,
> +    RSTQIO = 0xf5,
> +
>      RNVCR = 0xB5,
>      WNVCR = 0xB1,
> 
> @@ -366,6 +374,7 @@ typedef struct Flash {
>      bool write_enable;
>      bool four_bytes_address_mode;
>      bool reset_enable;
> +    bool quad_enable;
>      uint8_t ear;
> 
>      int64_t dirty_page;
> @@ -586,6 +595,16 @@ static void complete_collecting_data(Flash *s)
>          flash_erase(s, s->cur_addr, s->cmd_in_progress);
>          break;
>      case WRSR:
> +        switch (get_man(s)) {
> +        case MAN_SPANSION:
> +            s->quad_enable = !!(s->data[1] & 0x02);
> +            break;
> +        case MAN_MACRONIX:
> +            s->quad_enable = extract32(s->data[0], 6, 1);
> +            break;
> +        default:
> +            break;
> +        }
>          if (s->write_enable) {
>              s->write_enable = false;
>          }
> @@ -619,6 +638,7 @@ static void reset_memory(Flash *s)
>      s->state = STATE_IDLE;
>      s->write_enable = false;
>      s->reset_enable = false;
> +    s->quad_enable = false;
> 
>      switch (get_man(s)) {
>      case MAN_NUMONYX:
> @@ -747,10 +767,20 @@ static void decode_new_cmd(Flash *s, uint32_t value)
> 
>      case WRSR:
>          if (s->write_enable) {
> -            s->needed_bytes = 1;
> +            switch (get_man(s)) {
> +            case MAN_SPANSION:
> +                s->needed_bytes = 2;
> +                s->state = STATE_COLLECTING_DATA;
> +                break;
> +            case MAN_MACRONIX:
> +                s->needed_bytes = 2;
> +                s->state = STATE_COLLECTING_VAR_LEN_DATA;
> +                break;

ah. I am glad you address this topic because I am having a little issue
with the mx25l25635e and the mx25l25635f.

mx25l25635e is a macronix which supports the WRSR command with one byte 
and the mx25l25635f needs two. The fun part is that they have the same
JEDEC : 0xc22019.

So not all macronix need 2 bytes. Should we add a flag for that ? 


> +            default:
> +                s->needed_bytes = 1;
> +                s->state = STATE_COLLECTING_DATA;
> +            }
>              s->pos = 0;
> -            s->len = 0;
> -            s->state = STATE_COLLECTING_DATA;
>          }
>          break;
> 
> @@ -763,6 +793,9 @@ static void decode_new_cmd(Flash *s, uint32_t value)
> 
>      case RDSR:
>          s->data[0] = (!!s->write_enable) << 1;
> +        if (get_man(s) == MAN_MACRONIX) {
> +            s->data[0] |= (!!s->quad_enable) << 6;
> +        }
>          s->pos = 0;
>          s->len = 1;
>          s->state = STATE_READING_DATA;
> @@ -789,6 +822,14 @@ static void decode_new_cmd(Flash *s, uint32_t value)
>          s->state = STATE_READING_DATA;
>          break;
> 
> +    case RDCR:
> +        s->data[0] = s->volatile_cfg & 0xFF;
> +        s->data[0] |= (!!s->four_bytes_address_mode) << 5;
> +        s->pos = 0;
> +        s->len = 1;
> +        s->state = STATE_READING_DATA;
> +        break;
> +
>      case BULK_ERASE:
>          if (s->write_enable) {
>              DB_PRINT_L(0, "chip erase\n");
> @@ -828,7 +869,7 @@ static void decode_new_cmd(Flash *s, uint32_t value)
>          s->state = STATE_READING_DATA;
>          break;
>      case WNVCR:
> -        if (s->write_enable) {
> +        if (s->write_enable && get_man(s) == MAN_NUMONYX) {
>              s->needed_bytes = 2;
>              s->pos = 0;
>              s->len = 0;
> @@ -871,6 +912,24 @@ static void decode_new_cmd(Flash *s, uint32_t value)
>              reset_memory(s);
>          }
>          break;
> +    case RDCR_EQIO:
> +        switch (get_man(s)) {
> +        case MAN_SPANSION:
> +            s->data[0] = (!!s->quad_enable) << 1;
> +            s->pos = 0;
> +            s->len = 1;
> +            s->state = STATE_READING_DATA;
> +            break;
> +        case MAN_MACRONIX:
> +            s->quad_enable = true;
> +            break;
> +        default:
> +            break;
> +        }
> +        break;
> +    case RSTQIO:
> +        s->quad_enable = false;
> +        break;
>      default:
>          qemu_log_mask(LOG_GUEST_ERROR, "M25P80: Unknown cmd %x\n", value);
>          break;
> @@ -999,7 +1058,7 @@ static Property m25p80_properties[] = {
> 
>  static const VMStateDescription vmstate_m25p80 = {
>      .name = "xilinx_spi",
> -    .version_id = 2,
> +    .version_id = 3,
>      .minimum_version_id = 1,
>      .pre_save = m25p80_pre_save,
>      .fields = (VMStateField[]) {
> @@ -1017,6 +1076,7 @@ static const VMStateDescription vmstate_m25p80 = {
>          VMSTATE_UINT32_V(nonvolatile_cfg, Flash, 2),
>          VMSTATE_UINT32_V(volatile_cfg, Flash, 2),
>          VMSTATE_UINT32_V(enh_volatile_cfg, Flash, 2),
> +        VMSTATE_BOOL_V(quad_enable, Flash, 3),
>          VMSTATE_END_OF_LIST()
>      }
>  };
> 

^ permalink raw reply	[flat|nested] 28+ messages in thread

* [Qemu-devel] Odp.: [PATCH 1/9] m25p80: Replace JEDEC ID masking with function.
  2016-06-15 14:05   ` Cédric Le Goater
@ 2016-06-15 17:09     ` Krzeminski, Marcin (Nokia - PL/Wroclaw)
  0 siblings, 0 replies; 28+ messages in thread
From: Krzeminski, Marcin (Nokia - PL/Wroclaw) @ 2016-06-15 17:09 UTC (permalink / raw)
  To: Cédric Le Goater, qemu-devel@nongnu.org
  Cc: crosthwaitepeter@gmail.com, pawel.lenkow@itlen.com,
	peter.maydell@linaro.org



W dniu 15.06.2016 o 16:05, Cédric Le Goater pisze:

On 06/15/2016 03:41 PM, marcin.krzeminski@nokia.com wrote:


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

Instead of always reading and comparing jededc ID,
replace it by function.

Signed-off-by: Marcin Krzeminski <marcin.krzeminski@nokia.com>


Looks good to me. Some minor comments below.

Thanks,

C.




---
 hw/block/m25p80.c | 49 ++++++++++++++++++++++++++++++++++++++++---------
 1 file changed, 40 insertions(+), 9 deletions(-)

diff --git a/hw/block/m25p80.c b/hw/block/m25p80.c
index 4c856f5..15765f5 100644
--- a/hw/block/m25p80.c
+++ b/hw/block/m25p80.c
@@ -307,6 +307,14 @@ typedef enum {
     STATE_READING_DATA,
 } CMDState;

+typedef enum {
+    MAN_SPANSION,
+    MAN_MACRONIX,
+    MAN_NUMONYX,
+    MAN_WINBOND,
+    MAN_GENERIC,
+} Manufacturer;
+
 typedef struct Flash {
     SSISlave parent_obj;

@@ -350,6 +358,22 @@ typedef struct M25P80Class {
 #define M25P80_GET_CLASS(obj) \
      OBJECT_GET_CLASS(M25P80Class, (obj), TYPE_M25P80)

+static inline Manufacturer get_man(Flash *s)
+{
+    switch (((s->pi->jedec >> 16) & 0xFF)) {
+    case 0x20:
+        return MAN_NUMONYX;
+    case 0xEF:
+        return MAN_WINBOND;
+    case 0x01:
+        return MAN_SPANSION;
+    case 0xC2:
+        return MAN_MACRONIX;
+    default:
+        return MAN_GENERIC;
+    }
+}
+
 static void blk_sync_complete(void *opaque, int ret)
 {
     /* do nothing. Masters do not directly interact with the backing store,
@@ -562,7 +586,8 @@ static void reset_memory(Flash *s)
     s->write_enable = false;
     s->reset_enable = false;

-    if (((s->pi->jedec >> 16) & 0xFF) == JEDEC_NUMONYX) {


We could have kept the if ()

In patch 7 here will land reset procedures for Spansion and Macronix.
That is why there is a switch here.

+    switch (get_man(s)) {
+    case MAN_NUMONYX:
         s->volatile_cfg = 0;
         s->volatile_cfg |= VCFG_DUMMY;
         s->volatile_cfg |= VCFG_WRAP_SEQUENTIAL;
@@ -594,6 +619,9 @@ static void reset_memory(Flash *s)
         if (!(s->nonvolatile_cfg & NVCFG_LOWER_SEGMENT_MASK)) {
             s->ear = CFG_UPPER_128MB_SEG_ENABLED;
         }
+        break;
+    default:
+        break;
     }

     DB_PRINT_L(0, "Reset done.\n");
@@ -634,9 +662,12 @@ static void decode_new_cmd(Flash *s, uint32_t value)
     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 */


why remove the comment ?

Accidental, it will return in patch 8.

Thanks,
Marcin

+        switch (get_man(s)) {
+        case MAN_NUMONYX:
             s->needed_bytes += extract32(s->volatile_cfg, 4, 4);
+            break;
+        default:
+            break;
         }
         s->pos = 0;
         s->len = 0;
@@ -645,9 +676,9 @@ static void decode_new_cmd(Flash *s, uint32_t value)

     case DIOR:
     case DIOR4:
-        switch ((s->pi->jedec >> 16) & 0xFF) {
-        case JEDEC_WINBOND:
-        case JEDEC_SPANSION:
+        switch (get_man(s)) {
+        case MAN_WINBOND:
+        case MAN_SPANSION:
             s->needed_bytes = 4;
             break;
         default:
@@ -662,9 +693,9 @@ static void decode_new_cmd(Flash *s, uint32_t value)

     case QIOR:
     case QIOR4:
-        switch ((s->pi->jedec >> 16) & 0xFF) {
-        case JEDEC_WINBOND:
-        case JEDEC_SPANSION:
+        switch (get_man(s)) {
+        case MAN_WINBOND:
+        case MAN_SPANSION:
             s->needed_bytes = 6;
             break;
         default:



.

^ permalink raw reply related	[flat|nested] 28+ messages in thread

* [Qemu-devel] Odp.: [PATCH 6/9] m25p80: Introduce quad and equad modes.
  2016-06-15 14:25   ` Cédric Le Goater
@ 2016-06-15 17:40     ` Krzeminski, Marcin (Nokia - PL/Wroclaw)
  2016-06-17 12:43       ` Cédric Le Goater
  0 siblings, 1 reply; 28+ messages in thread
From: Krzeminski, Marcin (Nokia - PL/Wroclaw) @ 2016-06-15 17:40 UTC (permalink / raw)
  To: Cédric Le Goater, qemu-devel@nongnu.org
  Cc: crosthwaitepeter@gmail.com, pawel.lenkow@itlen.com,
	peter.maydell@linaro.org



W dniu 15.06.2016 o 16:25, Cédric Le Goater pisze:
> On 06/15/2016 03:41 PM, marcin.krzeminski@nokia.com wrote:
>> From: Marcin Krzeminski <marcin.krzeminski@nokia.com>
>>
>> Quad and Equad modes for Spansion and Macronix flash devices.
>> This commit also includes modification and new command to manipulate
>> quad mode (status registers and dedicated commands).
>> This work is based on Pawel Lenkow work.
>>
>> Signed-off-by: Marcin Krzeminski <marcin.krzeminski@nokia.com>
>> ---
>>  hw/block/m25p80.c | 70 +++++++++++++++++++++++++++++++++++++++++++++++++++----
>>  1 file changed, 65 insertions(+), 5 deletions(-)
>>
>> diff --git a/hw/block/m25p80.c b/hw/block/m25p80.c
>> index 55b4377..d1c4d46 100644
>> --- a/hw/block/m25p80.c
>> +++ b/hw/block/m25p80.c
>> @@ -281,6 +281,7 @@ typedef enum {
>>      JEDEC_READ = 0x9f,
>>      BULK_ERASE = 0xc7,
>>      READ_FSR = 0x70,
>> +    RDCR = 0x15,
>>
>>      READ = 0x03,
>>      READ4 = 0x13,
>> @@ -317,6 +318,13 @@ typedef enum {
>>      RESET_ENABLE = 0x66,
>>      RESET_MEMORY = 0x99,
>>
>> +    /*
>> +     * Micron: 0x35 - enable QPI
>> +     * Spansion: 0x35 - read control register
>> +     */
>> +    RDCR_EQIO = 0x35,
>> +    RSTQIO = 0xf5,
>> +
>>      RNVCR = 0xB5,
>>      WNVCR = 0xB1,
>>
>> @@ -366,6 +374,7 @@ typedef struct Flash {
>>      bool write_enable;
>>      bool four_bytes_address_mode;
>>      bool reset_enable;
>> +    bool quad_enable;
>>      uint8_t ear;
>>
>>      int64_t dirty_page;
>> @@ -586,6 +595,16 @@ static void complete_collecting_data(Flash *s)
>>          flash_erase(s, s->cur_addr, s->cmd_in_progress);
>>          break;
>>      case WRSR:
>> +        switch (get_man(s)) {
>> +        case MAN_SPANSION:
>> +            s->quad_enable = !!(s->data[1] & 0x02);
>> +            break;
>> +        case MAN_MACRONIX:
>> +            s->quad_enable = extract32(s->data[0], 6, 1);
>> +            break;
>> +        default:
>> +            break;
>> +        }
>>          if (s->write_enable) {
>>              s->write_enable = false;
>>          }
>> @@ -619,6 +638,7 @@ static void reset_memory(Flash *s)
>>      s->state = STATE_IDLE;
>>      s->write_enable = false;
>>      s->reset_enable = false;
>> +    s->quad_enable = false;
>>
>>      switch (get_man(s)) {
>>      case MAN_NUMONYX:
>> @@ -747,10 +767,20 @@ static void decode_new_cmd(Flash *s, uint32_t value)
>>
>>      case WRSR:
>>          if (s->write_enable) {
>> -            s->needed_bytes = 1;
>> +            switch (get_man(s)) {
>> +            case MAN_SPANSION:
>> +                s->needed_bytes = 2;
>> +                s->state = STATE_COLLECTING_DATA;
>> +                break;
>> +            case MAN_MACRONIX:
>> +                s->needed_bytes = 2;
>> +                s->state = STATE_COLLECTING_VAR_LEN_DATA;
>> +                break;
>
> ah. I am glad you address this topic because I am having a little issue
> with the mx25l25635e and the mx25l25635f.
>
> mx25l25635e is a macronix which supports the WRSR command with one byte 
> and the mx25l25635f needs two. The fun part is that they have the same
> JEDEC : 0xc22019.
>
> So not all macronix need 2 bytes. Should we add a flag for that ? 
>
I think this case should be partially handled with path 4 and this one.
The new state (patch 4) was introduced to allow use variable write cmds.
As in my case mx66u51235f allow write up to 2 bytes, but linux writes only one,
and that is allowed and works on real HW. From the datasheet mx25l25635f
should behave exactly same way.
In case you mention, the problem might pop up, when guest will try
to write 2 bytes to mx25l25635e. Real HW will treat second byte as new command,
but this model accept this situation.
Generally my approach and I think original author was to not emulate exactly
real hw behaviour but allow to model it in such way the quest can boot up and use it.
If we go witch very restricted command behaviour we will end up in unreadable code
(eg. newest Spansions/Cypress family does not support extended address register,
but such commands will work in this model). The question is if you really need to flag
and support this. If yes additional if is not a problem.
>
>> +            default:
>> +                s->needed_bytes = 1;
>> +                s->state = STATE_COLLECTING_DATA;
>> +            }
>>              s->pos = 0;
>> -            s->len = 0;
>> -            s->state = STATE_COLLECTING_DATA;
>>          }
>>          break;
>>
>> @@ -763,6 +793,9 @@ static void decode_new_cmd(Flash *s, uint32_t value)
>>
>>      case RDSR:
>>          s->data[0] = (!!s->write_enable) << 1;
>> +        if (get_man(s) == MAN_MACRONIX) {
>> +            s->data[0] |= (!!s->quad_enable) << 6;
>> +        }
>>          s->pos = 0;
>>          s->len = 1;
>>          s->state = STATE_READING_DATA;
>> @@ -789,6 +822,14 @@ static void decode_new_cmd(Flash *s, uint32_t value)
>>          s->state = STATE_READING_DATA;
>>          break;
>>
>> +    case RDCR:
>> +        s->data[0] = s->volatile_cfg & 0xFF;
>> +        s->data[0] |= (!!s->four_bytes_address_mode) << 5;
>> +        s->pos = 0;
>> +        s->len = 1;
>> +        s->state = STATE_READING_DATA;
>> +        break;
>> +
>>      case BULK_ERASE:
>>          if (s->write_enable) {
>>              DB_PRINT_L(0, "chip erase\n");
>> @@ -828,7 +869,7 @@ static void decode_new_cmd(Flash *s, uint32_t value)
>>          s->state = STATE_READING_DATA;
>>          break;
>>      case WNVCR:
>> -        if (s->write_enable) {
>> +        if (s->write_enable && get_man(s) == MAN_NUMONYX) {
>>              s->needed_bytes = 2;
>>              s->pos = 0;
>>              s->len = 0;
>> @@ -871,6 +912,24 @@ static void decode_new_cmd(Flash *s, uint32_t value)
>>              reset_memory(s);
>>          }
>>          break;
>> +    case RDCR_EQIO:
>> +        switch (get_man(s)) {
>> +        case MAN_SPANSION:
>> +            s->data[0] = (!!s->quad_enable) << 1;
>> +            s->pos = 0;
>> +            s->len = 1;
>> +            s->state = STATE_READING_DATA;
>> +            break;
>> +        case MAN_MACRONIX:
>> +            s->quad_enable = true;
>> +            break;
>> +        default:
>> +            break;
>> +        }
>> +        break;
>> +    case RSTQIO:
>> +        s->quad_enable = false;
>> +        break;
>>      default:
>>          qemu_log_mask(LOG_GUEST_ERROR, "M25P80: Unknown cmd %x\n", value);
>>          break;
>> @@ -999,7 +1058,7 @@ static Property m25p80_properties[] = {
>>
>>  static const VMStateDescription vmstate_m25p80 = {
>>      .name = "xilinx_spi",
>> -    .version_id = 2,
>> +    .version_id = 3,
>>      .minimum_version_id = 1,
>>      .pre_save = m25p80_pre_save,
>>      .fields = (VMStateField[]) {
>> @@ -1017,6 +1076,7 @@ static const VMStateDescription vmstate_m25p80 = {
>>          VMSTATE_UINT32_V(nonvolatile_cfg, Flash, 2),
>>          VMSTATE_UINT32_V(volatile_cfg, Flash, 2),
>>          VMSTATE_UINT32_V(enh_volatile_cfg, Flash, 2),
>> +        VMSTATE_BOOL_V(quad_enable, Flash, 3),
>>          VMSTATE_END_OF_LIST()
>>      }
>>  };
>>
>
>
>

^ permalink raw reply	[flat|nested] 28+ messages in thread

* Re: [Qemu-devel] [PATCH 3/9] m25p80: Allow more than four banks.
  2016-06-15 13:41 ` [Qemu-devel] [PATCH 3/9] m25p80: Allow more than four banks marcin.krzeminski
@ 2016-06-16  7:09   ` Cédric Le Goater
  0 siblings, 0 replies; 28+ messages in thread
From: Cédric Le Goater @ 2016-06-16  7:09 UTC (permalink / raw)
  To: marcin.krzeminski, qemu-devel
  Cc: crosthwaitepeter, pawel.lenkow, peter.maydell

On 06/15/2016 03:41 PM, marcin.krzeminski@nokia.com wrote:
> From: Marcin Krzeminski <marcin.krzeminski@nokia.com>
> 
> Allow to have more than four 16MiB regions for bigger flash devices.
> 
> Signed-off-by: Marcin Krzeminski <marcin.krzeminski@nokia.com>


Reviewed-by: Cédric Le Goater <clg@kaod.org>

> ---
>  hw/block/m25p80.c | 5 ++---
>  1 file changed, 2 insertions(+), 3 deletions(-)
> 
> diff --git a/hw/block/m25p80.c b/hw/block/m25p80.c
> index 342f7c9..6910c52 100644
> --- a/hw/block/m25p80.c
> +++ b/hw/block/m25p80.c
> @@ -129,7 +129,6 @@ typedef struct FlashPartInfo {
>  #define EVCFG_QUAD_IO_ENABLED (1 << 7)
>  #define NVCFG_4BYTE_ADDR_MASK (1 << 0)
>  #define NVCFG_LOWER_SEGMENT_MASK (1 << 1)
> -#define CFG_UPPER_128MB_SEG_ENABLED 0x3
> 
>  /* Numonyx (Micron) Flag Status Register macros */
>  #define FSR_4BYTE_ADDR_MODE_ENABLED 0x1
> @@ -545,7 +544,7 @@ static void complete_collecting_data(Flash *s)
>      }
> 
>      if (get_addr_length(s) == 3) {
> -        s->cur_addr += (s->ear & 0x3) * MAX_3BYTES_SIZE;
> +        s->cur_addr += s->ear * MAX_3BYTES_SIZE;
>      }
> 
>      s->state = STATE_IDLE;
> @@ -644,7 +643,7 @@ static void reset_memory(Flash *s)
>              s->four_bytes_address_mode = true;
>          }
>          if (!(s->nonvolatile_cfg & NVCFG_LOWER_SEGMENT_MASK)) {
> -            s->ear = CFG_UPPER_128MB_SEG_ENABLED;
> +            s->ear = s->size / MAX_3BYTES_SIZE - 1;
>          }
>          break;
>      default:
> 

^ permalink raw reply	[flat|nested] 28+ messages in thread

* Re: [Qemu-devel] [PATCH 4/9] m25p80: Introduce COLLECTING_VAR_LEN_DATA state.
  2016-06-15 13:41 ` [Qemu-devel] [PATCH 4/9] m25p80: Introduce COLLECTING_VAR_LEN_DATA state marcin.krzeminski
@ 2016-06-16  7:13   ` Cédric Le Goater
  2016-06-16  7:43     ` Krzeminski, Marcin (Nokia - PL/Wroclaw)
  0 siblings, 1 reply; 28+ messages in thread
From: Cédric Le Goater @ 2016-06-16  7:13 UTC (permalink / raw)
  To: marcin.krzeminski, qemu-devel
  Cc: crosthwaitepeter, pawel.lenkow, peter.maydell

On 06/15/2016 03:41 PM, marcin.krzeminski@nokia.com wrote:
> From: Marcin Krzeminski <marcin.krzeminski@nokia.com>
> 
> Some flash allows to stop read at any time.
> Allow framework to support this.
> 
> Signed-off-by: Marcin Krzeminski <marcin.krzeminski@nokia.com>
> ---
>  hw/block/m25p80.c | 5 +++++
>  1 file changed, 5 insertions(+)
> 
> diff --git a/hw/block/m25p80.c b/hw/block/m25p80.c
> index 6910c52..ca1f882 100644
> --- a/hw/block/m25p80.c
> +++ b/hw/block/m25p80.c
> @@ -330,6 +330,7 @@ typedef enum {
>      STATE_PAGE_PROGRAM,
>      STATE_READ,
>      STATE_COLLECTING_DATA,
> +    STATE_COLLECTING_VAR_LEN_DATA,
>      STATE_READING_DATA,
>  } CMDState;
> 
> @@ -872,6 +873,9 @@ static int m25p80_cs(SSISlave *ss, bool select)
>      Flash *s = M25P80(ss);
> 
>      if (select) {
> +        if (s->state == STATE_COLLECTING_VAR_LEN_DATA) {
> +            complete_collecting_data(s);
> +        }

So for example, we could trigger an erase without a completed address ? 

C. 

>          s->len = 0;
>          s->pos = 0;
>          s->state = STATE_IDLE;
> @@ -905,6 +909,7 @@ static uint32_t m25p80_transfer8(SSISlave *ss, uint32_t tx)
>          break;
> 
>      case STATE_COLLECTING_DATA:
> +    case STATE_COLLECTING_VAR_LEN_DATA:
>          s->data[s->len] = (uint8_t)tx;
>          s->len++;
> 

^ permalink raw reply	[flat|nested] 28+ messages in thread

* Re: [Qemu-devel] [PATCH 5/9] m25p80: Add additional flash commands:
  2016-06-15 13:41 ` [Qemu-devel] [PATCH 5/9] m25p80: Add additional flash commands: marcin.krzeminski
@ 2016-06-16  7:14   ` Cédric Le Goater
  0 siblings, 0 replies; 28+ messages in thread
From: Cédric Le Goater @ 2016-06-16  7:14 UTC (permalink / raw)
  To: marcin.krzeminski, qemu-devel
  Cc: crosthwaitepeter, pawel.lenkow, peter.maydell

On 06/15/2016 03:41 PM, marcin.krzeminski@nokia.com wrote:
> From: Marcin Krzeminski <marcin.krzeminski@nokia.com>
> 
> Page program 4byte/quad and erase 32K sectors 4 bytes.


Reviewed-by: Cédric Le Goater <clg@kaod.org>

C.

> 
> Signed-off-by: Marcin Krzeminski <marcin.krzeminski@nokia.com>
> ---
>  hw/block/m25p80.c | 9 +++++++++
>  1 file changed, 9 insertions(+)
> 
> diff --git a/hw/block/m25p80.c b/hw/block/m25p80.c
> index ca1f882..55b4377 100644
> --- a/hw/block/m25p80.c
> +++ b/hw/block/m25p80.c
> @@ -297,12 +297,14 @@ typedef enum {
> 
>      PP = 0x02,
>      PP4 = 0x12,
> +    PP4_4 = 0x3e,
>      DPP = 0xa2,
>      QPP = 0x32,
> 
>      ERASE_4K = 0x20,
>      ERASE4_4K = 0x21,
>      ERASE_32K = 0x52,
> +    ERASE4_32K = 0x5c,
>      ERASE_SECTOR = 0xd8,
>      ERASE4_SECTOR = 0xdc,
> 
> @@ -449,6 +451,7 @@ static void flash_erase(Flash *s, int offset, FlashCMD cmd)
>          capa_to_assert = ER_4K;
>          break;
>      case ERASE_32K:
> +    case ERASE4_32K:
>          len = 32 << 10;
>          capa_to_assert = ER_32K;
>          break;
> @@ -519,9 +522,11 @@ static inline int get_addr_length(Flash *s)
> 
>     switch (s->cmd_in_progress) {
>     case PP4:
> +   case PP4_4:
>     case READ4:
>     case QIOR4:
>     case ERASE4_4K:
> +   case ERASE4_32K:
>     case ERASE4_SECTOR:
>     case FAST_READ4:
>     case DOR4:
> @@ -555,6 +560,7 @@ static void complete_collecting_data(Flash *s)
>      case QPP:
>      case PP:
>      case PP4:
> +    case PP4_4:
>          s->state = STATE_PAGE_PROGRAM;
>          break;
>      case READ:
> @@ -574,6 +580,7 @@ static void complete_collecting_data(Flash *s)
>      case ERASE_4K:
>      case ERASE4_4K:
>      case ERASE_32K:
> +    case ERASE4_32K:
>      case ERASE_SECTOR:
>      case ERASE4_SECTOR:
>          flash_erase(s, s->cur_addr, s->cmd_in_progress);
> @@ -669,6 +676,7 @@ static void decode_new_cmd(Flash *s, uint32_t value)
>      case ERASE_4K:
>      case ERASE4_4K:
>      case ERASE_32K:
> +    case ERASE4_32K:
>      case ERASE_SECTOR:
>      case ERASE4_SECTOR:
>      case READ:
> @@ -677,6 +685,7 @@ static void decode_new_cmd(Flash *s, uint32_t value)
>      case QPP:
>      case PP:
>      case PP4:
> +    case PP4_4:
>          s->needed_bytes = get_addr_length(s);
>          s->pos = 0;
>          s->len = 0;
> 

^ permalink raw reply	[flat|nested] 28+ messages in thread

* Re: [Qemu-devel] [PATCH 8/9] m25p80: Fast read commands family changes.
  2016-06-15 13:41 ` [Qemu-devel] [PATCH 8/9] m25p80: Fast read commands family changes marcin.krzeminski
@ 2016-06-16  7:19   ` Cédric Le Goater
  2016-06-16  7:53     ` Krzeminski, Marcin (Nokia - PL/Wroclaw)
  0 siblings, 1 reply; 28+ messages in thread
From: Cédric Le Goater @ 2016-06-16  7:19 UTC (permalink / raw)
  To: marcin.krzeminski, qemu-devel
  Cc: crosthwaitepeter, pawel.lenkow, peter.maydell

On 06/15/2016 03:41 PM, marcin.krzeminski@nokia.com wrote:
> From: Marcin Krzeminski <marcin.krzeminski@nokia.com>
> 
> Add support for Spansion and Macronix flashes.
> Additionally Numonyx(Micron) move from default
> in fast read commands family.

maybe we could start adding sub routines to decode some of the 
larger commands, to ease reading. DIOR and QIOR are good candidates.

C. 

> Signed-off-by: Marcin Krzeminski <marcin.krzeminski@nokia.com>
> ---
>  hw/block/m25p80.c | 72 ++++++++++++++++++++++++++++++++++++++++++++++++-------
>  1 file changed, 64 insertions(+), 8 deletions(-)
> 
> diff --git a/hw/block/m25p80.c b/hw/block/m25p80.c
> index 21998db..7bc0e03 100644
> --- a/hw/block/m25p80.c
> +++ b/hw/block/m25p80.c
> @@ -758,9 +758,23 @@ static void decode_new_cmd(Flash *s, uint32_t value)
>      case QOR4:
>          s->needed_bytes = get_addr_length(s);
>          switch (get_man(s)) {
> +        /* Dummy cycles - modeled with bytes writes instead of bits */
>          case MAN_NUMONYX:
>              s->needed_bytes += extract32(s->volatile_cfg, 4, 4);
>              break;
> +        case MAN_MACRONIX:
> +            if (extract32(s->volatile_cfg, 6, 2) == 1) {
> +                s->needed_bytes += 6;
> +            } else {
> +                s->needed_bytes += 8;
> +            }
> +            break;
> +        case MAN_SPANSION:
> +            s->needed_bytes += extract32(s->spansion_cr2v,
> +                                        SPANSION_DUMMY_CLK_POS,
> +                                        SPANSION_DUMMY_CLK_LEN
> +                                        );
> +            break;
>          default:
>              break;
>          }
> @@ -771,15 +785,36 @@ static void decode_new_cmd(Flash *s, uint32_t value)
> 
>      case DIOR:
>      case DIOR4:
> +        s->needed_bytes = get_addr_length(s);
> +        /* Dummy cycles modeled with bytes writes instead of bits */
>          switch (get_man(s)) {
>          case MAN_WINBOND:
> +            s->needed_bytes += 4;
> +            break;
>          case MAN_SPANSION:
> -            s->needed_bytes = 4;
> +            s->needed_bytes += extract32(s->spansion_cr2v,
> +                                        SPANSION_DUMMY_CLK_POS,
> +                                        SPANSION_DUMMY_CLK_LEN
> +                                        );
>              break;
> -        default:
> -            s->needed_bytes = get_addr_length(s);
> -            /* Dummy cycles modeled with bytes writes instead of bits */
> +        case MAN_NUMONYX:
>              s->needed_bytes += extract32(s->volatile_cfg, 4, 4);
> +            break;
> +        case MAN_MACRONIX:
> +            switch (extract32(s->volatile_cfg, 6, 2)) {
> +            case 1:
> +                s->needed_bytes += 6;
> +                break;
> +            case 2:
> +                s->needed_bytes += 8;
> +                break;
> +            default:
> +                s->needed_bytes += 4;
> +                break;
> +            }
> +            break;
> +        default:
> +            break;
>          }
>          s->pos = 0;
>          s->len = 0;
> @@ -788,15 +823,36 @@ static void decode_new_cmd(Flash *s, uint32_t value)
> 
>      case QIOR:
>      case QIOR4:
> +        s->needed_bytes = get_addr_length(s);
> +        /* Dummy cycles modeled with bytes writes instead of bits */
>          switch (get_man(s)) {
>          case MAN_WINBOND:
> +            s->needed_bytes += 4;
> +            break;
>          case MAN_SPANSION:
> -            s->needed_bytes = 6;
> +            s->needed_bytes += extract32(s->spansion_cr2v,
> +                                        SPANSION_DUMMY_CLK_POS,
> +                                        SPANSION_DUMMY_CLK_LEN
> +                                        );
>              break;
> -        default:
> -            s->needed_bytes = get_addr_length(s);
> -            /* Dummy cycles modeled with bytes writes instead of bits */
> +        case MAN_NUMONYX:
>              s->needed_bytes += extract32(s->volatile_cfg, 4, 4);
> +            break;
> +        case MAN_MACRONIX:
> +            switch (extract32(s->volatile_cfg, 6, 2)) {
> +            case 1:
> +                s->needed_bytes += 4;
> +                break;
> +            case 2:
> +                s->needed_bytes += 8;
> +                break;
> +            default:
> +                s->needed_bytes += 6;
> +                break;
> +            }
> +            break;
> +        default:
> +            break;
>          }
>          s->pos = 0;
>          s->len = 0;
> 

^ permalink raw reply	[flat|nested] 28+ messages in thread

* Re: [Qemu-devel] [PATCH 9/9] m25p80: New flash devices.
  2016-06-15 13:41 ` [Qemu-devel] [PATCH 9/9] m25p80: New flash devices marcin.krzeminski
@ 2016-06-16  7:20   ` Cédric Le Goater
  0 siblings, 0 replies; 28+ messages in thread
From: Cédric Le Goater @ 2016-06-16  7:20 UTC (permalink / raw)
  To: marcin.krzeminski, qemu-devel
  Cc: crosthwaitepeter, pawel.lenkow, peter.maydell

On 06/15/2016 03:41 PM, marcin.krzeminski@nokia.com wrote:
> From: Marcin Krzeminski <marcin.krzeminski@nokia.com>
> 
> Macronix: mx66u51235f and mx66u1g45g
> Micron: mt25ql01g and mt25qu01g
> Spansion: s25fs512s and s70fs01gs
>
> Signed-off-by: Marcin Krzeminski <marcin.krzeminski@nokia.com>


Reviewed-by: Cédric Le Goater <clg@kaod.org>

> ---
>  hw/block/m25p80.c | 19 +++++++++++++------
>  1 file changed, 13 insertions(+), 6 deletions(-)
> 
> diff --git a/hw/block/m25p80.c b/hw/block/m25p80.c
> index 7bc0e03..e1a7698 100644
> --- a/hw/block/m25p80.c
> +++ b/hw/block/m25p80.c
> @@ -192,6 +192,8 @@ static const FlashPartInfo known_devices[] = {
>      { INFO("mx25l12855e", 0xc22618,      0,  64 << 10, 256, 0) },
>      { INFO("mx25l25635e", 0xc22019,      0,  64 << 10, 512, 0) },
>      { INFO("mx25l25655e", 0xc22619,      0,  64 << 10, 512, 0) },
> +    { INFO("mx66u51235f", 0xc2253a,      0,  64 << 10, 1024, ER_4K | ER_32K) },
> +    { INFO("mx66u1g45g",  0xc2253b,      0,  64 << 10, 2048, ER_4K | ER_32K) },
> 
>      /* Micron */
>      { INFO("n25q032a11",  0x20bb16,      0,  64 << 10,  64, ER_4K) },
> @@ -202,6 +204,11 @@ static const FlashPartInfo known_devices[] = {
>      { INFO("n25q128a13",  0x20ba18,      0,  64 << 10, 256, ER_4K) },
>      { INFO("n25q256a11",  0x20bb19,      0,  64 << 10, 512, ER_4K) },
>      { INFO("n25q256a13",  0x20ba19,      0,  64 << 10, 512, ER_4K) },
> +    { 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) },
> +    { INFO("mt25ql01g",   0x20ba21,      0,  64 << 10, 2048, ER_4K) },
> +    { INFO("mt25qu01g",   0x20bb21,      0,  64 << 10, 2048, ER_4K) },
> 
>      /* Spansion -- single (large) sector size only, at least
>       * for the chips listed here (without boot sectors).
> @@ -210,8 +217,8 @@ static const FlashPartInfo known_devices[] = {
>      { INFO("s25sl064p",   0x010216, 0x4d00,  64 << 10, 128, ER_4K) },
>      { INFO("s25fl256s0",  0x010219, 0x4d00, 256 << 10, 128, 0) },
>      { INFO("s25fl256s1",  0x010219, 0x4d01,  64 << 10, 512, 0) },
> -    { INFO("s25fl512s",   0x010220, 0x4d00, 256 << 10, 256, 0) },
> -    { INFO("s70fl01gs",   0x010221, 0x4d00, 256 << 10, 256, 0) },
> +    { INFO6("s25fl512s",  0x010220, 0x4d0080, 256 << 10, 256, 0) },
> +    { INFO6("s70fl01gs",  0x010221, 0x4d0080, 256 << 10, 512, 0) },
>      { INFO("s25sl12800",  0x012018, 0x0300, 256 << 10,  64, 0) },
>      { INFO("s25sl12801",  0x012018, 0x0301,  64 << 10, 256, 0) },
>      { INFO("s25fl129p0",  0x012018, 0x4d00, 256 << 10,  64, 0) },
> @@ -224,6 +231,10 @@ static const FlashPartInfo known_devices[] = {
>      { INFO("s25fl016k",   0xef4015,      0,  64 << 10,  32, ER_4K | ER_32K) },
>      { INFO("s25fl064k",   0xef4017,      0,  64 << 10, 128, ER_4K | ER_32K) },
> 
> +    /* Spansion --  boot sectors support  */
> +    { INFO6("s25fs512s",    0x010220, 0x4d0081, 256 << 10, 256, 0) },
> +    { INFO6("s70fs01gs",    0x010221, 0x4d0081, 256 << 10, 512, 0) },
> +
>      /* SST -- large erase sizes are "overlays", "sectors" are 4<< 10 */
>      { INFO("sst25vf040b", 0xbf258d,      0,  64 << 10,   8, ER_4K) },
>      { INFO("sst25vf080b", 0xbf258e,      0,  64 << 10,  16, ER_4K) },
> @@ -274,10 +285,6 @@ static const FlashPartInfo known_devices[] = {
>      { INFO("w25q80",      0xef5014,      0,  64 << 10,  16, ER_4K) },
>      { INFO("w25q80bl",    0xef4014,      0,  64 << 10,  16, ER_4K) },
>      { INFO("w25q256",     0xef4019,      0,  64 << 10, 512, ER_4K) },
> -
> -    { 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 {
> 

^ permalink raw reply	[flat|nested] 28+ messages in thread

* Re: [Qemu-devel] [PATCH 7/9] m25p80: Introduce configuration registers.
  2016-06-15 13:41 ` [Qemu-devel] [PATCH 7/9] m25p80: Introduce configuration registers marcin.krzeminski
@ 2016-06-16  7:24   ` Cédric Le Goater
  2016-06-16  7:52     ` Krzeminski, Marcin (Nokia - PL/Wroclaw)
  0 siblings, 1 reply; 28+ messages in thread
From: Cédric Le Goater @ 2016-06-16  7:24 UTC (permalink / raw)
  To: marcin.krzeminski, qemu-devel
  Cc: crosthwaitepeter, pawel.lenkow, peter.maydell

On 06/15/2016 03:41 PM, marcin.krzeminski@nokia.com wrote:
> From: Marcin Krzeminski <marcin.krzeminski@nokia.com>
> 
> Configuration registers for Spansion and Macronix devices.
> 
> Signed-off-by: Marcin Krzeminski <marcin.krzeminski@nokia.com>


Reviewed-by: Cédric Le Goater <clg@kaod.org>

I don't think we can define a property array. can we ? 

> ---
>  hw/block/m25p80.c | 47 +++++++++++++++++++++++++++++++++++++++++++++++
>  1 file changed, 47 insertions(+)
> 
> diff --git a/hw/block/m25p80.c b/hw/block/m25p80.c
> index d1c4d46..21998db 100644
> --- a/hw/block/m25p80.c
> +++ b/hw/block/m25p80.c
> @@ -134,6 +134,14 @@ typedef struct FlashPartInfo {
>  #define FSR_4BYTE_ADDR_MODE_ENABLED 0x1
>  #define FSR_FLASH_READY (1 << 7)
> 
> +/* Spansion configuration registers macros. */
> +#define SPANSION_QUAD_CFG_POS 0
> +#define SPANSION_QUAD_CFG_LEN 1
> +#define SPANSION_DUMMY_CLK_POS 0
> +#define SPANSION_DUMMY_CLK_LEN 4
> +#define SPANSION_ADDR_LEN_POS 7
> +#define SPANSION_ADDR_LEN_LEN 1
> +
>  static const FlashPartInfo known_devices[] = {
>      /* Atmel -- some are (confusingly) marketed as "DataFlash" */
>      { INFO("at25fs010",   0x1f6601,      0,  32 << 10,   4, ER_4K) },
> @@ -369,8 +377,18 @@ typedef struct Flash {
>      uint8_t cmd_in_progress;
>      uint64_t cur_addr;
>      uint32_t nonvolatile_cfg;
> +    /* Configuration register for Macronix */
>      uint32_t volatile_cfg;
>      uint32_t enh_volatile_cfg;
> +    /* Spansion cfg registers. */
> +    uint8_t spansion_cr1nv;
> +    uint8_t spansion_cr2nv;
> +    uint8_t spansion_cr3nv;
> +    uint8_t spansion_cr4nv;
> +    uint8_t spansion_cr1v;
> +    uint8_t spansion_cr2v;
> +    uint8_t spansion_cr3v;
> +    uint8_t spansion_cr4v;
>      bool write_enable;
>      bool four_bytes_address_mode;
>      bool reset_enable;
> @@ -601,6 +619,9 @@ static void complete_collecting_data(Flash *s)
>              break;
>          case MAN_MACRONIX:
>              s->quad_enable = extract32(s->data[0], 6, 1);
> +            if (s->len > 1) {
> +                s->four_bytes_address_mode = extract32(s->data[1], 5, 1);
> +            }
>              break;
>          default:
>              break;
> @@ -674,6 +695,23 @@ static void reset_memory(Flash *s)
>              s->ear = s->size / MAX_3BYTES_SIZE - 1;
>          }
>          break;
> +    case MAN_MACRONIX:
> +        s->volatile_cfg = 0x7;
> +        break;
> +    case MAN_SPANSION:
> +        s->spansion_cr1v = s->spansion_cr1nv;
> +        s->spansion_cr2v = s->spansion_cr2nv;
> +        s->spansion_cr3v = s->spansion_cr3nv;
> +        s->spansion_cr4v = s->spansion_cr4nv;
> +        s->quad_enable = extract32(s->spansion_cr1v,
> +                                   SPANSION_QUAD_CFG_POS,
> +                                   SPANSION_QUAD_CFG_LEN
> +                                   );
> +        s->four_bytes_address_mode = extract32(s->spansion_cr2v,
> +                SPANSION_ADDR_LEN_POS,
> +                SPANSION_ADDR_LEN_LEN
> +                );
> +        break;
>      default:
>          break;
>      }
> @@ -1052,7 +1090,12 @@ static void m25p80_pre_save(void *opaque)
>  }
> 
>  static Property m25p80_properties[] = {
> +    /* This is default value for Micron flash */
>      DEFINE_PROP_UINT32("nonvolatile-cfg", Flash, nonvolatile_cfg, 0x8FFF),
> +    DEFINE_PROP_UINT8("spansion-cr1nv", Flash, spansion_cr1nv, 0x0),
> +    DEFINE_PROP_UINT8("spansion-cr2nv", Flash, spansion_cr2nv, 0x8),
> +    DEFINE_PROP_UINT8("spansion-cr3nv", Flash, spansion_cr3nv, 0x2),
> +    DEFINE_PROP_UINT8("spansion-cr4nv", Flash, spansion_cr4nv, 0x10),
>      DEFINE_PROP_END_OF_LIST(),
>  };
> 
> @@ -1077,6 +1120,10 @@ static const VMStateDescription vmstate_m25p80 = {
>          VMSTATE_UINT32_V(volatile_cfg, Flash, 2),
>          VMSTATE_UINT32_V(enh_volatile_cfg, Flash, 2),
>          VMSTATE_BOOL_V(quad_enable, Flash, 3),
> +        VMSTATE_UINT8_V(spansion_cr1nv, Flash, 3),
> +        VMSTATE_UINT8_V(spansion_cr2nv, Flash, 3),
> +        VMSTATE_UINT8_V(spansion_cr3nv, Flash, 3),
> +        VMSTATE_UINT8_V(spansion_cr4nv, Flash, 3),
>          VMSTATE_END_OF_LIST()
>      }
>  };
> 

^ permalink raw reply	[flat|nested] 28+ messages in thread

* Re: [Qemu-devel] [PATCH 4/9] m25p80: Introduce COLLECTING_VAR_LEN_DATA state.
  2016-06-16  7:13   ` Cédric Le Goater
@ 2016-06-16  7:43     ` Krzeminski, Marcin (Nokia - PL/Wroclaw)
  2016-06-16  8:13       ` Cédric Le Goater
  0 siblings, 1 reply; 28+ messages in thread
From: Krzeminski, Marcin (Nokia - PL/Wroclaw) @ 2016-06-16  7:43 UTC (permalink / raw)
  To: Cédric Le Goater, qemu-devel@nongnu.org
  Cc: crosthwaitepeter@gmail.com, pawel.lenkow@itlen.com,
	peter.maydell@linaro.org



> -----Original Message-----
> From: Cédric Le Goater [mailto:clg@kaod.org]
> Sent: Thursday, June 16, 2016 9:14 AM
> To: Krzeminski, Marcin (Nokia - PL/Wroclaw)
> <marcin.krzeminski@nokia.com>; qemu-devel@nongnu.org
> Cc: crosthwaitepeter@gmail.com; pawel.lenkow@itlen.com;
> peter.maydell@linaro.org
> Subject: Re: [PATCH 4/9] m25p80: Introduce COLLECTING_VAR_LEN_DATA
> state.
> 
> On 06/15/2016 03:41 PM, marcin.krzeminski@nokia.com wrote:
> > From: Marcin Krzeminski <marcin.krzeminski@nokia.com>
> >
> > Some flash allows to stop read at any time.
> > Allow framework to support this.
> >
> > Signed-off-by: Marcin Krzeminski <marcin.krzeminski@nokia.com>
> > ---
> >  hw/block/m25p80.c | 5 +++++
> >  1 file changed, 5 insertions(+)
> >
> > diff --git a/hw/block/m25p80.c b/hw/block/m25p80.c index
> > 6910c52..ca1f882 100644
> > --- a/hw/block/m25p80.c
> > +++ b/hw/block/m25p80.c
> > @@ -330,6 +330,7 @@ typedef enum {
> >      STATE_PAGE_PROGRAM,
> >      STATE_READ,
> >      STATE_COLLECTING_DATA,
> > +    STATE_COLLECTING_VAR_LEN_DATA,
> >      STATE_READING_DATA,
> >  } CMDState;
> >
> > @@ -872,6 +873,9 @@ static int m25p80_cs(SSISlave *ss, bool select)
> >      Flash *s = M25P80(ss);
> >
> >      if (select) {
> > +        if (s->state == STATE_COLLECTING_VAR_LEN_DATA) {
> > +            complete_collecting_data(s);
> > +        }
> 
> So for example, we could trigger an erase without a completed address ?
> 
> C.
Yes if you explicitly want that. This is new state and you need to go
into it in decode_new_cmd.
For now this is only used in WRSR command only.

Thanks,
Marcin
> 
> >          s->len = 0;
> >          s->pos = 0;
> >          s->state = STATE_IDLE;
> > @@ -905,6 +909,7 @@ static uint32_t m25p80_transfer8(SSISlave *ss,
> uint32_t tx)
> >          break;
> >
> >      case STATE_COLLECTING_DATA:
> > +    case STATE_COLLECTING_VAR_LEN_DATA:
> >          s->data[s->len] = (uint8_t)tx;
> >          s->len++;
> >

^ permalink raw reply	[flat|nested] 28+ messages in thread

* Re: [Qemu-devel] [PATCH 7/9] m25p80: Introduce configuration registers.
  2016-06-16  7:24   ` Cédric Le Goater
@ 2016-06-16  7:52     ` Krzeminski, Marcin (Nokia - PL/Wroclaw)
  2016-06-16  8:05       ` Cédric Le Goater
  0 siblings, 1 reply; 28+ messages in thread
From: Krzeminski, Marcin (Nokia - PL/Wroclaw) @ 2016-06-16  7:52 UTC (permalink / raw)
  To: Cédric Le Goater, qemu-devel@nongnu.org
  Cc: crosthwaitepeter@gmail.com, pawel.lenkow@itlen.com,
	peter.maydell@linaro.org, rfsw-patches@mlist.nsn-intra.net



> -----Original Message-----
> From: Cédric Le Goater [mailto:clg@kaod.org]
> Sent: Thursday, June 16, 2016 9:25 AM
> To: Krzeminski, Marcin (Nokia - PL/Wroclaw)
> <marcin.krzeminski@nokia.com>; qemu-devel@nongnu.org
> Cc: crosthwaitepeter@gmail.com; pawel.lenkow@itlen.com;
> peter.maydell@linaro.org
> Subject: Re: [PATCH 7/9] m25p80: Introduce configuration registers.
> 
> On 06/15/2016 03:41 PM, marcin.krzeminski@nokia.com wrote:
> > From: Marcin Krzeminski <marcin.krzeminski@nokia.com>
> >
> > Configuration registers for Spansion and Macronix devices.
> >
> > Signed-off-by: Marcin Krzeminski <marcin.krzeminski@nokia.com>
> 
> 
> Reviewed-by: Cédric Le Goater <clg@kaod.org>
> 
> I don't think we can define a property array. can we ?
You mean Spansion config registers?
If yes 4 variables are used mostly to allow user easy setting of initial values.
It could be changed to one uint32_t or table but I do not know what
We benefit from that?

Thanks,
Marcin
> 
> > ---
> >  hw/block/m25p80.c | 47
> > +++++++++++++++++++++++++++++++++++++++++++++++
> >  1 file changed, 47 insertions(+)
> >
> > diff --git a/hw/block/m25p80.c b/hw/block/m25p80.c index
> > d1c4d46..21998db 100644
> > --- a/hw/block/m25p80.c
> > +++ b/hw/block/m25p80.c
> > @@ -134,6 +134,14 @@ typedef struct FlashPartInfo {  #define
> > FSR_4BYTE_ADDR_MODE_ENABLED 0x1  #define FSR_FLASH_READY (1 <<
> 7)
> >
> > +/* Spansion configuration registers macros. */ #define
> > +SPANSION_QUAD_CFG_POS 0 #define SPANSION_QUAD_CFG_LEN 1
> #define
> > +SPANSION_DUMMY_CLK_POS 0 #define SPANSION_DUMMY_CLK_LEN 4
> #define
> > +SPANSION_ADDR_LEN_POS 7 #define SPANSION_ADDR_LEN_LEN 1
> > +
> >  static const FlashPartInfo known_devices[] = {
> >      /* Atmel -- some are (confusingly) marketed as "DataFlash" */
> >      { INFO("at25fs010",   0x1f6601,      0,  32 << 10,   4, ER_4K) },
> > @@ -369,8 +377,18 @@ typedef struct Flash {
> >      uint8_t cmd_in_progress;
> >      uint64_t cur_addr;
> >      uint32_t nonvolatile_cfg;
> > +    /* Configuration register for Macronix */
> >      uint32_t volatile_cfg;
> >      uint32_t enh_volatile_cfg;
> > +    /* Spansion cfg registers. */
> > +    uint8_t spansion_cr1nv;
> > +    uint8_t spansion_cr2nv;
> > +    uint8_t spansion_cr3nv;
> > +    uint8_t spansion_cr4nv;
> > +    uint8_t spansion_cr1v;
> > +    uint8_t spansion_cr2v;
> > +    uint8_t spansion_cr3v;
> > +    uint8_t spansion_cr4v;
> >      bool write_enable;
> >      bool four_bytes_address_mode;
> >      bool reset_enable;
> > @@ -601,6 +619,9 @@ static void complete_collecting_data(Flash *s)
> >              break;
> >          case MAN_MACRONIX:
> >              s->quad_enable = extract32(s->data[0], 6, 1);
> > +            if (s->len > 1) {
> > +                s->four_bytes_address_mode = extract32(s->data[1], 5, 1);
> > +            }
> >              break;
> >          default:
> >              break;
> > @@ -674,6 +695,23 @@ static void reset_memory(Flash *s)
> >              s->ear = s->size / MAX_3BYTES_SIZE - 1;
> >          }
> >          break;
> > +    case MAN_MACRONIX:
> > +        s->volatile_cfg = 0x7;
> > +        break;
> > +    case MAN_SPANSION:
> > +        s->spansion_cr1v = s->spansion_cr1nv;
> > +        s->spansion_cr2v = s->spansion_cr2nv;
> > +        s->spansion_cr3v = s->spansion_cr3nv;
> > +        s->spansion_cr4v = s->spansion_cr4nv;
> > +        s->quad_enable = extract32(s->spansion_cr1v,
> > +                                   SPANSION_QUAD_CFG_POS,
> > +                                   SPANSION_QUAD_CFG_LEN
> > +                                   );
> > +        s->four_bytes_address_mode = extract32(s->spansion_cr2v,
> > +                SPANSION_ADDR_LEN_POS,
> > +                SPANSION_ADDR_LEN_LEN
> > +                );
> > +        break;
> >      default:
> >          break;
> >      }
> > @@ -1052,7 +1090,12 @@ static void m25p80_pre_save(void *opaque)  }
> >
> >  static Property m25p80_properties[] = {
> > +    /* This is default value for Micron flash */
> >      DEFINE_PROP_UINT32("nonvolatile-cfg", Flash, nonvolatile_cfg,
> > 0x8FFF),
> > +    DEFINE_PROP_UINT8("spansion-cr1nv", Flash, spansion_cr1nv, 0x0),
> > +    DEFINE_PROP_UINT8("spansion-cr2nv", Flash, spansion_cr2nv, 0x8),
> > +    DEFINE_PROP_UINT8("spansion-cr3nv", Flash, spansion_cr3nv, 0x2),
> > +    DEFINE_PROP_UINT8("spansion-cr4nv", Flash, spansion_cr4nv, 0x10),
> >      DEFINE_PROP_END_OF_LIST(),
> >  };
> >
> > @@ -1077,6 +1120,10 @@ static const VMStateDescription
> vmstate_m25p80 = {
> >          VMSTATE_UINT32_V(volatile_cfg, Flash, 2),
> >          VMSTATE_UINT32_V(enh_volatile_cfg, Flash, 2),
> >          VMSTATE_BOOL_V(quad_enable, Flash, 3),
> > +        VMSTATE_UINT8_V(spansion_cr1nv, Flash, 3),
> > +        VMSTATE_UINT8_V(spansion_cr2nv, Flash, 3),
> > +        VMSTATE_UINT8_V(spansion_cr3nv, Flash, 3),
> > +        VMSTATE_UINT8_V(spansion_cr4nv, Flash, 3),
> >          VMSTATE_END_OF_LIST()
> >      }
> >  };
> >

^ permalink raw reply	[flat|nested] 28+ messages in thread

* Re: [Qemu-devel] [PATCH 8/9] m25p80: Fast read commands family changes.
  2016-06-16  7:19   ` Cédric Le Goater
@ 2016-06-16  7:53     ` Krzeminski, Marcin (Nokia - PL/Wroclaw)
  0 siblings, 0 replies; 28+ messages in thread
From: Krzeminski, Marcin (Nokia - PL/Wroclaw) @ 2016-06-16  7:53 UTC (permalink / raw)
  To: Cédric Le Goater, qemu-devel@nongnu.org
  Cc: crosthwaitepeter@gmail.com, pawel.lenkow@itlen.com,
	peter.maydell@linaro.org, rfsw-patches@mlist.nsn-intra.net



> -----Original Message-----
> From: Cédric Le Goater [mailto:clg@kaod.org]
> Sent: Thursday, June 16, 2016 9:20 AM
> To: Krzeminski, Marcin (Nokia - PL/Wroclaw)
> <marcin.krzeminski@nokia.com>; qemu-devel@nongnu.org
> Cc: crosthwaitepeter@gmail.com; pawel.lenkow@itlen.com;
> peter.maydell@linaro.org
> Subject: Re: [PATCH 8/9] m25p80: Fast read commands family changes.
> 
> On 06/15/2016 03:41 PM, marcin.krzeminski@nokia.com wrote:
> > From: Marcin Krzeminski <marcin.krzeminski@nokia.com>
> >
> > Add support for Spansion and Macronix flashes.
> > Additionally Numonyx(Micron) move from default in fast read commands
> > family.
> 
> maybe we could start adding sub routines to decode some of the larger
> commands, to ease reading. DIOR and QIOR are good candidates.
> 
> C.
Agree. I will put it in V2.

Thanks,
Marcin
> 
> > Signed-off-by: Marcin Krzeminski <marcin.krzeminski@nokia.com>
> > ---
> >  hw/block/m25p80.c | 72
> > ++++++++++++++++++++++++++++++++++++++++++++++++-------
> >  1 file changed, 64 insertions(+), 8 deletions(-)
> >
> > diff --git a/hw/block/m25p80.c b/hw/block/m25p80.c index
> > 21998db..7bc0e03 100644
> > --- a/hw/block/m25p80.c
> > +++ b/hw/block/m25p80.c
> > @@ -758,9 +758,23 @@ static void decode_new_cmd(Flash *s, uint32_t
> value)
> >      case QOR4:
> >          s->needed_bytes = get_addr_length(s);
> >          switch (get_man(s)) {
> > +        /* Dummy cycles - modeled with bytes writes instead of bits
> > + */
> >          case MAN_NUMONYX:
> >              s->needed_bytes += extract32(s->volatile_cfg, 4, 4);
> >              break;
> > +        case MAN_MACRONIX:
> > +            if (extract32(s->volatile_cfg, 6, 2) == 1) {
> > +                s->needed_bytes += 6;
> > +            } else {
> > +                s->needed_bytes += 8;
> > +            }
> > +            break;
> > +        case MAN_SPANSION:
> > +            s->needed_bytes += extract32(s->spansion_cr2v,
> > +                                        SPANSION_DUMMY_CLK_POS,
> > +                                        SPANSION_DUMMY_CLK_LEN
> > +                                        );
> > +            break;
> >          default:
> >              break;
> >          }
> > @@ -771,15 +785,36 @@ static void decode_new_cmd(Flash *s, uint32_t
> > value)
> >
> >      case DIOR:
> >      case DIOR4:
> > +        s->needed_bytes = get_addr_length(s);
> > +        /* Dummy cycles modeled with bytes writes instead of bits */
> >          switch (get_man(s)) {
> >          case MAN_WINBOND:
> > +            s->needed_bytes += 4;
> > +            break;
> >          case MAN_SPANSION:
> > -            s->needed_bytes = 4;
> > +            s->needed_bytes += extract32(s->spansion_cr2v,
> > +                                        SPANSION_DUMMY_CLK_POS,
> > +                                        SPANSION_DUMMY_CLK_LEN
> > +                                        );
> >              break;
> > -        default:
> > -            s->needed_bytes = get_addr_length(s);
> > -            /* Dummy cycles modeled with bytes writes instead of bits */
> > +        case MAN_NUMONYX:
> >              s->needed_bytes += extract32(s->volatile_cfg, 4, 4);
> > +            break;
> > +        case MAN_MACRONIX:
> > +            switch (extract32(s->volatile_cfg, 6, 2)) {
> > +            case 1:
> > +                s->needed_bytes += 6;
> > +                break;
> > +            case 2:
> > +                s->needed_bytes += 8;
> > +                break;
> > +            default:
> > +                s->needed_bytes += 4;
> > +                break;
> > +            }
> > +            break;
> > +        default:
> > +            break;
> >          }
> >          s->pos = 0;
> >          s->len = 0;
> > @@ -788,15 +823,36 @@ static void decode_new_cmd(Flash *s, uint32_t
> > value)
> >
> >      case QIOR:
> >      case QIOR4:
> > +        s->needed_bytes = get_addr_length(s);
> > +        /* Dummy cycles modeled with bytes writes instead of bits */
> >          switch (get_man(s)) {
> >          case MAN_WINBOND:
> > +            s->needed_bytes += 4;
> > +            break;
> >          case MAN_SPANSION:
> > -            s->needed_bytes = 6;
> > +            s->needed_bytes += extract32(s->spansion_cr2v,
> > +                                        SPANSION_DUMMY_CLK_POS,
> > +                                        SPANSION_DUMMY_CLK_LEN
> > +                                        );
> >              break;
> > -        default:
> > -            s->needed_bytes = get_addr_length(s);
> > -            /* Dummy cycles modeled with bytes writes instead of bits */
> > +        case MAN_NUMONYX:
> >              s->needed_bytes += extract32(s->volatile_cfg, 4, 4);
> > +            break;
> > +        case MAN_MACRONIX:
> > +            switch (extract32(s->volatile_cfg, 6, 2)) {
> > +            case 1:
> > +                s->needed_bytes += 4;
> > +                break;
> > +            case 2:
> > +                s->needed_bytes += 8;
> > +                break;
> > +            default:
> > +                s->needed_bytes += 6;
> > +                break;
> > +            }
> > +            break;
> > +        default:
> > +            break;
> >          }
> >          s->pos = 0;
> >          s->len = 0;
> >

^ permalink raw reply	[flat|nested] 28+ messages in thread

* Re: [Qemu-devel] [PATCH 7/9] m25p80: Introduce configuration registers.
  2016-06-16  7:52     ` Krzeminski, Marcin (Nokia - PL/Wroclaw)
@ 2016-06-16  8:05       ` Cédric Le Goater
  2016-06-17 10:31         ` Krzeminski, Marcin (Nokia - PL/Wroclaw)
  0 siblings, 1 reply; 28+ messages in thread
From: Cédric Le Goater @ 2016-06-16  8:05 UTC (permalink / raw)
  To: Krzeminski, Marcin (Nokia - PL/Wroclaw), qemu-devel@nongnu.org
  Cc: crosthwaitepeter@gmail.com, pawel.lenkow@itlen.com,
	peter.maydell@linaro.org

On 06/16/2016 09:52 AM, Krzeminski, Marcin (Nokia - PL/Wroclaw) wrote:
> 
> 
>> -----Original Message-----
>> From: Cédric Le Goater [mailto:clg@kaod.org]
>> Sent: Thursday, June 16, 2016 9:25 AM
>> To: Krzeminski, Marcin (Nokia - PL/Wroclaw)
>> <marcin.krzeminski@nokia.com>; qemu-devel@nongnu.org
>> Cc: crosthwaitepeter@gmail.com; pawel.lenkow@itlen.com;
>> peter.maydell@linaro.org
>> Subject: Re: [PATCH 7/9] m25p80: Introduce configuration registers.
>>
>> On 06/15/2016 03:41 PM, marcin.krzeminski@nokia.com wrote:
>>> From: Marcin Krzeminski <marcin.krzeminski@nokia.com>
>>>
>>> Configuration registers for Spansion and Macronix devices.
>>>
>>> Signed-off-by: Marcin Krzeminski <marcin.krzeminski@nokia.com>
>>
>>
>> Reviewed-by: Cédric Le Goater <clg@kaod.org>
>>
>> I don't think we can define a property array. can we ?
> You mean Spansion config registers?
> If yes 4 variables are used mostly to allow user easy setting of initial values.
> It could be changed to one uint32_t or table but I do not know what
> We benefit from that?

We would just reduce the number of 'spansion_cr*'. This is minor.

C.


> 
> Thanks,
> Marcin
>>
>>> ---
>>>  hw/block/m25p80.c | 47
>>> +++++++++++++++++++++++++++++++++++++++++++++++
>>>  1 file changed, 47 insertions(+)
>>>
>>> diff --git a/hw/block/m25p80.c b/hw/block/m25p80.c index
>>> d1c4d46..21998db 100644
>>> --- a/hw/block/m25p80.c
>>> +++ b/hw/block/m25p80.c
>>> @@ -134,6 +134,14 @@ typedef struct FlashPartInfo {  #define
>>> FSR_4BYTE_ADDR_MODE_ENABLED 0x1  #define FSR_FLASH_READY (1 <<
>> 7)
>>>
>>> +/* Spansion configuration registers macros. */ #define
>>> +SPANSION_QUAD_CFG_POS 0 #define SPANSION_QUAD_CFG_LEN 1
>> #define
>>> +SPANSION_DUMMY_CLK_POS 0 #define SPANSION_DUMMY_CLK_LEN 4
>> #define
>>> +SPANSION_ADDR_LEN_POS 7 #define SPANSION_ADDR_LEN_LEN 1
>>> +
>>>  static const FlashPartInfo known_devices[] = {
>>>      /* Atmel -- some are (confusingly) marketed as "DataFlash" */
>>>      { INFO("at25fs010",   0x1f6601,      0,  32 << 10,   4, ER_4K) },
>>> @@ -369,8 +377,18 @@ typedef struct Flash {
>>>      uint8_t cmd_in_progress;
>>>      uint64_t cur_addr;
>>>      uint32_t nonvolatile_cfg;
>>> +    /* Configuration register for Macronix */
>>>      uint32_t volatile_cfg;
>>>      uint32_t enh_volatile_cfg;
>>> +    /* Spansion cfg registers. */
>>> +    uint8_t spansion_cr1nv;
>>> +    uint8_t spansion_cr2nv;
>>> +    uint8_t spansion_cr3nv;
>>> +    uint8_t spansion_cr4nv;
>>> +    uint8_t spansion_cr1v;
>>> +    uint8_t spansion_cr2v;
>>> +    uint8_t spansion_cr3v;
>>> +    uint8_t spansion_cr4v;
>>>      bool write_enable;
>>>      bool four_bytes_address_mode;
>>>      bool reset_enable;
>>> @@ -601,6 +619,9 @@ static void complete_collecting_data(Flash *s)
>>>              break;
>>>          case MAN_MACRONIX:
>>>              s->quad_enable = extract32(s->data[0], 6, 1);
>>> +            if (s->len > 1) {
>>> +                s->four_bytes_address_mode = extract32(s->data[1], 5, 1);
>>> +            }
>>>              break;
>>>          default:
>>>              break;
>>> @@ -674,6 +695,23 @@ static void reset_memory(Flash *s)
>>>              s->ear = s->size / MAX_3BYTES_SIZE - 1;
>>>          }
>>>          break;
>>> +    case MAN_MACRONIX:
>>> +        s->volatile_cfg = 0x7;
>>> +        break;
>>> +    case MAN_SPANSION:
>>> +        s->spansion_cr1v = s->spansion_cr1nv;
>>> +        s->spansion_cr2v = s->spansion_cr2nv;
>>> +        s->spansion_cr3v = s->spansion_cr3nv;
>>> +        s->spansion_cr4v = s->spansion_cr4nv;
>>> +        s->quad_enable = extract32(s->spansion_cr1v,
>>> +                                   SPANSION_QUAD_CFG_POS,
>>> +                                   SPANSION_QUAD_CFG_LEN
>>> +                                   );
>>> +        s->four_bytes_address_mode = extract32(s->spansion_cr2v,
>>> +                SPANSION_ADDR_LEN_POS,
>>> +                SPANSION_ADDR_LEN_LEN
>>> +                );
>>> +        break;
>>>      default:
>>>          break;
>>>      }
>>> @@ -1052,7 +1090,12 @@ static void m25p80_pre_save(void *opaque)  }
>>>
>>>  static Property m25p80_properties[] = {
>>> +    /* This is default value for Micron flash */
>>>      DEFINE_PROP_UINT32("nonvolatile-cfg", Flash, nonvolatile_cfg,
>>> 0x8FFF),
>>> +    DEFINE_PROP_UINT8("spansion-cr1nv", Flash, spansion_cr1nv, 0x0),
>>> +    DEFINE_PROP_UINT8("spansion-cr2nv", Flash, spansion_cr2nv, 0x8),
>>> +    DEFINE_PROP_UINT8("spansion-cr3nv", Flash, spansion_cr3nv, 0x2),
>>> +    DEFINE_PROP_UINT8("spansion-cr4nv", Flash, spansion_cr4nv, 0x10),
>>>      DEFINE_PROP_END_OF_LIST(),
>>>  };
>>>
>>> @@ -1077,6 +1120,10 @@ static const VMStateDescription
>> vmstate_m25p80 = {
>>>          VMSTATE_UINT32_V(volatile_cfg, Flash, 2),
>>>          VMSTATE_UINT32_V(enh_volatile_cfg, Flash, 2),
>>>          VMSTATE_BOOL_V(quad_enable, Flash, 3),
>>> +        VMSTATE_UINT8_V(spansion_cr1nv, Flash, 3),
>>> +        VMSTATE_UINT8_V(spansion_cr2nv, Flash, 3),
>>> +        VMSTATE_UINT8_V(spansion_cr3nv, Flash, 3),
>>> +        VMSTATE_UINT8_V(spansion_cr4nv, Flash, 3),
>>>          VMSTATE_END_OF_LIST()
>>>      }
>>>  };
>>>
> 

^ permalink raw reply	[flat|nested] 28+ messages in thread

* Re: [Qemu-devel] [PATCH 4/9] m25p80: Introduce COLLECTING_VAR_LEN_DATA state.
  2016-06-16  7:43     ` Krzeminski, Marcin (Nokia - PL/Wroclaw)
@ 2016-06-16  8:13       ` Cédric Le Goater
  0 siblings, 0 replies; 28+ messages in thread
From: Cédric Le Goater @ 2016-06-16  8:13 UTC (permalink / raw)
  To: Krzeminski, Marcin (Nokia - PL/Wroclaw), qemu-devel@nongnu.org
  Cc: crosthwaitepeter@gmail.com, pawel.lenkow@itlen.com,
	peter.maydell@linaro.org

On 06/16/2016 09:43 AM, Krzeminski, Marcin (Nokia - PL/Wroclaw) wrote:
> 
> 
>> -----Original Message-----
>> From: Cédric Le Goater [mailto:clg@kaod.org]
>> Sent: Thursday, June 16, 2016 9:14 AM
>> To: Krzeminski, Marcin (Nokia - PL/Wroclaw)
>> <marcin.krzeminski@nokia.com>; qemu-devel@nongnu.org
>> Cc: crosthwaitepeter@gmail.com; pawel.lenkow@itlen.com;
>> peter.maydell@linaro.org
>> Subject: Re: [PATCH 4/9] m25p80: Introduce COLLECTING_VAR_LEN_DATA
>> state.
>>
>> On 06/15/2016 03:41 PM, marcin.krzeminski@nokia.com wrote:
>>> From: Marcin Krzeminski <marcin.krzeminski@nokia.com>
>>>
>>> Some flash allows to stop read at any time.
>>> Allow framework to support this.
>>>
>>> Signed-off-by: Marcin Krzeminski <marcin.krzeminski@nokia.com>
>>> ---
>>>  hw/block/m25p80.c | 5 +++++
>>>  1 file changed, 5 insertions(+)
>>>
>>> diff --git a/hw/block/m25p80.c b/hw/block/m25p80.c index
>>> 6910c52..ca1f882 100644
>>> --- a/hw/block/m25p80.c
>>> +++ b/hw/block/m25p80.c
>>> @@ -330,6 +330,7 @@ typedef enum {
>>>      STATE_PAGE_PROGRAM,
>>>      STATE_READ,
>>>      STATE_COLLECTING_DATA,
>>> +    STATE_COLLECTING_VAR_LEN_DATA,
>>>      STATE_READING_DATA,
>>>  } CMDState;
>>>
>>> @@ -872,6 +873,9 @@ static int m25p80_cs(SSISlave *ss, bool select)
>>>      Flash *s = M25P80(ss);
>>>
>>>      if (select) {
>>> +        if (s->state == STATE_COLLECTING_VAR_LEN_DATA) {
>>> +            complete_collecting_data(s);
>>> +        }
>>
>> So for example, we could trigger an erase without a completed address ?
>>
>> C.
> Yes if you explicitly want that. This is new state and you need to go
> into it in decode_new_cmd.
> For now this is only used in WRSR command only.

OK. I see and I understand the use in that case.

Reviewed-by: Cédric Le Goater <clg@kaod.org>

C.


> 
> Thanks,
> Marcin
>>
>>>          s->len = 0;
>>>          s->pos = 0;
>>>          s->state = STATE_IDLE;
>>> @@ -905,6 +909,7 @@ static uint32_t m25p80_transfer8(SSISlave *ss,
>> uint32_t tx)
>>>          break;
>>>
>>>      case STATE_COLLECTING_DATA:
>>> +    case STATE_COLLECTING_VAR_LEN_DATA:
>>>          s->data[s->len] = (uint8_t)tx;
>>>          s->len++;
>>>
> 

^ permalink raw reply	[flat|nested] 28+ messages in thread

* Re: [Qemu-devel] [PATCH 7/9] m25p80: Introduce configuration registers.
  2016-06-16  8:05       ` Cédric Le Goater
@ 2016-06-17 10:31         ` Krzeminski, Marcin (Nokia - PL/Wroclaw)
  0 siblings, 0 replies; 28+ messages in thread
From: Krzeminski, Marcin (Nokia - PL/Wroclaw) @ 2016-06-17 10:31 UTC (permalink / raw)
  To: Cédric Le Goater, qemu-devel@nongnu.org
  Cc: crosthwaitepeter@gmail.com, pawel.lenkow@itlen.com,
	peter.maydell@linaro.org



> -----Original Message-----
> From: Cédric Le Goater [mailto:clg@kaod.org]
> Sent: Thursday, June 16, 2016 10:06 AM
> To: Krzeminski, Marcin (Nokia - PL/Wroclaw)
> <marcin.krzeminski@nokia.com>; qemu-devel@nongnu.org
> Cc: crosthwaitepeter@gmail.com; pawel.lenkow@itlen.com;
> peter.maydell@linaro.org
> Subject: Re: [PATCH 7/9] m25p80: Introduce configuration registers.
> 
> On 06/16/2016 09:52 AM, Krzeminski, Marcin (Nokia - PL/Wroclaw) wrote:
> >
> >
> >> -----Original Message-----
> >> From: Cédric Le Goater [mailto:clg@kaod.org]
> >> Sent: Thursday, June 16, 2016 9:25 AM
> >> To: Krzeminski, Marcin (Nokia - PL/Wroclaw)
> >> <marcin.krzeminski@nokia.com>; qemu-devel@nongnu.org
> >> Cc: crosthwaitepeter@gmail.com; pawel.lenkow@itlen.com;
> >> peter.maydell@linaro.org
> >> Subject: Re: [PATCH 7/9] m25p80: Introduce configuration registers.
> >>
> >> On 06/15/2016 03:41 PM, marcin.krzeminski@nokia.com wrote:
> >>> From: Marcin Krzeminski <marcin.krzeminski@nokia.com>
> >>>
> >>> Configuration registers for Spansion and Macronix devices.
> >>>
> >>> Signed-off-by: Marcin Krzeminski <marcin.krzeminski@nokia.com>
> >>
> >>
> >> Reviewed-by: Cédric Le Goater <clg@kaod.org>
> >>
> >> I don't think we can define a property array. can we ?
> > You mean Spansion config registers?
> > If yes 4 variables are used mostly to allow user easy setting of initial values.
> > It could be changed to one uint32_t or table but I do not know what We
> > benefit from that?
> 
> We would just reduce the number of 'spansion_cr*'. This is minor.
> 
> C.
So let it stay at it is. Thanks for the review. V2 is out.

Regards,
Marcin

> 
> 
> >
> > Thanks,
> > Marcin
> >>
> >>> ---
> >>>  hw/block/m25p80.c | 47
> >>> +++++++++++++++++++++++++++++++++++++++++++++++
> >>>  1 file changed, 47 insertions(+)
> >>>
> >>> diff --git a/hw/block/m25p80.c b/hw/block/m25p80.c index
> >>> d1c4d46..21998db 100644
> >>> --- a/hw/block/m25p80.c
> >>> +++ b/hw/block/m25p80.c
> >>> @@ -134,6 +134,14 @@ typedef struct FlashPartInfo {  #define
> >>> FSR_4BYTE_ADDR_MODE_ENABLED 0x1  #define FSR_FLASH_READY (1
> <<
> >> 7)
> >>>
> >>> +/* Spansion configuration registers macros. */ #define
> >>> +SPANSION_QUAD_CFG_POS 0 #define SPANSION_QUAD_CFG_LEN 1
> >> #define
> >>> +SPANSION_DUMMY_CLK_POS 0 #define
> SPANSION_DUMMY_CLK_LEN 4
> >> #define
> >>> +SPANSION_ADDR_LEN_POS 7 #define SPANSION_ADDR_LEN_LEN 1
> >>> +
> >>>  static const FlashPartInfo known_devices[] = {
> >>>      /* Atmel -- some are (confusingly) marketed as "DataFlash" */
> >>>      { INFO("at25fs010",   0x1f6601,      0,  32 << 10,   4, ER_4K) },
> >>> @@ -369,8 +377,18 @@ typedef struct Flash {
> >>>      uint8_t cmd_in_progress;
> >>>      uint64_t cur_addr;
> >>>      uint32_t nonvolatile_cfg;
> >>> +    /* Configuration register for Macronix */
> >>>      uint32_t volatile_cfg;
> >>>      uint32_t enh_volatile_cfg;
> >>> +    /* Spansion cfg registers. */
> >>> +    uint8_t spansion_cr1nv;
> >>> +    uint8_t spansion_cr2nv;
> >>> +    uint8_t spansion_cr3nv;
> >>> +    uint8_t spansion_cr4nv;
> >>> +    uint8_t spansion_cr1v;
> >>> +    uint8_t spansion_cr2v;
> >>> +    uint8_t spansion_cr3v;
> >>> +    uint8_t spansion_cr4v;
> >>>      bool write_enable;
> >>>      bool four_bytes_address_mode;
> >>>      bool reset_enable;
> >>> @@ -601,6 +619,9 @@ static void complete_collecting_data(Flash *s)
> >>>              break;
> >>>          case MAN_MACRONIX:
> >>>              s->quad_enable = extract32(s->data[0], 6, 1);
> >>> +            if (s->len > 1) {
> >>> +                s->four_bytes_address_mode = extract32(s->data[1], 5, 1);
> >>> +            }
> >>>              break;
> >>>          default:
> >>>              break;
> >>> @@ -674,6 +695,23 @@ static void reset_memory(Flash *s)
> >>>              s->ear = s->size / MAX_3BYTES_SIZE - 1;
> >>>          }
> >>>          break;
> >>> +    case MAN_MACRONIX:
> >>> +        s->volatile_cfg = 0x7;
> >>> +        break;
> >>> +    case MAN_SPANSION:
> >>> +        s->spansion_cr1v = s->spansion_cr1nv;
> >>> +        s->spansion_cr2v = s->spansion_cr2nv;
> >>> +        s->spansion_cr3v = s->spansion_cr3nv;
> >>> +        s->spansion_cr4v = s->spansion_cr4nv;
> >>> +        s->quad_enable = extract32(s->spansion_cr1v,
> >>> +                                   SPANSION_QUAD_CFG_POS,
> >>> +                                   SPANSION_QUAD_CFG_LEN
> >>> +                                   );
> >>> +        s->four_bytes_address_mode = extract32(s->spansion_cr2v,
> >>> +                SPANSION_ADDR_LEN_POS,
> >>> +                SPANSION_ADDR_LEN_LEN
> >>> +                );
> >>> +        break;
> >>>      default:
> >>>          break;
> >>>      }
> >>> @@ -1052,7 +1090,12 @@ static void m25p80_pre_save(void *opaque)
> }
> >>>
> >>>  static Property m25p80_properties[] = {
> >>> +    /* This is default value for Micron flash */
> >>>      DEFINE_PROP_UINT32("nonvolatile-cfg", Flash, nonvolatile_cfg,
> >>> 0x8FFF),
> >>> +    DEFINE_PROP_UINT8("spansion-cr1nv", Flash, spansion_cr1nv, 0x0),
> >>> +    DEFINE_PROP_UINT8("spansion-cr2nv", Flash, spansion_cr2nv, 0x8),
> >>> +    DEFINE_PROP_UINT8("spansion-cr3nv", Flash, spansion_cr3nv, 0x2),
> >>> +    DEFINE_PROP_UINT8("spansion-cr4nv", Flash, spansion_cr4nv,
> >>> + 0x10),
> >>>      DEFINE_PROP_END_OF_LIST(),
> >>>  };
> >>>
> >>> @@ -1077,6 +1120,10 @@ static const VMStateDescription
> >> vmstate_m25p80 = {
> >>>          VMSTATE_UINT32_V(volatile_cfg, Flash, 2),
> >>>          VMSTATE_UINT32_V(enh_volatile_cfg, Flash, 2),
> >>>          VMSTATE_BOOL_V(quad_enable, Flash, 3),
> >>> +        VMSTATE_UINT8_V(spansion_cr1nv, Flash, 3),
> >>> +        VMSTATE_UINT8_V(spansion_cr2nv, Flash, 3),
> >>> +        VMSTATE_UINT8_V(spansion_cr3nv, Flash, 3),
> >>> +        VMSTATE_UINT8_V(spansion_cr4nv, Flash, 3),
> >>>          VMSTATE_END_OF_LIST()
> >>>      }
> >>>  };
> >>>
> >

^ permalink raw reply	[flat|nested] 28+ messages in thread

* Re: [Qemu-devel] Odp.: [PATCH 6/9] m25p80: Introduce quad and equad modes.
  2016-06-15 17:40     ` [Qemu-devel] Odp.: " Krzeminski, Marcin (Nokia - PL/Wroclaw)
@ 2016-06-17 12:43       ` Cédric Le Goater
  0 siblings, 0 replies; 28+ messages in thread
From: Cédric Le Goater @ 2016-06-17 12:43 UTC (permalink / raw)
  To: Krzeminski, Marcin (Nokia - PL/Wroclaw), qemu-devel@nongnu.org
  Cc: crosthwaitepeter@gmail.com, pawel.lenkow@itlen.com,
	peter.maydell@linaro.org

On 06/15/2016 07:40 PM, Krzeminski, Marcin (Nokia - PL/Wroclaw) wrote:
> 
> 
> W dniu 15.06.2016 o 16:25, Cédric Le Goater pisze:
>> On 06/15/2016 03:41 PM, marcin.krzeminski@nokia.com wrote:
>>> From: Marcin Krzeminski <marcin.krzeminski@nokia.com>
>>>
>>> Quad and Equad modes for Spansion and Macronix flash devices.
>>> This commit also includes modification and new command to manipulate
>>> quad mode (status registers and dedicated commands).
>>> This work is based on Pawel Lenkow work.
>>>
>>> Signed-off-by: Marcin Krzeminski <marcin.krzeminski@nokia.com>
>>> ---
>>>  hw/block/m25p80.c | 70 +++++++++++++++++++++++++++++++++++++++++++++++++++----
>>>  1 file changed, 65 insertions(+), 5 deletions(-)
>>>
>>> diff --git a/hw/block/m25p80.c b/hw/block/m25p80.c
>>> index 55b4377..d1c4d46 100644
>>> --- a/hw/block/m25p80.c
>>> +++ b/hw/block/m25p80.c
>>> @@ -281,6 +281,7 @@ typedef enum {
>>>      JEDEC_READ = 0x9f,
>>>      BULK_ERASE = 0xc7,
>>>      READ_FSR = 0x70,
>>> +    RDCR = 0x15,
>>>
>>>      READ = 0x03,
>>>      READ4 = 0x13,
>>> @@ -317,6 +318,13 @@ typedef enum {
>>>      RESET_ENABLE = 0x66,
>>>      RESET_MEMORY = 0x99,
>>>
>>> +    /*
>>> +     * Micron: 0x35 - enable QPI
>>> +     * Spansion: 0x35 - read control register
>>> +     */
>>> +    RDCR_EQIO = 0x35,
>>> +    RSTQIO = 0xf5,
>>> +
>>>      RNVCR = 0xB5,
>>>      WNVCR = 0xB1,
>>>
>>> @@ -366,6 +374,7 @@ typedef struct Flash {
>>>      bool write_enable;
>>>      bool four_bytes_address_mode;
>>>      bool reset_enable;
>>> +    bool quad_enable;
>>>      uint8_t ear;
>>>
>>>      int64_t dirty_page;
>>> @@ -586,6 +595,16 @@ static void complete_collecting_data(Flash *s)
>>>          flash_erase(s, s->cur_addr, s->cmd_in_progress);
>>>          break;
>>>      case WRSR:
>>> +        switch (get_man(s)) {
>>> +        case MAN_SPANSION:
>>> +            s->quad_enable = !!(s->data[1] & 0x02);
>>> +            break;
>>> +        case MAN_MACRONIX:
>>> +            s->quad_enable = extract32(s->data[0], 6, 1);
>>> +            break;
>>> +        default:
>>> +            break;
>>> +        }
>>>          if (s->write_enable) {
>>>              s->write_enable = false;
>>>          }
>>> @@ -619,6 +638,7 @@ static void reset_memory(Flash *s)
>>>      s->state = STATE_IDLE;
>>>      s->write_enable = false;
>>>      s->reset_enable = false;
>>> +    s->quad_enable = false;
>>>
>>>      switch (get_man(s)) {
>>>      case MAN_NUMONYX:
>>> @@ -747,10 +767,20 @@ static void decode_new_cmd(Flash *s, uint32_t value)
>>>
>>>      case WRSR:
>>>          if (s->write_enable) {
>>> -            s->needed_bytes = 1;
>>> +            switch (get_man(s)) {
>>> +            case MAN_SPANSION:
>>> +                s->needed_bytes = 2;
>>> +                s->state = STATE_COLLECTING_DATA;
>>> +                break;
>>> +            case MAN_MACRONIX:
>>> +                s->needed_bytes = 2;
>>> +                s->state = STATE_COLLECTING_VAR_LEN_DATA;
>>> +                break;
>>
>> ah. I am glad you address this topic because I am having a little issue
>> with the mx25l25635e and the mx25l25635f.
>>
>> mx25l25635e is a macronix which supports the WRSR command with one byte 
>> and the mx25l25635f needs two. The fun part is that they have the same
>> JEDEC : 0xc22019.
>>
>> So not all macronix need 2 bytes. Should we add a flag for that ? 
>>
> I think this case should be partially handled with path 4 and this one.
> The new state (patch 4) was introduced to allow use variable write cmds.
> As in my case mx66u51235f allow write up to 2 bytes, but linux writes only one,
> and that is allowed and works on real HW. From the datasheet mx25l25635f
> should behave exactly same way.
> In case you mention, the problem might pop up, when guest will try
> to write 2 bytes to mx25l25635e. Real HW will treat second byte as new command,
> but this model accept this situation.

qemu complains a bit but that's OK.

> Generally my approach and I think original author was to not emulate exactly
> real hw behaviour but allow to model it in such way the quest can boot up and use it.
> If we go witch very restricted command behaviour we will end up in unreadable code
> (eg. newest Spansions/Cypress family does not support extended address register,
> but such commands will work in this model). The question is if you really need to flag
> and support this. If yes additional if is not a problem.

I will wait for your patchset to be merged and then see if the mx25l25635f
needs anything more. This is really minor. It should be a onliner.

Thanks,

C. 

>>
>>> +            default:
>>> +                s->needed_bytes = 1;
>>> +                s->state = STATE_COLLECTING_DATA;
>>> +            }
>>>              s->pos = 0;
>>> -            s->len = 0;
>>> -            s->state = STATE_COLLECTING_DATA;
>>>          }
>>>          break;
>>>
>>> @@ -763,6 +793,9 @@ static void decode_new_cmd(Flash *s, uint32_t value)
>>>
>>>      case RDSR:
>>>          s->data[0] = (!!s->write_enable) << 1;
>>> +        if (get_man(s) == MAN_MACRONIX) {
>>> +            s->data[0] |= (!!s->quad_enable) << 6;
>>> +        }
>>>          s->pos = 0;
>>>          s->len = 1;
>>>          s->state = STATE_READING_DATA;
>>> @@ -789,6 +822,14 @@ static void decode_new_cmd(Flash *s, uint32_t value)
>>>          s->state = STATE_READING_DATA;
>>>          break;
>>>
>>> +    case RDCR:
>>> +        s->data[0] = s->volatile_cfg & 0xFF;
>>> +        s->data[0] |= (!!s->four_bytes_address_mode) << 5;
>>> +        s->pos = 0;
>>> +        s->len = 1;
>>> +        s->state = STATE_READING_DATA;
>>> +        break;
>>> +
>>>      case BULK_ERASE:
>>>          if (s->write_enable) {
>>>              DB_PRINT_L(0, "chip erase\n");
>>> @@ -828,7 +869,7 @@ static void decode_new_cmd(Flash *s, uint32_t value)
>>>          s->state = STATE_READING_DATA;
>>>          break;
>>>      case WNVCR:
>>> -        if (s->write_enable) {
>>> +        if (s->write_enable && get_man(s) == MAN_NUMONYX) {
>>>              s->needed_bytes = 2;
>>>              s->pos = 0;
>>>              s->len = 0;
>>> @@ -871,6 +912,24 @@ static void decode_new_cmd(Flash *s, uint32_t value)
>>>              reset_memory(s);
>>>          }
>>>          break;
>>> +    case RDCR_EQIO:
>>> +        switch (get_man(s)) {
>>> +        case MAN_SPANSION:
>>> +            s->data[0] = (!!s->quad_enable) << 1;
>>> +            s->pos = 0;
>>> +            s->len = 1;
>>> +            s->state = STATE_READING_DATA;
>>> +            break;
>>> +        case MAN_MACRONIX:
>>> +            s->quad_enable = true;
>>> +            break;
>>> +        default:
>>> +            break;
>>> +        }
>>> +        break;
>>> +    case RSTQIO:
>>> +        s->quad_enable = false;
>>> +        break;
>>>      default:
>>>          qemu_log_mask(LOG_GUEST_ERROR, "M25P80: Unknown cmd %x\n", value);
>>>          break;
>>> @@ -999,7 +1058,7 @@ static Property m25p80_properties[] = {
>>>
>>>  static const VMStateDescription vmstate_m25p80 = {
>>>      .name = "xilinx_spi",
>>> -    .version_id = 2,
>>> +    .version_id = 3,
>>>      .minimum_version_id = 1,
>>>      .pre_save = m25p80_pre_save,
>>>      .fields = (VMStateField[]) {
>>> @@ -1017,6 +1076,7 @@ static const VMStateDescription vmstate_m25p80 = {
>>>          VMSTATE_UINT32_V(nonvolatile_cfg, Flash, 2),
>>>          VMSTATE_UINT32_V(volatile_cfg, Flash, 2),
>>>          VMSTATE_UINT32_V(enh_volatile_cfg, Flash, 2),
>>> +        VMSTATE_BOOL_V(quad_enable, Flash, 3),
>>>          VMSTATE_END_OF_LIST()
>>>      }
>>>  };
>>>
>>
>>
>>
> 

^ permalink raw reply	[flat|nested] 28+ messages in thread

end of thread, other threads:[~2016-06-17 12:43 UTC | newest]

Thread overview: 28+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2016-06-15 13:41 [Qemu-devel] [PATCH 0/9] m25p80: Add new 512Mbit and 1Gbit devices marcin.krzeminski
2016-06-15 13:41 ` [Qemu-devel] [PATCH 1/9] m25p80: Replace JEDEC ID masking with function marcin.krzeminski
2016-06-15 14:05   ` Cédric Le Goater
2016-06-15 17:09     ` [Qemu-devel] Odp.: " Krzeminski, Marcin (Nokia - PL/Wroclaw)
2016-06-15 13:41 ` [Qemu-devel] [PATCH 2/9] m25p80: Make a table for JEDEC ID marcin.krzeminski
2016-06-15 14:17   ` Cédric Le Goater
2016-06-15 13:41 ` [Qemu-devel] [PATCH 3/9] m25p80: Allow more than four banks marcin.krzeminski
2016-06-16  7:09   ` Cédric Le Goater
2016-06-15 13:41 ` [Qemu-devel] [PATCH 4/9] m25p80: Introduce COLLECTING_VAR_LEN_DATA state marcin.krzeminski
2016-06-16  7:13   ` Cédric Le Goater
2016-06-16  7:43     ` Krzeminski, Marcin (Nokia - PL/Wroclaw)
2016-06-16  8:13       ` Cédric Le Goater
2016-06-15 13:41 ` [Qemu-devel] [PATCH 5/9] m25p80: Add additional flash commands: marcin.krzeminski
2016-06-16  7:14   ` Cédric Le Goater
2016-06-15 13:41 ` [Qemu-devel] [PATCH 6/9] m25p80: Introduce quad and equad modes marcin.krzeminski
2016-06-15 14:25   ` Cédric Le Goater
2016-06-15 17:40     ` [Qemu-devel] Odp.: " Krzeminski, Marcin (Nokia - PL/Wroclaw)
2016-06-17 12:43       ` Cédric Le Goater
2016-06-15 13:41 ` [Qemu-devel] [PATCH 7/9] m25p80: Introduce configuration registers marcin.krzeminski
2016-06-16  7:24   ` Cédric Le Goater
2016-06-16  7:52     ` Krzeminski, Marcin (Nokia - PL/Wroclaw)
2016-06-16  8:05       ` Cédric Le Goater
2016-06-17 10:31         ` Krzeminski, Marcin (Nokia - PL/Wroclaw)
2016-06-15 13:41 ` [Qemu-devel] [PATCH 8/9] m25p80: Fast read commands family changes marcin.krzeminski
2016-06-16  7:19   ` Cédric Le Goater
2016-06-16  7:53     ` Krzeminski, Marcin (Nokia - PL/Wroclaw)
2016-06-15 13:41 ` [Qemu-devel] [PATCH 9/9] m25p80: New flash devices marcin.krzeminski
2016-06-16  7:20   ` Cédric Le Goater

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).