Am 24.06.2011 16:53 schrieb Stefan Tauner:
> Similar to ICH Hardware Sequencing this uses a generic struct flashchip
> element in flashrom.c with dummy values and a special probe function that
> fills the
> obtained values into that generic struct.
> Documentation used:
> http://www.jedec.org/standards-documents/docs/jesd216 (2011-04)
> W25Q32BV data sheet Revision F (2011-04-01)
> EN25QH16 data sheet Revision F (2011-06-01)
>
> Signed-off-by: Stefan Tauner <[email protected]>
>
Nice feature.
> ---
> chipdrivers.h | 1 +
> flashchips.c | 24 +++++
> flashchips.h | 1 +
> flashrom.c | 28 ++++++-
> spi.h | 5 +
> spi25.c | 273
> +++++++++++++++++++++++++++++++++++++++++++++++++++++++++
> 6 files changed, 331 insertions(+), 1 deletions(-)
>
> diff --git a/chipdrivers.h b/chipdrivers.h
> index 92ddbea..0a3df50 100644
> --- a/chipdrivers.h
> +++ b/chipdrivers.h
> @@ -26,6 +26,7 @@
> #define __CHIPDRIVERS_H__ 1
>
> /* spi.c, should probably be in spi_chip.c */
> +int probe_spi_sfdp(struct flashchip *flash);
> int probe_spi_rdid(struct flashchip *flash);
> int probe_spi_rdid4(struct flashchip *flash);
> int probe_spi_rems(struct flashchip *flash);
> diff --git a/flashchips.c b/flashchips.c
> index 865ba2f..6fbbd2c 100644
> --- a/flashchips.c
> +++ b/flashchips.c
> @@ -8507,6 +8507,30 @@ const struct flashchip flashchips[] = {
> .write = write_jedec_1,
> .read = read_memmapped,
> },
> +
> + {
> + .vendor = "Unknown",
> + .name = "SFDP device",
> + .bustype = CHIP_BUSTYPE_SPI,
> + .manufacture_id = GENERIC_MANUF_ID,
> + .model_id = SFDP_DEVICE_ID,
> + /* We present our own "report this" text hence we do not
> + * want the default "This flash part has status UNTESTED..."
> + * text to be printed. */
> + .tested = TEST_OK_PREW,
> + .probe = probe_spi_sfdp,
> + .page_size = 256,
> + .write = spi_chip_write_256,
>
Does SFDP really specify 256-byte write capability?
> + .read = spi_chip_read,
> + /* FIXME: some vendor extensions define this */
> + .voltage = {},
> + /* Everything below will be set by the probing function. */
> + .total_size = 0,
> + .feature_bits = 0,
> + .block_erasers = {},
> + .unlock = NULL,
> + .printlock = NULL,
> + },
>
> {
> .vendor = "AMIC",
> diff --git a/flashchips.h b/flashchips.h
> index 3b2b94f..82333c8 100644
> --- a/flashchips.h
> +++ b/flashchips.h
> @@ -36,6 +36,7 @@
>
> #define GENERIC_MANUF_ID 0xffff /* Check if there is a vendor ID */
> #define GENERIC_DEVICE_ID 0xffff /* Only match the vendor ID */
> +#define SFDP_DEVICE_ID 0xfffe
>
Just a comment, not something that needs to be changed: We probably need
CFI_DEVICE_ID as well once we support CFI (which is SFDP for non-SPI flash).
>
> #define ALLIANCE_ID 0x52 /* Alliance Semiconductor */
> #define ALLIANCE_AS29F002B 0x34
> diff --git a/flashrom.c b/flashrom.c
> index aed10aa..56af373 100644
> --- a/flashrom.c
> +++ b/flashrom.c
> @@ -1206,7 +1206,33 @@ int probe_flash(int startchip, struct flashchip
> *fill_flash, int force)
> * probe_flash() is the first one and thus no chip has been
> * found before.
> */
> - if (startchip == 0 || fill_flash->model_id != GENERIC_DEVICE_ID)
> + if (startchip == 0 && fill_flash->model_id == SFDP_DEVICE_ID) {
> + msg_cinfo("===\n"
> + "SFDP has autodetected a flash chip which is "
> + "not natively supported by flashrom yet.\n");
> + if (!check_block_erasers(flash, 0))
> + msg_cinfo("The standard operations read and "
> + "verify should work, but to support "
> + "erase, write and all other "
> + "possible features");
> + else
> + msg_cinfo("All standard operations (read, "
> + "verify, erase and write) should "
> + "work, but to support all possible "
> + "features");
> +
> + msg_cinfo(" we need to add them manually.\nYou "
> + "can help us by mailing us the output of "
> + "the following command to flashrom@flashrom."
> + "org: \n'flashrom -VV [plus the "
> + "-p/--programmer parameter (if needed)]"
> + "'\nThanks for your help!\n"
> + "===\n");
> + }
>
Should we refactor those SFDP messages into a separate function and do
that for the untested chip messages as well? It would probably improve
code readability here.
> +
> + if (startchip == 0 ||
> + ((fill_flash->model_id != GENERIC_DEVICE_ID) &&
> + (fill_flash->model_id != SFDP_DEVICE_ID)))
> break;
>
> notfound:
> diff --git a/spi.h b/spi.h
> index b908603..5f07eae 100644
> --- a/spi.h
> +++ b/spi.h
> @@ -40,6 +40,11 @@
> #define JEDEC_REMS_OUTSIZE 0x04
> #define JEDEC_REMS_INSIZE 0x02
>
> +/* Read Serial Flash Discoverable Parameters (SFDP) */
> +#define JEDEC_SFDP 0x5a
> +#define JEDEC_SFDP_OUTSIZE 0x05 /* 8b op, 24b addr, 8b dummy */
>
Ouch. A few SPI masters will have pretty explosions with that outsize.
This reminds me... I should resend my patch which can deal with dummy
bytes between write and read in a SPI command.
> +/* JEDEC_SFDP_INSIZE : any length */
> +
> /* Read Electronic Signature */
> #define JEDEC_RES 0xab
> #define JEDEC_RES_OUTSIZE 0x04
> diff --git a/spi25.c b/spi25.c
> index d3680fb..af81f19 100644
> --- a/spi25.c
> +++ b/spi25.c
> @@ -23,6 +23,7 @@
> */
>
> #include <string.h>
> +#include <stdlib.h>
> #include "flash.h"
> #include "flashchips.h"
> #include "chipdrivers.h"
> @@ -113,6 +114,278 @@ int spi_write_disable(void)
> return spi_send_command(sizeof(cmd), 0, cmd, NULL);
> }
>
>
The code below is huge, and I'd argue it is for a chip family and should
live in sfdp25.c (or something with a similar name).
> +static int spi_sfdp(uint32_t address, uint8_t *buf, int len)
> +{
> + const unsigned char cmd[JEDEC_SFDP_OUTSIZE] = {
> + JEDEC_SFDP,
> + (address >> 16) & 0xff,
> + (address >> 8) & 0xff,
> + (address >> 0) & 0xff,
> + 0
> + };
> + return spi_send_command(sizeof(cmd), len, cmd, buf);
> +}
> +
> +struct sfdp_tbl_hdr {
> + uint8_t id;
> + uint8_t v_minor;
> + uint8_t v_major;
> + uint8_t len;
> + uint32_t ptp; /* 24b pointer */
> +};
> +
> +struct sfdp_tbl_0 {
> + uint8_t id;
> + uint8_t v_minor;
> + uint8_t v_major;
> + uint8_t len;
> + uint32_t ptp; /* 24b pointer */
> +};
> +
> +static int sfdp_fill_flash(struct flashchip *f, uint8_t *buf, uint16_t len)
> +{
> + uint32_t tmp32;
> + uint8_t tmp8;
> + uint32_t total_size; /* in bytes */
> + uint32_t bsize;
> + int i, j;
> +
> + msg_cspew("Parsing JEDEC SFDP parameter table...\n");
> + if (len == 9 * 4) {
> + msg_cerr("%s: len out of spec\n", __func__);
> + return 1;
> + }
> +
> + /* 1. double word */
> + i = 0;
> + tmp32 = buf[(4 * i) + 0];
> + tmp32 |= ((unsigned int)buf[(4 * i) + 1]) << 8;
> + tmp32 |= ((unsigned int)buf[(4 * i) + 2]) << 16;
> + tmp32 |= ((unsigned int)buf[(4 * i) + 3]) << 24;
> +
> + tmp8 = (tmp32 >> 17) & 0x3;
> + switch (tmp8) {
> + case 0x0:
> + msg_cspew(" 3-Byte only addressing.\n");
> + break;
> + case 0x1:
> + msg_cspew(" 3-Byte (and optionally 4-Byte) addressing.\n");
> + break;
> + case 0x2:
> + msg_cdbg(" 4-Byte only addressing not supported.\n");
> + return 1;
> + default:
> + msg_cdbg(" Required addressing mode (0x%x) not supported.\n",
> + tmp8);
> + return 1;
> + }
> +
> + msg_cspew(" Writes to the status register have ");
> + if (tmp32 & (1 << 3)) {
> + f->unlock = spi_disable_blockprotect;
> + msg_cspew("to be enabled with ");
> + if (tmp32 & (1 << 4)) {
> + f->feature_bits = FEATURE_WRSR_WREN;
> + msg_cspew("WREN (0x06).\n");
> + } else {
> + f->feature_bits = FEATURE_WRSR_EWSR;
> + msg_cspew("EWSR (0x50).\n");
> + }
> + } else
> + msg_cspew("not to be especially enabled.\n");
> +
> + /* 2. double word */
> + i = 1;
> + tmp32 = buf[(4 * i) + 0];
> + tmp32 |= ((unsigned int)buf[(4 * i) + 1]) << 8;
> + tmp32 |= ((unsigned int)buf[(4 * i) + 2]) << 16;
> + tmp32 |= ((unsigned int)buf[(4 * i) + 3]) << 24;
> +
> + if (tmp32 & (1<<31)) {
> + msg_cdbg(" Flash chip size >= 4 Gb/ 500 MB not supported.\n");
> + return 1;
> + }
> + total_size = (tmp32 & 0x7FFFFFFF) / 8;
> + f->total_size = total_size / 1024;
> + msg_cspew(" Flash chip size is %d kB.\n", f->total_size);
> +
> + i = 8;
> + for(j = 0; j < 4; j++) {
> + /* 8 double words from the start + 2 words for every eraser */
> + bsize = 2^(buf[(4 * i) + (2 * j)]);
> + if (bsize == 0)
> + continue;
> +
> + tmp32 = buf[(4 * i) + (2 * j) + 1];
> + switch(tmp32){
> + case 0x00:
> + case 0xff:
> + /* Not specified, assuming "not supported". */
> + continue;
> + case 0x20:
> + f->block_erasers[j].block_erase = spi_block_erase_20;
> + break;
> + case 0x52:
> + f->block_erasers[j].block_erase = spi_block_erase_52;
> + break;
> + case 0x60:
> + f->block_erasers[j].block_erase = spi_block_erase_60;
> + break;
> + case 0xc7:
> + f->block_erasers[j].block_erase = spi_block_erase_c7;
> + break;
> + case 0xd7:
> + f->block_erasers[j].block_erase = spi_block_erase_d7;
> + break;
> + case 0xd8:
> + f->block_erasers[j].block_erase = spi_block_erase_d8;
> + break;
> + default:
> + msg_cinfo("%s: unknown opcode (0x%02x) in SFDP table. "
> + "Please report this at "
> + "[email protected]\n",
> + __func__, tmp32);
> + continue;
> + }
> + f->block_erasers[j].eraseblocks[0].size = bsize;
> + f->block_erasers[j].eraseblocks[0].count = total_size/bsize;
> + msg_cspew(" Block eraser %d: %d x %d B with opcode 0x%02x\n",
> + j, total_size/bsize, bsize, tmp32);
> + }
> + return 0;
> +}
> +
> +static int sfdp_fetch_pt(uint32_t addr, uint8_t *buf, uint16_t len)
> +{
> + uint16_t i;
> + if (spi_sfdp(addr, buf, len)) {
> + msg_cerr("Receiving SFDP parameter table failed.\n");
> + return 1;
> + }
> + msg_cspew(" Parameter table contents:\n");
> + for(i = 0; i < len; i++) {
> + if ((i % 8) == 0) {
> + msg_cspew(" 0x%03x: ", i);
> + }
> + msg_cspew(" 0x%02x", buf[i]);
> + if ((i % 8) == 7) {
> + msg_cspew("\n");
> + continue;
> + }
> + if ((i % 8) == 3) {
> + msg_cspew(" ");
> + continue;
> + }
> + }
> + msg_cspew("\n");
> + return 0;
> +}
> +
> +int probe_spi_sfdp(struct flashchip *flash)
> +{
> + int ret = 0;
> + uint8_t buf[8];
> + uint16_t tmp16;
> + uint32_t tmp32;
> + uint8_t nph;
> + uint8_t i;
> + struct sfdp_tbl_hdr *hdrs;
> + uint8_t *hbuf;
> + uint8_t *tbuf;
> +
> + if (spi_sfdp(0x0, buf, 4)) {
> + msg_cerr("Receiving SFDP signature failed.\n");
> + return 0;
> + }
> + tmp32 = buf[0];
> + tmp32 |= ((unsigned int)buf[1]) << 8;
> + tmp32 |= ((unsigned int)buf[2]) << 16;
> + tmp32 |= ((unsigned int)buf[3]) << 24;
> +
> + msg_cspew("SFDP signature = 0x%08x (should be 0x50444653)\n", tmp32);
> + if (tmp32 != 0x50444653) {
> + msg_cdbg("No SFDP signature found.\n");
> + return 0;
> + }
> + if (spi_sfdp(0x4, buf, 3)) {
> + msg_cerr("Receiving SFDP revision and number of parameter "
> + "headers (NPH) failed. ");
> + return 0;
> + }
> + msg_cspew("SFDP revision = %d.%d\n", buf[1], buf[0]);
> + nph = buf[3];
> + msg_cspew("SFDP number of parameter headers (NPH) = %d (+ 1 mandatory)"
> + "\n", nph);
> +
> + /* Fetch all parameter headers, even if we don't use them all (yet). */
> + hbuf = malloc(sizeof(struct sfdp_tbl_hdr) * (nph + 1));
> + hdrs = malloc((nph + 1) * 8);
> + if (hbuf == NULL || hdrs == NULL ) {
> + msg_gerr("Out of memory!\n");
> + exit(1); /* FIXME: shutdown gracefully */
> + }
> + if (spi_sfdp(0x8, hbuf, (nph + 1) * 8)) {
> + msg_cerr("Receiving SFDP parameter table headers failed.\n");
> + goto cleanup_hdrs;
> + }
> +
> + i = 0;
> + do{
> + hdrs[i].id = hbuf[(8 * i) + 0];
> + hdrs[i].v_minor = hbuf[(8 * i) + 1];
> + hdrs[i].v_major = hbuf[(8 * i) + 2];
> + hdrs[i].len = hbuf[(8 * i) + 3];
> + hdrs[i].ptp = hbuf[(8 * i) + 4];
> + hdrs[i].ptp |= ((unsigned int)hbuf[(8 * i) + 5]) << 8;
> + hdrs[i].ptp |= ((unsigned int)hbuf[(8 * i) + 6]) << 16;
> + msg_cspew("SFDP parameter table header %d:\n", i);
> + msg_cspew(" ID 0x%02x, version %d.%d\n", hdrs[i].id,
> + hdrs[i].v_major, hdrs[i].v_minor);
> + tmp16 = hdrs[i].len * 4;
> + tmp32 = hdrs[i].ptp;
> + msg_cspew(" Length %d B, Parameter Table Pointer 0x%06x\n",
> + tmp16, tmp32);
> +
> + tbuf = malloc(tmp16);
> + if (tbuf == NULL) {
> + msg_gerr("Out of memory!\n");
> + exit(1); /* FIXME: shutdown gracefully */
> + }
> + if (sfdp_fetch_pt(tmp32, tbuf, tmp16)){
> + msg_cerr("Fetching SFDP parameter table %d failed.\n",
> + i);
> + free(tbuf);
> + break;
> + }
> + if (i == 0) { /* Mandatory JEDEC SFDP parameter table */
> + if (hdrs[i].id != 0)
> + msg_cdbg("ID of the mandatory JEDEC SFDP "
> + "parameter table is not 0 as demanded "
> + "by JESD216 (warning only).\n");
> +
> + if (hdrs[i].len != (9 * 4)) {
> + msg_cdbg("Length of the mandatory JEDEC SFDP "
> + "parameter table is not 24 B as "
> + "demanded by JESD216.\n");
> + if (hdrs[i].len == (4 * 4))
> + msg_cdbg("It seems like it is the "
> + "preliminary Intel version of "
> + "SFDP, which we don't support."
> + "\n");
> + } else if (sfdp_fill_flash(flash, tbuf, tmp16) == 0)
> + ret = 1;
> + }
> +
> + free(tbuf);
> + i++;
> + } while(i <= nph);
> +
> +cleanup_hdrs:
> + free(hdrs);
> + free(hbuf);
> + return ret;
> +}
> +
> static int probe_spi_rdid_generic(struct flashchip *flash, int bytes)
> {
> unsigned char readarr[4];
>
Could you resend this with the comments addressed and with the squash
patch merged in? I didn't review the SFDP code yet, but the right
presentation makes reviewing easier.
Regards,
Carl-Daniel
--
http://www.hailfinger.org/
_______________________________________________
flashrom mailing list
[email protected]
http://www.flashrom.org/mailman/listinfo/flashrom
