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

["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:

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
>