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

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

[Tzung-Bi Shih]

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

[PATCH 01/11] platform/chrome: cros_ec_proto: add "cros_ec_" prefix to send_command()

[Tzung-Bi Shih]

To be neat, add "cros_ec_" prefix to static function send_command().

Signed-off-by: Tzung-Bi Shih <tzungbi@kernel.org>
---
 drivers/platform/chrome/cros_ec_proto.c | 15 +++++++--------
 1 file changed, 7 insertions(+), 8 deletions(-)

diff --git a/drivers/platform/chrome/cros_ec_proto.c b/drivers/platform/chrome/cros_ec_proto.c
index 6923ea4401e5..b02fd1414e52 100644
--- a/drivers/platform/chrome/cros_ec_proto.c
+++ b/drivers/platform/chrome/cros_ec_proto.c
@@ -107,8 +107,7 @@ static int prepare_tx_legacy(struct cros_ec_device *ec_dev,
 	return EC_MSG_TX_PROTO_BYTES + msg->outsize;
 }
 
-static int send_command(struct cros_ec_device *ec_dev,
-			struct cros_ec_command *msg)
+static int cros_ec_send_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);
@@ -255,7 +254,7 @@ static int cros_ec_get_host_event_wake_mask(struct cros_ec_device *ec_dev, uint3
 	msg->command = EC_CMD_HOST_EVENT_GET_WAKE_MASK;
 	msg->insize = sizeof(*r);
 
-	ret = send_command(ec_dev, msg);
+	ret = cros_ec_send_command(ec_dev, msg);
 	if (ret < 0)
 		goto exit;
 
@@ -295,7 +294,7 @@ static int cros_ec_get_proto_info(struct cros_ec_device *ec_dev, int devidx)
 	msg->command = EC_CMD_PASSTHRU_OFFSET(devidx) | EC_CMD_GET_PROTOCOL_INFO;
 	msg->insize = sizeof(*info);
 
-	ret = send_command(ec_dev, msg);
+	ret = cros_ec_send_command(ec_dev, msg);
 	/*
 	 * Send command once again when timeout occurred.
 	 * Fingerprint MCU (FPMCU) is restarted during system boot which
@@ -304,7 +303,7 @@ static int cros_ec_get_proto_info(struct cros_ec_device *ec_dev, int devidx)
 	 * attempt because we waited at least EC_MSG_DEADLINE_MS.
 	 */
 	if (ret == -ETIMEDOUT)
-		ret = send_command(ec_dev, msg);
+		ret = cros_ec_send_command(ec_dev, msg);
 
 	if (ret < 0) {
 		dev_dbg(ec_dev->dev,
@@ -376,7 +375,7 @@ static int cros_ec_get_proto_info_legacy(struct cros_ec_device *ec_dev)
 	params = (struct ec_params_hello *)msg->data;
 	params->in_data = 0xa0b0c0d0;
 
-	ret = send_command(ec_dev, msg);
+	ret = cros_ec_send_command(ec_dev, msg);
 	if (ret < 0) {
 		dev_dbg(ec_dev->dev, "EC failed to respond to v2 hello: %d\n", ret);
 		goto exit;
@@ -453,7 +452,7 @@ static int cros_ec_get_host_command_version_mask(struct cros_ec_device *ec_dev,
 	pver = (struct ec_params_get_cmd_versions *)msg->data;
 	pver->cmd = cmd;
 
-	ret = send_command(ec_dev, msg);
+	ret = cros_ec_send_command(ec_dev, msg);
 	if (ret < 0)
 		goto exit;
 
@@ -634,7 +633,7 @@ int cros_ec_cmd_xfer(struct cros_ec_device *ec_dev, struct cros_ec_command *msg)
 		}
 	}
 
-	ret = send_command(ec_dev, msg);
+	ret = cros_ec_send_command(ec_dev, msg);
 	mutex_unlock(&ec_dev->lock);
 
 	return ret;
-- 
2.36.1.476.g0c4daa206d-goog

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

[Tzung-Bi Shih]

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 <tzungbi@kernel.org>
---
 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

[PATCH 03/11] platform/chrome: cros_ec_proto: add Kunit tests for cros_ec_send_command()

[Tzung-Bi Shih]

cros_ec_cmd_xfer() is the only exported function that calls static
function cros_ec_send_command().

Add Kunit tests for cros_ec_send_command() through calling
cros_ec_cmd_xfer().

Signed-off-by: Tzung-Bi Shih <tzungbi@kernel.org>
---
 drivers/platform/chrome/cros_ec_proto_test.c | 265 +++++++++++++++++++
 drivers/platform/chrome/cros_kunit_util.c    |  20 ++
 drivers/platform/chrome/cros_kunit_util.h    |   4 +
 3 files changed, 289 insertions(+)

diff --git a/drivers/platform/chrome/cros_ec_proto_test.c b/drivers/platform/chrome/cros_ec_proto_test.c
index 33721607a5b9..64100fd81c6a 100644
--- a/drivers/platform/chrome/cros_ec_proto_test.c
+++ b/drivers/platform/chrome/cros_ec_proto_test.c
@@ -1680,6 +1680,262 @@ static void cros_ec_proto_test_cmd_xfer_excess_msg_outsize_with_passthru(struct
 	KUNIT_EXPECT_EQ(test, ret, -EMSGSIZE);
 }
 
+static void cros_ec_proto_test_cmd_xfer_protocol_v3_normal(struct kunit *test)
+{
+	struct cros_ec_proto_test_priv *priv = test->priv;
+	struct cros_ec_device *ec_dev = &priv->ec_dev;
+	int ret;
+	struct cros_ec_command msg;
+
+	memset(&msg, 0, sizeof(msg));
+
+	ec_dev->proto_version = 3;
+	ec_dev->cmd_xfer = cros_kunit_ec_cmd_xfer_mock;
+	ec_dev->pkt_xfer = cros_kunit_ec_pkt_xfer_mock;
+
+	ret = cros_ec_cmd_xfer(ec_dev, &msg);
+	KUNIT_EXPECT_EQ(test, ret, 0);
+
+	KUNIT_EXPECT_EQ(test, cros_kunit_ec_cmd_xfer_mock_called, 0);
+	KUNIT_EXPECT_EQ(test, cros_kunit_ec_pkt_xfer_mock_called, 1);
+}
+
+static void cros_ec_proto_test_cmd_xfer_protocol_v3_no_op(struct kunit *test)
+{
+	struct cros_ec_proto_test_priv *priv = test->priv;
+	struct cros_ec_device *ec_dev = &priv->ec_dev;
+	int ret;
+	struct cros_ec_command msg;
+
+	memset(&msg, 0, sizeof(msg));
+
+	ec_dev->proto_version = 3;
+	ec_dev->cmd_xfer = cros_kunit_ec_cmd_xfer_mock;
+	ec_dev->pkt_xfer = NULL;
+
+	ret = cros_ec_cmd_xfer(ec_dev, &msg);
+	KUNIT_EXPECT_EQ(test, ret, -EIO);
+}
+
+static void cros_ec_proto_test_cmd_xfer_protocol_v2_normal(struct kunit *test)
+{
+	struct cros_ec_proto_test_priv *priv = test->priv;
+	struct cros_ec_device *ec_dev = &priv->ec_dev;
+	int ret;
+	struct cros_ec_command msg;
+
+	memset(&msg, 0, sizeof(msg));
+
+	ec_dev->proto_version = 2;
+	ec_dev->cmd_xfer = cros_kunit_ec_cmd_xfer_mock;
+	ec_dev->pkt_xfer = cros_kunit_ec_pkt_xfer_mock;
+
+	ret = cros_ec_cmd_xfer(ec_dev, &msg);
+	KUNIT_EXPECT_EQ(test, ret, 0);
+
+	KUNIT_EXPECT_EQ(test, cros_kunit_ec_cmd_xfer_mock_called, 1);
+	KUNIT_EXPECT_EQ(test, cros_kunit_ec_pkt_xfer_mock_called, 0);
+}
+
+static void cros_ec_proto_test_cmd_xfer_protocol_v2_no_op(struct kunit *test)
+{
+	struct cros_ec_proto_test_priv *priv = test->priv;
+	struct cros_ec_device *ec_dev = &priv->ec_dev;
+	int ret;
+	struct cros_ec_command msg;
+
+	memset(&msg, 0, sizeof(msg));
+
+	ec_dev->proto_version = 2;
+	ec_dev->cmd_xfer = NULL;
+	ec_dev->pkt_xfer = cros_kunit_ec_pkt_xfer_mock;
+
+	ret = cros_ec_cmd_xfer(ec_dev, &msg);
+	KUNIT_EXPECT_EQ(test, ret, -EIO);
+}
+
+static void cros_ec_proto_test_cmd_xfer_in_progress_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 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. */
+	{
+		struct ec_response_get_comms_status *data;
+
+		mock = cros_kunit_ec_xfer_mock_add(test, sizeof(*data));
+		KUNIT_ASSERT_PTR_NE(test, mock, NULL);
+
+		data = (struct ec_response_get_comms_status *)mock->o_data;
+		data->flags = 0;
+	}
+
+	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);
+
+	/* For the first host command to return EC_RES_IN_PROGRESS. */
+	{
+		mock = cros_kunit_ec_xfer_mock_next();
+		KUNIT_EXPECT_PTR_NE(test, mock, NULL);
+	}
+
+	/* For EC_CMD_GET_COMMS_STATUS. */
+	{
+		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_GET_COMMS_STATUS);
+		KUNIT_EXPECT_EQ(test, mock->msg.insize,
+				sizeof(struct ec_response_get_comms_status));
+		KUNIT_EXPECT_EQ(test, mock->msg.outsize, 0);
+	}
+
+	KUNIT_EXPECT_EQ(test, cros_kunit_ec_pkt_xfer_mock_called, 2);
+}
+
+static void cros_ec_proto_test_cmd_xfer_in_progress_retries_eagain(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 EC_COMMAND_RETRIES times. */
+	cros_kunit_ec_xfer_mock_default_ret = -EAGAIN;
+
+	ret = cros_ec_cmd_xfer(ec_dev, &msg);
+	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);
+}
+
+static void cros_ec_proto_test_cmd_xfer_in_progress_retries_status_processing(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 EC_COMMAND_RETRIES times. */
+	{
+		struct ec_response_get_comms_status *data;
+		int i;
+
+		for (i = 0; i < 50; ++i) {
+			mock = cros_kunit_ec_xfer_mock_add(test, sizeof(*data));
+			KUNIT_ASSERT_PTR_NE(test, mock, NULL);
+
+			data = (struct ec_response_get_comms_status *)mock->o_data;
+			data->flags |= EC_COMMS_STATUS_PROCESSING;
+		}
+	}
+
+	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);
+
+	/* For EC_CMD_GET_COMMS_STATUS EC_COMMAND_RETRIES times. */
+	KUNIT_EXPECT_EQ(test, cros_kunit_ec_pkt_xfer_mock_called, 51);
+}
+
+static void cros_ec_proto_test_cmd_xfer_in_progress_xfer_error(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));
+
+	/* 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_addx(test, -EIO, EC_RES_SUCCESS, 0);
+		KUNIT_ASSERT_PTR_NE(test, mock, NULL);
+	}
+
+	ret = cros_ec_cmd_xfer(ec_dev, &msg);
+	KUNIT_EXPECT_EQ(test, ret, -EIO);
+}
+
+static void cros_ec_proto_test_cmd_xfer_in_progress_return_error(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_addx(test, 0, EC_RES_INVALID_COMMAND, 0);
+		KUNIT_ASSERT_PTR_NE(test, mock, NULL);
+	}
+
+	ret = cros_ec_cmd_xfer(ec_dev, &msg);
+	KUNIT_EXPECT_EQ(test, ret, 0);
+
+	KUNIT_EXPECT_EQ(test, msg.result, EC_RES_INVALID_COMMAND);
+
+	KUNIT_EXPECT_EQ(test, cros_kunit_ec_pkt_xfer_mock_called, 2);
+}
+
 static void cros_ec_proto_test_release(struct device *dev)
 {
 }
@@ -1750,6 +2006,15 @@ static struct kunit_case cros_ec_proto_test_cases[] = {
 	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),
+	KUNIT_CASE(cros_ec_proto_test_cmd_xfer_protocol_v3_normal),
+	KUNIT_CASE(cros_ec_proto_test_cmd_xfer_protocol_v3_no_op),
+	KUNIT_CASE(cros_ec_proto_test_cmd_xfer_protocol_v2_normal),
+	KUNIT_CASE(cros_ec_proto_test_cmd_xfer_protocol_v2_no_op),
+	KUNIT_CASE(cros_ec_proto_test_cmd_xfer_in_progress_normal),
+	KUNIT_CASE(cros_ec_proto_test_cmd_xfer_in_progress_retries_eagain),
+	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),
 	{}
 };
 
diff --git a/drivers/platform/chrome/cros_kunit_util.c b/drivers/platform/chrome/cros_kunit_util.c
index e031777dea87..3ede971e82ee 100644
--- a/drivers/platform/chrome/cros_kunit_util.c
+++ b/drivers/platform/chrome/cros_kunit_util.c
@@ -15,6 +15,10 @@
 
 int cros_kunit_ec_xfer_mock_default_ret;
 EXPORT_SYMBOL_GPL(cros_kunit_ec_xfer_mock_default_ret);
+int cros_kunit_ec_cmd_xfer_mock_called;
+EXPORT_SYMBOL_GPL(cros_kunit_ec_cmd_xfer_mock_called);
+int cros_kunit_ec_pkt_xfer_mock_called;
+EXPORT_SYMBOL_GPL(cros_kunit_ec_pkt_xfer_mock_called);
 
 static struct list_head cros_kunit_ec_xfer_mock_in;
 static struct list_head cros_kunit_ec_xfer_mock_out;
@@ -46,6 +50,20 @@ int cros_kunit_ec_xfer_mock(struct cros_ec_device *ec_dev, struct cros_ec_comman
 }
 EXPORT_SYMBOL_GPL(cros_kunit_ec_xfer_mock);
 
+int cros_kunit_ec_cmd_xfer_mock(struct cros_ec_device *ec_dev, struct cros_ec_command *msg)
+{
+	++cros_kunit_ec_cmd_xfer_mock_called;
+	return cros_kunit_ec_xfer_mock(ec_dev, msg);
+}
+EXPORT_SYMBOL_GPL(cros_kunit_ec_cmd_xfer_mock);
+
+int cros_kunit_ec_pkt_xfer_mock(struct cros_ec_device *ec_dev, struct cros_ec_command *msg)
+{
+	++cros_kunit_ec_pkt_xfer_mock_called;
+	return cros_kunit_ec_xfer_mock(ec_dev, msg);
+}
+EXPORT_SYMBOL_GPL(cros_kunit_ec_pkt_xfer_mock);
+
 struct ec_xfer_mock *cros_kunit_ec_xfer_mock_add(struct kunit *test, size_t size)
 {
 	return cros_kunit_ec_xfer_mock_addx(test, size, EC_RES_SUCCESS, size);
@@ -90,6 +108,8 @@ EXPORT_SYMBOL_GPL(cros_kunit_ec_xfer_mock_next);
 void cros_kunit_mock_reset(void)
 {
 	cros_kunit_ec_xfer_mock_default_ret = 0;
+	cros_kunit_ec_cmd_xfer_mock_called = 0;
+	cros_kunit_ec_pkt_xfer_mock_called = 0;
 	INIT_LIST_HEAD(&cros_kunit_ec_xfer_mock_in);
 	INIT_LIST_HEAD(&cros_kunit_ec_xfer_mock_out);
 }
diff --git a/drivers/platform/chrome/cros_kunit_util.h b/drivers/platform/chrome/cros_kunit_util.h
index 79c4525f873c..ae4080cb13f1 100644
--- a/drivers/platform/chrome/cros_kunit_util.h
+++ b/drivers/platform/chrome/cros_kunit_util.h
@@ -24,8 +24,12 @@ struct ec_xfer_mock {
 };
 
 extern int cros_kunit_ec_xfer_mock_default_ret;
+extern int cros_kunit_ec_cmd_xfer_mock_called;
+extern int cros_kunit_ec_pkt_xfer_mock_called;
 
 int cros_kunit_ec_xfer_mock(struct cros_ec_device *ec_dev, struct cros_ec_command *msg);
+int cros_kunit_ec_cmd_xfer_mock(struct cros_ec_device *ec_dev, struct cros_ec_command *msg);
+int cros_kunit_ec_pkt_xfer_mock(struct cros_ec_device *ec_dev, struct cros_ec_command *msg);
 struct ec_xfer_mock *cros_kunit_ec_xfer_mock_add(struct kunit *test, size_t size);
 struct ec_xfer_mock *cros_kunit_ec_xfer_mock_addx(struct kunit *test,
 						  int ret, int result, size_t size);
-- 
2.36.1.476.g0c4daa206d-goog

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

[Tzung-Bi Shih]

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 <tzungbi@kernel.org>
---
 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

[PATCH 05/11] platform/chrome: cros_ec_proto: separate cros_ec_wait_until_complete()

[Tzung-Bi Shih]

EC returns EC_RES_IN_PROGRESS if the host command needs more time to
complete.  Whenever receives the return code, cros_ec_send_command()
sends EC_CMD_GET_COMMS_STATUS to query the command status.

Separate cros_ec_wait_until_complete() from cros_ec_send_command().
It sends EC_CMD_GET_COMMS_STATUS and waits until the previous command
was completed, or encountered error, or timed out.

Signed-off-by: Tzung-Bi Shih <tzungbi@kernel.org>
---
 drivers/platform/chrome/cros_ec_proto.c | 75 ++++++++++++-------------
 1 file changed, 36 insertions(+), 39 deletions(-)

diff --git a/drivers/platform/chrome/cros_ec_proto.c b/drivers/platform/chrome/cros_ec_proto.c
index 0cec013be3d3..466ecb063bd6 100644
--- a/drivers/platform/chrome/cros_ec_proto.c
+++ b/drivers/platform/chrome/cros_ec_proto.c
@@ -134,53 +134,50 @@ static int cros_ec_xfer_command(struct cros_ec_device *ec_dev, struct cros_ec_co
 	return ret;
 }
 
-static int cros_ec_send_command(struct cros_ec_device *ec_dev, struct cros_ec_command *msg)
+static int cros_ec_wait_until_complete(struct cros_ec_device *ec_dev, uint32_t *result)
 {
-	int ret = cros_ec_xfer_command(ec_dev, msg);
+	struct cros_ec_command *msg;
+	struct ec_response_get_comms_status *status;
+	int ret = 0, i;
+
+	msg = kzalloc(sizeof(*msg) + sizeof(*status), GFP_KERNEL);
+	if (!msg)
+		return -ENOMEM;
 
-	if (msg->result == EC_RES_IN_PROGRESS) {
-		int i;
-		struct cros_ec_command *status_msg;
-		struct ec_response_get_comms_status *status;
+	msg->command = EC_CMD_GET_COMMS_STATUS;
+	msg->insize = sizeof(*status);
 
-		status_msg = kmalloc(sizeof(*status_msg) + sizeof(*status),
-				     GFP_KERNEL);
-		if (!status_msg)
-			return -ENOMEM;
+	status = (struct ec_response_get_comms_status *)msg->data;
 
-		status_msg->version = 0;
-		status_msg->command = EC_CMD_GET_COMMS_STATUS;
-		status_msg->insize = sizeof(*status);
-		status_msg->outsize = 0;
+	/* Query the EC's status until it's no longer busy or we encounter an error. */
+	for (i = 0; i < EC_COMMAND_RETRIES; ++i) {
+		usleep_range(10000, 11000);
 
-		/*
-		 * Query the EC's status until it's no longer busy or
-		 * we encounter an error.
-		 */
-		for (i = 0; i < EC_COMMAND_RETRIES; i++) {
-			usleep_range(10000, 11000);
-
-			trace_cros_ec_request_start(status_msg);
-			ret = (*xfer_fxn)(ec_dev, status_msg);
-			trace_cros_ec_request_done(status_msg, ret);
-			if (ret == -EAGAIN)
-				continue;
-			if (ret < 0)
-				break;
-
-			msg->result = status_msg->result;
-			if (status_msg->result != EC_RES_SUCCESS)
-				break;
-
-			status = (struct ec_response_get_comms_status *)
-				 status_msg->data;
-			if (!(status->flags & EC_COMMS_STATUS_PROCESSING))
-				break;
-		}
+		ret = cros_ec_xfer_command(ec_dev, msg);
+		if (ret == -EAGAIN)
+			continue;
+		if (ret < 0)
+			break;
+
+		*result = msg->result;
+		if (msg->result != EC_RES_SUCCESS)
+			break;
 
-		kfree(status_msg);
+		if (!(status->flags & EC_COMMS_STATUS_PROCESSING))
+			break;
 	}
 
