1. Patchew
  2. QEMU
  3. [Qemu-devel] [PATCH v6 00/13] Add support for the ZynqMP Generic QSPI
  4. View patch

[Qemu-devel] [PATCH v6 01/13] m25p80: Add support for continuous read out of RDSR and READ_FSR

Francisco Iglesias posted 13 patches 8 years, 3 months ago
There is a newer version of this series
[Qemu-devel] [PATCH v6 01/13] m25p80: Add support for continuous read out of RDSR and READ_FSR
Posted by Francisco Iglesias 8 years, 3 months ago 
Add support for continuous read out of the RDSR and READ_FSR status
registers until the chip select is deasserted. This feature is supported
by amongst others 1 or more flashtypes manufactured by Numonyx (Micron),
Windbond, SST, Gigadevice, Eon and Macronix.

Signed-off-by: Francisco Iglesias <frasse.iglesias@gmail.com>
---
 hw/block/m25p80.c | 9 ++++++++-
 1 file changed, 8 insertions(+), 1 deletion(-)

diff --git a/hw/block/m25p80.c b/hw/block/m25p80.c
index a2438b9..721ae1a 100644
--- a/hw/block/m25p80.c
+++ b/hw/block/m25p80.c
@@ -423,6 +423,7 @@ typedef struct Flash {
     uint8_t data[M25P80_INTERNAL_DATA_BUFFER_SZ];
     uint32_t len;
     uint32_t pos;
+    bool data_read_loop;
     uint8_t needed_bytes;
     uint8_t cmd_in_progress;
     uint32_t cur_addr;
@@ -983,6 +984,7 @@ static void decode_new_cmd(Flash *s, uint32_t value)
         }
         s->pos = 0;
         s->len = 1;
+        s->data_read_loop = true;
         s->state = STATE_READING_DATA;
         break;
 
@@ -993,6 +995,7 @@ static void decode_new_cmd(Flash *s, uint32_t value)
         }
         s->pos = 0;
         s->len = 1;
+        s->data_read_loop = true;
         s->state = STATE_READING_DATA;
         break;
 
@@ -1133,6 +1136,7 @@ static int m25p80_cs(SSISlave *ss, bool select)
         s->pos = 0;
         s->state = STATE_IDLE;
         flash_sync_dirty(s, -1);
+        s->data_read_loop = false;
     }
 
     DB_PRINT_L(0, "%sselect\n", select ? "de" : "");
@@ -1198,7 +1202,9 @@ static uint32_t m25p80_transfer8(SSISlave *ss, uint32_t tx)
         s->pos++;
         if (s->pos == s->len) {
             s->pos = 0;
-            s->state = STATE_IDLE;
+            if (!s->data_read_loop) {
+                s->state = STATE_IDLE;
+            }
         }
         break;
 
