May 2020 - flashrom-gerrit - mail.coreboot.org

Change in flashrom[master]: [WIP] Add .clang-format to help with patch formatting
by David Hendricks (Code Review) 23 Jun '20

23 Jun '20

Hello Vadim Bendebury, I'd like you to do a code review. Please visit https://review.coreboot.org/c/flashrom/+/38673 to review the following change. Change subject: [WIP] Add .clang-format to help with patch formatting ...................................................................... [WIP] Add .clang-format to help with patch formatting This is based on CB:38208 by Vadim, but with a few added changes. Here is the original commit message. This is a copy from the coreboot tree, could be tweaked to better match the flashrom tree if there are any differences. Once this file is in place a script could be deployed which would format only new/changed lines in git patches. Change-Id: I1fe22809c4a2d73fa02f4de55290d990a9750b86 Signed-off-by: Vadim Bendebury <vbendeb(a)chromium.org> Signed-off-by: David Hendricks <david.hendricks(a)gmail.com> --- A .clang-format 1 file changed, 21 insertions(+), 0 deletions(-) git pull ssh://review.coreboot.org:29418/flashrom refs/changes/73/38673/1 diff --git a/.clang-format b/.clang-format new file mode 100644 index 0000000..45c116b --- /dev/null +++ b/.clang-format @@ -0,0 +1,21 @@ +BasedOnStyle: LLVM +Language: Cpp +IndentWidth: 8 +UseTab: Always +BreakBeforeBraces: Linux +AllowShortIfStatementsOnASingleLine: false +IndentCaseLabels: false +SortIncludes: false +ContinuationIndentWidth: 8 +ColumnLimit: 112 +AlwaysBreakBeforeMultilineStrings: true +AllowShortLoopsOnASingleLine: false +AllowShortFunctionsOnASingleLine: false +AlignEscapedNewlinesLeft: false +AlignTrailingComments: true +AllowAllParametersOfDeclarationOnNextLine: false +AlignAfterOpenBracket: true +SpaceAfterCStyleCast: false +MaxEmptyLinesToKeep: 2 +BreakBeforeBinaryOperators: NonAssignment +BreakStringLiterals: false -- To view, visit https://review.coreboot.org/c/flashrom/+/38673 To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings Gerrit-Project: flashrom Gerrit-Branch: master Gerrit-Change-Id: I1fe22809c4a2d73fa02f4de55290d990a9750b86 Gerrit-Change-Number: 38673 Gerrit-PatchSet: 1 Gerrit-Owner: David Hendricks <david.hendricks(a)gmail.com> Gerrit-Reviewer: Vadim Bendebury <vbendeb(a)chromium.org> Gerrit-MessageType: newchange

2 7

Change in flashrom[master]: tests/spi25.c: Add unit-test coverage of spi95.c
by Edward O'Callaghan (Code Review) 17 Jun '20

17 Jun '20

Edward O'Callaghan has uploaded this change for review. ( https://review.coreboot.org/c/flashrom/+/41656 ) Change subject: tests/spi25.c: Add unit-test coverage of spi95.c ...................................................................... tests/spi25.c: Add unit-test coverage of spi95.c BUG=b:157280555 BRANCH=none TEST=builds Change-Id: I6de59451b82131b58114b268ff6dd0b18cd5952b Signed-off-by: Edward O'Callaghan <quasisec(a)google.com> --- M tests/spi25.c M tests/tests.c M tests/tests.h 3 files changed, 21 insertions(+), 0 deletions(-) git pull ssh://review.coreboot.org:29418/flashrom refs/changes/56/41656/1 diff --git a/tests/spi25.c b/tests/spi25.c index cf8345c..8a9b9ee 100644 --- a/tests/spi25.c +++ b/tests/spi25.c @@ -168,3 +168,22 @@ will_return(__wrap_spi_send_command, AT25F_RDID_INSIZE); assert_int_equal(0, probe_spi_at25f(&flashctx)); } + +/* spi95.c */ +void probe_spi_st95_test_success(void **state) +{ + (void) state; /* unused */ + + /* setup initial test state. */ + struct flashctx flashctx = { .chip = &mock_chip }; + expect_memory(__wrap_spi_send_command, flash, + &flashctx, sizeof(flashctx)); + + /* chip total size < 64K. */ + uint32_t rdid_outsize = ST_M95_RDID_2BA_OUTSIZE; // 16 bit address + + will_return(__wrap_spi_send_command, rdid_outsize); + will_return(__wrap_spi_send_command, ST_M95_RDID); + will_return(__wrap_spi_send_command, ST_M95_RDID_INSIZE); + assert_int_equal(0, probe_spi_st95(&flashctx)); +} diff --git a/tests/tests.c b/tests/tests.c index 2ba293b..e04e204 100644 --- a/tests/tests.c +++ b/tests/tests.c @@ -50,6 +50,7 @@ cmocka_unit_test(probe_spi_res2_test_success), cmocka_unit_test(probe_spi_res3_test_success), cmocka_unit_test(probe_spi_at25f_test_success), + cmocka_unit_test(probe_spi_st95_test_success), /* spi95.c */ }; ret |= cmocka_run_group_tests_name("spi25.c tests", spi25_tests, NULL, NULL); diff --git a/tests/tests.h b/tests/tests.h index b4913f1..a20fb95 100644 --- a/tests/tests.h +++ b/tests/tests.h @@ -26,5 +26,6 @@ void probe_spi_res2_test_success(void **state); void probe_spi_res3_test_success(void **state); void probe_spi_at25f_test_success(void **state); +void probe_spi_st95_test_success(void **state); /* spi95.c */ #endif /* TESTS_H */ -- To view, visit https://review.coreboot.org/c/flashrom/+/41656 To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings Gerrit-Project: flashrom Gerrit-Branch: master Gerrit-Change-Id: I6de59451b82131b58114b268ff6dd0b18cd5952b Gerrit-Change-Number: 41656 Gerrit-PatchSet: 1 Gerrit-Owner: Edward O'Callaghan <quasisec(a)chromium.org> Gerrit-MessageType: newchange

