[flashrom] [PATCH] Add support for generic SPI RES matching.
Carl-Daniel Hailfinger
c-d.hailfinger.devel.2006 at gmx.net
Fri Aug 31 02:18:33 CEST 2012
Am 30.08.2012 23:55 schrieb Stefan Tauner:
> Support generic 1-byte SPI RES matching (low match quality) and enable
> read support by guessing the chip size from the ID.
> Support generic 2-byte SPI RES matching (high match quality).
>
> Signed-off-by: Carl-Daniel Hailfinger <c-d.hailfinger.devel.2006 at gmx.net>
> Signed-off-by: Stefan Tauner <stefan.tauner at student.tuwien.ac.at>
> ---
> chipdrivers.h | 1 +
> flashchips.c | 31 +++++++++++++++++++++++++++++
> flashrom.c | 4 +++-
> spi25.c | 61 +++++++++++++++++++++++++++++++++++++++++++++------------
> 4 files changed, 84 insertions(+), 13 deletions(-)
>
> diff --git a/chipdrivers.h b/chipdrivers.h
> index b43e16e..63c5d37 100644
> --- a/chipdrivers.h
> +++ b/chipdrivers.h
> @@ -33,6 +33,7 @@ int probe_spi_rdid4(struct flashctx *flash);
> int probe_spi_rems(struct flashctx *flash);
> int probe_spi_res1(struct flashctx *flash);
> int probe_spi_res2(struct flashctx *flash);
> +int probe_spi_automagic_res1(struct flashctx *flash);
> int spi_write_enable(struct flashctx *flash);
> int spi_write_disable(struct flashctx *flash);
> int spi_block_erase_20(struct flashctx *flash, unsigned int addr, unsigned int blocklen);
> diff --git a/flashchips.c b/flashchips.c
> index 8fe50bd..c175108 100644
> --- a/flashchips.c
> +++ b/flashchips.c
> @@ -9662,6 +9662,24 @@ const struct flashchip flashchips[] = {
>
> {
> .vendor = "Generic",
> + .name = "unknown SPI chip (RES1)",
> + .bustype = BUS_SPI,
> + .manufacture_id = GENERIC_MANUF_ID,
> + .model_id = GENERIC_DEVICE_ID,
> + .total_size = 0,
> + .page_size = 256,
> + .tested = TEST_BAD_PREW,
> + .probe = probe_spi_automagic_res1,
> + .probe_timing = TIMING_ZERO,
> + .block_erasers = {},
> + .unlock = spi_disable_blockprotect,
Please kill the unlock here. AFAICS we have no way to determine how to
unlock for a REMS1 chip.
> + .write = NULL,
> + .read = spi_chip_read,
> + .voltage = {},
> + },
The RES1 chip should be below the RES2 chip to make sure the RES2 chip
matches first (better match quality) and prevents RES1 matching if RES2
matches.
> +
> + {
> + .vendor = "Generic",
> .name = "unknown SPI chip (REMS)",
> .bustype = BUS_SPI,
> .manufacture_id = GENERIC_MANUF_ID,
> @@ -9673,5 +9691,18 @@ const struct flashchip flashchips[] = {
> .write = NULL,
> },
>
> + {
> + .vendor = "Generic",
> + .name = "unknown SPI chip (RES2)",
> + .bustype = BUS_SPI,
> + .manufacture_id = GENERIC_MANUF_ID,
> + .model_id = GENERIC_DEVICE_ID,
> + .total_size = 0,
> + .page_size = 256,
> + .tested = TEST_BAD_PREW,
> + .probe = probe_spi_res2,
> + .write = NULL,
> + },
> +
> { NULL }
> };
> diff --git a/flashrom.c b/flashrom.c
> index fdc5412..148f969 100644
> --- a/flashrom.c
> +++ b/flashrom.c
> @@ -1028,7 +1028,9 @@ int probe_flash(struct registered_programmer *pgm, int startchip, struct flashct
> if (startchip == 0)
> break;
> /* Not the first flash chip detected on this bus, but not a generic match either. */
> - if ((flash->chip->model_id != GENERIC_DEVICE_ID) && (flash->chip->model_id != SFDP_DEVICE_ID))
> + if ((flash->chip->manufacture_id != GENERIC_MANUF_ID) &&
> + (flash->chip->model_id != GENERIC_DEVICE_ID) &&
> + (flash->chip->model_id != SFDP_DEVICE_ID))
> break;
> /* Not the first flash chip detected on this bus, and it's just a generic match. Ignore it. */
> notfound:
> diff --git a/spi25.c b/spi25.c
> index 914b821..7d9e948 100644
> --- a/spi25.c
> +++ b/spi25.c
> @@ -232,6 +232,7 @@ int probe_spi_rems(struct flashctx *flash)
>
> int probe_spi_res1(struct flashctx *flash)
> {
> + const struct flashchip *chip = flash->chip;
> static const unsigned char allff[] = {0xff, 0xff, 0xff};
> static const unsigned char all00[] = {0x00, 0x00, 0x00};
> unsigned char readarr[3];
> @@ -265,18 +266,23 @@ int probe_spi_res1(struct flashctx *flash)
>
> msg_cdbg("%s: id 0x%x\n", __func__, id2);
>
> - if (id2 != flash->chip->model_id)
> - return 0;
> + if (id2 == flash->chip->model_id) {
my bad, this could have been simplified to use chip->model_id instead of
flash->chip->model_id
> + /* Print the status register to tell the user about possible write protection. */
> + spi_prettyprint_status_register(flash);
>
> - /* Print the status register to tell the
> - * user about possible write protection.
> - */
> - spi_prettyprint_status_register(flash);
> - return 1;
> + return 1;
> + }
> +
> + /* Test if there is any device ID. */
> + if (GENERIC_MANUF_ID == chip->manufacture_id && id2 == chip->model_id)
> + return 1;
> +
> + return 0;
> }
>
> int probe_spi_res2(struct flashctx *flash)
> {
> + const struct flashchip *chip = flash->chip;
> unsigned char readarr[2];
> uint32_t id1, id2;
>
> @@ -289,13 +295,44 @@ int probe_spi_res2(struct flashctx *flash)
>
> msg_cdbg("%s: id1 0x%x, id2 0x%x\n", __func__, id1, id2);
>
> - if (id1 != flash->chip->manufacture_id || id2 != flash->chip->model_id)
> + if (id1 == chip->manufacture_id && id2 == chip->model_id) {
> + /* Print the status register to tell the
> + * user about possible write protection.
> + */
> + spi_prettyprint_status_register(flash);
> +
> + return 1;
> + }
> +
> + /* Test if this is a pure vendor match. */
> + if (id1 == chip->manufacture_id && GENERIC_DEVICE_ID == chip->model_id)
> + return 1;
> +
> + /* Test if there is any vendor ID and if this is indeed a two-byte RES. */
> + if (GENERIC_MANUF_ID == chip->manufacture_id && id1 != 0xff && id1 != id2)
> + return 1;
> +
> + return 0;
> +}
> +
> +int probe_spi_automagic_res1(struct flashctx *flash)
> +{
> + unsigned char readarr[1];
> + unsigned int total_size;
> + uint32_t id1;
IMHO we should check here that neither RDID nor REMS nor RES2 work
before proceeding with RES1.
> +
> + if (spi_res(flash, readarr, 1))
> return 0;
>
> - /* Print the status register to tell the
> - * user about possible write protection.
> - */
> - spi_prettyprint_status_register(flash);
> + id1 = readarr[0];
> + msg_cdbg("%s: id1 0x%02x\n", __func__, id1);
> + if (id1 < 0x10 || id1 > 0x17) {
> + msg_cdbg("RES ID out of bounds!\n");
> + return 0;
> + }
> + total_size = (2 << id1) / 1024;
> + flash->chip->total_size = total_size;
> + flash->chip->tested = TEST_OK_PR | TEST_BAD_ERASE | TEST_BAD_WRITE;
> return 1;
> }
>
I might have missed something, but right now I hope the review is complete.
Regards,
Carl-Daniel
--
http://www.hailfinger.org/
More information about the flashrom
mailing list