[Qemu-devel] [PATCH v3 0/3] block: m25p80: Improve mt25qu01g chip model

[Qemu-devel] [PATCH v3 0/3] block: m25p80: Improve mt25qu01g chip model

[Marcin Krzeminski]

This series introduce some improvememnt targeting mt25qu01g and adds
support for n25q00 device.

v3:
* Removed debug printf
v2:
* Rework die erase command functionality. Since the number of dies
is constant per chip, new field to flash description structure has
been added (instead of dynamic code from v1).
* Add definition for n25q00.

Marcin Krzeminski (3):
  block: m25p80: Add Quad Page Program 4byte
  block: m25p80: Introduce die erase command
  block: m25p80: Improve 1GiB Micron flash definition

 hw/block/m25p80.c | 51 ++++++++++++++++++++++++++++++++++++++++++++++++---
 1 file changed, 48 insertions(+), 3 deletions(-)

-- 
2.9.3

[Qemu-devel] [PATCH v3 1/3] block: m25p80: Add Quad Page Program 4byte

[Marcin Krzeminski]

Some flash chips have additional page program opcode that
takes only 4 byte address. This commit adds support
for such command in Qemu.

Signed-off-by: Marcin Krzeminski <mar.krzeminski@gmail.com>
Reviewed-by: Edgar E. Iglesias <edgar.iglesias@xilinx.com>
---
 hw/block/m25p80.c | 4 ++++
 1 file changed, 4 insertions(+)

