On 06/15/2016 07:40 PM, Krzeminski, Marcin (Nokia - PL/Wroclaw) wrote: > > > W dniu 15.06.2016 o 16:25, Cédric Le Goater pisze: >> On 06/15/2016 03:41 PM, marcin.krzemin...@nokia.com wrote: >>> From: Marcin Krzeminski <marcin.krzemin...@nokia.com> >>> >>> Quad and Equad modes for Spansion and Macronix flash devices. >>> This commit also includes modification and new command to manipulate >>> quad mode (status registers and dedicated commands). >>> This work is based on Pawel Lenkow work. >>> >>> Signed-off-by: Marcin Krzeminski <marcin.krzemin...@nokia.com> >>> --- >>> hw/block/m25p80.c | 70 >>> +++++++++++++++++++++++++++++++++++++++++++++++++++---- >>> 1 file changed, 65 insertions(+), 5 deletions(-) >>> >>> diff --git a/hw/block/m25p80.c b/hw/block/m25p80.c >>> index 55b4377..d1c4d46 100644 >>> --- a/hw/block/m25p80.c >>> +++ b/hw/block/m25p80.c >>> @@ -281,6 +281,7 @@ typedef enum { >>> JEDEC_READ = 0x9f, >>> BULK_ERASE = 0xc7, >>> READ_FSR = 0x70, >>> + RDCR = 0x15, >>> >>> READ = 0x03, >>> READ4 = 0x13, >>> @@ -317,6 +318,13 @@ typedef enum { >>> RESET_ENABLE = 0x66, >>> RESET_MEMORY = 0x99, >>> >>> + /* >>> + * Micron: 0x35 - enable QPI >>> + * Spansion: 0x35 - read control register >>> + */ >>> + RDCR_EQIO = 0x35, >>> + RSTQIO = 0xf5, >>> + >>> RNVCR = 0xB5, >>> WNVCR = 0xB1, >>> >>> @@ -366,6 +374,7 @@ typedef struct Flash { >>> bool write_enable; >>> bool four_bytes_address_mode; >>> bool reset_enable; >>> + bool quad_enable; >>> uint8_t ear; >>> >>> int64_t dirty_page; >>> @@ -586,6 +595,16 @@ static void complete_collecting_data(Flash *s) >>> flash_erase(s, s->cur_addr, s->cmd_in_progress); >>> break; >>> case WRSR: >>> + switch (get_man(s)) { >>> + case MAN_SPANSION: >>> + s->quad_enable = !!(s->data[1] & 0x02); >>> + break; >>> + case MAN_MACRONIX: >>> + s->quad_enable = extract32(s->data[0], 6, 1); >>> + break; >>> + default: >>> + break; >>> + } >>> if (s->write_enable) { >>> s->write_enable = false; >>> } >>> @@ -619,6 +638,7 @@ static void reset_memory(Flash *s) >>> s->state = STATE_IDLE; >>> s->write_enable = false; >>> s->reset_enable = false; >>> + s->quad_enable = false; >>> >>> switch (get_man(s)) { >>> case MAN_NUMONYX: >>> @@ -747,10 +767,20 @@ static void decode_new_cmd(Flash *s, uint32_t value) >>> >>> case WRSR: >>> if (s->write_enable) { >>> - s->needed_bytes = 1; >>> + switch (get_man(s)) { >>> + case MAN_SPANSION: >>> + s->needed_bytes = 2; >>> + s->state = STATE_COLLECTING_DATA; >>> + break; >>> + case MAN_MACRONIX: >>> + s->needed_bytes = 2; >>> + s->state = STATE_COLLECTING_VAR_LEN_DATA; >>> + break; >> >> ah. I am glad you address this topic because I am having a little issue >> with the mx25l25635e and the mx25l25635f. >> >> mx25l25635e is a macronix which supports the WRSR command with one byte >> and the mx25l25635f needs two. The fun part is that they have the same >> JEDEC : 0xc22019. >> >> So not all macronix need 2 bytes. Should we add a flag for that ? >> > I think this case should be partially handled with path 4 and this one. > The new state (patch 4) was introduced to allow use variable write cmds. > As in my case mx66u51235f allow write up to 2 bytes, but linux writes only > one, > and that is allowed and works on real HW. From the datasheet mx25l25635f > should behave exactly same way. > In case you mention, the problem might pop up, when guest will try > to write 2 bytes to mx25l25635e. Real HW will treat second byte as new > command, > but this model accept this situation.
qemu complains a bit but that's OK. > Generally my approach and I think original author was to not emulate exactly > real hw behaviour but allow to model it in such way the quest can boot up and > use it. > If we go witch very restricted command behaviour we will end up in unreadable > code > (eg. newest Spansions/Cypress family does not support extended address > register, > but such commands will work in this model). The question is if you really > need to flag > and support this. If yes additional if is not a problem. I will wait for your patchset to be merged and then see if the mx25l25635f needs anything more. This is really minor. It should be a onliner. Thanks, C. >> >>> + default: >>> + s->needed_bytes = 1; >>> + s->state = STATE_COLLECTING_DATA; >>> + } >>> s->pos = 0; >>> - s->len = 0; >>> - s->state = STATE_COLLECTING_DATA; >>> } >>> break; >>> >>> @@ -763,6 +793,9 @@ static void decode_new_cmd(Flash *s, uint32_t value) >>> >>> case RDSR: >>> s->data[0] = (!!s->write_enable) << 1; >>> + if (get_man(s) == MAN_MACRONIX) { >>> + s->data[0] |= (!!s->quad_enable) << 6; >>> + } >>> s->pos = 0; >>> s->len = 1; >>> s->state = STATE_READING_DATA; >>> @@ -789,6 +822,14 @@ static void decode_new_cmd(Flash *s, uint32_t value) >>> s->state = STATE_READING_DATA; >>> break; >>> >>> + case RDCR: >>> + s->data[0] = s->volatile_cfg & 0xFF; >>> + s->data[0] |= (!!s->four_bytes_address_mode) << 5; >>> + s->pos = 0; >>> + s->len = 1; >>> + s->state = STATE_READING_DATA; >>> + break; >>> + >>> case BULK_ERASE: >>> if (s->write_enable) { >>> DB_PRINT_L(0, "chip erase\n"); >>> @@ -828,7 +869,7 @@ static void decode_new_cmd(Flash *s, uint32_t value) >>> s->state = STATE_READING_DATA; >>> break; >>> case WNVCR: >>> - if (s->write_enable) { >>> + if (s->write_enable && get_man(s) == MAN_NUMONYX) { >>> s->needed_bytes = 2; >>> s->pos = 0; >>> s->len = 0; >>> @@ -871,6 +912,24 @@ static void decode_new_cmd(Flash *s, uint32_t value) >>> reset_memory(s); >>> } >>> break; >>> + case RDCR_EQIO: >>> + switch (get_man(s)) { >>> + case MAN_SPANSION: >>> + s->data[0] = (!!s->quad_enable) << 1; >>> + s->pos = 0; >>> + s->len = 1; >>> + s->state = STATE_READING_DATA; >>> + break; >>> + case MAN_MACRONIX: >>> + s->quad_enable = true; >>> + break; >>> + default: >>> + break; >>> + } >>> + break; >>> + case RSTQIO: >>> + s->quad_enable = false; >>> + break; >>> default: >>> qemu_log_mask(LOG_GUEST_ERROR, "M25P80: Unknown cmd %x\n", value); >>> break; >>> @@ -999,7 +1058,7 @@ static Property m25p80_properties[] = { >>> >>> static const VMStateDescription vmstate_m25p80 = { >>> .name = "xilinx_spi", >>> - .version_id = 2, >>> + .version_id = 3, >>> .minimum_version_id = 1, >>> .pre_save = m25p80_pre_save, >>> .fields = (VMStateField[]) { >>> @@ -1017,6 +1076,7 @@ static const VMStateDescription vmstate_m25p80 = { >>> VMSTATE_UINT32_V(nonvolatile_cfg, Flash, 2), >>> VMSTATE_UINT32_V(volatile_cfg, Flash, 2), >>> VMSTATE_UINT32_V(enh_volatile_cfg, Flash, 2), >>> + VMSTATE_BOOL_V(quad_enable, Flash, 3), >>> VMSTATE_END_OF_LIST() >>> } >>> }; >>> >> >> >> >