Change in coreboot[master]: tests: Add device/i2c-test test case

[Name of user not set (Code Review)]

Name of user not set #1002873 has uploaded this change for review. ( https://review.coreboot.org/c/coreboot/+/40539 )

Change subject: tests: Add device/i2c-test test case
......................................................................

tests: Add device/i2c-test test case

Add unit test for src/device/i2c.c module.

This patch is also used as an example for incorporating Cmocka mocking
feature (-wrap linker flag).

Signed-off-by: Jan Dabros 
Change-Id: I2eeb565aacc724ae3b9f5c76ef4b98ef695416d6
---
A tests/device/Makefile.inc
A tests/device/i2c-test.c
2 files changed, 203 insertions(+), 0 deletions(-)

git pull ssh://review.coreboot.org:29418/coreboot refs/changes/39/40539/1

diff --git a/tests/device/Makefile.inc b/tests/device/Makefile.inc
new file mode 100644
index 0000000..ec421af
--- /dev/null
+++ b/tests/device/Makefile.inc
@@ -0,0 +1,18 @@
+##
+## This file is part of the coreboot project.
+##
+## 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.
+##
+
+tests += i2c-test
+
+i2c-test-srcs += tests/device/i2c-test.c
+i2c-test-srcs += src/device/i2c.c
+i2c-test-mocks += platform_i2c_transfer
diff --git a/tests/device/i2c-test.c b/tests/device/i2c-test.c
new file mode 100644
index 0000000..6233f78
--- /dev/null
+++ b/tests/device/i2c-test.c
@@ -0,0 +1,185 @@
+/*
+ * This file is part of the coreboot project.
+ *
+ * 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 <stdarg.h>
+#include <stddef.h>
+#include <setjmp.h>
+#include <cmocka.h>
+
+#include <device/i2c_simple.h>
+
+/* Simulate two i2c devices, both on bus 0, each with three uint8_t regs
+ * implemented.
+ */
+typedef struct {
+	uint8_t reg;
+	uint8_t data;
+} i2c_ex_regs_t;
+
+typedef struct {
+	unsigned int bus;
+	uint8_t slave;
+	i2c_ex_regs_t regs[3];
+} i2c_ex_devs_t;
+
+i2c_ex_devs_t i2c_ex_devs[] = {
+	{0, 0xA, { /* bus, slave */
+		{0x0, 0xB}, /* reg, data */
+		{0x1, 0x6},
+		{0x2, 0xF},
+	} },
+	{0, 0x3, {
+		{0x0, 0xDE},
+		{0x1, 0xAD},
+		{0x2, 0xBE},
+	} },
+};
+
+int __wrap_platform_i2c_transfer(unsigned int bus, struct i2c_msg *segments,
+	int count)
+{
+	int i;
+	int reg;
+	struct i2c_msg *tmp = segments;
+	i2c_ex_devs_t *i2c_dev = NULL;
+
+	for (i = 0; i < count; i++, segments++) {
+		check_expected_ptr(segments->buf);
+		check_expected(segments->flags);
+	}
+
+	reg = tmp->buf[0];
+
+	/* Find object for requested device */
+	for (i = 0; i < ARRAY_SIZE(i2c_ex_devs); i++, i2c_dev++)
+		if (i2c_ex_devs[i].slave == tmp->slave) {
+			i2c_dev = &i2c_ex_devs[i];
+			break;
+		}
+
+	if (i2c_dev == NULL)
+		return -1;
+
+	/* Write commands */
+	if (tmp->len > 1) {
+		i2c_dev->regs[reg].data = tmp->buf[1];
+	};
+
+	/* Read commands */
+	for (i = 0; i < count; i++, tmp++)
+		if (tmp->flags & I2C_M_RD) {
+			*(tmp->buf) = i2c_dev->regs[reg].data;
+		};
+}
+
+static void mock_check_params(void)
+{
+	unsigned long int expected_flags[] = {0, I2C_M_RD, I2C_M_TEN,
+		I2C_M_RECV_LEN, I2C_M_NOSTART};
+
+	/* Flags should always be only within supported range */
+	expect_in_set_count(__wrap_platform_i2c_transfer, segments->flags,
+		expected_flags, -1);
+
+	expect_not_value_count(__wrap_platform_i2c_transfer, segments->buf,
+		NULL, -1);
+}
+
+#define MASK	0x3
+#define SHIFT	0x1
+
+static void i2c_read_field_test(void **state)
+{
+	int bus, slave, reg;
+	int i, j;
+	uint8_t buf;
+
+	mock_check_params();
+
+	/* Read particular bits in all registers in all devices, then compare
+	 * with expected value.
+	 */
+	for (i = 0; i < ARRAY_SIZE(i2c_ex_devs); i++)
+		for (j = 0; j < ARRAY_SIZE(i2c_ex_devs[0].regs); j++) {
+			i2c_read_field(i2c_ex_devs[i].bus,
+				i2c_ex_devs[i].slave,
+				i2c_ex_devs[i].regs[j].reg,
+				&buf, MASK, SHIFT);
+			assert_int_equal((i2c_ex_devs[i].regs[j].data &
+				(MASK << SHIFT)) >> SHIFT, buf);
+		};
+
+	/* Read whole registers */
+	for (i = 0; i < ARRAY_SIZE(i2c_ex_devs); i++)
+		for (j = 0; j < ARRAY_SIZE(i2c_ex_devs[0].regs); j++) {
+			i2c_read_field(i2c_ex_devs[i].bus,
+				i2c_ex_devs[i].slave,
+				i2c_ex_devs[i].regs[j].reg,
+				&buf, 0xFF, 0);
+			assert_int_equal(i2c_ex_devs[i].regs[j].data, buf);
+		};
+}
+
+static void i2c_write_field_test(void **state)
+{
+	int bus, slave, reg;
+	int i, j;
+	uint8_t buf, tmp;
+
+	mock_check_params();
+
+	/* Clear particular bits in all registers in all devices, then compare
+	 * with expected value.
+	 */
+	for (i = 0; i < ARRAY_SIZE(i2c_ex_devs); i++)
+		for (j = 0; j < ARRAY_SIZE(i2c_ex_devs[0].regs); j++) {
+			buf = 0x0;
+			tmp = i2c_ex_devs[i].regs[j].data;
+			i2c_write_field(i2c_ex_devs[i].bus,
+				i2c_ex_devs[i].slave,
+				i2c_ex_devs[i].regs[j].reg,
+				buf, MASK, SHIFT);
+			assert_int_equal(i2c_ex_devs[i].regs[j].data,
+				(tmp & ~(MASK << SHIFT)) | (buf << SHIFT));
+			i2c_read_field(i2c_ex_devs[i].bus,
+				i2c_ex_devs[i].slave,
+				i2c_ex_devs[i].regs[j].reg,
+				&buf, MASK, SHIFT);
+		};
+
+	/* Set all bits in all registers, this time verify using
+	 * i2c_read_field() accessor.
+	 */
+	for (i = 0; i < ARRAY_SIZE(i2c_ex_devs); i++)
+		for (j = 0; j < ARRAY_SIZE(i2c_ex_devs[0].regs); j++) {
+			i2c_write_field(i2c_ex_devs[i].bus,
+				i2c_ex_devs[i].slave,
+				i2c_ex_devs[i].regs[j].reg,
+				0xFF, 0xFF, 0);
+			i2c_read_field(i2c_ex_devs[i].bus,
+				i2c_ex_devs[i].slave,
+				i2c_ex_devs[i].regs[j].reg,
+				&buf, 0xFF, 0);
+			assert_int_equal(buf, 0xFF);
+		};
+}
+
+int main(void)
+{
+	const struct CMUnitTest tests[] = {
+		cmocka_unit_test(i2c_read_field_test),
+		cmocka_unit_test(i2c_write_field_test)
+	};
+
+	return cmocka_run_group_tests(tests, NULL, NULL);
+}