2 5

Change in flashrom[master]: tests/: Add helper.c unit tests
by Edward O'Callaghan (Code Review) 17 Jun '20

17 Jun '20

Edward O'Callaghan has uploaded this change for review. ( https://review.coreboot.org/c/flashrom/+/41655 ) Change subject: tests/: Add helper.c unit tests ...................................................................... tests/: Add helper.c unit tests BUG=b:157280555 BRANCH=none TEST=builds Change-Id: If4a1fe7c499f51bb9d7cd48ef26caf9dfae3c1fa Signed-off-by: Edward O'Callaghan <quasisec(a)google.com> --- A tests/helpers.c M tests/meson.build M tests/tests.c M tests/tests.h 4 files changed, 85 insertions(+), 0 deletions(-) git pull ssh://review.coreboot.org:29418/flashrom refs/changes/55/41655/1 diff --git a/tests/helpers.c b/tests/helpers.c new file mode 100644 index 0000000..ab0863f --- /dev/null +++ b/tests/helpers.c @@ -0,0 +1,64 @@ +#include <include/test.h> + +#include "flash.h" + +#include <stdint.h> +#include <stdlib.h> + + +void address_to_bits_test_success(void **state) +{ + (void) state; /* unused */ + assert_int_equal(16, address_to_bits(0xAA55)); +} + +void bitcount_test_success(void **state) +{ + (void) state; /* unused */ + assert_int_equal(4, bitcount(0xAA)); +} + +void minmax_test_success(void **state) +{ + (void) state; /* unused */ + assert_int_equal(0x55, min(0xAA, 0x55)); + assert_int_equal(0xAA, max(0xAA, 0x55)); +} + +void strcat_realloc_test_success(void **state) +{ + (void) state; /* unused */ + const char src0[] = "hello"; + const char src1[] = " world"; + char *dest = calloc(1, 1); + dest = strcat_realloc(dest, src0); + dest = strcat_realloc(dest, src1); + assert_string_equal("hello world", dest); + free(dest); +} + +void tolower_string_test_success(void **state) +{ + (void) state; /* unused */ + char str[] = "HELLO AGAIN"; + assert_string_equal("HELLO AGAIN", str); + tolower_string(str); + assert_string_equal("hello again", str); +} + +void reverse_byte_test_success(void **state) +{ + (void) state; /* unused */ + assert_int_equal(0x5A, reverse_byte(0x5A)); + assert_int_equal(0x0F, reverse_byte(0xF0)); +} + +void reverse_bytes_test_success(void **state) +{ + (void) state; /* unused */ + uint8_t src[] = { 0xAA, 0x55 }; + uint8_t dst[2]; + reverse_bytes(dst, src, 2); + assert_int_equal(src[0], dst[1]); + assert_int_equal(src[1], dst[0]); +} diff --git a/tests/meson.build b/tests/meson.build index aad1372..3cd67b8 100644 --- a/tests/meson.build +++ b/tests/meson.build @@ -2,6 +2,7 @@ srcs = [ 'tests.c', + 'helpers.c', 'spi.c', 'flashrom.c', 'spi25.c', diff --git a/tests/tests.c b/tests/tests.c index a3ad791..2ba293b 100644 --- a/tests/tests.c +++ b/tests/tests.c @@ -18,6 +18,17 @@ { int ret = 0; + const struct CMUnitTest helpers_tests[] = { + cmocka_unit_test(address_to_bits_test_success), + cmocka_unit_test(bitcount_test_success), + cmocka_unit_test(minmax_test_success), + cmocka_unit_test(strcat_realloc_test_success), + cmocka_unit_test(tolower_string_test_success), + cmocka_unit_test(reverse_byte_test_success), + cmocka_unit_test(reverse_bytes_test_success), + }; + ret |= cmocka_run_group_tests_name("helpers.c tests", helpers_tests, NULL, NULL); + const struct CMUnitTest spi_tests[] = { cmocka_unit_test(registered_masters_test_success), cmocka_unit_test(programmer_init_test_success), diff --git a/tests/tests.h b/tests/tests.h index 688c280..b4913f1 100644 --- a/tests/tests.h +++ b/tests/tests.h @@ -1,6 +1,15 @@ #ifndef TESTS_H #define TESTS_H +/* helpers.c */ +void address_to_bits_test_success(void **state); +void bitcount_test_success(void **state); +void minmax_test_success(void **state); +void strcat_realloc_test_success(void **state); +void tolower_string_test_success(void **state); +void reverse_byte_test_success(void **state); +void reverse_bytes_test_success(void **state); + void registered_masters_test_success(void **state); void programmer_init_test_success(void **state); -- To view, visit https://review.coreboot.org/c/flashrom/+/41655 To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings Gerrit-Project: flashrom Gerrit-Branch: master Gerrit-Change-Id: If4a1fe7c499f51bb9d7cd48ef26caf9dfae3c1fa Gerrit-Change-Number: 41655 Gerrit-PatchSet: 1 Gerrit-Owner: Edward O'Callaghan <quasisec(a)chromium.org> Gerrit-MessageType: newchange

