qemu-devel
[Top][All Lists]
Advanced

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

[Qemu-devel] Odp.: [PATCH 1/9] m25p80: Replace JEDEC ID masking with fun


From: Krzeminski, Marcin (Nokia - PL/Wroclaw)
Subject: [Qemu-devel] Odp.: [PATCH 1/9] m25p80: Replace JEDEC ID masking with function.
Date: Wed, 15 Jun 2016 17:09:34 +0000


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

On 06/15/2016 03:41 PM, address@hidden wrote:


From: Marcin Krzeminski <address@hidden>

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

Signed-off-by: Marcin Krzeminski <address@hidden>


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:



.






reply via email to

[Prev in Thread] Current Thread [Next in Thread]