[Name of user not set (Code Review)]

Hello build bot (Jenkins), Patrick Georgi, Martin Roth,

I'd like you to reexamine a change. Please visit

https://review.coreboot.org/c/coreboot/+/40539

to look at the new patch set (#2).

Change subject: tests: Add device/i2c-test test case
......................................................................

tests: Add device/i2c-test test case

Add unit test for src/device/i2c.c module.

This patch is also used as an example for incorporating Cmocka mocking
feature (-wrap linker flag).

Signed-off-by: Jan Dabros 
Change-Id: I2eeb565aacc724ae3b9f5c76ef4b98ef695416d6
---
A tests/device/Makefile.inc
A tests/device/i2c-test.c
2 files changed, 203 insertions(+), 0 deletions(-)

git pull ssh://review.coreboot.org:29418/coreboot refs/changes/39/40539/2

[Julius Werner (Code Review)]

Julius Werner has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/40539 )

Change subject: tests: Add device/i2c-test test case
......................................................................

Patch Set 2: Code-Review+2

(1 comment)

https://review.coreboot.org/c/coreboot/+/40539/1/tests/device/i2c-test.c 
File tests/device/i2c-test.c:

https://review.coreboot.org/c/coreboot/+/40539/1/tests/device/i2c-test.c@36 
PS1, Line 36: 	{0, 0xA, { /* bus, slave */
nit: Rather than comments, we usually make struct initializers more readable like this:

...devs[] = {
   { .bus = 0, .slave = 0xa, .regs = {
     { .reg = 0x0, .data = 0xb },
     ...

[Name of user not set (Code Review)]

Hello build bot (Jenkins), Patrick Georgi, Martin Roth, Julius Werner,

I'd like you to reexamine a change. Please visit

https://review.coreboot.org/c/coreboot/+/40539

to look at the new patch set (#3).

Change subject: tests: Add device/i2c-test test case
......................................................................

tests: Add device/i2c-test test case

Add unit test for src/device/i2c.c module.

This patch is also used as an example for incorporating Cmocka mocking
feature (-wrap linker flag).

Signed-off-by: Jan Dabros 
Change-Id: I2eeb565aacc724ae3b9f5c76ef4b98ef695416d6
---
A tests/device/Makefile.inc
A tests/device/i2c-test.c
2 files changed, 199 insertions(+), 0 deletions(-)

git pull ssh://review.coreboot.org:29418/coreboot refs/changes/39/40539/3

[Name of user not set (Code Review)]

Name of user not set #1002873 has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/40539 )

Change subject: tests: Add device/i2c-test test case
......................................................................

Patch Set 3:

(2 comments)

https://review.coreboot.org/c/coreboot/+/40539/1/tests/device/i2c-test.c 
File tests/device/i2c-test.c:

https://review.coreboot.org/c/coreboot/+/40539/1/tests/device/i2c-test.c@36 
PS1, Line 36: 	{0, 0xA, { /* bus, slave */
> nit: Rather than comments, we usually make struct initializers more readable like this: […]

Will fix this in next version. I'm also adding an additional change with removal of unnecessary code. See comment below.

https://review.coreboot.org/c/coreboot/+/40539/1/tests/device/i2c-test.c@154 
PS1, Line 154: 			i2c_read_field(i2c_ex_devs[i].bus,
             : 				i2c_ex_devs[i].slave,
             : 				i2c_ex_devs[i].regs[j].reg,
             : 				&buf, MASK, SHIFT);
             : 		};
This block is actually unused here. Should have removed this during cleanup. Will fix in v4.

[Paul Fagerburg (Code Review)]

Paul Fagerburg has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/40539 )

Change subject: tests: Add device/i2c-test test case
......................................................................

Patch Set 3:

(3 comments)

https://review.coreboot.org/c/coreboot/+/40539/3/tests/device/i2c-test.c 
File tests/device/i2c-test.c:

https://review.coreboot.org/c/coreboot/+/40539/3/tests/device/i2c-test.c@48 
PS3, Line 48: int __wrap_platform_i2c_transfer(unsigned int bus, struct i2c_msg *segments,
Can you put this mock into a separate C file? In general, we would like mocks to be available to multiple unit tests without having to copy them, so it would be good to show the reader how to do it and if there are any issues they need to be aware of.

https://review.coreboot.org/c/coreboot/+/40539/3/tests/device/i2c-test.c@85 
PS3, Line 85: mock_check_params
The name of this function is a little confusing. It isn't checking the parameters, it is setting expectations for the parameters (which will get checked later). Consider changing to mock_expect_params_i2c_transfer() or something like that?

BTW, "There are only two hard things in Computer Science: cache invalidation and naming things." -- Phil Karlton

https://review.coreboot.org/c/coreboot/+/40539/3/tests/device/i2c-test.c@96 
PS3, Line 96: }
I would also add an expectation that the 'count' parameter is 1 or more.

[Julius Werner (Code Review)]

Julius Werner has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/40539 )

Change subject: tests: Add device/i2c-test test case
......................................................................

Patch Set 3:

(1 comment)

https://review.coreboot.org/c/coreboot/+/40539/3/tests/device/i2c-test.c 
File tests/device/i2c-test.c:

https://review.coreboot.org/c/coreboot/+/40539/3/tests/device/i2c-test.c@48 
PS3, Line 48: int __wrap_platform_i2c_transfer(unsigned int bus, struct i2c_msg *segments,
> Can you put this mock into a separate C file? In general, we would like mocks to be available to mul […]

I don't think I agree with this, at least not as a general rule. Sometimes the same function will need to be mocked in multiple tests, yes, but that doesn't mean they will actually be the same mocks. The mock often has to be carefully designed to match the thing you want to test. In this case for example, if we wrote tests for the TPM I2C driver we might also want to mock platform_i2c_transfer(), but it would have to be a more complicated mock that is able to return answers like a real TPM.

I'm sure we will have cases where we want to share mocks, but I wouldn't want to force that on every mock written for every test (that just seems to create unnecessary friction which could keep people from wanting to write tests). If we want to demonstrate a test with a shared mock for others to copy, let's first find a situation where that would actually be useful and then implement it there.

[Paul Fagerburg (Code Review)]

Paul Fagerburg has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/40539 )

Change subject: tests: Add device/i2c-test test case
......................................................................

Patch Set 3:

(1 comment)

https://review.coreboot.org/c/coreboot/+/40539/3/tests/device/i2c-test.c 
File tests/device/i2c-test.c:

https://review.coreboot.org/c/coreboot/+/40539/3/tests/device/i2c-test.c@48 
PS3, Line 48: int __wrap_platform_i2c_transfer(unsigned int bus, struct i2c_msg *segments,
> I don't think I agree with this, at least not as a general rule. […]

OK, then I guess this one is OK for here. But we should probably look at making a future example where two separate tests share one mock, to demonstrate the concept.

[Name of user not set (Code Review)]

Hello build bot (Jenkins), Patrick Georgi, Martin Roth, Julius Werner,

I'd like you to reexamine a change. Please visit

https://review.coreboot.org/c/coreboot/+/40539

to look at the new patch set (#4).

Change subject: tests: Add device/i2c-test test case
......................................................................

tests: Add device/i2c-test test case

Add unit test for src/device/i2c.c module.

This patch is also used as an example for incorporating Cmocka mocking
feature (-wrap linker flag).

Signed-off-by: Jan Dabros 
Change-Id: I2eeb565aacc724ae3b9f5c76ef4b98ef695416d6
---
A tests/device/Makefile.inc
A tests/device/i2c-test.c
2 files changed, 205 insertions(+), 0 deletions(-)

git pull ssh://review.coreboot.org:29418/coreboot refs/changes/39/40539/4

[Name of user not set (Code Review)]

Name of user not set #1002873 has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/40539 )

Change subject: tests: Add device/i2c-test test case
......................................................................

Patch Set 4:

(3 comments)

https://review.coreboot.org/c/coreboot/+/40539/3/tests/device/i2c-test.c 
File tests/device/i2c-test.c:

https://review.coreboot.org/c/coreboot/+/40539/3/tests/device/i2c-test.c@48 
PS3, Line 48: int __wrap_platform_i2c_transfer(unsigned int bus, struct i2c_msg *segments,
> OK, then I guess this one is OK for here. […]

This is important thing, thanks for bringing this up.

Yes, let's keep this flexible in the meaning that we shouldn't force anything on the dev. It's sometimes better to keep test really simple, than use "overcomplicated" mock.

That being said, I would say that sharing mocks should be a default, to avoid code duplication. I think that in future will come up with some implementation were mock will be shared. This will be valuable example. Resolving this comment for now.

https://review.coreboot.org/c/coreboot/+/40539/3/tests/device/i2c-test.c@85 
PS3, Line 85: mock_check_params
> The name of this function is a little confusing. […]

:)

Good point. I would like to avoid too long name "trains", but the only candidate which sounds good to me and is addressing your comment is mock_expect_params_platform_i2c_transfer().. Even though I don't like it, at least it is really telling what the function is doing.

https://review.coreboot.org/c/coreboot/+/40539/3/tests/device/i2c-test.c@96 
PS3, Line 96: }
> I would also add an expectation that the 'count' parameter is 1 or more.

Done

[Paul Fagerburg (Code Review)]

Paul Fagerburg has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/40539 )

Change subject: tests: Add device/i2c-test test case
......................................................................

Patch Set 4: Code-Review+1

[Stefan Reinauer (Code Review)]

Stefan Reinauer has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/40539 )

Change subject: tests: Add device/i2c-test test case
......................................................................

Patch Set 4: Code-Review+2

[Name of user not set (Code Review)]

Hello build bot (Jenkins), Patrick Georgi, Martin Roth, Stefan Reinauer, Paul Fagerburg, Julius Werner,

I'd like you to reexamine a change. Please visit

https://review.coreboot.org/c/coreboot/+/40539

to look at the new patch set (#5).

Change subject: tests: Add device/i2c-test test case
......................................................................

tests: Add device/i2c-test test case

Add unit test for src/device/i2c.c module.

This patch is also used as an example for incorporating Cmocka mocking
feature (-wrap linker flag).

Signed-off-by: Jan Dabros 
Change-Id: I2eeb565aacc724ae3b9f5c76ef4b98ef695416d6
---
A tests/device/Makefile.inc
A tests/device/i2c-test.c
2 files changed, 201 insertions(+), 0 deletions(-)

git pull ssh://review.coreboot.org:29418/coreboot refs/changes/39/40539/5

[Julius Werner (Code Review)]

Julius Werner has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/40539 )

Change subject: tests: Add device/i2c-test test case
......................................................................

Patch Set 5: Code-Review+2

[Paul Fagerburg (Code Review)]

Paul Fagerburg has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/40539 )

Change subject: tests: Add device/i2c-test test case
......................................................................

Patch Set 5: Code-Review+2

[Patrick Georgi (Code Review)]

Patrick Georgi has submitted this change. ( https://review.coreboot.org/c/coreboot/+/40539 )

Change subject: tests: Add device/i2c-test test case
......................................................................

tests: Add device/i2c-test test case

Add unit test for src/device/i2c.c module.

This patch is also used as an example for incorporating Cmocka mocking
feature (-wrap linker flag).

Signed-off-by: Jan Dabros 
Change-Id: I2eeb565aacc724ae3b9f5c76ef4b98ef695416d6
Reviewed-on: https://review.coreboot.org/c/coreboot/+/40539
Tested-by: build bot (Jenkins) 
Reviewed-by: Julius Werner 
Reviewed-by: Paul Fagerburg 
---
A tests/device/Makefile.inc
A tests/device/i2c-test.c
2 files changed, 201 insertions(+), 0 deletions(-)

Approvals:
  build bot (Jenkins): Verified
  Julius Werner: Looks good to me, approved
  Paul Fagerburg: Looks good to me, approved

diff --git a/tests/device/Makefile.inc b/tests/device/Makefile.inc
new file mode 100644
index 0000000..f23e72f
--- /dev/null
+++ b/tests/device/Makefile.inc
@@ -0,0 +1,18 @@
+##
+## This file is part of the coreboot project.
+##
+## 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.
+##
+
+tests-y += i2c-test
+
+i2c-test-srcs += tests/device/i2c-test.c
+i2c-test-srcs += src/device/i2c.c
+i2c-test-mocks += platform_i2c_transfer
diff --git a/tests/device/i2c-test.c b/tests/device/i2c-test.c
new file mode 100644
index 0000000..16e4d0d
--- /dev/null
+++ b/tests/device/i2c-test.c
@@ -0,0 +1,183 @@
+/*
+ * This file is part of the coreboot project.
+ *
+ * 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 <stdarg.h>
+#include <stddef.h>
+#include <setjmp.h>
+#include <limits.h>
+#include <cmocka.h>
+
+#include <device/i2c_simple.h>
+
+/* Simulate two i2c devices, both on bus 0, each with three uint8_t regs
+   implemented. */
+typedef struct {
+	uint8_t reg;
+	uint8_t data;
+} i2c_ex_regs_t;
+
+typedef struct {
+	unsigned int bus;
+	uint8_t slave;
+	i2c_ex_regs_t regs[3];
+} i2c_ex_devs_t;
+
+i2c_ex_devs_t i2c_ex_devs[] = {
+	{.bus = 0, .slave = 0xA, .regs = {
+		{.reg = 0x0, .data = 0xB},
+		{.reg = 0x1, .data = 0x6},
+		{.reg = 0x2, .data = 0xF},
+	} },
+	{.bus = 0, .slave = 0x3, .regs = {
+		{.reg = 0x0, .data = 0xDE},
+		{.reg = 0x1, .data = 0xAD},
+		{.reg = 0x2, .data = 0xBE},
+	} },
+};
+
+int __wrap_platform_i2c_transfer(unsigned int bus, struct i2c_msg *segments,
+	int count)
+{
+	int i;
+	int reg;
+	struct i2c_msg *tmp = segments;
+	i2c_ex_devs_t *i2c_dev = NULL;
+
+	check_expected(count);
+
+	for (i = 0; i < count; i++, segments++) {
+		check_expected_ptr(segments->buf);
+		check_expected(segments->flags);
+	}
+
+	reg = tmp->buf[0];
+
+	/* Find object for requested device */
+	for (i = 0; i < ARRAY_SIZE(i2c_ex_devs); i++, i2c_dev++)
+		if (i2c_ex_devs[i].slave == tmp->slave) {
+			i2c_dev = &i2c_ex_devs[i];
+			break;
+		}
+
+	if (i2c_dev == NULL)
+		return -1;
+
+	/* Write commands */
+	if (tmp->len > 1) {
+		i2c_dev->regs[reg].data = tmp->buf[1];
+	};
+
+	/* Read commands */
+	for (i = 0; i < count; i++, tmp++)
+		if (tmp->flags & I2C_M_RD) {
+			*(tmp->buf) = i2c_dev->regs[reg].data;
+		};
+}
+
+static void mock_expect_params_platform_i2c_transfer(void)
+{
+	unsigned long int expected_flags[] = {0, I2C_M_RD, I2C_M_TEN,
+		I2C_M_RECV_LEN, I2C_M_NOSTART};
+
+	/* Flags should always be only within supported range */
+	expect_in_set_count(__wrap_platform_i2c_transfer, segments->flags,
+		expected_flags, -1);
+
+	expect_not_value_count(__wrap_platform_i2c_transfer, segments->buf,
+		NULL, -1);
+
+	expect_in_range_count(__wrap_platform_i2c_transfer, count, 1, INT_MAX,
+		-1);
+}
+
+#define MASK	0x3
+#define SHIFT	0x1
+
+static void i2c_read_field_test(void **state)
+{
+	int bus, slave, reg;
+	int i, j;
+	uint8_t buf;
+
+	mock_expect_params_platform_i2c_transfer();
+
+	/* Read particular bits in all registers in all devices, then compare
+	   with expected value. */
+	for (i = 0; i < ARRAY_SIZE(i2c_ex_devs); i++)
+		for (j = 0; j < ARRAY_SIZE(i2c_ex_devs[0].regs); j++) {
+			i2c_read_field(i2c_ex_devs[i].bus,
+				i2c_ex_devs[i].slave,
+				i2c_ex_devs[i].regs[j].reg,
+				&buf, MASK, SHIFT);
+			assert_int_equal((i2c_ex_devs[i].regs[j].data &
+				(MASK << SHIFT)) >> SHIFT, buf);
+		};
+
+	/* Read whole registers */
+	for (i = 0; i < ARRAY_SIZE(i2c_ex_devs); i++)
+		for (j = 0; j < ARRAY_SIZE(i2c_ex_devs[0].regs); j++) {
+			i2c_read_field(i2c_ex_devs[i].bus,
+				i2c_ex_devs[i].slave,
+				i2c_ex_devs[i].regs[j].reg,
+				&buf, 0xFF, 0);
+			assert_int_equal(i2c_ex_devs[i].regs[j].data, buf);
+		};
+}
+
+static void i2c_write_field_test(void **state)
+{
+	int bus, slave, reg;
+	int i, j;
+	uint8_t buf, tmp;
+
+	mock_expect_params_platform_i2c_transfer();
+
+	/* Clear particular bits in all registers in all devices, then compare
+	   with expected value. */
+	for (i = 0; i < ARRAY_SIZE(i2c_ex_devs); i++)
+		for (j = 0; j < ARRAY_SIZE(i2c_ex_devs[0].regs); j++) {
+			buf = 0x0;
+			tmp = i2c_ex_devs[i].regs[j].data;
+			i2c_write_field(i2c_ex_devs[i].bus,
+				i2c_ex_devs[i].slave,
+				i2c_ex_devs[i].regs[j].reg,
+				buf, MASK, SHIFT);
+			assert_int_equal(i2c_ex_devs[i].regs[j].data,
+				(tmp & ~(MASK << SHIFT)) | (buf << SHIFT));
+		};
+
+	/* Set all bits in all registers, this time verify using
+	   i2c_read_field() accessor. */
+	for (i = 0; i < ARRAY_SIZE(i2c_ex_devs); i++)
+		for (j = 0; j < ARRAY_SIZE(i2c_ex_devs[0].regs); j++) {
+			i2c_write_field(i2c_ex_devs[i].bus,
+				i2c_ex_devs[i].slave,
+				i2c_ex_devs[i].regs[j].reg,
+				0xFF, 0xFF, 0);
+			i2c_read_field(i2c_ex_devs[i].bus,
+				i2c_ex_devs[i].slave,
+				i2c_ex_devs[i].regs[j].reg,
+				&buf, 0xFF, 0);
+			assert_int_equal(buf, 0xFF);
+		};
+}
+
+int main(void)
+{
+	const struct CMUnitTest tests[] = {
+		cmocka_unit_test(i2c_read_field_test),
+		cmocka_unit_test(i2c_write_field_test)
+	};
+
+	return cmocka_run_group_tests(tests, NULL, NULL);
+}

[Patrick Georgi (Code Review)]

Patrick Georgi has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/40539 )

Change subject: tests: Add device/i2c-test test case
......................................................................

Patch Set 6:

Jan, now that these changes are in, could you prepare a commit that adds some words about this new testing facility to Documentation/releases/coreboot-4.12-relnotes.md?

[Name of user not set (Code Review)]

Name of user not set #1002873 has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/40539 )

Change subject: tests: Add device/i2c-test test case
......................................................................

Patch Set 6:
> Patch Set 6:
>
> Jan, now that these changes are in, could you prepare a commit that adds some words about this new testing facility to Documentation/releases/coreboot-4.12-relnotes.md?

Hi Patrick, thanks for merging. I've just sent relnotes change for review - https://review.coreboot.org/c/coreboot/+/41025