Hi Francisco, On Thu, Dec 3, 2020 at 4:38 PM Francisco Iglesias <frasse.igles...@gmail.com> wrote: > > Hi Bin and Alistair, > > On [2020 Dec 02] Wed 11:40:11, Alistair Francis wrote: > > On Sun, Nov 29, 2020 at 6:55 PM Bin Meng <bmeng...@gmail.com> wrote: > > > > > > From: Bin Meng <bin.m...@windriver.com> > > > > > > SST flashes require a dummy byte after the address bits. > > > > > > Signed-off-by: Bin Meng <bin.m...@windriver.com> > > > > I couldn't find a datasheet that says this... But the actual code > > change looks fine, so: > > > > Acked-by: Alistair Francis <alistair.fran...@wdc.com> > > > > Alistair > > > > > --- > > > > > > hw/block/m25p80.c | 3 +++ > > > 1 file changed, 3 insertions(+) > > > > > > diff --git a/hw/block/m25p80.c b/hw/block/m25p80.c > > > index 483925f..9b36762 100644 > > > --- a/hw/block/m25p80.c > > > +++ b/hw/block/m25p80.c > > > @@ -825,6 +825,9 @@ static void decode_fast_read_cmd(Flash *s) > > > s->needed_bytes = get_addr_length(s); > > > switch (get_man(s)) { > > > /* Dummy cycles - modeled with bytes writes instead of bits */ > > > + case MAN_SST: > > > + s->needed_bytes += 1; > > 1 dummy clk cycle is modelled as 1 byte write (see the comment above), so 1 > dummy byte (8 dummy clk cycles) will need +8 above.
I think you were confused by the WINBOND codes. The comments are correct. It is modeled with bytes instead of bits, so we should +=1. > An option could be to fall > through to the Windbond case below instead (since it seems to operate > likewise). > > Best regards, > Francisco Iglesias > > > > > + break; > > > case MAN_WINBOND: > > > s->needed_bytes += 8; Actually this is wrong. This should be corrected to +=1. I will prepare a patch for it. > > > break; > > > -- Regards, Bin