Hello Francisco, On 2/1/19 8:55 AM, Francisco Iglesias wrote: > Hi Cedric, > > On [2019 Jan 21] Mon 17:00:02, Cédric Le Goater wrote: >> JEDEC STANDARD JESD216 for Serial Flash Discovery Parameters (SFDP) >> provides a mean to describe the features of a serial flash device >> using a set of internal parameter tables. >> >> This is the initial framework for the RDSFPD command which is given > > Is it possible that above should be RDSFDP? (In that case we need to do > s/PD/DP/ + s/pd/dp/ in the code aswell)
ah yes. I completely missed. > >> access to a private SFPD area under the flash. This area now needs to >> be populated with the flash device characteristics, presumingly with >> the values from the FlashPartInfo array. >> >> Signed-off-by: Cédric Le Goater <c...@kaod.org> >> --- >> hw/block/m25p80.c | 67 +++++++++++++++++++++++++++++++++++++++++++++++ >> 1 file changed, 67 insertions(+) >> >> diff --git a/hw/block/m25p80.c b/hw/block/m25p80.c >> index e8dfa14b332f..581a9e1fb82e 100644 >> --- a/hw/block/m25p80.c >> +++ b/hw/block/m25p80.c >> @@ -340,6 +340,7 @@ typedef enum { >> BULK_ERASE = 0xc7, >> READ_FSR = 0x70, >> RDCR = 0x15, >> + RDSFPD = 0x5a, >> >> READ = 0x03, >> READ4 = 0x13, >> @@ -405,6 +406,7 @@ typedef enum { >> STATE_COLLECTING_DATA, >> STATE_COLLECTING_VAR_LEN_DATA, >> STATE_READING_DATA, >> + STATE_READING_SFPD, >> } CMDState; >> >> typedef enum { >> @@ -456,6 +458,8 @@ typedef struct Flash { >> >> int64_t dirty_page; >> >> +#define M25P80_SFPD_AREA_SIZE 0x100 >> + uint8_t sfpd_area[M25P80_SFPD_AREA_SIZE]; >> const FlashPartInfo *pi; >> >> } Flash; >> @@ -626,6 +630,8 @@ static inline int get_addr_length(Flash *s) >> } >> >> switch (s->cmd_in_progress) { >> + case RDSFPD: >> + return 3; >> case PP4: >> case PP4_4: >> case QPP_4: >> @@ -748,11 +754,55 @@ static void complete_collecting_data(Flash *s) >> " by device\n"); >> } >> break; >> + >> + case RDSFPD: >> + if (s->cur_addr < M25P80_SFPD_AREA_SIZE) { >> + s->state = STATE_READING_SFPD; >> + } else { >> + qemu_log_mask(LOG_GUEST_ERROR, >> + "M25P80: Invalid SFPD address %#" PRIx32 "\n", >> + s->cur_addr); >> + } >> + break; >> + >> default: >> break; >> } >> } >> >> +static void reset_memory_sfpd(Flash *s) >> +{ >> + s->sfpd_area[0x00] = 'S'; >> + s->sfpd_area[0x01] = 'F'; >> + s->sfpd_area[0x02] = 'P'; > > We also need to swap here so [0x02] == 'D' and [0x02] == 'P'. yes. > >> + s->sfpd_area[0x03] = 'D'; >> + >> + s->sfpd_area[0x04] = 0x00; /* SFDP Minor Revision Number */ >> + s->sfpd_area[0x05] = 0x01; /* SFDP Major Revision Number */ >> + s->sfpd_area[0x06] = 0x01; /* Number of Parameter Headers */ >> + s->sfpd_area[0x07] = 0xFF; /* Unused */ >> + >> + s->sfpd_area[0x08] = 0x00; /* ID Number. 0x0 : JEDEC */ >> + s->sfpd_area[0x09] = 0x00; /* Parameter Table Minor Revision */ >> + s->sfpd_area[0x0A] = 0x01; /* Parameter Table Major Revision */ >> + s->sfpd_area[0x0B] = 0x09; /* Parameter Table Length (DWORD) */ >> + s->sfpd_area[0x0C] = 0x30; /* Parameter Table Pointer */ >> + s->sfpd_area[0x0D] = 0x00; /* Parameter Table Pointer */ >> + s->sfpd_area[0x0E] = 0x00; /* Parameter Table Pointer */ >> + s->sfpd_area[0x0F] = 0xFF; /* Unused */ >> + >> + s->sfpd_area[0x10] = s->pi->id[0]; /* ID Number. Manufacturer */ >> + s->sfpd_area[0x11] = 0x00; /* Parameter Table Minor Revision */ >> + s->sfpd_area[0x12] = 0x01; /* Parameter Table Major Revision */ >> + s->sfpd_area[0x13] = 0x04; /* Parameter Table Length (DWORD) */ >> + s->sfpd_area[0x14] = 0x60; /* Parameter Table Pointer */ >> + s->sfpd_area[0x15] = 0x00; /* Parameter Table Pointer */ >> + s->sfpd_area[0x16] = 0x00; /* Parameter Table Pointer */ >> + s->sfpd_area[0x17] = 0xFF; /* Unused */ >> + >> + /* TODO: populate accordingly to chip model */ > > Could an option be to move this into the FlashPartInfo perhaps? (Since the > SFDP > area will vary between flashes it is maybe easier to scale if put there...) I thought instead we could build it from the FlashPartInfo but it is true that there will some vendor parts in the SFDP area. I will take a look at it. Thanks, C. > > Best regards, > Francisco Iglesias > > >> +} >> + >> static void reset_memory(Flash *s) >> { >> s->cmd_in_progress = NOP; >> @@ -822,6 +872,8 @@ static void reset_memory(Flash *s) >> break; >> } >> >> + reset_memory_sfpd(s); >> + >> DB_PRINT_L(0, "Reset done.\n"); >> } >> >> @@ -1049,6 +1101,13 @@ static void decode_new_cmd(Flash *s, uint32_t value) >> s->state = STATE_READING_DATA; >> break; >> >> + case RDSFPD: >> + s->needed_bytes = get_addr_length(s) + 1 ; /* SFPD addr + dummy */ >> + s->pos = 0; >> + s->len = 0; >> + s->state = STATE_COLLECTING_DATA; >> + break; >> + >> case RDCR: >> s->data[0] = s->volatile_cfg & 0xFF; >> s->data[0] |= (!!s->four_bytes_address_mode) << 5; >> @@ -1241,6 +1300,8 @@ static uint32_t m25p80_transfer8(SSISlave *ss, >> uint32_t tx) >> } >> >> r = s->data[s->pos]; >> + DB_PRINT_L(1, "READ DATA 0x%" PRIx32 "=%" PRIx8 "\n", s->pos, >> + (uint8_t)r); >> s->pos++; >> if (s->pos == s->len) { >> s->pos = 0; >> @@ -1249,6 +1310,12 @@ static uint32_t m25p80_transfer8(SSISlave *ss, >> uint32_t tx) >> } >> } >> break; >> + case STATE_READING_SFPD: >> + r = s->sfpd_area[s->cur_addr]; >> + DB_PRINT_L(1, "READ SFPD 0x%" PRIx32 "=%" PRIx8 "\n", s->cur_addr, >> + (uint8_t)r); >> + s->cur_addr = (s->cur_addr + 1) & (M25P80_SFPD_AREA_SIZE - 1); >> + break; >> >> default: >> case STATE_IDLE: >> -- >> 2.20.1 >> >>