* [Qemu-devel] [PATCH] m25p80: add basic support for the SFPD command
@ 2019-01-21 16:00 Cédric Le Goater
2019-02-01 7:55 ` Francisco Iglesias
2019-02-01 20:22 ` Philippe Mathieu-Daudé
0 siblings, 2 replies; 5+ messages in thread
From: Cédric Le Goater @ 2019-01-21 16:00 UTC (permalink / raw)
To: qemu-devel
Cc: qemu-block, Alistair Francis, Peter Crosthwaite, Max Reitz,
Kevin Wolf, Cédric Le Goater
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
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 <clg@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';
+ 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 */
+}
+
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
^ permalink raw reply related [flat|nested] 5+ messages in thread
* Re: [Qemu-devel] [PATCH] m25p80: add basic support for the SFPD command
2019-01-21 16:00 [Qemu-devel] [PATCH] m25p80: add basic support for the SFPD command Cédric Le Goater
@ 2019-02-01 7:55 ` Francisco Iglesias
2019-02-01 17:06 ` Cédric Le Goater
2019-02-01 20:22 ` Philippe Mathieu-Daudé
1 sibling, 1 reply; 5+ messages in thread
From: Francisco Iglesias @ 2019-02-01 7:55 UTC (permalink / raw)
To: Cédric Le Goater
Cc: qemu-devel, Kevin Wolf, qemu-block, Peter Crosthwaite,
Alistair Francis, Max Reitz
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)
> 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 <clg@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'.
> + 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...)
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
>
>
^ permalink raw reply [flat|nested] 5+ messages in thread
* Re: [Qemu-devel] [PATCH] m25p80: add basic support for the SFPD command
2019-02-01 7:55 ` Francisco Iglesias
@ 2019-02-01 17:06 ` Cédric Le Goater
0 siblings, 0 replies; 5+ messages in thread
From: Cédric Le Goater @ 2019-02-01 17:06 UTC (permalink / raw)
To: Francisco Iglesias
Cc: qemu-devel, Kevin Wolf, qemu-block, Peter Crosthwaite,
Alistair Francis, Max Reitz
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 <clg@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
>>
>>
^ permalink raw reply [flat|nested] 5+ messages in thread
* Re: [Qemu-devel] [PATCH] m25p80: add basic support for the SFPD command
2019-01-21 16:00 [Qemu-devel] [PATCH] m25p80: add basic support for the SFPD command Cédric Le Goater
2019-02-01 7:55 ` Francisco Iglesias
@ 2019-02-01 20:22 ` Philippe Mathieu-Daudé
2019-02-04 9:54 ` Cédric Le Goater
1 sibling, 1 reply; 5+ messages in thread
From: Philippe Mathieu-Daudé @ 2019-02-01 20:22 UTC (permalink / raw)
To: Cédric Le Goater, qemu-devel
Cc: Kevin Wolf, qemu-block, Peter Crosthwaite, Alistair Francis,
Max Reitz
Hi Cédric,
On 1/21/19 5:00 PM, 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
> 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 <clg@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';
> + 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 */
Shouldn't this be filled with 0xFF meanwhile?
> +}
> +
> 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);
I understand this info is const, so we can move this to the realize()
function to call it only once. Maybe you can rename it prepare_sfdp() or
build_sfdp()?
> +
> 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:
>
^ permalink raw reply [flat|nested] 5+ messages in thread
* Re: [Qemu-devel] [PATCH] m25p80: add basic support for the SFPD command
2019-02-01 20:22 ` Philippe Mathieu-Daudé
@ 2019-02-04 9:54 ` Cédric Le Goater
0 siblings, 0 replies; 5+ messages in thread
From: Cédric Le Goater @ 2019-02-04 9:54 UTC (permalink / raw)
To: Philippe Mathieu-Daudé, qemu-devel
Cc: Kevin Wolf, qemu-block, Peter Crosthwaite, Alistair Francis,
Max Reitz
On 2/1/19 9:22 PM, Philippe Mathieu-Daudé wrote:
> Hi Cédric,
>
> On 1/21/19 5:00 PM, 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
>> 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 <clg@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';
>> + 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 */
>
> Shouldn't this be filled with 0xFF meanwhile?
I would say 0x0 looking at the specs. 0x0 == not supported.
Idealy, for chips that support SFDP, we should compute some of the values
of the first table (JEDEC) using the FlashPartInfo.
>> +}
>> +
>> 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);
>
> I understand this info is const, so we can move this to the realize()
> function to call it only once. Maybe you can rename it prepare_sfdp() or
> build_sfdp()?
OK.
Thanks,
C.
>
>> +
>> 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:
>>
^ permalink raw reply [flat|nested] 5+ messages in thread
end of thread, other threads:[~2019-02-04 9:54 UTC | newest]
Thread overview: 5+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2019-01-21 16:00 [Qemu-devel] [PATCH] m25p80: add basic support for the SFPD command Cédric Le Goater
2019-02-01 7:55 ` Francisco Iglesias
2019-02-01 17:06 ` Cédric Le Goater
2019-02-01 20:22 ` Philippe Mathieu-Daudé
2019-02-04 9:54 ` Cédric Le Goater
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).