diff mbox

[v4,06/11] block: m25p80: Add configuration registers

Message ID 1456128205-5092-7-git-send-email-marcin.krzeminski@nokia.com
State New
Headers show

Commit Message

Krzeminski, Marcin (Nokia - PL/Wroclaw) Feb. 22, 2016, 8:03 a.m. UTC 
From: Marcin Krzeminski <marcin.krzeminski@nokia.com>

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

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

Comments

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

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

> +        s->volatile_cfg |= 0x3;

Why you set a reserved bit?

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

An example,
#define NVCFG_XIP_MODE_DISABLED 0x7

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

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

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

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

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

Regards,
Peter

> +        /* Reset/hold & reserved */
> +        s->enh_volatile_cfg |= (1 << 4);
> +        /* Dual I/O protocol */
> +        if ((s->nonvolatile_cfg >> 1) & 0x1) {
> +            s->enh_volatile_cfg |= (1 << 6);
> +        }
> +        /* Quad I/O protocol */
> +        if ((s->nonvolatile_cfg >> 3) & 0x1) {
> +            s->enh_volatile_cfg |= (1 << 7);
> +        }
> +
> +        if (!(s->nonvolatile_cfg & 0x1)) {
> +            s->four_bytes_address_mode = true;
> +        }
> +
> +        if (!((s->nonvolatile_cfg >> 1) & 0x1)) {
> +            s->ear = 0x3;
> +        }
> +    }
> +
>      DB_PRINT_L(0, "Reset done.\n");
>  }
>
> @@ -617,6 +675,49 @@ static void decode_new_cmd(Flash *s, uint32_t value)
>              s->state = STATE_COLLECTING_DATA;
>          }
>          break;
> +    case RNVCR:
> +        s->data[0] = s->nonvolatile_cfg & 0xFF;
> +        s->data[1] = (s->nonvolatile_cfg >> 8) & 0xFF;
> +        s->pos = 0;
> +        s->len = 2;
> +        s->state = STATE_READING_DATA;
> +        break;
> +    case WNVCR:
> +        if (s->write_enable) {
> +            s->needed_bytes = 2;
> +            s->pos = 0;
> +            s->len = 0;
> +            s->state = STATE_COLLECTING_DATA;
> +        }
> +        break;
> +    case RVCR:
> +        s->data[0] = s->volatile_cfg & 0xFF;
> +        s->pos = 0;
> +        s->len = 1;
> +        s->state = STATE_READING_DATA;
> +        break;
> +    case WVCR:
> +        if (s->write_enable) {
> +            s->needed_bytes = 1;
> +            s->pos = 0;
> +            s->len = 0;
> +            s->state = STATE_COLLECTING_DATA;
> +        }
> +        break;
> +    case REVCR:
> +        s->data[0] = s->enh_volatile_cfg & 0xFF;
> +        s->pos = 0;
> +        s->len = 1;
> +        s->state = STATE_READING_DATA;
> +        break;
> +    case WEVCR:
> +        if (s->write_enable) {
> +            s->needed_bytes = 1;
> +            s->pos = 0;
> +            s->len = 0;
> +            s->state = STATE_COLLECTING_DATA;
> +        }
> +        break;
>      case RESET_ENABLE:
>          s->reset_enable = true;
>          break;
> @@ -738,6 +839,11 @@ static void m25p80_pre_save(void *opaque)
>      flash_sync_dirty((Flash *)opaque, -1);
>  }
>
> +static Property m25p80_properties[] = {
> +    DEFINE_PROP_UINT32("nonvolatile-cfg", Flash, nonvolatile_cfg, 0x8FFF),
> +    DEFINE_PROP_END_OF_LIST(),
> +};
> +
>  static const VMStateDescription vmstate_m25p80 = {
>      .name = "xilinx_spi",
>      .version_id = 2,
> @@ -755,6 +861,9 @@ static const VMStateDescription vmstate_m25p80 = {
>          VMSTATE_BOOL(four_bytes_address_mode, Flash),
>          VMSTATE_UINT8(ear, Flash),
>          VMSTATE_BOOL(reset_enable, Flash),
> +        VMSTATE_UINT32(nonvolatile_cfg, Flash),
> +        VMSTATE_UINT32(volatile_cfg, Flash),
> +        VMSTATE_UINT32(enh_volatile_cfg, Flash),
>          VMSTATE_END_OF_LIST()
>      }
>  };
> @@ -770,6 +879,7 @@ static void m25p80_class_init(ObjectClass *klass, void *data)
>      k->set_cs = m25p80_cs;
>      k->cs_polarity = SSI_CS_LOW;
>      dc->vmsd = &vmstate_m25p80;
> +    dc->props = m25p80_properties;
>      mc->pi = data;
>  }
>
> --
> 2.5.0
>
Krzeminski, Marcin (Nokia - PL/Wroclaw) March 17, 2016, 9:46 p.m. UTC | #2 
W dniu 17.03.2016 o 18:24, Peter Crosthwaite pisze:
> On Mon, Feb 22, 2016 at 12:03 AM,  <marcin.krzeminski@nokia.com> wrote:
>> From: Marcin Krzeminski <marcin.krzeminski@nokia.com>
>>
>> This patch adds both volatile and non volatile configuration registers
>> and commands to allow modify them. It is needed for proper handling
>> dummy cycles. Initialization of those registers and flash state
>> has been included as well.
>> Some of this registers are used by kernel.
>>
>> Signed-off-by: Marcin Krzeminski <marcin.krzeminski@nokia.com>
>> ---
>>  hw/block/m25p80.c | 110 ++++++++++++++++++++++++++++++++++++++++++++++++++++++
>>  1 file changed, 110 insertions(+)
>>
>> diff --git a/hw/block/m25p80.c b/hw/block/m25p80.c
>> index 0698e7b..9d5a071 100644
>> --- a/hw/block/m25p80.c
>> +++ b/hw/block/m25p80.c
>> @@ -26,6 +26,7 @@
>>  #include "sysemu/block-backend.h"
>>  #include "sysemu/blockdev.h"
>>  #include "hw/ssi/ssi.h"
>> +#include "qemu/bitops.h"
>>
>>  #ifndef M25P80_ERR_DEBUG
>>  #define M25P80_ERR_DEBUG 0
>> @@ -245,6 +246,15 @@ typedef enum {
>>
>>      RESET_ENABLE = 0x66,
>>      RESET_MEMORY = 0x99,
>> +
>> +    RNVCR = 0xB5,
>> +    WNVCR = 0xB1,
>> +
>> +    RVCR = 0x85,
>> +    WVCR = 0x81,
>> +
>> +    REVCR = 0x65,
>> +    WEVCR = 0x61,
>>  } FlashCMD;
>>
>>  typedef enum {
>> @@ -271,6 +281,9 @@ typedef struct Flash {
>>      uint8_t needed_bytes;
>>      uint8_t cmd_in_progress;
>>      uint64_t cur_addr;
>> +    uint32_t nonvolatile_cfg;
>> +    uint32_t volatile_cfg;
>> +    uint32_t enh_volatile_cfg;
>>      bool write_enable;
>>      bool four_bytes_address_mode;
>>      bool reset_enable;
>> @@ -459,6 +472,15 @@ static void complete_collecting_data(Flash *s)
>>      case EXTEND_ADDR_WRITE:
>>          s->ear = s->data[0];
>>          break;
>> +    case WNVCR:
>> +        s->nonvolatile_cfg = s->data[0] | (s->data[1] << 8);
>> +        break;
>> +    case WVCR:
>> +        s->volatile_cfg = s->data[0];
>> +        break;
>> +    case WEVCR:
>> +        s->enh_volatile_cfg = s->data[0];
>> +        break;
>>      default:
>>          break;
>>      }
>> @@ -477,6 +499,42 @@ static void reset_memory(Flash *s)
>>      s->write_enable = false;
>>      s->reset_enable = false;
>>
>> +    if (((s->pi->jedec >> 16) & 0xFF) == JEDEC_NUMONYX) {
>> +        s->volatile_cfg = 0;
>> +        /* WRAP & reserved*/
>
> These bitfield masks and their values need some macrofication. Then
> the comments explaining what bits are what can be dropped.
>
>> +        s->volatile_cfg |= 0x3;
>
> Why you set a reserved bit?
From datasheet: The device ships from
the factory with all bits erased to 1 (FFFFh).
>
>
>> +        /* XIP */
>> +        if (extract32(s->nonvolatile_cfg, 9, 3) != 0x7) {
>
> An example,
> #define NVCFG_XIP_MODE_DISABLED 0x7
>
>> +            s->volatile_cfg |= (1 << 3);
>> +        }
>> +        /* Number of dummy cycles */
>> +        s->volatile_cfg |= deposit32(s->volatile_cfg,
>> +                            4, 4, extract32(s->nonvolatile_cfg, 12, 4));
>> +        s->enh_volatile_cfg = 0;
>> +        /* Output driver strength */
>> +        s->enh_volatile_cfg |= 0x7;
>> +        /* Vpp accelerator */
>> +        s->enh_volatile_cfg |= (1 << 3);
>
> #define EVCFG_VPP_ACCELERATOR (1 << 3) ...
Macros here is a good idea. I will add them in v5.

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

Patch

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