diff --git a/hw/block/m25p80.c b/hw/block/m25p80.c
index e3c1166..6dff81b 100644
--- a/hw/block/m25p80.c
+++ b/hw/block/m25p80.c
@@ -326,6 +326,7 @@ typedef enum {
     PP4_4 = 0x3e,
     DPP = 0xa2,
     QPP = 0x32,
+    QPP_4 = 0x34,
 
     ERASE_4K = 0x20,
     ERASE4_4K = 0x21,
@@ -574,6 +575,7 @@ static inline int get_addr_length(Flash *s)
    switch (s->cmd_in_progress) {
    case PP4:
    case PP4_4:
+   case QPP_4:
    case READ4:
    case QIOR4:
    case ERASE4_4K:
@@ -607,6 +609,7 @@ static void complete_collecting_data(Flash *s)
     switch (s->cmd_in_progress) {
     case DPP:
     case QPP:
+    case QPP_4:
     case PP:
     case PP4:
     case PP4_4:
@@ -874,6 +877,7 @@ static void decode_new_cmd(Flash *s, uint32_t value)
     case READ4:
     case DPP:
     case QPP:
+    case QPP_4:
     case PP:
     case PP4:
     case PP4_4:
-- 
2.9.3

[Qemu-devel] [PATCH v3 2/3] block: m25p80: Introduce die erase command

[Marcin Krzeminski]

Modern big flash nor devices consist from more than one die.
Some of them do not support chip erase and instead have die
erase command that can erase one die only. This commit adds
possibility to define number of dies in the chip and adds
support for die erase command. Nor flash model is not strict
thus option to disable chip eras was not added.

Signed-off-by: Marcin Krzeminski <mar.krzeminski@gmail.com>
---
 hw/block/m25p80.c | 41 ++++++++++++++++++++++++++++++++++++++++-
 1 file changed, 40 insertions(+), 1 deletion(-)

diff --git a/hw/block/m25p80.c b/hw/block/m25p80.c
index 6dff81b..a9b025b 100644
--- a/hw/block/m25p80.c
+++ b/hw/block/m25p80.c
@@ -73,6 +73,12 @@ typedef struct FlashPartInfo {
     uint32_t n_sectors;
     uint32_t page_size;
     uint16_t flags;
+    /*
+     * Big sized spi nor are often stacked devices, thus sometime
+     * replace chip erase with die erase.
+     * This field inform how many die is in the chip.
+     */
+    uint8_t die_cnt;
 } FlashPartInfo;
 
 /* adapted from linux */
@@ -90,7 +96,8 @@ typedef struct FlashPartInfo {
     .sector_size = (_sector_size),\
     .n_sectors = (_n_sectors),\
     .page_size = 256,\
-    .flags = (_flags),
+    .flags = (_flags),\
+    .die_cnt = 0
 
 #define INFO6(_part_name, _jedec_id, _ext_id, _sector_size, _n_sectors, _flags)\
     .part_name = _part_name,\
@@ -107,6 +114,24 @@ typedef struct FlashPartInfo {
     .n_sectors = (_n_sectors),\
     .page_size = 256,\
     .flags = (_flags),\
+    .die_cnt = 0
+
+#define INFO_STACKED(_part_name, _jedec_id, _ext_id, _sector_size, _n_sectors,\
+                    _flags, _die_cnt)\
+    .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),\
+    .die_cnt = _die_cnt
 
 #define JEDEC_NUMONYX 0x20
 #define JEDEC_WINBOND 0xEF
@@ -359,6 +384,8 @@ typedef enum {
 
     REVCR = 0x65,
     WEVCR = 0x61,
+
+    DIE_ERASE = 0xC4,
 } FlashCMD;
 
 typedef enum {
@@ -514,6 +541,16 @@ static void flash_erase(Flash *s, int offset, FlashCMD cmd)
     case BULK_ERASE:
         len = s->size;
         break;
+    case DIE_ERASE:
+        if (s->pi->die_cnt) {
+            len = s->size / s->pi->die_cnt;
+            offset = offset & (~(len - 1));
+        } else {
+            qemu_log_mask(LOG_GUEST_ERROR, "M25P80: die erase is not supported"
+                          " by device\n");
+            return;
+        }
+        break;
     default:
         abort();
     }
@@ -635,6 +672,7 @@ static void complete_collecting_data(Flash *s)
     case ERASE4_32K:
     case ERASE_SECTOR:
     case ERASE4_SECTOR:
+    case DIE_ERASE:
         flash_erase(s, s->cur_addr, s->cmd_in_progress);
         break;
     case WRSR:
@@ -881,6 +919,7 @@ static void decode_new_cmd(Flash *s, uint32_t value)
     case PP:
     case PP4:
     case PP4_4:
+    case DIE_ERASE:
         s->needed_bytes = get_addr_length(s);
         s->pos = 0;
         s->len = 0;
-- 
2.9.3

[Qemu-devel] [PATCH v3 3/3] block: m25p80: Improve 1GiB Micron flash definition

[Marcin Krzeminski]

n25q00 and mt25q01 devices share the same JEDEC ID. The difference
between those two devices is number of dies and one bit in extended
JEDEC bytes. This commit adds proper entry for both devices by
introduction the number of dies and and new 25q00 entries.

Signed-off-by: Marcin Krzeminski <mar.krzeminski@gmail.com>
---
 hw/block/m25p80.c | 6 ++++--
 1 file changed, 4 insertions(+), 2 deletions(-)

diff --git a/hw/block/m25p80.c b/hw/block/m25p80.c
index a9b025b..fb5cef3 100644
--- a/hw/block/m25p80.c
+++ b/hw/block/m25p80.c
@@ -242,8 +242,10 @@ static const FlashPartInfo known_devices[] = {
     { 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) },
+    { INFO_STACKED("n25q00",    0x20ba21, 0x1000, 64 << 10, 2048, ER_4K, 4) },
+    { INFO_STACKED("n25q00a",   0x20bb21, 0x1000, 64 << 10, 2048, ER_4K, 4) },
+    { INFO_STACKED("mt25ql01g", 0x20ba21, 0x1040, 64 << 10, 2048, ER_4K, 2) },
+    { INFO_STACKED("mt25qu01g", 0x20bb21, 0x1040, 64 << 10, 2048, ER_4K, 2) },
 
     /* Spansion -- single (large) sector size only, at least
      * for the chips listed here (without boot sectors).
-- 
2.9.3

Re: [Qemu-devel] [PATCH v3 3/3] block: m25p80: Improve 1GiB Micron flash definition

[Cédric Le Goater]

On 01/08/2017 09:38 AM, Marcin Krzeminski wrote:
> n25q00 and mt25q01 devices share the same JEDEC ID. The difference
> between those two devices is number of dies and one bit in extended
> JEDEC bytes. This commit adds proper entry for both devices by
> introduction the number of dies and and new 25q00 entries.
> 
> Signed-off-by: Marcin Krzeminski <mar.krzeminski@gmail.com>


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

Thanks,

C.

> ---
>  hw/block/m25p80.c | 6 ++++--
>  1 file changed, 4 insertions(+), 2 deletions(-)
> 
> diff --git a/hw/block/m25p80.c b/hw/block/m25p80.c
> index a9b025b..fb5cef3 100644
> --- a/hw/block/m25p80.c
> +++ b/hw/block/m25p80.c
> @@ -242,8 +242,10 @@ static const FlashPartInfo known_devices[] = {
>      { 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) },
> +    { INFO_STACKED("n25q00",    0x20ba21, 0x1000, 64 << 10, 2048, ER_4K, 4) },
> +    { INFO_STACKED("n25q00a",   0x20bb21, 0x1000, 64 << 10, 2048, ER_4K, 4) },
> +    { INFO_STACKED("mt25ql01g", 0x20ba21, 0x1040, 64 << 10, 2048, ER_4K, 2) },
> +    { INFO_STACKED("mt25qu01g", 0x20bb21, 0x1040, 64 << 10, 2048, ER_4K, 2) },
>  
>      /* Spansion -- single (large) sector size only, at least
>       * for the chips listed here (without boot sectors).
> 

Re: [Qemu-devel] [PATCH v3 2/3] block: m25p80: Introduce die erase command

[Cédric Le Goater]

On 01/08/2017 09:38 AM, Marcin Krzeminski wrote:
> Modern big flash nor devices consist from more than one die.
> Some of them do not support chip erase and instead have die
> erase command that can erase one die only. This commit adds
> possibility to define number of dies in the chip and adds
> support for die erase command. Nor flash model is not strict
> thus option to disable chip eras was not added.
> 
> Signed-off-by: Marcin Krzeminski <mar.krzeminski@gmail.com>


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

Thanks,

C.

> ---
>  hw/block/m25p80.c | 41 ++++++++++++++++++++++++++++++++++++++++-
>  1 file changed, 40 insertions(+), 1 deletion(-)
> 
> diff --git a/hw/block/m25p80.c b/hw/block/m25p80.c
> index 6dff81b..a9b025b 100644
> --- a/hw/block/m25p80.c
> +++ b/hw/block/m25p80.c
> @@ -73,6 +73,12 @@ typedef struct FlashPartInfo {
>      uint32_t n_sectors;
>      uint32_t page_size;
>      uint16_t flags;
> +    /*
> +     * Big sized spi nor are often stacked devices, thus sometime
> +     * replace chip erase with die erase.
> +     * This field inform how many die is in the chip.
> +     */
> +    uint8_t die_cnt;
>  } FlashPartInfo;
>  
>  /* adapted from linux */
> @@ -90,7 +96,8 @@ typedef struct FlashPartInfo {
>      .sector_size = (_sector_size),\
>      .n_sectors = (_n_sectors),\
>      .page_size = 256,\
> -    .flags = (_flags),
> +    .flags = (_flags),\
> +    .die_cnt = 0
>  
>  #define INFO6(_part_name, _jedec_id, _ext_id, _sector_size, _n_sectors, _flags)\
>      .part_name = _part_name,\
> @@ -107,6 +114,24 @@ typedef struct FlashPartInfo {
>      .n_sectors = (_n_sectors),\
>      .page_size = 256,\
>      .flags = (_flags),\
> +    .die_cnt = 0
> +
> +#define INFO_STACKED(_part_name, _jedec_id, _ext_id, _sector_size, _n_sectors,\
> +                    _flags, _die_cnt)\
> +    .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),\
> +    .die_cnt = _die_cnt
>  
>  #define JEDEC_NUMONYX 0x20
>  #define JEDEC_WINBOND 0xEF
> @@ -359,6 +384,8 @@ typedef enum {
>  
>      REVCR = 0x65,
>      WEVCR = 0x61,
> +
> +    DIE_ERASE = 0xC4,
>  } FlashCMD;
>  
>  typedef enum {
> @@ -514,6 +541,16 @@ static void flash_erase(Flash *s, int offset, FlashCMD cmd)
>      case BULK_ERASE:
>          len = s->size;
>          break;
> +    case DIE_ERASE:
> +        if (s->pi->die_cnt) {
> +            len = s->size / s->pi->die_cnt;
> +            offset = offset & (~(len - 1));
> +        } else {
> +            qemu_log_mask(LOG_GUEST_ERROR, "M25P80: die erase is not supported"
> +                          " by device\n");
> +            return;
> +        }
> +        break;
>      default:
>          abort();
>      }
> @@ -635,6 +672,7 @@ static void complete_collecting_data(Flash *s)
>      case ERASE4_32K:
>      case ERASE_SECTOR:
>      case ERASE4_SECTOR:
> +    case DIE_ERASE:
>          flash_erase(s, s->cur_addr, s->cmd_in_progress);
>          break;
>      case WRSR:
> @@ -881,6 +919,7 @@ static void decode_new_cmd(Flash *s, uint32_t value)
>      case PP:
>      case PP4:
>      case PP4_4:
> +    case DIE_ERASE:
>          s->needed_bytes = get_addr_length(s);
>          s->pos = 0;
>          s->len = 0;
> 

Re: [Qemu-devel] [PATCH v3 2/3] block: m25p80: Introduce die erase command

[Edgar E. Iglesias]

On Sun, Jan 08, 2017 at 09:38:53AM +0100, Marcin Krzeminski wrote:
> Modern big flash nor devices consist from more than one die.
> Some of them do not support chip erase and instead have die
> erase command that can erase one die only. This commit adds
> possibility to define number of dies in the chip and adds
> support for die erase command. Nor flash model is not strict
> thus option to disable chip eras was not added.
                                 ^^
                              erase

The commit message could be improved but codewise this looks
good to me:

Reviewed-by: Edgar E. Iglesias <edgar.iglesias@xilinx.com>


> 
> Signed-off-by: Marcin Krzeminski <mar.krzeminski@gmail.com>
> ---
>  hw/block/m25p80.c | 41 ++++++++++++++++++++++++++++++++++++++++-
>  1 file changed, 40 insertions(+), 1 deletion(-)
> 
> diff --git a/hw/block/m25p80.c b/hw/block/m25p80.c
> index 6dff81b..a9b025b 100644
> --- a/hw/block/m25p80.c
> +++ b/hw/block/m25p80.c
> @@ -73,6 +73,12 @@ typedef struct FlashPartInfo {
>      uint32_t n_sectors;
>      uint32_t page_size;
>      uint16_t flags;
> +    /*
> +     * Big sized spi nor are often stacked devices, thus sometime
> +     * replace chip erase with die erase.
> +     * This field inform how many die is in the chip.
> +     */
> +    uint8_t die_cnt;
>  } FlashPartInfo;
>  
>  /* adapted from linux */
> @@ -90,7 +96,8 @@ typedef struct FlashPartInfo {
>      .sector_size = (_sector_size),\
>      .n_sectors = (_n_sectors),\
>      .page_size = 256,\
> -    .flags = (_flags),
> +    .flags = (_flags),\
> +    .die_cnt = 0
>  
>  #define INFO6(_part_name, _jedec_id, _ext_id, _sector_size, _n_sectors, _flags)\
>      .part_name = _part_name,\
> @@ -107,6 +114,24 @@ typedef struct FlashPartInfo {
>      .n_sectors = (_n_sectors),\
>      .page_size = 256,\
>      .flags = (_flags),\
> +    .die_cnt = 0
> +
> +#define INFO_STACKED(_part_name, _jedec_id, _ext_id, _sector_size, _n_sectors,\
> +                    _flags, _die_cnt)\
> +    .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),\
> +    .die_cnt = _die_cnt
>  
>  #define JEDEC_NUMONYX 0x20
>  #define JEDEC_WINBOND 0xEF
> @@ -359,6 +384,8 @@ typedef enum {
>  
>      REVCR = 0x65,
>      WEVCR = 0x61,
> +
> +    DIE_ERASE = 0xC4,
>  } FlashCMD;
>  
>  typedef enum {
> @@ -514,6 +541,16 @@ static void flash_erase(Flash *s, int offset, FlashCMD cmd)
>      case BULK_ERASE:
>          len = s->size;
>          break;
> +    case DIE_ERASE:
> +        if (s->pi->die_cnt) {
> +            len = s->size / s->pi->die_cnt;
> +            offset = offset & (~(len - 1));
> +        } else {
> +            qemu_log_mask(LOG_GUEST_ERROR, "M25P80: die erase is not supported"
> +                          " by device\n");
> +            return;
> +        }
> +        break;
>      default:
>          abort();
>      }
> @@ -635,6 +672,7 @@ static void complete_collecting_data(Flash *s)
>      case ERASE4_32K:
>      case ERASE_SECTOR:
>      case ERASE4_SECTOR:
> +    case DIE_ERASE:
>          flash_erase(s, s->cur_addr, s->cmd_in_progress);
>          break;
>      case WRSR:
> @@ -881,6 +919,7 @@ static void decode_new_cmd(Flash *s, uint32_t value)
>      case PP:
>      case PP4:
>      case PP4_4:
> +    case DIE_ERASE:
>          s->needed_bytes = get_addr_length(s);
>          s->pos = 0;
>          s->len = 0;
> -- 
> 2.9.3
> 

Re: [Qemu-devel] [PATCH v3 3/3] block: m25p80: Improve 1GiB Micron flash definition

[Edgar E. Iglesias]

On Sun, Jan 08, 2017 at 09:38:54AM +0100, Marcin Krzeminski wrote:
> n25q00 and mt25q01 devices share the same JEDEC ID. The difference
> between those two devices is number of dies and one bit in extended
> JEDEC bytes. This commit adds proper entry for both devices by
> introduction the number of dies and and new 25q00 entries.
> 
> Signed-off-by: Marcin Krzeminski <mar.krzeminski@gmail.com>

Reviewed-by: Edgar E. Iglesias <edgar.iglesias@xilinx.com>


> ---
>  hw/block/m25p80.c | 6 ++++--
>  1 file changed, 4 insertions(+), 2 deletions(-)
> 
> diff --git a/hw/block/m25p80.c b/hw/block/m25p80.c
> index a9b025b..fb5cef3 100644
> --- a/hw/block/m25p80.c
> +++ b/hw/block/m25p80.c
> @@ -242,8 +242,10 @@ static const FlashPartInfo known_devices[] = {
>      { 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) },
> +    { INFO_STACKED("n25q00",    0x20ba21, 0x1000, 64 << 10, 2048, ER_4K, 4) },
> +    { INFO_STACKED("n25q00a",   0x20bb21, 0x1000, 64 << 10, 2048, ER_4K, 4) },
> +    { INFO_STACKED("mt25ql01g", 0x20ba21, 0x1040, 64 << 10, 2048, ER_4K, 2) },
> +    { INFO_STACKED("mt25qu01g", 0x20bb21, 0x1040, 64 << 10, 2048, ER_4K, 2) },
>  
>      /* Spansion -- single (large) sector size only, at least
>       * for the chips listed here (without boot sectors).
> -- 
> 2.9.3
> 

Re: [Qemu-devel] [PATCH v3 2/3] block: m25p80: Introduce die erase command

[Marcin Krzemiński]

2017-01-09 12:10 GMT+01:00 Edgar E. Iglesias <edgar.iglesias@xilinx.com>:
> On Sun, Jan 08, 2017 at 09:38:53AM +0100, Marcin Krzeminski wrote:
>> Modern big flash nor devices consist from more than one die.
>> Some of them do not support chip erase and instead have die
>> erase command that can erase one die only. This commit adds
>> possibility to define number of dies in the chip and adds
>> support for die erase command. Nor flash model is not strict
>> thus option to disable chip eras was not added.
>                                  ^^
>                               erase
>
> The commit message could be improved but codewise this looks
> good to me:

Beside fixing the typo, some more information/changes should I add?

Thanks,
Marcin
>
> Reviewed-by: Edgar E. Iglesias <edgar.iglesias@xilinx.com>
>
>
>>
>> Signed-off-by: Marcin Krzeminski <mar.krzeminski@gmail.com>
>> ---
>>  hw/block/m25p80.c | 41 ++++++++++++++++++++++++++++++++++++++++-
>>  1 file changed, 40 insertions(+), 1 deletion(-)
>>
>> diff --git a/hw/block/m25p80.c b/hw/block/m25p80.c
>> index 6dff81b..a9b025b 100644
>> --- a/hw/block/m25p80.c
>> +++ b/hw/block/m25p80.c
>> @@ -73,6 +73,12 @@ typedef struct FlashPartInfo {
>>      uint32_t n_sectors;
>>      uint32_t page_size;
>>      uint16_t flags;
>> +    /*
>> +     * Big sized spi nor are often stacked devices, thus sometime
>> +     * replace chip erase with die erase.
>> +     * This field inform how many die is in the chip.
>> +     */
>> +    uint8_t die_cnt;
>>  } FlashPartInfo;
>>
>>  /* adapted from linux */
>> @@ -90,7 +96,8 @@ typedef struct FlashPartInfo {
>>      .sector_size = (_sector_size),\
>>      .n_sectors = (_n_sectors),\
>>      .page_size = 256,\
>> -    .flags = (_flags),
>> +    .flags = (_flags),\
>> +    .die_cnt = 0
>>
>>  #define INFO6(_part_name, _jedec_id, _ext_id, _sector_size, _n_sectors, _flags)\
>>      .part_name = _part_name,\
>> @@ -107,6 +114,24 @@ typedef struct FlashPartInfo {
>>      .n_sectors = (_n_sectors),\
>>      .page_size = 256,\
>>      .flags = (_flags),\
>> +    .die_cnt = 0
>> +
>> +#define INFO_STACKED(_part_name, _jedec_id, _ext_id, _sector_size, _n_sectors,\
>> +                    _flags, _die_cnt)\
>> +    .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),\
>> +    .die_cnt = _die_cnt
>>
>>  #define JEDEC_NUMONYX 0x20
>>  #define JEDEC_WINBOND 0xEF
>> @@ -359,6 +384,8 @@ typedef enum {
>>
>>      REVCR = 0x65,
>>      WEVCR = 0x61,
>> +
>> +    DIE_ERASE = 0xC4,
>>  } FlashCMD;
>>
>>  typedef enum {
>> @@ -514,6 +541,16 @@ static void flash_erase(Flash *s, int offset, FlashCMD cmd)
>>      case BULK_ERASE:
>>          len = s->size;
>>          break;
>> +    case DIE_ERASE:
>> +        if (s->pi->die_cnt) {
>> +            len = s->size / s->pi->die_cnt;
>> +            offset = offset & (~(len - 1));
>> +        } else {
>> +            qemu_log_mask(LOG_GUEST_ERROR, "M25P80: die erase is not supported"
>> +                          " by device\n");
>> +            return;
>> +        }
>> +        break;
>>      default:
>>          abort();
>>      }
>> @@ -635,6 +672,7 @@ static void complete_collecting_data(Flash *s)
>>      case ERASE4_32K:
>>      case ERASE_SECTOR:
>>      case ERASE4_SECTOR:
>> +    case DIE_ERASE:
>>          flash_erase(s, s->cur_addr, s->cmd_in_progress);
>>          break;
>>      case WRSR:
>> @@ -881,6 +919,7 @@ static void decode_new_cmd(Flash *s, uint32_t value)
>>      case PP:
>>      case PP4:
>>      case PP4_4:
>> +    case DIE_ERASE:
>>          s->needed_bytes = get_addr_length(s);
>>          s->pos = 0;
>>          s->len = 0;
>> --
>> 2.9.3
>>

Re: [Qemu-devel] [PATCH v3 2/3] block: m25p80: Introduce die erase command

[Edgar E. Iglesias]

On Mon, Jan 09, 2017 at 12:44:43PM +0100, Marcin Krzemiński wrote:
> 2017-01-09 12:10 GMT+01:00 Edgar E. Iglesias <edgar.iglesias@xilinx.com>:
> > On Sun, Jan 08, 2017 at 09:38:53AM +0100, Marcin Krzeminski wrote:
> >> Modern big flash nor devices consist from more than one die.
> >> Some of them do not support chip erase and instead have die
> >> erase command that can erase one die only. This commit adds
> >> possibility to define number of dies in the chip and adds
> >> support for die erase command. Nor flash model is not strict
> >> thus option to disable chip eras was not added.
> >                                  ^^
> >                               erase
> >
> > The commit message could be improved but codewise this looks
> > good to me:
> 
> Beside fixing the typo, some more information/changes should I add?

It's not very important but:

I'm not a native english speaker but here are a few suggestions:
I would suggest spelling out NOR flashes to avoid confusion with
the word nor.

I also think it should say

"... devices consist of more than one die."
"This commit adds support for defining the number of dies..."

Perhaps Peter can suggest something better.

Cheers,
Edgar


> 
> Thanks,
> Marcin
> >
> > Reviewed-by: Edgar E. Iglesias <edgar.iglesias@xilinx.com>
> >
> >
> >>
> >> Signed-off-by: Marcin Krzeminski <mar.krzeminski@gmail.com>
> >> ---
> >>  hw/block/m25p80.c | 41 ++++++++++++++++++++++++++++++++++++++++-
> >>  1 file changed, 40 insertions(+), 1 deletion(-)
> >>
> >> diff --git a/hw/block/m25p80.c b/hw/block/m25p80.c
> >> index 6dff81b..a9b025b 100644
> >> --- a/hw/block/m25p80.c
> >> +++ b/hw/block/m25p80.c
> >> @@ -73,6 +73,12 @@ typedef struct FlashPartInfo {
> >>      uint32_t n_sectors;
> >>      uint32_t page_size;
> >>      uint16_t flags;
> >> +    /*
> >> +     * Big sized spi nor are often stacked devices, thus sometime
> >> +     * replace chip erase with die erase.
> >> +     * This field inform how many die is in the chip.
> >> +     */
> >> +    uint8_t die_cnt;
> >>  } FlashPartInfo;
> >>
> >>  /* adapted from linux */
> >> @@ -90,7 +96,8 @@ typedef struct FlashPartInfo {
> >>      .sector_size = (_sector_size),\
> >>      .n_sectors = (_n_sectors),\
> >>      .page_size = 256,\
> >> -    .flags = (_flags),
> >> +    .flags = (_flags),\
> >> +    .die_cnt = 0
> >>
> >>  #define INFO6(_part_name, _jedec_id, _ext_id, _sector_size, _n_sectors, _flags)\
> >>      .part_name = _part_name,\
> >> @@ -107,6 +114,24 @@ typedef struct FlashPartInfo {
> >>      .n_sectors = (_n_sectors),\
> >>      .page_size = 256,\
> >>      .flags = (_flags),\
> >> +    .die_cnt = 0
> >> +
> >> +#define INFO_STACKED(_part_name, _jedec_id, _ext_id, _sector_size, _n_sectors,\
> >> +                    _flags, _die_cnt)\
> >> +    .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),\
> >> +    .die_cnt = _die_cnt
> >>
> >>  #define JEDEC_NUMONYX 0x20
> >>  #define JEDEC_WINBOND 0xEF
> >> @@ -359,6 +384,8 @@ typedef enum {
> >>
> >>      REVCR = 0x65,
> >>      WEVCR = 0x61,
> >> +
> >> +    DIE_ERASE = 0xC4,
> >>  } FlashCMD;
> >>
> >>  typedef enum {
> >> @@ -514,6 +541,16 @@ static void flash_erase(Flash *s, int offset, FlashCMD cmd)
> >>      case BULK_ERASE:
> >>          len = s->size;
> >>          break;
> >> +    case DIE_ERASE:
> >> +        if (s->pi->die_cnt) {
> >> +            len = s->size / s->pi->die_cnt;
> >> +            offset = offset & (~(len - 1));
> >> +        } else {
> >> +            qemu_log_mask(LOG_GUEST_ERROR, "M25P80: die erase is not supported"
> >> +                          " by device\n");
> >> +            return;
> >> +        }
> >> +        break;
> >>      default:
> >>          abort();
> >>      }
> >> @@ -635,6 +672,7 @@ static void complete_collecting_data(Flash *s)
> >>      case ERASE4_32K:
> >>      case ERASE_SECTOR:
> >>      case ERASE4_SECTOR:
> >> +    case DIE_ERASE:
> >>          flash_erase(s, s->cur_addr, s->cmd_in_progress);
> >>          break;
> >>      case WRSR:
> >> @@ -881,6 +919,7 @@ static void decode_new_cmd(Flash *s, uint32_t value)
> >>      case PP:
> >>      case PP4:
> >>      case PP4_4:
> >> +    case DIE_ERASE:
> >>          s->needed_bytes = get_addr_length(s);
> >>          s->pos = 0;
> >>          s->len = 0;
> >> --
> >> 2.9.3
> >>

Re: [Qemu-devel] [PATCH v3 0/3] block: m25p80: Improve mt25qu01g chip model

[Peter Maydell]

On 8 January 2017 at 08:38, Marcin Krzeminski <mar.krzeminski@gmail.com> wrote:
> This series introduce some improvememnt targeting mt25qu01g and adds
> support for n25q00 device.
>
> v3:
> * Removed debug printf
> v2:
> * Rework die erase command functionality. Since the number of dies
> is constant per chip, new field to flash description structure has
> been added (instead of dynamic code from v1).
> * Add definition for n25q00.

Thanks, applied to target-arm.next (with some tweaks to the patch 2
commit message).

-- PMM