[flashrom-gerrit] [M] Change in flashrom[master]: tests/parade_lspcon.c: Add emulation for ioctl, read, write operations

[Anastasia Klimchuk (Code Review)]

Anastasia Klimchuk has submitted this change. ( https://review.coreboot.org/c/flashrom/+/67161 )

Change subject: tests/parade_lspcon.c: Add emulation for ioctl, read, write operations
......................................................................

tests/parade_lspcon.c: Add emulation for ioctl, read, write operations

Previously, parade lspcon unit test had no custom emulation and
was running basic lifecycle with default mocks (default mocks do
nothing and return success). I have discovered that it does not
work in all environments.

Specifically, in functions `parade_lspcon_wait_command_done`
and `parade_lspcon_wait_rom_free` there is a local variable
`uint8_t val` which is declared and not explicitly initialised. It can
get different initial values depending on the environment/toolchain
(for example gcc vs clang).
Later during the code execution, this variable is used as a holder
for writing/reading data from register(s). For unit test, reading and
writing data from registers is emulated. With default mocks, initial
value of variable was propagated further as if it was "read" from
a register. So the unit test could pass or fail depending on the
initial value of local variable, which in turn depends on environment
and toolchain.

If initial value was 0 the test reliably passed (this is the case with
upstream build environment, locally I have gcc 11.3.0 on x86_64).
If it was 1 the test reliably failed (this is the case of chromium
flashrom tree, this is clang 15.0 under chroot, tested on x86_64 and
arm boards).
If it was any other value then it could be anything.

This patch adds custom mocks for ioctl/read/write operations
and emulates a successful scenario of running a lifecycle. Local
variable is initialised by reading from the register, same as it
happens in the real (non-test) run for this programmer.

BUG=b:242816982
TEST=ninja test

Change-Id: I98f52507a0ddbbfbeb390038d14192cacc2be683
Signed-off-by: Anastasia Klimchuk 
Reviewed-on: https://review.coreboot.org/c/flashrom/+/67161
Reviewed-by: Peter Marheine 
Tested-by: build bot (Jenkins) 
Reviewed-by: Felix Singer 
---
M tests/parade_lspcon.c
1 file changed, 133 insertions(+), 0 deletions(-)

Approvals:
  build bot (Jenkins): Verified
  Felix Singer: Looks good to me, approved
  Peter Marheine: Looks good to me, but someone else must approve

diff --git a/tests/parade_lspcon.c b/tests/parade_lspcon.c
index 478e296..a1c9f72 100644
--- a/tests/parade_lspcon.c
+++ b/tests/parade_lspcon.c
@@ -17,14 +17,100 @@

#if CONFIG_PARADE_LSPCON == 1

+/* Same macros as in parade_lspcon.c programmer. */
+/* FIXME(aklm): should driver register maps be defined in `include/drivers/` for sharing with tests? */
+#define REGISTER_ADDRESS                        0x4a
+#define SPISTATUS                               0x9e
+#define SPISTATUS_SECTOR_ERASE_FINISHED         0
+#define SWSPICTL                                0x93
+#define SWSPICTL_ENABLE_READBACK                0x8
+#define SWSPI_RDATA                             0x91
+/* Macros for test run. */
+#define DATA_TO_READ                            0
+#define MAX_REG_BUF_LEN                         2
+
+struct parade_lspcon_io_state {
+	unsigned long addr;                     /* Address to read and write */
+	uint8_t reg_buf[MAX_REG_BUF_LEN];       /* Last value written to the register address */
+};
+
+static int parade_lspcon_ioctl(void *state, int fd, unsigned long request, va_list args)
+{
+	struct parade_lspcon_io_state *io_state = state;
+	if (request == I2C_SLAVE)
+		/* Addr is the next (and the only) argument in the parameters list for this ioctl call. */
+		io_state->addr = va_arg(args, unsigned long);
+
+	return 0;
+}
+
+static int parade_lspcon_read(void *state, int fd, void *buf, size_t sz)
+{
+	struct parade_lspcon_io_state *io_state = state;
+
+	/*
+	 * Parade_lspcon programmer has operations over register address and
+	 * page address. In the current emulation for basic lifecycle we need
+	 * to emulate operations over register address. Page address can do
+	 * nothing for now, and just return successful return value.
+	 *
+	 * For future, if this unit test is upgraded to run probing lifecycle,
+	 * page address operations might need to be fully emulated.
+	 */
+	if (io_state->addr != REGISTER_ADDRESS)
+		return sz;
+
+	assert_int_equal(sz, 1);
+
+	switch (io_state->reg_buf[0]) {
+	case SPISTATUS:
+		memset(buf, SPISTATUS_SECTOR_ERASE_FINISHED, sz);
+		break;
+	case SWSPICTL:
+		memset(buf, SWSPICTL_ENABLE_READBACK, sz);
+		break;
+	case SWSPI_RDATA:
+		memset(buf, DATA_TO_READ, sz);
+		break;
+	default:
+		memset(buf, 0, sz);
+		break;
+	}
+
+	return sz;
+}
+
+static int parade_lspcon_write(void *state, int fd, const void *buf, size_t sz)
+{
+	struct parade_lspcon_io_state *io_state = state;
+
+	/*
+	 * Only register address operations are needed to be emulated for basic lifecycle.
+	 * See also comment in `parade_lspcon_read`.
+	 */
+	if (io_state->addr != REGISTER_ADDRESS)
+		return sz;
+
+	assert_true(sz <= MAX_REG_BUF_LEN);
+
+	memcpy(io_state->reg_buf, buf, sz);
+
+	return sz;
+}
+
 void parade_lspcon_basic_lifecycle_test_success(void **state)
 {
+	struct parade_lspcon_io_state parade_lspcon_io_state = { 0 };
    struct io_mock_fallback_open_state parade_lspcon_fallback_open_state = {
    	.noc = 0,
    	.paths = { "/dev/i2c-254", NULL },
    	.flags = { O_RDWR },
    };
    const struct io_mock parade_lspcon_io = {
+		.state = &parade_lspcon_io_state,
+		.ioctl = parade_lspcon_ioctl,
+		.read = parade_lspcon_read,
+		.write = parade_lspcon_write,
    	.fallback_open_state = &parade_lspcon_fallback_open_state,
    };