> -----Original Message-----
> From: EXT Peter Crosthwaite [mailto:crosthwaitepe...@gmail.com]
> Sent: Wednesday, December 23, 2015 3:25 AM
> To: Krzeminski, Marcin (Nokia - PL/Wroclaw); Cédric Le Goater
> Cc: qemu-devel@nongnu.org Developers; Lenkow, Pawel (Nokia -
> PL/Wroclaw)
> Subject: Re: [Qemu-devel] [PATCH 10/12] Support for quad commands.
>
> On Tue, Dec 22, 2015 at 1:40 PM, Peter Crosthwaite
> <crosthwaitepe...@gmail.com> wrote:
> > On Wed, Dec 16, 2015 at 4:57 AM, <marcin.krzemin...@nokia.com> wrote:
> >> From: Marcin Krzeminski <marcin.krzemin...@nokia.com>
> >>
> >> Signed-off-by: Pawel Lenkow <pawel.len...@nokia.com>
> >> ---
> >> hw/block/m25p80.c | 38 ++++++++++++++++++++++++++++++++++++-
> -
> >> 1 file changed, 36 insertions(+), 2 deletions(-)
> >>
> >> diff --git a/hw/block/m25p80.c b/hw/block/m25p80.c
> >> index 6fc55a3..25ec666 100644
> >> --- a/hw/block/m25p80.c
> >> +++ b/hw/block/m25p80.c
> >> @@ -255,19 +255,24 @@ typedef enum {
> >> BULK_ERASE = 0xc7,
> >>
> >> READ = 0x3,
> >> + READ4 = 0x13,
> >> FAST_READ = 0xb,
> >> DOR = 0x3b,
> >> QOR = 0x6b,
> >> DIOR = 0xbb,
> >> QIOR = 0xeb,
> >> + QIOR4 = 0xec,
> >>
> >> PP = 0x2,
> >> + PP4 = 0x12,
> >> DPP = 0xa2,
> >> QPP = 0x32,
> >>
> >> ERASE_4K = 0x20,
> >> + ERASE4_4K = 0x21,
> >> ERASE_32K = 0x52,
> >> ERASE_SECTOR = 0xd8,
> >> + ERASE4_SECTOR = 0xdc,
> >>
> >> EN_4BYTE_ADDR = 0xB7,
> >> EX_4BYTE_ADDR = 0xE9,
> >> @@ -307,6 +312,7 @@ typedef struct Flash {
> >> bool write_enable;
> >> bool four_bytes_address_mode;
> >> bool reset_enable;
> >> + bool quad_enable;
> >> bool initialized;
> >> uint8_t reset_pin;
> >> uint8_t ear;
> >> @@ -381,6 +387,7 @@ static void flash_erase(Flash *s, int offset,
> FlashCMD cmd)
> >>
> >> switch (cmd) {
> >> case ERASE_4K:
> >> + case ERASE4_4K:
> >> len = 4 << 10;
> >> capa_to_assert = ER_4K;
> >> break;
> >> @@ -389,6 +396,7 @@ static void flash_erase(Flash *s, int offset,
> FlashCMD cmd)
> >> capa_to_assert = ER_32K;
> >> break;
> >> case ERASE_SECTOR:
> >> + case ERASE4_SECTOR:
> >> len = s->pi->sector_size;
> >> break;
> >> case BULK_ERASE:
> >> @@ -447,6 +455,14 @@ void flash_write8(Flash *s, uint64_t addr, uint8_t
> data)
> >>
> >> static inline int is_4bytes(Flash *s)
> >> {
> >> + switch (s->cmd_in_progress) {
> >> + case PP4:
> >> + case READ4:
> >> + case QIOR4:
> >> + case ERASE4_4K:
> >> + case ERASE4_SECTOR:
> >> + return 1;
> >> + default:
> >> return s->four_bytes_address_mode;
> >> }
> >> }
> >> @@ -472,19 +488,24 @@ static void complete_collecting_data(Flash *s)
> >> case DPP:
> >> case QPP:
> >> case PP:
> >> + case PP4:
> >> s->state = STATE_PAGE_PROGRAM;
> >> break;
> >> case READ:
> >> + case READ4:
> >> case FAST_READ:
> >> case DOR:
> >> case QOR:
> >> case DIOR:
> >> case QIOR:
> >> + case QIOR4:
> >> s->state = STATE_READ;
> >> break;
> >> case ERASE_4K:
> >> + case ERASE4_4K:
> >> case ERASE_32K:
> >> case ERASE_SECTOR:
> >> + case ERASE4_SECTOR:
> >> flash_erase(s, s->cur_addr, s->cmd_in_progress);
> >> break;
> >> case WRSR:
> >> @@ -512,6 +533,7 @@ static void reset_memory(Flash *s)
> >> s->state = STATE_IDLE;
> >> s->write_enable = false;
> >> s->reset_enable = false;
> >> + s->quad_enable = false;
> >>
> >> DB_PRINT_L(0, "Reset done.\n");
> >> }
> >> @@ -519,18 +541,23 @@ static void reset_memory(Flash *s)
> >> static void decode_new_cmd(Flash *s, uint32_t value)
> >> {
> >> s->cmd_in_progress = value;
> >> + int i;
> >> DB_PRINT_L(0, "decoded new command:%x\n", value);
> >>
> >> switch (value) {
> >>
> >> case ERASE_4K:
> >> + case ERASE4_4K:
> >> case ERASE_32K:
> >> case ERASE_SECTOR:
> >> + case ERASE4_SECTOR:
> >> case READ:
> >> + case READ4:
> >> case DPP:
> >> case QPP:
> >> case PP:
> >> - s->needed_bytes = 3;
> >> + case PP4:
> >> + s->needed_bytes = is_4bytes(s) ? 4 : 3;
> >> s->pos = 0;
> >> s->len = 0;
> >> s->state = STATE_COLLECTING_DATA;
> >> @@ -574,7 +601,13 @@ static void decode_new_cmd(Flash *s, uint32_t
> value)
> >> s->len = 0;
> >> s->state = STATE_COLLECTING_DATA;
> >> break;
> >> -
> >> + case QIOR4:
> >> + /* 4 address + 1 dummy */
> >> + s->needed_bytes = 5;
> >
> > So looking at the current QIOR handling, the number of "dummy bytes"
> > is calculated as:
> >
> > num_dummy_cycles X 4.
> >
> > This makes sense, as both the address and data-phase on either side of
> > the dummy phase is done on four-wires, so it is cleaner that the dummy
> > phase is also on 4 wires. Ideally we implement Dual and Quad mode
> > awareness and cycle accuracy on the SSI layer itself to handle these
> > cases, but until then we stuck with this strange fixed policy.
> >
> > For numonyx, this gives 1 command + 3 address + 4 dummies ( == 8
> > cycles) = 8 needed_bytes.
> >
>
> Correction, there is no command byte in the needed bytes, rather there
> a 5 dummies (for 10 cycles).
>
> Regards,
> Peter
>
Yes you are right, I will fixed it.
Micron has by default 10 cycles for QIOR so it will be 5 bytes extra.
I need to check to Spansion datasheets, because there is no clear for me how
many dummy cycles are required.
> > So this would be 9 for the extra address byte in 4b mode.
> >
> >> + s->pos = 0;
> >> + s->len = 0;
> >> + s->state = STATE_COLLECTING_DATA;
> >> + break;
> >
> > Following on from, Cedric's spot, should this be a fallthrough to
> > regular QIOR, which then uses is_4bytes to implement the +1 on needed
> > bytes?
Yes, good idea.
Br,
Pawel Lenkow
> >
> > Regards,
> > Peter
> >
> >> case WRSR:
> >> if (s->write_enable) {
> >> s->needed_bytes = 1;
> >> @@ -792,6 +825,7 @@ static const VMStateDescription vmstate_m25p80
> = {
> >> VMSTATE_BOOL(four_bytes_address_mode, Flash),
> >> VMSTATE_UINT8(ear, Flash),
> >> VMSTATE_BOOL(reset_enable, Flash),
> >> + VMSTATE_BOOL(quad_enable, Flash),
> >> VMSTATE_BOOL(initialized, Flash),
> >> VMSTATE_UINT8(reset_pin, Flash),
> >> VMSTATE_END_OF_LIST()
> >> --
> >> 2.5.0
> >>
> >>
