diff mbox

[06/12] 4byte address mode support added.

Message ID 1450270635-27080-7-git-send-email-marcin.krzeminski@nokia.com
State New
Headers show

Commit Message

Krzeminski, Marcin (Nokia - PL/Wroclaw) Dec. 16, 2015, 12:57 p.m. UTC 
From: Marcin Krzeminski <marcin.krzeminski@nokia.com>

Signed-off-by: Marcin Krzeminski <marcin.krzeminski@nokia.com>
---
 hw/block/m25p80.c | 31 ++++++++++++++++++++++++++++---
 1 file changed, 28 insertions(+), 3 deletions(-)

Comments

Peter Crosthwaite Dec. 21, 2015, 11:35 a.m. UTC | #1 
Commit subject subsystem prefix (block: m25p80:) needed.

On Wed, Dec 16, 2015 at 4:57 AM,  <marcin.krzeminski@nokia.com> wrote:
> From: Marcin Krzeminski <marcin.krzeminski@nokia.com>
>
> Signed-off-by: Marcin Krzeminski <marcin.krzeminski@nokia.com>
> ---
>  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];
> +    }

Avoid the duped code with a loop:

s->cur_addr = 0;
for (i = 0; i < (is_4bytes(s) ? 4 : 3); i++) {
    s->cur_addr <<= 8;
    s->cur_addr |= s->data[i];
}

Otherwise:

Reviewed-by: Peter Crosthwaite <crosthwaite.peter@gmail.com>

Regards,
Peter

>
>      s->state = STATE_IDLE;
>
> @@ -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),
> --
> 2.5.0
>
>
Cédric Le Goater Dec. 22, 2015, 6:41 p.m. UTC | #2 
Hello Marcin,


On 12/16/2015 01:57 PM, marcin.krzeminski@nokia.com wrote:
> From: Marcin Krzeminski <marcin.krzeminski@nokia.com>
> 
> Signed-off-by: Marcin Krzeminski <marcin.krzeminski@nokia.com>
> ---
>  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 ? 

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.

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),
>
Peter Crosthwaite Dec. 22, 2015, 9:28 p.m. UTC | #3 
On Tue, Dec 22, 2015 at 10:41 AM, Cédric Le Goater <clg@fr.ibm.com> wrote:
> Hello Marcin,
>
>
> On 12/16/2015 01:57 PM, marcin.krzeminski@nokia.com wrote:
>> From: Marcin Krzeminski <marcin.krzeminski@nokia.com>
>>
>> Signed-off-by: Marcin Krzeminski <marcin.krzeminski@nokia.com>
>> ---
>>  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),
>>
>
>
>
Krzeminski, Marcin (Nokia - PL/Wroclaw) Feb. 4, 2016, 11:58 a.m. UTC | #4 
> -----Original Message-----

> From: EXT Peter Crosthwaite [mailto:crosthwaitepeter@gmail.com]

> Sent: Tuesday, December 22, 2015 10:29 PM

> To: Cédric Le Goater; git@xilinx.com

> Cc: Krzeminski, Marcin (Nokia - PL/Wroclaw); qemu-devel@nongnu.org

> Developers; Lenkow, Pawel (Nokia - PL/Wroclaw)

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

> added.

> 

> On Tue, Dec 22, 2015 at 10:41 AM, Cédric Le Goater <clg@fr.ibm.com> wrote:

> > Hello Marcin,

> >

> >

> > On 12/16/2015 01:57 PM, marcin.krzeminski@nokia.com wrote:

> >> From: Marcin Krzeminski <marcin.krzeminski@nokia.com>

> >>

> >> Signed-off-by: Marcin Krzeminski <marcin.krzeminski@nokia.com>

> >> ---

> >>  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

> 


Hello Cedric,

Sorry for late response.
As peter has responded, needed bytes for 4bytes address mode/cmd length is handled partially (not for all commands).
Dummy cycles are not handled since my QSPI controller model had a bug so I missed this feature.
Thanks for finding - it will be covered in v2.

Regards,
Marcin
> > 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),

> >>

> >

> >

> >
diff mbox

Patch

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;
 
@@ -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),