+	kfree(msg);
+	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)
+		ret = cros_ec_wait_until_complete(ec_dev, &msg->result);
+
 	return ret;
 }
 
-- 
2.36.1.476.g0c4daa206d-goog

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

[Tzung-Bi Shih]

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 <tzungbi@kernel.org>
---
 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

[PATCH 07/11] platform/chrome: cros_ec_proto: return -EAGAIN when retries timed out

[Tzung-Bi Shih]

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.

Return -EAGAIN in the case instead.

Signed-off-by: Tzung-Bi Shih <tzungbi@kernel.org>
---
 drivers/platform/chrome/cros_ec_proto.c | 3 +++
 1 file changed, 3 insertions(+)

diff --git a/drivers/platform/chrome/cros_ec_proto.c b/drivers/platform/chrome/cros_ec_proto.c
index 466ecb063bd6..49772a4c5353 100644
--- a/drivers/platform/chrome/cros_ec_proto.c
+++ b/drivers/platform/chrome/cros_ec_proto.c
@@ -167,6 +167,9 @@ static int cros_ec_wait_until_complete(struct cros_ec_device *ec_dev, uint32_t *
 			break;
 	}
 
+	if (i >= EC_COMMAND_RETRIES)
+		ret = -EAGAIN;
+
 	kfree(msg);
 	return ret;
 }
