| Message ID | 20171101071652.19375-2-frasse.iglesias@gmail.com |
|---|---|
| State | New |
| Headers | show |
| Series | [v6,01/13] m25p80: Add support for continuous read out of RDSR and READ_FSR | expand |
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),
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), >> > >
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),
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(-)