From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from eggs.gnu.org ([2001:4830:134:3::10]:48069) by lists.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1cQZIF-0004ro-00 for qemu-devel@nongnu.org; Mon, 09 Jan 2017 07:43:20 -0500 Received: from Debian-exim by eggs.gnu.org with spam-scanned (Exim 4.71) (envelope-from ) id 1cQZI9-0006R6-U1 for qemu-devel@nongnu.org; Mon, 09 Jan 2017 07:43:19 -0500 Date: Mon, 9 Jan 2017 12:10:04 +0100 From: "Edgar E. Iglesias" Message-ID: <20170109111004.GH14990@toto> References: <20170108083854.5006-1-mar.krzeminski@gmail.com> <20170108083854.5006-3-mar.krzeminski@gmail.com> MIME-Version: 1.0 Content-Type: text/plain; charset="us-ascii" Content-Disposition: inline In-Reply-To: <20170108083854.5006-3-mar.krzeminski@gmail.com> Subject: Re: [Qemu-devel] [PATCH v3 2/3] block: m25p80: Introduce die erase command List-Id: List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , To: Marcin Krzeminski Cc: qemu-devel@nongnu.org, peter.maydell@linaro.org, qemu-arm@nongnu.org, clg@kaod.org 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 > > Signed-off-by: Marcin Krzeminski > --- > 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 >