-- 
2.36.1.476.g0c4daa206d-goog

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

[Tzung-Bi Shih]

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 <tzungbi@kernel.org>
---
 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

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

[Tzung-Bi Shih]

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 <tzungbi@kernel.org>
---
 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

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

[Tzung-Bi Shih]

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 <tzungbi@kernel.org>
---
 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

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

[Tzung-Bi Shih]

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 <tzungbi@kernel.org>
---
 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

Re: [PATCH 01/11] platform/chrome: cros_ec_proto: add "cros_ec_" prefix to send_command()

[Guenter Roeck]

On Tue, Jun 14, 2022 at 10:13 PM Tzung-Bi Shih <tzungbi@kernel.org> wrote:
>
> To be neat, add "cros_ec_" prefix to static function send_command().
>
> Signed-off-by: Tzung-Bi Shih <tzungbi@kernel.org>

Reviewed-by: Guenter Roeck <groeck@chromium.org>

> ---
>  drivers/platform/chrome/cros_ec_proto.c | 15 +++++++--------
>  1 file changed, 7 insertions(+), 8 deletions(-)
>
> diff --git a/drivers/platform/chrome/cros_ec_proto.c b/drivers/platform/chrome/cros_ec_proto.c
> index 6923ea4401e5..b02fd1414e52 100644
> --- a/drivers/platform/chrome/cros_ec_proto.c
> +++ b/drivers/platform/chrome/cros_ec_proto.c
> @@ -107,8 +107,7 @@ static int prepare_tx_legacy(struct cros_ec_device *ec_dev,
>         return EC_MSG_TX_PROTO_BYTES + msg->outsize;
>  }
>
> -static int send_command(struct cros_ec_device *ec_dev,
> -                       struct cros_ec_command *msg)
> +static int cros_ec_send_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);
> @@ -255,7 +254,7 @@ static int cros_ec_get_host_event_wake_mask(struct cros_ec_device *ec_dev, uint3
>         msg->command = EC_CMD_HOST_EVENT_GET_WAKE_MASK;
>         msg->insize = sizeof(*r);
>
> -       ret = send_command(ec_dev, msg);
> +       ret = cros_ec_send_command(ec_dev, msg);
>         if (ret < 0)
>                 goto exit;
>
> @@ -295,7 +294,7 @@ static int cros_ec_get_proto_info(struct cros_ec_device *ec_dev, int devidx)
>         msg->command = EC_CMD_PASSTHRU_OFFSET(devidx) | EC_CMD_GET_PROTOCOL_INFO;
>         msg->insize = sizeof(*info);
>
> -       ret = send_command(ec_dev, msg);
> +       ret = cros_ec_send_command(ec_dev, msg);
>         /*
>          * Send command once again when timeout occurred.
>          * Fingerprint MCU (FPMCU) is restarted during system boot which
> @@ -304,7 +303,7 @@ static int cros_ec_get_proto_info(struct cros_ec_device *ec_dev, int devidx)
>          * attempt because we waited at least EC_MSG_DEADLINE_MS.
>          */
>         if (ret == -ETIMEDOUT)
> -               ret = send_command(ec_dev, msg);
> +               ret = cros_ec_send_command(ec_dev, msg);
>
>         if (ret < 0) {
>                 dev_dbg(ec_dev->dev,
> @@ -376,7 +375,7 @@ static int cros_ec_get_proto_info_legacy(struct cros_ec_device *ec_dev)
>         params = (struct ec_params_hello *)msg->data;
>         params->in_data = 0xa0b0c0d0;
>
> -       ret = send_command(ec_dev, msg);
> +       ret = cros_ec_send_command(ec_dev, msg);
>         if (ret < 0) {
>                 dev_dbg(ec_dev->dev, "EC failed to respond to v2 hello: %d\n", ret);
>                 goto exit;
> @@ -453,7 +452,7 @@ static int cros_ec_get_host_command_version_mask(struct cros_ec_device *ec_dev,
>         pver = (struct ec_params_get_cmd_versions *)msg->data;
>         pver->cmd = cmd;
>
> -       ret = send_command(ec_dev, msg);
> +       ret = cros_ec_send_command(ec_dev, msg);
>         if (ret < 0)
>                 goto exit;
>
> @@ -634,7 +633,7 @@ int cros_ec_cmd_xfer(struct cros_ec_device *ec_dev, struct cros_ec_command *msg)
>                 }
>         }
>
> -       ret = send_command(ec_dev, msg);
> +       ret = cros_ec_send_command(ec_dev, msg);
>         mutex_unlock(&ec_dev->lock);
>
>         return ret;
> --
> 2.36.1.476.g0c4daa206d-goog
>

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

[Guenter Roeck]

On Tue, Jun 14, 2022 at 10:13 PM Tzung-Bi Shih <tzungbi@kernel.org> 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 <tzungbi@kernel.org>

Reviewed-by: Guenter Roeck <groeck@chromium.org>

> ---
>  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
>

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

[Guenter Roeck]

On Tue, Jun 14, 2022 at 10:13 PM Tzung-Bi Shih <tzungbi@kernel.org> 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 <tzungbi@kernel.org>

Reviewed-by: Guenter Roeck <groeck@chromium.org>

> ---
>  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
>

Re: [PATCH 03/11] platform/chrome: cros_ec_proto: add Kunit tests for cros_ec_send_command()

[Guenter Roeck]

On Tue, Jun 14, 2022 at 10:13 PM Tzung-Bi Shih <tzungbi@kernel.org> wrote:
>
> cros_ec_cmd_xfer() is the only exported function that calls static
> function cros_ec_send_command().
>
> Add Kunit tests for cros_ec_send_command() through calling
> cros_ec_cmd_xfer().
>
> Signed-off-by: Tzung-Bi Shih <tzungbi@kernel.org>

Reviewed-by: Guenter Roeck <groeck@chromium.org>

> ---
>  drivers/platform/chrome/cros_ec_proto_test.c | 265 +++++++++++++++++++
>  drivers/platform/chrome/cros_kunit_util.c    |  20 ++
>  drivers/platform/chrome/cros_kunit_util.h    |   4 +
>  3 files changed, 289 insertions(+)
>
> diff --git a/drivers/platform/chrome/cros_ec_proto_test.c b/drivers/platform/chrome/cros_ec_proto_test.c
> index 33721607a5b9..64100fd81c6a 100644
> --- a/drivers/platform/chrome/cros_ec_proto_test.c
> +++ b/drivers/platform/chrome/cros_ec_proto_test.c
> @@ -1680,6 +1680,262 @@ static void cros_ec_proto_test_cmd_xfer_excess_msg_outsize_with_passthru(struct
>         KUNIT_EXPECT_EQ(test, ret, -EMSGSIZE);
>  }
>
> +static void cros_ec_proto_test_cmd_xfer_protocol_v3_normal(struct kunit *test)
> +{
> +       struct cros_ec_proto_test_priv *priv = test->priv;
> +       struct cros_ec_device *ec_dev = &priv->ec_dev;
> +       int ret;
> +       struct cros_ec_command msg;
> +
> +       memset(&msg, 0, sizeof(msg));
> +
> +       ec_dev->proto_version = 3;
> +       ec_dev->cmd_xfer = cros_kunit_ec_cmd_xfer_mock;
> +       ec_dev->pkt_xfer = cros_kunit_ec_pkt_xfer_mock;
> +
> +       ret = cros_ec_cmd_xfer(ec_dev, &msg);
> +       KUNIT_EXPECT_EQ(test, ret, 0);
> +
> +       KUNIT_EXPECT_EQ(test, cros_kunit_ec_cmd_xfer_mock_called, 0);
> +       KUNIT_EXPECT_EQ(test, cros_kunit_ec_pkt_xfer_mock_called, 1);
> +}
> +
> +static void cros_ec_proto_test_cmd_xfer_protocol_v3_no_op(struct kunit *test)
> +{
> +       struct cros_ec_proto_test_priv *priv = test->priv;
> +       struct cros_ec_device *ec_dev = &priv->ec_dev;
> +       int ret;
> +       struct cros_ec_command msg;
> +
> +       memset(&msg, 0, sizeof(msg));
> +
> +       ec_dev->proto_version = 3;
> +       ec_dev->cmd_xfer = cros_kunit_ec_cmd_xfer_mock;
> +       ec_dev->pkt_xfer = NULL;
> +
> +       ret = cros_ec_cmd_xfer(ec_dev, &msg);
> +       KUNIT_EXPECT_EQ(test, ret, -EIO);
> +}
> +
> +static void cros_ec_proto_test_cmd_xfer_protocol_v2_normal(struct kunit *test)
> +{
> +       struct cros_ec_proto_test_priv *priv = test->priv;
> +       struct cros_ec_device *ec_dev = &priv->ec_dev;
> +       int ret;
> +       struct cros_ec_command msg;
> +
> +       memset(&msg, 0, sizeof(msg));
> +
> +       ec_dev->proto_version = 2;
> +       ec_dev->cmd_xfer = cros_kunit_ec_cmd_xfer_mock;
> +       ec_dev->pkt_xfer = cros_kunit_ec_pkt_xfer_mock;
> +
> +       ret = cros_ec_cmd_xfer(ec_dev, &msg);
> +       KUNIT_EXPECT_EQ(test, ret, 0);
> +
> +       KUNIT_EXPECT_EQ(test, cros_kunit_ec_cmd_xfer_mock_called, 1);
> +       KUNIT_EXPECT_EQ(test, cros_kunit_ec_pkt_xfer_mock_called, 0);
> +}
> +
> +static void cros_ec_proto_test_cmd_xfer_protocol_v2_no_op(struct kunit *test)
> +{
> +       struct cros_ec_proto_test_priv *priv = test->priv;
> +       struct cros_ec_device *ec_dev = &priv->ec_dev;
> +       int ret;
> +       struct cros_ec_command msg;
> +
> +       memset(&msg, 0, sizeof(msg));
> +
> +       ec_dev->proto_version = 2;
> +       ec_dev->cmd_xfer = NULL;
> +       ec_dev->pkt_xfer = cros_kunit_ec_pkt_xfer_mock;
> +
> +       ret = cros_ec_cmd_xfer(ec_dev, &msg);
> +       KUNIT_EXPECT_EQ(test, ret, -EIO);
> +}
> +
> +static void cros_ec_proto_test_cmd_xfer_in_progress_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 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. */
> +       {
> +               struct ec_response_get_comms_status *data;
> +
> +               mock = cros_kunit_ec_xfer_mock_add(test, sizeof(*data));
> +               KUNIT_ASSERT_PTR_NE(test, mock, NULL);
> +
> +               data = (struct ec_response_get_comms_status *)mock->o_data;
> +               data->flags = 0;
> +       }
> +
> +       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);
> +
> +       /* For the first host command to return EC_RES_IN_PROGRESS. */
> +       {
> +               mock = cros_kunit_ec_xfer_mock_next();
> +               KUNIT_EXPECT_PTR_NE(test, mock, NULL);
> +       }
> +
> +       /* For EC_CMD_GET_COMMS_STATUS. */
> +       {
> +               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_GET_COMMS_STATUS);
> +               KUNIT_EXPECT_EQ(test, mock->msg.insize,
> +                               sizeof(struct ec_response_get_comms_status));
> +               KUNIT_EXPECT_EQ(test, mock->msg.outsize, 0);
> +       }
> +
> +       KUNIT_EXPECT_EQ(test, cros_kunit_ec_pkt_xfer_mock_called, 2);
> +}
> +
> +static void cros_ec_proto_test_cmd_xfer_in_progress_retries_eagain(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 EC_COMMAND_RETRIES times. */
> +       cros_kunit_ec_xfer_mock_default_ret = -EAGAIN;
> +
> +       ret = cros_ec_cmd_xfer(ec_dev, &msg);
> +       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);
> +}
> +
> +static void cros_ec_proto_test_cmd_xfer_in_progress_retries_status_processing(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 EC_COMMAND_RETRIES times. */
> +       {
> +               struct ec_response_get_comms_status *data;
> +               int i;
> +
> +               for (i = 0; i < 50; ++i) {
> +                       mock = cros_kunit_ec_xfer_mock_add(test, sizeof(*data));
> +                       KUNIT_ASSERT_PTR_NE(test, mock, NULL);
> +
> +                       data = (struct ec_response_get_comms_status *)mock->o_data;
> +                       data->flags |= EC_COMMS_STATUS_PROCESSING;
> +               }
> +       }
> +
> +       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);
> +
> +       /* For EC_CMD_GET_COMMS_STATUS EC_COMMAND_RETRIES times. */
> +       KUNIT_EXPECT_EQ(test, cros_kunit_ec_pkt_xfer_mock_called, 51);
> +}
> +
> +static void cros_ec_proto_test_cmd_xfer_in_progress_xfer_error(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));
> +
> +       /* 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_addx(test, -EIO, EC_RES_SUCCESS, 0);
> +               KUNIT_ASSERT_PTR_NE(test, mock, NULL);
> +       }
> +
> +       ret = cros_ec_cmd_xfer(ec_dev, &msg);
> +       KUNIT_EXPECT_EQ(test, ret, -EIO);
> +}
> +
> +static void cros_ec_proto_test_cmd_xfer_in_progress_return_error(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_addx(test, 0, EC_RES_INVALID_COMMAND, 0);
> +               KUNIT_ASSERT_PTR_NE(test, mock, NULL);
> +       }
> +
> +       ret = cros_ec_cmd_xfer(ec_dev, &msg);
> +       KUNIT_EXPECT_EQ(test, ret, 0);
> +
> +       KUNIT_EXPECT_EQ(test, msg.result, EC_RES_INVALID_COMMAND);
> +
> +       KUNIT_EXPECT_EQ(test, cros_kunit_ec_pkt_xfer_mock_called, 2);
> +}
> +
>  static void cros_ec_proto_test_release(struct device *dev)
>  {
>  }
> @@ -1750,6 +2006,15 @@ static struct kunit_case cros_ec_proto_test_cases[] = {
>         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),
> +       KUNIT_CASE(cros_ec_proto_test_cmd_xfer_protocol_v3_normal),
> +       KUNIT_CASE(cros_ec_proto_test_cmd_xfer_protocol_v3_no_op),
> +       KUNIT_CASE(cros_ec_proto_test_cmd_xfer_protocol_v2_normal),
> +       KUNIT_CASE(cros_ec_proto_test_cmd_xfer_protocol_v2_no_op),
> +       KUNIT_CASE(cros_ec_proto_test_cmd_xfer_in_progress_normal),
> +       KUNIT_CASE(cros_ec_proto_test_cmd_xfer_in_progress_retries_eagain),
> +       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),
>         {}
>  };
>
> diff --git a/drivers/platform/chrome/cros_kunit_util.c b/drivers/platform/chrome/cros_kunit_util.c
> index e031777dea87..3ede971e82ee 100644
> --- a/drivers/platform/chrome/cros_kunit_util.c
> +++ b/drivers/platform/chrome/cros_kunit_util.c
> @@ -15,6 +15,10 @@
>
>  int cros_kunit_ec_xfer_mock_default_ret;
>  EXPORT_SYMBOL_GPL(cros_kunit_ec_xfer_mock_default_ret);
> +int cros_kunit_ec_cmd_xfer_mock_called;
> +EXPORT_SYMBOL_GPL(cros_kunit_ec_cmd_xfer_mock_called);
> +int cros_kunit_ec_pkt_xfer_mock_called;
> +EXPORT_SYMBOL_GPL(cros_kunit_ec_pkt_xfer_mock_called);
>
>  static struct list_head cros_kunit_ec_xfer_mock_in;
>  static struct list_head cros_kunit_ec_xfer_mock_out;
> @@ -46,6 +50,20 @@ int cros_kunit_ec_xfer_mock(struct cros_ec_device *ec_dev, struct cros_ec_comman
>  }
>  EXPORT_SYMBOL_GPL(cros_kunit_ec_xfer_mock);
>
> +int cros_kunit_ec_cmd_xfer_mock(struct cros_ec_device *ec_dev, struct cros_ec_command *msg)
> +{
> +       ++cros_kunit_ec_cmd_xfer_mock_called;
> +       return cros_kunit_ec_xfer_mock(ec_dev, msg);
> +}
> +EXPORT_SYMBOL_GPL(cros_kunit_ec_cmd_xfer_mock);
> +
> +int cros_kunit_ec_pkt_xfer_mock(struct cros_ec_device *ec_dev, struct cros_ec_command *msg)
> +{
> +       ++cros_kunit_ec_pkt_xfer_mock_called;
> +       return cros_kunit_ec_xfer_mock(ec_dev, msg);
> +}
> +EXPORT_SYMBOL_GPL(cros_kunit_ec_pkt_xfer_mock);
> +
>  struct ec_xfer_mock *cros_kunit_ec_xfer_mock_add(struct kunit *test, size_t size)
>  {
>         return cros_kunit_ec_xfer_mock_addx(test, size, EC_RES_SUCCESS, size);
> @@ -90,6 +108,8 @@ EXPORT_SYMBOL_GPL(cros_kunit_ec_xfer_mock_next);
>  void cros_kunit_mock_reset(void)
>  {
>         cros_kunit_ec_xfer_mock_default_ret = 0;
> +       cros_kunit_ec_cmd_xfer_mock_called = 0;
> +       cros_kunit_ec_pkt_xfer_mock_called = 0;
>         INIT_LIST_HEAD(&cros_kunit_ec_xfer_mock_in);
>         INIT_LIST_HEAD(&cros_kunit_ec_xfer_mock_out);
>  }
> diff --git a/drivers/platform/chrome/cros_kunit_util.h b/drivers/platform/chrome/cros_kunit_util.h
> index 79c4525f873c..ae4080cb13f1 100644
> --- a/drivers/platform/chrome/cros_kunit_util.h
> +++ b/drivers/platform/chrome/cros_kunit_util.h
> @@ -24,8 +24,12 @@ struct ec_xfer_mock {
>  };
>
>  extern int cros_kunit_ec_xfer_mock_default_ret;
> +extern int cros_kunit_ec_cmd_xfer_mock_called;
> +extern int cros_kunit_ec_pkt_xfer_mock_called;
>
>  int cros_kunit_ec_xfer_mock(struct cros_ec_device *ec_dev, struct cros_ec_command *msg);
> +int cros_kunit_ec_cmd_xfer_mock(struct cros_ec_device *ec_dev, struct cros_ec_command *msg);
> +int cros_kunit_ec_pkt_xfer_mock(struct cros_ec_device *ec_dev, struct cros_ec_command *msg);
>  struct ec_xfer_mock *cros_kunit_ec_xfer_mock_add(struct kunit *test, size_t size);
>  struct ec_xfer_mock *cros_kunit_ec_xfer_mock_addx(struct kunit *test,
>                                                   int ret, int result, size_t size);
> --
> 2.36.1.476.g0c4daa206d-goog
>

Re: [PATCH 05/11] platform/chrome: cros_ec_proto: separate cros_ec_wait_until_complete()

[Guenter Roeck]

On Tue, Jun 14, 2022 at 10:13 PM Tzung-Bi Shih <tzungbi@kernel.org> wrote:
>
> EC returns EC_RES_IN_PROGRESS if the host command needs more time to
> complete.  Whenever receives the return code, cros_ec_send_command()
> sends EC_CMD_GET_COMMS_STATUS to query the command status.
>
> Separate cros_ec_wait_until_complete() from cros_ec_send_command().
> It sends EC_CMD_GET_COMMS_STATUS and waits until the previous command
> was completed, or encountered error, or timed out.
>
> Signed-off-by: Tzung-Bi Shih <tzungbi@kernel.org>
> ---
>  drivers/platform/chrome/cros_ec_proto.c | 75 ++++++++++++-------------
>  1 file changed, 36 insertions(+), 39 deletions(-)
>
> diff --git a/drivers/platform/chrome/cros_ec_proto.c b/drivers/platform/chrome/cros_ec_proto.c
> index 0cec013be3d3..466ecb063bd6 100644
> --- a/drivers/platform/chrome/cros_ec_proto.c
> +++ b/drivers/platform/chrome/cros_ec_proto.c
> @@ -134,53 +134,50 @@ static int cros_ec_xfer_command(struct cros_ec_device *ec_dev, struct cros_ec_co
>         return ret;
>  }
>
> -static int cros_ec_send_command(struct cros_ec_device *ec_dev, struct cros_ec_command *msg)
> +static int cros_ec_wait_until_complete(struct cros_ec_device *ec_dev, uint32_t *result)
>  {
> -       int ret = cros_ec_xfer_command(ec_dev, msg);
> +       struct cros_ec_command *msg;
> +       struct ec_response_get_comms_status *status;
> +       int ret = 0, i;
> +
> +       msg = kzalloc(sizeof(*msg) + sizeof(*status), GFP_KERNEL);
> +       if (!msg)
> +               return -ENOMEM;
>
> -       if (msg->result == EC_RES_IN_PROGRESS) {
> -               int i;
> -               struct cros_ec_command *status_msg;
> -               struct ec_response_get_comms_status *status;
> +       msg->command = EC_CMD_GET_COMMS_STATUS;
> +       msg->insize = sizeof(*status);
>
> -               status_msg = kmalloc(sizeof(*status_msg) + sizeof(*status),
> -                                    GFP_KERNEL);
> -               if (!status_msg)
> -                       return -ENOMEM;
> +       status = (struct ec_response_get_comms_status *)msg->data;
>
> -               status_msg->version = 0;
> -               status_msg->command = EC_CMD_GET_COMMS_STATUS;
> -               status_msg->insize = sizeof(*status);
> -               status_msg->outsize = 0;
> +       /* Query the EC's status until it's no longer busy or we encounter an error. */
> +       for (i = 0; i < EC_COMMAND_RETRIES; ++i) {
> +               usleep_range(10000, 11000);
>
> -               /*
> -                * Query the EC's status until it's no longer busy or
> -                * we encounter an error.
> -                */
> -               for (i = 0; i < EC_COMMAND_RETRIES; i++) {
> -                       usleep_range(10000, 11000);
> -
> -                       trace_cros_ec_request_start(status_msg);
> -                       ret = (*xfer_fxn)(ec_dev, status_msg);
> -                       trace_cros_ec_request_done(status_msg, ret);

I'll have to spend more time on this; I don't immediately see (from
the diff) the replacement for the traces, and I don't see what
happened with xfer_fxn.

Guenter

> -                       if (ret == -EAGAIN)
> -                               continue;
> -                       if (ret < 0)
> -                               break;
> -
> -                       msg->result = status_msg->result;
> -                       if (status_msg->result != EC_RES_SUCCESS)
> -                               break;
> -
> -                       status = (struct ec_response_get_comms_status *)
> -                                status_msg->data;
> -                       if (!(status->flags & EC_COMMS_STATUS_PROCESSING))
> -                               break;
> -               }
> +               ret = cros_ec_xfer_command(ec_dev, msg);
> +               if (ret == -EAGAIN)
> +                       continue;
> +               if (ret < 0)
> +                       break;
> +
> +               *result = msg->result;
> +               if (msg->result != EC_RES_SUCCESS)
> +                       break;
>
> -               kfree(status_msg);
> +               if (!(status->flags & EC_COMMS_STATUS_PROCESSING))
> +                       break;
>         }
>
> +       kfree(msg);
> +       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)
> +               ret = cros_ec_wait_until_complete(ec_dev, &msg->result);
> +
>         return ret;
>  }
>
> --
> 2.36.1.476.g0c4daa206d-goog
>

