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

[Francisco Iglesias]

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
>