2022-06-15 05:17:19

by Tzung-Bi Shih

[permalink] [raw]
Subject: [PATCH 00/11] platform/chrome: Kunit tests and refactor for cros_ec_cmd_xfer()

The first 5 patches add Kunit tests and refactors for cros_ec_cmd_xfer().

The last 6 patches change the behavior a bit by altering return codes.


Asynchronous mechanism in EC protocol:
EC returns EC_RES_IN_PROGRESS if the host command needs more time to complete.
It saves the result into `saved_result` once the command completed[1].

By design, AP should send another command EC_CMD_RESEND_RESPONSE for getting
the result from the previous pending command[2]. The mechanism was only
designed for commands that don't need responses[3].

However, the kernel code doesn't have such logic when dealing with
EC_RES_IN_PROGRESS.

The series doesn't fix it but leave it as is. I doubt there is no existing
use case.

[1]: https://crrev.com/9b80051e01872d8cae86fff999b5d31a0bea985b/common/host_command.c#113
[2]: https://crrev.com/9b80051e01872d8cae86fff999b5d31a0bea985b/common/host_command.c#748
[3]: https://crrev.com/9b80051e01872d8cae86fff999b5d31a0bea985b/common/host_command.c#126


Return value overridden in cros_ec_send_command() if asynchronous:
By original intention, cros_ec_send_command() returns number of available
bytes of input payload.

When it falls into asynchronous path (i.e. EC_RES_IN_PROGRESS), both return
value and `msg->result` will be overridden by the subsequent
EC_CMD_GET_COMMS_STATUS.

The series doesn't fix it but leave it as is.

Tzung-Bi Shih (11):
platform/chrome: cros_ec_proto: add "cros_ec_" prefix to
send_command()
platform/chrome: cros_ec_proto: add Kunit tests for cros_ec_cmd_xfer()
platform/chrome: cros_ec_proto: add Kunit tests for
cros_ec_send_command()
platform/chrome: cros_ec_proto: separate cros_ec_xfer_command()
platform/chrome: cros_ec_proto: separate cros_ec_wait_until_complete()
platform/chrome: cros_ec_proto: change Kunit expectation when timed
out
platform/chrome: cros_ec_proto: return -EAGAIN when retries timed out
platform/chrome: cros_ec_proto: change Kunit expectation for EC errors
platform/chrome: cros_ec_proto: return standard error codes for EC
errors
platform/chrome: cros_ec_proto: add Kunit test for empty payload
platform/chrome: cros_ec_proto: return -EPROTO if empty payload

drivers/platform/chrome/cros_ec_proto.c | 106 +++--
drivers/platform/chrome/cros_ec_proto_test.c | 443 +++++++++++++++++++
drivers/platform/chrome/cros_kunit_util.c | 20 +
drivers/platform/chrome/cros_kunit_util.h | 4 +
4 files changed, 527 insertions(+), 46 deletions(-)

--
2.36.1.476.g0c4daa206d-goog


2022-06-15 05:23:48

by Tzung-Bi Shih

[permalink] [raw]
Subject: [PATCH 11/11] platform/chrome: cros_ec_proto: return -EPROTO if empty payload

cros_ec_wait_until_complete() sends EC_CMD_GET_COMMS_STATUS which expects
to receive sizeof(struct ec_response_get_comms_status) from
cros_ec_xfer_command().

Return -EPROTO if cros_ec_xfer_command() returns 0.

Signed-off-by: Tzung-Bi Shih <[email protected]>
---
drivers/platform/chrome/cros_ec_proto.c | 5 +++++
1 file changed, 5 insertions(+)

diff --git a/drivers/platform/chrome/cros_ec_proto.c b/drivers/platform/chrome/cros_ec_proto.c
index 5323edddb540..0c7042aa2640 100644
--- a/drivers/platform/chrome/cros_ec_proto.c
+++ b/drivers/platform/chrome/cros_ec_proto.c
@@ -166,6 +166,11 @@ static int cros_ec_wait_until_complete(struct cros_ec_device *ec_dev, uint32_t *
break;
}

+ if (ret == 0) {
+ ret = -EPROTO;
+ break;
+ }
+
if (!(status->flags & EC_COMMS_STATUS_PROCESSING))
break;
}
--
2.36.1.476.g0c4daa206d-goog