Re: [PATCH 05/11] platform/chrome: cros_ec_proto: separate cros_ec_wait_until_complete()

[Tzung-Bi Shih]

On Wed, Jun 15, 2022 at 01:50:26PM -0700, Guenter Roeck wrote:
> On Tue, Jun 14, 2022 at 10:13 PM Tzung-Bi Shih <tzungbi@kernel.org> wrote:
> > -static int cros_ec_send_command(struct cros_ec_device *ec_dev, struct cros_ec_command *msg)
> > +static int cros_ec_wait_until_complete(struct cros_ec_device *ec_dev, uint32_t *result)
> >  {
> > -       int ret = cros_ec_xfer_command(ec_dev, msg);
> > +       struct cros_ec_command *msg;
> > +       struct ec_response_get_comms_status *status;
> > +       int ret = 0, i;
> > +
> > +       msg = kzalloc(sizeof(*msg) + sizeof(*status), GFP_KERNEL);
> > +       if (!msg)
> > +               return -ENOMEM;
> >
> > -       if (msg->result == EC_RES_IN_PROGRESS) {
> > -               int i;
> > -               struct cros_ec_command *status_msg;
> > -               struct ec_response_get_comms_status *status;
> > +       msg->command = EC_CMD_GET_COMMS_STATUS;
> > +       msg->insize = sizeof(*status);
> >
> > -               status_msg = kmalloc(sizeof(*status_msg) + sizeof(*status),
> > -                                    GFP_KERNEL);
> > -               if (!status_msg)
> > -                       return -ENOMEM;
> > +       status = (struct ec_response_get_comms_status *)msg->data;
> >
> > -               status_msg->version = 0;
> > -               status_msg->command = EC_CMD_GET_COMMS_STATUS;
> > -               status_msg->insize = sizeof(*status);
> > -               status_msg->outsize = 0;
> > +       /* Query the EC's status until it's no longer busy or we encounter an error. */
> > +       for (i = 0; i < EC_COMMAND_RETRIES; ++i) {
> > +               usleep_range(10000, 11000);
> >
> > -               /*
> > -                * Query the EC's status until it's no longer busy or
> > -                * we encounter an error.
> > -                */
> > -               for (i = 0; i < EC_COMMAND_RETRIES; i++) {
> > -                       usleep_range(10000, 11000);
> > -
> > -                       trace_cros_ec_request_start(status_msg);
> > -                       ret = (*xfer_fxn)(ec_dev, status_msg);
> > -                       trace_cros_ec_request_done(status_msg, ret);
> 
> I'll have to spend more time on this; I don't immediately see (from
> the diff) the replacement for the traces, and I don't see what
> happened with xfer_fxn.

They are replaced by calling cros_ec_xfer_command() (see below) which does the
same things.

> > -                       if (ret == -EAGAIN)
> > -                               continue;
> > -                       if (ret < 0)
> > -                               break;
> > -
> > -                       msg->result = status_msg->result;
> > -                       if (status_msg->result != EC_RES_SUCCESS)
> > -                               break;
> > -
> > -                       status = (struct ec_response_get_comms_status *)
> > -                                status_msg->data;
> > -                       if (!(status->flags & EC_COMMS_STATUS_PROCESSING))
> > -                               break;
> > -               }
> > +               ret = cros_ec_xfer_command(ec_dev, msg);
> > +               if (ret == -EAGAIN)
> > +                       continue;
> > +               if (ret < 0)
> > +                       break;