diff --git a/cli_classic.c b/cli_classic.c index 3e5dab9f5..f465785d2 100644 --- a/cli_classic.c +++ b/cli_classic.c @@ -1050,9 +1050,10 @@ int main(int argc, char *argv[]) struct flashctx flashes[8] = {{0}}; struct flashctx *fill_flash; char *tempstr = NULL; - int startchip = -1, chipcount = 0; int i, j; int ret = 0; + int all_matched_count = 0; + const char **all_matched_names = NULL; struct cli_options options = { 0 }; static const char optstring[] = "r::Rw::v::nNVEfc:l:i:p:Lzho:x"; @@ -1209,26 +1210,26 @@ int main(int argc, char *argv[]) msg_pdbg("The following protocols are supported: %s.\n", tempstr ? tempstr : "?"); free(tempstr); - for (j = 0; j < registered_master_count; j++) { - startchip = 0; - while (chipcount < (int)ARRAY_SIZE(flashes)) { - startchip = probe_flash(®istered_masters[j], startchip, &flashes[chipcount], 0, options.chip_to_probe); - if (startchip == -1) - break; - chipcount++; - startchip++; - } + all_matched_count = flashrom_flash_probe_v2(&flashes[0], &all_matched_names, + NULL, options.chip_to_probe); + if (all_matched_count == -1) { + /* -1 is the ret code which means "something went wrong". + * Multiple match and no match are different ret codes. + * More details about the error were printed during actual probing. */ + msg_cerr("Error: probing failed.\n"); + ret = 1; + goto out_shutdown; } - if (chipcount > 1) { + if (all_matched_count > 1) { msg_cinfo("Multiple flash chip definitions match the detected chip(s): \"%s\"", flashes[0].chip->name); - for (i = 1; i < chipcount; i++) - msg_cinfo(", \"%s\"", flashes[i].chip->name); + for (int ind = 1; ind < all_matched_count; ind++) + msg_cinfo(", \"%s\"", all_matched_names[ind]); msg_cinfo("\nPlease specify which chip definition to use with the -c option.\n"); ret = 1; goto out_shutdown; - } else if (!chipcount) { + } else if (!all_matched_count) { msg_cinfo("No EEPROM/flash device found.\n"); if (!options.force || !options.chip_to_probe) { msg_cinfo("Note: flashrom can never write if the flash chip isn't found " @@ -1253,6 +1254,8 @@ int main(int argc, char *argv[]) if (compatible_masters > 1) msg_cinfo("More than one compatible controller found for the requested flash " "chip, using the first one.\n"); + + int startchip = -1; for (j = 0; j < registered_master_count; j++) { mst = ®istered_masters[j]; startchip = probe_flash(mst, 0, &flashes[0], 1, options.chip_to_probe); @@ -1535,10 +1538,11 @@ out_release: out_shutdown: flashrom_programmer_shutdown(NULL); out: - for (i = 0; i < chipcount; i++) { - flashrom_layout_release(flashes[i].default_layout); - free(flashes[i].chip); + for (int ind = 0; ind < all_matched_count; ind++) { + flashrom_layout_release(flashes[ind].default_layout); + free(flashes[ind].chip); } + flashrom_data_free(all_matched_names); free_options(&options); ret |= close_logfile(); diff --git a/include/libflashrom.h b/include/libflashrom.h index f30ad6b2d..ca9a5e360 100644 --- a/include/libflashrom.h +++ b/include/libflashrom.h @@ -279,6 +279,45 @@ int flashrom_programmer_shutdown(struct flashrom_programmer *flashprog); * or 1 on any other error. */ int flashrom_flash_probe(struct flashrom_flashctx **flashctx, const struct flashrom_programmer *flashprog, const char *chip_name); + +/** + * @brief Probe for a flash chip, v2 + * + * Probes for a flash chip and returns a flash context, that can be used + * later with flash chip and @ref flashrom-ops "image operations", if + * exactly one matching chip is found. + * + * Returns the list of names for all chips that matched, and the count of + * how many chips matched. + * + * Memory for the list of chips is dynamically allocated according to the + * number of chips found, and always needs to be freed with flashrom_data_free + * afterwards (including when no matches found or error happened). + * + * Note that if chip_name param is set, then probing happens only once, only + * for this one requested chip name. So the number of matches that can be + * returned in this case will be either 1 or 0 (and -1 for error). + * + * @param[out] flashctx Points to a struct flashrom_flashctx + * that will be set if exactly one chip is found. *flashctx + * has to be freed by the caller with @ref flashrom_flash_release. + * @param[out] all_matched_names pointer to an array containing the names of all chips + * that were successfully probed, terminated with a NULL pointer. + * If no chips are found, the returned array contains + * a single NULL element. Callers must free the array once unused + * by calling `flashrom_data_free`. + * @param[in] flashprog The flash programmer used to access the chip, + * currently unused. + * @param[in] chip_name Name of a chip to probe for, or NULL to probe for + * all known chips. + * @return the number of matched chips (which can be 0) on success, + * -1 if error happened during probing. + */ +int flashrom_flash_probe_v2(struct flashrom_flashctx *flashctx, + const char *** const all_matched_names, + const struct flashrom_programmer *flashprog, + const char *chip_name); + /** * @brief Returns the size of the specified flash chip in bytes. * diff --git a/libflashrom.c b/libflashrom.c index d6bd09828..a45e0d170 100644 --- a/libflashrom.c +++ b/libflashrom.c @@ -329,8 +329,6 @@ int flashrom_programmer_shutdown(struct flashrom_programmer *const flashprog) return programmer_shutdown(); } -/* TODO: flashrom_programmer_capabilities()? */ - int flashrom_flash_probe(struct flashrom_flashctx **const flashctx, const struct flashrom_programmer *const flashprog, const char *const chip_name) @@ -363,6 +361,56 @@ int flashrom_flash_probe(struct flashrom_flashctx **const flashctx, return ret; } +int flashrom_flash_probe_v2(struct flashrom_flashctx *flashctx, + const char *** const all_matched_names, + const struct flashrom_programmer *flashprog, + const char *chip_name) +{ + int startchip; + unsigned int all_matched_count = 0; // start with no match found + const char **matched_names = calloc(flashchips_size + 1, sizeof(char*)); + + for (int i = 0; i < registered_master_count; i++) { + startchip = 0; + while (all_matched_count < flashchips_size) { + struct flashrom_flashctx second_flashctx = { 0, }; // used for second and more matches + struct flashctx *context_for_probing = (all_matched_count > 0) ? &second_flashctx : flashctx; + startchip = probe_flash(®istered_masters[i], startchip, context_for_probing, 0, chip_name); + + if (startchip == -1) + break; + + matched_names[all_matched_count] = context_for_probing->chip->name; + all_matched_count++; + startchip++; + + if (all_matched_count > 1) { + /* It's used for the second and subsequent probing. */ + flashrom_layout_release(second_flashctx.default_layout); + free(second_flashctx.chip); + } + } + } + + matched_names[all_matched_count] = NULL; + matched_names = realloc(matched_names, (all_matched_count + 1) * sizeof(char*)); + *all_matched_names = matched_names; + + /* The return value should be the number of matched chips, or -1 on error. + * However, currently the error is never returned, because probe_flash return code + * is -1 for both cases of error and no match found, so there is no way + * to distinguish between probing error (e.g. error talking to hw, or out of memory) + * and no match. + * (no match can happen if chip is not in our database, even if hw work perfectly). + * + * TODO improve probe_flash return code to distinguish between + * probing error and no match found. */ + int ret = (int) all_matched_count; + + return ret; +} + + size_t flashrom_flash_getsize(const struct flashrom_flashctx *const flashctx) { return flashctx->chip->total_size * 1024; diff --git a/libflashrom.map b/libflashrom.map index fc6724ad0..2b757e5c2 100644 --- a/libflashrom.map +++ b/libflashrom.map @@ -6,6 +6,7 @@ LIBFLASHROM_1.0 { flashrom_flash_erase; flashrom_flash_getsize; flashrom_flash_probe; + flashrom_flash_probe_v2; flashrom_flash_release; flashrom_image_read; flashrom_image_verify; diff --git a/tests/dummyflasher.c b/tests/dummyflasher.c index 9878bef70..0faecb949 100644 --- a/tests/dummyflasher.c +++ b/tests/dummyflasher.c @@ -42,6 +42,57 @@ void dummy_probe_lifecycle_test_success(void **state) run_probe_lifecycle(state, &dummy_io, &programmer_dummy, "bus=spi,emulate=W25Q128FV", "W25Q128.V"); } +void dummy_probe_v2_one_match_for_W25Q128FV(void **state) +{ + struct io_mock_fallback_open_state dummy_fallback_open_state = { + .noc = 0, + .paths = { NULL }, + }; + const struct io_mock dummy_io = { + .fallback_open_state = &dummy_fallback_open_state, + }; + + const char *expected_matched_names[1] = {"W25Q128.V"}; + run_probe_v2_lifecycle(state, &dummy_io, &programmer_dummy, "bus=spi,emulate=W25Q128FV", + NULL, /* any chip name */ + expected_matched_names, 1); +} + +void dummy_probe_v2_six_matches_for_MX25L6436(void **state) +{ + struct io_mock_fallback_open_state dummy_fallback_open_state = { + .noc = 0, + .paths = { NULL }, + }; + const struct io_mock dummy_io = { + .fallback_open_state = &dummy_fallback_open_state, + }; + + const char *expected_matched_names[6] = {"MX25L6405", + "MX25L6405D", + "MX25L6406E/MX25L6408E", + "MX25L6436E/MX25L6445E/MX25L6465E", + "MX25L6473E", + "MX25L6473F"}; + run_probe_v2_lifecycle(state, &dummy_io, &programmer_dummy, "bus=spi,emulate=MX25L6436", + NULL, /* any chip name */ + expected_matched_names, 6); +} + +void dummy_probe_v2_no_matches_found(void **state) +{ + struct io_mock_fallback_open_state dummy_fallback_open_state = { + .noc = 0, + .paths = { NULL }, + }; + const struct io_mock dummy_io = { + .fallback_open_state = &dummy_fallback_open_state, + }; + + run_probe_v2_lifecycle(state, &dummy_io, &programmer_dummy, "bus=spi,emulate=MX25L6436", + "NONEXISTENT", NULL /* no matched names */, 0); +} + void dummy_probe_variable_size_test_success(void **state) { struct io_mock_fallback_open_state dummy_fallback_open_state = { @@ -163,6 +214,9 @@ void dummy_freq_param_init(void **state) #else SKIP_TEST(dummy_basic_lifecycle_test_success) SKIP_TEST(dummy_probe_lifecycle_test_success) + SKIP_TEST(dummy_probe_v2_one_match_for_W25Q128FV) + SKIP_TEST(dummy_probe_v2_six_matches_for_MX25L6436) + SKIP_TEST(dummy_probe_v2_no_matches_found) SKIP_TEST(dummy_probe_variable_size_test_success) SKIP_TEST(dummy_init_fails_unhandled_param_test_success) SKIP_TEST(dummy_init_success_invalid_param_test_success) diff --git a/tests/lifecycle.c b/tests/lifecycle.c index 95c424969..a92e93efc 100644 --- a/tests/lifecycle.c +++ b/tests/lifecycle.c @@ -17,22 +17,61 @@ static void probe_chip(const struct programmer_entry *prog, struct flashrom_programmer *flashprog, - const char *const chip_name) + const char *const chip_name, + const char **expected_matched_names, /* unused in probe v1 */ + unsigned int expected_matched_count /* unused in probe v1 */) { struct flashrom_flashctx *flashctx; printf("Testing flashrom_flash_probe for programmer=%s, chip=%s ... \n", prog->name, chip_name); + assert_int_equal(0, flashrom_flash_probe(&flashctx, flashprog, chip_name)); + if (chip_name) + assert_int_equal(0, strcmp(chip_name, flashctx->chip->name)); + printf("... flashrom_flash_probe for programmer=%s successful\n", prog->name); flashrom_flash_release(flashctx); /* cleanup */ } +static void probe_chip_v2(const struct programmer_entry *prog, + struct flashrom_programmer *flashprog, + const char *const chip_name, + const char **expected_matched_names, + unsigned int expected_matched_count) +{ + struct flashrom_flashctx flashctx = { 0 }; + const char **all_matched_names = NULL; + + printf("Testing flashrom_flash_probe_v2 for programmer=%s, chip=%s ... \n", prog->name, chip_name); + assert_int_equal(expected_matched_count, flashrom_flash_probe_v2(&flashctx, &all_matched_names, + flashprog, chip_name)); + + for (unsigned int i = 0; i < expected_matched_count; i++) + assert_int_equal(0, strcmp(expected_matched_names[i], all_matched_names[i])); + + assert_null(all_matched_names[expected_matched_count]); + + if (chip_name && expected_matched_count > 0) + assert_int_equal(0, strcmp(chip_name, flashctx.chip->name)); + + printf("... flashrom_flash_probe_v2 for programmer=%s successful\n", prog->name); + + /* cleanup */ + flashrom_data_free(all_matched_names); + flashrom_layout_release(flashctx.default_layout); + free(flashctx.chip); +} + static void run_lifecycle(void **state, const struct io_mock *io, const struct programmer_entry *prog, const char *param, const char *const chip_name, + const char **expected_matched_names, + unsigned int expected_matched_count, void (*action)(const struct programmer_entry *prog, struct flashrom_programmer *flashprog, - const char *const chip_name)) + const char *const chip_name, + const char **expected_matched_names, + unsigned int expected_matched_count)) { (void) state; /* unused */ @@ -45,7 +84,7 @@ static void run_lifecycle(void **state, const struct io_mock *io, const struct p printf("... flashrom_programmer_init for programmer=%s successful\n", prog->name); if (action) - action(prog, flashprog, chip_name); + action(prog, flashprog, chip_name, expected_matched_names, expected_matched_count); printf("Testing flashrom_programmer_shutdown for programmer=%s ...\n", prog->name); assert_int_equal(0, flashrom_programmer_shutdown(flashprog)); @@ -59,7 +98,9 @@ void run_basic_lifecycle(void **state, const struct io_mock *io, { /* Basic lifecycle only does init and shutdown, * so neither chip name nor action is needed. */ - run_lifecycle(state, io, prog, param, NULL /* chip_name */, NULL /* action */); + run_lifecycle(state, io, prog, param, NULL /* chip_name */, + NULL /* expected_matched_names, */, 0 /* expected_matched_count, */, + NULL /* action */); } void run_probe_lifecycle(void **state, const struct io_mock *io, @@ -67,7 +108,20 @@ void run_probe_lifecycle(void **state, const struct io_mock *io, { /* Each probe lifecycle should run independently, without cache. */ clear_spi_id_cache(); - run_lifecycle(state, io, prog, param, chip_name, &probe_chip); + run_lifecycle(state, io, prog, param, chip_name, + NULL /* expected_matched_names, */, 0 /* expected_matched_count, */, + &probe_chip); +} + +void run_probe_v2_lifecycle(void **state, const struct io_mock *io, + const struct programmer_entry *prog, const char *param, + const char *const chip_name, + const char **expected_matched_names, unsigned int expected_matched_count) +{ + /* Each probe lifecycle should run independently, without cache. */ + clear_spi_id_cache(); + run_lifecycle(state, io, prog, param, chip_name, + expected_matched_names, expected_matched_count, &probe_chip_v2); } void run_init_error_path(void **state, const struct io_mock *io, const struct programmer_entry *prog, diff --git a/tests/lifecycle.h b/tests/lifecycle.h index b4c2fbedd..1565a698d 100644 --- a/tests/lifecycle.h +++ b/tests/lifecycle.h @@ -34,6 +34,11 @@ void run_basic_lifecycle(void **state, const struct io_mock *io, void run_probe_lifecycle(void **state, const struct io_mock *io, const struct programmer_entry *prog, const char *param, const char *const chip_name); +void run_probe_v2_lifecycle(void **state, const struct io_mock *io, + const struct programmer_entry *prog, const char *param, + const char *const chip_name, + const char **expected_matched_names, unsigned int expected_matched_count); + void run_init_error_path(void **state, const struct io_mock *io, const struct programmer_entry *prog, const char *param, const int error_code); #endif /* __LIFECYCLE_H__ */ diff --git a/tests/tests.c b/tests/tests.c index 2a79d327d..c43af3715 100644 --- a/tests/tests.c +++ b/tests/tests.c @@ -504,6 +504,9 @@ int main(int argc, char *argv[]) const struct CMUnitTest lifecycle_tests[] = { cmocka_unit_test(dummy_basic_lifecycle_test_success), cmocka_unit_test(dummy_probe_lifecycle_test_success), + cmocka_unit_test(dummy_probe_v2_one_match_for_W25Q128FV), + cmocka_unit_test(dummy_probe_v2_six_matches_for_MX25L6436), + cmocka_unit_test(dummy_probe_v2_no_matches_found), cmocka_unit_test(dummy_probe_variable_size_test_success), cmocka_unit_test(dummy_init_fails_unhandled_param_test_success), cmocka_unit_test(dummy_init_success_invalid_param_test_success), diff --git a/tests/tests.h b/tests/tests.h index e87c00ca6..6ec40568e 100644 --- a/tests/tests.h +++ b/tests/tests.h @@ -53,6 +53,9 @@ void probe_spi_st95_test_success(void **state); /* spi95.c */ /* lifecycle.c */ void dummy_basic_lifecycle_test_success(void **state); void dummy_probe_lifecycle_test_success(void **state); +void dummy_probe_v2_one_match_for_W25Q128FV(void **state); +void dummy_probe_v2_six_matches_for_MX25L6436(void **state); +void dummy_probe_v2_no_matches_found(void **state); void dummy_probe_variable_size_test_success(void **state); void dummy_init_fails_unhandled_param_test_success(void **state); void dummy_init_success_invalid_param_test_success(void **state);