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

Change in flashrom[master]: linux_spi.c: Resolve comments left from previous patches
by Angel Pons (Code Review) 19 Apr '21

19 Apr '21

Attention is currently required from: Nico Huber, Anastasia Klimchuk. Angel Pons has posted comments on this change. ( https://review.coreboot.org/c/flashrom/+/52492 ) Change subject: linux_spi.c: Resolve comments left from previous patches ...................................................................... Patch Set 1: Code-Review+1 -- To view, visit https://review.coreboot.org/c/flashrom/+/52492 To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings Gerrit-Project: flashrom Gerrit-Branch: master Gerrit-Change-Id: I27a718b515fc474f63b3e61be58a6f9302527559 Gerrit-Change-Number: 52492 Gerrit-PatchSet: 1 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: Nikolai Artemiev <nartemiev(a)google.com> Gerrit-CC: Paul Menzel <paulepanter(a)mailbox.org> Gerrit-CC: Sam McNally <sammc(a)google.com> Gerrit-Attention: Nico Huber <nico.h(a)gmx.de> Gerrit-Attention: Anastasia Klimchuk <aklm(a)chromium.org> Gerrit-Comment-Date: Mon, 19 Apr 2021 08:57:17 +0000 Gerrit-HasComments: No Gerrit-Has-Labels: Yes Gerrit-MessageType: comment

1 0

Change in flashrom[master]: lspcon, realtek_mst: move i2c bus resolution from params to i2c_helper
by Angel Pons (Code Review) 19 Apr '21

19 Apr '21

Attention is currently required from: Sam McNally, Peter Marheine. Angel Pons has posted comments on this change. ( https://review.coreboot.org/c/flashrom/+/52405 ) Change subject: lspcon, realtek_mst: move i2c bus resolution from params to i2c_helper ...................................................................... Patch Set 6: (2 comments) File i2c_helper_linux.c: https://review.coreboot.org/c/flashrom/+/52405/comment/3818f2ab_ddde4eb1 PS6, Line 36: { style nit: The opening brace for functions should go on the next line https://review.coreboot.org/c/flashrom/+/52405/comment/56b0cd2b_fc1332b4 PS6, Line 42: char *dev = malloc(sizeof(I2C_DEV_PREFIX)); This effectively undoes what CB:52415 did... -- To view, visit https://review.coreboot.org/c/flashrom/+/52405 To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings Gerrit-Project: flashrom Gerrit-Branch: master Gerrit-Change-Id: Ieea57b2445f3aca16c87d8f839dff131009531a0 Gerrit-Change-Number: 52405 Gerrit-PatchSet: 6 Gerrit-Owner: Peter Marheine <pmarheine(a)chromium.org> Gerrit-Reviewer: Edward O'Callaghan <quasisec(a)chromium.org> Gerrit-Reviewer: Sam McNally <sammc(a)google.com> Gerrit-Reviewer: build bot (Jenkins) <no-reply(a)coreboot.org> Gerrit-CC: Angel Pons <th3fanbus(a)gmail.com> Gerrit-Attention: Sam McNally <sammc(a)google.com> Gerrit-Attention: Peter Marheine <pmarheine(a)chromium.org> Gerrit-Comment-Date: Mon, 19 Apr 2021 08:53:41 +0000 Gerrit-HasComments: Yes Gerrit-Has-Labels: No Gerrit-MessageType: comment

1 0

Change in flashrom[master]: tests: Add unit test to run init/shutdown for mec1308.c
by Anastasia Klimchuk (Code Review) 19 Apr '21

19 Apr '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/+/51487 ) Change subject: tests: Add unit test to run init/shutdown for mec1308.c ...................................................................... Patch Set 3: (2 comments) Patchset: PS2: > Please split and thanks for the patience! […] I split this into 5 patches, and first three are ready, these ones: https://review.coreboot.org/c/flashrom/+/52496 https://review.coreboot.org/c/flashrom/+/52497 https://review.coreboot.org/c/flashrom/+/52498 For the remaining two I still need to do testing and resolve comments. This patch is now #5 out of chain of 5. File tests/tests.c: https://review.coreboot.org/c/flashrom/+/51487/comment/9980bbe6_31ac8f23 PS2, Line 99: : 0)); > This can get quite complex when additional tests are added. We can […] Agree, this can happen (although I noticed for some cases it was sufficient to just return 0). But I am very happy that now all this is contained within tests/ directory which makes it easy to experiment and change. If tests pass means tests are not broken :) -- To view, visit https://review.coreboot.org/c/flashrom/+/51487 To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings Gerrit-Project: flashrom Gerrit-Branch: master Gerrit-Change-Id: I3af612defe1af3850dfc1626a208d873e3a3eddc Gerrit-Change-Number: 51487 Gerrit-PatchSet: 3 Gerrit-Owner: Anastasia Klimchuk <aklm(a)chromium.org> Gerrit-Reviewer: Angel Pons <th3fanbus(a)gmail.com> Gerrit-Reviewer: Nico Huber <nico.h(a)gmx.de> Gerrit-Reviewer: build bot (Jenkins) <no-reply(a)coreboot.org> Gerrit-CC: Edward O'Callaghan <quasisec(a)chromium.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: Mon, 19 Apr 2021 06:59:49 +0000 Gerrit-HasComments: Yes Gerrit-Has-Labels: No Comment-In-Reply-To: Nico Huber <nico.h(a)gmx.de> Comment-In-Reply-To: Anastasia Klimchuk <aklm(a)chromium.org> Gerrit-MessageType: comment

1 0

Change in flashrom[master]: hwaccess.h: Split hwaccess.h and extract hwaccess_io.h out of it
by Anastasia Klimchuk (Code Review) 19 Apr '21

19 Apr '21

Anastasia Klimchuk has posted comments on this change. ( https://review.coreboot.org/c/flashrom/+/52499 ) Change subject: hwaccess.h: Split hwaccess.h and extract hwaccess_io.h out of it ...................................................................... Patch Set 1: (1 comment) Patchset: PS1: This needs to be tested by comparing binaries -- To view, visit https://review.coreboot.org/c/flashrom/+/52499 To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings Gerrit-Project: flashrom Gerrit-Branch: master Gerrit-Change-Id: Idd04c7b36b24e9da339348a015df4f43a03744f7 Gerrit-Change-Number: 52499 Gerrit-PatchSet: 1 Gerrit-Owner: Anastasia Klimchuk <aklm(a)chromium.org> Gerrit-Comment-Date: Mon, 19 Apr 2021 06:51:30 +0000 Gerrit-HasComments: Yes Gerrit-Has-Labels: No Gerrit-MessageType: comment

1 0

Change in flashrom[master]: hwaccess.h: Split hwaccess.h and extract hwaccess_io.h out of it
by Anastasia Klimchuk (Code Review) 19 Apr '21

19 Apr '21

Anastasia Klimchuk has uploaded this change for review. ( https://review.coreboot.org/c/flashrom/+/52499 ) Change subject: hwaccess.h: Split hwaccess.h and extract hwaccess_io.h out of it ...................................................................... hwaccess.h: Split hwaccess.h and extract hwaccess_io.h out of it This change makes it possible to mock functions from hwaccess_io.h in tests. The rest of hwaccess.h is fine and works for the test environment. BUG=b:181803212 TEST=builds Change-Id: Idd04c7b36b24e9da339348a015df4f43a03744f7 Signed-off-by: Anastasia Klimchuk <aklm(a)chromium.org> --- M hwaccess.h A hwaccess_io.h 2 files changed, 157 insertions(+), 136 deletions(-) git pull ssh://review.coreboot.org:29418/flashrom refs/changes/99/52499/1 diff --git a/hwaccess.h b/hwaccess.h index 5602c15..0f33017 100644 --- a/hwaccess.h +++ b/hwaccess.h @@ -112,142 +112,7 @@ #if NEED_RAW_ACCESS == 1 #if IS_X86 -/* sys/io.h provides iopl(2) and x86 I/O port access functions (inb, outb etc). - * It is included in glibc (thus available also on debian/kFreeBSD) but also in other libcs that mimic glibc, - * e.g. musl and uclibc. Because we cannot detect the libc or existence of the header or of the instructions - * themselves safely in here we use some heuristic below: - * On Android we don't have the header file and no way for I/O port access at all. However, sys/glibc-syscalls.h - * refers to an iopl implementation and we therefore include at least that one for now. On non-Android we assume - * that a Linux system's libc has a suitable sys/io.h or (on non-Linux) we depend on glibc to offer it. */ -#if defined(__ANDROID__) -#include <sys/glibc-syscalls.h> -#elif defined(__linux__) || defined(__GLIBC__) -#include <sys/io.h> -#endif - -#define __FLASHROM_HAVE_OUTB__ 1 - -/* for iopl and outb under Solaris */ -#if defined (__sun) -#include <sys/sysi86.h> -#include <sys/psw.h> -#include <asm/sunddi.h> -#endif - -/* Clarification about OUTB/OUTW/OUTL argument order: - * OUT[BWL](val, port) - */ - -#if defined(__FreeBSD__) || defined(__DragonFly__) - /* Note that Debian/kFreeBSD (FreeBSD kernel with glibc) has conflicting - * out[bwl] definitions in machine/cpufunc.h and sys/io.h at least in some - * versions. Use machine/cpufunc.h only for plain FreeBSD/DragonFlyBSD. - */ - #include <sys/types.h> - #include <machine/cpufunc.h> - #define OUTB(x, y) do { u_int outb_tmp = (y); outb(outb_tmp, (x)); } while (0) - #define OUTW(x, y) do { u_int outw_tmp = (y); outw(outw_tmp, (x)); } while (0) - #define OUTL(x, y) do { u_int outl_tmp = (y); outl(outl_tmp, (x)); } while (0) - #define INB(x) __extension__ ({ u_int inb_tmp = (x); inb(inb_tmp); }) - #define INW(x) __extension__ ({ u_int inw_tmp = (x); inw(inw_tmp); }) - #define INL(x) __extension__ ({ u_int inl_tmp = (x); inl(inl_tmp); }) -#else - -#if defined (__sun) - /* Note different order for outb */ - #define OUTB(x,y) outb(y, x) - #define OUTW(x,y) outw(y, x) - #define OUTL(x,y) outl(y, x) - #define INB inb - #define INW inw - #define INL inl -#else - -#ifdef __DJGPP__ - -#include <pc.h> - - #define OUTB(x,y) outportb(y, x) - #define OUTW(x,y) outportw(y, x) - #define OUTL(x,y) outportl(y, x) - - #define INB inportb - #define INW inportw - #define INL inportl - -#else - -#if defined(__MACH__) && defined(__APPLE__) - /* Header is part of the DirectHW library. */ - #include <DirectHW/DirectHW.h> -#endif - - /* This is the usual glibc interface. */ - #define OUTB outb - #define OUTW outw - #define OUTL outl - #define INB inb - #define INW inw - #define INL inl -#endif -#endif -#endif - -#if defined(__NetBSD__) || defined (__OpenBSD__) - #if defined(__i386__) || defined(__x86_64__) - #include <sys/types.h> - #include <machine/sysarch.h> - #if defined(__NetBSD__) - #if defined(__i386__) - #define iopl i386_iopl - #elif defined(__x86_64__) - #define iopl x86_64_iopl - #endif - #elif defined (__OpenBSD__) - #if defined(__i386__) - #define iopl i386_iopl - #elif defined(__amd64__) - #define iopl amd64_iopl - #endif - #endif - -static inline void outb(uint8_t value, uint16_t port) -{ - __asm__ volatile ("outb %b0,%w1": :"a" (value), "Nd" (port)); -} - -static inline uint8_t inb(uint16_t port) -{ - uint8_t value; - __asm__ volatile ("inb %w1,%0":"=a" (value):"Nd" (port)); - return value; -} - -static inline void outw(uint16_t value, uint16_t port) -{ - __asm__ volatile ("outw %w0,%w1": :"a" (value), "Nd" (port)); -} - -static inline uint16_t inw(uint16_t port) -{ - uint16_t value; - __asm__ volatile ("inw %w1,%0":"=a" (value):"Nd" (port)); - return value; -} - -static inline void outl(uint32_t value, uint16_t port) -{ - __asm__ volatile ("outl %0,%w1": :"a" (value), "Nd" (port)); -} - -static inline uint32_t inl(uint16_t port) -{ - uint32_t value; - __asm__ volatile ("inl %1,%0":"=a" (value):"Nd" (port)); - return value; -} - #endif -#endif +#include "hwaccess_io.h" #if !(defined(__MACH__) && defined(__APPLE__)) && !defined(__FreeBSD__) && !defined(__FreeBSD_kernel__) && !defined(__DragonFly__) && !defined(__LIBPAYLOAD__) typedef struct { uint32_t hi, lo; } msr_t; diff --git a/hwaccess_io.h b/hwaccess_io.h new file mode 100644 index 0000000..272b879 --- /dev/null +++ b/hwaccess_io.h @@ -0,0 +1,156 @@ +/* + * This file is part of the flashrom project. + * + * Copyright (C) 2009 Carl-Daniel Hailfinger + * + * This program is free software; you can redistribute it and/or modify + * it under the terms of the GNU General Public License as published by + * the Free Software Foundation; version 2 of the License. + * + * This program is distributed in the hope that it will be useful, + * but WITHOUT ANY WARRANTY; without even the implied warranty of + * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the + * GNU General Public License for more details. + */ + +#ifndef __HWACCESS_IO_H__ +#define __HWACCESS_IO_H__ 1 + +/* sys/io.h provides iopl(2) and x86 I/O port access functions (inb, outb etc). + * It is included in glibc (thus available also on debian/kFreeBSD) but also in other libcs that mimic glibc, + * e.g. musl and uclibc. Because we cannot detect the libc or existence of the header or of the instructions + * themselves safely in here we use some heuristic below: + * On Android we don't have the header file and no way for I/O port access at all. However, sys/glibc-syscalls.h + * refers to an iopl implementation and we therefore include at least that one for now. On non-Android we assume + * that a Linux system's libc has a suitable sys/io.h or (on non-Linux) we depend on glibc to offer it. */ +#if defined(__ANDROID__) +#include <sys/glibc-syscalls.h> +#elif defined(__linux__) || defined(__GLIBC__) +#include <sys/io.h> +#endif + +#define __FLASHROM_HAVE_OUTB__ 1 + +/* for iopl and outb under Solaris */ +#if defined (__sun) +#include <sys/sysi86.h> +#include <sys/psw.h> +#include <asm/sunddi.h> +#endif + +/* Clarification about OUTB/OUTW/OUTL argument order: + * OUT[BWL](val, port) + */ + +#if defined(__FreeBSD__) || defined(__DragonFly__) + /* Note that Debian/kFreeBSD (FreeBSD kernel with glibc) has conflicting + * out[bwl] definitions in machine/cpufunc.h and sys/io.h at least in some + * versions. Use machine/cpufunc.h only for plain FreeBSD/DragonFlyBSD. + */ + #include <sys/types.h> + #include <machine/cpufunc.h> + #define OUTB(x, y) do { u_int outb_tmp = (y); outb(outb_tmp, (x)); } while (0) + #define OUTW(x, y) do { u_int outw_tmp = (y); outw(outw_tmp, (x)); } while (0) + #define OUTL(x, y) do { u_int outl_tmp = (y); outl(outl_tmp, (x)); } while (0) + #define INB(x) __extension__ ({ u_int inb_tmp = (x); inb(inb_tmp); }) + #define INW(x) __extension__ ({ u_int inw_tmp = (x); inw(inw_tmp); }) + #define INL(x) __extension__ ({ u_int inl_tmp = (x); inl(inl_tmp); }) +#else + +#if defined (__sun) + /* Note different order for outb */ + #define OUTB(x,y) outb(y, x) + #define OUTW(x,y) outw(y, x) + #define OUTL(x,y) outl(y, x) + #define INB inb + #define INW inw + #define INL inl +#else + +#ifdef __DJGPP__ + +#include <pc.h> + + #define OUTB(x,y) outportb(y, x) + #define OUTW(x,y) outportw(y, x) + #define OUTL(x,y) outportl(y, x) + + #define INB inportb + #define INW inportw + #define INL inportl + +#else + +#if defined(__MACH__) && defined(__APPLE__) + /* Header is part of the DirectHW library. */ + #include <DirectHW/DirectHW.h> +#endif + + /* This is the usual glibc interface. */ + #define OUTB outb + #define OUTW outw + #define OUTL outl + #define INB inb + #define INW inw + #define INL inl +#endif +#endif +#endif + +#if defined(__NetBSD__) || defined (__OpenBSD__) + #if defined(__i386__) || defined(__x86_64__) + #include <sys/types.h> + #include <machine/sysarch.h> + #if defined(__NetBSD__) + #if defined(__i386__) + #define iopl i386_iopl + #elif defined(__x86_64__) + #define iopl x86_64_iopl + #endif + #elif defined (__OpenBSD__) + #if defined(__i386__) + #define iopl i386_iopl + #elif defined(__amd64__) + #define iopl amd64_iopl + #endif + #endif + +static inline void outb(uint8_t value, uint16_t port) +{ + __asm__ volatile ("outb %b0,%w1": :"a" (value), "Nd" (port)); +} + +static inline uint8_t inb(uint16_t port) +{ + uint8_t value; + __asm__ volatile ("inb %w1,%0":"=a" (value):"Nd" (port)); + return value; +} + +static inline void outw(uint16_t value, uint16_t port) +{ + __asm__ volatile ("outw %w0,%w1": :"a" (value), "Nd" (port)); +} + +static inline uint16_t inw(uint16_t port) +{ + uint16_t value; + __asm__ volatile ("inw %w1,%0":"=a" (value):"Nd" (port)); + return value; +} + +static inline void outl(uint32_t value, uint16_t port) +{ + __asm__ volatile ("outl %0,%w1": :"a" (value), "Nd" (port)); +} + +static inline uint32_t inl(uint16_t port) +{ + uint32_t value; + __asm__ volatile ("inl %1,%0":"=a" (value):"Nd" (port)); + return value; +} + #endif +#endif + +#endif /* __HWACCESS_IO_H__ */ -- To view, visit https://review.coreboot.org/c/flashrom/+/52499 To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings Gerrit-Project: flashrom Gerrit-Branch: master Gerrit-Change-Id: Idd04c7b36b24e9da339348a015df4f43a03744f7 Gerrit-Change-Number: 52499 Gerrit-PatchSet: 1 Gerrit-Owner: Anastasia Klimchuk <aklm(a)chromium.org> Gerrit-MessageType: newchange

1 0

Change in flashrom[master]: tests: Add unit test to run init/shutdown for linux_spi.c
by Anastasia Klimchuk (Code Review) 19 Apr '21

19 Apr '21

Anastasia Klimchuk has uploaded this change for review. ( https://review.coreboot.org/c/flashrom/+/52498 ) Change subject: tests: Add unit test to run init/shutdown for linux_spi.c ...................................................................... tests: Add unit test to run init/shutdown for linux_spi.c Extract from meson-logs/testlog.txt for new test: [ RUN ] linux_spi_init_and_shutdown_test_success Testing programmer_init for programmer=25 ... __wrap_open64 is called __wrap_ioctl is called __wrap_ioctl is called __wrap_ioctl is called __wrap_fopen64 is called ... programmer_init for programmer=25 successful Testing programmer_shutdown for programmer=25 ... ... programmer_shutdown for programmer=25 successful [ OK ] linux_spi_init_and_shutdown_test_success BUG=b:181803212 TEST=builds and ninja test Change-Id: I4911fbb6f04371283f0e62d2196bdd691a227584 Signed-off-by: Anastasia Klimchuk <aklm(a)chromium.org> --- M tests/init_shutdown.c M tests/meson.build M tests/tests.c M tests/tests.h 4 files changed, 56 insertions(+), 0 deletions(-) git pull ssh://review.coreboot.org:29418/flashrom refs/changes/98/52498/1 diff --git a/tests/init_shutdown.c b/tests/init_shutdown.c index 5226860..5882a4b 100644 --- a/tests/init_shutdown.c +++ b/tests/init_shutdown.c @@ -35,3 +35,8 @@ { run_lifecycle(state, PROGRAMMER_DUMMY, "bus=parallel+lpc+fwh+spi"); } + +void linux_spi_init_and_shutdown_test_success(void **state) +{ + run_lifecycle(state, PROGRAMMER_LINUX_SPI, "dev=/dev/null"); +} diff --git a/tests/meson.build b/tests/meson.build index 815ea76..b087f2c 100644 --- a/tests/meson.build +++ b/tests/meson.build @@ -25,6 +25,13 @@ '-Wl,--wrap=physunmap', '-Wl,--wrap=physmap', '-Wl,--wrap=spi_send_command', + '-Wl,--wrap=sio_write', + '-Wl,--wrap=sio_read', + '-Wl,--wrap=open', + '-Wl,--wrap=open64', + '-Wl,--wrap=ioctl', + '-Wl,--wrap=fopen', + '-Wl,--wrap=fopen64', '-Wl,--gc-sections', ] diff --git a/tests/tests.c b/tests/tests.c index 1dc819e..701770f 100644 --- a/tests/tests.c +++ b/tests/tests.c @@ -17,6 +17,7 @@ #include "tests.h" #include <stdio.h> +#include <stdint.h> /* redefinitions/wrapping */ #define LOG_ME printf("%s is called\n", __func__) @@ -32,6 +33,47 @@ return NULL; } +void __wrap_sio_write(uint16_t port, uint8_t reg, uint8_t data) +{ + LOG_ME; +} + +uint8_t __wrap_sio_read(uint16_t port, uint8_t reg) +{ + LOG_ME; + return (uint8_t)mock(); +} + +int __wrap_open(const char *pathname, int flags) +{ + LOG_ME; + return 2021; +} + +int __wrap_open64(const char *pathname, int flags) +{ + LOG_ME; + return 2021; +} + +int __wrap_ioctl(int fd, unsigned long int request, ...) +{ + LOG_ME; + return 2021; +} + +FILE *__wrap_fopen(const char *pathname, const char *mode) +{ + LOG_ME; + return NULL; +} + +FILE *__wrap_fopen64(const char *pathname, const char *mode) +{ + LOG_ME; + return NULL; +} + int main(void) { int ret = 0; @@ -70,6 +112,7 @@ const struct CMUnitTest init_shutdown_tests[] = { cmocka_unit_test(dummy_init_and_shutdown_test_success), + cmocka_unit_test(linux_spi_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 32fed0a..16476c2 100644 --- a/tests/tests.h +++ b/tests/tests.h @@ -42,5 +42,6 @@ /* init_shutdown.c */ void dummy_init_and_shutdown_test_success(void **state); +void linux_spi_init_and_shutdown_test_success(void **state); #endif /* TESTS_H */ -- To view, visit https://review.coreboot.org/c/flashrom/+/52498 To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings Gerrit-Project: flashrom Gerrit-Branch: master Gerrit-Change-Id: I4911fbb6f04371283f0e62d2196bdd691a227584 Gerrit-Change-Number: 52498 Gerrit-PatchSet: 1 Gerrit-Owner: Anastasia Klimchuk <aklm(a)chromium.org> Gerrit-MessageType: newchange

1 0

Change in flashrom[master]: Add unit test to run init/shutdown for dummyflasher.c
by Anastasia Klimchuk (Code Review) 19 Apr '21

19 Apr '21

Anastasia Klimchuk has uploaded this change for review. ( https://review.coreboot.org/c/flashrom/+/52497 ) Change subject: Add unit test to run init/shutdown for dummyflasher.c ...................................................................... Add unit test to run init/shutdown for dummyflasher.c I plan to do this for all drivers in programmer table, one at a time. Dummy is the first and the easiest. After this was rebased on the top of memory checks https://review.coreboot.org/c/flashrom/+/51243 , dummy has been discovered to leak memory on shutdown, so I fix it here. TEST=builds and ninja test BUG=b:181803212 Change-Id: I3c0ef73397f00c1db7aabb5f9f00cb43525af29c Signed-off-by: Anastasia Klimchuk <aklm(a)chromium.org> --- M dummyflasher.c A tests/init_shutdown.c M tests/meson.build M tests/tests.c M tests/tests.h 5 files changed, 47 insertions(+), 0 deletions(-) git pull ssh://review.coreboot.org:29418/flashrom refs/changes/97/52497/1 diff --git a/dummyflasher.c b/dummyflasher.c index 5190282..ec5060d 100644 --- a/dummyflasher.c +++ b/dummyflasher.c @@ -650,6 +650,7 @@ free(flashchip_contents); } #endif + free(data); return 0; } diff --git a/tests/init_shutdown.c b/tests/init_shutdown.c new file mode 100644 index 0000000..5226860 --- /dev/null +++ b/tests/init_shutdown.c @@ -0,0 +1,37 @@ +/* + * This file is part of the flashrom project. + * + * Copyright 2021 Google LLC + * + * This program is free software; you can redistribute it and/or modify + * it under the terms of the GNU General Public License as published by + * the Free Software Foundation; version 2 of the License. + * + * This program is distributed in the hope that it will be useful, + * but WITHOUT ANY WARRANTY; without even the implied warranty of + * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the + * GNU General Public License for more details. + */ + +#include <include/test.h> +#include <string.h> + +#include "programmer.h" + +static void run_lifecycle(void **state, enum programmer prog, const char *param) +{ + (void) state; /* unused */ + + printf("Testing programmer_init for programmer=%u ...\n", prog); + assert_int_equal(0, programmer_init(prog, strdup(param))); + printf("... programmer_init for programmer=%u successful\n", prog); + + printf("Testing programmer_shutdown for programmer=%u ...\n", prog); + assert_int_equal(0, programmer_shutdown()); + printf("... programmer_shutdown for programmer=%u successful\n", prog); +} + +void dummy_init_and_shutdown_test_success(void **state) +{ + run_lifecycle(state, PROGRAMMER_DUMMY, "bus=parallel+lpc+fwh+spi"); +} diff --git a/tests/meson.build b/tests/meson.build index f0cb76d..815ea76 100644 --- a/tests/meson.build +++ b/tests/meson.build @@ -18,6 +18,7 @@ 'helpers.c', 'flashrom.c', 'spi25.c', + 'init_shutdown.c', ] mocks = [ diff --git a/tests/tests.c b/tests/tests.c index 5be4216..1dc819e 100644 --- a/tests/tests.c +++ b/tests/tests.c @@ -68,5 +68,10 @@ }; ret |= cmocka_run_group_tests_name("spi25.c tests", spi25_tests, NULL, NULL); + const struct CMUnitTest init_shutdown_tests[] = { + cmocka_unit_test(dummy_init_and_shutdown_test_success), + }; + ret |= cmocka_run_group_tests_name("init_shutdown.c tests", init_shutdown_tests, NULL, NULL); + return ret; } diff --git a/tests/tests.h b/tests/tests.h index cb905fd..32fed0a 100644 --- a/tests/tests.h +++ b/tests/tests.h @@ -40,4 +40,7 @@ void probe_spi_at25f_test_success(void **state); void probe_spi_st95_test_success(void **state); /* spi95.c */ +/* init_shutdown.c */ +void dummy_init_and_shutdown_test_success(void **state); + #endif /* TESTS_H */ -- To view, visit https://review.coreboot.org/c/flashrom/+/52497 To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings Gerrit-Project: flashrom Gerrit-Branch: master Gerrit-Change-Id: I3c0ef73397f00c1db7aabb5f9f00cb43525af29c Gerrit-Change-Number: 52497 Gerrit-PatchSet: 1 Gerrit-Owner: Anastasia Klimchuk <aklm(a)chromium.org> Gerrit-MessageType: newchange

1 0

Change in flashrom[master]: tests: Add logging to STDOUT when __wrap function is called
by Anastasia Klimchuk (Code Review) 19 Apr '21

19 Apr '21

Anastasia Klimchuk has uploaded this change for review. ( https://review.coreboot.org/c/flashrom/+/52496 ) Change subject: tests: Add logging to STDOUT when __wrap function is called ...................................................................... tests: Add logging to STDOUT when __wrap function is called Calls to __wrap functions are now logged to stdout, makes it easier to understand what’s happening and I find it really helps when writing tests (and can be useful when debugging tests). TEST=builds and ninja test BUG=b:181803212 Change-Id: Ifcef55c9cdb7756c38dcc44fdc57cd88c3d65e70 Signed-off-by: Anastasia Klimchuk <aklm(a)chromium.org> --- M tests/tests.c 1 file changed, 7 insertions(+), 2 deletions(-) git pull ssh://review.coreboot.org:29418/flashrom refs/changes/96/52496/1 diff --git a/tests/tests.c b/tests/tests.c index 3124607..5be4216 100644 --- a/tests/tests.c +++ b/tests/tests.c @@ -19,13 +19,16 @@ #include <stdio.h> /* redefinitions/wrapping */ +#define LOG_ME printf("%s is called\n", __func__) + void __wrap_physunmap(void *virt_addr, size_t len) { - fprintf(stderr, "%s\n", __func__); + LOG_ME; } + void *__wrap_physmap(const char *descr, uintptr_t phys_addr, size_t len) { - fprintf(stderr, "%s\n", __func__); + LOG_ME; return NULL; } @@ -33,6 +36,8 @@ { int ret = 0; + cmocka_set_message_output(CM_OUTPUT_STDOUT); + const struct CMUnitTest helpers_tests[] = { cmocka_unit_test(address_to_bits_test_success), cmocka_unit_test(bitcount_test_success), -- To view, visit https://review.coreboot.org/c/flashrom/+/52496 To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings Gerrit-Project: flashrom Gerrit-Branch: master Gerrit-Change-Id: Ifcef55c9cdb7756c38dcc44fdc57cd88c3d65e70 Gerrit-Change-Number: 52496 Gerrit-PatchSet: 1 Gerrit-Owner: Anastasia Klimchuk <aklm(a)chromium.org> Gerrit-MessageType: newchange

1 0

Change in flashrom[master]: tests: Add unit test to run init/shutdown for mec1308.c
by Anastasia Klimchuk (Code Review) 19 Apr '21

19 Apr '21

Attention is currently required from: Edward O'Callaghan, Angel Pons, Anastasia Klimchuk. Hello build bot (Jenkins), Angel Pons, I'd like you to reexamine a change. Please visit https://review.coreboot.org/c/flashrom/+/51487 to look at the new patch set (#3). Change subject: tests: Add unit test to run init/shutdown for mec1308.c ...................................................................... tests: Add unit test to run init/shutdown for mec1308.c This patch includes mocks for io operations in hwaccess_io.h because those are needed to test lifecycle of mec1308.c BUG=b:181803212 TEST=builds and ninja test Change-Id: I3af612defe1af3850dfc1626a208d873e3a3eddc Signed-off-by: Anastasia Klimchuk <aklm(a)chromium.org> --- A hwaccess_io_unittest.h M meson.build M tests/init_shutdown.c M tests/meson.build M tests/tests.c M tests/tests.h 6 files changed, 107 insertions(+), 0 deletions(-) git pull ssh://review.coreboot.org:29418/flashrom refs/changes/87/51487/3 -- To view, visit https://review.coreboot.org/c/flashrom/+/51487 To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings Gerrit-Project: flashrom Gerrit-Branch: master Gerrit-Change-Id: I3af612defe1af3850dfc1626a208d873e3a3eddc Gerrit-Change-Number: 51487 Gerrit-PatchSet: 3 Gerrit-Owner: Anastasia Klimchuk <aklm(a)chromium.org> Gerrit-Reviewer: Angel Pons <th3fanbus(a)gmail.com> Gerrit-Reviewer: build bot (Jenkins) <no-reply(a)coreboot.org> Gerrit-CC: Edward O'Callaghan <quasisec(a)chromium.org> Gerrit-CC: Nico Huber <nico.h(a)gmx.de> Gerrit-CC: Paul Menzel <paulepanter(a)mailbox.org> Gerrit-Attention: Edward O'Callaghan <quasisec(a)chromium.org> Gerrit-Attention: Angel Pons <th3fanbus(a)gmail.com> Gerrit-Attention: Anastasia Klimchuk <aklm(a)chromium.org> Gerrit-MessageType: newpatchset

1 0

Change in flashrom[master]: lspcon_i2c_spi.c: Rename PAGE_SIZE macro
by Edward O'Callaghan (Code Review) 19 Apr '21

19 Apr '21

Edward O'Callaghan has submitted this change. ( https://review.coreboot.org/c/flashrom/+/52469 ) Change subject: lspcon_i2c_spi.c: Rename PAGE_SIZE macro ...................................................................... lspcon_i2c_spi.c: Rename PAGE_SIZE macro This fixes building with musl libc on alpine:i386-v3.7. Change-Id: Ibd478f3fcd6b6803098035dbdc47f72f6ab3fa6b Signed-off-by: Angel Pons <th3fanbus(a)gmail.com> Reviewed-on: https://review.coreboot.org/c/flashrom/+/52469 Tested-by: build bot (Jenkins) <no-reply(a)coreboot.org> Reviewed-by: Edward O'Callaghan <quasisec(a)chromium.org> Reviewed-by: Nico Huber <nico.h(a)gmx.de> --- M lspcon_i2c_spi.c 1 file changed, 8 insertions(+), 8 deletions(-) Approvals: build bot (Jenkins): Verified Nico Huber: Looks good to me, approved Edward O'Callaghan: Looks good to me, approved diff --git a/lspcon_i2c_spi.c b/lspcon_i2c_spi.c index 7b9f1c0..7599db6 100644 --- a/lspcon_i2c_spi.c +++ b/lspcon_i2c_spi.c @@ -27,7 +27,7 @@ #define REGISTER_ADDRESS (0x94 >> 1) #define PAGE_ADDRESS (0x9e >> 1) -#define PAGE_SIZE 256 +#define LSPCON_PAGE_SIZE 256 #define MAX_SPI_WAIT_RETRIES 1000 #define CLT2_SPI 0x82 @@ -330,7 +330,7 @@ static int lspcon_i2c_spi_map_page(int fd, unsigned int offset) { int ret = 0; - /* Page number byte, need to / PAGE_SIZE. */ + /* Page number byte, need to / LSPCON_PAGE_SIZE. */ ret |= lspcon_i2c_spi_write_register(fd, ROMADDR_BYTE1, (offset >> 8) & 0xff); ret |= lspcon_i2c_spi_write_register(fd, ROMADDR_BYTE2, (offset >> 16)); @@ -349,9 +349,9 @@ if (fd < 0) return SPI_GENERIC_ERROR; - for (i = 0; i < len; i += PAGE_SIZE) { + for (i = 0; i < len; i += LSPCON_PAGE_SIZE) { ret |= lspcon_i2c_spi_map_page(fd, start + i); - ret |= lspcon_i2c_spi_read_data(fd, PAGE_ADDRESS, buf + i, min(len - i, PAGE_SIZE)); + ret |= lspcon_i2c_spi_read_data(fd, PAGE_ADDRESS, buf + i, min(len - i, LSPCON_PAGE_SIZE)); } return ret; @@ -363,8 +363,8 @@ * Using static buffer with maximum possible size, * extra byte is needed for prefixing zero at index 0. */ - uint8_t write_buffer[PAGE_SIZE + 1] = { 0 }; - if (len > PAGE_SIZE) + uint8_t write_buffer[LSPCON_PAGE_SIZE + 1] = { 0 }; + if (len > LSPCON_PAGE_SIZE) return SPI_GENERIC_ERROR; /* First byte represents the writing offset and should always be zero. */ @@ -389,9 +389,9 @@ ret |= lspcon_i2c_spi_enable_hw_write(fd); ret |= lspcon_i2c_clt2_spi_reset(fd); - for (unsigned int i = 0; i < len; i += PAGE_SIZE) { + for (unsigned int i = 0; i < len; i += LSPCON_PAGE_SIZE) { ret |= lspcon_i2c_spi_map_page(fd, start + i); - ret |= lspcon_i2c_spi_write_page(fd, buf + i, min(len - i, PAGE_SIZE)); + ret |= lspcon_i2c_spi_write_page(fd, buf + i, min(len - i, LSPCON_PAGE_SIZE)); } ret |= lspcon_i2c_spi_enable_write_protection(fd); -- To view, visit https://review.coreboot.org/c/flashrom/+/52469 To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings Gerrit-Project: flashrom Gerrit-Branch: master Gerrit-Change-Id: Ibd478f3fcd6b6803098035dbdc47f72f6ab3fa6b Gerrit-Change-Number: 52469 Gerrit-PatchSet: 2 Gerrit-Owner: Angel Pons <th3fanbus(a)gmail.com> Gerrit-Reviewer: Arthur Heymans <arthur(a)aheymans.xyz> 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-MessageType: merged

1 0

2024

2023

2022

2021

2020

2019

2018

2017

flashrom-gerrit April 2021