2 6

Change in flashrom[master]: tests/: Add flashrom.c unit tests
by Edward O'Callaghan (Code Review) 17 Jun '20

17 Jun '20

Edward O'Callaghan has uploaded this change for review. ( https://review.coreboot.org/c/flashrom/+/41646 ) Change subject: tests/: Add flashrom.c unit tests ...................................................................... tests/: Add flashrom.c unit tests Change-Id: I2d9213f98c6c9639f2417466ba4895117e8d600a Signed-off-by: Edward O'Callaghan <quasisec(a)google.com> --- A tests/flashrom.c M tests/meson.build M tests/tests.c M tests/tests.h 4 files changed, 45 insertions(+), 0 deletions(-) git pull ssh://review.coreboot.org:29418/flashrom refs/changes/46/41646/1 diff --git a/tests/flashrom.c b/tests/flashrom.c new file mode 100644 index 0000000..ad46dd8 --- /dev/null +++ b/tests/flashrom.c @@ -0,0 +1,36 @@ +#include <include/test.h> + +#include "programmer.h" + +void flashbuses_to_text_test_success(void **state) +{ + (void) state; /* unused */ + + enum chipbustype bustype; + + bustype = BUS_NONSPI; + assert_string_equal(flashbuses_to_text(bustype), "Non-SPI"); + + bustype |= BUS_PARALLEL; + assert_string_not_equal(flashbuses_to_text(bustype), "Non-SPI, Parallel"); + + bustype = BUS_PARALLEL; + bustype |= BUS_LPC; + assert_string_equal(flashbuses_to_text(bustype), "Parallel, LPC"); + + bustype |= BUS_FWH; + //BUS_NONSPI = BUS_PARALLEL | BUS_LPC | BUS_FWH, + assert_string_equal(flashbuses_to_text(bustype), "Non-SPI"); + + bustype |= BUS_SPI; + assert_string_equal(flashbuses_to_text(bustype), "Parallel, LPC, FWH, SPI"); + + bustype |= BUS_PROG; + assert_string_equal( + flashbuses_to_text(bustype), + "Parallel, LPC, FWH, SPI, Programmer-specific" + ); + + bustype = BUS_NONE; + assert_string_equal(flashbuses_to_text(bustype), "None"); +} diff --git a/tests/meson.build b/tests/meson.build index 3ed1c3b..aad1372 100644 --- a/tests/meson.build +++ b/tests/meson.build @@ -3,6 +3,7 @@ srcs = [ 'tests.c', 'spi.c', + 'flashrom.c', 'spi25.c', ] diff --git a/tests/tests.c b/tests/tests.c index 16f4863..a3ad791 100644 --- a/tests/tests.c +++ b/tests/tests.c @@ -24,6 +24,11 @@ }; ret |= cmocka_run_group_tests_name("spi.c tests", spi_tests, NULL, NULL); + const struct CMUnitTest flashrom_tests[] = { + cmocka_unit_test(flashbuses_to_text_test_success), + }; + ret |= cmocka_run_group_tests_name("flashrom.c tests", flashrom_tests, NULL, NULL); + const struct CMUnitTest spi25_tests[] = { cmocka_unit_test(spi_write_enable_test_success), cmocka_unit_test(spi_write_disable_test_success), diff --git a/tests/tests.h b/tests/tests.h index 9377817..688c280 100644 --- a/tests/tests.h +++ b/tests/tests.h @@ -4,6 +4,9 @@ void registered_masters_test_success(void **state); void programmer_init_test_success(void **state); +/* flashrom.c */ +void flashbuses_to_text_test_success(void **state); + /* spi25.c */ void spi_write_enable_test_success(void **state); void spi_write_disable_test_success(void **state); -- To view, visit https://review.coreboot.org/c/flashrom/+/41646 To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings Gerrit-Project: flashrom Gerrit-Branch: master Gerrit-Change-Id: I2d9213f98c6c9639f2417466ba4895117e8d600a Gerrit-Change-Number: 41646 Gerrit-PatchSet: 1 Gerrit-Owner: Edward O'Callaghan <quasisec(a)chromium.org> Gerrit-MessageType: newchange

2 6

Change in flashrom[master]: flashchips: Added W25Q256.W
by Simon Buhrow (Code Review) 16 Jun '20

16 Jun '20

