Re: [Qemu-devel] [PATCH] m25p80: Fix QIOR/DIOR handling for Winbond

[Krzeminski, Marcin (Nokia - PL/Wroclaw)]

> -----Original Message-----
> From: Cédric Le Goater [mailto:address@hidden
> Sent: Thursday, July 07, 2016 2:57 PM
> To: Krzeminski, Marcin (Nokia - PL/Wroclaw)
> <address@hidden>; address@hidden
> Cc: address@hidden; address@hidden;
> address@hidden; address@hidden
> Subject: Re: [PATCH] m25p80: Fix QIOR/DIOR handling for Winbond
> 
> Hello Marcin,
> 
> On 07/06/2016 02:43 PM, address@hidden wrote:
> > From: Marcin Krzeminski <address@hidden>
> >
> > Winbond also support continuous read mode, but as an opposite for
> > other flash type read mode clock cycles are included to dummy cycles
> number.
> > This path add proper handling of read mode byte and update needed
> > dummy cycles. QPI mode and dummy cycles configuration are not
> supported.
> >
> > Signed-off-by: Marcin Krzeminski <address@hidden>
> > ---
> >  hw/block/m25p80.c | 6 ++++--
> >  1 file changed, 4 insertions(+), 2 deletions(-)
> >
> > diff --git a/hw/block/m25p80.c b/hw/block/m25p80.c index
> > d9b2793..a349544 100644
> > --- a/hw/block/m25p80.c
> > +++ b/hw/block/m25p80.c
> > @@ -149,6 +149,7 @@ typedef struct FlashPartInfo {  */
> >
> >  #define SPANSION_CONTINUOUS_READ_MODE_CMD_LEN 1
> > +#define WINBOND_CONTINUOUS_READ_MODE_CMD_LEN 1
> 
> I checked the W25Q256FV datasheet and
> 
> >  static const FlashPartInfo known_devices[] = {
> >      /* Atmel -- some are (confusingly) marketed as "DataFlash" */ @@
> > -775,7 +776,7 @@ static void decode_dio_read_cmd(Flash *s)
> >      /* Dummy cycles modeled with bytes writes instead of bits */
> >      switch (get_man(s)) {
> >      case MAN_WINBOND:
> > -        s->needed_bytes += 8;
> > +        s->needed_bytes +=
> WINBOND_CONTINUOUS_READ_MODE_CMD_LEN;
> 
> this looks fine,
> 
> >          break;
> >      case MAN_SPANSION:
> >          s->needed_bytes +=
> SPANSION_CONTINUOUS_READ_MODE_CMD_LEN;
> > @@ -814,7 +815,8 @@ static void decode_qio_read_cmd(Flash *s)
> >      /* Dummy cycles modeled with bytes writes instead of bits */
> >      switch (get_man(s)) {
> >      case MAN_WINBOND:
> > -        s->needed_bytes += 8;
> > +        s->needed_bytes +=
> WINBOND_CONTINUOUS_READ_MODE_CMD_LEN;
> > +        s->needed_bytes += 4;
> 
> but I don't understand the above. I see the address + M7-0 + 2 dummies.
> 
Hello Cedric,

This is because the problematic interface in here. Generally you need
to send 2 dummy bytes, but this is four clocks in this mode (Fig 24a).
I want to send a proposal to change this dummy clock interface
after 2.7 will be released, because now it is really confusing and unclear.

Thanks,
Marcin

> Thanks,
> 
> C.
> 
> >          break;
> >      case MAN_SPANSION:
> >          s->needed_bytes +=
> SPANSION_CONTINUOUS_READ_MODE_CMD_LEN;
> >