Anastasia Klimchuk submitted this change.
Fix FEATURE_NO_ERASE chips and add test for them
New check was added to `check_block_eraser` in
commit 0f389aea9e630c3b21547a5dd8dbe572a8502853 but it was not
handling FEATURE_NO_ERASE chips.
This patch fixes processing such chips and adds test to run
write and verify with dummyflasher for FEATURE_NO_ERASE chips.
Ticket: https://ticket.coreboot.org/issues/553
Change-Id: I582fe00da0715e9b5e92fcc9d15d5a90a2615117
Signed-off-by: Anastasia Klimchuk <aklm@flashrom.org>
Reviewed-on: https://review.coreboot.org/c/flashrom/+/84203
Tested-by: build bot (Jenkins) <no-reply@coreboot.org>
Reviewed-by: Peter Marheine <pmarheine@chromium.org>
---
M flashrom.c
M include/flash.h
M tests/chip.c
M tests/tests.c
M tests/tests.h
5 files changed, 80 insertions(+), 7 deletions(-)
diff --git a/flashrom.c b/flashrom.c
index 01a41c4..e2c60f4 100644
--- a/flashrom.c
+++ b/flashrom.c
@@ -484,14 +484,15 @@
if (flash->mst->buses_supported & BUS_SPI) {
const uint8_t *opcode = spi_get_opcode_from_erasefn(eraser.block_erase);
- for (int i = 0; opcode[i]; i++) {
- if (!spi_probe_opcode(flash, opcode[i])) {
- if (log)
- msg_cdbg("block erase function and layout found "
- "but SPI master doesn't support the function. ");
- return 1;
+ if (opcode)
+ for (int i = 0; opcode[i]; i++) {
+ if (!spi_probe_opcode(flash, opcode[i])) {
+ if (log)
+ msg_cdbg("block erase function and layout found "
+ "but SPI master doesn't support the function. ");
+ return 1;
+ }
}
- }
}
// TODO: Once erase functions are annotated with allowed buses, check that as well.
return 0;
diff --git a/include/flash.h b/include/flash.h
index d9a20ad..d0e55af 100644
--- a/include/flash.h
+++ b/include/flash.h
@@ -148,6 +148,14 @@
* other flash chips, such as the ENE KB9012 internal flash, work the opposite way.
*/
#define FEATURE_ERASED_ZERO (1 << 18)
+/*
+ * Feature indicates that the chip does not require erase before writing:
+ * write operations can set any bit to any value without first doing an erase,
+ * but bulk erase operations may still be supported.
+ *
+ * EEPROMs usually behave this way (compare to Flash, which requires erase),
+ * for example the ST M95M02.
+ */
#define FEATURE_NO_ERASE (1 << 19)
#define FEATURE_WRSR_EXT2 (1 << 20)
diff --git a/tests/chip.c b/tests/chip.c
index 3e1ae3b..604d79e 100644
--- a/tests/chip.c
+++ b/tests/chip.c
@@ -27,6 +27,7 @@
#include <include/test.h>
#include <stdio.h>
+#include <stdlib.h>
#include <string.h>
#include "tests.h"
@@ -132,6 +133,25 @@
}},
};
+/* Chip expected to be processed with dummyflasher, so using real op functions. */
+static const struct flashchip chip_no_erase = {
+ .vendor = "aklm&dummyflasher",
+ .total_size = 16 * 1024,
+ .tested = TEST_OK_PREW,
+ .read = SPI_CHIP_READ,
+ .write = SPI_CHIP_WRITE256,
+ .page_size = 256,
+ .feature_bits = FEATURE_NO_ERASE | FEATURE_ERASED_ZERO,
+ .block_erasers =
+ {
+ {
+ .eraseblocks = { {16 * 1024 * 1024, 1} },
+ /* Special erase fn for chips without erase capability. */
+ .block_erase = SPI_BLOCK_ERASE_EMULATION,
+ }
+ },
+};
+
/* Setup the struct for W25Q128.V, all values come from flashchips.c */
static const struct flashchip chip_W25Q128_V = {
.vendor = "aklm&dummyflasher",
@@ -384,6 +404,48 @@
free(newcontents);
}
+void write_chip_feature_no_erase(void **state)
+{
+ (void) state; /* unused */
+
+ static struct io_mock_fallback_open_state data = {
+ .noc = 0,
+ .paths = { NULL },
+ };
+ const struct io_mock chip_io = {
+ .fallback_open_state = &data,
+ };
+
+ struct flashrom_flashctx flashctx = { 0 };
+ struct flashrom_layout *layout;
+
+ /*
+ * Tricking the dummyflasher by asking to emulate W25Q128FV but giving to it
+ * mock chip with FEATURE_NO_ERASE.
+ * As long as chip size is the same, this is fine.
+ */
+ struct flashchip mock_chip = chip_no_erase;
+ const char *param_dup = "bus=spi,emulate=W25Q128FV";
+
+ setup_chip(&flashctx, &layout, &mock_chip, param_dup, &chip_io);
+
+ /* See comment in write_chip_test_success */
+ const char *const filename = "-";
+ unsigned long size = mock_chip.total_size * 1024;
+ uint8_t *const newcontents = malloc(size);
+ assert_non_null(newcontents);
+
+ printf("Write chip operation started.\n");
+ assert_int_equal(0, read_buf_from_file(newcontents, size, filename));
+ assert_int_equal(0, flashrom_image_write(&flashctx, newcontents, size, NULL));
+ assert_int_equal(0, flashrom_image_verify(&flashctx, newcontents, size));
+ printf("Write chip operation done.\n");
+
+ teardown(&layout);
+
+ free(newcontents);
+}
+
void write_nonaligned_region_with_dummyflasher_test_success(void **state)
{
(void) state; /* unused */
diff --git a/tests/tests.c b/tests/tests.c
index 165390a..78f1b47 100644
--- a/tests/tests.c
+++ b/tests/tests.c
@@ -498,6 +498,7 @@
cmocka_unit_test(read_chip_with_dummyflasher_test_success),
cmocka_unit_test(write_chip_test_success),
cmocka_unit_test(write_chip_with_dummyflasher_test_success),
+ cmocka_unit_test(write_chip_feature_no_erase),
cmocka_unit_test(write_nonaligned_region_with_dummyflasher_test_success),
cmocka_unit_test(verify_chip_test_success),
cmocka_unit_test(verify_chip_with_dummyflasher_test_success),
diff --git a/tests/tests.h b/tests/tests.h
index 8cf8be0..3536d1d 100644
--- a/tests/tests.h
+++ b/tests/tests.h
@@ -86,6 +86,7 @@
void read_chip_with_dummyflasher_test_success(void **state);
void write_chip_test_success(void **state);
void write_chip_with_dummyflasher_test_success(void **state);
+void write_chip_feature_no_erase(void **state);
void write_nonaligned_region_with_dummyflasher_test_success(void **state);
void verify_chip_test_success(void **state);
void verify_chip_with_dummyflasher_test_success(void **state);
To view, visit change 84203. To unsubscribe, or for help writing mail filters, visit settings.