August 2021 - flashrom-gerrit - mail.coreboot.org

Change in flashrom[master]: tests: Mock file i/o for linux_mtd and linux_spi tests
by Angel Pons (Code Review) 11 Aug '21

11 Aug '21

Attention is currently required from: Nico Huber, Edward O'Callaghan, Anastasia Klimchuk, Peter Marheine. Angel Pons has posted comments on this change. ( https://review.coreboot.org/c/flashrom/+/56413 ) Change subject: tests: Mock file i/o for linux_mtd and linux_spi tests ...................................................................... Patch Set 5: (3 comments) File tests/init_shutdown.c: https://review.coreboot.org/c/flashrom/+/56413/comment/0dbb5e43_d3c4e469 PS4, Line 214: int num_bytes = 0; Hmmm, I'd like to get rid of `num_bytes`: size_t linux_mtd_fread(void *state, void *buf, size_t size, size_t len, FILE *fp) { struct linux_mtd_fread_mock_entry { const char *path; const char *data; }; const struct linux_mtd_fread_mock_entry fread_mock_map[] = { { "/sys/class/mtd/mtd0//type", "nor" }, { "/sys/class/mtd/mtd0//name", "Device" }, { "/sys/class/mtd/mtd0//flags", "" }, { "/sys/class/mtd/mtd0//size", "1024" }, { "/sys/class/mtd/mtd0//erasesize", "512" }, { "/sys/class/mtd/mtd0//numeraseregions", "0" }, }; struct linux_mtd_io_state *io_state = state; const char *data = NULL; unsigned int i; if (!io_state->fopen_path) return 0; for (i = 0; i < ARRAY_SIZE(fread_mock_map); i++) { const struct linux_mtd_fread_mock_entry *entry = fread_mock_map[i]; if (!strcmp(io_state->fopen_path, entry->path)) { strcpy(buf, entry->data); return strlen(entry->data); } } return 0; } https://review.coreboot.org/c/flashrom/+/56413/comment/22613ca1_811fe928 PS4, Line 221: memcpy(buf, "nor", num_bytes); Use strcpy instead? https://review.coreboot.org/c/flashrom/+/56413/comment/26fc4e9b_ce63184e PS4, Line 275: memcpy(buf, "1048576", len); > just, `return memcpy(buf, "1048576", len);` memcpy returns the ptr to dest (buf in this case). Hmmm, why use `len` here? If its value is larger than the length of "1048576", undefined behavior would happen. -- To view, visit https://review.coreboot.org/c/flashrom/+/56413 To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings Gerrit-Project: flashrom Gerrit-Branch: master Gerrit-Change-Id: I73f0d6ff2ad5074add7a721ed3416230d3647e3f Gerrit-Change-Number: 56413 Gerrit-PatchSet: 5 Gerrit-Owner: Anastasia Klimchuk <aklm(a)chromium.org> Gerrit-Reviewer: Angel Pons <th3fanbus(a)gmail.com> Gerrit-Reviewer: Edward O'Callaghan <quasisec(a)chromium.org> Gerrit-Reviewer: Nico Huber <nico.h(a)gmx.de> Gerrit-Reviewer: build bot (Jenkins) <no-reply(a)coreboot.org> Gerrit-CC: Paul Menzel <paulepanter(a)mailbox.org> Gerrit-Attention: Nico Huber <nico.h(a)gmx.de> Gerrit-Attention: Edward O'Callaghan <quasisec(a)chromium.org> Gerrit-Attention: Anastasia Klimchuk <aklm(a)chromium.org> Gerrit-Attention: Peter Marheine <pmarheine(a)chromium.org> Gerrit-Comment-Date: Wed, 11 Aug 2021 08:59:25 +0000 Gerrit-HasComments: Yes Gerrit-Has-Labels: No Comment-In-Reply-To: Edward O'Callaghan <quasisec(a)chromium.org> Gerrit-MessageType: comment

1 0

Change in flashrom[master]: tests: add init_shutdown test for realtek_mst_i2c_spi
by Peter Marheine (Code Review) 11 Aug '21

11 Aug '21

