Re: [Qemu-devel] [PATCH 10/12] Support for quad commands.

[Peter Crosthwaite]

On Tue, Dec 22, 2015 at 1:40 PM, Peter Crosthwaite
<address@hidden> wrote:
> On Wed, Dec 16, 2015 at 4:57 AM,  <address@hidden> wrote:
>> From: Marcin Krzeminski <address@hidden>
>>
>> Signed-off-by: Pawel Lenkow <address@hidden>
>> ---
>>  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
>>
>>