| Message ID | 1601425716-204629-2-git-send-email-komlodi@xilinx.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
| Series | hw/block/m25p80: Fix Numonyx flash dummy cycle register behavior | expand |
Hi Joe, On Tue, Sep 29, 2020 at 05:28:35PM -0700, 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] http://www.micron.com/-/media/client/global/documents/products/ > data-sheet/nor-flash/serial-nor/n25q/n25q_512mb_1_8v_65nm.pdf > > Page 22 note 2, and page 30 notes 5 and 10. > > 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..43830c9 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_fast_read_num_dummies(Flash *s) Should we rename the function to something like 'numonyx_extract_cfg_num_dummies' (since it is not only used inside 'decode_fast_read_cmd')? > +{ > + 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) { QOR and QOR4 also has 10 dummy cycles on default so we will have to check for those aswell, perhaps something similar like below migth work: uint8_t n_dummies = extract32(s->volatile_cfg, 4, 4); if (!n_dummies || n_dummies == 0xF) { switch(s->cmd_in_progress){ case QOR: case QOR4 case QIOR: case QIOR4: n_dummies = 10; break; default: n_dummies = 8; break; } } return n_dummies; Best regards, Francisco Iglesias > + 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_fast_read_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_fast_read_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_fast_read_num_dummies(s); > break; > case MAN_MACRONIX: > switch (extract32(s->volatile_cfg, 6, 2)) { > -- > 2.7.4 >
Hi Francisco, Comments marked with [Joe] -----Original Message----- From: Francisco Iglesias <francisco.iglesias@xilinx.com> Sent: Tuesday, October 20, 2020 6:50 AM To: Joe Komlodi <komlodi@xilinx.com> Cc: qemu-devel@nongnu.org; alistair@alistair23.me; kwolf@redhat.com; mreitz@redhat.com; qemu-block@nongnu.org Subject: Re: [PATCH 1/2] hw/block/m25p80: Fix Numonyx dummy cycle register behavior Hi Joe, On Tue, Sep 29, 2020 at 05:28:35PM -0700, 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] http://www.micron.com/-/media/client/global/documents/products/ > data-sheet/nor-flash/serial-nor/n25q/n25q_512mb_1_8v_65nm.pdf > > Page 22 note 2, and page 30 notes 5 and 10. > > 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..43830c9 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_fast_read_num_dummies(Flash *s) Should we rename the function to something like 'numonyx_extract_cfg_num_dummies' (since it is not only used inside 'decode_fast_read_cmd')? [Joe] Yeah, that name makes more sense. > +{ > + 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) { QOR and QOR4 also has 10 dummy cycles on default so we will have to check for those aswell, perhaps something similar like below migth work: uint8_t n_dummies = extract32(s->volatile_cfg, 4, 4); if (!n_dummies || n_dummies == 0xF) { switch(s->cmd_in_progress){ case QOR: case QOR4 case QIOR: case QIOR4: n_dummies = 10; break; default: n_dummies = 8; break; } } return n_dummies; [Joe] As talked about offline, the datasheet in the commit message just has confusing wording. 8 dummies for QOR seems to be correct, and I'll update the datasheet in the commit message with one that's more clear. Thanks! Joe Best regards, Francisco Iglesias > + 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_fast_read_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_fast_read_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_fast_read_num_dummies(s); > break; > case MAN_MACRONIX: > switch (extract32(s->volatile_cfg, 6, 2)) { > -- > 2.7.4 >
diff --git a/hw/block/m25p80.c b/hw/block/m25p80.c index 483925f..43830c9 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_fast_read_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_fast_read_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_fast_read_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_fast_read_num_dummies(s); break; case MAN_MACRONIX: switch (extract32(s->volatile_cfg, 6, 2)) {
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] http://www.micron.com/-/media/client/global/documents/products/ data-sheet/nor-flash/serial-nor/n25q/n25q_512mb_1_8v_65nm.pdf Page 22 note 2, and page 30 notes 5 and 10. Signed-off-by: Joe Komlodi <komlodi@xilinx.com> --- hw/block/m25p80.c | 26 +++++++++++++++++++++++--- 1 file changed, 23 insertions(+), 3 deletions(-)