2022-06-15 05:26:51

by Tzung-Bi Shih

[permalink] [raw]
Subject: [PATCH 04/11] platform/chrome: cros_ec_proto: separate cros_ec_xfer_command()

cros_ec_send_command() has extra logic to handle EC_RES_IN_PROGRESS.
Separate the command transfer part into cros_ec_xfer_command() so
that other functions can re-use it.

Signed-off-by: Tzung-Bi Shih <[email protected]>
---
drivers/platform/chrome/cros_ec_proto.c | 13 ++++++++++---
1 file changed, 10 insertions(+), 3 deletions(-)

diff --git a/drivers/platform/chrome/cros_ec_proto.c b/drivers/platform/chrome/cros_ec_proto.c
index b02fd1414e52..0cec013be3d3 100644
--- a/drivers/platform/chrome/cros_ec_proto.c
+++ b/drivers/platform/chrome/cros_ec_proto.c
@@ -107,7 +107,7 @@ static int prepare_tx_legacy(struct cros_ec_device *ec_dev,
return EC_MSG_TX_PROTO_BYTES + msg->outsize;
}

-static int cros_ec_send_command(struct cros_ec_device *ec_dev, struct cros_ec_command *msg)
+static int cros_ec_xfer_command(struct cros_ec_device *ec_dev, struct cros_ec_command *msg)
{
int ret;
int (*xfer_fxn)(struct cros_ec_device *ec, struct cros_ec_command *msg);
@@ -123,14 +123,21 @@ static int cros_ec_send_command(struct cros_ec_device *ec_dev, struct cros_ec_co
* the EC is trying to use protocol v2, on an underlying
* communication mechanism that does not support v2.
*/
- dev_err_once(ec_dev->dev,
- "missing EC transfer API, cannot send command\n");
+ dev_err_once(ec_dev->dev, "missing EC transfer API, cannot send command\n");
return -EIO;
}

trace_cros_ec_request_start(msg);
ret = (*xfer_fxn)(ec_dev, msg);
trace_cros_ec_request_done(msg, ret);
+
+ return ret;
+}
+
+static int cros_ec_send_command(struct cros_ec_device *ec_dev, struct cros_ec_command *msg)
+{
+ int ret = cros_ec_xfer_command(ec_dev, msg);
+
if (msg->result == EC_RES_IN_PROGRESS) {
int i;
struct cros_ec_command *status_msg;
--
2.36.1.476.g0c4daa206d-goog

2022-06-15 05:29:07

by Tzung-Bi Shih

[permalink] [raw]
Subject: [PATCH 10/11] platform/chrome: cros_ec_proto: add Kunit test for empty payload

cros_ec_wait_until_complete() sends EC_CMD_GET_COMMS_STATUS which expects
to receive sizeof(struct ec_response_get_comms_status) from
cros_ec_xfer_command().

Add Kunit test and expect to receive an error code when
cros_ec_xfer_command() returns 0.

Signed-off-by: Tzung-Bi Shih <[email protected]>
---
drivers/platform/chrome/cros_ec_proto_test.c | 31 ++++++++++++++++++++
1 file changed, 31 insertions(+)

diff --git a/drivers/platform/chrome/cros_ec_proto_test.c b/drivers/platform/chrome/cros_ec_proto_test.c
index 2a6b099fbfd9..7d45e5022221 100644
--- a/drivers/platform/chrome/cros_ec_proto_test.c
+++ b/drivers/platform/chrome/cros_ec_proto_test.c
@@ -1934,6 +1934,36 @@ static void cros_ec_proto_test_cmd_xfer_in_progress_return_error(struct kunit *t
KUNIT_EXPECT_EQ(test, cros_kunit_ec_pkt_xfer_mock_called, 2);
}

+static void cros_ec_proto_test_cmd_xfer_in_progress_return0(struct kunit *test)
+{
+ struct cros_ec_proto_test_priv *priv = test->priv;
+ struct cros_ec_device *ec_dev = &priv->ec_dev;
+ struct ec_xfer_mock *mock;
+ int ret;
+ struct cros_ec_command msg;
+
+ memset(&msg, 0, sizeof(msg));
+
+ ec_dev->pkt_xfer = cros_kunit_ec_pkt_xfer_mock;
+
+ /* For the first host command to return EC_RES_IN_PROGRESS. */
+ {
+ mock = cros_kunit_ec_xfer_mock_addx(test, 0, EC_RES_IN_PROGRESS, 0);
+ KUNIT_ASSERT_PTR_NE(test, mock, NULL);
+ }
+
+ /* For EC_CMD_GET_COMMS_STATUS. */
+ {
+ mock = cros_kunit_ec_xfer_mock_add(test, 0);
+ KUNIT_ASSERT_PTR_NE(test, mock, NULL);
+ }
+
+ ret = cros_ec_cmd_xfer(ec_dev, &msg);
+ KUNIT_EXPECT_EQ(test, ret, -EPROTO);
+
+ KUNIT_EXPECT_EQ(test, cros_kunit_ec_pkt_xfer_mock_called, 2);
+}
+
static void cros_ec_proto_test_release(struct device *dev)
{
}
@@ -2013,6 +2043,7 @@ static struct kunit_case cros_ec_proto_test_cases[] = {
KUNIT_CASE(cros_ec_proto_test_cmd_xfer_in_progress_retries_status_processing),
KUNIT_CASE(cros_ec_proto_test_cmd_xfer_in_progress_xfer_error),
KUNIT_CASE(cros_ec_proto_test_cmd_xfer_in_progress_return_error),
+ KUNIT_CASE(cros_ec_proto_test_cmd_xfer_in_progress_return0),
{}
};

--
2.36.1.476.g0c4daa206d-goog

2022-06-15 05:32:54

by Tzung-Bi Shih

[permalink] [raw]
Subject: [PATCH 02/11] platform/chrome: cros_ec_proto: add Kunit tests for cros_ec_cmd_xfer()

cros_ec_cmd_xfer() transfers the given command and data if any. It
performs some sanity checks and calls cros_ec_send_command().

Add Kunit tests for cros_ec_cmd_xfer().

Signed-off-by: Tzung-Bi Shih <[email protected]>
---
drivers/platform/chrome/cros_ec_proto_test.c | 149 +++++++++++++++++++
1 file changed, 149 insertions(+)

diff --git a/drivers/platform/chrome/cros_ec_proto_test.c b/drivers/platform/chrome/cros_ec_proto_test.c
index 1e2a1522c288..33721607a5b9 100644
--- a/drivers/platform/chrome/cros_ec_proto_test.c
+++ b/drivers/platform/chrome/cros_ec_proto_test.c
@@ -1535,6 +1535,151 @@ static void cros_ec_proto_test_query_all_default_wake_mask_return0(struct kunit
}
}

+static void cros_ec_proto_test_cmd_xfer_normal(struct kunit *test)
+{
+ struct cros_ec_proto_test_priv *priv = test->priv;
+ struct cros_ec_device *ec_dev = &priv->ec_dev;
+ struct ec_xfer_mock *mock;
+ int ret;
+ struct {
+ struct cros_ec_command msg;
+ u8 data[0x100];
+ } __packed buf;
+
+ ec_dev->max_request = 0xff;
+ ec_dev->max_response = 0xee;
+ ec_dev->max_passthru = 0xdd;
+
+ buf.msg.version = 0;
+ buf.msg.command = EC_CMD_HELLO;
+ buf.msg.insize = 4;
+ buf.msg.outsize = 2;
+ buf.data[0] = 0x55;
+ buf.data[1] = 0xaa;
+
+ {
+ u8 *data;
+
+ mock = cros_kunit_ec_xfer_mock_add(test, 4);
+ KUNIT_ASSERT_PTR_NE(test, mock, NULL);
+
+ data = (u8 *)mock->o_data;
+ data[0] = 0xaa;
+ data[1] = 0x55;
+ data[2] = 0xcc;
+ data[3] = 0x33;
+ }
+
+ ret = cros_ec_cmd_xfer(ec_dev, &buf.msg);
+ KUNIT_EXPECT_EQ(test, ret, 4);
+
+ {
+ u8 *data;
+
+ mock = cros_kunit_ec_xfer_mock_next();
+ KUNIT_EXPECT_PTR_NE(test, mock, NULL);
+
+ KUNIT_EXPECT_EQ(test, mock->msg.version, 0);
+ KUNIT_EXPECT_EQ(test, mock->msg.command, EC_CMD_HELLO);
+ KUNIT_EXPECT_EQ(test, mock->msg.insize, 4);
+ KUNIT_EXPECT_EQ(test, mock->msg.outsize, 2);
+
+ data = (u8 *)mock->i_data;
+ KUNIT_EXPECT_EQ(test, data[0], 0x55);
+ KUNIT_EXPECT_EQ(test, data[1], 0xaa);
+
+ KUNIT_EXPECT_EQ(test, buf.data[0], 0xaa);
+ KUNIT_EXPECT_EQ(test, buf.data[1], 0x55);
+ KUNIT_EXPECT_EQ(test, buf.data[2], 0xcc);
+ KUNIT_EXPECT_EQ(test, buf.data[3], 0x33);
+ }
+}
+
+static void cros_ec_proto_test_cmd_xfer_excess_msg_insize(struct kunit *test)
+{
+ struct cros_ec_proto_test_priv *priv = test->priv;
+ struct cros_ec_device *ec_dev = &priv->ec_dev;
+ struct ec_xfer_mock *mock;
+ int ret;
+ struct {
+ struct cros_ec_command msg;
+ u8 data[0x100];
+ } __packed buf;
+
+ ec_dev->max_request = 0xff;
+ ec_dev->max_response = 0xee;
+ ec_dev->max_passthru = 0xdd;
+
+ buf.msg.version = 0;
+ buf.msg.command = EC_CMD_HELLO;
+ buf.msg.insize = 0xee + 1;
+ buf.msg.outsize = 2;
+
+ {
+ mock = cros_kunit_ec_xfer_mock_add(test, 0xcc);
+ KUNIT_ASSERT_PTR_NE(test, mock, NULL);
+ }
+
+ ret = cros_ec_cmd_xfer(ec_dev, &buf.msg);
+ KUNIT_EXPECT_EQ(test, ret, 0xcc);
+
+ {
+ mock = cros_kunit_ec_xfer_mock_next();
+ KUNIT_EXPECT_PTR_NE(test, mock, NULL);
+
+ KUNIT_EXPECT_EQ(test, mock->msg.version, 0);
+ KUNIT_EXPECT_EQ(test, mock->msg.command, EC_CMD_HELLO);
+ KUNIT_EXPECT_EQ(test, mock->msg.insize, 0xee);
+ KUNIT_EXPECT_EQ(test, mock->msg.outsize, 2);
+ }
+}
+
+static void cros_ec_proto_test_cmd_xfer_excess_msg_outsize_without_passthru(struct kunit *test)
+{
+ struct cros_ec_proto_test_priv *priv = test->priv;
+ struct cros_ec_device *ec_dev = &priv->ec_dev;
+ int ret;
+ struct {
+ struct cros_ec_command msg;
+ u8 data[0x100];
+ } __packed buf;
+
+ ec_dev->max_request = 0xff;
+ ec_dev->max_response = 0xee;
+ ec_dev->max_passthru = 0xdd;
+
+ buf.msg.version = 0;
+ buf.msg.command = EC_CMD_HELLO;
+ buf.msg.insize = 4;
+ buf.msg.outsize = 0xff + 1;
+
+ ret = cros_ec_cmd_xfer(ec_dev, &buf.msg);
+ KUNIT_EXPECT_EQ(test, ret, -EMSGSIZE);
+}
+
+static void cros_ec_proto_test_cmd_xfer_excess_msg_outsize_with_passthru(struct kunit *test)
+{
+ struct cros_ec_proto_test_priv *priv = test->priv;
+ struct cros_ec_device *ec_dev = &priv->ec_dev;
+ int ret;
+ struct {
+ struct cros_ec_command msg;
+ u8 data[0x100];
+ } __packed buf;
+
+ ec_dev->max_request = 0xff;
+ ec_dev->max_response = 0xee;
+ ec_dev->max_passthru = 0xdd;
+
+ buf.msg.version = 0;
+ buf.msg.command = EC_CMD_PASSTHRU_OFFSET(CROS_EC_DEV_PD_INDEX) + EC_CMD_HELLO;
+ buf.msg.insize = 4;
+ buf.msg.outsize = 0xdd + 1;
+
+ ret = cros_ec_cmd_xfer(ec_dev, &buf.msg);
+ KUNIT_EXPECT_EQ(test, ret, -EMSGSIZE);
+}
+
static void cros_ec_proto_test_release(struct device *dev)
{
}
@@ -1601,6 +1746,10 @@ static struct kunit_case cros_ec_proto_test_cases[] = {
KUNIT_CASE(cros_ec_proto_test_query_all_no_host_sleep_return0),
KUNIT_CASE(cros_ec_proto_test_query_all_default_wake_mask_return_error),
KUNIT_CASE(cros_ec_proto_test_query_all_default_wake_mask_return0),
+ KUNIT_CASE(cros_ec_proto_test_cmd_xfer_normal),
+ KUNIT_CASE(cros_ec_proto_test_cmd_xfer_excess_msg_insize),
+ KUNIT_CASE(cros_ec_proto_test_cmd_xfer_excess_msg_outsize_without_passthru),
+ KUNIT_CASE(cros_ec_proto_test_cmd_xfer_excess_msg_outsize_with_passthru),
{}
};

--
2.36.1.476.g0c4daa206d-goog

2022-06-15 05:38:31

by Tzung-Bi Shih

[permalink] [raw]
Subject: [PATCH 06/11] platform/chrome: cros_ec_proto: change Kunit expectation when timed out

While EC_COMMS_STATUS_PROCESSING flag is still on after it tries
EC_COMMAND_RETRIES times for sending EC_CMD_GET_COMMS_STATUS,
cros_ec_wait_until_complete() doesn't return an error code.

Change the expectation to an error code.

Signed-off-by: Tzung-Bi Shih <[email protected]>
---
drivers/platform/chrome/cros_ec_proto_test.c | 4 +---
1 file changed, 1 insertion(+), 3 deletions(-)

diff --git a/drivers/platform/chrome/cros_ec_proto_test.c b/drivers/platform/chrome/cros_ec_proto_test.c
index 64100fd81c6a..fbb872040711 100644
--- a/drivers/platform/chrome/cros_ec_proto_test.c
+++ b/drivers/platform/chrome/cros_ec_proto_test.c
@@ -1870,9 +1870,7 @@ static void cros_ec_proto_test_cmd_xfer_in_progress_retries_status_processing(st
}

ret = cros_ec_cmd_xfer(ec_dev, &msg);
- KUNIT_EXPECT_EQ(test, ret, sizeof(struct ec_response_get_comms_status));
-
- KUNIT_EXPECT_EQ(test, msg.result, EC_RES_SUCCESS);
+ KUNIT_EXPECT_EQ(test, ret, -EAGAIN);

/* For EC_CMD_GET_COMMS_STATUS EC_COMMAND_RETRIES times. */
KUNIT_EXPECT_EQ(test, cros_kunit_ec_pkt_xfer_mock_called, 51);
--
2.36.1.476.g0c4daa206d-goog

2022-06-15 05:40:36

by Tzung-Bi Shih

[permalink] [raw]
Subject: [PATCH 08/11] platform/chrome: cros_ec_proto: change Kunit expectation for EC errors

cros_ec_wait_until_complete() checks `msg->result` for
EC_CMD_GET_COMMS_STATUS. However, it doesn't return standard error codes
like most of others.

Change the Kunit test expectation to align them.

Signed-off-by: Tzung-Bi Shih <[email protected]>
---
drivers/platform/chrome/cros_ec_proto_test.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/drivers/platform/chrome/cros_ec_proto_test.c b/drivers/platform/chrome/cros_ec_proto_test.c
index fbb872040711..2a6b099fbfd9 100644
--- a/drivers/platform/chrome/cros_ec_proto_test.c
+++ b/drivers/platform/chrome/cros_ec_proto_test.c
@@ -1927,7 +1927,7 @@ static void cros_ec_proto_test_cmd_xfer_in_progress_return_error(struct kunit *t
}

ret = cros_ec_cmd_xfer(ec_dev, &msg);
- KUNIT_EXPECT_EQ(test, ret, 0);
+ KUNIT_EXPECT_EQ(test, ret, -EOPNOTSUPP);

KUNIT_EXPECT_EQ(test, msg.result, EC_RES_INVALID_COMMAND);

--
2.36.1.476.g0c4daa206d-goog

2022-06-15 05:50:52

by Tzung-Bi Shih

[permalink] [raw]
Subject: [PATCH 09/11] platform/chrome: cros_ec_proto: return standard error codes for EC errors

cros_ec_wait_until_complete() checks `msg->result` for
EC_CMD_GET_COMMS_STATUS. However, it doesn't return standard error codes
like most of others.

Use cros_ec_map_error() to align them.

Signed-off-by: Tzung-Bi Shih <[email protected]>
---
drivers/platform/chrome/cros_ec_proto.c | 7 +++++--
1 file changed, 5 insertions(+), 2 deletions(-)

diff --git a/drivers/platform/chrome/cros_ec_proto.c b/drivers/platform/chrome/cros_ec_proto.c
index 49772a4c5353..5323edddb540 100644
--- a/drivers/platform/chrome/cros_ec_proto.c
+++ b/drivers/platform/chrome/cros_ec_proto.c
@@ -138,7 +138,7 @@ static int cros_ec_wait_until_complete(struct cros_ec_device *ec_dev, uint32_t *
{
struct cros_ec_command *msg;
struct ec_response_get_comms_status *status;
- int ret = 0, i;
+ int ret = 0, i, mapped;

msg = kzalloc(sizeof(*msg) + sizeof(*status), GFP_KERNEL);
if (!msg)
@@ -160,8 +160,11 @@ static int cros_ec_wait_until_complete(struct cros_ec_device *ec_dev, uint32_t *
break;

*result = msg->result;
- if (msg->result != EC_RES_SUCCESS)
+ mapped = cros_ec_map_error(msg->result);
+ if (mapped) {
+ ret = mapped;
break;
+ }

if (!(status->flags & EC_COMMS_STATUS_PROCESSING))
break;
--
2.36.1.476.g0c4daa206d-goog

2022-06-15 16:38:51

by Guenter Roeck

[permalink] [raw]
Subject: Re: [PATCH 04/11] platform/chrome: cros_ec_proto: separate cros_ec_xfer_command()

On Tue, Jun 14, 2022 at 10:13 PM Tzung-Bi Shih <[email protected]> wrote:
>
> cros_ec_send_command() has extra logic to handle EC_RES_IN_PROGRESS.
> Separate the command transfer part into cros_ec_xfer_command() so
> that other functions can re-use it.
>
> Signed-off-by: Tzung-Bi Shih <[email protected]>

Reviewed-by: Guenter Roeck <[email protected]>

> ---
> drivers/platform/chrome/cros_ec_proto.c | 13 ++++++++++---
> 1 file changed, 10 insertions(+), 3 deletions(-)
>
> diff --git a/drivers/platform/chrome/cros_ec_proto.c b/drivers/platform/chrome/cros_ec_proto.c
> index b02fd1414e52..0cec013be3d3 100644
> --- a/drivers/platform/chrome/cros_ec_proto.c
> +++ b/drivers/platform/chrome/cros_ec_proto.c
> @@ -107,7 +107,7 @@ static int prepare_tx_legacy(struct cros_ec_device *ec_dev,
> return EC_MSG_TX_PROTO_BYTES + msg->outsize;
> }
>
> -static int cros_ec_send_command(struct cros_ec_device *ec_dev, struct cros_ec_command *msg)
> +static int cros_ec_xfer_command(struct cros_ec_device *ec_dev, struct cros_ec_command *msg)
> {
> int ret;
> int (*xfer_fxn)(struct cros_ec_device *ec, struct cros_ec_command *msg);
> @@ -123,14 +123,21 @@ static int cros_ec_send_command(struct cros_ec_device *ec_dev, struct cros_ec_co
> * the EC is trying to use protocol v2, on an underlying
> * communication mechanism that does not support v2.
> */
> - dev_err_once(ec_dev->dev,
> - "missing EC transfer API, cannot send command\n");
> + dev_err_once(ec_dev->dev, "missing EC transfer API, cannot send command\n");
> return -EIO;
> }
>
> trace_cros_ec_request_start(msg);
> ret = (*xfer_fxn)(ec_dev, msg);
> trace_cros_ec_request_done(msg, ret);
> +
> + return ret;
> +}
> +
> +static int cros_ec_send_command(struct cros_ec_device *ec_dev, struct cros_ec_command *msg)
> +{
> + int ret = cros_ec_xfer_command(ec_dev, msg);
> +
> if (msg->result == EC_RES_IN_PROGRESS) {
> int i;
> struct cros_ec_command *status_msg;
> --
> 2.36.1.476.g0c4daa206d-goog
>

2022-06-15 17:18:39

by Guenter Roeck

[permalink] [raw]
Subject: Re: [PATCH 02/11] platform/chrome: cros_ec_proto: add Kunit tests for cros_ec_cmd_xfer()

On Tue, Jun 14, 2022 at 10:13 PM Tzung-Bi Shih <[email protected]> wrote:
>
> cros_ec_cmd_xfer() transfers the given command and data if any. It
> performs some sanity checks and calls cros_ec_send_command().
>
> Add Kunit tests for cros_ec_cmd_xfer().
>
> Signed-off-by: Tzung-Bi Shih <[email protected]>

Reviewed-by: Guenter Roeck <[email protected]>

> ---
> drivers/platform/chrome/cros_ec_proto_test.c | 149 +++++++++++++++++++
> 1 file changed, 149 insertions(+)
>
> diff --git a/drivers/platform/chrome/cros_ec_proto_test.c b/drivers/platform/chrome/cros_ec_proto_test.c
> index 1e2a1522c288..33721607a5b9 100644
> --- a/drivers/platform/chrome/cros_ec_proto_test.c
> +++ b/drivers/platform/chrome/cros_ec_proto_test.c
> @@ -1535,6 +1535,151 @@ static void cros_ec_proto_test_query_all_default_wake_mask_return0(struct kunit
> }
> }
>
> +static void cros_ec_proto_test_cmd_xfer_normal(struct kunit *test)
> +{
> + struct cros_ec_proto_test_priv *priv = test->priv;
> + struct cros_ec_device *ec_dev = &priv->ec_dev;
> + struct ec_xfer_mock *mock;
> + int ret;
> + struct {
> + struct cros_ec_command msg;
> + u8 data[0x100];
> + } __packed buf;
> +
> + ec_dev->max_request = 0xff;
> + ec_dev->max_response = 0xee;
> + ec_dev->max_passthru = 0xdd;
> +
> + buf.msg.version = 0;
> + buf.msg.command = EC_CMD_HELLO;
> + buf.msg.insize = 4;
> + buf.msg.outsize = 2;
> + buf.data[0] = 0x55;
> + buf.data[1] = 0xaa;
> +
> + {
> + u8 *data;
> +
> + mock = cros_kunit_ec_xfer_mock_add(test, 4);
> + KUNIT_ASSERT_PTR_NE(test, mock, NULL);
> +
> + data = (u8 *)mock->o_data;
> + data[0] = 0xaa;
> + data[1] = 0x55;
> + data[2] = 0xcc;
> + data[3] = 0x33;
> + }
> +
> + ret = cros_ec_cmd_xfer(ec_dev, &buf.msg);
> + KUNIT_EXPECT_EQ(test, ret, 4);
> +
> + {
> + u8 *data;
> +
> + mock = cros_kunit_ec_xfer_mock_next();
> + KUNIT_EXPECT_PTR_NE(test, mock, NULL);
> +
> + KUNIT_EXPECT_EQ(test, mock->msg.version, 0);
> + KUNIT_EXPECT_EQ(test, mock->msg.command, EC_CMD_HELLO);
> + KUNIT_EXPECT_EQ(test, mock->msg.insize, 4);
> + KUNIT_EXPECT_EQ(test, mock->msg.outsize, 2);
> +
> + data = (u8 *)mock->i_data;
> + KUNIT_EXPECT_EQ(test, data[0], 0x55);
> + KUNIT_EXPECT_EQ(test, data[1], 0xaa);
> +
> + KUNIT_EXPECT_EQ(test, buf.data[0], 0xaa);
> + KUNIT_EXPECT_EQ(test, buf.data[1], 0x55);
> + KUNIT_EXPECT_EQ(test, buf.data[2], 0xcc);
> + KUNIT_EXPECT_EQ(test, buf.data[3], 0x33);
> + }
> +}
> +
> +static void cros_ec_proto_test_cmd_xfer_excess_msg_insize(struct kunit *test)
> +{
> + struct cros_ec_proto_test_priv *priv = test->priv;
> + struct cros_ec_device *ec_dev = &priv->ec_dev;
> + struct ec_xfer_mock *mock;
> + int ret;
> + struct {
> + struct cros_ec_command msg;
> + u8 data[0x100];
> + } __packed buf;
> +
> + ec_dev->max_request = 0xff;
> + ec_dev->max_response = 0xee;
> + ec_dev->max_passthru = 0xdd;
> +
> + buf.msg.version = 0;
> + buf.msg.command = EC_CMD_HELLO;
> + buf.msg.insize = 0xee + 1;
> + buf.msg.outsize = 2;
> +
> + {
> + mock = cros_kunit_ec_xfer_mock_add(test, 0xcc);
> + KUNIT_ASSERT_PTR_NE(test, mock, NULL);
> + }
> +
> + ret = cros_ec_cmd_xfer(ec_dev, &buf.msg);
> + KUNIT_EXPECT_EQ(test, ret, 0xcc);
> +
> + {
> + mock = cros_kunit_ec_xfer_mock_next();
> + KUNIT_EXPECT_PTR_NE(test, mock, NULL);
> +
> + KUNIT_EXPECT_EQ(test, mock->msg.version, 0);
> + KUNIT_EXPECT_EQ(test, mock->msg.command, EC_CMD_HELLO);
> + KUNIT_EXPECT_EQ(test, mock->msg.insize, 0xee);
> + KUNIT_EXPECT_EQ(test, mock->msg.outsize, 2);
> + }
> +}
> +
> +static void cros_ec_proto_test_cmd_xfer_excess_msg_outsize_without_passthru(struct kunit *test)
> +{
> + struct cros_ec_proto_test_priv *priv = test->priv;
> + struct cros_ec_device *ec_dev = &priv->ec_dev;
> + int ret;
> + struct {
> + struct cros_ec_command msg;
> + u8 data[0x100];
> + } __packed buf;
> +
> + ec_dev->max_request = 0xff;
> + ec_dev->max_response = 0xee;
> + ec_dev->max_passthru = 0xdd;
> +
> + buf.msg.version = 0;
> + buf.msg.command = EC_CMD_HELLO;
> + buf.msg.insize = 4;
> + buf.msg.outsize = 0xff + 1;
> +
> + ret = cros_ec_cmd_xfer(ec_dev, &buf.msg);
> + KUNIT_EXPECT_EQ(test, ret, -EMSGSIZE);
> +}
> +
> +static void cros_ec_proto_test_cmd_xfer_excess_msg_outsize_with_passthru(struct kunit *test)
> +{
> + struct cros_ec_proto_test_priv *priv = test->priv;
> + struct cros_ec_device *ec_dev = &priv->ec_dev;
> + int ret;
> + struct {
> + struct cros_ec_command msg;
> + u8 data[0x100];
> + } __packed buf;
> +
> + ec_dev->max_request = 0xff;
> + ec_dev->max_response = 0xee;
> + ec_dev->max_passthru = 0xdd;
> +
> + buf.msg.version = 0;
> + buf.msg.command = EC_CMD_PASSTHRU_OFFSET(CROS_EC_DEV_PD_INDEX) + EC_CMD_HELLO;
> + buf.msg.insize = 4;
> + buf.msg.outsize = 0xdd + 1;
> +
> + ret = cros_ec_cmd_xfer(ec_dev, &buf.msg);
> + KUNIT_EXPECT_EQ(test, ret, -EMSGSIZE);
> +}
> +
> static void cros_ec_proto_test_release(struct device *dev)
> {
> }
> @@ -1601,6 +1746,10 @@ static struct kunit_case cros_ec_proto_test_cases[] = {
> KUNIT_CASE(cros_ec_proto_test_query_all_no_host_sleep_return0),
> KUNIT_CASE(cros_ec_proto_test_query_all_default_wake_mask_return_error),
> KUNIT_CASE(cros_ec_proto_test_query_all_default_wake_mask_return0),
> + KUNIT_CASE(cros_ec_proto_test_cmd_xfer_normal),
> + KUNIT_CASE(cros_ec_proto_test_cmd_xfer_excess_msg_insize),
> + KUNIT_CASE(cros_ec_proto_test_cmd_xfer_excess_msg_outsize_without_passthru),
> + KUNIT_CASE(cros_ec_proto_test_cmd_xfer_excess_msg_outsize_with_passthru),
> {}
> };
>
> --
> 2.36.1.476.g0c4daa206d-goog
>