Attention is currently required from: Anastasia Klimchuk. Peter Marheine has posted comments on this change. ( https://review.coreboot.org/c/flashrom/+/56911 ) Change subject: tests: add init_shutdown test for realtek_mst_i2c_spi ...................................................................... Patch Set 1: (1 comment) Patchset: PS1: Not much complexity in the actual mocks here, but it does need to add support for read(), write() and ioctl(). -- To view, visit https://review.coreboot.org/c/flashrom/+/56911 To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings Gerrit-Project: flashrom Gerrit-Branch: master Gerrit-Change-Id: I5a5c617d1ec35d2a3bbe622e5add82a65eb396f0 Gerrit-Change-Number: 56911 Gerrit-PatchSet: 1 Gerrit-Owner: Peter Marheine <pmarheine(a)chromium.org> Gerrit-Reviewer: Anastasia Klimchuk <aklm(a)chromium.org> Gerrit-Reviewer: build bot (Jenkins) <no-reply(a)coreboot.org> Gerrit-Attention: Anastasia Klimchuk <aklm(a)chromium.org> Gerrit-Comment-Date: Wed, 11 Aug 2021 06:23:35 +0000 Gerrit-HasComments: Yes Gerrit-Has-Labels: No Gerrit-MessageType: comment

1 0

Change in flashrom[master]: tests: add init_shutdown test for realtek_mst_i2c_spi
by Peter Marheine (Code Review) 11 Aug '21

11 Aug '21

Peter Marheine has uploaded this change for review. ( https://review.coreboot.org/c/flashrom/+/56911 ) Change subject: tests: add init_shutdown test for realtek_mst_i2c_spi ...................................................................... tests: add init_shutdown test for realtek_mst_i2c_spi This can catch regressions like the earlier one in this programmer that caused initialization to always fail. Requires support for mocking POSIX file I/O functions because the programmer does ioctls on the opened file. TEST=ninja test Signed-off-by: Peter Marheine <pmarheine(a)chromium.org> Change-Id: I5a5c617d1ec35d2a3bbe622e5add82a65eb396f0 --- M tests/init_shutdown.c M tests/io_mock.h M tests/meson.build M tests/tests.c M tests/tests.h 5 files changed, 117 insertions(+), 1 deletion(-) git pull ssh://review.coreboot.org:29418/flashrom refs/changes/11/56911/1 diff --git a/tests/init_shutdown.c b/tests/init_shutdown.c index 2698ea1..7078f79 100644 --- a/tests/init_shutdown.c +++ b/tests/init_shutdown.c @@ -296,3 +296,78 @@ skip(); #endif } + +#if IS_LINUX == 1 && CONFIG_REALTEK_MST_I2C_SPI == 1 + +#include <fcntl.h> +#include <linux/i2c-dev.h> + +static int realtek_mst_open64(void *state, const char *pathname, int flags) +{ + if (strcmp(pathname, "/dev/i2c-254") != 0) + return -1; + if ((flags & O_RDWR) != O_RDWR) + return -1; + + return 0x10ec; +} + +static int realtek_mst_ioctl(void *state, int fd, unsigned long request, va_list args) +{ + unsigned long addr; + + if (fd != 0x10ec) + return -1; + if (request != I2C_SLAVE) + return -1; + addr = va_arg(args, unsigned long); + printf("ioctl(0x10ec, I2C_SLAVE, %#lx)\n", addr); + if (addr != 0x4a) + return -1; + + return 0; +} + +static int realtek_mst_read(void *state, int fd, void *buf, size_t sz) +{ + if (fd != 0x10ec || sz != 1) + return -1; + + printf("i2c register read\n"); + return 1; +} + +static int realtek_mst_write(void *state, int fd, const void *buf, size_t sz) +{ + const uint8_t *bytes_buf = buf; + if (fd != 0x10ec) { + return -1; + } else if (sz == 1) { + printf("i2c set register address %#x\n", bytes_buf[0]); + return 1; + } else if (sz == 2) { + printf("i2c write %#x -> %#x\n", bytes_buf[1], bytes_buf[0]); + return 2; + } else { + return -1; + } +} + +void realtek_mst_init_and_shutdown_test_success(void **state) +{ + const struct io_mock realtek_mst_io = { + .open = realtek_mst_open64, + .ioctl = realtek_mst_ioctl, + .read = realtek_mst_read, + .write = realtek_mst_write, + }; + io_mock_register(&realtek_mst_io); + run_lifecycle(state, &programmer_realtek_mst_i2c_spi, "bus=254,enter-isp=0"); +} + +#else +void realtek_mst_init_and_shutdown_test_success(void **state) +{ + skip(); +} +#endif /* IS_LINUX && CONFIG_REALTEK_MST_I2C_SPI */ diff --git a/tests/io_mock.h b/tests/io_mock.h index bad13ba..732d8d5 100644 --- a/tests/io_mock.h +++ b/tests/io_mock.h @@ -68,10 +68,17 @@ unsigned char *data, uint16_t wLength, unsigned int timeout); - /* File I/O */ + /* Standard C File I/O */ FILE* (*fopen)(void *state, const char *pathname, const char *mode); char* (*fgets)(void *state, char *buf, int len, FILE *fp); size_t (*fread)(void *state, void *buf, size_t size, size_t len, FILE *fp); + + /* POSIX File I/O */ + int (*open)(void *state, const char *pathname, int flags); + int (*open64)(void *state, const char *pathname, int flags); + int (*ioctl)(void *state, int fd, unsigned long request, va_list args); + int (*read)(void *state, int fd, void *buf, size_t sz); + int (*write)(void *state, int fd, const void *buf, size_t sz); }; void io_mock_register(const struct io_mock *io); diff --git a/tests/meson.build b/tests/meson.build index 014ff02..2fb601e 100644 --- a/tests/meson.build +++ b/tests/meson.build @@ -34,6 +34,8 @@ '-Wl,--wrap=open', '-Wl,--wrap=open64', '-Wl,--wrap=ioctl', + '-Wl,--wrap=read', + '-Wl,--wrap=write', '-Wl,--wrap=fopen', '-Wl,--wrap=fopen64', '-Wl,--wrap=stat', diff --git a/tests/tests.c b/tests/tests.c index aab7d06..5ddb87c 100644 --- a/tests/tests.c +++ b/tests/tests.c @@ -89,15 +89,45 @@ int __wrap_open64(const char *pathname, int flags) { LOG_ME; + if (current_io) { + if (current_io->open64) + return current_io->open64(current_io->state, pathname, flags); + if (current_io->open) + return current_io->open(current_io->state, pathname, flags); + } return MOCK_HANDLE; } int __wrap_ioctl(int fd, unsigned long int request, ...) { LOG_ME; + if (current_io && current_io->ioctl) { + va_list args; + int out; + va_start(args, request); + out = current_io->ioctl(current_io->state, fd, request, args); + va_end(args); + return out; + } return MOCK_HANDLE; } +int __wrap_write(int fd, const void *buf, size_t sz) +{ + LOG_ME; + if (current_io && current_io->write) + return current_io->write(current_io->state, fd, buf, sz); + return -1; +} + +int __wrap_read(int fd, void *buf, size_t sz) +{ + LOG_ME; + if (current_io && current_io->read) + return current_io->read(current_io->state, fd, buf, sz); + return -1; +} + FILE *__wrap_fopen(const char *pathname, const char *mode) { LOG_ME; @@ -312,6 +342,7 @@ cmocka_unit_test(ene_lpc_init_and_shutdown_test_success), cmocka_unit_test(linux_mtd_init_and_shutdown_test_success), cmocka_unit_test(linux_spi_init_and_shutdown_test_success), + cmocka_unit_test(realtek_mst_init_and_shutdown_test_success), }; ret |= cmocka_run_group_tests_name("init_shutdown.c tests", init_shutdown_tests, NULL, NULL); diff --git a/tests/tests.h b/tests/tests.h index 989f83f..7088f05 100644 --- a/tests/tests.h +++ b/tests/tests.h @@ -48,6 +48,7 @@ void ene_lpc_init_and_shutdown_test_success(void **state); void linux_mtd_init_and_shutdown_test_success(void **state); void linux_spi_init_and_shutdown_test_success(void **state); +void realtek_mst_init_and_shutdown_test_success(void **state); /* layout.c */ void included_regions_dont_overlap_test_success(void **state); -- To view, visit https://review.coreboot.org/c/flashrom/+/56911 To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings Gerrit-Project: flashrom Gerrit-Branch: master Gerrit-Change-Id: I5a5c617d1ec35d2a3bbe622e5add82a65eb396f0 Gerrit-Change-Number: 56911 Gerrit-PatchSet: 1 Gerrit-Owner: Peter Marheine <pmarheine(a)chromium.org> Gerrit-MessageType: newchange

1 0

Change in flashrom[master]: tests: Mock file i/o for linux_mtd and linux_spi tests
by Edward O'Callaghan (Code Review) 11 Aug '21

11 Aug '21

Attention is currently required from: Nico Huber, Angel Pons, Anastasia Klimchuk. Edward O'Callaghan has posted comments on this change. ( https://review.coreboot.org/c/flashrom/+/56413 ) Change subject: tests: Mock file i/o for linux_mtd and linux_spi tests ...................................................................... Patch Set 4: (5 comments) File tests/init_shutdown.c: https://review.coreboot.org/c/flashrom/+/56413/comment/7ef74282_f8bda603 PS4, Line 208: 2021 this number is very arbitrary. Something that is obvious like -1 or 0x55AA [01010101 10101010] is better. https://review.coreboot.org/c/flashrom/+/56413/comment/688237a7_01994af0 PS4, Line 219: // why not just `/` is something special happening here? https://review.coreboot.org/c/flashrom/+/56413/comment/198059a8_9841a0f3 PS4, Line 230: num_bytes = 0; seems like a bug? strings in C are null terminated which is a byte. https://review.coreboot.org/c/flashrom/+/56413/comment/289d44f8_ddde1b1a PS4, Line 274: Provide pagesize into buf. It is better to describe where the value is from and why it has it the particular value than re-describing what the code already shows. `/* Fix pagesize to 1MB. */` seems to be wht you are doing here? https://review.coreboot.org/c/flashrom/+/56413/comment/21c89362_580c2fcc PS4, Line 275: memcpy(buf, "1048576", len); just, `return memcpy(buf, "1048576", len);` memcpy returns the ptr to dest (buf in this case). -- To view, visit https://review.coreboot.org/c/flashrom/+/56413 To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings Gerrit-Project: flashrom Gerrit-Branch: master Gerrit-Change-Id: I73f0d6ff2ad5074add7a721ed3416230d3647e3f Gerrit-Change-Number: 56413 Gerrit-PatchSet: 4 Gerrit-Owner: Anastasia Klimchuk <aklm(a)chromium.org> Gerrit-Reviewer: Angel Pons <th3fanbus(a)gmail.com> Gerrit-Reviewer: Edward O'Callaghan <quasisec(a)chromium.org> Gerrit-Reviewer: Nico Huber <nico.h(a)gmx.de> Gerrit-Reviewer: build bot (Jenkins) <no-reply(a)coreboot.org> Gerrit-CC: Paul Menzel <paulepanter(a)mailbox.org> Gerrit-Attention: Nico Huber <nico.h(a)gmx.de> Gerrit-Attention: Angel Pons <th3fanbus(a)gmail.com> Gerrit-Attention: Anastasia Klimchuk <aklm(a)chromium.org> Gerrit-Comment-Date: Wed, 11 Aug 2021 01:31:23 +0000 Gerrit-HasComments: Yes Gerrit-Has-Labels: No Gerrit-MessageType: comment

1 0

Change in flashrom[master]: tests: Mock file i/o for linux_mtd and linux_spi tests
by Anastasia Klimchuk (Code Review) 10 Aug '21

10 Aug '21

Attention is currently required from: Nico Huber, Edward O'Callaghan, Angel Pons. Anastasia Klimchuk has posted comments on this change. ( https://review.coreboot.org/c/flashrom/+/56413 ) Change subject: tests: Mock file i/o for linux_mtd and linux_spi tests ...................................................................... Patch Set 4: (1 comment) Patchset: PS4: I rebased to the latest -- To view, visit https://review.coreboot.org/c/flashrom/+/56413 To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings Gerrit-Project: flashrom Gerrit-Branch: master Gerrit-Change-Id: I73f0d6ff2ad5074add7a721ed3416230d3647e3f Gerrit-Change-Number: 56413 Gerrit-PatchSet: 4 Gerrit-Owner: Anastasia Klimchuk <aklm(a)chromium.org> Gerrit-Reviewer: Angel Pons <th3fanbus(a)gmail.com> Gerrit-Reviewer: Edward O'Callaghan <quasisec(a)chromium.org> Gerrit-Reviewer: Nico Huber <nico.h(a)gmx.de> Gerrit-Reviewer: build bot (Jenkins) <no-reply(a)coreboot.org> Gerrit-CC: Paul Menzel <paulepanter(a)mailbox.org> Gerrit-Attention: Nico Huber <nico.h(a)gmx.de> Gerrit-Attention: Edward O'Callaghan <quasisec(a)chromium.org> Gerrit-Attention: Angel Pons <th3fanbus(a)gmail.com> Gerrit-Comment-Date: Tue, 10 Aug 2021 04:43:52 +0000 Gerrit-HasComments: Yes Gerrit-Has-Labels: No Gerrit-MessageType: comment

1 0

Change in flashrom[master]: tests: Mock file i/o for linux_mtd and linux_spi tests
by Anastasia Klimchuk (Code Review) 10 Aug '21

10 Aug '21

Attention is currently required from: Nico Huber, Edward O'Callaghan, Angel Pons. Hello build bot (Jenkins), Nico Huber, Edward O'Callaghan, Angel Pons, I'd like you to reexamine a change. Please visit https://review.coreboot.org/c/flashrom/+/56413 to look at the new patch set (#4). Change subject: tests: Mock file i/o for linux_mtd and linux_spi tests ...................................................................... tests: Mock file i/o for linux_mtd and linux_spi tests This patch adds an init-shutdown test for linux_mtd. Since linux_mtd is using file i/o operations, those are added to the framework and mocked. Another driver linux_spi which is also using file i/o, got an upgrade in this patch, and it is now reading pagesize from sysfs (using mocked file i/o). A good side-effect is that linux_mtd is the first test for opaque masters, which is great to have in preparation for a change like CB:56103 but for opaque masters. BUG=b:181803212 TEST=builds and ninja test Change-Id: I73f0d6ff2ad5074add7a721ed3416230d3647e3f Signed-off-by: Anastasia Klimchuk <aklm(a)chromium.org> --- M tests/init_shutdown.c M tests/io_mock.h M tests/meson.build M tests/tests.c M tests/tests.h 5 files changed, 170 insertions(+), 4 deletions(-) git pull ssh://review.coreboot.org:29418/flashrom refs/changes/13/56413/4 -- To view, visit https://review.coreboot.org/c/flashrom/+/56413 To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings Gerrit-Project: flashrom Gerrit-Branch: master Gerrit-Change-Id: I73f0d6ff2ad5074add7a721ed3416230d3647e3f Gerrit-Change-Number: 56413 Gerrit-PatchSet: 4 Gerrit-Owner: Anastasia Klimchuk <aklm(a)chromium.org> Gerrit-Reviewer: Angel Pons <th3fanbus(a)gmail.com> Gerrit-Reviewer: Edward O'Callaghan <quasisec(a)chromium.org> Gerrit-Reviewer: Nico Huber <nico.h(a)gmx.de> Gerrit-Reviewer: build bot (Jenkins) <no-reply(a)coreboot.org> Gerrit-CC: Paul Menzel <paulepanter(a)mailbox.org> Gerrit-Attention: Nico Huber <nico.h(a)gmx.de> Gerrit-Attention: Edward O'Callaghan <quasisec(a)chromium.org> Gerrit-Attention: Angel Pons <th3fanbus(a)gmail.com> Gerrit-MessageType: newpatchset

1 0

Change in flashrom[master]: realtek_mst_i2c_spi: don't always fail init
by Edward O'Callaghan (Code Review) 10 Aug '21

10 Aug '21

Edward O'Callaghan has submitted this change. ( https://review.coreboot.org/c/flashrom/+/56799 ) Change subject: realtek_mst_i2c_spi: don't always fail init ...................................................................... realtek_mst_i2c_spi: don't always fail init In some earlier refactoring the init function for this programmer was broken such that it never succeeds, since the return value was never set to a value other than a generic failure. Fix it to return success unless there is a problem. TEST=flashrom -p realtek_mst_i2c_spi:bus=n --flash-size now works when n is a bus that has a connected MST. Change-Id: I0596014fc9b76b4d11de230f9f5c414c1321dff1 Signed-off-by: Peter Marheine <pmarheine(a)chromium.org> Reviewed-on: https://review.coreboot.org/c/flashrom/+/56799 Reviewed-by: Edward O'Callaghan <quasisec(a)chromium.org> Reviewed-by: Anastasia Klimchuk <aklm(a)chromium.org> Reviewed-by: Angel Pons <th3fanbus(a)gmail.com> Tested-by: build bot (Jenkins) <no-reply(a)coreboot.org> --- M realtek_mst_i2c_spi.c 1 file changed, 1 insertion(+), 1 deletion(-) Approvals: build bot (Jenkins): Verified Angel Pons: Looks good to me, approved Edward O'Callaghan: Looks good to me, approved Anastasia Klimchuk: Looks good to me, approved diff --git a/realtek_mst_i2c_spi.c b/realtek_mst_i2c_spi.c index 22f7e7a..a45838c 100644 --- a/realtek_mst_i2c_spi.c +++ b/realtek_mst_i2c_spi.c @@ -445,7 +445,7 @@ static int get_params(int *reset, int *enter_isp) { char *reset_str = NULL, *isp_str = NULL; - int ret = SPI_GENERIC_ERROR; + int ret = 0; reset_str = extract_programmer_param("reset-mcu"); if (reset_str) { -- To view, visit https://review.coreboot.org/c/flashrom/+/56799 To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings Gerrit-Project: flashrom Gerrit-Branch: master Gerrit-Change-Id: I0596014fc9b76b4d11de230f9f5c414c1321dff1 Gerrit-Change-Number: 56799 Gerrit-PatchSet: 3 Gerrit-Owner: Peter Marheine <pmarheine(a)chromium.org> Gerrit-Reviewer: Anastasia Klimchuk <aklm(a)chromium.org> Gerrit-Reviewer: Angel Pons <th3fanbus(a)gmail.com> Gerrit-Reviewer: Edward O'Callaghan <quasisec(a)chromium.org> Gerrit-Reviewer: build bot (Jenkins) <no-reply(a)coreboot.org> Gerrit-CC: Paul Menzel <paulepanter(a)mailbox.org> Gerrit-MessageType: merged

1 0

Change in flashrom[master]: internal: Return early from map_flash for >16MiB on x86
by Tim Wawrzynczak (Code Review) 09 Aug '21

09 Aug '21

Attention is currently required from: Nico Huber, Furquan Shaikh. Tim Wawrzynczak has posted comments on this change. ( https://review.coreboot.org/c/flashrom/+/56721 ) Change subject: internal: Return early from map_flash for >16MiB on x86 ...................................................................... Patch Set 3: (1 comment) Patchset: PS3: Could this get merged? -- To view, visit https://review.coreboot.org/c/flashrom/+/56721 To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings Gerrit-Project: flashrom Gerrit-Branch: master Gerrit-Change-Id: I60244b6970118dbdfdbb5b8424fc0a8acd9def2e Gerrit-Change-Number: 56721 Gerrit-PatchSet: 3 Gerrit-Owner: Tim Wawrzynczak <twawrzynczak(a)chromium.org> Gerrit-Reviewer: Edward O'Callaghan <quasisec(a)chromium.org> Gerrit-Reviewer: Furquan Shaikh <furquan(a)google.com> Gerrit-Reviewer: build bot (Jenkins) <no-reply(a)coreboot.org> Gerrit-CC: Angel Pons <th3fanbus(a)gmail.com> Gerrit-CC: Nico Huber <nico.h(a)gmx.de> Gerrit-CC: Paul Menzel <paulepanter(a)mailbox.org> Gerrit-Attention: Nico Huber <nico.h(a)gmx.de> Gerrit-Attention: Furquan Shaikh <furquan(a)google.com> Gerrit-Comment-Date: Mon, 09 Aug 2021 16:02:11 +0000 Gerrit-HasComments: Yes Gerrit-Has-Labels: No Gerrit-MessageType: comment

1 0

Change in flashrom[master]: tests: Add init-shutdown test for nicrealtek
by Edward O'Callaghan (Code Review) 09 Aug '21

09 Aug '21

Edward O'Callaghan has submitted this change. ( https://review.coreboot.org/c/flashrom/+/56470 ) Change subject: tests: Add init-shutdown test for nicrealtek ...................................................................... tests: Add init-shutdown test for nicrealtek This patch adds a test and mocks for two pci functions that nicreltek is using. Main reason for this is to have at least one test for par master (currently there are none), in preparation for a change like CB:56103 but for par masters. BUG=b:181803212 TEST=ninja test Change-Id: Iaed14fe1d83c8eb45ec185ebd3f1c97cb81941f4 Signed-off-by: Anastasia Klimchuk <aklm(a)chromium.org> Reviewed-on: https://review.coreboot.org/c/flashrom/+/56470 Tested-by: build bot (Jenkins) <no-reply(a)coreboot.org> Reviewed-by: Edward O'Callaghan <quasisec(a)chromium.org> --- M tests/init_shutdown.c M tests/io_mock.h M tests/meson.build M tests/tests.c M tests/tests.h 5 files changed, 34 insertions(+), 0 deletions(-) Approvals: build bot (Jenkins): Verified Edward O'Callaghan: Looks good to me, approved diff --git a/tests/init_shutdown.c b/tests/init_shutdown.c index 79918e6..eeea3cc 100644 --- a/tests/init_shutdown.c +++ b/tests/init_shutdown.c @@ -89,6 +89,15 @@ #endif } +void nicrealtek_init_and_shutdown_test_success(void **state) +{ +#if CONFIG_NICREALTEK == 1 + run_lifecycle(state, &programmer_nicrealtek, ""); +#else + skip(); +#endif +} + int dediprog_libusb_control_transfer(void *state, libusb_device_handle *devh, uint8_t bmRequestType, diff --git a/tests/io_mock.h b/tests/io_mock.h index 72d89c5..5cddca0 100644 --- a/tests/io_mock.h +++ b/tests/io_mock.h @@ -37,6 +37,11 @@ struct libusb_context; typedef struct libusb_context libusb_context; +/* Define struct pci_dev to avoid dependency on pci.h */ +struct pci_dev { + unsigned int device_id; +}; + struct io_mock { void *state; diff --git a/tests/meson.build b/tests/meson.build index 5776862..9fb86c7 100644 --- a/tests/meson.build +++ b/tests/meson.build @@ -26,6 +26,8 @@ '-Wl,--wrap=strdup', '-Wl,--wrap=physunmap', '-Wl,--wrap=physmap', + '-Wl,--wrap=pcidev_init', + '-Wl,--wrap=pcidev_readbar', '-Wl,--wrap=spi_send_command', '-Wl,--wrap=sio_write', '-Wl,--wrap=sio_read', diff --git a/tests/tests.c b/tests/tests.c index 00c987c..ece3e8a 100644 --- a/tests/tests.c +++ b/tests/tests.c @@ -53,6 +53,22 @@ return NULL; } +struct pci_dev mock_pci_dev = { + .device_id = MOCK_HANDLE, +}; + +struct pci_dev *__wrap_pcidev_init(void *devs, int bar) +{ + LOG_ME; + return &mock_pci_dev; +} + +uintptr_t __wrap_pcidev_readbar(void *dev, int bar) +{ + LOG_ME; + return MOCK_HANDLE; +} + void __wrap_sio_write(uint16_t port, uint8_t reg, uint8_t data) { LOG_ME; @@ -230,6 +246,7 @@ const struct CMUnitTest init_shutdown_tests[] = { cmocka_unit_test(dummy_init_and_shutdown_test_success), cmocka_unit_test(mec1308_init_and_shutdown_test_success), + cmocka_unit_test(nicrealtek_init_and_shutdown_test_success), cmocka_unit_test(dediprog_init_and_shutdown_test_success), cmocka_unit_test(ene_lpc_init_and_shutdown_test_success), cmocka_unit_test(linux_spi_init_and_shutdown_test_success), diff --git a/tests/tests.h b/tests/tests.h index 14cfee0..9eacf13 100644 --- a/tests/tests.h +++ b/tests/tests.h @@ -43,6 +43,7 @@ /* init_shutdown.c */ void dummy_init_and_shutdown_test_success(void **state); void mec1308_init_and_shutdown_test_success(void **state); +void nicrealtek_init_and_shutdown_test_success(void **state); void dediprog_init_and_shutdown_test_success(void **state); void ene_lpc_init_and_shutdown_test_success(void **state); void linux_spi_init_and_shutdown_test_success(void **state); -- To view, visit https://review.coreboot.org/c/flashrom/+/56470 To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings Gerrit-Project: flashrom Gerrit-Branch: master Gerrit-Change-Id: Iaed14fe1d83c8eb45ec185ebd3f1c97cb81941f4 Gerrit-Change-Number: 56470 Gerrit-PatchSet: 3 Gerrit-Owner: Anastasia Klimchuk <aklm(a)chromium.org> Gerrit-Reviewer: Angel Pons <th3fanbus(a)gmail.com> Gerrit-Reviewer: Edward O'Callaghan <quasisec(a)chromium.org> Gerrit-Reviewer: Nico Huber <nico.h(a)gmx.de> Gerrit-Reviewer: build bot (Jenkins) <no-reply(a)coreboot.org> Gerrit-CC: Paul Menzel <paulepanter(a)mailbox.org> Gerrit-MessageType: merged

1 0

Change in flashrom[master]: programmer_table: Remove space between address operator and variable
by Edward O'Callaghan (Code Review) 09 Aug '21

09 Aug '21

Edward O'Callaghan has submitted this change. ( https://review.coreboot.org/c/flashrom/+/56469 ) Change subject: programmer_table: Remove space between address operator and variable ...................................................................... programmer_table: Remove space between address operator and variable BUG=none TEST=builds Change-Id: Iba29b1bc3d77b8c17c51fc0552c129e6965fa308 Signed-off-by: Anastasia Klimchuk <aklm(a)chromium.org> Reviewed-on: https://review.coreboot.org/c/flashrom/+/56469 Tested-by: build bot (Jenkins) <no-reply(a)coreboot.org> Reviewed-by: Thomas Heijligen <src(a)posteo.de> Reviewed-by: Paul Menzel <paulepanter(a)mailbox.org> Reviewed-by: Angel Pons <th3fanbus(a)gmail.com> Reviewed-by: Nico Huber <nico.h(a)gmx.de> Reviewed-by: Edward O'Callaghan <quasisec(a)chromium.org> --- M programmer_table.c 1 file changed, 2 insertions(+), 2 deletions(-) Approvals: build bot (Jenkins): Verified Nico Huber: Looks good to me, approved Paul Menzel: Looks good to me, but someone else must approve Thomas Heijligen: Looks good to me, but someone else must approve Angel Pons: Looks good to me, approved Edward O'Callaghan: Looks good to me, approved diff --git a/programmer_table.c b/programmer_table.c index e7f2c98..6eb6b77 100644 --- a/programmer_table.c +++ b/programmer_table.c @@ -33,7 +33,7 @@ #endif #if CONFIG_NICREALTEK == 1 - & programmer_nicrealtek, + &programmer_nicrealtek, #endif #if CONFIG_NICNATSEMI == 1 @@ -173,4 +173,4 @@ #endif }; -const size_t programmer_table_size = ARRAY_SIZE(programmer_table); \ No newline at end of file +const size_t programmer_table_size = ARRAY_SIZE(programmer_table); -- To view, visit https://review.coreboot.org/c/flashrom/+/56469 To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings Gerrit-Project: flashrom Gerrit-Branch: master Gerrit-Change-Id: Iba29b1bc3d77b8c17c51fc0552c129e6965fa308 Gerrit-Change-Number: 56469 Gerrit-PatchSet: 3 Gerrit-Owner: Anastasia Klimchuk <aklm(a)chromium.org> Gerrit-Reviewer: Angel Pons <th3fanbus(a)gmail.com> Gerrit-Reviewer: Edward O'Callaghan <quasisec(a)chromium.org> Gerrit-Reviewer: Nico Huber <nico.h(a)gmx.de> Gerrit-Reviewer: Paul Menzel <paulepanter(a)mailbox.org> Gerrit-Reviewer: Thomas Heijligen <src(a)posteo.de> Gerrit-Reviewer: build bot (Jenkins) <no-reply(a)coreboot.org> Gerrit-MessageType: merged

1 0

2024

2023

2022

2021

2020

2019

2018

2017

flashrom-gerrit August 2021