[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]
Re: [Qemu-devel] [PATCH 06/12] 4byte address mode support added.
From: |
Krzeminski, Marcin (Nokia - PL/Wroclaw) |
Subject: |
Re: [Qemu-devel] [PATCH 06/12] 4byte address mode support added. |
Date: |
Thu, 4 Feb 2016 11:58:30 +0000 |
> -----Original Message-----
> From: EXT Peter Crosthwaite [mailto:address@hidden
> Sent: Tuesday, December 22, 2015 10:29 PM
> To: Cédric Le Goater; address@hidden
> Cc: Krzeminski, Marcin (Nokia - PL/Wroclaw); address@hidden
> 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 <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
>
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),
> >>
> >
> >
> >
[Prev in Thread] |
Current Thread |
[Next in Thread] |
- Re: [Qemu-devel] [PATCH 06/12] 4byte address mode support added.,
Krzeminski, Marcin (Nokia - PL/Wroclaw) <=