[PULL 20/20] hw/block/m25p80: Fix Numonyx fast read dummy cycle count

There is a newer version of this series
[PULL 20/20] hw/block/m25p80: Fix Numonyx fast read dummy cycle count
Posted by Peter Maydell 4 years, 4 months ago
From: Joe Komlodi <joe.komlodi@xilinx.com>

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 for QIOR and QIOR4 commands or when in QIO mode, and otherwise 8 for
the currently supported fast read commands. [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

Signed-off-by: Joe Komlodi <komlodi@xilinx.com>
Reviewed-by: Francisco Iglesias <francisco.iglesias@xilinx.com>
Message-id: 1605568264-26376-5-git-send-email-komlodi@xilinx.com
Signed-off-by: Peter Maydell <peter.maydell@linaro.org>
---
 hw/block/m25p80.c | 30 +++++++++++++++++++++++++++---
 1 file changed, 27 insertions(+), 3 deletions(-)

diff --git a/hw/block/m25p80.c b/hw/block/m25p80.c
index f1d7da65c85..c45afdd2cb3 100644
--- a/hw/block/m25p80.c
+++ b/hw/block/m25p80.c
@@ -837,6 +837,30 @@ static uint8_t numonyx_mode(Flash *s)
     }
 }
 
+static uint8_t numonyx_extract_cfg_num_dummies(Flash *s)
+{
+    uint8_t num_dummies;
+    uint8_t mode;
+    assert(get_man(s) == MAN_NUMONYX);
+
+    mode = numonyx_mode(s);
+    num_dummies = extract32(s->volatile_cfg, 4, 4);
+
+    if (num_dummies == 0x0 || num_dummies == 0xf) {
+        switch (s->cmd_in_progress) {
+        case QIOR:
+        case QIOR4:
+            num_dummies = 10;
+            break;
+        default:
+            num_dummies = (mode == MODE_QIO) ? 10 : 8;
+            break;
+        }
+    }
+
+    return num_dummies;
+}
+
 static void decode_fast_read_cmd(Flash *s)
 {
     s->needed_bytes = get_addr_length(s);
@@ -846,7 +870,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) {
@@ -885,7 +909,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)) {
@@ -925,7 +949,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.20.1


Re: [PULL 20/20] hw/block/m25p80: Fix Numonyx fast read dummy cycle count
Posted by Bin Meng 4 years, 4 months ago
Hi Joe,

On Tue, Dec 15, 2020 at 10:35 PM Peter Maydell <peter.maydell@linaro.org> wrote:
>
> From: Joe Komlodi <joe.komlodi@xilinx.com>
>
> 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 for QIOR and QIOR4 commands or when in QIO mode, and otherwise 8 for
> the currently supported fast read commands. [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
>
> Signed-off-by: Joe Komlodi <komlodi@xilinx.com>
> Reviewed-by: Francisco Iglesias <francisco.iglesias@xilinx.com>
> Message-id: 1605568264-26376-5-git-send-email-komlodi@xilinx.com
> Signed-off-by: Peter Maydell <peter.maydell@linaro.org>
> ---
>  hw/block/m25p80.c | 30 +++++++++++++++++++++++++++---
>  1 file changed, 27 insertions(+), 3 deletions(-)
>

Sorry for jumping in, but I just noticed this patch.

I believe you tested this with Xilinx SPIPS but not some other controllers.
Francisco and I had a discussion about dummy cycles implementation
with different SPI controllers @
http://patchwork.ozlabs.org/project/qemu-devel/patch/1606704602-59435-1-git-send-email-bmeng.cn@gmail.com/
I would like to hear your thoughts. I think we should figure out a
solution that fits all types of controllers.

Regards,
Bin

Re: [PULL 20/20] hw/block/m25p80: Fix Numonyx fast read dummy cycle count
Posted by Peter Maydell 4 years, 4 months ago
On Tue, 15 Dec 2020 at 15:06, Bin Meng <bmeng.cn@gmail.com> wrote:
>
> Hi Joe,
>
> On Tue, Dec 15, 2020 at 10:35 PM Peter Maydell <peter.maydell@linaro.org> wrote:
> >
> > From: Joe Komlodi <joe.komlodi@xilinx.com>
> >
> > 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 for QIOR and QIOR4 commands or when in QIO mode, and otherwise 8 for
> > the currently supported fast read commands. [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
> >
> > Signed-off-by: Joe Komlodi <komlodi@xilinx.com>
> > Reviewed-by: Francisco Iglesias <francisco.iglesias@xilinx.com>
> > Message-id: 1605568264-26376-5-git-send-email-komlodi@xilinx.com
> > Signed-off-by: Peter Maydell <peter.maydell@linaro.org>
> > ---
> >  hw/block/m25p80.c | 30 +++++++++++++++++++++++++++---
> >  1 file changed, 27 insertions(+), 3 deletions(-)
> >
>
> Sorry for jumping in, but I just noticed this patch.
>
> I believe you tested this with Xilinx SPIPS but not some other controllers.
> Francisco and I had a discussion about dummy cycles implementation
> with different SPI controllers @
> http://patchwork.ozlabs.org/project/qemu-devel/patch/1606704602-59435-1-git-send-email-bmeng.cn@gmail.com/
> I would like to hear your thoughts. I think we should figure out a
> solution that fits all types of controllers.

I don't have an opinion on the technical question. Do you want me
to drop this patch from the pullreq ?

thanks
-- PMM

Re: [PULL 20/20] hw/block/m25p80: Fix Numonyx fast read dummy cycle count
Posted by Francisco Iglesias 4 years, 4 months ago
Hello Peter,

On [2020 Dec 15] Tue 15:11:00, Peter Maydell wrote:
> On Tue, 15 Dec 2020 at 15:06, Bin Meng <bmeng.cn@gmail.com> wrote:
> >
> > Hi Joe,
> >
> > On Tue, Dec 15, 2020 at 10:35 PM Peter Maydell <peter.maydell@linaro.org> wrote:
> > >
> > > From: Joe Komlodi <joe.komlodi@xilinx.com>
> > >
> > > 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 for QIOR and QIOR4 commands or when in QIO mode, and otherwise 8 for
> > > the currently supported fast read commands. [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
> > >
> > > Signed-off-by: Joe Komlodi <komlodi@xilinx.com>
> > > Reviewed-by: Francisco Iglesias <francisco.iglesias@xilinx.com>
> > > Message-id: 1605568264-26376-5-git-send-email-komlodi@xilinx.com
> > > Signed-off-by: Peter Maydell <peter.maydell@linaro.org>
> > > ---
> > >  hw/block/m25p80.c | 30 +++++++++++++++++++++++++++---
> > >  1 file changed, 27 insertions(+), 3 deletions(-)
> > >
> >
> > Sorry for jumping in, but I just noticed this patch.
> >
> > I believe you tested this with Xilinx SPIPS but not some other controllers.
> > Francisco and I had a discussion about dummy cycles implementation
> > with different SPI controllers @
> > http://patchwork.ozlabs.org/project/qemu-devel/patch/1606704602-59435-1-git-send-email-bmeng.cn@gmail.com/
> > I would like to hear your thoughts. I think we should figure out a
> > solution that fits all types of controllers.
> 
> I don't have an opinion on the technical question. Do you want me
> to drop this patch from the pullreq ?

The patch is correct, it hasn't changed anything regarding how dummy cycles are
modelled in m25p80 (nor this command currently works), it just corrects the
situtation for when the volatile configuration register contains 0x0 or 0xF (as
the commit message mentions).

Best regards,
Francisco Iglesias

> 
> thanks
> -- PMM
> 

Re: [PULL 20/20] hw/block/m25p80: Fix Numonyx fast read dummy cycle count
Posted by Peter Maydell 4 years, 4 months ago
On Tue, 15 Dec 2020 at 15:42, Francisco Iglesias
<frasse.iglesias@gmail.com> wrote:
>
> Hello Peter,
>
> On [2020 Dec 15] Tue 15:11:00, Peter Maydell wrote:
> > On Tue, 15 Dec 2020 at 15:06, Bin Meng <bmeng.cn@gmail.com> wrote:
> > > I believe you tested this with Xilinx SPIPS but not some other controllers.
> > > Francisco and I had a discussion about dummy cycles implementation
> > > with different SPI controllers @
> > > http://patchwork.ozlabs.org/project/qemu-devel/patch/1606704602-59435-1-git-send-email-bmeng.cn@gmail.com/
> > > I would like to hear your thoughts. I think we should figure out a
> > > solution that fits all types of controllers.
> >
> > I don't have an opinion on the technical question. Do you want me
> > to drop this patch from the pullreq ?
>
> The patch is correct, it hasn't changed anything regarding how dummy cycles are
> modelled in m25p80 (nor this command currently works), it just corrects the
> situtation for when the volatile configuration register contains 0x0 or 0xF (as
> the commit message mentions).

OK. I've applied the pullreq (partly because this is my last working
day of the year and I don't have the time to respin it). We can
always revert/add fixes in January if necessary.

thanks
- PMM