qemu-devel
[Top][All Lists]
Advanced

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

Re: [Qemu-devel] [PATCH 06/12] 4byte address mode support added.


From: Peter Crosthwaite
Subject: Re: [Qemu-devel] [PATCH 06/12] 4byte address mode support added.
Date: Tue, 22 Dec 2015 13:28:47 -0800

On Tue, Dec 22, 2015 at 10:41 AM, Cédric Le Goater <address@hidden> wrote:
> Hello Marcin,
>
>
> On 12/16/2015 01:57 PM, address@hidden wrote:
>> From: Marcin Krzeminski <address@hidden>
>>
>> Signed-off-by: Marcin Krzeminski <address@hidden>
>> ---
>>  hw/block/m25p80.c | 31 ++++++++++++++++++++++++++++---
>>  1 file changed, 28 insertions(+), 3 deletions(-)
>>
>> diff --git a/hw/block/m25p80.c b/hw/block/m25p80.c
>> index 1a547ae..6d5d90d 100644
>> --- a/hw/block/m25p80.c
>> +++ b/hw/block/m25p80.c
>> @@ -237,6 +237,9 @@ typedef enum {
>>      ERASE_32K = 0x52,
>>      ERASE_SECTOR = 0xd8,
>>
>> +    EN_4BYTE_ADDR = 0xB7,
>> +    EX_4BYTE_ADDR = 0xE9,
>> +
>>      RESET_ENABLE = 0x66,
>>      RESET_MEMORY = 0x99,
>>
>> @@ -267,6 +270,7 @@ typedef struct Flash {
>>      uint8_t cmd_in_progress;
>>      uint64_t cur_addr;
>>      bool write_enable;
>> +    bool four_bytes_address_mode;
>>      bool reset_enable;
>>      bool initialized;
>>      uint8_t reset_pin;
>> @@ -405,11 +409,24 @@ void flash_write8(Flash *s, uint64_t addr, uint8_t 
>> data)
>>      s->dirty_page = page;
>>  }
>>
>> +static inline int is_4bytes(Flash *s)
>> +{
>> +       return s->four_bytes_address_mode;
>> +   }
>> +}
>> +
>>  static void complete_collecting_data(Flash *s)
>>  {
>> -    s->cur_addr = s->data[0] << 16;
>> -    s->cur_addr |= s->data[1] << 8;
>> -    s->cur_addr |= s->data[2];
>> +    if (is_4bytes(s)) {
>> +        s->cur_addr = s->data[0] << 24;
>> +        s->cur_addr |= s->data[1] << 16;
>> +        s->cur_addr |= s->data[2] << 8;
>> +        s->cur_addr |= s->data[3];
>> +    } else {
>> +        s->cur_addr = s->data[0] << 16;
>> +        s->cur_addr |= s->data[1] << 8;
>> +        s->cur_addr |= s->data[2];
>> +    }
>>
>>      s->state = STATE_IDLE;
>
>
> Don't we need to also change 'needed_bytes' in the decode_new_cmd() routine
> to increase the number of bytes expected by some commands ?
>

I think you are right, and it may be solved later in the series, from patch 10:

     case QPP:
     case PP:
-        s->needed_bytes = 3;
+       s->needed_bytes = is_4bytes(s) ? 4 : 3;
         s->pos = 0;
         s->len = 0;
         s->state = STATE_COLLECTING_DATA;

This hunk should be brought forward to this patch.

> If so, we could add a width attribute to 'struct Flash' and to something like 
> :
>
>         @@ -260,6 +263,7 @@ typedef struct Flash {
>              uint8_t cmd_in_progress;
>              uint64_t cur_addr;
>              bool write_enable;
>         +    uint8_t width;
>
>              int64_t dirty_page;
>
>         @@ -401,6 +405,10 @@ static void complete_collecting_data(Fla
>              s->cur_addr |= s->data[1] << 8;
>              s->cur_addr |= s->data[2];
>
>         +    if (s->width == 4) {
>         +        s->cur_addr = s->cur_addr << 8 | s->data[4];
>         +    }
>         +
>              s->state = STATE_IDLE;
>
>              switch (s->cmd_in_progress) {
>         @@ -446,7 +454,7 @@ static void decode_new_cmd(Flash *s, uin
>              case DPP:
>              case QPP:
>              case PP:
>         -        s->needed_bytes = 3;
>         +        s->needed_bytes = s->width;
>                  s->pos = 0;
>                  s->len = 0;
>                  s->state = STATE_COLLECTING_DATA;
>         @@ -644,6 +658,7 @@ static int m25p80_init(SSISlave *ss)
>                  memset(s->storage, 0xFF, s->size);
>              }
>
>         +    s->width = 3;
>              return 0;
>          }
>
>
>
> QOR, DIOR, QIOR command also need a check. I suppose an address and some
> number of dummy bytes are expected before the fast read command is done.
> I would need to check the datasheets.
>

I just checked an n25q256 datasheet, and yes you are right. The same
logic as in the hunk above needs to apply to these commands. CC
Xilinx, this bug is in their tree as well I think.

https://github.com/Xilinx/qemu/blob/pub/2015.2.plnx/hw/block/m25p80.c

Where PP, READ and friends have the 4 byte correction logic based on
addr_4b but QIOR does not.

Nice catch :)

Regards,
Peter

> Cheers,
>
> C.
>
>> @@ -446,6 +463,7 @@ static void reset_memory(Flash *s)
>>  {
>>      s->cmd_in_progress = NOP;
>>      s->cur_addr = 0;
>> +    s->four_bytes_address_mode = false;
>>      s->len = 0;
>>      s->needed_bytes = 0;
>>      s->pos = 0;
>> @@ -565,6 +583,12 @@ static void decode_new_cmd(Flash *s, uint32_t value)
>>          break;
>>      case NOP:
>>          break;
>> +    case EN_4BYTE_ADDR:
>> +        s->four_bytes_address_mode = true;
>> +        break;
>> +    case EX_4BYTE_ADDR:
>> +        s->four_bytes_address_mode = false;
>> +        break;
>>      case RESET_ENABLE:
>>          s->reset_enable = true;
>>          break;
>> @@ -715,6 +739,7 @@ static const VMStateDescription vmstate_m25p80 = {
>>          VMSTATE_UINT8(cmd_in_progress, Flash),
>>          VMSTATE_UINT64(cur_addr, Flash),
>>          VMSTATE_BOOL(write_enable, Flash),
>> +        VMSTATE_BOOL(four_bytes_address_mode, Flash),
>>          VMSTATE_BOOL(reset_enable, Flash),
>>          VMSTATE_BOOL(initialized, Flash),
>>          VMSTATE_UINT8(reset_pin, Flash),
>>
>
>
>



reply via email to

[Prev in Thread] Current Thread [Next in Thread]