| Message ID | 1450270635-27080-11-git-send-email-marcin.krzeminski@nokia.com |
|---|---|
| State | New |
| Headers | show |
On Wed, Dec 16, 2015 at 4:57 AM, <marcin.krzeminski@nokia.com> wrote: > From: Marcin Krzeminski <marcin.krzeminski@nokia.com> > > Signed-off-by: Pawel Lenkow <pawel.lenkow@nokia.com> Same comments as for earlier patches. Need to clarify authorship and add subsystem prefix and commit paragraph. > --- > 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; What is this used for? > 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; I can't see where i is used? > 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; > + s->pos = 0; > + s->len = 0; > + s->state = STATE_COLLECTING_DATA; > + break; Blank line needed here. The blank is omitted between between logical groupings of commands (such as read-write pairs or different types of the same op) but there is no grouping between the data RW ops and WRSR. 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 > >
W dniu 21.12.2015 o 12:52, Peter Crosthwaite pisze: > On Wed, Dec 16, 2015 at 4:57 AM, <marcin.krzeminski@nokia.com> wrote: >> From: Marcin Krzeminski <marcin.krzeminski@nokia.com> >> >> Signed-off-by: Pawel Lenkow <pawel.lenkow@nokia.com> > > Same comments as for earlier patches. Need to clarify authorship and > add subsystem prefix and commit paragraph. > >> --- >> 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; > > What is this used for? > >> 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; > > I can't see where i is used? Should be in patch 09/12. I will correct this. Thanks, Marcin > > >> 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; >> + s->pos = 0; >> + s->len = 0; >> + s->state = STATE_COLLECTING_DATA; >> + break; > > Blank line needed here. The blank is omitted between between logical > groupings of commands (such as read-write pairs or different types of > the same op) but there is no grouping between the data RW ops and > WRSR. > > 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 >> >> > >
On Wed, Dec 16, 2015 at 4:57 AM, <marcin.krzeminski@nokia.com> wrote: > From: Marcin Krzeminski <marcin.krzeminski@nokia.com> > > Signed-off-by: Pawel Lenkow <pawel.lenkow@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. 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? 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 > >
On Tue, Dec 22, 2015 at 1:40 PM, Peter Crosthwaite <crosthwaitepeter@gmail.com> wrote: > On Wed, Dec 16, 2015 at 4:57 AM, <marcin.krzeminski@nokia.com> wrote: >> From: Marcin Krzeminski <marcin.krzeminski@nokia.com> >> >> Signed-off-by: Pawel Lenkow <pawel.lenkow@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 > 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? > > 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 >> >>
> -----Original Message----- > From: EXT Peter Crosthwaite [mailto:crosthwaitepeter@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 > <crosthwaitepeter@gmail.com> wrote: > > On Wed, Dec 16, 2015 at 4:57 AM, <marcin.krzeminski@nokia.com> wrote: > >> From: Marcin Krzeminski <marcin.krzeminski@nokia.com> > >> > >> Signed-off-by: Pawel Lenkow <pawel.lenkow@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 > >> > >>
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; + s->pos = 0; + s->len = 0; + s->state = STATE_COLLECTING_DATA; + break; 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()