@@ -1279,6 +1285,7 @@ static const VMStateDescription vmstate_m25p80 = {
         VMSTATE_UINT8_ARRAY(data, Flash, M25P80_INTERNAL_DATA_BUFFER_SZ),
         VMSTATE_UINT32(len, Flash),
         VMSTATE_UINT32(pos, Flash),
+        VMSTATE_BOOL(data_read_loop, Flash),
         VMSTATE_UINT8(needed_bytes, Flash),
         VMSTATE_UINT8(cmd_in_progress, Flash),
         VMSTATE_UINT32(cur_addr, Flash),
-- 
2.9.3
Re: [Qemu-devel] [PATCH v6 01/13] m25p80: Add support for continuous read out of RDSR and READ_FSR
Posted by mar.krzeminski 8 years, 3 months ago 
Hi Francisco,

W dniu 01.11.2017 o 08:16, Francisco Iglesias pisze:
> Add support for continuous read out of the RDSR and READ_FSR status
> registers until the chip select is deasserted. This feature is supported
> by amongst others 1 or more flashtypes manufactured by Numonyx (Micron),
> Windbond, SST, Gigadevice, Eon and Macronix.
>
> Signed-off-by: Francisco Iglesias <frasse.iglesias@gmail.com>
> ---
>   hw/block/m25p80.c | 9 ++++++++-
>   1 file changed, 8 insertions(+), 1 deletion(-)
>
> diff --git a/hw/block/m25p80.c b/hw/block/m25p80.c
> index a2438b9..721ae1a 100644
> --- a/hw/block/m25p80.c
> +++ b/hw/block/m25p80.c
> @@ -423,6 +423,7 @@ typedef struct Flash {
>       uint8_t data[M25P80_INTERNAL_DATA_BUFFER_SZ];
>       uint32_t len;
>       uint32_t pos;
> +    bool data_read_loop;
>       uint8_t needed_bytes;
>       uint8_t cmd_in_progress;
>       uint32_t cur_addr;
> @@ -983,6 +984,7 @@ static void decode_new_cmd(Flash *s, uint32_t value)
>           }
>           s->pos = 0;
>           s->len = 1;
> +        s->data_read_loop = true;
>           s->state = STATE_READING_DATA;
>           break;
>   
> @@ -993,6 +995,7 @@ static void decode_new_cmd(Flash *s, uint32_t value)
>           }
>           s->pos = 0;
>           s->len = 1;
> +        s->data_read_loop = true;
>           s->state = STATE_READING_DATA;
>           break;
>   
> @@ -1133,6 +1136,7 @@ static int m25p80_cs(SSISlave *ss, bool select)
>           s->pos = 0;
>           s->state = STATE_IDLE;
>           flash_sync_dirty(s, -1);
> +        s->data_read_loop = false;
>       }
>   
>       DB_PRINT_L(0, "%sselect\n", select ? "de" : "");
> @@ -1198,7 +1202,9 @@ static uint32_t m25p80_transfer8(SSISlave *ss, uint32_t tx)
>           s->pos++;
>           if (s->pos == s->len) {
>               s->pos = 0;
> -            s->state = STATE_IDLE;
> +            if (!s->data_read_loop) {
> +                s->state = STATE_IDLE;
> +            }
>           }
>           break;
>   
> @@ -1279,6 +1285,7 @@ static const VMStateDescription vmstate_m25p80 = {
>           VMSTATE_UINT8_ARRAY(data, Flash, M25P80_INTERNAL_DATA_BUFFER_SZ),
>           VMSTATE_UINT32(len, Flash),
>           VMSTATE_UINT32(pos, Flash),
> +        VMSTATE_BOOL(data_read_loop, Flash),
This is not so simple I am afraid. Sorry I have not mentioned that in 
previous review.
Beside adding the field, you need to take care of VMSTATE (migration).
Documentation is here:
https://github.com/qemu/qemu/blob/master/docs/devel/migration.txt

My suggestion would be to bump version and just set false to 
data_read_loopwhen importing
older states - in fact it should be false by default, so do nothing?
Since I am away from Qemu for a while, and there are some new features 
(subsections),
could be that different solution is preferred.
Maybe Peter can help.

Regards,
Marcin
>           VMSTATE_UINT8(needed_bytes, Flash),
>           VMSTATE_UINT8(cmd_in_progress, Flash),
>           VMSTATE_UINT32(cur_addr, Flash),
Re: [Qemu-devel] [PATCH v6 01/13] m25p80: Add support for continuous read out of RDSR and READ_FSR
Posted by francisco iglesias 8 years, 3 months ago 
On 1 November 2017 at 20:20, mar.krzeminski <mar.krzeminski@gmail.com>
wrote:

> Hi Francisco,
>
> W dniu 01.11.2017 o 08:16, Francisco Iglesias pisze:
>
> Add support for continuous read out of the RDSR and READ_FSR status
>> registers until the chip select is deasserted. This feature is supported
>> by amongst others 1 or more flashtypes manufactured by Numonyx (Micron),
>> Windbond, SST, Gigadevice, Eon and Macronix.
>>
>> Signed-off-by: Francisco Iglesias <frasse.iglesias@gmail.com>
>> ---
>>   hw/block/m25p80.c | 9 ++++++++-
>>   1 file changed, 8 insertions(+), 1 deletion(-)
>>
>> diff --git a/hw/block/m25p80.c b/hw/block/m25p80.c
>> index a2438b9..721ae1a 100644
>> --- a/hw/block/m25p80.c
>> +++ b/hw/block/m25p80.c
>> @@ -423,6 +423,7 @@ typedef struct Flash {
>>       uint8_t data[M25P80_INTERNAL_DATA_BUFFER_SZ];
>>       uint32_t len;
>>       uint32_t pos;
>> +    bool data_read_loop;
>>       uint8_t needed_bytes;
>>       uint8_t cmd_in_progress;
>>       uint32_t cur_addr;
>> @@ -983,6 +984,7 @@ static void decode_new_cmd(Flash *s, uint32_t value)
>>           }
>>           s->pos = 0;
>>           s->len = 1;
>> +        s->data_read_loop = true;
>>           s->state = STATE_READING_DATA;
>>           break;
>>   @@ -993,6 +995,7 @@ static void decode_new_cmd(Flash *s, uint32_t value)
>>           }
>>           s->pos = 0;
>>           s->len = 1;
>> +        s->data_read_loop = true;
>>           s->state = STATE_READING_DATA;
>>           break;
>>   @@ -1133,6 +1136,7 @@ static int m25p80_cs(SSISlave *ss, bool select)
>>           s->pos = 0;
>>           s->state = STATE_IDLE;
>>           flash_sync_dirty(s, -1);
>> +        s->data_read_loop = false;
>>       }
>>         DB_PRINT_L(0, "%sselect\n", select ? "de" : "");
>> @@ -1198,7 +1202,9 @@ static uint32_t m25p80_transfer8(SSISlave *ss,
>> uint32_t tx)
>>           s->pos++;
>>           if (s->pos == s->len) {
>>               s->pos = 0;
>> -            s->state = STATE_IDLE;
>> +            if (!s->data_read_loop) {
>> +                s->state = STATE_IDLE;
>> +            }
>>           }
>>           break;
>>   @@ -1279,6 +1285,7 @@ static const VMStateDescription vmstate_m25p80 = {
>>           VMSTATE_UINT8_ARRAY(data, Flash, M25P80_INTERNAL_DATA_BUFFER_SZ
>> ),
>>           VMSTATE_UINT32(len, Flash),
>>           VMSTATE_UINT32(pos, Flash),
>> +        VMSTATE_BOOL(data_read_loop, Flash),
>>
> This is not so simple I am afraid. Sorry I have not mentioned that in
> previous review.
> Beside adding the field, you need to take care of VMSTATE (migration).
> Documentation is here:
> https://github.com/qemu/qemu/blob/master/docs/devel/migration.txt
>
> My suggestion would be to bump version and just set false to
> data_read_loopwhen importing
> older states - in fact it should be false by default, so do nothing?
> Since I am away from Qemu for a while, and there are some new features
> (subsections),
> could be that different solution is preferred.
> Maybe Peter can help.
>

Regards,
> Marcin


Hi Marcin,

I'll dive into the documentation (thank you for pinpointing!), code and
comeback with a new patch trying to correct this! (If Peter knows this
should be done in a certain way i'll will of course never turn down help)

Thank you once again Marcin!

Best regards,
Francisco Iglesias




>
>           VMSTATE_UINT8(needed_bytes, Flash),
>>           VMSTATE_UINT8(cmd_in_progress, Flash),
>>           VMSTATE_UINT32(cur_addr, Flash),
>>
>
>

Patchew 2.1-devel-b67e4c2

© 2016 - 2026 Red Hat, Inc.