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

Change in flashrom[master]: tests: Use real spi_send_command for all tests except spi25.c
by Anastasia Klimchuk (Code Review) 12 Aug '21

12 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/+/56753 ) Change subject: tests: Use real spi_send_command for all tests except spi25.c ...................................................................... Patch Set 3: (1 comment) Patchset: PS3: I have good news! I was able to make it work without splitting into two executables, I am so happy! So this patch is completely re-done (and it's much smaller now). The next one in the chain, with erase test, stays the same. -- To view, visit https://review.coreboot.org/c/flashrom/+/56753 To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings Gerrit-Project: flashrom Gerrit-Branch: master Gerrit-Change-Id: I22945cce3d0f36adaa8032167a3ef4e54eccb611 Gerrit-Change-Number: 56753 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-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: Thu, 12 Aug 2021 01:24:29 +0000 Gerrit-HasComments: Yes Gerrit-Has-Labels: No Gerrit-MessageType: comment

1 0

Change in flashrom[master]: tests: Use real spi_send_command for all tests except spi25.c
by Anastasia Klimchuk (Code Review) 12 Aug '21

12 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/+/56753 to look at the new patch set (#3). Change subject: tests: Use real spi_send_command for all tests except spi25.c ...................................................................... tests: Use real spi_send_command for all tests except spi25.c At the moment one test (spi25.c) uses wrap of spi_send_command, and all other existing tests don't care about this function (don't call it at all). However in the next patch a new test in introduced, which needs a real spi_send_command. Following the approach "don't mock unless it is strictly necessary", all tests that don't care go into real function bucket. BUG=b:181803212 TEST=builds and ninja test Change-Id: I22945cce3d0f36adaa8032167a3ef4e54eccb611 Signed-off-by: Anastasia Klimchuk <aklm(a)chromium.org> --- M tests/spi25.c 1 file changed, 28 insertions(+), 16 deletions(-) git pull ssh://review.coreboot.org:29418/flashrom refs/changes/53/56753/3 -- To view, visit https://review.coreboot.org/c/flashrom/+/56753 To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings Gerrit-Project: flashrom Gerrit-Branch: master Gerrit-Change-Id: I22945cce3d0f36adaa8032167a3ef4e54eccb611 Gerrit-Change-Number: 56753 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-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]: tests: add init_shutdown test for realtek_mst_i2c_spi
by Anastasia Klimchuk (Code Review) 11 Aug '21

11 Aug '21

Attention is currently required from: Peter Marheine. Anastasia Klimchuk 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: (8 comments) Patchset: PS1: Thanks for writing a test! File tests/init_shutdown.c: https://review.coreboot.org/c/flashrom/+/56911/comment/8df145ce_db617b87 PS1, Line 302: #include <fcntl.h> : #include <linux/i2c-dev.h> Tests should be as lightweight and independent as possible, they only thing to check is CONFIG_REALTEK_MST_I2C_SPI == 1, and that's inside test method. Detect exactly what is the minimum set of symbols you need from these libraries (is it O_RDWR and I2C_SLAVE?) and define them in io_mock.h (there are some examples of that already). Remove these includes, remove check for IS_LINUX. Mock implementations (open,ioctl,read,write) should compile as is, you only check CONFIG_REALTEK_MST_I2C_SPI == 1 inside the test method. Follow the example of all other tests. https://review.coreboot.org/c/flashrom/+/56911/comment/4f925a5e_348e9737 PS1, Line 364: io_mock_register(&realtek_mst_io); You need to unregister at the end, `io_mock_register(NULL);` (you can look at the other tests) File tests/tests.c: https://review.coreboot.org/c/flashrom/+/56911/comment/0bdaa2c5_1fdc3cc9 PS1, Line 93: 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); Why do you put both open and open64 into one wrap? There is a dedicated wrap for open, so `current_io->open` should go there. https://review.coreboot.org/c/flashrom/+/56911/comment/7c4aa9ad_0703901f PS1, Line 120: return -1; Default behaviour for our wraps is "do nothing, pretend everything is successful". You need to return a value which emulates success. Is it sz for this write operation? https://review.coreboot.org/c/flashrom/+/56911/comment/27a21a61_69f228be PS1, Line 127: return current_io->read(current_io->state, fd, buf, sz); We are following Linux kernel coding style https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git/tree/Doc… which says that indentations are 8 chars. https://review.coreboot.org/c/flashrom/+/56911/comment/01ad558a_8a5fc193 PS1, Line 128: return -1; Same here, you need to return value which means success. https://review.coreboot.org/c/flashrom/+/56911/comment/cb85340b_14ff6424 PS1, Line 345: cmocka_unit_test Same thing, indentations should be 8 chars -- 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: Peter Marheine <pmarheine(a)chromium.org> Gerrit-Comment-Date: Wed, 11 Aug 2021 23:30:56 +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 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

2024

2023

2022

2021

2020

2019

2018

2017

flashrom-gerrit August 2021