* [PATCH v2 0/1] hw/block/m25p80: Fix Numonyx flash dummy cycle register behavior @ 2020-10-27 23:43 Joe Komlodi 2020-10-27 23:43 ` [PATCH v2 1/1] hw/block/m25p80: Fix Numonyx fast read dummy cycle count Joe Komlodi 0 siblings, 1 reply; 5+ messages in thread From: Joe Komlodi @ 2020-10-27 23:43 UTC (permalink / raw) To: qemu-devel; +Cc: figlesia, kwolf, alistair, qemu-block, mreitz Changelog: v1 -> v2 - 1/2: Change function name to be more accurate - 2/2: Dropped Hi all, The series fixes the behavior of the dummy cycle register for Numonyx flashes so it's closer to how hardware behaves. On hardware, the dummy cycles for fast read commands are set to a specific value (8 or 10) if the register is all 0s or 1s. If the register value isn't all 0s or 1s, then the flash expects the amount of cycles sent to be equal to the count in the register. Thanks! Joe Joe Komlodi (1): hw/block/m25p80: Fix Numonyx fast read dummy cycle count hw/block/m25p80.c | 26 +++++++++++++++++++++++--- 1 file changed, 23 insertions(+), 3 deletions(-) -- 2.7.4 ^ permalink raw reply [flat|nested] 5+ messages in thread
* [PATCH v2 1/1] hw/block/m25p80: Fix Numonyx fast read dummy cycle count 2020-10-27 23:43 [PATCH v2 0/1] hw/block/m25p80: Fix Numonyx flash dummy cycle register behavior Joe Komlodi @ 2020-10-27 23:43 ` Joe Komlodi 2020-10-28 9:26 ` Philippe Mathieu-Daudé 0 siblings, 1 reply; 5+ messages in thread From: Joe Komlodi @ 2020-10-27 23:43 UTC (permalink / raw) To: qemu-devel; +Cc: figlesia, kwolf, alistair, qemu-block, mreitz Numonyx chips determine the number of cycles to wait based on bits 7:4 in the volatile configuration register. However, if these bits are 0x0 or 0xF, the number of dummy cycles to wait is 10 on a QIOR or QIOR4 command, or 8 on any other currently supported fast read command. [1] [1] https://www.micron.com/-/media/client/global/documents/products/data-sheet/ nor-flash/serial-nor/mt25q/die-rev-b/mt25q_qlkt_u_02g_cbb_0.pdf ?rev=9b167fbf2b3645efba6385949a72e453 Page 34, page 39 note 5 Signed-off-by: Joe Komlodi <komlodi@xilinx.com> --- hw/block/m25p80.c | 26 +++++++++++++++++++++++--- 1 file changed, 23 insertions(+), 3 deletions(-) diff --git a/hw/block/m25p80.c b/hw/block/m25p80.c index 483925f..302ed9d 100644 --- a/hw/block/m25p80.c +++ b/hw/block/m25p80.c @@ -820,6 +820,26 @@ static void reset_memory(Flash *s) trace_m25p80_reset_done(s); } +static uint8_t numonyx_extract_cfg_num_dummies(Flash *s) +{ + uint8_t cycle_count; + uint8_t num_dummies; + assert(get_man(s) == MAN_NUMONYX); + + cycle_count = extract32(s->volatile_cfg, 4, 4); + if (cycle_count == 0x0 || cycle_count == 0x0F) { + if (s->cmd_in_progress == QIOR || s->cmd_in_progress == QIOR4) { + num_dummies = 10; + } else { + num_dummies = 8; + } + } else { + num_dummies = cycle_count; + } + + return num_dummies; +} + static void decode_fast_read_cmd(Flash *s) { s->needed_bytes = get_addr_length(s); @@ -829,7 +849,7 @@ static void decode_fast_read_cmd(Flash *s) s->needed_bytes += 8; break; case MAN_NUMONYX: - s->needed_bytes += extract32(s->volatile_cfg, 4, 4); + s->needed_bytes += numonyx_extract_cfg_num_dummies(s); break; case MAN_MACRONIX: if (extract32(s->volatile_cfg, 6, 2) == 1) { @@ -868,7 +888,7 @@ static void decode_dio_read_cmd(Flash *s) ); break; case MAN_NUMONYX: - s->needed_bytes += extract32(s->volatile_cfg, 4, 4); + s->needed_bytes += numonyx_extract_cfg_num_dummies(s); break; case MAN_MACRONIX: switch (extract32(s->volatile_cfg, 6, 2)) { @@ -908,7 +928,7 @@ static void decode_qio_read_cmd(Flash *s) ); break; case MAN_NUMONYX: - s->needed_bytes += extract32(s->volatile_cfg, 4, 4); + s->needed_bytes += numonyx_extract_cfg_num_dummies(s); break; case MAN_MACRONIX: switch (extract32(s->volatile_cfg, 6, 2)) { -- 2.7.4 ^ permalink raw reply related [flat|nested] 5+ messages in thread
* Re: [PATCH v2 1/1] hw/block/m25p80: Fix Numonyx fast read dummy cycle count 2020-10-27 23:43 ` [PATCH v2 1/1] hw/block/m25p80: Fix Numonyx fast read dummy cycle count Joe Komlodi @ 2020-10-28 9:26 ` Philippe Mathieu-Daudé 2020-10-28 23:15 ` Joe Komlodi 0 siblings, 1 reply; 5+ messages in thread From: Philippe Mathieu-Daudé @ 2020-10-28 9:26 UTC (permalink / raw) To: Joe Komlodi, qemu-devel; +Cc: figlesia, kwolf, alistair, qemu-block, mreitz Hi Joe, On 10/28/20 12:43 AM, Joe Komlodi wrote: > Numonyx chips determine the number of cycles to wait based on bits 7:4 in the > volatile configuration register. > > However, if these bits are 0x0 or 0xF, the number of dummy cycles to wait is > 10 on a QIOR or QIOR4 command, or 8 on any other currently supported > fast read command. [1] > > [1] https://www.micron.com/-/media/client/global/documents/products/data-sheet/ > nor-flash/serial-nor/mt25q/die-rev-b/mt25q_qlkt_u_02g_cbb_0.pdf > ?rev=9b167fbf2b3645efba6385949a72e453 Please use single line for URL (even longer than 80 characters): https://www.micron.com/-/media/client/global/documents/products/data-sheet/nor-flash/serial-nor/mt25q/die-rev-b/mt25q_qlkt_u_02g_cbb_0.pdf Or use Datasheet: "MT25QU02GCBB Rev. H 05/19 EN" > Page 34, page 39 note 5 The note 5 is not restricted to QIOR/QIOR4 commands, so your patch seems incomplete. > > Signed-off-by: Joe Komlodi <komlodi@xilinx.com> > --- > hw/block/m25p80.c | 26 +++++++++++++++++++++++--- > 1 file changed, 23 insertions(+), 3 deletions(-) > > diff --git a/hw/block/m25p80.c b/hw/block/m25p80.c > index 483925f..302ed9d 100644 > --- a/hw/block/m25p80.c > +++ b/hw/block/m25p80.c > @@ -820,6 +820,26 @@ static void reset_memory(Flash *s) > trace_m25p80_reset_done(s); > } > > +static uint8_t numonyx_extract_cfg_num_dummies(Flash *s) > +{ > + uint8_t cycle_count; > + uint8_t num_dummies; > + assert(get_man(s) == MAN_NUMONYX); > + > + cycle_count = extract32(s->volatile_cfg, 4, 4); Could be easier to review as: num_dummies = extract32(s->volatile_cfg, 4, 4); switch (s->cmd_in_progress) { /* note 5 comment ... */ case FAST_READ: case ... /* field erased or reset in NVRAM */ if (cycle_count == 0x0 || cycle_count == 0xf) { switch (s->cmd_in_progress) { case FAST_READ: case ...: num_dummies = 10; break; case ...: case ...: /* assert(s->quad_enable); */ num_dummies = 8; break; default: qemu_log_mask(GUEST_ERROR, ...); break; } } break; default: break; } return num_dummies; > + if (cycle_count == 0x0 || cycle_count == 0x0F) { > + if (s->cmd_in_progress == QIOR || s->cmd_in_progress == QIOR4) { > + num_dummies = 10; > + } else { > + num_dummies = 8; Note, this is only true if s->quad_enable. Maybe clever to use the dumbest approach and copy the table... static uint8_t numonyx_extract_cfg_num_dummies(Flash *s) { static const unsigned dummy_clock_cycles[0x100][3] = { [FAST_READ] = {8, 8, 10}, ... }; uint8_t num_dummies = extract32(s->volatile_cfg, 4, 4); if (cycle_count == 0x0 || cycle_count == 0xf) { num_dummies = dummy_clock_cycles[s->cmd_in_progress][mode]; } return num_dummies; } > + } > + } else { > + num_dummies = cycle_count; > + } > + > + return num_dummies; > +} > + > static void decode_fast_read_cmd(Flash *s) > { > s->needed_bytes = get_addr_length(s); > @@ -829,7 +849,7 @@ static void decode_fast_read_cmd(Flash *s) > s->needed_bytes += 8; > break; > case MAN_NUMONYX: > - s->needed_bytes += extract32(s->volatile_cfg, 4, 4); > + s->needed_bytes += numonyx_extract_cfg_num_dummies(s); > break; > case MAN_MACRONIX: > if (extract32(s->volatile_cfg, 6, 2) == 1) { > @@ -868,7 +888,7 @@ static void decode_dio_read_cmd(Flash *s) > ); > break; > case MAN_NUMONYX: > - s->needed_bytes += extract32(s->volatile_cfg, 4, 4); > + s->needed_bytes += numonyx_extract_cfg_num_dummies(s); > break; > case MAN_MACRONIX: > switch (extract32(s->volatile_cfg, 6, 2)) { > @@ -908,7 +928,7 @@ static void decode_qio_read_cmd(Flash *s) > ); > break; > case MAN_NUMONYX: > - s->needed_bytes += extract32(s->volatile_cfg, 4, 4); > + s->needed_bytes += numonyx_extract_cfg_num_dummies(s); > break; > case MAN_MACRONIX: > switch (extract32(s->volatile_cfg, 6, 2)) { > ^ permalink raw reply [flat|nested] 5+ messages in thread
* RE: [PATCH v2 1/1] hw/block/m25p80: Fix Numonyx fast read dummy cycle count 2020-10-28 9:26 ` Philippe Mathieu-Daudé @ 2020-10-28 23:15 ` Joe Komlodi 2020-10-29 6:53 ` Philippe Mathieu-Daudé 0 siblings, 1 reply; 5+ messages in thread From: Joe Komlodi @ 2020-10-28 23:15 UTC (permalink / raw) To: Philippe Mathieu-Daudé, qemu-devel@nongnu.org Cc: Francisco Eduardo Iglesias, kwolf@redhat.com, alistair@alistair23.me, qemu-block@nongnu.org, mreitz@redhat.com Hi Philippe, Comments marked inline with [Joe] -----Original Message----- From: Philippe Mathieu-Daudé <philippe.mathieu.daude@gmail.com> On Behalf Of Philippe Mathieu-Daudé Sent: Wednesday, October 28, 2020 2:27 AM To: Joe Komlodi <komlodi@xilinx.com>; qemu-devel@nongnu.org Cc: Francisco Eduardo Iglesias <figlesia@xilinx.com>; kwolf@redhat.com; alistair@alistair23.me; qemu-block@nongnu.org; mreitz@redhat.com Subject: Re: [PATCH v2 1/1] hw/block/m25p80: Fix Numonyx fast read dummy cycle count Hi Joe, On 10/28/20 12:43 AM, Joe Komlodi wrote: > Numonyx chips determine the number of cycles to wait based on bits 7:4 > in the volatile configuration register. > > However, if these bits are 0x0 or 0xF, the number of dummy cycles to > wait is > 10 on a QIOR or QIOR4 command, or 8 on any other currently supported > fast read command. [1] > > [1] > https://www.micron.com/-/media/client/global/documents/products/data-s > heet/ nor-flash/serial-nor/mt25q/die-rev-b/mt25q_qlkt_u_02g_cbb_0.pdf > ?rev=9b167fbf2b3645efba6385949a72e453 Please use single line for URL (even longer than 80 characters): https://www.micron.com/-/media/client/global/documents/products/data-sheet/nor-flash/serial-nor/mt25q/die-rev-b/mt25q_qlkt_u_02g_cbb_0.pdf Or use Datasheet: "MT25QU02GCBB Rev. H 05/19 EN" [Joe] Ah, sorry about that, I'll put it all on one line in v3. > Page 34, page 39 note 5 The note 5 is not restricted to QIOR/QIOR4 commands, so your patch seems incomplete. [Joe] Right. Right now it's only checking QIOR/QIOR4 because we don't have a way to put Numonyx chips in QIO mode (s->quad_enable == true), and we don't model DDR commands. Because of those restrictions, all the read commands need 8 cycles, except for QIOR/QIOR4. > > Signed-off-by: Joe Komlodi <komlodi@xilinx.com> > --- > hw/block/m25p80.c | 26 +++++++++++++++++++++++--- > 1 file changed, 23 insertions(+), 3 deletions(-) > > diff --git a/hw/block/m25p80.c b/hw/block/m25p80.c index > 483925f..302ed9d 100644 > --- a/hw/block/m25p80.c > +++ b/hw/block/m25p80.c > @@ -820,6 +820,26 @@ static void reset_memory(Flash *s) > trace_m25p80_reset_done(s); > } > > +static uint8_t numonyx_extract_cfg_num_dummies(Flash *s) { > + uint8_t cycle_count; > + uint8_t num_dummies; > + assert(get_man(s) == MAN_NUMONYX); > + > + cycle_count = extract32(s->volatile_cfg, 4, 4); Could be easier to review as: num_dummies = extract32(s->volatile_cfg, 4, 4); switch (s->cmd_in_progress) { /* note 5 comment ... */ case FAST_READ: case ... /* field erased or reset in NVRAM */ if (cycle_count == 0x0 || cycle_count == 0xf) { switch (s->cmd_in_progress) { case FAST_READ: case ...: num_dummies = 10; break; case ...: case ...: /* assert(s->quad_enable); */ num_dummies = 8; break; default: qemu_log_mask(GUEST_ERROR, ...); break; } } break; default: break; } return num_dummies; > + if (cycle_count == 0x0 || cycle_count == 0x0F) { > + if (s->cmd_in_progress == QIOR || s->cmd_in_progress == QIOR4) { > + num_dummies = 10; > + } else { > + num_dummies = 8; Note, this is only true if s->quad_enable. Maybe clever to use the dumbest approach and copy the table... static uint8_t numonyx_extract_cfg_num_dummies(Flash *s) { static const unsigned dummy_clock_cycles[0x100][3] = { [FAST_READ] = {8, 8, 10}, ... }; uint8_t num_dummies = extract32(s->volatile_cfg, 4, 4); if (cycle_count == 0x0 || cycle_count == 0xf) { num_dummies = dummy_clock_cycles[s->cmd_in_progress][mode]; } return num_dummies; } [Joe] I think either this or the switch statement would work fine, it just depends if we want to add a way to set s->quad_enable and s->dual_enable (doesn't exist in the model) for Numonyx chips. To be the most accurate, it probably would be best to add a way to enable/disable QIO and DIO mode for Numonyx chips, then change the cycles needed accordingly. Let me know what you think. Thanks! Joe > + } > + } else { > + num_dummies = cycle_count; > + } > + > + return num_dummies; > +} > + > static void decode_fast_read_cmd(Flash *s) { > s->needed_bytes = get_addr_length(s); @@ -829,7 +849,7 @@ static > void decode_fast_read_cmd(Flash *s) > s->needed_bytes += 8; > break; > case MAN_NUMONYX: > - s->needed_bytes += extract32(s->volatile_cfg, 4, 4); > + s->needed_bytes += numonyx_extract_cfg_num_dummies(s); > break; > case MAN_MACRONIX: > if (extract32(s->volatile_cfg, 6, 2) == 1) { @@ -868,7 +888,7 > @@ static void decode_dio_read_cmd(Flash *s) > ); > break; > case MAN_NUMONYX: > - s->needed_bytes += extract32(s->volatile_cfg, 4, 4); > + s->needed_bytes += numonyx_extract_cfg_num_dummies(s); > break; > case MAN_MACRONIX: > switch (extract32(s->volatile_cfg, 6, 2)) { @@ -908,7 +928,7 > @@ static void decode_qio_read_cmd(Flash *s) > ); > break; > case MAN_NUMONYX: > - s->needed_bytes += extract32(s->volatile_cfg, 4, 4); > + s->needed_bytes += numonyx_extract_cfg_num_dummies(s); > break; > case MAN_MACRONIX: > switch (extract32(s->volatile_cfg, 6, 2)) { > ^ permalink raw reply [flat|nested] 5+ messages in thread
* Re: [PATCH v2 1/1] hw/block/m25p80: Fix Numonyx fast read dummy cycle count 2020-10-28 23:15 ` Joe Komlodi @ 2020-10-29 6:53 ` Philippe Mathieu-Daudé 0 siblings, 0 replies; 5+ messages in thread From: Philippe Mathieu-Daudé @ 2020-10-29 6:53 UTC (permalink / raw) To: Joe Komlodi, qemu-devel@nongnu.org Cc: Francisco Eduardo Iglesias, kwolf@redhat.com, alistair@alistair23.me, qemu-block@nongnu.org, mreitz@redhat.com On 10/29/20 12:15 AM, Joe Komlodi wrote: > Hi Philippe, > > Comments marked inline with [Joe] > > -----Original Message----- > From: Philippe Mathieu-Daudé <philippe.mathieu.daude@gmail.com> On Behalf Of Philippe Mathieu-Daudé > Sent: Wednesday, October 28, 2020 2:27 AM > To: Joe Komlodi <komlodi@xilinx.com>; qemu-devel@nongnu.org > Cc: Francisco Eduardo Iglesias <figlesia@xilinx.com>; kwolf@redhat.com; alistair@alistair23.me; qemu-block@nongnu.org; mreitz@redhat.com > Subject: Re: [PATCH v2 1/1] hw/block/m25p80: Fix Numonyx fast read dummy cycle count > > Hi Joe, > > On 10/28/20 12:43 AM, Joe Komlodi wrote: >> Numonyx chips determine the number of cycles to wait based on bits 7:4 >> in the volatile configuration register. >> >> However, if these bits are 0x0 or 0xF, the number of dummy cycles to >> wait is >> 10 on a QIOR or QIOR4 command, or 8 on any other currently supported >> fast read command. [1] >> >> [1] >> https://www.micron.com/-/media/client/global/documents/products/data-s >> heet/ nor-flash/serial-nor/mt25q/die-rev-b/mt25q_qlkt_u_02g_cbb_0.pdf >> ?rev=9b167fbf2b3645efba6385949a72e453 > > Please use single line for URL (even longer than 80 characters): > https://www.micron.com/-/media/client/global/documents/products/data-sheet/nor-flash/serial-nor/mt25q/die-rev-b/mt25q_qlkt_u_02g_cbb_0.pdf > > Or use > > Datasheet: "MT25QU02GCBB Rev. H 05/19 EN" > > [Joe] Ah, sorry about that, I'll put it all on one line in v3. > >> Page 34, page 39 note 5 > > The note 5 is not restricted to QIOR/QIOR4 commands, so your patch seems incomplete. > > [Joe] Right. Right now it's only checking QIOR/QIOR4 because we don't have a way to put Numonyx chips in QIO mode (s->quad_enable == true), and we don't model DDR commands. > Because of those restrictions, all the read commands need 8 cycles, except for QIOR/QIOR4. > >> >> Signed-off-by: Joe Komlodi <komlodi@xilinx.com> >> --- >> hw/block/m25p80.c | 26 +++++++++++++++++++++++--- >> 1 file changed, 23 insertions(+), 3 deletions(-) >> >> diff --git a/hw/block/m25p80.c b/hw/block/m25p80.c index >> 483925f..302ed9d 100644 >> --- a/hw/block/m25p80.c >> +++ b/hw/block/m25p80.c >> @@ -820,6 +820,26 @@ static void reset_memory(Flash *s) >> trace_m25p80_reset_done(s); >> } >> >> +static uint8_t numonyx_extract_cfg_num_dummies(Flash *s) { >> + uint8_t cycle_count; >> + uint8_t num_dummies; >> + assert(get_man(s) == MAN_NUMONYX); >> + >> + cycle_count = extract32(s->volatile_cfg, 4, 4); > > Could be easier to review as: > > num_dummies = extract32(s->volatile_cfg, 4, 4); > > switch (s->cmd_in_progress) { > /* note 5 comment ... */ > case FAST_READ: > case ... > /* field erased or reset in NVRAM */ > if (cycle_count == 0x0 || cycle_count == 0xf) { > switch (s->cmd_in_progress) { > case FAST_READ: > case ...: > num_dummies = 10; > break; > case ...: > case ...: > /* assert(s->quad_enable); */ > num_dummies = 8; > break; > default: > qemu_log_mask(GUEST_ERROR, ...); > break; > } > } > break; > default: > break; > } > > return num_dummies; > >> + if (cycle_count == 0x0 || cycle_count == 0x0F) { >> + if (s->cmd_in_progress == QIOR || s->cmd_in_progress == QIOR4) { >> + num_dummies = 10; >> + } else { >> + num_dummies = 8; > > Note, this is only true if s->quad_enable. > > Maybe clever to use the dumbest approach and copy the table... > > static uint8_t numonyx_extract_cfg_num_dummies(Flash *s) { > static const unsigned dummy_clock_cycles[0x100][3] = { > [FAST_READ] = {8, 8, 10}, > ... > }; > uint8_t num_dummies = extract32(s->volatile_cfg, 4, 4); > > if (cycle_count == 0x0 || cycle_count == 0xf) { > num_dummies = dummy_clock_cycles[s->cmd_in_progress][mode]; > } > > return num_dummies; > } > > [Joe] I think either this or the switch statement would work fine, it just depends if we want to add a way to set s->quad_enable and s->dual_enable (doesn't exist in the model) for Numonyx chips. > To be the most accurate, it probably would be best to add a way to enable/disable QIO and DIO mode for Numonyx chips, then change the cycles needed accordingly. > > Let me know what you think. It is OK to not implement unused registers/fields, but please use qemu_log_mask(LOG_UNIMP,...) there, so we can: - notice the guest is accessing unimplemented registers at runtime - easily find the missing place in the code. ^ permalink raw reply [flat|nested] 5+ messages in thread
end of thread, other threads:[~2020-10-29 6:55 UTC | newest] Thread overview: 5+ messages (download: mbox.gz follow: Atom feed -- links below jump to the message on this page -- 2020-10-27 23:43 [PATCH v2 0/1] hw/block/m25p80: Fix Numonyx flash dummy cycle register behavior Joe Komlodi 2020-10-27 23:43 ` [PATCH v2 1/1] hw/block/m25p80: Fix Numonyx fast read dummy cycle count Joe Komlodi 2020-10-28 9:26 ` Philippe Mathieu-Daudé 2020-10-28 23:15 ` Joe Komlodi 2020-10-29 6:53 ` Philippe Mathieu-Daudé
This is a public inbox, see mirroring instructions for how to clone and mirror all data and code used for this inbox; as well as URLs for NNTP newsgroup(s).