Simon Buhrow has uploaded this change for review. ( https://review.coreboot.org/c/flashrom/+/40855 ) Change subject: flashchips: Added W25Q256.W ...................................................................... flashchips: Added W25Q256.W Change-Id: I3608945a38b5ed42b8b2b9b2b388cdea97141d7b --- M flashchips.c M flashchips.h 2 files changed, 42 insertions(+), 0 deletions(-) git pull ssh://review.coreboot.org:29418/flashrom refs/changes/55/40855/1 diff --git a/flashchips.c b/flashchips.c index 974ba81..f2bd68b 100644 --- a/flashchips.c +++ b/flashchips.c @@ -16626,6 +16626,47 @@ { .vendor = "Winbond", + .name = "W25Q256.W", + .bustype = BUS_SPI, + .manufacture_id = WINBOND_NEX_ID, + .model_id = WINBOND_NEX_W25Q256_W, + .total_size = 32768, + .page_size = 256, + /* supports SFDP */ + /* OTP: 1024B total, 256B reserved; read 0x48; write 0x42, erase 0x44, read ID 0x4B */ + .feature_bits = FEATURE_WRSR_WREN | FEATURE_OTP | FEATURE_4BA_ENTER_WREN + | FEATURE_4BA_EXT_ADDR | FEATURE_4BA_READ | FEATURE_4BA_FAST_READ, + .tested = TEST_OK_PREW, + .probe = probe_spi_rdid, + .probe_timing = TIMING_ZERO, + .block_erasers = + { + { + .eraseblocks = { {4 * 1024, 8192} }, + .block_erase = spi_block_erase_20, + }, { + .eraseblocks = { {32 * 1024, 1024} }, + .block_erase = spi_block_erase_52, + }, { + .eraseblocks = { {64 * 1024, 512} }, + .block_erase = spi_block_erase_d8, + }, { + .eraseblocks = { {32 * 1024 * 1024, 1} }, + .block_erase = spi_block_erase_60, + }, { + .eraseblocks = { {32 * 1024 * 1024, 1} }, + .block_erase = spi_block_erase_c7, + } + }, + .printlock = spi_prettyprint_status_register_plain, /* TODO: improve */ + .unlock = spi_disable_blockprotect, + .write = spi_chip_write_256, + .read = spi_chip_read, + .voltage = {1650, 1950}, + }, + + { + .vendor = "Winbond", .name = "W25Q128.JW.DTR", .bustype = BUS_SPI, .manufacture_id = WINBOND_NEX_ID, diff --git a/flashchips.h b/flashchips.h index 0c77d1d..3332974 100644 --- a/flashchips.h +++ b/flashchips.h @@ -943,6 +943,7 @@ #define WINBOND_NEX_W25Q32_W 0x6016 /* W25Q32DW; W25Q32FV in QPI mode */ #define WINBOND_NEX_W25Q64_W 0x6017 /* W25Q64DW; W25Q64FV in QPI mode */ #define WINBOND_NEX_W25Q128_W 0x6018 /* W25Q128FW; W25Q128FV in QPI mode */ +#define WINBOND_NEX_W25Q256_W 0x6019 /* W25Q256JW */ #define WINBOND_NEX_W25Q128_V_M 0x7018 /* W25Q128JVSM */ #define WINBOND_NEX_W25Q256JV_M 0x7019 /* W25Q256JV_M (QE=0) */ #define WINBOND_NEX_W25Q64JW 0x8017 -- To view, visit https://review.coreboot.org/c/flashrom/+/40855 To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings Gerrit-Project: flashrom Gerrit-Branch: master Gerrit-Change-Id: I3608945a38b5ed42b8b2b9b2b388cdea97141d7b Gerrit-Change-Number: 40855 Gerrit-PatchSet: 1 Gerrit-Owner: Simon Buhrow Gerrit-MessageType: newchange

5 20

Change in flashrom[master]: tests/: Add spi25.c unit tests
by Edward O'Callaghan (Code Review) 16 Jun '20

16 Jun '20

Edward O'Callaghan has uploaded this change for review. ( https://review.coreboot.org/c/flashrom/+/41645 ) Change subject: tests/: Add spi25.c unit tests ...................................................................... tests/: Add spi25.c unit tests Change-Id: I47112952835ce2c4c773a9d90379ff8ceefaaf9a Signed-off-by: Edward O'Callaghan <quasisec(a)google.com> --- M tests/meson.build A tests/spi25.c M tests/tests.c M tests/tests.h 4 files changed, 196 insertions(+), 0 deletions(-) git pull ssh://review.coreboot.org:29418/flashrom refs/changes/45/41645/1 diff --git a/tests/meson.build b/tests/meson.build index 5af0a3e..3ed1c3b 100644 --- a/tests/meson.build +++ b/tests/meson.build @@ -3,11 +3,13 @@ srcs = [ 'tests.c', 'spi.c', + 'spi25.c', ] mocks = [ '-Wl,--wrap=physunmap', '-Wl,--wrap=physmap', + '-Wl,--wrap=spi_send_command', '-Wl,--gc-sections', ] diff --git a/tests/spi25.c b/tests/spi25.c new file mode 100644 index 0000000..cf8345c --- /dev/null +++ b/tests/spi25.c @@ -0,0 +1,170 @@ +#include <include/test.h> + +#include "programmer.h" +#include "flashchips.h" +#include "chipdrivers.h" +#include "spi.h" + +int __wrap_spi_send_command(const struct flashctx *flash, + unsigned int writecnt, unsigned int readcnt, + const unsigned char *writearr, unsigned char *readarr) +{ + check_expected_ptr(flash); + assert_int_equal(writecnt, mock_type(int)); + assert_int_equal(writearr[0], mock_type(int)); + + int rcnt = mock_type(int); + assert_int_equal(readcnt, rcnt); + for (int i = 0; i < rcnt; i++) + readarr[i] = i; + + return 0; +} + +struct flashchip mock_chip = { + .vendor = "Generic", + .name = "unknown SPI chip (RDID)", + .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_rdid, + .write = NULL, +}; + +void spi_write_enable_test_success(void **state) +{ + (void) state; /* unused */ + + /* setup initial test state. */ + struct flashctx flashctx = { .chip = &mock_chip }; + expect_memory(__wrap_spi_send_command, flash, + &flashctx, sizeof(flashctx)); + + will_return(__wrap_spi_send_command, JEDEC_WREN_OUTSIZE); + will_return(__wrap_spi_send_command, JEDEC_WREN); + will_return(__wrap_spi_send_command, JEDEC_WREN_INSIZE); + assert_int_equal(0, spi_write_enable(&flashctx)); +} + +void spi_write_disable_test_success(void **state) +{ + (void) state; /* unused */ + + /* setup initial test state. */ + struct flashctx flashctx = { .chip = &mock_chip }; + expect_memory(__wrap_spi_send_command, flash, + &flashctx, sizeof(flashctx)); + + will_return(__wrap_spi_send_command, JEDEC_WRDI_OUTSIZE); + will_return(__wrap_spi_send_command, JEDEC_WRDI); + will_return(__wrap_spi_send_command, JEDEC_WRDI_INSIZE); + assert_int_equal(0, spi_write_disable(&flashctx)); +} + +void probe_spi_rdid_test_success(void **state) +{ + (void) state; /* unused */ + + /* setup initial test state. */ + struct flashctx flashctx = { .chip = &mock_chip }; + expect_memory(__wrap_spi_send_command, flash, + &flashctx, sizeof(flashctx)); + + will_return(__wrap_spi_send_command, JEDEC_RDID_OUTSIZE); + will_return(__wrap_spi_send_command, JEDEC_RDID); + will_return(__wrap_spi_send_command, JEDEC_RDID_INSIZE); + assert_int_equal(0, probe_spi_rdid(&flashctx)); +} + +void probe_spi_rdid4_test_success(void **state) +{ + (void) state; /* unused */ + + /* setup initial test state. */ + struct flashctx flashctx = { .chip = &mock_chip }; + expect_memory(__wrap_spi_send_command, flash, + &flashctx, sizeof(flashctx)); + + will_return(__wrap_spi_send_command, JEDEC_RDID_OUTSIZE); + will_return(__wrap_spi_send_command, JEDEC_RDID); + will_return(__wrap_spi_send_command, JEDEC_RDID_INSIZE + 1); + assert_int_equal(0, probe_spi_rdid4(&flashctx)); +} + +void probe_spi_rems_test_success(void **state) +{ + (void) state; /* unused */ + + /* setup initial test state. */ + struct flashctx flashctx = { .chip = &mock_chip }; + expect_memory(__wrap_spi_send_command, flash, + &flashctx, sizeof(flashctx)); + + will_return(__wrap_spi_send_command, JEDEC_REMS_OUTSIZE); + will_return(__wrap_spi_send_command, JEDEC_REMS); + will_return(__wrap_spi_send_command, JEDEC_REMS_INSIZE); + assert_int_equal(0, probe_spi_rems(&flashctx)); +} + +void probe_spi_res1_test_success(void **state) +{ + (void) state; /* unused */ + + /* setup initial test state. */ + struct flashctx flashctx = { .chip = &mock_chip }; + expect_memory(__wrap_spi_send_command, flash, + &flashctx, sizeof(flashctx)); + + will_return(__wrap_spi_send_command, JEDEC_RES_OUTSIZE); + will_return(__wrap_spi_send_command, JEDEC_RES); + will_return(__wrap_spi_send_command, JEDEC_RES_INSIZE + 1); + assert_int_equal(0, probe_spi_res2(&flashctx)); +} + +void probe_spi_res2_test_success(void **state) +{ + (void) state; /* unused */ + + /* setup initial test state. */ + struct flashctx flashctx = { .chip = &mock_chip }; + expect_memory(__wrap_spi_send_command, flash, + &flashctx, sizeof(flashctx)); + + will_return(__wrap_spi_send_command, JEDEC_RES_OUTSIZE); + will_return(__wrap_spi_send_command, JEDEC_RES); + will_return(__wrap_spi_send_command, JEDEC_RES_INSIZE + 1); + assert_int_equal(0, probe_spi_res2(&flashctx)); +} + +void probe_spi_res3_test_success(void **state) +{ + (void) state; /* unused */ + + /* setup initial test state. */ + struct flashctx flashctx = { .chip = &mock_chip }; + expect_memory(__wrap_spi_send_command, flash, + &flashctx, sizeof(flashctx)); + + will_return(__wrap_spi_send_command, JEDEC_RES_OUTSIZE); + will_return(__wrap_spi_send_command, JEDEC_RES); + will_return(__wrap_spi_send_command, JEDEC_RES_INSIZE + 2); + assert_int_equal(0, probe_spi_res3(&flashctx)); +} + +void probe_spi_at25f_test_success(void **state) +{ + (void) state; /* unused */ + + /* setup initial test state. */ + struct flashctx flashctx = { .chip = &mock_chip }; + expect_memory(__wrap_spi_send_command, flash, + &flashctx, sizeof(flashctx)); + + will_return(__wrap_spi_send_command, AT25F_RDID_OUTSIZE); + will_return(__wrap_spi_send_command, AT25F_RDID); + will_return(__wrap_spi_send_command, AT25F_RDID_INSIZE); + assert_int_equal(0, probe_spi_at25f(&flashctx)); +} diff --git a/tests/tests.c b/tests/tests.c index 338646e..16f4863 100644 --- a/tests/tests.c +++ b/tests/tests.c @@ -24,5 +24,18 @@ }; ret |= cmocka_run_group_tests_name("spi.c tests", spi_tests, NULL, NULL); + const struct CMUnitTest spi25_tests[] = { + cmocka_unit_test(spi_write_enable_test_success), + cmocka_unit_test(spi_write_disable_test_success), + cmocka_unit_test(probe_spi_rdid_test_success), + cmocka_unit_test(probe_spi_rdid4_test_success), + cmocka_unit_test(probe_spi_rems_test_success), + cmocka_unit_test(probe_spi_res1_test_success), + cmocka_unit_test(probe_spi_res2_test_success), + cmocka_unit_test(probe_spi_res3_test_success), + cmocka_unit_test(probe_spi_at25f_test_success), + }; + ret |= cmocka_run_group_tests_name("spi25.c tests", spi25_tests, NULL, NULL); + return ret; } diff --git a/tests/tests.h b/tests/tests.h index ccc3f5e..9377817 100644 --- a/tests/tests.h +++ b/tests/tests.h @@ -4,4 +4,15 @@ void registered_masters_test_success(void **state); void programmer_init_test_success(void **state); +/* spi25.c */ +void spi_write_enable_test_success(void **state); +void spi_write_disable_test_success(void **state); +void probe_spi_rdid_test_success(void **state); +void probe_spi_rdid4_test_success(void **state); +void probe_spi_rems_test_success(void **state); +void probe_spi_res1_test_success(void **state); +void probe_spi_res2_test_success(void **state); +void probe_spi_res3_test_success(void **state); +void probe_spi_at25f_test_success(void **state); + #endif /* TESTS_H */ -- To view, visit https://review.coreboot.org/c/flashrom/+/41645 To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings Gerrit-Project: flashrom Gerrit-Branch: master Gerrit-Change-Id: I47112952835ce2c4c773a9d90379ff8ceefaaf9a Gerrit-Change-Number: 41645 Gerrit-PatchSet: 1 Gerrit-Owner: Edward O'Callaghan <quasisec(a)chromium.org> Gerrit-MessageType: newchange

3 6

Change in flashrom[master]: [WIP]: CMocka unit-test suite
by Edward O'Callaghan (Code Review) 16 Jun '20

16 Jun '20

Edward O'Callaghan has uploaded this change for review. ( https://review.coreboot.org/c/flashrom/+/41622 ) Change subject: [WIP]: CMocka unit-test suite ...................................................................... [WIP]: CMocka unit-test suite Change-Id: I66665f56627b3d99049176bfbebbd771b080370a Signed-off-by: Edward O'Callaghan <quasisec(a)google.com> --- M meson.build A subprojects/cmocka.wrap A tests/include/test.h A tests/meson.build A tests/spi_test.c 5 files changed, 147 insertions(+), 0 deletions(-) git pull ssh://review.coreboot.org:29418/flashrom refs/changes/22/41622/1 diff --git a/meson.build b/meson.build index 5d8e630..3d395e8 100644 --- a/meson.build +++ b/meson.build @@ -424,3 +424,23 @@ ) subdir('util') + +# unit-test framework +cmocka_dep = dependency( + 'cmocka', + fallback: ['cmocka', 'cmocka_dep'] +) +flashrom_test_dep = declare_dependency( + include_directories : include_directories('.'), + sources : [ + srcs, + 'cli_common.c', + 'cli_output.c', + 'flashrom.c', + ], + dependencies : [ + deps, + ], +) + +subdir('tests') diff --git a/subprojects/cmocka.wrap b/subprojects/cmocka.wrap new file mode 100644 index 0000000..21e84f9 --- /dev/null +++ b/subprojects/cmocka.wrap @@ -0,0 +1,10 @@ +[wrap-file] +directory = cmocka-1.1.5 + +source_url = https://cmocka.org/files/1.1/cmocka-1.1.5.tar.xz +source_filename = cmocka-1.1.5.tar.xz +source_hash = f0ccd8242d55e2fd74b16ba518359151f6f8383ff8aef4976e48393f77bba8b6 + +patch_url = https://wrapdb.mesonbuild.com/v1/projects/cmocka/1.1.5/3/get_zip +patch_filename = cmocka-1.1.5-3-wrap.zip +patch_hash = 81ce48613680d3c3a0b396ac570c852b290adcd18202fb16aaf703a9493f4348 diff --git a/tests/include/test.h b/tests/include/test.h new file mode 100644 index 0000000..b4e0dd2 --- /dev/null +++ b/tests/include/test.h @@ -0,0 +1,18 @@ +/* SPDX-License-Identifier: GPL-2.0-only */ +/* This file is part of the coreboot project. */ + +#ifndef _TESTS_TEST_H +#define _TESTS_TEST_H + +/* + * Standard test header that should be included in all tests. For now it just encapsulates the + * include dependencies for Cmocka. Test-specific APIs that are so generic we would want them + * available everywhere could also be added here. + */ + +#include <stdarg.h> +#include <stddef.h> +#include <setjmp.h> +#include <cmocka.h> + +#endif /* _TESTS_TEST_H */ diff --git a/tests/meson.build b/tests/meson.build new file mode 100644 index 0000000..6d9f8c2 --- /dev/null +++ b/tests/meson.build @@ -0,0 +1,26 @@ +root_includes = include_directories('../subprojects') + +srcs = files('spi_test.c') + +mocks = [ + '-Wl,--wrap=physunmap', + '-Wl,--wrap=physmap', + '-Wl,--gc-sections', +] + +flashrom_tests = executable('flashrom_unit_tests', + srcs, + include_directories : root_includes, + c_args : [ + cargs, + '-ffunction-sections', + '-fdata-sections', + # '-DSTANDALONE', + '-DCONFIG_DEFAULT_PROGRAMMER=PROGRAMMER_DUMMY', + '-DCONFIG_DEFAULT_PROGRAMMER_ARGS=""', + ], + export_dynamic : true, + link_args : mocks, + dependencies : [cmocka_dep, flashrom_test_dep], +) +test('cmocka test flashrom', flashrom_tests) diff --git a/tests/spi_test.c b/tests/spi_test.c new file mode 100644 index 0000000..0a0453d --- /dev/null +++ b/tests/spi_test.c @@ -0,0 +1,73 @@ +#include <include/test.h> + +#include "programmer.h" + +#include <string.h> + + +static int mock_spi_send_command(const struct flashctx *flash, + unsigned int write_count, + unsigned int read_count, + const unsigned char *write_buffer, + unsigned char *read_buffer) +{ + return 0xAA55; +} + +static void registered_masters_test_success(void **state) +{ + (void) state; /* unused */ + + static struct spi_master mst = { + .max_data_read = 16, + .max_data_write = 8, + .command = mock_spi_send_command, + .multicommand = default_spi_send_multicommand, + .read = default_spi_read, + .write_256 = default_spi_write_256, + .write_aai = default_spi_write_aai, + }; + assert_int_not_equal(register_spi_master(&mst), ERROR_FLASHROM_BUG); + + struct spi_master *master = &registered_masters[0].spi; + assert_int_equal(master->max_data_read, 16); + assert_int_equal(master->max_data_write, 8); + assert_int_equal(master->command(NULL,0,0,NULL,NULL), 0xAA55); +} + +/* redefinitions/wrapping */ +void __wrap_physunmap(void *virt_addr, size_t len) +{ + fprintf(stderr, "%s\n", __func__); +} +void *__wrap_physmap(const char *descr, uintptr_t phys_addr, size_t len) +{ + fprintf(stderr, "%s\n", __func__); + return NULL; +} + +static void programmer_init_test_success(void **state) +{ + (void) state; /* unused */ + + assert_int_equal( + programmer_init( + CONFIG_DEFAULT_PROGRAMMER, + strdup(CONFIG_DEFAULT_PROGRAMMER_ARGS) + ), 0 + ); + + struct flashrom_flashctx flashctx = { 0, }; + int ret = probe_flash(&registered_masters[0], 0, &flashctx, 0); + assert_int_not_equal(ret, -1); +} + +int main(void) +{ + const struct CMUnitTest tests[] = { + cmocka_unit_test(registered_masters_test_success), + cmocka_unit_test(programmer_init_test_success), + }; + + return cmocka_run_group_tests_name("spi_master tests", tests, NULL, NULL); +} -- To view, visit https://review.coreboot.org/c/flashrom/+/41622 To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings Gerrit-Project: flashrom Gerrit-Branch: master Gerrit-Change-Id: I66665f56627b3d99049176bfbebbd771b080370a Gerrit-Change-Number: 41622 Gerrit-PatchSet: 1 Gerrit-Owner: Edward O'Callaghan <quasisec(a)chromium.org> Gerrit-MessageType: newchange

3 9

Change in flashrom[master]: libflashrom.c: Fix indentation of a brace
by Angel Pons (Code Review) 28 May '20

28 May '20

Angel Pons has uploaded this change for review. ( https://review.coreboot.org/c/flashrom/+/41776 ) Change subject: libflashrom.c: Fix indentation of a brace ...................................................................... libflashrom.c: Fix indentation of a brace It was indented with two spaces instead of one tab. Fix it. Change-Id: I18051ae4433b267b9552a034a67d7830b9206c20 Signed-off-by: Angel Pons <th3fanbus(a)gmail.com> --- M libflashrom.c 1 file changed, 1 insertion(+), 1 deletion(-) git pull ssh://review.coreboot.org:29418/flashrom refs/changes/76/41776/1 diff --git a/libflashrom.c b/libflashrom.c index c20d9c7..ae2d33d 100644 --- a/libflashrom.c +++ b/libflashrom.c @@ -229,7 +229,7 @@ supported_chipsets[i].chipset_id = chipset[i].device_id; supported_chipsets[i].status = (enum flashrom_test_state) chipset[i].status; - } + } } else { msg_gerr("Memory allocation error!\n"); } -- To view, visit https://review.coreboot.org/c/flashrom/+/41776 To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings Gerrit-Project: flashrom Gerrit-Branch: master Gerrit-Change-Id: I18051ae4433b267b9552a034a67d7830b9206c20 Gerrit-Change-Number: 41776 Gerrit-PatchSet: 1 Gerrit-Owner: Angel Pons <th3fanbus(a)gmail.com> Gerrit-MessageType: newchange

4 4

Change in flashrom[master]: libflashrom.c: Use casts on enum conversions
by Angel Pons (Code Review) 28 May '20

28 May '20

Angel Pons has uploaded this change for review. ( https://review.coreboot.org/c/flashrom/+/41775 ) Change subject: libflashrom.c: Use casts on enum conversions ...................................................................... libflashrom.c: Use casts on enum conversions This allows flashrom to build with GCC 10. Change-Id: I2166cdf3681452631ef8e980face2924e9a6c81a Signed-off-by: Angel Pons <th3fanbus(a)gmail.com> --- M libflashrom.c 1 file changed, 4 insertions(+), 2 deletions(-) git pull ssh://review.coreboot.org:29418/flashrom refs/changes/75/41775/1 diff --git a/libflashrom.c b/libflashrom.c index ab7e364..c20d9c7 100644 --- a/libflashrom.c +++ b/libflashrom.c @@ -188,7 +188,8 @@ for (; i < boards_known_size; ++i) { supported_boards[i].vendor = binfo[i].vendor; supported_boards[i].name = binfo[i].name; - supported_boards[i].working = binfo[i].working; + supported_boards[i].working = + (enum flashrom_test_state) binfo[i].working; } } else { msg_gerr("Memory allocation error!\n"); @@ -226,7 +227,8 @@ supported_chipsets[i].chipset = chipset[i].device_name; supported_chipsets[i].vendor_id = chipset[i].vendor_id; supported_chipsets[i].chipset_id = chipset[i].device_id; - supported_chipsets[i].status = chipset[i].status; + supported_chipsets[i].status = + (enum flashrom_test_state) chipset[i].status; } } else { msg_gerr("Memory allocation error!\n"); -- To view, visit https://review.coreboot.org/c/flashrom/+/41775 To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings Gerrit-Project: flashrom Gerrit-Branch: master Gerrit-Change-Id: I2166cdf3681452631ef8e980face2924e9a6c81a Gerrit-Change-Number: 41775 Gerrit-PatchSet: 1 Gerrit-Owner: Angel Pons <th3fanbus(a)gmail.com> Gerrit-MessageType: newchange

2 2

Change in flashrom[master]: Fix a handful of compiler warnings
by Ryan O'Leary (Code Review) 27 May '20

27 May '20

Ryan O'Leary has uploaded this change for review. ( https://review.coreboot.org/c/flashrom/+/36946 ) Change subject: Fix a handful of compiler warnings ...................................................................... Fix a handful of compiler warnings These warnings (which I believe are not enabled by default) were generated by -Wformat-security and -Wenum-conversion. Change-Id: I3d01f4ff620f4c4111dd675d49ce3254f2360b29 Signed-off-by: Ryan O'Leary <ryanoleary(a)google.com> --- M libflashrom.c M print.c M programmer.h 3 files changed, 7 insertions(+), 5 deletions(-) git pull ssh://review.coreboot.org:29418/flashrom refs/changes/46/36946/1 diff --git a/libflashrom.c b/libflashrom.c index 0dec22e..b15b5f6 100644 --- a/libflashrom.c +++ b/libflashrom.c @@ -188,7 +188,8 @@ for (; i < boards_known_size; ++i) { supported_boards[i].vendor = binfo[i].vendor; supported_boards[i].name = binfo[i].name; - supported_boards[i].working = binfo[i].working; + supported_boards[i].working = + (enum flashrom_test_state)binfo[i].working; } } else { msg_gerr("Memory allocation error!\n"); @@ -226,8 +227,9 @@ supported_chipsets[i].chipset = chipset[i].device_name; supported_chipsets[i].vendor_id = chipset[i].vendor_id; supported_chipsets[i].chipset_id = chipset[i].device_id; - supported_chipsets[i].status = chipset[i].status; - } + supported_chipsets[i].status = + (enum flashrom_test_state)chipset[i].status; + } } else { msg_gerr("Memory allocation error!\n"); } diff --git a/print.c b/print.c index 596fc53..173c7da 100644 --- a/print.c +++ b/print.c @@ -401,7 +401,7 @@ for (i = 0; i < maxboardlen - strlen(b->name); i++) msg_ginfo(" "); - msg_pinfo(test_state_to_text(b->working)); + msg_pinfo("%s", test_state_to_text(b->working)); for (e = board_matches; e->vendor_name != NULL; e++) { if (strcmp(e->vendor_name, b->vendor) diff --git a/programmer.h b/programmer.h index 5a21b2e..96add05 100644 --- a/programmer.h +++ b/programmer.h @@ -307,7 +307,7 @@ /* cbtable.c */ int cb_parse_table(const char **vendor, const char **model); -int cb_check_image(const uint8_t *bios, unsigned int size); +int cb_check_image(const uint8_t *image, unsigned int size); /* dmi.c */ #if defined(__i386__) || defined(__x86_64__) -- To view, visit https://review.coreboot.org/c/flashrom/+/36946 To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings Gerrit-Project: flashrom Gerrit-Branch: master Gerrit-Change-Id: I3d01f4ff620f4c4111dd675d49ce3254f2360b29 Gerrit-Change-Number: 36946 Gerrit-PatchSet: 1 Gerrit-Owner: Ryan O'Leary <ryanoleary(a)google.com> Gerrit-MessageType: newchange

7 19

2024

2023

2022

2021

2020

2019

2018

2017

flashrom-gerrit May 2020