* [PATCH v2 01/15] platform/chrome: cros_ec_commands: fix compile errors
2022-06-07 14:56 [PATCH v2 00/15] platform/chrome: Kunit tests and refactor for cros_ec_query_all() Tzung-Bi Shih
@ 2022-06-07 14:56 ` Tzung-Bi Shih
2022-06-07 14:56 ` [PATCH v2 02/15] platform/chrome: cros_ec_proto: add Kunit tests for cros_ec_query_all() Tzung-Bi Shih
` (13 subsequent siblings)
14 siblings, 0 replies; 28+ messages in thread
From: Tzung-Bi Shih @ 2022-06-07 14:56 UTC (permalink / raw)
To: bleung, groeck; +Cc: chrome-platform, linux-kernel, tzungbi
Fix compile errors when including cros_ec_commands.h solely.
1.
cros_ec_commands.h:587:9: error: unknown type name 'uint8_t'
587 | uint8_t flags;
| ^~~~~~~
2.
cros_ec_commands.h:1105:43: error: implicit declaration of function 'BIT'
1105 | EC_COMMS_STATUS_PROCESSING = BIT(0),
| ^~~
Reviewed-by: Guenter Roeck <groeck@chromium.org>
Signed-off-by: Tzung-Bi Shih <tzungbi@kernel.org>
---
Changes from v1:
- Add R-b tag.
include/linux/platform_data/cros_ec_commands.h | 4 ++--
1 file changed, 2 insertions(+), 2 deletions(-)
diff --git a/include/linux/platform_data/cros_ec_commands.h b/include/linux/platform_data/cros_ec_commands.h
index 8cfa8cfca77e..a5b749a85707 100644
--- a/include/linux/platform_data/cros_ec_commands.h
+++ b/include/linux/platform_data/cros_ec_commands.h
@@ -13,8 +13,8 @@
#ifndef __CROS_EC_COMMANDS_H
#define __CROS_EC_COMMANDS_H
-
-
+#include <linux/bits.h>
+#include <linux/types.h>
#define BUILD_ASSERT(_cond)
--
2.36.1.255.ge46751e96f-goog
^ permalink raw reply related [flat|nested] 28+ messages in thread* [PATCH v2 02/15] platform/chrome: cros_ec_proto: add Kunit tests for cros_ec_query_all()
2022-06-07 14:56 [PATCH v2 00/15] platform/chrome: Kunit tests and refactor for cros_ec_query_all() Tzung-Bi Shih
2022-06-07 14:56 ` [PATCH v2 01/15] platform/chrome: cros_ec_commands: fix compile errors Tzung-Bi Shih
@ 2022-06-07 14:56 ` Tzung-Bi Shih
2022-06-07 14:56 ` [PATCH v2 03/15] platform/chrome: use macros for passthru indexes Tzung-Bi Shih
` (12 subsequent siblings)
14 siblings, 0 replies; 28+ messages in thread
From: Tzung-Bi Shih @ 2022-06-07 14:56 UTC (permalink / raw)
To: bleung, groeck; +Cc: chrome-platform, linux-kernel, tzungbi
cros_ec_query_all() sends multiple host commands to EC for querying
supported protocols and settings.
Add required mock for interacting with cros_ec_query_all() and Kunit
tests.
Signed-off-by: Tzung-Bi Shih <tzungbi@kernel.org>
---
Changes from v1:
- Initialize the mock ec_dev->dev more to get rid of kernel WARN().
- Elaborate more on the test case names.
drivers/platform/chrome/Kconfig | 6 +
drivers/platform/chrome/Makefile | 1 +
drivers/platform/chrome/cros_ec_proto_test.c | 801 +++++++++++++++++++
drivers/platform/chrome/cros_kunit_util.c | 98 +++
drivers/platform/chrome/cros_kunit_util.h | 36 +
5 files changed, 942 insertions(+)
create mode 100644 drivers/platform/chrome/cros_kunit_util.c
create mode 100644 drivers/platform/chrome/cros_kunit_util.h
diff --git a/drivers/platform/chrome/Kconfig b/drivers/platform/chrome/Kconfig
index 4b3d2427e8dd..0b069d874845 100644
--- a/drivers/platform/chrome/Kconfig
+++ b/drivers/platform/chrome/Kconfig
@@ -268,11 +268,17 @@ config CHROMEOS_PRIVACY_SCREEN
source "drivers/platform/chrome/wilco_ec/Kconfig"
# Kunit test cases
+config CROS_KUNIT
+ tristate
+ help
+ ChromeOS Kunit.
+
config CROS_EC_PROTO_KUNIT_TEST
tristate "Kunit tests for ChromeOS EC protocol" if !KUNIT_ALL_TESTS
depends on KUNIT && CROS_EC
default KUNIT_ALL_TESTS
select CROS_EC_PROTO
+ select CROS_KUNIT
help
Kunit tests for the ChromeOS Embedded Controller protocol.
diff --git a/drivers/platform/chrome/Makefile b/drivers/platform/chrome/Makefile
index 3c380066c6b6..a06bc56d12a8 100644
--- a/drivers/platform/chrome/Makefile
+++ b/drivers/platform/chrome/Makefile
@@ -32,4 +32,5 @@ obj-$(CONFIG_CROS_USBPD_NOTIFY) += cros_usbpd_notify.o
obj-$(CONFIG_WILCO_EC) += wilco_ec/
# Kunit test cases
+obj-$(CONFIG_CROS_KUNIT) += cros_kunit_util.o
obj-$(CONFIG_CROS_EC_PROTO_KUNIT_TEST) += cros_ec_proto_test.o
diff --git a/drivers/platform/chrome/cros_ec_proto_test.c b/drivers/platform/chrome/cros_ec_proto_test.c
index 25c4fca5c165..7ec53de4e256 100644
--- a/drivers/platform/chrome/cros_ec_proto_test.c
+++ b/drivers/platform/chrome/cros_ec_proto_test.c
@@ -9,6 +9,7 @@
#include <linux/platform_data/cros_ec_proto.h>
#include "cros_ec.h"
+#include "cros_kunit_util.h"
#define BUFSIZE 512
@@ -172,6 +173,778 @@ static void cros_ec_proto_test_check_result(struct kunit *test)
KUNIT_EXPECT_EQ(test, ret, -EAGAIN);
}
+static void cros_ec_proto_test_query_all_pretest(struct kunit *test)
+{
+ struct cros_ec_proto_test_priv *priv = test->priv;
+ struct cros_ec_device *ec_dev = &priv->ec_dev;
+
+ /*
+ * cros_ec_query_all() will free din and dout and allocate them again to fit the usage by
+ * calling devm_kfree() and devm_kzalloc(). Set them to NULL as they aren't managed by
+ * ec_dev->dev.
+ */
+ ec_dev->din = NULL;
+ ec_dev->dout = NULL;
+}
+
+static void cros_ec_proto_test_query_all_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;
+
+ /* For cros_ec_host_command_proto_query() without passthru. */
+ {
+ struct ec_response_get_protocol_info *data;
+
+ mock = cros_kunit_ec_xfer_mock_add(test, sizeof(*data));
+ KUNIT_ASSERT_PTR_NE(test, mock, NULL);
+
+ data = (struct ec_response_get_protocol_info *)mock->o_data;
+ data->protocol_versions = BIT(3) | BIT(2);
+ data->max_request_packet_size = 0xbe;
+ data->max_response_packet_size = 0xef;
+ }
+
+ /* For cros_ec_host_command_proto_query() with passthru. */
+ {
+ struct ec_response_get_protocol_info *data;
+
+ mock = cros_kunit_ec_xfer_mock_add(test, sizeof(*data));
+ KUNIT_ASSERT_PTR_NE(test, mock, NULL);
+
+ data = (struct ec_response_get_protocol_info *)mock->o_data;
+ data->max_request_packet_size = 0xbf;
+ }
+
+ /* For cros_ec_get_host_command_version_mask() for MKBP. */
+ {
+ struct ec_response_get_cmd_versions *data;
+
+ mock = cros_kunit_ec_xfer_mock_add(test, sizeof(*data));
+ KUNIT_ASSERT_PTR_NE(test, mock, NULL);
+
+ data = (struct ec_response_get_cmd_versions *)mock->o_data;
+ data->version_mask = BIT(6) | BIT(5);
+ }
+
+ /* For cros_ec_get_host_command_version_mask() for host sleep v1. */
+ {
+ struct ec_response_get_cmd_versions *data;
+
+ mock = cros_kunit_ec_xfer_mock_add(test, sizeof(*data));
+ KUNIT_ASSERT_PTR_NE(test, mock, NULL);
+
+ data = (struct ec_response_get_cmd_versions *)mock->o_data;
+ data->version_mask = BIT(1);
+ }
+
+ /* For cros_ec_get_host_event_wake_mask(). */
+ {
+ struct ec_response_host_event_mask *data;
+
+ mock = cros_kunit_ec_xfer_mock_add(test, sizeof(*data));
+ KUNIT_ASSERT_PTR_NE(test, mock, NULL);
+
+ data = (struct ec_response_host_event_mask *)mock->o_data;
+ data->mask = 0xbeef;
+ }
+
+ cros_ec_proto_test_query_all_pretest(test);
+ ret = cros_ec_query_all(ec_dev);
+ KUNIT_EXPECT_EQ(test, ret, 0);
+
+ /* For cros_ec_host_command_proto_query() without passthru. */
+ {
+ 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_PROTOCOL_INFO);
+ KUNIT_EXPECT_EQ(test, mock->msg.insize,
+ sizeof(struct ec_response_get_protocol_info));
+ KUNIT_EXPECT_EQ(test, mock->msg.outsize, 0);
+
+ KUNIT_EXPECT_EQ(test, ec_dev->max_request, 0xbe - sizeof(struct ec_host_request));
+ KUNIT_EXPECT_EQ(test, ec_dev->max_response, 0xef - sizeof(struct ec_host_response));
+ KUNIT_EXPECT_EQ(test, ec_dev->proto_version, 3);
+ KUNIT_EXPECT_EQ(test, ec_dev->din_size, 0xef + EC_MAX_RESPONSE_OVERHEAD);
+ KUNIT_EXPECT_EQ(test, ec_dev->dout_size, 0xbe + EC_MAX_REQUEST_OVERHEAD);
+ }
+
+ /* For cros_ec_host_command_proto_query() with passthru. */
+ {
+ 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_PASSTHRU_OFFSET(1) | EC_CMD_GET_PROTOCOL_INFO);
+ KUNIT_EXPECT_EQ(test, mock->msg.insize,
+ sizeof(struct ec_response_get_protocol_info));
+ KUNIT_EXPECT_EQ(test, mock->msg.outsize, 0);
+
+ KUNIT_EXPECT_EQ(test, ec_dev->max_passthru, 0xbf - sizeof(struct ec_host_request));
+ }
+
+ /* For cros_ec_get_host_command_version_mask() for MKBP. */
+ {
+ struct ec_params_get_cmd_versions *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_GET_CMD_VERSIONS);
+ KUNIT_EXPECT_EQ(test, mock->msg.insize,
+ sizeof(struct ec_response_get_cmd_versions));
+ KUNIT_EXPECT_EQ(test, mock->msg.outsize, sizeof(*data));
+
+ data = (struct ec_params_get_cmd_versions *)mock->i_data;
+ KUNIT_EXPECT_EQ(test, data->cmd, EC_CMD_GET_NEXT_EVENT);
+
+ KUNIT_EXPECT_EQ(test, ec_dev->mkbp_event_supported, 7);
+ }
+
+ /* For cros_ec_get_host_command_version_mask() for host sleep v1. */
+ {
+ struct ec_params_get_cmd_versions *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_GET_CMD_VERSIONS);
+ KUNIT_EXPECT_EQ(test, mock->msg.insize,
+ sizeof(struct ec_response_get_cmd_versions));
+ KUNIT_EXPECT_EQ(test, mock->msg.outsize, sizeof(*data));
+
+ data = (struct ec_params_get_cmd_versions *)mock->i_data;
+ KUNIT_EXPECT_EQ(test, data->cmd, EC_CMD_HOST_SLEEP_EVENT);
+
+ KUNIT_EXPECT_TRUE(test, ec_dev->host_sleep_v1);
+ }
+
+ /* For cros_ec_get_host_event_wake_mask(). */
+ {
+ 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_HOST_EVENT_GET_WAKE_MASK);
+ KUNIT_EXPECT_EQ(test, mock->msg.insize, sizeof(struct ec_response_host_event_mask));
+ KUNIT_EXPECT_EQ(test, mock->msg.outsize, 0);
+
+ KUNIT_EXPECT_EQ(test, ec_dev->host_event_wake_mask, 0xbeef);
+ }
+}
+
+static void cros_ec_proto_test_query_all_no_pd_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;
+
+ /* Set some garbage bytes. */
+ ec_dev->max_passthru = 0xbf;
+
+ /* For cros_ec_host_command_proto_query() without passthru. */
+ {
+ struct ec_response_get_protocol_info *data;
+
+ mock = cros_kunit_ec_xfer_mock_add(test, sizeof(*data));
+ KUNIT_ASSERT_PTR_NE(test, mock, NULL);
+
+ /*
+ * Although it doesn't check the value, provides valid sizes so that
+ * cros_ec_query_all() allocates din and dout correctly.
+ */
+ data = (struct ec_response_get_protocol_info *)mock->o_data;
+ data->max_request_packet_size = 0xbe;
+ data->max_response_packet_size = 0xef;
+ }
+
+ /* For cros_ec_host_command_proto_query() with passthru. */
+ {
+ mock = cros_kunit_ec_xfer_mock_addx(test, 0, EC_RES_INVALID_COMMAND, 0);
+ KUNIT_ASSERT_PTR_NE(test, mock, NULL);
+ }
+
+ cros_ec_proto_test_query_all_pretest(test);
+ ret = cros_ec_query_all(ec_dev);
+ KUNIT_EXPECT_EQ(test, ret, 0);
+
+ /* For cros_ec_host_command_proto_query() without passthru. */
+ {
+ 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_PROTOCOL_INFO);
+ KUNIT_EXPECT_EQ(test, mock->msg.insize,
+ sizeof(struct ec_response_get_protocol_info));
+ KUNIT_EXPECT_EQ(test, mock->msg.outsize, 0);
+ }
+
+ /* For cros_ec_host_command_proto_query() with passthru. */
+ {
+ 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_PASSTHRU_OFFSET(1) | EC_CMD_GET_PROTOCOL_INFO);
+ KUNIT_EXPECT_EQ(test, mock->msg.insize,
+ sizeof(struct ec_response_get_protocol_info));
+ KUNIT_EXPECT_EQ(test, mock->msg.outsize, 0);
+
+ KUNIT_EXPECT_EQ(test, ec_dev->max_passthru, 0);
+ }
+}
+
+static void cros_ec_proto_test_query_all_legacy_normal_v3_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;
+
+ /* For cros_ec_host_command_proto_query() without passthru. */
+ {
+ mock = cros_kunit_ec_xfer_mock_addx(test, 0, EC_RES_INVALID_COMMAND, 0);
+ KUNIT_ASSERT_PTR_NE(test, mock, NULL);
+ }
+
+ /* For cros_ec_host_command_proto_query_v2(). */
+ {
+ struct ec_response_hello *data;
+
+ mock = cros_kunit_ec_xfer_mock_add(test, sizeof(*data));
+ KUNIT_ASSERT_PTR_NE(test, mock, NULL);
+
+ data = (struct ec_response_hello *)mock->o_data;
+ data->out_data = 0xa1b2c3d4;
+ }
+
+ cros_ec_proto_test_query_all_pretest(test);
+ ret = cros_ec_query_all(ec_dev);
+ KUNIT_EXPECT_EQ(test, ret, 0);
+
+ /* For cros_ec_host_command_proto_query() without passthru. */
+ {
+ 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_PROTOCOL_INFO);
+ KUNIT_EXPECT_EQ(test, mock->msg.insize,
+ sizeof(struct ec_response_get_protocol_info));
+ KUNIT_EXPECT_EQ(test, mock->msg.outsize, 0);
+ }
+
+ /* For cros_ec_host_command_proto_query_v2(). */
+ {
+ struct ec_params_hello *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, sizeof(struct ec_response_hello));
+ KUNIT_EXPECT_EQ(test, mock->msg.outsize, sizeof(*data));
+
+ data = (struct ec_params_hello *)mock->i_data;
+ KUNIT_EXPECT_EQ(test, data->in_data, 0xa0b0c0d0);
+
+ KUNIT_EXPECT_EQ(test, ec_dev->proto_version, 2);
+ KUNIT_EXPECT_EQ(test, ec_dev->max_request, EC_PROTO2_MAX_PARAM_SIZE);
+ KUNIT_EXPECT_EQ(test, ec_dev->max_response, EC_PROTO2_MAX_PARAM_SIZE);
+ KUNIT_EXPECT_EQ(test, ec_dev->max_passthru, 0);
+ KUNIT_EXPECT_PTR_EQ(test, ec_dev->pkt_xfer, NULL);
+ KUNIT_EXPECT_EQ(test, ec_dev->din_size, EC_PROTO2_MSG_BYTES);
+ KUNIT_EXPECT_EQ(test, ec_dev->dout_size, EC_PROTO2_MSG_BYTES);
+ }
+}
+
+static void cros_ec_proto_test_query_all_legacy_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;
+
+ /* For cros_ec_host_command_proto_query() without passthru. */
+ {
+ mock = cros_kunit_ec_xfer_mock_addx(test, 0, EC_RES_INVALID_COMMAND, 0);
+ KUNIT_ASSERT_PTR_NE(test, mock, NULL);
+ }
+
+ /* For cros_ec_host_command_proto_query_v2(). */
+ {
+ mock = cros_kunit_ec_xfer_mock_addx(test, -EIO, EC_RES_SUCCESS, 0);
+ KUNIT_ASSERT_PTR_NE(test, mock, NULL);
+ }
+
+ cros_ec_proto_test_query_all_pretest(test);
+ ret = cros_ec_query_all(ec_dev);
+ KUNIT_EXPECT_EQ(test, ret, -EIO);
+ KUNIT_EXPECT_EQ(test, ec_dev->proto_version, EC_PROTO_VERSION_UNKNOWN);
+
+ /* For cros_ec_host_command_proto_query() without passthru. */
+ {
+ 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_PROTOCOL_INFO);
+ KUNIT_EXPECT_EQ(test, mock->msg.insize,
+ sizeof(struct ec_response_get_protocol_info));
+ KUNIT_EXPECT_EQ(test, mock->msg.outsize, 0);
+ }
+
+ /* For cros_ec_host_command_proto_query_v2(). */
+ {
+ 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, sizeof(struct ec_response_hello));
+ KUNIT_EXPECT_EQ(test, mock->msg.outsize, sizeof(struct ec_params_hello));
+ }
+}
+
+static void cros_ec_proto_test_query_all_legacy_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;
+
+ /* For cros_ec_host_command_proto_query() without passthru. */
+ {
+ mock = cros_kunit_ec_xfer_mock_addx(test, 0, EC_RES_INVALID_COMMAND, 0);
+ KUNIT_ASSERT_PTR_NE(test, mock, NULL);
+ }
+
+ /* For cros_ec_host_command_proto_query_v2(). */
+ {
+ mock = cros_kunit_ec_xfer_mock_addx(test, 0, EC_RES_INVALID_COMMAND, 0);
+ KUNIT_ASSERT_PTR_NE(test, mock, NULL);
+ }
+
+ cros_ec_proto_test_query_all_pretest(test);
+ ret = cros_ec_query_all(ec_dev);
+ KUNIT_EXPECT_EQ(test, ret, EC_RES_INVALID_COMMAND);
+ KUNIT_EXPECT_EQ(test, ec_dev->proto_version, EC_PROTO_VERSION_UNKNOWN);
+
+ /* For cros_ec_host_command_proto_query() without passthru. */
+ {
+ 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_PROTOCOL_INFO);
+ KUNIT_EXPECT_EQ(test, mock->msg.insize,
+ sizeof(struct ec_response_get_protocol_info));
+ KUNIT_EXPECT_EQ(test, mock->msg.outsize, 0);
+ }
+
+ /* For cros_ec_host_command_proto_query_v2(). */
+ {
+ 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, sizeof(struct ec_response_hello));
+ KUNIT_EXPECT_EQ(test, mock->msg.outsize, sizeof(struct ec_params_hello));
+ }
+}
+
+static void cros_ec_proto_test_query_all_legacy_data_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;
+
+ /* For cros_ec_host_command_proto_query() without passthru. */
+ {
+ mock = cros_kunit_ec_xfer_mock_addx(test, 0, EC_RES_INVALID_COMMAND, 0);
+ KUNIT_ASSERT_PTR_NE(test, mock, NULL);
+ }
+
+ /* For cros_ec_host_command_proto_query_v2(). */
+ {
+ struct ec_response_hello *data;
+
+ mock = cros_kunit_ec_xfer_mock_add(test, sizeof(*data));
+ KUNIT_ASSERT_PTR_NE(test, mock, NULL);
+
+ data = (struct ec_response_hello *)mock->o_data;
+ data->out_data = 0xbeefbfbf;
+ }
+
+ cros_ec_proto_test_query_all_pretest(test);
+ ret = cros_ec_query_all(ec_dev);
+ KUNIT_EXPECT_EQ(test, ret, -EBADMSG);
+ KUNIT_EXPECT_EQ(test, ec_dev->proto_version, EC_PROTO_VERSION_UNKNOWN);
+
+ /* For cros_ec_host_command_proto_query() without passthru. */
+ {
+ 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_PROTOCOL_INFO);
+ KUNIT_EXPECT_EQ(test, mock->msg.insize,
+ sizeof(struct ec_response_get_protocol_info));
+ KUNIT_EXPECT_EQ(test, mock->msg.outsize, 0);
+ }
+
+ /* For cros_ec_host_command_proto_query_v2(). */
+ {
+ 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, sizeof(struct ec_response_hello));
+ KUNIT_EXPECT_EQ(test, mock->msg.outsize, sizeof(struct ec_params_hello));
+ }
+}
+
+static void cros_ec_proto_test_query_all_no_mkbp(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;
+
+ /* Set some garbage bytes. */
+ ec_dev->mkbp_event_supported = 0xbf;
+
+ /* For cros_ec_host_command_proto_query() without passthru. */
+ {
+ struct ec_response_get_protocol_info *data;
+
+ mock = cros_kunit_ec_xfer_mock_add(test, sizeof(*data));
+ KUNIT_ASSERT_PTR_NE(test, mock, NULL);
+
+ /*
+ * Although it doesn't check the value, provides valid sizes so that
+ * cros_ec_query_all() allocates din and dout correctly.
+ */
+ data = (struct ec_response_get_protocol_info *)mock->o_data;
+ data->max_request_packet_size = 0xbe;
+ data->max_response_packet_size = 0xef;
+ }
+
+ /* For cros_ec_host_command_proto_query() with passthru. */
+ {
+ mock = cros_kunit_ec_xfer_mock_addx(test, 0, EC_RES_INVALID_COMMAND, 0);
+ KUNIT_ASSERT_PTR_NE(test, mock, NULL);
+ }
+
+ /* For cros_ec_get_host_command_version_mask() for MKBP. */
+ {
+ struct ec_response_get_cmd_versions *data;
+
+ mock = cros_kunit_ec_xfer_mock_add(test, sizeof(*data));
+ KUNIT_ASSERT_PTR_NE(test, mock, NULL);
+
+ data = (struct ec_response_get_cmd_versions *)mock->o_data;
+ data->version_mask = 0;
+ }
+
+ cros_ec_proto_test_query_all_pretest(test);
+ ret = cros_ec_query_all(ec_dev);
+ KUNIT_EXPECT_EQ(test, ret, 0);
+
+ /* For cros_ec_host_command_proto_query() without passthru. */
+ {
+ 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_PROTOCOL_INFO);
+ KUNIT_EXPECT_EQ(test, mock->msg.insize,
+ sizeof(struct ec_response_get_protocol_info));
+ KUNIT_EXPECT_EQ(test, mock->msg.outsize, 0);
+ }
+
+ /* For cros_ec_host_command_proto_query() with passthru. */
+ {
+ 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_PASSTHRU_OFFSET(1) | EC_CMD_GET_PROTOCOL_INFO);
+ KUNIT_EXPECT_EQ(test, mock->msg.insize,
+ sizeof(struct ec_response_get_protocol_info));
+ KUNIT_EXPECT_EQ(test, mock->msg.outsize, 0);
+ }
+
+ /* For cros_ec_get_host_command_version_mask() for MKBP. */
+ {
+ struct ec_params_get_cmd_versions *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_GET_CMD_VERSIONS);
+ KUNIT_EXPECT_EQ(test, mock->msg.insize,
+ sizeof(struct ec_response_get_cmd_versions));
+ KUNIT_EXPECT_EQ(test, mock->msg.outsize, sizeof(*data));
+
+ data = (struct ec_params_get_cmd_versions *)mock->i_data;
+ KUNIT_EXPECT_EQ(test, data->cmd, EC_CMD_GET_NEXT_EVENT);
+
+ KUNIT_EXPECT_EQ(test, ec_dev->mkbp_event_supported, 0);
+ }
+}
+
+static void cros_ec_proto_test_query_all_no_host_sleep(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;
+
+ /* Set some garbage bytes. */
+ ec_dev->host_sleep_v1 = true;
+
+ /* For cros_ec_host_command_proto_query() without passthru. */
+ {
+ struct ec_response_get_protocol_info *data;
+
+ mock = cros_kunit_ec_xfer_mock_add(test, sizeof(*data));
+ KUNIT_ASSERT_PTR_NE(test, mock, NULL);
+
+ /*
+ * Although it doesn't check the value, provides valid sizes so that
+ * cros_ec_query_all() allocates din and dout correctly.
+ */
+ data = (struct ec_response_get_protocol_info *)mock->o_data;
+ data->max_request_packet_size = 0xbe;
+ data->max_response_packet_size = 0xef;
+ }
+
+ /* For cros_ec_host_command_proto_query() with passthru. */
+ {
+ mock = cros_kunit_ec_xfer_mock_addx(test, 0, EC_RES_INVALID_COMMAND, 0);
+ KUNIT_ASSERT_PTR_NE(test, mock, NULL);
+ }
+
+ /* For cros_ec_get_host_command_version_mask() for MKBP. */
+ {
+ mock = cros_kunit_ec_xfer_mock_add(test, 0);
+ KUNIT_ASSERT_PTR_NE(test, mock, NULL);
+ }
+
+ /* For cros_ec_get_host_command_version_mask() for host sleep v1. */
+ {
+ struct ec_response_get_cmd_versions *data;
+
+ mock = cros_kunit_ec_xfer_mock_add(test, sizeof(*data));
+ KUNIT_ASSERT_PTR_NE(test, mock, NULL);
+
+ data = (struct ec_response_get_cmd_versions *)mock->o_data;
+ data->version_mask = 0;
+ }
+
+ cros_ec_proto_test_query_all_pretest(test);
+ ret = cros_ec_query_all(ec_dev);
+ KUNIT_EXPECT_EQ(test, ret, 0);
+
+ /* For cros_ec_host_command_proto_query() without passthru. */
+ {
+ 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_PROTOCOL_INFO);
+ KUNIT_EXPECT_EQ(test, mock->msg.insize,
+ sizeof(struct ec_response_get_protocol_info));
+ KUNIT_EXPECT_EQ(test, mock->msg.outsize, 0);
+ }
+
+ /* For cros_ec_host_command_proto_query() with passthru. */
+ {
+ 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_PASSTHRU_OFFSET(1) | EC_CMD_GET_PROTOCOL_INFO);
+ KUNIT_EXPECT_EQ(test, mock->msg.insize,
+ sizeof(struct ec_response_get_protocol_info));
+ KUNIT_EXPECT_EQ(test, mock->msg.outsize, 0);
+ }
+
+ /* For cros_ec_get_host_command_version_mask() for MKBP. */
+ {
+ 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_CMD_VERSIONS);
+ KUNIT_EXPECT_EQ(test, mock->msg.insize,
+ sizeof(struct ec_response_get_cmd_versions));
+ KUNIT_EXPECT_EQ(test, mock->msg.outsize, sizeof(struct ec_params_get_cmd_versions));
+ }
+
+ /* For cros_ec_get_host_command_version_mask() for host sleep v1. */
+ {
+ 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_CMD_VERSIONS);
+ KUNIT_EXPECT_EQ(test, mock->msg.insize,
+ sizeof(struct ec_response_get_cmd_versions));
+ KUNIT_EXPECT_EQ(test, mock->msg.outsize, sizeof(struct ec_params_get_cmd_versions));
+
+ KUNIT_EXPECT_FALSE(test, ec_dev->host_sleep_v1);
+ }
+}
+
+static void cros_ec_proto_test_query_all_default_wake_mask_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;
+
+ /* Set some garbage bytes. */
+ ec_dev->host_event_wake_mask = U32_MAX;
+
+ /* For cros_ec_host_command_proto_query() without passthru. */
+ {
+ struct ec_response_get_protocol_info *data;
+
+ mock = cros_kunit_ec_xfer_mock_add(test, sizeof(*data));
+ KUNIT_ASSERT_PTR_NE(test, mock, NULL);
+
+ /*
+ * Although it doesn't check the value, provides valid sizes so that
+ * cros_ec_query_all() allocates din and dout correctly.
+ */
+ data = (struct ec_response_get_protocol_info *)mock->o_data;
+ data->max_request_packet_size = 0xbe;
+ data->max_response_packet_size = 0xef;
+ }
+
+ /* For cros_ec_host_command_proto_query() with passthru. */
+ {
+ mock = cros_kunit_ec_xfer_mock_addx(test, 0, EC_RES_INVALID_COMMAND, 0);
+ KUNIT_ASSERT_PTR_NE(test, mock, NULL);
+ }
+
+ /* For cros_ec_get_host_command_version_mask() for MKBP. */
+ {
+ mock = cros_kunit_ec_xfer_mock_add(test, 0);
+ KUNIT_ASSERT_PTR_NE(test, mock, NULL);
+ }
+
+ /* For cros_ec_get_host_command_version_mask() for host sleep v1. */
+ {
+ mock = cros_kunit_ec_xfer_mock_add(test, 0);
+ KUNIT_ASSERT_PTR_NE(test, mock, NULL);
+ }
+
+ /* For cros_ec_get_host_event_wake_mask(). */
+ {
+ mock = cros_kunit_ec_xfer_mock_addx(test, 0, EC_RES_INVALID_COMMAND, 0);
+ KUNIT_ASSERT_PTR_NE(test, mock, NULL);
+ }
+
+ cros_ec_proto_test_query_all_pretest(test);
+ ret = cros_ec_query_all(ec_dev);
+ KUNIT_EXPECT_EQ(test, ret, 0);
+
+ /* For cros_ec_host_command_proto_query() without passthru. */
+ {
+ 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_PROTOCOL_INFO);
+ KUNIT_EXPECT_EQ(test, mock->msg.insize,
+ sizeof(struct ec_response_get_protocol_info));
+ KUNIT_EXPECT_EQ(test, mock->msg.outsize, 0);
+ }
+
+ /* For cros_ec_host_command_proto_query() with passthru. */
+ {
+ 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_PASSTHRU_OFFSET(1) | EC_CMD_GET_PROTOCOL_INFO);
+ KUNIT_EXPECT_EQ(test, mock->msg.insize,
+ sizeof(struct ec_response_get_protocol_info));
+ KUNIT_EXPECT_EQ(test, mock->msg.outsize, 0);
+ }
+
+ /* For cros_ec_get_host_command_version_mask() for MKBP. */
+ {
+ 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_CMD_VERSIONS);
+ KUNIT_EXPECT_EQ(test, mock->msg.insize,
+ sizeof(struct ec_response_get_cmd_versions));
+ KUNIT_EXPECT_EQ(test, mock->msg.outsize, sizeof(struct ec_params_get_cmd_versions));
+ }
+
+ /* For cros_ec_get_host_command_version_mask() for host sleep v1. */
+ {
+ 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_CMD_VERSIONS);
+ KUNIT_EXPECT_EQ(test, mock->msg.insize,
+ sizeof(struct ec_response_get_cmd_versions));
+ KUNIT_EXPECT_EQ(test, mock->msg.outsize, sizeof(struct ec_params_get_cmd_versions));
+ }
+
+ /* For cros_ec_get_host_event_wake_mask(). */
+ {
+ u32 mask;
+
+ 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_HOST_EVENT_GET_WAKE_MASK);
+ KUNIT_EXPECT_EQ(test, mock->msg.insize, sizeof(struct ec_response_host_event_mask));
+ KUNIT_EXPECT_EQ(test, mock->msg.outsize, 0);
+
+ mask = ec_dev->host_event_wake_mask;
+ KUNIT_EXPECT_EQ(test, mask & EC_HOST_EVENT_MASK(EC_HOST_EVENT_LID_CLOSED), 0);
+ KUNIT_EXPECT_EQ(test, mask & EC_HOST_EVENT_MASK(EC_HOST_EVENT_AC_DISCONNECTED), 0);
+ KUNIT_EXPECT_EQ(test, mask & EC_HOST_EVENT_MASK(EC_HOST_EVENT_BATTERY_LOW), 0);
+ KUNIT_EXPECT_EQ(test, mask & EC_HOST_EVENT_MASK(EC_HOST_EVENT_BATTERY_CRITICAL), 0);
+ KUNIT_EXPECT_EQ(test, mask & EC_HOST_EVENT_MASK(EC_HOST_EVENT_BATTERY), 0);
+ KUNIT_EXPECT_EQ(test, mask & EC_HOST_EVENT_MASK(EC_HOST_EVENT_PD_MCU), 0);
+ KUNIT_EXPECT_EQ(test, mask & EC_HOST_EVENT_MASK(EC_HOST_EVENT_BATTERY_STATUS), 0);
+ }
+}
+
+static void cros_ec_proto_test_release(struct device *dev)
+{
+}
+
static int cros_ec_proto_test_init(struct kunit *test)
{
struct cros_ec_proto_test_priv *priv;
@@ -188,24 +961,52 @@ static int cros_ec_proto_test_init(struct kunit *test)
ec_dev->din = (u8 *)priv->din;
ec_dev->din_size = ARRAY_SIZE(priv->din);
ec_dev->proto_version = EC_HOST_REQUEST_VERSION;
+ ec_dev->dev = kunit_kzalloc(test, sizeof(*ec_dev->dev), GFP_KERNEL);
+ if (!ec_dev->dev)
+ return -ENOMEM;
+ device_initialize(ec_dev->dev);
+ dev_set_name(ec_dev->dev, "cros_ec_proto_test");
+ ec_dev->dev->release = cros_ec_proto_test_release;
+ ec_dev->cmd_xfer = cros_kunit_ec_xfer_mock;
+ ec_dev->pkt_xfer = cros_kunit_ec_xfer_mock;
priv->msg = (struct cros_ec_command *)priv->_msg;
+ cros_kunit_mock_reset();
+
return 0;
}
+static void cros_ec_proto_test_exit(struct kunit *test)
+{
+ struct cros_ec_proto_test_priv *priv = test->priv;
+ struct cros_ec_device *ec_dev = &priv->ec_dev;
+
+ put_device(ec_dev->dev);
+}
+
static struct kunit_case cros_ec_proto_test_cases[] = {
KUNIT_CASE(cros_ec_proto_test_prepare_tx_legacy_normal),
KUNIT_CASE(cros_ec_proto_test_prepare_tx_legacy_bad_msg_outsize),
KUNIT_CASE(cros_ec_proto_test_prepare_tx_normal),
KUNIT_CASE(cros_ec_proto_test_prepare_tx_bad_msg_outsize),
KUNIT_CASE(cros_ec_proto_test_check_result),
+ KUNIT_CASE(cros_ec_proto_test_query_all_normal),
+ KUNIT_CASE(cros_ec_proto_test_query_all_no_pd_return_error),
+ KUNIT_CASE(cros_ec_proto_test_query_all_legacy_normal_v3_return_error),
+ KUNIT_CASE(cros_ec_proto_test_query_all_legacy_xfer_error),
+ KUNIT_CASE(cros_ec_proto_test_query_all_legacy_return_error),
+ KUNIT_CASE(cros_ec_proto_test_query_all_legacy_data_error),
+ KUNIT_CASE(cros_ec_proto_test_query_all_no_mkbp),
+ KUNIT_CASE(cros_ec_proto_test_query_all_no_host_sleep),
+ KUNIT_CASE(cros_ec_proto_test_query_all_default_wake_mask_return_error),
{}
};
static struct kunit_suite cros_ec_proto_test_suite = {
.name = "cros_ec_proto_test",
.init = cros_ec_proto_test_init,
+ .exit = cros_ec_proto_test_exit,
.test_cases = cros_ec_proto_test_cases,
};
diff --git a/drivers/platform/chrome/cros_kunit_util.c b/drivers/platform/chrome/cros_kunit_util.c
new file mode 100644
index 000000000000..e031777dea87
--- /dev/null
+++ b/drivers/platform/chrome/cros_kunit_util.c
@@ -0,0 +1,98 @@
+// SPDX-License-Identifier: GPL-2.0
+/*
+ * CrOS Kunit tests utilities.
+ */
+
+#include <kunit/test.h>
+
+#include <linux/list.h>
+#include <linux/minmax.h>
+#include <linux/platform_data/cros_ec_commands.h>
+#include <linux/platform_data/cros_ec_proto.h>
+
+#include "cros_ec.h"
+#include "cros_kunit_util.h"
+
+int cros_kunit_ec_xfer_mock_default_ret;
+EXPORT_SYMBOL_GPL(cros_kunit_ec_xfer_mock_default_ret);
+
+static struct list_head cros_kunit_ec_xfer_mock_in;
+static struct list_head cros_kunit_ec_xfer_mock_out;
+
+int cros_kunit_ec_xfer_mock(struct cros_ec_device *ec_dev, struct cros_ec_command *msg)
+{
+ struct ec_xfer_mock *mock;
+
+ mock = list_first_entry_or_null(&cros_kunit_ec_xfer_mock_in, struct ec_xfer_mock, list);
+ if (!mock)
+ return cros_kunit_ec_xfer_mock_default_ret;
+
+ list_del(&mock->list);
+
+ memcpy(&mock->msg, msg, sizeof(*msg));
+ if (msg->outsize) {
+ mock->i_data = kunit_kzalloc(mock->test, msg->outsize, GFP_KERNEL);
+ if (mock->i_data)
+ memcpy(mock->i_data, msg->data, msg->outsize);
+ }
+
+ msg->result = mock->result;
+ if (msg->insize)
+ memcpy(msg->data, mock->o_data, min(msg->insize, mock->o_data_len));
+
+ list_add_tail(&mock->list, &cros_kunit_ec_xfer_mock_out);
+
+ return mock->ret;
+}
+EXPORT_SYMBOL_GPL(cros_kunit_ec_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);
+}
+EXPORT_SYMBOL_GPL(cros_kunit_ec_xfer_mock_add);
+
+struct ec_xfer_mock *cros_kunit_ec_xfer_mock_addx(struct kunit *test,
+ int ret, int result, size_t size)
+{
+ struct ec_xfer_mock *mock;
+
+ mock = kunit_kzalloc(test, sizeof(*mock), GFP_KERNEL);
+ if (!mock)
+ return NULL;
+
+ list_add_tail(&mock->list, &cros_kunit_ec_xfer_mock_in);
+ mock->test = test;
+
+ mock->ret = ret;
+ mock->result = result;
+ mock->o_data = kunit_kzalloc(test, size, GFP_KERNEL);
+ if (!mock->o_data)
+ return NULL;
+ mock->o_data_len = size;
+
+ return mock;
+}
+EXPORT_SYMBOL_GPL(cros_kunit_ec_xfer_mock_addx);
+
+struct ec_xfer_mock *cros_kunit_ec_xfer_mock_next(void)
+{
+ struct ec_xfer_mock *mock;
+
+ mock = list_first_entry_or_null(&cros_kunit_ec_xfer_mock_out, struct ec_xfer_mock, list);
+ if (mock)
+ list_del(&mock->list);
+
+ return mock;
+}
+EXPORT_SYMBOL_GPL(cros_kunit_ec_xfer_mock_next);
+
+void cros_kunit_mock_reset(void)
+{
+ cros_kunit_ec_xfer_mock_default_ret = 0;
+ INIT_LIST_HEAD(&cros_kunit_ec_xfer_mock_in);
+ INIT_LIST_HEAD(&cros_kunit_ec_xfer_mock_out);
+}
+EXPORT_SYMBOL_GPL(cros_kunit_mock_reset);
+
+MODULE_LICENSE("GPL");
diff --git a/drivers/platform/chrome/cros_kunit_util.h b/drivers/platform/chrome/cros_kunit_util.h
new file mode 100644
index 000000000000..79c4525f873c
--- /dev/null
+++ b/drivers/platform/chrome/cros_kunit_util.h
@@ -0,0 +1,36 @@
+/* SPDX-License-Identifier: GPL-2.0 */
+/*
+ * CrOS Kunit tests utilities.
+ */
+
+#ifndef _CROS_KUNIT_UTIL_H_
+#define _CROS_KUNIT_UTIL_H_
+
+#include <linux/platform_data/cros_ec_proto.h>
+
+struct ec_xfer_mock {
+ struct list_head list;
+ struct kunit *test;
+
+ /* input */
+ struct cros_ec_command msg;
+ void *i_data;
+
+ /* output */
+ int ret;
+ int result;
+ void *o_data;
+ u32 o_data_len;
+};
+
+extern int cros_kunit_ec_xfer_mock_default_ret;
+
+int cros_kunit_ec_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);
+struct ec_xfer_mock *cros_kunit_ec_xfer_mock_next(void);
+
+void cros_kunit_mock_reset(void);
+
+#endif
--
2.36.1.255.ge46751e96f-goog
^ permalink raw reply related [flat|nested] 28+ messages in thread* [PATCH v2 03/15] platform/chrome: use macros for passthru indexes
2022-06-07 14:56 [PATCH v2 00/15] platform/chrome: Kunit tests and refactor for cros_ec_query_all() Tzung-Bi Shih
2022-06-07 14:56 ` [PATCH v2 01/15] platform/chrome: cros_ec_commands: fix compile errors Tzung-Bi Shih
2022-06-07 14:56 ` [PATCH v2 02/15] platform/chrome: cros_ec_proto: add Kunit tests for cros_ec_query_all() Tzung-Bi Shih
@ 2022-06-07 14:56 ` Tzung-Bi Shih
2022-06-07 14:56 ` [PATCH v2 04/15] platform/chrome: cros_ec_proto: assign buffer size from protocol info Tzung-Bi Shih
` (11 subsequent siblings)
14 siblings, 0 replies; 28+ messages in thread
From: Tzung-Bi Shih @ 2022-06-07 14:56 UTC (permalink / raw)
To: bleung, groeck; +Cc: chrome-platform, linux-kernel, tzungbi
Move passthru indexes for EC and PD devices to common header. Also use
them instead of literal constants.
Reviewed-by: Guenter Roeck <groeck@chromium.org>
Signed-off-by: Tzung-Bi Shih <tzungbi@kernel.org>
---
Changes from v1:
- Add R-b tag.
drivers/platform/chrome/cros_ec.c | 3 ---
drivers/platform/chrome/cros_ec_proto.c | 6 +++---
drivers/platform/chrome/cros_ec_proto_test.c | 15 ++++++++++-----
drivers/platform/chrome/cros_ec_trace.h | 8 ++++----
include/linux/platform_data/cros_ec_proto.h | 3 +++
5 files changed, 20 insertions(+), 15 deletions(-)
diff --git a/drivers/platform/chrome/cros_ec.c b/drivers/platform/chrome/cros_ec.c
index b3e94cdf7d1a..e51a3f2176c7 100644
--- a/drivers/platform/chrome/cros_ec.c
+++ b/drivers/platform/chrome/cros_ec.c
@@ -19,9 +19,6 @@
#include "cros_ec.h"
-#define CROS_EC_DEV_EC_INDEX 0
-#define CROS_EC_DEV_PD_INDEX 1
-
static struct cros_ec_platform ec_p = {
.ec_name = CROS_EC_DEV_NAME,
.cmd_offset = EC_CMD_PASSTHRU_OFFSET(CROS_EC_DEV_EC_INDEX),
diff --git a/drivers/platform/chrome/cros_ec_proto.c b/drivers/platform/chrome/cros_ec_proto.c
index 13ced9d2dd71..65191af5139c 100644
--- a/drivers/platform/chrome/cros_ec_proto.c
+++ b/drivers/platform/chrome/cros_ec_proto.c
@@ -433,7 +433,7 @@ int cros_ec_query_all(struct cros_ec_device *ec_dev)
/* First try sending with proto v3. */
ec_dev->proto_version = 3;
- ret = cros_ec_host_command_proto_query(ec_dev, 0, proto_msg);
+ ret = cros_ec_host_command_proto_query(ec_dev, CROS_EC_DEV_EC_INDEX, proto_msg);
if (ret == 0) {
proto_info = (struct ec_response_get_protocol_info *)
@@ -459,7 +459,7 @@ int cros_ec_query_all(struct cros_ec_device *ec_dev)
/*
* Check for PD
*/
- ret = cros_ec_host_command_proto_query(ec_dev, 1, proto_msg);
+ ret = cros_ec_host_command_proto_query(ec_dev, CROS_EC_DEV_PD_INDEX, proto_msg);
if (ret) {
dev_dbg(ec_dev->dev, "no PD chip found: %d\n", ret);
@@ -609,7 +609,7 @@ int cros_ec_cmd_xfer(struct cros_ec_device *ec_dev, struct cros_ec_command *msg)
msg->insize = ec_dev->max_response;
}
- if (msg->command < EC_CMD_PASSTHRU_OFFSET(1)) {
+ if (msg->command < EC_CMD_PASSTHRU_OFFSET(CROS_EC_DEV_PD_INDEX)) {
if (msg->outsize > ec_dev->max_request) {
dev_err(ec_dev->dev,
"request of size %u is too big (max: %u)\n",
diff --git a/drivers/platform/chrome/cros_ec_proto_test.c b/drivers/platform/chrome/cros_ec_proto_test.c
index 7ec53de4e256..628c6582ca78 100644
--- a/drivers/platform/chrome/cros_ec_proto_test.c
+++ b/drivers/platform/chrome/cros_ec_proto_test.c
@@ -280,7 +280,8 @@ static void cros_ec_proto_test_query_all_normal(struct kunit *test)
KUNIT_EXPECT_EQ(test, mock->msg.version, 0);
KUNIT_EXPECT_EQ(test, mock->msg.command,
- EC_CMD_PASSTHRU_OFFSET(1) | EC_CMD_GET_PROTOCOL_INFO);
+ EC_CMD_PASSTHRU_OFFSET(CROS_EC_DEV_PD_INDEX) |
+ EC_CMD_GET_PROTOCOL_INFO);
KUNIT_EXPECT_EQ(test, mock->msg.insize,
sizeof(struct ec_response_get_protocol_info));
KUNIT_EXPECT_EQ(test, mock->msg.outsize, 0);
@@ -395,7 +396,8 @@ static void cros_ec_proto_test_query_all_no_pd_return_error(struct kunit *test)
KUNIT_EXPECT_EQ(test, mock->msg.version, 0);
KUNIT_EXPECT_EQ(test, mock->msg.command,
- EC_CMD_PASSTHRU_OFFSET(1) | EC_CMD_GET_PROTOCOL_INFO);
+ EC_CMD_PASSTHRU_OFFSET(CROS_EC_DEV_PD_INDEX) |
+ EC_CMD_GET_PROTOCOL_INFO);
KUNIT_EXPECT_EQ(test, mock->msg.insize,
sizeof(struct ec_response_get_protocol_info));
KUNIT_EXPECT_EQ(test, mock->msg.outsize, 0);
@@ -684,7 +686,8 @@ static void cros_ec_proto_test_query_all_no_mkbp(struct kunit *test)
KUNIT_EXPECT_EQ(test, mock->msg.version, 0);
KUNIT_EXPECT_EQ(test, mock->msg.command,
- EC_CMD_PASSTHRU_OFFSET(1) | EC_CMD_GET_PROTOCOL_INFO);
+ EC_CMD_PASSTHRU_OFFSET(CROS_EC_DEV_PD_INDEX) |
+ EC_CMD_GET_PROTOCOL_INFO);
KUNIT_EXPECT_EQ(test, mock->msg.insize,
sizeof(struct ec_response_get_protocol_info));
KUNIT_EXPECT_EQ(test, mock->msg.outsize, 0);
@@ -782,7 +785,8 @@ static void cros_ec_proto_test_query_all_no_host_sleep(struct kunit *test)
KUNIT_EXPECT_EQ(test, mock->msg.version, 0);
KUNIT_EXPECT_EQ(test, mock->msg.command,
- EC_CMD_PASSTHRU_OFFSET(1) | EC_CMD_GET_PROTOCOL_INFO);
+ EC_CMD_PASSTHRU_OFFSET(CROS_EC_DEV_PD_INDEX) |
+ EC_CMD_GET_PROTOCOL_INFO);
KUNIT_EXPECT_EQ(test, mock->msg.insize,
sizeof(struct ec_response_get_protocol_info));
KUNIT_EXPECT_EQ(test, mock->msg.outsize, 0);
@@ -888,7 +892,8 @@ static void cros_ec_proto_test_query_all_default_wake_mask_return_error(struct k
KUNIT_EXPECT_EQ(test, mock->msg.version, 0);
KUNIT_EXPECT_EQ(test, mock->msg.command,
- EC_CMD_PASSTHRU_OFFSET(1) | EC_CMD_GET_PROTOCOL_INFO);
+ EC_CMD_PASSTHRU_OFFSET(CROS_EC_DEV_PD_INDEX) |
+ EC_CMD_GET_PROTOCOL_INFO);
KUNIT_EXPECT_EQ(test, mock->msg.insize,
sizeof(struct ec_response_get_protocol_info));
KUNIT_EXPECT_EQ(test, mock->msg.outsize, 0);
diff --git a/drivers/platform/chrome/cros_ec_trace.h b/drivers/platform/chrome/cros_ec_trace.h
index 9bb5cd2c98b8..d7e407de88df 100644
--- a/drivers/platform/chrome/cros_ec_trace.h
+++ b/drivers/platform/chrome/cros_ec_trace.h
@@ -30,8 +30,8 @@ TRACE_EVENT(cros_ec_request_start,
),
TP_fast_assign(
__entry->version = cmd->version;
- __entry->offset = cmd->command / EC_CMD_PASSTHRU_OFFSET(1);
- __entry->command = cmd->command % EC_CMD_PASSTHRU_OFFSET(1);
+ __entry->offset = cmd->command / EC_CMD_PASSTHRU_OFFSET(CROS_EC_DEV_PD_INDEX);
+ __entry->command = cmd->command % EC_CMD_PASSTHRU_OFFSET(CROS_EC_DEV_PD_INDEX);
__entry->outsize = cmd->outsize;
__entry->insize = cmd->insize;
),
@@ -55,8 +55,8 @@ TRACE_EVENT(cros_ec_request_done,
),
TP_fast_assign(
__entry->version = cmd->version;
- __entry->offset = cmd->command / EC_CMD_PASSTHRU_OFFSET(1);
- __entry->command = cmd->command % EC_CMD_PASSTHRU_OFFSET(1);
+ __entry->offset = cmd->command / EC_CMD_PASSTHRU_OFFSET(CROS_EC_DEV_PD_INDEX);
+ __entry->command = cmd->command % EC_CMD_PASSTHRU_OFFSET(CROS_EC_DEV_PD_INDEX);
__entry->outsize = cmd->outsize;
__entry->insize = cmd->insize;
__entry->result = cmd->result;
diff --git a/include/linux/platform_data/cros_ec_proto.h b/include/linux/platform_data/cros_ec_proto.h
index 138fd912c808..6475a8066f00 100644
--- a/include/linux/platform_data/cros_ec_proto.h
+++ b/include/linux/platform_data/cros_ec_proto.h
@@ -21,6 +21,9 @@
#define CROS_EC_DEV_SCP_NAME "cros_scp"
#define CROS_EC_DEV_TP_NAME "cros_tp"
+#define CROS_EC_DEV_EC_INDEX 0
+#define CROS_EC_DEV_PD_INDEX 1
+
/*
* The EC is unresponsive for a time after a reboot command. Add a
* simple delay to make sure that the bus stays locked.
--
2.36.1.255.ge46751e96f-goog
^ permalink raw reply related [flat|nested] 28+ messages in thread* [PATCH v2 04/15] platform/chrome: cros_ec_proto: assign buffer size from protocol info
2022-06-07 14:56 [PATCH v2 00/15] platform/chrome: Kunit tests and refactor for cros_ec_query_all() Tzung-Bi Shih
` (2 preceding siblings ...)
2022-06-07 14:56 ` [PATCH v2 03/15] platform/chrome: use macros for passthru indexes Tzung-Bi Shih
@ 2022-06-07 14:56 ` Tzung-Bi Shih
2022-06-07 14:56 ` [PATCH v2 05/15] platform/chrome: cros_ec_proto: remove redundant NULL check Tzung-Bi Shih
` (10 subsequent siblings)
14 siblings, 0 replies; 28+ messages in thread
From: Tzung-Bi Shih @ 2022-06-07 14:56 UTC (permalink / raw)
To: bleung, groeck; +Cc: chrome-platform, linux-kernel, tzungbi
`din_size` is calculated from `ec_dev->max_response`.
`ec_dev->max_response` is further calculated from the protocol info.
To make it clear, assign `din_size` and `dout_size` from protocol info
directly.
Reviewed-by: Guenter Roeck <groeck@chromium.org>
Signed-off-by: Tzung-Bi Shih <tzungbi@kernel.org>
---
Changes from v1:
- Add R-b tag.
drivers/platform/chrome/cros_ec_proto.c | 8 ++------
1 file changed, 2 insertions(+), 6 deletions(-)
diff --git a/drivers/platform/chrome/cros_ec_proto.c b/drivers/platform/chrome/cros_ec_proto.c
index 65191af5139c..629dce3e6ab3 100644
--- a/drivers/platform/chrome/cros_ec_proto.c
+++ b/drivers/platform/chrome/cros_ec_proto.c
@@ -449,12 +449,8 @@ int cros_ec_query_all(struct cros_ec_device *ec_dev)
"using proto v%u\n",
ec_dev->proto_version);
- ec_dev->din_size = ec_dev->max_response +
- sizeof(struct ec_host_response) +
- EC_MAX_RESPONSE_OVERHEAD;
- ec_dev->dout_size = ec_dev->max_request +
- sizeof(struct ec_host_request) +
- EC_MAX_REQUEST_OVERHEAD;
+ ec_dev->din_size = proto_info->max_response_packet_size + EC_MAX_RESPONSE_OVERHEAD;
+ ec_dev->dout_size = proto_info->max_request_packet_size + EC_MAX_REQUEST_OVERHEAD;
/*
* Check for PD
--
2.36.1.255.ge46751e96f-goog
^ permalink raw reply related [flat|nested] 28+ messages in thread* [PATCH v2 05/15] platform/chrome: cros_ec_proto: remove redundant NULL check
2022-06-07 14:56 [PATCH v2 00/15] platform/chrome: Kunit tests and refactor for cros_ec_query_all() Tzung-Bi Shih
` (3 preceding siblings ...)
2022-06-07 14:56 ` [PATCH v2 04/15] platform/chrome: cros_ec_proto: assign buffer size from protocol info Tzung-Bi Shih
@ 2022-06-07 14:56 ` Tzung-Bi Shih
2022-06-07 14:56 ` [PATCH v2 06/15] platform/chrome: cros_ec_proto: use cros_ec_map_error() Tzung-Bi Shih
` (9 subsequent siblings)
14 siblings, 0 replies; 28+ messages in thread
From: Tzung-Bi Shih @ 2022-06-07 14:56 UTC (permalink / raw)
To: bleung, groeck; +Cc: chrome-platform, linux-kernel, tzungbi
send_command() already checks if `ec_dev->pkt_xfer` is NULL. Remove the
redundant check.
Reviewed-by: Guenter Roeck <groeck@chromium.org>
Signed-off-by: Tzung-Bi Shih <tzungbi@kernel.org>
---
Changes from v1:
- Add R-b tag.
drivers/platform/chrome/cros_ec_proto.c | 3 ---
1 file changed, 3 deletions(-)
diff --git a/drivers/platform/chrome/cros_ec_proto.c b/drivers/platform/chrome/cros_ec_proto.c
index 629dce3e6ab3..1b851dcd20c9 100644
--- a/drivers/platform/chrome/cros_ec_proto.c
+++ b/drivers/platform/chrome/cros_ec_proto.c
@@ -281,9 +281,6 @@ static int cros_ec_host_command_proto_query(struct cros_ec_device *ec_dev,
*/
int ret;
- if (!ec_dev->pkt_xfer)
- return -EPROTONOSUPPORT;
-
memset(msg, 0, sizeof(*msg));
msg->command = EC_CMD_PASSTHRU_OFFSET(devidx) | EC_CMD_GET_PROTOCOL_INFO;
msg->insize = sizeof(struct ec_response_get_protocol_info);
--
2.36.1.255.ge46751e96f-goog
^ permalink raw reply related [flat|nested] 28+ messages in thread* [PATCH v2 06/15] platform/chrome: cros_ec_proto: use cros_ec_map_error()
2022-06-07 14:56 [PATCH v2 00/15] platform/chrome: Kunit tests and refactor for cros_ec_query_all() Tzung-Bi Shih
` (4 preceding siblings ...)
2022-06-07 14:56 ` [PATCH v2 05/15] platform/chrome: cros_ec_proto: remove redundant NULL check Tzung-Bi Shih
@ 2022-06-07 14:56 ` Tzung-Bi Shih
2022-06-07 14:56 ` [PATCH v2 07/15] platform/chrome: cros_ec_proto: separate cros_ec_get_proto_info() Tzung-Bi Shih
` (8 subsequent siblings)
14 siblings, 0 replies; 28+ messages in thread
From: Tzung-Bi Shih @ 2022-06-07 14:56 UTC (permalink / raw)
To: bleung, groeck; +Cc: chrome-platform, linux-kernel, tzungbi
Use cros_ec_map_error() in cros_ec_get_host_event_wake_mask().
The behavior of cros_ec_get_host_event_wake_mask() slightly changed. It
is acceptable because the caller only needs it returns negative integers
for indicating errors. Especially, the EC_RES_INVALID_COMMAND still
maps to -EOPNOTSUPP.
Reviewed-by: Guenter Roeck <groeck@chromium.org>
Signed-off-by: Tzung-Bi Shih <tzungbi@kernel.org>
---
Changes from v1:
- Add R-b tag.
drivers/platform/chrome/cros_ec_proto.c | 9 ++++-----
1 file changed, 4 insertions(+), 5 deletions(-)
diff --git a/drivers/platform/chrome/cros_ec_proto.c b/drivers/platform/chrome/cros_ec_proto.c
index 1b851dcd20c9..71ba6a56ad7c 100644
--- a/drivers/platform/chrome/cros_ec_proto.c
+++ b/drivers/platform/chrome/cros_ec_proto.c
@@ -247,7 +247,7 @@ static int cros_ec_get_host_event_wake_mask(struct cros_ec_device *ec_dev,
uint32_t *mask)
{
struct ec_response_host_event_mask *r;
- int ret;
+ int ret, mapped;
msg->command = EC_CMD_HOST_EVENT_GET_WAKE_MASK;
msg->version = 0;
@@ -256,10 +256,9 @@ static int cros_ec_get_host_event_wake_mask(struct cros_ec_device *ec_dev,
ret = send_command(ec_dev, msg);
if (ret >= 0) {
- if (msg->result == EC_RES_INVALID_COMMAND)
- return -EOPNOTSUPP;
- if (msg->result != EC_RES_SUCCESS)
- return -EPROTO;
+ mapped = cros_ec_map_error(msg->result);
+ if (mapped)
+ return mapped;
}
if (ret > 0) {
r = (struct ec_response_host_event_mask *)msg->data;
--
2.36.1.255.ge46751e96f-goog
^ permalink raw reply related [flat|nested] 28+ messages in thread* [PATCH v2 07/15] platform/chrome: cros_ec_proto: separate cros_ec_get_proto_info()
2022-06-07 14:56 [PATCH v2 00/15] platform/chrome: Kunit tests and refactor for cros_ec_query_all() Tzung-Bi Shih
` (5 preceding siblings ...)
2022-06-07 14:56 ` [PATCH v2 06/15] platform/chrome: cros_ec_proto: use cros_ec_map_error() Tzung-Bi Shih
@ 2022-06-07 14:56 ` Tzung-Bi Shih
2022-06-07 18:45 ` Guenter Roeck
2022-06-07 14:56 ` [PATCH v2 08/15] platform/chrome: cros_ec_proto: handle empty payload in getting proto info Tzung-Bi Shih
` (7 subsequent siblings)
14 siblings, 1 reply; 28+ messages in thread
From: Tzung-Bi Shih @ 2022-06-07 14:56 UTC (permalink / raw)
To: bleung, groeck; +Cc: chrome-platform, linux-kernel, tzungbi
Rename cros_ec_host_command_proto_query() to cros_ec_get_proto_info()
and make it responsible for setting `ec_dev` fields according to the
response protocol info.
Also make cros_ec_get_host_event_wake_mask() allocate its own message
buffer. It was lucky that size of `struct ec_response_host_event_mask`
is less than `struct ec_response_get_protocol_info`. Thus, the buffer
wasn't overflow.
Signed-off-by: Tzung-Bi Shih <tzungbi@kernel.org>
---
Changes from v1:
- Preserve the "cros_ec_" prefix.
drivers/platform/chrome/cros_ec_proto.c | 134 +++++++++----------
drivers/platform/chrome/cros_ec_proto_test.c | 56 ++++----
2 files changed, 93 insertions(+), 97 deletions(-)
diff --git a/drivers/platform/chrome/cros_ec_proto.c b/drivers/platform/chrome/cros_ec_proto.c
index 71ba6a56ad7c..893b76703da6 100644
--- a/drivers/platform/chrome/cros_ec_proto.c
+++ b/drivers/platform/chrome/cros_ec_proto.c
@@ -242,47 +242,53 @@ EXPORT_SYMBOL(cros_ec_check_result);
* the caller has ec_dev->lock mutex, or the caller knows there is
* no other command in progress.
*/
-static int cros_ec_get_host_event_wake_mask(struct cros_ec_device *ec_dev,
- struct cros_ec_command *msg,
- uint32_t *mask)
+static int cros_ec_get_host_event_wake_mask(struct cros_ec_device *ec_dev, uint32_t *mask)
{
+ struct cros_ec_command *msg;
struct ec_response_host_event_mask *r;
int ret, mapped;
+ msg = kzalloc(sizeof(*msg) + sizeof(*r), GFP_KERNEL);
+ if (!msg)
+ return -ENOMEM;
+
msg->command = EC_CMD_HOST_EVENT_GET_WAKE_MASK;
- msg->version = 0;
- msg->outsize = 0;
msg->insize = sizeof(*r);
ret = send_command(ec_dev, msg);
if (ret >= 0) {
mapped = cros_ec_map_error(msg->result);
- if (mapped)
- return mapped;
+ if (mapped) {
+ ret = mapped;
+ goto exit;
+ }
}
if (ret > 0) {
r = (struct ec_response_host_event_mask *)msg->data;
*mask = r->mask;
}
+exit:
+ kfree(msg);
return ret;
}
-static int cros_ec_host_command_proto_query(struct cros_ec_device *ec_dev,
- int devidx,
- struct cros_ec_command *msg)
+static int cros_ec_get_proto_info(struct cros_ec_device *ec_dev, int devidx)
{
- /*
- * Try using v3+ to query for supported protocols. If this
- * command fails, fall back to v2. Returns the highest protocol
- * supported by the EC.
- * Also sets the max request/response/passthru size.
- */
- int ret;
+ struct cros_ec_command *msg;
+ struct ec_response_get_protocol_info *info;
+ int ret, mapped;
+
+ ec_dev->proto_version = 3;
+ if (devidx > 0)
+ ec_dev->max_passthru = 0;
+
+ msg = kzalloc(sizeof(*msg) + sizeof(*info), GFP_KERNEL);
+ if (!msg)
+ return -ENOMEM;
- memset(msg, 0, sizeof(*msg));
msg->command = EC_CMD_PASSTHRU_OFFSET(devidx) | EC_CMD_GET_PROTOCOL_INFO;
- msg->insize = sizeof(struct ec_response_get_protocol_info);
+ msg->insize = sizeof(*info);
ret = send_command(ec_dev, msg);
/*
@@ -299,15 +305,45 @@ static int cros_ec_host_command_proto_query(struct cros_ec_device *ec_dev,
dev_dbg(ec_dev->dev,
"failed to check for EC[%d] protocol version: %d\n",
devidx, ret);
- return ret;
+ goto exit;
+ }
+
+ mapped = cros_ec_map_error(msg->result);
+ if (mapped) {
+ ret = mapped;
+ goto exit;
}
- if (devidx > 0 && msg->result == EC_RES_INVALID_COMMAND)
- return -ENODEV;
- else if (msg->result != EC_RES_SUCCESS)
- return msg->result;
+ info = (struct ec_response_get_protocol_info *)msg->data;
+
+ switch (devidx) {
+ case CROS_EC_DEV_EC_INDEX:
+ ec_dev->max_request = info->max_request_packet_size -
+ sizeof(struct ec_host_request);
+ ec_dev->max_response = info->max_response_packet_size -
+ sizeof(struct ec_host_response);
+ ec_dev->proto_version = min(EC_HOST_REQUEST_VERSION,
+ fls(info->protocol_versions) - 1);
+ ec_dev->din_size = info->max_response_packet_size + EC_MAX_RESPONSE_OVERHEAD;
+ ec_dev->dout_size = info->max_request_packet_size + EC_MAX_REQUEST_OVERHEAD;
+
+ dev_dbg(ec_dev->dev, "using proto v%u\n", ec_dev->proto_version);
+ break;
+ case CROS_EC_DEV_PD_INDEX:
+ ec_dev->max_passthru = info->max_request_packet_size -
+ sizeof(struct ec_host_request);
+
+ dev_dbg(ec_dev->dev, "found PD chip\n");
+ break;
+ default:
+ dev_dbg(ec_dev->dev, "unknwon passthru index: %d\n", devidx);
+ break;
+ }
- return 0;
+ ret = 0;
+exit:
+ kfree(msg);
+ return ret;
}
static int cros_ec_host_command_proto_query_v2(struct cros_ec_device *ec_dev)
@@ -417,51 +453,13 @@ static int cros_ec_get_host_command_version_mask(struct cros_ec_device *ec_dev,
int cros_ec_query_all(struct cros_ec_device *ec_dev)
{
struct device *dev = ec_dev->dev;
- struct cros_ec_command *proto_msg;
- struct ec_response_get_protocol_info *proto_info;
u32 ver_mask = 0;
int ret;
- proto_msg = kzalloc(sizeof(*proto_msg) + sizeof(*proto_info),
- GFP_KERNEL);
- if (!proto_msg)
- return -ENOMEM;
-
/* First try sending with proto v3. */
- ec_dev->proto_version = 3;
- ret = cros_ec_host_command_proto_query(ec_dev, CROS_EC_DEV_EC_INDEX, proto_msg);
-
- if (ret == 0) {
- proto_info = (struct ec_response_get_protocol_info *)
- proto_msg->data;
- ec_dev->max_request = proto_info->max_request_packet_size -
- sizeof(struct ec_host_request);
- ec_dev->max_response = proto_info->max_response_packet_size -
- sizeof(struct ec_host_response);
- ec_dev->proto_version =
- min(EC_HOST_REQUEST_VERSION,
- fls(proto_info->protocol_versions) - 1);
- dev_dbg(ec_dev->dev,
- "using proto v%u\n",
- ec_dev->proto_version);
-
- ec_dev->din_size = proto_info->max_response_packet_size + EC_MAX_RESPONSE_OVERHEAD;
- ec_dev->dout_size = proto_info->max_request_packet_size + EC_MAX_REQUEST_OVERHEAD;
-
- /*
- * Check for PD
- */
- ret = cros_ec_host_command_proto_query(ec_dev, CROS_EC_DEV_PD_INDEX, proto_msg);
-
- if (ret) {
- dev_dbg(ec_dev->dev, "no PD chip found: %d\n", ret);
- ec_dev->max_passthru = 0;
- } else {
- dev_dbg(ec_dev->dev, "found PD chip\n");
- ec_dev->max_passthru =
- proto_info->max_request_packet_size -
- sizeof(struct ec_host_request);
- }
+ if (!cros_ec_get_proto_info(ec_dev, CROS_EC_DEV_EC_INDEX)) {
+ /* Check for PD. */
+ cros_ec_get_proto_info(ec_dev, CROS_EC_DEV_PD_INDEX);
} else {
/* Try querying with a v2 hello message. */
ec_dev->proto_version = 2;
@@ -524,8 +522,7 @@ int cros_ec_query_all(struct cros_ec_device *ec_dev)
ec_dev->host_sleep_v1 = (ret >= 0 && (ver_mask & EC_VER_MASK(1)));
/* Get host event wake mask. */
- ret = cros_ec_get_host_event_wake_mask(ec_dev, proto_msg,
- &ec_dev->host_event_wake_mask);
+ ret = cros_ec_get_host_event_wake_mask(ec_dev, &ec_dev->host_event_wake_mask);
if (ret < 0) {
/*
* If the EC doesn't support EC_CMD_HOST_EVENT_GET_WAKE_MASK,
@@ -556,7 +553,6 @@ int cros_ec_query_all(struct cros_ec_device *ec_dev)
ret = 0;
exit:
- kfree(proto_msg);
return ret;
}
EXPORT_SYMBOL(cros_ec_query_all);
diff --git a/drivers/platform/chrome/cros_ec_proto_test.c b/drivers/platform/chrome/cros_ec_proto_test.c
index 628c6582ca78..14a4441a39fc 100644
--- a/drivers/platform/chrome/cros_ec_proto_test.c
+++ b/drivers/platform/chrome/cros_ec_proto_test.c
@@ -194,7 +194,7 @@ static void cros_ec_proto_test_query_all_normal(struct kunit *test)
struct ec_xfer_mock *mock;
int ret;
- /* For cros_ec_host_command_proto_query() without passthru. */
+ /* For cros_ec_get_proto_info() without passthru. */
{
struct ec_response_get_protocol_info *data;
@@ -207,7 +207,7 @@ static void cros_ec_proto_test_query_all_normal(struct kunit *test)
data->max_response_packet_size = 0xef;
}
- /* For cros_ec_host_command_proto_query() with passthru. */
+ /* For cros_ec_get_proto_info() with passthru. */
{
struct ec_response_get_protocol_info *data;
@@ -255,7 +255,7 @@ static void cros_ec_proto_test_query_all_normal(struct kunit *test)
ret = cros_ec_query_all(ec_dev);
KUNIT_EXPECT_EQ(test, ret, 0);
- /* For cros_ec_host_command_proto_query() without passthru. */
+ /* For cros_ec_get_proto_info() without passthru. */
{
mock = cros_kunit_ec_xfer_mock_next();
KUNIT_EXPECT_PTR_NE(test, mock, NULL);
@@ -273,7 +273,7 @@ static void cros_ec_proto_test_query_all_normal(struct kunit *test)
KUNIT_EXPECT_EQ(test, ec_dev->dout_size, 0xbe + EC_MAX_REQUEST_OVERHEAD);
}
- /* For cros_ec_host_command_proto_query() with passthru. */
+ /* For cros_ec_get_proto_info() with passthru. */
{
mock = cros_kunit_ec_xfer_mock_next();
KUNIT_EXPECT_PTR_NE(test, mock, NULL);
@@ -351,7 +351,7 @@ static void cros_ec_proto_test_query_all_no_pd_return_error(struct kunit *test)
/* Set some garbage bytes. */
ec_dev->max_passthru = 0xbf;
- /* For cros_ec_host_command_proto_query() without passthru. */
+ /* For cros_ec_get_proto_info() without passthru. */
{
struct ec_response_get_protocol_info *data;
@@ -367,7 +367,7 @@ static void cros_ec_proto_test_query_all_no_pd_return_error(struct kunit *test)
data->max_response_packet_size = 0xef;
}
- /* For cros_ec_host_command_proto_query() with passthru. */
+ /* For cros_ec_get_proto_info() with passthru. */
{
mock = cros_kunit_ec_xfer_mock_addx(test, 0, EC_RES_INVALID_COMMAND, 0);
KUNIT_ASSERT_PTR_NE(test, mock, NULL);
@@ -377,7 +377,7 @@ static void cros_ec_proto_test_query_all_no_pd_return_error(struct kunit *test)
ret = cros_ec_query_all(ec_dev);
KUNIT_EXPECT_EQ(test, ret, 0);
- /* For cros_ec_host_command_proto_query() without passthru. */
+ /* For cros_ec_get_proto_info() without passthru. */
{
mock = cros_kunit_ec_xfer_mock_next();
KUNIT_EXPECT_PTR_NE(test, mock, NULL);
@@ -389,7 +389,7 @@ static void cros_ec_proto_test_query_all_no_pd_return_error(struct kunit *test)
KUNIT_EXPECT_EQ(test, mock->msg.outsize, 0);
}
- /* For cros_ec_host_command_proto_query() with passthru. */
+ /* For cros_ec_get_proto_info() with passthru. */
{
mock = cros_kunit_ec_xfer_mock_next();
KUNIT_EXPECT_PTR_NE(test, mock, NULL);
@@ -413,7 +413,7 @@ static void cros_ec_proto_test_query_all_legacy_normal_v3_return_error(struct ku
struct ec_xfer_mock *mock;
int ret;
- /* For cros_ec_host_command_proto_query() without passthru. */
+ /* For cros_ec_get_proto_info() without passthru. */
{
mock = cros_kunit_ec_xfer_mock_addx(test, 0, EC_RES_INVALID_COMMAND, 0);
KUNIT_ASSERT_PTR_NE(test, mock, NULL);
@@ -434,7 +434,7 @@ static void cros_ec_proto_test_query_all_legacy_normal_v3_return_error(struct ku
ret = cros_ec_query_all(ec_dev);
KUNIT_EXPECT_EQ(test, ret, 0);
- /* For cros_ec_host_command_proto_query() without passthru. */
+ /* For cros_ec_get_proto_info() without passthru. */
{
mock = cros_kunit_ec_xfer_mock_next();
KUNIT_EXPECT_PTR_NE(test, mock, NULL);
@@ -478,7 +478,7 @@ static void cros_ec_proto_test_query_all_legacy_xfer_error(struct kunit *test)
struct ec_xfer_mock *mock;
int ret;
- /* For cros_ec_host_command_proto_query() without passthru. */
+ /* For cros_ec_get_proto_info() without passthru. */
{
mock = cros_kunit_ec_xfer_mock_addx(test, 0, EC_RES_INVALID_COMMAND, 0);
KUNIT_ASSERT_PTR_NE(test, mock, NULL);
@@ -495,7 +495,7 @@ static void cros_ec_proto_test_query_all_legacy_xfer_error(struct kunit *test)
KUNIT_EXPECT_EQ(test, ret, -EIO);
KUNIT_EXPECT_EQ(test, ec_dev->proto_version, EC_PROTO_VERSION_UNKNOWN);
- /* For cros_ec_host_command_proto_query() without passthru. */
+ /* For cros_ec_get_proto_info() without passthru. */
{
mock = cros_kunit_ec_xfer_mock_next();
KUNIT_EXPECT_PTR_NE(test, mock, NULL);
@@ -526,7 +526,7 @@ static void cros_ec_proto_test_query_all_legacy_return_error(struct kunit *test)
struct ec_xfer_mock *mock;
int ret;
- /* For cros_ec_host_command_proto_query() without passthru. */
+ /* For cros_ec_get_proto_info() without passthru. */
{
mock = cros_kunit_ec_xfer_mock_addx(test, 0, EC_RES_INVALID_COMMAND, 0);
KUNIT_ASSERT_PTR_NE(test, mock, NULL);
@@ -543,7 +543,7 @@ static void cros_ec_proto_test_query_all_legacy_return_error(struct kunit *test)
KUNIT_EXPECT_EQ(test, ret, EC_RES_INVALID_COMMAND);
KUNIT_EXPECT_EQ(test, ec_dev->proto_version, EC_PROTO_VERSION_UNKNOWN);
- /* For cros_ec_host_command_proto_query() without passthru. */
+ /* For cros_ec_get_proto_info() without passthru. */
{
mock = cros_kunit_ec_xfer_mock_next();
KUNIT_EXPECT_PTR_NE(test, mock, NULL);
@@ -574,7 +574,7 @@ static void cros_ec_proto_test_query_all_legacy_data_error(struct kunit *test)
struct ec_xfer_mock *mock;
int ret;
- /* For cros_ec_host_command_proto_query() without passthru. */
+ /* For cros_ec_get_proto_info() without passthru. */
{
mock = cros_kunit_ec_xfer_mock_addx(test, 0, EC_RES_INVALID_COMMAND, 0);
KUNIT_ASSERT_PTR_NE(test, mock, NULL);
@@ -596,7 +596,7 @@ static void cros_ec_proto_test_query_all_legacy_data_error(struct kunit *test)
KUNIT_EXPECT_EQ(test, ret, -EBADMSG);
KUNIT_EXPECT_EQ(test, ec_dev->proto_version, EC_PROTO_VERSION_UNKNOWN);
- /* For cros_ec_host_command_proto_query() without passthru. */
+ /* For cros_ec_get_proto_info() without passthru. */
{
mock = cros_kunit_ec_xfer_mock_next();
KUNIT_EXPECT_PTR_NE(test, mock, NULL);
@@ -630,7 +630,7 @@ static void cros_ec_proto_test_query_all_no_mkbp(struct kunit *test)
/* Set some garbage bytes. */
ec_dev->mkbp_event_supported = 0xbf;
- /* For cros_ec_host_command_proto_query() without passthru. */
+ /* For cros_ec_get_proto_info() without passthru. */
{
struct ec_response_get_protocol_info *data;
@@ -646,7 +646,7 @@ static void cros_ec_proto_test_query_all_no_mkbp(struct kunit *test)
data->max_response_packet_size = 0xef;
}
- /* For cros_ec_host_command_proto_query() with passthru. */
+ /* For cros_ec_get_proto_info() with passthru. */
{
mock = cros_kunit_ec_xfer_mock_addx(test, 0, EC_RES_INVALID_COMMAND, 0);
KUNIT_ASSERT_PTR_NE(test, mock, NULL);
@@ -667,7 +667,7 @@ static void cros_ec_proto_test_query_all_no_mkbp(struct kunit *test)
ret = cros_ec_query_all(ec_dev);
KUNIT_EXPECT_EQ(test, ret, 0);
- /* For cros_ec_host_command_proto_query() without passthru. */
+ /* For cros_ec_get_proto_info() without passthru. */
{
mock = cros_kunit_ec_xfer_mock_next();
KUNIT_EXPECT_PTR_NE(test, mock, NULL);
@@ -679,7 +679,7 @@ static void cros_ec_proto_test_query_all_no_mkbp(struct kunit *test)
KUNIT_EXPECT_EQ(test, mock->msg.outsize, 0);
}
- /* For cros_ec_host_command_proto_query() with passthru. */
+ /* For cros_ec_get_proto_info() with passthru. */
{
mock = cros_kunit_ec_xfer_mock_next();
KUNIT_EXPECT_PTR_NE(test, mock, NULL);
@@ -723,7 +723,7 @@ static void cros_ec_proto_test_query_all_no_host_sleep(struct kunit *test)
/* Set some garbage bytes. */
ec_dev->host_sleep_v1 = true;
- /* For cros_ec_host_command_proto_query() without passthru. */
+ /* For cros_ec_get_proto_info() without passthru. */
{
struct ec_response_get_protocol_info *data;
@@ -739,7 +739,7 @@ static void cros_ec_proto_test_query_all_no_host_sleep(struct kunit *test)
data->max_response_packet_size = 0xef;
}
- /* For cros_ec_host_command_proto_query() with passthru. */
+ /* For cros_ec_get_proto_info() with passthru. */
{
mock = cros_kunit_ec_xfer_mock_addx(test, 0, EC_RES_INVALID_COMMAND, 0);
KUNIT_ASSERT_PTR_NE(test, mock, NULL);
@@ -766,7 +766,7 @@ static void cros_ec_proto_test_query_all_no_host_sleep(struct kunit *test)
ret = cros_ec_query_all(ec_dev);
KUNIT_EXPECT_EQ(test, ret, 0);
- /* For cros_ec_host_command_proto_query() without passthru. */
+ /* For cros_ec_get_proto_info() without passthru. */
{
mock = cros_kunit_ec_xfer_mock_next();
KUNIT_EXPECT_PTR_NE(test, mock, NULL);
@@ -778,7 +778,7 @@ static void cros_ec_proto_test_query_all_no_host_sleep(struct kunit *test)
KUNIT_EXPECT_EQ(test, mock->msg.outsize, 0);
}
- /* For cros_ec_host_command_proto_query() with passthru. */
+ /* For cros_ec_get_proto_info() with passthru. */
{
mock = cros_kunit_ec_xfer_mock_next();
KUNIT_EXPECT_PTR_NE(test, mock, NULL);
@@ -829,7 +829,7 @@ static void cros_ec_proto_test_query_all_default_wake_mask_return_error(struct k
/* Set some garbage bytes. */
ec_dev->host_event_wake_mask = U32_MAX;
- /* For cros_ec_host_command_proto_query() without passthru. */
+ /* For cros_ec_get_proto_info() without passthru. */
{
struct ec_response_get_protocol_info *data;
@@ -845,7 +845,7 @@ static void cros_ec_proto_test_query_all_default_wake_mask_return_error(struct k
data->max_response_packet_size = 0xef;
}
- /* For cros_ec_host_command_proto_query() with passthru. */
+ /* For cros_ec_get_proto_info() with passthru. */
{
mock = cros_kunit_ec_xfer_mock_addx(test, 0, EC_RES_INVALID_COMMAND, 0);
KUNIT_ASSERT_PTR_NE(test, mock, NULL);
@@ -873,7 +873,7 @@ static void cros_ec_proto_test_query_all_default_wake_mask_return_error(struct k
ret = cros_ec_query_all(ec_dev);
KUNIT_EXPECT_EQ(test, ret, 0);
- /* For cros_ec_host_command_proto_query() without passthru. */
+ /* For cros_ec_get_proto_info() without passthru. */
{
mock = cros_kunit_ec_xfer_mock_next();
KUNIT_EXPECT_PTR_NE(test, mock, NULL);
@@ -885,7 +885,7 @@ static void cros_ec_proto_test_query_all_default_wake_mask_return_error(struct k
KUNIT_EXPECT_EQ(test, mock->msg.outsize, 0);
}
- /* For cros_ec_host_command_proto_query() with passthru. */
+ /* For cros_ec_get_proto_info() with passthru. */
{
mock = cros_kunit_ec_xfer_mock_next();
KUNIT_EXPECT_PTR_NE(test, mock, NULL);
--
2.36.1.255.ge46751e96f-goog
^ permalink raw reply related [flat|nested] 28+ messages in thread* Re: [PATCH v2 07/15] platform/chrome: cros_ec_proto: separate cros_ec_get_proto_info()
2022-06-07 14:56 ` [PATCH v2 07/15] platform/chrome: cros_ec_proto: separate cros_ec_get_proto_info() Tzung-Bi Shih
@ 2022-06-07 18:45 ` Guenter Roeck
0 siblings, 0 replies; 28+ messages in thread
From: Guenter Roeck @ 2022-06-07 18:45 UTC (permalink / raw)
To: Tzung-Bi Shih; +Cc: bleung, groeck, chrome-platform, linux-kernel
On Tue, Jun 7, 2022 at 7:57 AM Tzung-Bi Shih <tzungbi@kernel.org> wrote:
>
> Rename cros_ec_host_command_proto_query() to cros_ec_get_proto_info()
> and make it responsible for setting `ec_dev` fields according to the
> response protocol info.
>
> Also make cros_ec_get_host_event_wake_mask() allocate its own message
> buffer. It was lucky that size of `struct ec_response_host_event_mask`
> is less than `struct ec_response_get_protocol_info`. Thus, the buffer
> wasn't overflow.
>
> Signed-off-by: Tzung-Bi Shih <tzungbi@kernel.org>
Reviewed-by: Guenter Roeck <groeck@chromium.org>
> ---
> Changes from v1:
> - Preserve the "cros_ec_" prefix.
>
> drivers/platform/chrome/cros_ec_proto.c | 134 +++++++++----------
> drivers/platform/chrome/cros_ec_proto_test.c | 56 ++++----
> 2 files changed, 93 insertions(+), 97 deletions(-)
>
> diff --git a/drivers/platform/chrome/cros_ec_proto.c b/drivers/platform/chrome/cros_ec_proto.c
> index 71ba6a56ad7c..893b76703da6 100644
> --- a/drivers/platform/chrome/cros_ec_proto.c
> +++ b/drivers/platform/chrome/cros_ec_proto.c
> @@ -242,47 +242,53 @@ EXPORT_SYMBOL(cros_ec_check_result);
> * the caller has ec_dev->lock mutex, or the caller knows there is
> * no other command in progress.
> */
> -static int cros_ec_get_host_event_wake_mask(struct cros_ec_device *ec_dev,
> - struct cros_ec_command *msg,
> - uint32_t *mask)
> +static int cros_ec_get_host_event_wake_mask(struct cros_ec_device *ec_dev, uint32_t *mask)
> {
> + struct cros_ec_command *msg;
> struct ec_response_host_event_mask *r;
> int ret, mapped;
>
> + msg = kzalloc(sizeof(*msg) + sizeof(*r), GFP_KERNEL);
> + if (!msg)
> + return -ENOMEM;
> +
> msg->command = EC_CMD_HOST_EVENT_GET_WAKE_MASK;
> - msg->version = 0;
> - msg->outsize = 0;
> msg->insize = sizeof(*r);
>
> ret = send_command(ec_dev, msg);
> if (ret >= 0) {
> mapped = cros_ec_map_error(msg->result);
> - if (mapped)
> - return mapped;
> + if (mapped) {
> + ret = mapped;
> + goto exit;
> + }
> }
> if (ret > 0) {
> r = (struct ec_response_host_event_mask *)msg->data;
> *mask = r->mask;
> }
>
> +exit:
> + kfree(msg);
> return ret;
> }
>
> -static int cros_ec_host_command_proto_query(struct cros_ec_device *ec_dev,
> - int devidx,
> - struct cros_ec_command *msg)
> +static int cros_ec_get_proto_info(struct cros_ec_device *ec_dev, int devidx)
> {
> - /*
> - * Try using v3+ to query for supported protocols. If this
> - * command fails, fall back to v2. Returns the highest protocol
> - * supported by the EC.
> - * Also sets the max request/response/passthru size.
> - */
> - int ret;
> + struct cros_ec_command *msg;
> + struct ec_response_get_protocol_info *info;
> + int ret, mapped;
> +
> + ec_dev->proto_version = 3;
> + if (devidx > 0)
> + ec_dev->max_passthru = 0;
> +
> + msg = kzalloc(sizeof(*msg) + sizeof(*info), GFP_KERNEL);
> + if (!msg)
> + return -ENOMEM;
>
> - memset(msg, 0, sizeof(*msg));
> msg->command = EC_CMD_PASSTHRU_OFFSET(devidx) | EC_CMD_GET_PROTOCOL_INFO;
> - msg->insize = sizeof(struct ec_response_get_protocol_info);
> + msg->insize = sizeof(*info);
>
> ret = send_command(ec_dev, msg);
> /*
> @@ -299,15 +305,45 @@ static int cros_ec_host_command_proto_query(struct cros_ec_device *ec_dev,
> dev_dbg(ec_dev->dev,
> "failed to check for EC[%d] protocol version: %d\n",
> devidx, ret);
> - return ret;
> + goto exit;
> + }
> +
> + mapped = cros_ec_map_error(msg->result);
> + if (mapped) {
> + ret = mapped;
> + goto exit;
> }
>
> - if (devidx > 0 && msg->result == EC_RES_INVALID_COMMAND)
> - return -ENODEV;
> - else if (msg->result != EC_RES_SUCCESS)
> - return msg->result;
> + info = (struct ec_response_get_protocol_info *)msg->data;
> +
> + switch (devidx) {
> + case CROS_EC_DEV_EC_INDEX:
> + ec_dev->max_request = info->max_request_packet_size -
> + sizeof(struct ec_host_request);
> + ec_dev->max_response = info->max_response_packet_size -
> + sizeof(struct ec_host_response);
> + ec_dev->proto_version = min(EC_HOST_REQUEST_VERSION,
> + fls(info->protocol_versions) - 1);
> + ec_dev->din_size = info->max_response_packet_size + EC_MAX_RESPONSE_OVERHEAD;
> + ec_dev->dout_size = info->max_request_packet_size + EC_MAX_REQUEST_OVERHEAD;
> +
> + dev_dbg(ec_dev->dev, "using proto v%u\n", ec_dev->proto_version);
> + break;
> + case CROS_EC_DEV_PD_INDEX:
> + ec_dev->max_passthru = info->max_request_packet_size -
> + sizeof(struct ec_host_request);
> +
> + dev_dbg(ec_dev->dev, "found PD chip\n");
> + break;
> + default:
> + dev_dbg(ec_dev->dev, "unknwon passthru index: %d\n", devidx);
> + break;
> + }
>
> - return 0;
> + ret = 0;
> +exit:
> + kfree(msg);
> + return ret;
> }
>
> static int cros_ec_host_command_proto_query_v2(struct cros_ec_device *ec_dev)
> @@ -417,51 +453,13 @@ static int cros_ec_get_host_command_version_mask(struct cros_ec_device *ec_dev,
> int cros_ec_query_all(struct cros_ec_device *ec_dev)
> {
> struct device *dev = ec_dev->dev;
> - struct cros_ec_command *proto_msg;
> - struct ec_response_get_protocol_info *proto_info;
> u32 ver_mask = 0;
> int ret;
>
> - proto_msg = kzalloc(sizeof(*proto_msg) + sizeof(*proto_info),
> - GFP_KERNEL);
> - if (!proto_msg)
> - return -ENOMEM;
> -
> /* First try sending with proto v3. */
> - ec_dev->proto_version = 3;
> - ret = cros_ec_host_command_proto_query(ec_dev, CROS_EC_DEV_EC_INDEX, proto_msg);
> -
> - if (ret == 0) {
> - proto_info = (struct ec_response_get_protocol_info *)
> - proto_msg->data;
> - ec_dev->max_request = proto_info->max_request_packet_size -
> - sizeof(struct ec_host_request);
> - ec_dev->max_response = proto_info->max_response_packet_size -
> - sizeof(struct ec_host_response);
> - ec_dev->proto_version =
> - min(EC_HOST_REQUEST_VERSION,
> - fls(proto_info->protocol_versions) - 1);
> - dev_dbg(ec_dev->dev,
> - "using proto v%u\n",
> - ec_dev->proto_version);
> -
> - ec_dev->din_size = proto_info->max_response_packet_size + EC_MAX_RESPONSE_OVERHEAD;
> - ec_dev->dout_size = proto_info->max_request_packet_size + EC_MAX_REQUEST_OVERHEAD;
> -
> - /*
> - * Check for PD
> - */
> - ret = cros_ec_host_command_proto_query(ec_dev, CROS_EC_DEV_PD_INDEX, proto_msg);
> -
> - if (ret) {
> - dev_dbg(ec_dev->dev, "no PD chip found: %d\n", ret);
> - ec_dev->max_passthru = 0;
> - } else {
> - dev_dbg(ec_dev->dev, "found PD chip\n");
> - ec_dev->max_passthru =
> - proto_info->max_request_packet_size -
> - sizeof(struct ec_host_request);
> - }
> + if (!cros_ec_get_proto_info(ec_dev, CROS_EC_DEV_EC_INDEX)) {
> + /* Check for PD. */
> + cros_ec_get_proto_info(ec_dev, CROS_EC_DEV_PD_INDEX);
> } else {
> /* Try querying with a v2 hello message. */
> ec_dev->proto_version = 2;
> @@ -524,8 +522,7 @@ int cros_ec_query_all(struct cros_ec_device *ec_dev)
> ec_dev->host_sleep_v1 = (ret >= 0 && (ver_mask & EC_VER_MASK(1)));
>
> /* Get host event wake mask. */
> - ret = cros_ec_get_host_event_wake_mask(ec_dev, proto_msg,
> - &ec_dev->host_event_wake_mask);
> + ret = cros_ec_get_host_event_wake_mask(ec_dev, &ec_dev->host_event_wake_mask);
> if (ret < 0) {
> /*
> * If the EC doesn't support EC_CMD_HOST_EVENT_GET_WAKE_MASK,
> @@ -556,7 +553,6 @@ int cros_ec_query_all(struct cros_ec_device *ec_dev)
> ret = 0;
>
> exit:
> - kfree(proto_msg);
> return ret;
> }
> EXPORT_SYMBOL(cros_ec_query_all);
> diff --git a/drivers/platform/chrome/cros_ec_proto_test.c b/drivers/platform/chrome/cros_ec_proto_test.c
> index 628c6582ca78..14a4441a39fc 100644
> --- a/drivers/platform/chrome/cros_ec_proto_test.c
> +++ b/drivers/platform/chrome/cros_ec_proto_test.c
> @@ -194,7 +194,7 @@ static void cros_ec_proto_test_query_all_normal(struct kunit *test)
> struct ec_xfer_mock *mock;
> int ret;
>
> - /* For cros_ec_host_command_proto_query() without passthru. */
> + /* For cros_ec_get_proto_info() without passthru. */
> {
> struct ec_response_get_protocol_info *data;
>
> @@ -207,7 +207,7 @@ static void cros_ec_proto_test_query_all_normal(struct kunit *test)
> data->max_response_packet_size = 0xef;
> }
>
> - /* For cros_ec_host_command_proto_query() with passthru. */
> + /* For cros_ec_get_proto_info() with passthru. */
> {
> struct ec_response_get_protocol_info *data;
>
> @@ -255,7 +255,7 @@ static void cros_ec_proto_test_query_all_normal(struct kunit *test)
> ret = cros_ec_query_all(ec_dev);
> KUNIT_EXPECT_EQ(test, ret, 0);
>
> - /* For cros_ec_host_command_proto_query() without passthru. */
> + /* For cros_ec_get_proto_info() without passthru. */
> {
> mock = cros_kunit_ec_xfer_mock_next();
> KUNIT_EXPECT_PTR_NE(test, mock, NULL);
> @@ -273,7 +273,7 @@ static void cros_ec_proto_test_query_all_normal(struct kunit *test)
> KUNIT_EXPECT_EQ(test, ec_dev->dout_size, 0xbe + EC_MAX_REQUEST_OVERHEAD);
> }
>
> - /* For cros_ec_host_command_proto_query() with passthru. */
> + /* For cros_ec_get_proto_info() with passthru. */
> {
> mock = cros_kunit_ec_xfer_mock_next();
> KUNIT_EXPECT_PTR_NE(test, mock, NULL);
> @@ -351,7 +351,7 @@ static void cros_ec_proto_test_query_all_no_pd_return_error(struct kunit *test)
> /* Set some garbage bytes. */
> ec_dev->max_passthru = 0xbf;
>
> - /* For cros_ec_host_command_proto_query() without passthru. */
> + /* For cros_ec_get_proto_info() without passthru. */
> {
> struct ec_response_get_protocol_info *data;
>
> @@ -367,7 +367,7 @@ static void cros_ec_proto_test_query_all_no_pd_return_error(struct kunit *test)
> data->max_response_packet_size = 0xef;
> }
>
> - /* For cros_ec_host_command_proto_query() with passthru. */
> + /* For cros_ec_get_proto_info() with passthru. */
> {
> mock = cros_kunit_ec_xfer_mock_addx(test, 0, EC_RES_INVALID_COMMAND, 0);
> KUNIT_ASSERT_PTR_NE(test, mock, NULL);
> @@ -377,7 +377,7 @@ static void cros_ec_proto_test_query_all_no_pd_return_error(struct kunit *test)
> ret = cros_ec_query_all(ec_dev);
> KUNIT_EXPECT_EQ(test, ret, 0);
>
> - /* For cros_ec_host_command_proto_query() without passthru. */
> + /* For cros_ec_get_proto_info() without passthru. */
> {
> mock = cros_kunit_ec_xfer_mock_next();
> KUNIT_EXPECT_PTR_NE(test, mock, NULL);
> @@ -389,7 +389,7 @@ static void cros_ec_proto_test_query_all_no_pd_return_error(struct kunit *test)
> KUNIT_EXPECT_EQ(test, mock->msg.outsize, 0);
> }
>
> - /* For cros_ec_host_command_proto_query() with passthru. */
> + /* For cros_ec_get_proto_info() with passthru. */
> {
> mock = cros_kunit_ec_xfer_mock_next();
> KUNIT_EXPECT_PTR_NE(test, mock, NULL);
> @@ -413,7 +413,7 @@ static void cros_ec_proto_test_query_all_legacy_normal_v3_return_error(struct ku
> struct ec_xfer_mock *mock;
> int ret;
>
> - /* For cros_ec_host_command_proto_query() without passthru. */
> + /* For cros_ec_get_proto_info() without passthru. */
> {
> mock = cros_kunit_ec_xfer_mock_addx(test, 0, EC_RES_INVALID_COMMAND, 0);
> KUNIT_ASSERT_PTR_NE(test, mock, NULL);
> @@ -434,7 +434,7 @@ static void cros_ec_proto_test_query_all_legacy_normal_v3_return_error(struct ku
> ret = cros_ec_query_all(ec_dev);
> KUNIT_EXPECT_EQ(test, ret, 0);
>
> - /* For cros_ec_host_command_proto_query() without passthru. */
> + /* For cros_ec_get_proto_info() without passthru. */
> {
> mock = cros_kunit_ec_xfer_mock_next();
> KUNIT_EXPECT_PTR_NE(test, mock, NULL);
> @@ -478,7 +478,7 @@ static void cros_ec_proto_test_query_all_legacy_xfer_error(struct kunit *test)
> struct ec_xfer_mock *mock;
> int ret;
>
> - /* For cros_ec_host_command_proto_query() without passthru. */
> + /* For cros_ec_get_proto_info() without passthru. */
> {
> mock = cros_kunit_ec_xfer_mock_addx(test, 0, EC_RES_INVALID_COMMAND, 0);
> KUNIT_ASSERT_PTR_NE(test, mock, NULL);
> @@ -495,7 +495,7 @@ static void cros_ec_proto_test_query_all_legacy_xfer_error(struct kunit *test)
> KUNIT_EXPECT_EQ(test, ret, -EIO);
> KUNIT_EXPECT_EQ(test, ec_dev->proto_version, EC_PROTO_VERSION_UNKNOWN);
>
> - /* For cros_ec_host_command_proto_query() without passthru. */
> + /* For cros_ec_get_proto_info() without passthru. */
> {
> mock = cros_kunit_ec_xfer_mock_next();
> KUNIT_EXPECT_PTR_NE(test, mock, NULL);
> @@ -526,7 +526,7 @@ static void cros_ec_proto_test_query_all_legacy_return_error(struct kunit *test)
> struct ec_xfer_mock *mock;
> int ret;
>
> - /* For cros_ec_host_command_proto_query() without passthru. */
> + /* For cros_ec_get_proto_info() without passthru. */
> {
> mock = cros_kunit_ec_xfer_mock_addx(test, 0, EC_RES_INVALID_COMMAND, 0);
> KUNIT_ASSERT_PTR_NE(test, mock, NULL);
> @@ -543,7 +543,7 @@ static void cros_ec_proto_test_query_all_legacy_return_error(struct kunit *test)
> KUNIT_EXPECT_EQ(test, ret, EC_RES_INVALID_COMMAND);
> KUNIT_EXPECT_EQ(test, ec_dev->proto_version, EC_PROTO_VERSION_UNKNOWN);
>
> - /* For cros_ec_host_command_proto_query() without passthru. */
> + /* For cros_ec_get_proto_info() without passthru. */
> {
> mock = cros_kunit_ec_xfer_mock_next();
> KUNIT_EXPECT_PTR_NE(test, mock, NULL);
> @@ -574,7 +574,7 @@ static void cros_ec_proto_test_query_all_legacy_data_error(struct kunit *test)
> struct ec_xfer_mock *mock;
> int ret;
>
> - /* For cros_ec_host_command_proto_query() without passthru. */
> + /* For cros_ec_get_proto_info() without passthru. */
> {
> mock = cros_kunit_ec_xfer_mock_addx(test, 0, EC_RES_INVALID_COMMAND, 0);
> KUNIT_ASSERT_PTR_NE(test, mock, NULL);
> @@ -596,7 +596,7 @@ static void cros_ec_proto_test_query_all_legacy_data_error(struct kunit *test)
> KUNIT_EXPECT_EQ(test, ret, -EBADMSG);
> KUNIT_EXPECT_EQ(test, ec_dev->proto_version, EC_PROTO_VERSION_UNKNOWN);
>
> - /* For cros_ec_host_command_proto_query() without passthru. */
> + /* For cros_ec_get_proto_info() without passthru. */
> {
> mock = cros_kunit_ec_xfer_mock_next();
> KUNIT_EXPECT_PTR_NE(test, mock, NULL);
> @@ -630,7 +630,7 @@ static void cros_ec_proto_test_query_all_no_mkbp(struct kunit *test)
> /* Set some garbage bytes. */
> ec_dev->mkbp_event_supported = 0xbf;
>
> - /* For cros_ec_host_command_proto_query() without passthru. */
> + /* For cros_ec_get_proto_info() without passthru. */
> {
> struct ec_response_get_protocol_info *data;
>
> @@ -646,7 +646,7 @@ static void cros_ec_proto_test_query_all_no_mkbp(struct kunit *test)
> data->max_response_packet_size = 0xef;
> }
>
> - /* For cros_ec_host_command_proto_query() with passthru. */
> + /* For cros_ec_get_proto_info() with passthru. */
> {
> mock = cros_kunit_ec_xfer_mock_addx(test, 0, EC_RES_INVALID_COMMAND, 0);
> KUNIT_ASSERT_PTR_NE(test, mock, NULL);
> @@ -667,7 +667,7 @@ static void cros_ec_proto_test_query_all_no_mkbp(struct kunit *test)
> ret = cros_ec_query_all(ec_dev);
> KUNIT_EXPECT_EQ(test, ret, 0);
>
> - /* For cros_ec_host_command_proto_query() without passthru. */
> + /* For cros_ec_get_proto_info() without passthru. */
> {
> mock = cros_kunit_ec_xfer_mock_next();
> KUNIT_EXPECT_PTR_NE(test, mock, NULL);
> @@ -679,7 +679,7 @@ static void cros_ec_proto_test_query_all_no_mkbp(struct kunit *test)
> KUNIT_EXPECT_EQ(test, mock->msg.outsize, 0);
> }
>
> - /* For cros_ec_host_command_proto_query() with passthru. */
> + /* For cros_ec_get_proto_info() with passthru. */
> {
> mock = cros_kunit_ec_xfer_mock_next();
> KUNIT_EXPECT_PTR_NE(test, mock, NULL);
> @@ -723,7 +723,7 @@ static void cros_ec_proto_test_query_all_no_host_sleep(struct kunit *test)
> /* Set some garbage bytes. */
> ec_dev->host_sleep_v1 = true;
>
> - /* For cros_ec_host_command_proto_query() without passthru. */
> + /* For cros_ec_get_proto_info() without passthru. */
> {
> struct ec_response_get_protocol_info *data;
>
> @@ -739,7 +739,7 @@ static void cros_ec_proto_test_query_all_no_host_sleep(struct kunit *test)
> data->max_response_packet_size = 0xef;
> }
>
> - /* For cros_ec_host_command_proto_query() with passthru. */
> + /* For cros_ec_get_proto_info() with passthru. */
> {
> mock = cros_kunit_ec_xfer_mock_addx(test, 0, EC_RES_INVALID_COMMAND, 0);
> KUNIT_ASSERT_PTR_NE(test, mock, NULL);
> @@ -766,7 +766,7 @@ static void cros_ec_proto_test_query_all_no_host_sleep(struct kunit *test)
> ret = cros_ec_query_all(ec_dev);
> KUNIT_EXPECT_EQ(test, ret, 0);
>
> - /* For cros_ec_host_command_proto_query() without passthru. */
> + /* For cros_ec_get_proto_info() without passthru. */
> {
> mock = cros_kunit_ec_xfer_mock_next();
> KUNIT_EXPECT_PTR_NE(test, mock, NULL);
> @@ -778,7 +778,7 @@ static void cros_ec_proto_test_query_all_no_host_sleep(struct kunit *test)
> KUNIT_EXPECT_EQ(test, mock->msg.outsize, 0);
> }
>
> - /* For cros_ec_host_command_proto_query() with passthru. */
> + /* For cros_ec_get_proto_info() with passthru. */
> {
> mock = cros_kunit_ec_xfer_mock_next();
> KUNIT_EXPECT_PTR_NE(test, mock, NULL);
> @@ -829,7 +829,7 @@ static void cros_ec_proto_test_query_all_default_wake_mask_return_error(struct k
> /* Set some garbage bytes. */
> ec_dev->host_event_wake_mask = U32_MAX;
>
> - /* For cros_ec_host_command_proto_query() without passthru. */
> + /* For cros_ec_get_proto_info() without passthru. */
> {
> struct ec_response_get_protocol_info *data;
>
> @@ -845,7 +845,7 @@ static void cros_ec_proto_test_query_all_default_wake_mask_return_error(struct k
> data->max_response_packet_size = 0xef;
> }
>
> - /* For cros_ec_host_command_proto_query() with passthru. */
> + /* For cros_ec_get_proto_info() with passthru. */
> {
> mock = cros_kunit_ec_xfer_mock_addx(test, 0, EC_RES_INVALID_COMMAND, 0);
> KUNIT_ASSERT_PTR_NE(test, mock, NULL);
> @@ -873,7 +873,7 @@ static void cros_ec_proto_test_query_all_default_wake_mask_return_error(struct k
> ret = cros_ec_query_all(ec_dev);
> KUNIT_EXPECT_EQ(test, ret, 0);
>
> - /* For cros_ec_host_command_proto_query() without passthru. */
> + /* For cros_ec_get_proto_info() without passthru. */
> {
> mock = cros_kunit_ec_xfer_mock_next();
> KUNIT_EXPECT_PTR_NE(test, mock, NULL);
> @@ -885,7 +885,7 @@ static void cros_ec_proto_test_query_all_default_wake_mask_return_error(struct k
> KUNIT_EXPECT_EQ(test, mock->msg.outsize, 0);
> }
>
> - /* For cros_ec_host_command_proto_query() with passthru. */
> + /* For cros_ec_get_proto_info() with passthru. */
> {
> mock = cros_kunit_ec_xfer_mock_next();
> KUNIT_EXPECT_PTR_NE(test, mock, NULL);
> --
> 2.36.1.255.ge46751e96f-goog
>
^ permalink raw reply [flat|nested] 28+ messages in thread
* [PATCH v2 08/15] platform/chrome: cros_ec_proto: handle empty payload in getting proto info
2022-06-07 14:56 [PATCH v2 00/15] platform/chrome: Kunit tests and refactor for cros_ec_query_all() Tzung-Bi Shih
` (6 preceding siblings ...)
2022-06-07 14:56 ` [PATCH v2 07/15] platform/chrome: cros_ec_proto: separate cros_ec_get_proto_info() Tzung-Bi Shih
@ 2022-06-07 14:56 ` Tzung-Bi Shih
2022-06-07 18:47 ` Guenter Roeck
2022-06-07 14:56 ` [PATCH v2 09/15] platform/chrome: cros_ec_proto: separate cros_ec_get_proto_info_legacy() Tzung-Bi Shih
` (6 subsequent siblings)
14 siblings, 1 reply; 28+ messages in thread
From: Tzung-Bi Shih @ 2022-06-07 14:56 UTC (permalink / raw)
To: bleung, groeck; +Cc: chrome-platform, linux-kernel, tzungbi
cros_ec_get_proto_info() expects to receive
sizeof(struct ec_response_get_protocol_info) from send_command(). The
payload is valid only if the return value is positive.
Add Kunit tests for returning 0 in send_command() and handle the case in
cros_ec_get_proto_info().
Signed-off-by: Tzung-Bi Shih <tzungbi@kernel.org>
---
No v1. New in the series.
drivers/platform/chrome/cros_ec_proto.c | 5 +
drivers/platform/chrome/cros_ec_proto_test.c | 132 +++++++++++++++++++
2 files changed, 137 insertions(+)
diff --git a/drivers/platform/chrome/cros_ec_proto.c b/drivers/platform/chrome/cros_ec_proto.c
index 893b76703da6..6f5be9e5ede4 100644
--- a/drivers/platform/chrome/cros_ec_proto.c
+++ b/drivers/platform/chrome/cros_ec_proto.c
@@ -314,6 +314,11 @@ static int cros_ec_get_proto_info(struct cros_ec_device *ec_dev, int devidx)
goto exit;
}
+ if (ret == 0) {
+ ret = -EPROTO;
+ goto exit;
+ }
+
info = (struct ec_response_get_protocol_info *)msg->data;
switch (devidx) {
diff --git a/drivers/platform/chrome/cros_ec_proto_test.c b/drivers/platform/chrome/cros_ec_proto_test.c
index 14a4441a39fc..473714964cf2 100644
--- a/drivers/platform/chrome/cros_ec_proto_test.c
+++ b/drivers/platform/chrome/cros_ec_proto_test.c
@@ -406,6 +406,71 @@ static void cros_ec_proto_test_query_all_no_pd_return_error(struct kunit *test)
}
}
+static void cros_ec_proto_test_query_all_no_pd_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;
+
+ /* Set some garbage bytes. */
+ ec_dev->max_passthru = 0xbf;
+
+ /* For cros_ec_get_proto_info() without passthru. */
+ {
+ struct ec_response_get_protocol_info *data;
+
+ mock = cros_kunit_ec_xfer_mock_add(test, sizeof(*data));
+ KUNIT_ASSERT_PTR_NE(test, mock, NULL);
+
+ /*
+ * Although it doesn't check the value, provides valid sizes so that
+ * cros_ec_query_all() allocates din and dout correctly.
+ */
+ data = (struct ec_response_get_protocol_info *)mock->o_data;
+ data->max_request_packet_size = 0xbe;
+ data->max_response_packet_size = 0xef;
+ }
+
+ /* For cros_ec_get_proto_info() with passthru. */
+ {
+ mock = cros_kunit_ec_xfer_mock_add(test, 0);
+ KUNIT_ASSERT_PTR_NE(test, mock, NULL);
+ }
+
+ cros_ec_proto_test_query_all_pretest(test);
+ ret = cros_ec_query_all(ec_dev);
+ KUNIT_EXPECT_EQ(test, ret, 0);
+
+ /* For cros_ec_get_proto_info() without passthru. */
+ {
+ 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_PROTOCOL_INFO);
+ KUNIT_EXPECT_EQ(test, mock->msg.insize,
+ sizeof(struct ec_response_get_protocol_info));
+ KUNIT_EXPECT_EQ(test, mock->msg.outsize, 0);
+ }
+
+ /* For cros_ec_get_proto_info() with passthru. */
+ {
+ 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_PASSTHRU_OFFSET(CROS_EC_DEV_PD_INDEX) |
+ EC_CMD_GET_PROTOCOL_INFO);
+ KUNIT_EXPECT_EQ(test, mock->msg.insize,
+ sizeof(struct ec_response_get_protocol_info));
+ KUNIT_EXPECT_EQ(test, mock->msg.outsize, 0);
+
+ KUNIT_EXPECT_EQ(test, ec_dev->max_passthru, 0);
+ }
+}
+
static void cros_ec_proto_test_query_all_legacy_normal_v3_return_error(struct kunit *test)
{
struct cros_ec_proto_test_priv *priv = test->priv;
@@ -471,6 +536,71 @@ static void cros_ec_proto_test_query_all_legacy_normal_v3_return_error(struct ku
}
}
+static void cros_ec_proto_test_query_all_legacy_normal_v3_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;
+
+ /* For cros_ec_get_proto_info() without passthru. */
+ {
+ mock = cros_kunit_ec_xfer_mock_add(test, 0);
+ KUNIT_ASSERT_PTR_NE(test, mock, NULL);
+ }
+
+ /* For cros_ec_host_command_proto_query_v2(). */
+ {
+ struct ec_response_hello *data;
+
+ mock = cros_kunit_ec_xfer_mock_add(test, sizeof(*data));
+ KUNIT_ASSERT_PTR_NE(test, mock, NULL);
+
+ data = (struct ec_response_hello *)mock->o_data;
+ data->out_data = 0xa1b2c3d4;
+ }
+
+ cros_ec_proto_test_query_all_pretest(test);
+ ret = cros_ec_query_all(ec_dev);
+ KUNIT_EXPECT_EQ(test, ret, 0);
+
+ /* For cros_ec_get_proto_info() without passthru. */
+ {
+ 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_PROTOCOL_INFO);
+ KUNIT_EXPECT_EQ(test, mock->msg.insize,
+ sizeof(struct ec_response_get_protocol_info));
+ KUNIT_EXPECT_EQ(test, mock->msg.outsize, 0);
+ }
+
+ /* For cros_ec_host_command_proto_query_v2(). */
+ {
+ struct ec_params_hello *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, sizeof(struct ec_response_hello));
+ KUNIT_EXPECT_EQ(test, mock->msg.outsize, sizeof(*data));
+
+ data = (struct ec_params_hello *)mock->i_data;
+ KUNIT_EXPECT_EQ(test, data->in_data, 0xa0b0c0d0);
+
+ KUNIT_EXPECT_EQ(test, ec_dev->proto_version, 2);
+ KUNIT_EXPECT_EQ(test, ec_dev->max_request, EC_PROTO2_MAX_PARAM_SIZE);
+ KUNIT_EXPECT_EQ(test, ec_dev->max_response, EC_PROTO2_MAX_PARAM_SIZE);
+ KUNIT_EXPECT_EQ(test, ec_dev->max_passthru, 0);
+ KUNIT_EXPECT_PTR_EQ(test, ec_dev->pkt_xfer, NULL);
+ KUNIT_EXPECT_EQ(test, ec_dev->din_size, EC_PROTO2_MSG_BYTES);
+ KUNIT_EXPECT_EQ(test, ec_dev->dout_size, EC_PROTO2_MSG_BYTES);
+ }
+}
+
static void cros_ec_proto_test_query_all_legacy_xfer_error(struct kunit *test)
{
struct cros_ec_proto_test_priv *priv = test->priv;
@@ -998,7 +1128,9 @@ static struct kunit_case cros_ec_proto_test_cases[] = {
KUNIT_CASE(cros_ec_proto_test_check_result),
KUNIT_CASE(cros_ec_proto_test_query_all_normal),
KUNIT_CASE(cros_ec_proto_test_query_all_no_pd_return_error),
+ KUNIT_CASE(cros_ec_proto_test_query_all_no_pd_return0),
KUNIT_CASE(cros_ec_proto_test_query_all_legacy_normal_v3_return_error),
+ KUNIT_CASE(cros_ec_proto_test_query_all_legacy_normal_v3_return0),
KUNIT_CASE(cros_ec_proto_test_query_all_legacy_xfer_error),
KUNIT_CASE(cros_ec_proto_test_query_all_legacy_return_error),
KUNIT_CASE(cros_ec_proto_test_query_all_legacy_data_error),
--
2.36.1.255.ge46751e96f-goog
^ permalink raw reply related [flat|nested] 28+ messages in thread* Re: [PATCH v2 08/15] platform/chrome: cros_ec_proto: handle empty payload in getting proto info
2022-06-07 14:56 ` [PATCH v2 08/15] platform/chrome: cros_ec_proto: handle empty payload in getting proto info Tzung-Bi Shih
@ 2022-06-07 18:47 ` Guenter Roeck
2022-06-08 2:16 ` Tzung-Bi Shih
0 siblings, 1 reply; 28+ messages in thread
From: Guenter Roeck @ 2022-06-07 18:47 UTC (permalink / raw)
To: Tzung-Bi Shih; +Cc: bleung, groeck, chrome-platform, linux-kernel
On Tue, Jun 7, 2022 at 7:57 AM Tzung-Bi Shih <tzungbi@kernel.org> wrote:
>
> cros_ec_get_proto_info() expects to receive
> sizeof(struct ec_response_get_protocol_info) from send_command(). The
> payload is valid only if the return value is positive.
>
> Add Kunit tests for returning 0 in send_command() and handle the case in
> cros_ec_get_proto_info().
>
That should be two separate patches.
> Signed-off-by: Tzung-Bi Shih <tzungbi@kernel.org>
> ---
> No v1. New in the series.
>
> drivers/platform/chrome/cros_ec_proto.c | 5 +
> drivers/platform/chrome/cros_ec_proto_test.c | 132 +++++++++++++++++++
> 2 files changed, 137 insertions(+)
>
> diff --git a/drivers/platform/chrome/cros_ec_proto.c b/drivers/platform/chrome/cros_ec_proto.c
> index 893b76703da6..6f5be9e5ede4 100644
> --- a/drivers/platform/chrome/cros_ec_proto.c
> +++ b/drivers/platform/chrome/cros_ec_proto.c
> @@ -314,6 +314,11 @@ static int cros_ec_get_proto_info(struct cros_ec_device *ec_dev, int devidx)
> goto exit;
> }
>
> + if (ret == 0) {
> + ret = -EPROTO;
> + goto exit;
> + }
> +
I think you can move that into the if() statement above (which already
checks for ret >=0),
making it a special case of that situation.
Thanks,
Guenter
> info = (struct ec_response_get_protocol_info *)msg->data;
>
> switch (devidx) {
> diff --git a/drivers/platform/chrome/cros_ec_proto_test.c b/drivers/platform/chrome/cros_ec_proto_test.c
> index 14a4441a39fc..473714964cf2 100644
> --- a/drivers/platform/chrome/cros_ec_proto_test.c
> +++ b/drivers/platform/chrome/cros_ec_proto_test.c
> @@ -406,6 +406,71 @@ static void cros_ec_proto_test_query_all_no_pd_return_error(struct kunit *test)
> }
> }
>
> +static void cros_ec_proto_test_query_all_no_pd_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;
> +
> + /* Set some garbage bytes. */
> + ec_dev->max_passthru = 0xbf;
> +
> + /* For cros_ec_get_proto_info() without passthru. */
> + {
> + struct ec_response_get_protocol_info *data;
> +
> + mock = cros_kunit_ec_xfer_mock_add(test, sizeof(*data));
> + KUNIT_ASSERT_PTR_NE(test, mock, NULL);
> +
> + /*
> + * Although it doesn't check the value, provides valid sizes so that
> + * cros_ec_query_all() allocates din and dout correctly.
> + */
> + data = (struct ec_response_get_protocol_info *)mock->o_data;
> + data->max_request_packet_size = 0xbe;
> + data->max_response_packet_size = 0xef;
> + }
> +
> + /* For cros_ec_get_proto_info() with passthru. */
> + {
> + mock = cros_kunit_ec_xfer_mock_add(test, 0);
> + KUNIT_ASSERT_PTR_NE(test, mock, NULL);
> + }
> +
> + cros_ec_proto_test_query_all_pretest(test);
> + ret = cros_ec_query_all(ec_dev);
> + KUNIT_EXPECT_EQ(test, ret, 0);
> +
> + /* For cros_ec_get_proto_info() without passthru. */
> + {
> + 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_PROTOCOL_INFO);
> + KUNIT_EXPECT_EQ(test, mock->msg.insize,
> + sizeof(struct ec_response_get_protocol_info));
> + KUNIT_EXPECT_EQ(test, mock->msg.outsize, 0);
> + }
> +
> + /* For cros_ec_get_proto_info() with passthru. */
> + {
> + 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_PASSTHRU_OFFSET(CROS_EC_DEV_PD_INDEX) |
> + EC_CMD_GET_PROTOCOL_INFO);
> + KUNIT_EXPECT_EQ(test, mock->msg.insize,
> + sizeof(struct ec_response_get_protocol_info));
> + KUNIT_EXPECT_EQ(test, mock->msg.outsize, 0);
> +
> + KUNIT_EXPECT_EQ(test, ec_dev->max_passthru, 0);
> + }
> +}
> +
> static void cros_ec_proto_test_query_all_legacy_normal_v3_return_error(struct kunit *test)
> {
> struct cros_ec_proto_test_priv *priv = test->priv;
> @@ -471,6 +536,71 @@ static void cros_ec_proto_test_query_all_legacy_normal_v3_return_error(struct ku
> }
> }
>
> +static void cros_ec_proto_test_query_all_legacy_normal_v3_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;
> +
> + /* For cros_ec_get_proto_info() without passthru. */
> + {
> + mock = cros_kunit_ec_xfer_mock_add(test, 0);
> + KUNIT_ASSERT_PTR_NE(test, mock, NULL);
> + }
> +
> + /* For cros_ec_host_command_proto_query_v2(). */
> + {
> + struct ec_response_hello *data;
> +
> + mock = cros_kunit_ec_xfer_mock_add(test, sizeof(*data));
> + KUNIT_ASSERT_PTR_NE(test, mock, NULL);
> +
> + data = (struct ec_response_hello *)mock->o_data;
> + data->out_data = 0xa1b2c3d4;
> + }
> +
> + cros_ec_proto_test_query_all_pretest(test);
> + ret = cros_ec_query_all(ec_dev);
> + KUNIT_EXPECT_EQ(test, ret, 0);
> +
> + /* For cros_ec_get_proto_info() without passthru. */
> + {
> + 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_PROTOCOL_INFO);
> + KUNIT_EXPECT_EQ(test, mock->msg.insize,
> + sizeof(struct ec_response_get_protocol_info));
> + KUNIT_EXPECT_EQ(test, mock->msg.outsize, 0);
> + }
> +
> + /* For cros_ec_host_command_proto_query_v2(). */
> + {
> + struct ec_params_hello *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, sizeof(struct ec_response_hello));
> + KUNIT_EXPECT_EQ(test, mock->msg.outsize, sizeof(*data));
> +
> + data = (struct ec_params_hello *)mock->i_data;
> + KUNIT_EXPECT_EQ(test, data->in_data, 0xa0b0c0d0);
> +
> + KUNIT_EXPECT_EQ(test, ec_dev->proto_version, 2);
> + KUNIT_EXPECT_EQ(test, ec_dev->max_request, EC_PROTO2_MAX_PARAM_SIZE);
> + KUNIT_EXPECT_EQ(test, ec_dev->max_response, EC_PROTO2_MAX_PARAM_SIZE);
> + KUNIT_EXPECT_EQ(test, ec_dev->max_passthru, 0);
> + KUNIT_EXPECT_PTR_EQ(test, ec_dev->pkt_xfer, NULL);
> + KUNIT_EXPECT_EQ(test, ec_dev->din_size, EC_PROTO2_MSG_BYTES);
> + KUNIT_EXPECT_EQ(test, ec_dev->dout_size, EC_PROTO2_MSG_BYTES);
> + }
> +}
> +
> static void cros_ec_proto_test_query_all_legacy_xfer_error(struct kunit *test)
> {
> struct cros_ec_proto_test_priv *priv = test->priv;
> @@ -998,7 +1128,9 @@ static struct kunit_case cros_ec_proto_test_cases[] = {
> KUNIT_CASE(cros_ec_proto_test_check_result),
> KUNIT_CASE(cros_ec_proto_test_query_all_normal),
> KUNIT_CASE(cros_ec_proto_test_query_all_no_pd_return_error),
> + KUNIT_CASE(cros_ec_proto_test_query_all_no_pd_return0),
> KUNIT_CASE(cros_ec_proto_test_query_all_legacy_normal_v3_return_error),
> + KUNIT_CASE(cros_ec_proto_test_query_all_legacy_normal_v3_return0),
> KUNIT_CASE(cros_ec_proto_test_query_all_legacy_xfer_error),
> KUNIT_CASE(cros_ec_proto_test_query_all_legacy_return_error),
> KUNIT_CASE(cros_ec_proto_test_query_all_legacy_data_error),
> --
> 2.36.1.255.ge46751e96f-goog
>
^ permalink raw reply [flat|nested] 28+ messages in thread* Re: [PATCH v2 08/15] platform/chrome: cros_ec_proto: handle empty payload in getting proto info
2022-06-07 18:47 ` Guenter Roeck
@ 2022-06-08 2:16 ` Tzung-Bi Shih
2022-06-08 14:37 ` Guenter Roeck
0 siblings, 1 reply; 28+ messages in thread
From: Tzung-Bi Shih @ 2022-06-08 2:16 UTC (permalink / raw)
To: Guenter Roeck; +Cc: bleung, groeck, chrome-platform, linux-kernel
On Tue, Jun 07, 2022 at 11:47:56AM -0700, Guenter Roeck wrote:
> On Tue, Jun 7, 2022 at 7:57 AM Tzung-Bi Shih <tzungbi@kernel.org> wrote:
> >
> > cros_ec_get_proto_info() expects to receive
> > sizeof(struct ec_response_get_protocol_info) from send_command(). The
> > payload is valid only if the return value is positive.
> >
> > Add Kunit tests for returning 0 in send_command() and handle the case in
> > cros_ec_get_proto_info().
> >
> That should be two separate patches.
Ack, will separate them in next version. I put them together because I wrote
Kunit test first to make sure the second half takes effect (somehow TDD).
Could I still put the Kunit patch first (even if it introduces Kunit test
failure)?
>
> > Signed-off-by: Tzung-Bi Shih <tzungbi@kernel.org>
> > ---
> > No v1. New in the series.
> >
> > drivers/platform/chrome/cros_ec_proto.c | 5 +
> > drivers/platform/chrome/cros_ec_proto_test.c | 132 +++++++++++++++++++
> > 2 files changed, 137 insertions(+)
> >
> > diff --git a/drivers/platform/chrome/cros_ec_proto.c b/drivers/platform/chrome/cros_ec_proto.c
> > index 893b76703da6..6f5be9e5ede4 100644
> > --- a/drivers/platform/chrome/cros_ec_proto.c
> > +++ b/drivers/platform/chrome/cros_ec_proto.c
> > @@ -314,6 +314,11 @@ static int cros_ec_get_proto_info(struct cros_ec_device *ec_dev, int devidx)
> > goto exit;
> > }
> >
> > + if (ret == 0) {
> > + ret = -EPROTO;
> > + goto exit;
> > + }
> > +
>
> I think you can move that into the if() statement above (which already
> checks for ret >=0),
> making it a special case of that situation.
Nope, there is no "ret >= 0" (you could be confusing with
cros_ec_get_host_event_wake_mask()).
The result flow roughly like:
ret = send_command(...);
if (ret < 0)
goto exit;
mapped = cros_ec_map_error(...);
if (mapped) {
...
goto exit;
}
if (ret == 0) {
...
goto exit;
}
^ permalink raw reply [flat|nested] 28+ messages in thread* Re: [PATCH v2 08/15] platform/chrome: cros_ec_proto: handle empty payload in getting proto info
2022-06-08 2:16 ` Tzung-Bi Shih
@ 2022-06-08 14:37 ` Guenter Roeck
0 siblings, 0 replies; 28+ messages in thread
From: Guenter Roeck @ 2022-06-08 14:37 UTC (permalink / raw)
To: Tzung-Bi Shih
Cc: Benson Leung, Guenter Roeck,
open list:CHROME HARDWARE PLATFORM SUPPORT, linux-kernel
On Tue, Jun 7, 2022 at 7:17 PM Tzung-Bi Shih <tzungbi@kernel.org> wrote:
>
> On Tue, Jun 07, 2022 at 11:47:56AM -0700, Guenter Roeck wrote:
> > On Tue, Jun 7, 2022 at 7:57 AM Tzung-Bi Shih <tzungbi@kernel.org> wrote:
> > >
> > > cros_ec_get_proto_info() expects to receive
> > > sizeof(struct ec_response_get_protocol_info) from send_command(). The
> > > payload is valid only if the return value is positive.
> > >
> > > Add Kunit tests for returning 0 in send_command() and handle the case in
> > > cros_ec_get_proto_info().
> > >
> > That should be two separate patches.
>
> Ack, will separate them in next version. I put them together because I wrote
> Kunit test first to make sure the second half takes effect (somehow TDD).
>
> Could I still put the Kunit patch first (even if it introduces Kunit test
> failure)?
>
Sorry, I am running behind with e-mails.
If you want to, but why not let the fix come first ? If the unit test
patch is first, mayle add a note after --- indicating that it is
expected to fail and will be fixed with the next patch.
Thanks,
Guenter
> >
> > > Signed-off-by: Tzung-Bi Shih <tzungbi@kernel.org>
> > > ---
> > > No v1. New in the series.
> > >
> > > drivers/platform/chrome/cros_ec_proto.c | 5 +
> > > drivers/platform/chrome/cros_ec_proto_test.c | 132 +++++++++++++++++++
> > > 2 files changed, 137 insertions(+)
> > >
> > > diff --git a/drivers/platform/chrome/cros_ec_proto.c b/drivers/platform/chrome/cros_ec_proto.c
> > > index 893b76703da6..6f5be9e5ede4 100644
> > > --- a/drivers/platform/chrome/cros_ec_proto.c
> > > +++ b/drivers/platform/chrome/cros_ec_proto.c
> > > @@ -314,6 +314,11 @@ static int cros_ec_get_proto_info(struct cros_ec_device *ec_dev, int devidx)
> > > goto exit;
> > > }
> > >
> > > + if (ret == 0) {
> > > + ret = -EPROTO;
> > > + goto exit;
> > > + }
> > > +
> >
> > I think you can move that into the if() statement above (which already
> > checks for ret >=0),
> > making it a special case of that situation.
>
> Nope, there is no "ret >= 0" (you could be confusing with
> cros_ec_get_host_event_wake_mask()).
>
> The result flow roughly like:
>
> ret = send_command(...);
> if (ret < 0)
> goto exit;
>
> mapped = cros_ec_map_error(...);
> if (mapped) {
> ...
> goto exit;
> }
>
> if (ret == 0) {
> ...
> goto exit;
> }
^ permalink raw reply [flat|nested] 28+ messages in thread
* [PATCH v2 09/15] platform/chrome: cros_ec_proto: separate cros_ec_get_proto_info_legacy()
2022-06-07 14:56 [PATCH v2 00/15] platform/chrome: Kunit tests and refactor for cros_ec_query_all() Tzung-Bi Shih
` (7 preceding siblings ...)
2022-06-07 14:56 ` [PATCH v2 08/15] platform/chrome: cros_ec_proto: handle empty payload in getting proto info Tzung-Bi Shih
@ 2022-06-07 14:56 ` Tzung-Bi Shih
2022-06-07 18:50 ` Guenter Roeck
2022-06-07 14:56 ` [PATCH v2 10/15] platform/chrome: cros_ec_proto: handle empty payload in getting info legacy Tzung-Bi Shih
` (5 subsequent siblings)
14 siblings, 1 reply; 28+ messages in thread
From: Tzung-Bi Shih @ 2022-06-07 14:56 UTC (permalink / raw)
To: bleung, groeck; +Cc: chrome-platform, linux-kernel, tzungbi
Rename cros_ec_host_command_proto_query_v2() to
cros_ec_get_proto_info_legacy() and make it responsible for setting
`ec_dev` fields for EC protocol v2.
Signed-off-by: Tzung-Bi Shih <tzungbi@kernel.org>
---
Changes from v1:
- Preserve the "cros_ec_" prefix.
drivers/platform/chrome/cros_ec_proto.c | 72 +++++++++-----------
drivers/platform/chrome/cros_ec_proto_test.c | 22 +++---
2 files changed, 44 insertions(+), 50 deletions(-)
diff --git a/drivers/platform/chrome/cros_ec_proto.c b/drivers/platform/chrome/cros_ec_proto.c
index 6f5be9e5ede4..04b9704ed302 100644
--- a/drivers/platform/chrome/cros_ec_proto.c
+++ b/drivers/platform/chrome/cros_ec_proto.c
@@ -351,51 +351,57 @@ static int cros_ec_get_proto_info(struct cros_ec_device *ec_dev, int devidx)
return ret;
}
-static int cros_ec_host_command_proto_query_v2(struct cros_ec_device *ec_dev)
+static int cros_ec_get_proto_info_legacy(struct cros_ec_device *ec_dev)
{
struct cros_ec_command *msg;
- struct ec_params_hello *hello_params;
- struct ec_response_hello *hello_response;
+ struct ec_params_hello *params;
+ struct ec_response_hello *response;
int ret;
- int len = max(sizeof(*hello_params), sizeof(*hello_response));
- msg = kmalloc(sizeof(*msg) + len, GFP_KERNEL);
+ ec_dev->proto_version = 2;
+
+ msg = kzalloc(sizeof(*msg) + max(sizeof(*params), sizeof(*response)), GFP_KERNEL);
if (!msg)
return -ENOMEM;
- msg->version = 0;
msg->command = EC_CMD_HELLO;
- hello_params = (struct ec_params_hello *)msg->data;
- msg->outsize = sizeof(*hello_params);
- hello_response = (struct ec_response_hello *)msg->data;
- msg->insize = sizeof(*hello_response);
+ msg->insize = sizeof(*response);
+ msg->outsize = sizeof(*params);
- hello_params->in_data = 0xa0b0c0d0;
+ params = (struct ec_params_hello *)msg->data;
+ params->in_data = 0xa0b0c0d0;
ret = send_command(ec_dev, msg);
-
if (ret < 0) {
- dev_dbg(ec_dev->dev,
- "EC failed to respond to v2 hello: %d\n",
- ret);
+ dev_dbg(ec_dev->dev, "EC failed to respond to v2 hello: %d\n", ret);
goto exit;
- } else if (msg->result != EC_RES_SUCCESS) {
- dev_err(ec_dev->dev,
- "EC responded to v2 hello with error: %d\n",
- msg->result);
- ret = msg->result;
+ }
+
+ ret = cros_ec_map_error(msg->result);
+ if (ret) {
+ dev_err(ec_dev->dev, "EC responded to v2 hello with error: %d\n", msg->result);
goto exit;
- } else if (hello_response->out_data != 0xa1b2c3d4) {
+ }
+
+ response = (struct ec_response_hello *)msg->data;
+ if (response->out_data != 0xa1b2c3d4) {
dev_err(ec_dev->dev,
"EC responded to v2 hello with bad result: %u\n",
- hello_response->out_data);
+ response->out_data);
ret = -EBADMSG;
goto exit;
}
- ret = 0;
+ ec_dev->max_request = EC_PROTO2_MAX_PARAM_SIZE;
+ ec_dev->max_response = EC_PROTO2_MAX_PARAM_SIZE;
+ ec_dev->max_passthru = 0;
+ ec_dev->pkt_xfer = NULL;
+ ec_dev->din_size = EC_PROTO2_MSG_BYTES;
+ ec_dev->dout_size = EC_PROTO2_MSG_BYTES;
- exit:
+ dev_dbg(ec_dev->dev, "falling back to proto v2\n");
+ ret = 0;
+exit:
kfree(msg);
return ret;
}
@@ -467,20 +473,8 @@ int cros_ec_query_all(struct cros_ec_device *ec_dev)
cros_ec_get_proto_info(ec_dev, CROS_EC_DEV_PD_INDEX);
} else {
/* Try querying with a v2 hello message. */
- ec_dev->proto_version = 2;
- ret = cros_ec_host_command_proto_query_v2(ec_dev);
-
- if (ret == 0) {
- /* V2 hello succeeded. */
- dev_dbg(ec_dev->dev, "falling back to proto v2\n");
-
- ec_dev->max_request = EC_PROTO2_MAX_PARAM_SIZE;
- ec_dev->max_response = EC_PROTO2_MAX_PARAM_SIZE;
- ec_dev->max_passthru = 0;
- ec_dev->pkt_xfer = NULL;
- ec_dev->din_size = EC_PROTO2_MSG_BYTES;
- ec_dev->dout_size = EC_PROTO2_MSG_BYTES;
- } else {
+ ret = cros_ec_get_proto_info_legacy(ec_dev);
+ if (ret) {
/*
* It's possible for a test to occur too early when
* the EC isn't listening. If this happens, we'll
@@ -488,7 +482,7 @@ int cros_ec_query_all(struct cros_ec_device *ec_dev)
*/
ec_dev->proto_version = EC_PROTO_VERSION_UNKNOWN;
dev_dbg(ec_dev->dev, "EC query failed: %d\n", ret);
- goto exit;
+ return ret;
}
}
diff --git a/drivers/platform/chrome/cros_ec_proto_test.c b/drivers/platform/chrome/cros_ec_proto_test.c
index 473714964cf2..9f7d9666369f 100644
--- a/drivers/platform/chrome/cros_ec_proto_test.c
+++ b/drivers/platform/chrome/cros_ec_proto_test.c
@@ -484,7 +484,7 @@ static void cros_ec_proto_test_query_all_legacy_normal_v3_return_error(struct ku
KUNIT_ASSERT_PTR_NE(test, mock, NULL);
}
- /* For cros_ec_host_command_proto_query_v2(). */
+ /* For cros_ec_get_proto_info_legacy(). */
{
struct ec_response_hello *data;
@@ -511,7 +511,7 @@ static void cros_ec_proto_test_query_all_legacy_normal_v3_return_error(struct ku
KUNIT_EXPECT_EQ(test, mock->msg.outsize, 0);
}
- /* For cros_ec_host_command_proto_query_v2(). */
+ /* For cros_ec_get_proto_info_legacy(). */
{
struct ec_params_hello *data;
@@ -549,7 +549,7 @@ static void cros_ec_proto_test_query_all_legacy_normal_v3_return0(struct kunit *
KUNIT_ASSERT_PTR_NE(test, mock, NULL);
}
- /* For cros_ec_host_command_proto_query_v2(). */
+ /* For cros_ec_get_proto_info_legacy(). */
{
struct ec_response_hello *data;
@@ -576,7 +576,7 @@ static void cros_ec_proto_test_query_all_legacy_normal_v3_return0(struct kunit *
KUNIT_EXPECT_EQ(test, mock->msg.outsize, 0);
}
- /* For cros_ec_host_command_proto_query_v2(). */
+ /* For cros_ec_get_proto_info_legacy(). */
{
struct ec_params_hello *data;
@@ -614,7 +614,7 @@ static void cros_ec_proto_test_query_all_legacy_xfer_error(struct kunit *test)
KUNIT_ASSERT_PTR_NE(test, mock, NULL);
}
- /* For cros_ec_host_command_proto_query_v2(). */
+ /* For cros_ec_get_proto_info_legacy(). */
{
mock = cros_kunit_ec_xfer_mock_addx(test, -EIO, EC_RES_SUCCESS, 0);
KUNIT_ASSERT_PTR_NE(test, mock, NULL);
@@ -637,7 +637,7 @@ static void cros_ec_proto_test_query_all_legacy_xfer_error(struct kunit *test)
KUNIT_EXPECT_EQ(test, mock->msg.outsize, 0);
}
- /* For cros_ec_host_command_proto_query_v2(). */
+ /* For cros_ec_get_proto_info_legacy(). */
{
mock = cros_kunit_ec_xfer_mock_next();
KUNIT_EXPECT_PTR_NE(test, mock, NULL);
@@ -662,7 +662,7 @@ static void cros_ec_proto_test_query_all_legacy_return_error(struct kunit *test)
KUNIT_ASSERT_PTR_NE(test, mock, NULL);
}
- /* For cros_ec_host_command_proto_query_v2(). */
+ /* For cros_ec_get_proto_info_legacy(). */
{
mock = cros_kunit_ec_xfer_mock_addx(test, 0, EC_RES_INVALID_COMMAND, 0);
KUNIT_ASSERT_PTR_NE(test, mock, NULL);
@@ -670,7 +670,7 @@ static void cros_ec_proto_test_query_all_legacy_return_error(struct kunit *test)
cros_ec_proto_test_query_all_pretest(test);
ret = cros_ec_query_all(ec_dev);
- KUNIT_EXPECT_EQ(test, ret, EC_RES_INVALID_COMMAND);
+ KUNIT_EXPECT_EQ(test, ret, -EOPNOTSUPP);
KUNIT_EXPECT_EQ(test, ec_dev->proto_version, EC_PROTO_VERSION_UNKNOWN);
/* For cros_ec_get_proto_info() without passthru. */
@@ -685,7 +685,7 @@ static void cros_ec_proto_test_query_all_legacy_return_error(struct kunit *test)
KUNIT_EXPECT_EQ(test, mock->msg.outsize, 0);
}
- /* For cros_ec_host_command_proto_query_v2(). */
+ /* For cros_ec_get_proto_info_legacy(). */
{
mock = cros_kunit_ec_xfer_mock_next();
KUNIT_EXPECT_PTR_NE(test, mock, NULL);
@@ -710,7 +710,7 @@ static void cros_ec_proto_test_query_all_legacy_data_error(struct kunit *test)
KUNIT_ASSERT_PTR_NE(test, mock, NULL);
}
- /* For cros_ec_host_command_proto_query_v2(). */
+ /* For cros_ec_get_proto_info_legacy(). */
{
struct ec_response_hello *data;
@@ -738,7 +738,7 @@ static void cros_ec_proto_test_query_all_legacy_data_error(struct kunit *test)
KUNIT_EXPECT_EQ(test, mock->msg.outsize, 0);
}
- /* For cros_ec_host_command_proto_query_v2(). */
+ /* For cros_ec_get_proto_info_legacy(). */
{
mock = cros_kunit_ec_xfer_mock_next();
KUNIT_EXPECT_PTR_NE(test, mock, NULL);
--
2.36.1.255.ge46751e96f-goog
^ permalink raw reply related [flat|nested] 28+ messages in thread* Re: [PATCH v2 09/15] platform/chrome: cros_ec_proto: separate cros_ec_get_proto_info_legacy()
2022-06-07 14:56 ` [PATCH v2 09/15] platform/chrome: cros_ec_proto: separate cros_ec_get_proto_info_legacy() Tzung-Bi Shih
@ 2022-06-07 18:50 ` Guenter Roeck
0 siblings, 0 replies; 28+ messages in thread
From: Guenter Roeck @ 2022-06-07 18:50 UTC (permalink / raw)
To: Tzung-Bi Shih; +Cc: bleung, groeck, chrome-platform, linux-kernel
On Tue, Jun 7, 2022 at 7:57 AM Tzung-Bi Shih <tzungbi@kernel.org> wrote:
>
> Rename cros_ec_host_command_proto_query_v2() to
> cros_ec_get_proto_info_legacy() and make it responsible for setting
> `ec_dev` fields for EC protocol v2.
>
> Signed-off-by: Tzung-Bi Shih <tzungbi@kernel.org>
Reviewed-by: Guenter Roeck <groeck@chromium.org>
> ---
> Changes from v1:
> - Preserve the "cros_ec_" prefix.
>
> drivers/platform/chrome/cros_ec_proto.c | 72 +++++++++-----------
> drivers/platform/chrome/cros_ec_proto_test.c | 22 +++---
> 2 files changed, 44 insertions(+), 50 deletions(-)
>
> diff --git a/drivers/platform/chrome/cros_ec_proto.c b/drivers/platform/chrome/cros_ec_proto.c
> index 6f5be9e5ede4..04b9704ed302 100644
> --- a/drivers/platform/chrome/cros_ec_proto.c
> +++ b/drivers/platform/chrome/cros_ec_proto.c
> @@ -351,51 +351,57 @@ static int cros_ec_get_proto_info(struct cros_ec_device *ec_dev, int devidx)
> return ret;
> }
>
> -static int cros_ec_host_command_proto_query_v2(struct cros_ec_device *ec_dev)
> +static int cros_ec_get_proto_info_legacy(struct cros_ec_device *ec_dev)
> {
> struct cros_ec_command *msg;
> - struct ec_params_hello *hello_params;
> - struct ec_response_hello *hello_response;
> + struct ec_params_hello *params;
> + struct ec_response_hello *response;
> int ret;
> - int len = max(sizeof(*hello_params), sizeof(*hello_response));
>
> - msg = kmalloc(sizeof(*msg) + len, GFP_KERNEL);
> + ec_dev->proto_version = 2;
> +
> + msg = kzalloc(sizeof(*msg) + max(sizeof(*params), sizeof(*response)), GFP_KERNEL);
> if (!msg)
> return -ENOMEM;
>
> - msg->version = 0;
> msg->command = EC_CMD_HELLO;
> - hello_params = (struct ec_params_hello *)msg->data;
> - msg->outsize = sizeof(*hello_params);
> - hello_response = (struct ec_response_hello *)msg->data;
> - msg->insize = sizeof(*hello_response);
> + msg->insize = sizeof(*response);
> + msg->outsize = sizeof(*params);
>
> - hello_params->in_data = 0xa0b0c0d0;
> + params = (struct ec_params_hello *)msg->data;
> + params->in_data = 0xa0b0c0d0;
>
> ret = send_command(ec_dev, msg);
> -
> if (ret < 0) {
> - dev_dbg(ec_dev->dev,
> - "EC failed to respond to v2 hello: %d\n",
> - ret);
> + dev_dbg(ec_dev->dev, "EC failed to respond to v2 hello: %d\n", ret);
> goto exit;
> - } else if (msg->result != EC_RES_SUCCESS) {
> - dev_err(ec_dev->dev,
> - "EC responded to v2 hello with error: %d\n",
> - msg->result);
> - ret = msg->result;
> + }
> +
> + ret = cros_ec_map_error(msg->result);
> + if (ret) {
> + dev_err(ec_dev->dev, "EC responded to v2 hello with error: %d\n", msg->result);
> goto exit;
> - } else if (hello_response->out_data != 0xa1b2c3d4) {
> + }
> +
> + response = (struct ec_response_hello *)msg->data;
> + if (response->out_data != 0xa1b2c3d4) {
> dev_err(ec_dev->dev,
> "EC responded to v2 hello with bad result: %u\n",
> - hello_response->out_data);
> + response->out_data);
> ret = -EBADMSG;
> goto exit;
> }
>
> - ret = 0;
> + ec_dev->max_request = EC_PROTO2_MAX_PARAM_SIZE;
> + ec_dev->max_response = EC_PROTO2_MAX_PARAM_SIZE;
> + ec_dev->max_passthru = 0;
> + ec_dev->pkt_xfer = NULL;
> + ec_dev->din_size = EC_PROTO2_MSG_BYTES;
> + ec_dev->dout_size = EC_PROTO2_MSG_BYTES;
>
> - exit:
> + dev_dbg(ec_dev->dev, "falling back to proto v2\n");
> + ret = 0;
> +exit:
> kfree(msg);
> return ret;
> }
> @@ -467,20 +473,8 @@ int cros_ec_query_all(struct cros_ec_device *ec_dev)
> cros_ec_get_proto_info(ec_dev, CROS_EC_DEV_PD_INDEX);
> } else {
> /* Try querying with a v2 hello message. */
> - ec_dev->proto_version = 2;
> - ret = cros_ec_host_command_proto_query_v2(ec_dev);
> -
> - if (ret == 0) {
> - /* V2 hello succeeded. */
> - dev_dbg(ec_dev->dev, "falling back to proto v2\n");
> -
> - ec_dev->max_request = EC_PROTO2_MAX_PARAM_SIZE;
> - ec_dev->max_response = EC_PROTO2_MAX_PARAM_SIZE;
> - ec_dev->max_passthru = 0;
> - ec_dev->pkt_xfer = NULL;
> - ec_dev->din_size = EC_PROTO2_MSG_BYTES;
> - ec_dev->dout_size = EC_PROTO2_MSG_BYTES;
> - } else {
> + ret = cros_ec_get_proto_info_legacy(ec_dev);
> + if (ret) {
> /*
> * It's possible for a test to occur too early when
> * the EC isn't listening. If this happens, we'll
> @@ -488,7 +482,7 @@ int cros_ec_query_all(struct cros_ec_device *ec_dev)
> */
> ec_dev->proto_version = EC_PROTO_VERSION_UNKNOWN;
> dev_dbg(ec_dev->dev, "EC query failed: %d\n", ret);
> - goto exit;
> + return ret;
> }
> }
>
> diff --git a/drivers/platform/chrome/cros_ec_proto_test.c b/drivers/platform/chrome/cros_ec_proto_test.c
> index 473714964cf2..9f7d9666369f 100644
> --- a/drivers/platform/chrome/cros_ec_proto_test.c
> +++ b/drivers/platform/chrome/cros_ec_proto_test.c
> @@ -484,7 +484,7 @@ static void cros_ec_proto_test_query_all_legacy_normal_v3_return_error(struct ku
> KUNIT_ASSERT_PTR_NE(test, mock, NULL);
> }
>
> - /* For cros_ec_host_command_proto_query_v2(). */
> + /* For cros_ec_get_proto_info_legacy(). */
> {
> struct ec_response_hello *data;
>
> @@ -511,7 +511,7 @@ static void cros_ec_proto_test_query_all_legacy_normal_v3_return_error(struct ku
> KUNIT_EXPECT_EQ(test, mock->msg.outsize, 0);
> }
>
> - /* For cros_ec_host_command_proto_query_v2(). */
> + /* For cros_ec_get_proto_info_legacy(). */
> {
> struct ec_params_hello *data;
>
> @@ -549,7 +549,7 @@ static void cros_ec_proto_test_query_all_legacy_normal_v3_return0(struct kunit *
> KUNIT_ASSERT_PTR_NE(test, mock, NULL);
> }
>
> - /* For cros_ec_host_command_proto_query_v2(). */
> + /* For cros_ec_get_proto_info_legacy(). */
> {
> struct ec_response_hello *data;
>
> @@ -576,7 +576,7 @@ static void cros_ec_proto_test_query_all_legacy_normal_v3_return0(struct kunit *
> KUNIT_EXPECT_EQ(test, mock->msg.outsize, 0);
> }
>
> - /* For cros_ec_host_command_proto_query_v2(). */
> + /* For cros_ec_get_proto_info_legacy(). */
> {
> struct ec_params_hello *data;
>
> @@ -614,7 +614,7 @@ static void cros_ec_proto_test_query_all_legacy_xfer_error(struct kunit *test)
> KUNIT_ASSERT_PTR_NE(test, mock, NULL);
> }
>
> - /* For cros_ec_host_command_proto_query_v2(). */
> + /* For cros_ec_get_proto_info_legacy(). */
> {
> mock = cros_kunit_ec_xfer_mock_addx(test, -EIO, EC_RES_SUCCESS, 0);
> KUNIT_ASSERT_PTR_NE(test, mock, NULL);
> @@ -637,7 +637,7 @@ static void cros_ec_proto_test_query_all_legacy_xfer_error(struct kunit *test)
> KUNIT_EXPECT_EQ(test, mock->msg.outsize, 0);
> }
>
> - /* For cros_ec_host_command_proto_query_v2(). */
> + /* For cros_ec_get_proto_info_legacy(). */
> {
> mock = cros_kunit_ec_xfer_mock_next();
> KUNIT_EXPECT_PTR_NE(test, mock, NULL);
> @@ -662,7 +662,7 @@ static void cros_ec_proto_test_query_all_legacy_return_error(struct kunit *test)
> KUNIT_ASSERT_PTR_NE(test, mock, NULL);
> }
>
> - /* For cros_ec_host_command_proto_query_v2(). */
> + /* For cros_ec_get_proto_info_legacy(). */
> {
> mock = cros_kunit_ec_xfer_mock_addx(test, 0, EC_RES_INVALID_COMMAND, 0);
> KUNIT_ASSERT_PTR_NE(test, mock, NULL);
> @@ -670,7 +670,7 @@ static void cros_ec_proto_test_query_all_legacy_return_error(struct kunit *test)
>
> cros_ec_proto_test_query_all_pretest(test);
> ret = cros_ec_query_all(ec_dev);
> - KUNIT_EXPECT_EQ(test, ret, EC_RES_INVALID_COMMAND);
> + KUNIT_EXPECT_EQ(test, ret, -EOPNOTSUPP);
> KUNIT_EXPECT_EQ(test, ec_dev->proto_version, EC_PROTO_VERSION_UNKNOWN);
>
> /* For cros_ec_get_proto_info() without passthru. */
> @@ -685,7 +685,7 @@ static void cros_ec_proto_test_query_all_legacy_return_error(struct kunit *test)
> KUNIT_EXPECT_EQ(test, mock->msg.outsize, 0);
> }
>
> - /* For cros_ec_host_command_proto_query_v2(). */
> + /* For cros_ec_get_proto_info_legacy(). */
> {
> mock = cros_kunit_ec_xfer_mock_next();
> KUNIT_EXPECT_PTR_NE(test, mock, NULL);
> @@ -710,7 +710,7 @@ static void cros_ec_proto_test_query_all_legacy_data_error(struct kunit *test)
> KUNIT_ASSERT_PTR_NE(test, mock, NULL);
> }
>
> - /* For cros_ec_host_command_proto_query_v2(). */
> + /* For cros_ec_get_proto_info_legacy(). */
> {
> struct ec_response_hello *data;
>
> @@ -738,7 +738,7 @@ static void cros_ec_proto_test_query_all_legacy_data_error(struct kunit *test)
> KUNIT_EXPECT_EQ(test, mock->msg.outsize, 0);
> }
>
> - /* For cros_ec_host_command_proto_query_v2(). */
> + /* For cros_ec_get_proto_info_legacy(). */
> {
> mock = cros_kunit_ec_xfer_mock_next();
> KUNIT_EXPECT_PTR_NE(test, mock, NULL);
> --
> 2.36.1.255.ge46751e96f-goog
>
^ permalink raw reply [flat|nested] 28+ messages in thread
* [PATCH v2 10/15] platform/chrome: cros_ec_proto: handle empty payload in getting info legacy
2022-06-07 14:56 [PATCH v2 00/15] platform/chrome: Kunit tests and refactor for cros_ec_query_all() Tzung-Bi Shih
` (8 preceding siblings ...)
2022-06-07 14:56 ` [PATCH v2 09/15] platform/chrome: cros_ec_proto: separate cros_ec_get_proto_info_legacy() Tzung-Bi Shih
@ 2022-06-07 14:56 ` Tzung-Bi Shih
2022-06-07 14:56 ` [PATCH v2 11/15] platform/chrome: cros_ec: don't allocate `din` and `dout` in cros_ec_register() Tzung-Bi Shih
` (4 subsequent siblings)
14 siblings, 0 replies; 28+ messages in thread
From: Tzung-Bi Shih @ 2022-06-07 14:56 UTC (permalink / raw)
To: bleung, groeck; +Cc: chrome-platform, linux-kernel, tzungbi
cros_ec_get_proto_info_legacy() expects to receive
sizeof(struct ec_response_hello) from send_command(). The payload is
valid only if the return value is positive.
Add a Kunit test for returning 0 in send_command() and handle the case
in cros_ec_get_proto_info_legacy().
Signed-off-by: Tzung-Bi Shih <tzungbi@kernel.org>
---
No v1. New in the series.
drivers/platform/chrome/cros_ec_proto.c | 12 +++--
drivers/platform/chrome/cros_ec_proto_test.c | 49 ++++++++++++++++++++
2 files changed, 58 insertions(+), 3 deletions(-)
diff --git a/drivers/platform/chrome/cros_ec_proto.c b/drivers/platform/chrome/cros_ec_proto.c
index 04b9704ed302..473654f50bca 100644
--- a/drivers/platform/chrome/cros_ec_proto.c
+++ b/drivers/platform/chrome/cros_ec_proto.c
@@ -356,7 +356,7 @@ static int cros_ec_get_proto_info_legacy(struct cros_ec_device *ec_dev)
struct cros_ec_command *msg;
struct ec_params_hello *params;
struct ec_response_hello *response;
- int ret;
+ int ret, mapped;
ec_dev->proto_version = 2;
@@ -377,12 +377,18 @@ static int cros_ec_get_proto_info_legacy(struct cros_ec_device *ec_dev)
goto exit;
}
- ret = cros_ec_map_error(msg->result);
- if (ret) {
+ mapped = cros_ec_map_error(msg->result);
+ if (mapped) {
+ ret = mapped;
dev_err(ec_dev->dev, "EC responded to v2 hello with error: %d\n", msg->result);
goto exit;
}
+ if (ret == 0) {
+ ret = -EPROTO;
+ goto exit;
+ }
+
response = (struct ec_response_hello *)msg->data;
if (response->out_data != 0xa1b2c3d4) {
dev_err(ec_dev->dev,
diff --git a/drivers/platform/chrome/cros_ec_proto_test.c b/drivers/platform/chrome/cros_ec_proto_test.c
index 9f7d9666369f..730248be42a7 100644
--- a/drivers/platform/chrome/cros_ec_proto_test.c
+++ b/drivers/platform/chrome/cros_ec_proto_test.c
@@ -750,6 +750,54 @@ static void cros_ec_proto_test_query_all_legacy_data_error(struct kunit *test)
}
}
+static void cros_ec_proto_test_query_all_legacy_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;
+
+ /* For cros_ec_get_proto_info() without passthru. */
+ {
+ mock = cros_kunit_ec_xfer_mock_addx(test, 0, EC_RES_INVALID_COMMAND, 0);
+ KUNIT_ASSERT_PTR_NE(test, mock, NULL);
+ }
+
+ /* For cros_ec_get_proto_info_legacy(). */
+ {
+ mock = cros_kunit_ec_xfer_mock_add(test, 0);
+ KUNIT_ASSERT_PTR_NE(test, mock, NULL);
+ }
+
+ cros_ec_proto_test_query_all_pretest(test);
+ ret = cros_ec_query_all(ec_dev);
+ KUNIT_EXPECT_EQ(test, ret, -EPROTO);
+ KUNIT_EXPECT_EQ(test, ec_dev->proto_version, EC_PROTO_VERSION_UNKNOWN);
+
+ /* For cros_ec_get_proto_info() without passthru. */
+ {
+ 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_PROTOCOL_INFO);
+ KUNIT_EXPECT_EQ(test, mock->msg.insize,
+ sizeof(struct ec_response_get_protocol_info));
+ KUNIT_EXPECT_EQ(test, mock->msg.outsize, 0);
+ }
+
+ /* For cros_ec_get_proto_info_legacy(). */
+ {
+ 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, sizeof(struct ec_response_hello));
+ KUNIT_EXPECT_EQ(test, mock->msg.outsize, sizeof(struct ec_params_hello));
+ }
+}
+
static void cros_ec_proto_test_query_all_no_mkbp(struct kunit *test)
{
struct cros_ec_proto_test_priv *priv = test->priv;
@@ -1134,6 +1182,7 @@ static struct kunit_case cros_ec_proto_test_cases[] = {
KUNIT_CASE(cros_ec_proto_test_query_all_legacy_xfer_error),
KUNIT_CASE(cros_ec_proto_test_query_all_legacy_return_error),
KUNIT_CASE(cros_ec_proto_test_query_all_legacy_data_error),
+ KUNIT_CASE(cros_ec_proto_test_query_all_legacy_return0),
KUNIT_CASE(cros_ec_proto_test_query_all_no_mkbp),
KUNIT_CASE(cros_ec_proto_test_query_all_no_host_sleep),
KUNIT_CASE(cros_ec_proto_test_query_all_default_wake_mask_return_error),
--
2.36.1.255.ge46751e96f-goog
^ permalink raw reply related [flat|nested] 28+ messages in thread* [PATCH v2 11/15] platform/chrome: cros_ec: don't allocate `din` and `dout` in cros_ec_register()
2022-06-07 14:56 [PATCH v2 00/15] platform/chrome: Kunit tests and refactor for cros_ec_query_all() Tzung-Bi Shih
` (9 preceding siblings ...)
2022-06-07 14:56 ` [PATCH v2 10/15] platform/chrome: cros_ec_proto: handle empty payload in getting info legacy Tzung-Bi Shih
@ 2022-06-07 14:56 ` Tzung-Bi Shih
2022-06-07 14:56 ` [PATCH v2 12/15] platform/chrome: use krealloc() for `din` and `dout` Tzung-Bi Shih
` (3 subsequent siblings)
14 siblings, 0 replies; 28+ messages in thread
From: Tzung-Bi Shih @ 2022-06-07 14:56 UTC (permalink / raw)
To: bleung, groeck; +Cc: chrome-platform, linux-kernel, tzungbi
Don't allocate `din` and `dout` in cros_ec_register() as they will be
allocated soon in cros_ec_query_all().
Signed-off-by: Tzung-Bi Shih <tzungbi@kernel.org>
---
No v1. New in the series.
drivers/platform/chrome/cros_ec.c | 10 ++--------
1 file changed, 2 insertions(+), 8 deletions(-)
diff --git a/drivers/platform/chrome/cros_ec.c b/drivers/platform/chrome/cros_ec.c
index e51a3f2176c7..29d3b544dafb 100644
--- a/drivers/platform/chrome/cros_ec.c
+++ b/drivers/platform/chrome/cros_ec.c
@@ -188,14 +188,8 @@ int cros_ec_register(struct cros_ec_device *ec_dev)
ec_dev->max_passthru = 0;
ec_dev->ec = NULL;
ec_dev->pd = NULL;
-
- ec_dev->din = devm_kzalloc(dev, ec_dev->din_size, GFP_KERNEL);
- if (!ec_dev->din)
- return -ENOMEM;
-
- ec_dev->dout = devm_kzalloc(dev, ec_dev->dout_size, GFP_KERNEL);
- if (!ec_dev->dout)
- return -ENOMEM;
+ ec_dev->din = NULL;
+ ec_dev->dout = NULL;
mutex_init(&ec_dev->lock);
--
2.36.1.255.ge46751e96f-goog
^ permalink raw reply related [flat|nested] 28+ messages in thread* [PATCH v2 12/15] platform/chrome: use krealloc() for `din` and `dout`
2022-06-07 14:56 [PATCH v2 00/15] platform/chrome: Kunit tests and refactor for cros_ec_query_all() Tzung-Bi Shih
` (10 preceding siblings ...)
2022-06-07 14:56 ` [PATCH v2 11/15] platform/chrome: cros_ec: don't allocate `din` and `dout` in cros_ec_register() Tzung-Bi Shih
@ 2022-06-07 14:56 ` Tzung-Bi Shih
2022-06-07 19:06 ` Guenter Roeck
2022-06-07 14:56 ` [PATCH v2 13/15] platform/chrome: cros_ec_proto: don't show MKBP version if unsupported Tzung-Bi Shih
` (2 subsequent siblings)
14 siblings, 1 reply; 28+ messages in thread
From: Tzung-Bi Shih @ 2022-06-07 14:56 UTC (permalink / raw)
To: bleung, groeck; +Cc: chrome-platform, linux-kernel, tzungbi
Use krealloc() to re-allocate `din` and `dout`. Don't use devm variant
because the two buffers could be re-allocated multiple times during
runtime. Their life cycles aren't quite aligned to the device's.
Free the memory in cros_ec_unregister() if any.
No need to free memory if krealloc() fails. They will be freed
eventually in either of the following:
- Error handling path in cros_ec_register().
- In cros_ec_unregister().
- Next krealloc() in cros_ec_query_all().
Signed-off-by: Tzung-Bi Shih <tzungbi@kernel.org>
---
Changes from v1:
- Don't use devm.
- Free in cros_ec_unregister().
drivers/platform/chrome/cros_ec.c | 4 +++
drivers/platform/chrome/cros_ec_proto.c | 29 +++++++-------------
drivers/platform/chrome/cros_ec_proto_test.c | 3 +-
3 files changed, 15 insertions(+), 21 deletions(-)
diff --git a/drivers/platform/chrome/cros_ec.c b/drivers/platform/chrome/cros_ec.c
index 29d3b544dafb..fb8cb8a73295 100644
--- a/drivers/platform/chrome/cros_ec.c
+++ b/drivers/platform/chrome/cros_ec.c
@@ -285,6 +285,8 @@ int cros_ec_register(struct cros_ec_device *ec_dev)
exit:
platform_device_unregister(ec_dev->ec);
platform_device_unregister(ec_dev->pd);
+ kfree(ec_dev->din);
+ kfree(ec_dev->dout);
return err;
}
EXPORT_SYMBOL(cros_ec_register);
@@ -302,6 +304,8 @@ void cros_ec_unregister(struct cros_ec_device *ec_dev)
if (ec_dev->pd)
platform_device_unregister(ec_dev->pd);
platform_device_unregister(ec_dev->ec);
+ kfree(ec_dev->din);
+ kfree(ec_dev->dout);
}
EXPORT_SYMBOL(cros_ec_unregister);
diff --git a/drivers/platform/chrome/cros_ec_proto.c b/drivers/platform/chrome/cros_ec_proto.c
index 473654f50bca..de6bc457e496 100644
--- a/drivers/platform/chrome/cros_ec_proto.c
+++ b/drivers/platform/chrome/cros_ec_proto.c
@@ -469,9 +469,9 @@ static int cros_ec_get_host_command_version_mask(struct cros_ec_device *ec_dev,
*/
int cros_ec_query_all(struct cros_ec_device *ec_dev)
{
- struct device *dev = ec_dev->dev;
u32 ver_mask = 0;
int ret;
+ u8 *din, *dout;
/* First try sending with proto v3. */
if (!cros_ec_get_proto_info(ec_dev, CROS_EC_DEV_EC_INDEX)) {
@@ -492,21 +492,15 @@ int cros_ec_query_all(struct cros_ec_device *ec_dev)
}
}
- devm_kfree(dev, ec_dev->din);
- devm_kfree(dev, ec_dev->dout);
-
- ec_dev->din = devm_kzalloc(dev, ec_dev->din_size, GFP_KERNEL);
- if (!ec_dev->din) {
- ret = -ENOMEM;
- goto exit;
- }
+ din = krealloc(ec_dev->din, ec_dev->din_size, GFP_KERNEL);
+ if (!din)
+ return -ENOMEM;
+ ec_dev->din = din;
- ec_dev->dout = devm_kzalloc(dev, ec_dev->dout_size, GFP_KERNEL);
- if (!ec_dev->dout) {
- devm_kfree(dev, ec_dev->din);
- ret = -ENOMEM;
- goto exit;
- }
+ dout = krealloc(ec_dev->dout, ec_dev->dout_size, GFP_KERNEL);
+ if (!dout)
+ return -ENOMEM;
+ ec_dev->dout = dout;
/* Probe if MKBP event is supported */
ret = cros_ec_get_host_command_version_mask(ec_dev,
@@ -555,10 +549,7 @@ int cros_ec_query_all(struct cros_ec_device *ec_dev)
"failed to retrieve wake mask: %d\n", ret);
}
- ret = 0;
-
-exit:
- return ret;
+ return 0;
}
EXPORT_SYMBOL(cros_ec_query_all);
diff --git a/drivers/platform/chrome/cros_ec_proto_test.c b/drivers/platform/chrome/cros_ec_proto_test.c
index 730248be42a7..27b81a5a9880 100644
--- a/drivers/platform/chrome/cros_ec_proto_test.c
+++ b/drivers/platform/chrome/cros_ec_proto_test.c
@@ -180,8 +180,7 @@ static void cros_ec_proto_test_query_all_pretest(struct kunit *test)
/*
* cros_ec_query_all() will free din and dout and allocate them again to fit the usage by
- * calling devm_kfree() and devm_kzalloc(). Set them to NULL as they aren't managed by
- * ec_dev->dev.
+ * calling krealloc(). Set them to NULL as they aren't allocated by kalloc().
*/
ec_dev->din = NULL;
ec_dev->dout = NULL;
--
2.36.1.255.ge46751e96f-goog
^ permalink raw reply related [flat|nested] 28+ messages in thread* Re: [PATCH v2 12/15] platform/chrome: use krealloc() for `din` and `dout`
2022-06-07 14:56 ` [PATCH v2 12/15] platform/chrome: use krealloc() for `din` and `dout` Tzung-Bi Shih
@ 2022-06-07 19:06 ` Guenter Roeck
2022-06-08 2:17 ` Tzung-Bi Shih
0 siblings, 1 reply; 28+ messages in thread
From: Guenter Roeck @ 2022-06-07 19:06 UTC (permalink / raw)
To: Tzung-Bi Shih; +Cc: bleung, groeck, chrome-platform, linux-kernel
On Tue, Jun 7, 2022 at 7:57 AM Tzung-Bi Shih <tzungbi@kernel.org> wrote:
>
> Use krealloc() to re-allocate `din` and `dout`. Don't use devm variant
> because the two buffers could be re-allocated multiple times during
> runtime. Their life cycles aren't quite aligned to the device's.
While this saves a few lines of code, it is runtime-expensive:
krealloc() copies the old data, which is a waste of time/resources.
Maybe it would be better to just use kfree() followed by kzalloc().
>
> Free the memory in cros_ec_unregister() if any.
>
> No need to free memory if krealloc() fails. They will be freed
> eventually in either of the following:
> - Error handling path in cros_ec_register().
> - In cros_ec_unregister().
> - Next krealloc() in cros_ec_query_all().
>
> Signed-off-by: Tzung-Bi Shih <tzungbi@kernel.org>
> ---
> Changes from v1:
> - Don't use devm.
> - Free in cros_ec_unregister().
>
> drivers/platform/chrome/cros_ec.c | 4 +++
> drivers/platform/chrome/cros_ec_proto.c | 29 +++++++-------------
> drivers/platform/chrome/cros_ec_proto_test.c | 3 +-
> 3 files changed, 15 insertions(+), 21 deletions(-)
>
> diff --git a/drivers/platform/chrome/cros_ec.c b/drivers/platform/chrome/cros_ec.c
> index 29d3b544dafb..fb8cb8a73295 100644
> --- a/drivers/platform/chrome/cros_ec.c
> +++ b/drivers/platform/chrome/cros_ec.c
> @@ -285,6 +285,8 @@ int cros_ec_register(struct cros_ec_device *ec_dev)
> exit:
> platform_device_unregister(ec_dev->ec);
> platform_device_unregister(ec_dev->pd);
> + kfree(ec_dev->din);
> + kfree(ec_dev->dout);
> return err;
> }
> EXPORT_SYMBOL(cros_ec_register);
> @@ -302,6 +304,8 @@ void cros_ec_unregister(struct cros_ec_device *ec_dev)
> if (ec_dev->pd)
> platform_device_unregister(ec_dev->pd);
> platform_device_unregister(ec_dev->ec);
> + kfree(ec_dev->din);
> + kfree(ec_dev->dout);
> }
> EXPORT_SYMBOL(cros_ec_unregister);
>
> diff --git a/drivers/platform/chrome/cros_ec_proto.c b/drivers/platform/chrome/cros_ec_proto.c
> index 473654f50bca..de6bc457e496 100644
> --- a/drivers/platform/chrome/cros_ec_proto.c
> +++ b/drivers/platform/chrome/cros_ec_proto.c
> @@ -469,9 +469,9 @@ static int cros_ec_get_host_command_version_mask(struct cros_ec_device *ec_dev,
> */
> int cros_ec_query_all(struct cros_ec_device *ec_dev)
> {
> - struct device *dev = ec_dev->dev;
> u32 ver_mask = 0;
> int ret;
> + u8 *din, *dout;
>
> /* First try sending with proto v3. */
> if (!cros_ec_get_proto_info(ec_dev, CROS_EC_DEV_EC_INDEX)) {
> @@ -492,21 +492,15 @@ int cros_ec_query_all(struct cros_ec_device *ec_dev)
> }
> }
>
> - devm_kfree(dev, ec_dev->din);
> - devm_kfree(dev, ec_dev->dout);
> -
> - ec_dev->din = devm_kzalloc(dev, ec_dev->din_size, GFP_KERNEL);
> - if (!ec_dev->din) {
> - ret = -ENOMEM;
> - goto exit;
> - }
> + din = krealloc(ec_dev->din, ec_dev->din_size, GFP_KERNEL);
> + if (!din)
> + return -ENOMEM;
I would suggest assigning the values directly; the new variables don't
really add value.
Thanks,
Guenter
> + ec_dev->din = din;
>
> - ec_dev->dout = devm_kzalloc(dev, ec_dev->dout_size, GFP_KERNEL);
> - if (!ec_dev->dout) {
> - devm_kfree(dev, ec_dev->din);
> - ret = -ENOMEM;
> - goto exit;
> - }
> + dout = krealloc(ec_dev->dout, ec_dev->dout_size, GFP_KERNEL);
> + if (!dout)
> + return -ENOMEM;
> + ec_dev->dout = dout;
>
> /* Probe if MKBP event is supported */
> ret = cros_ec_get_host_command_version_mask(ec_dev,
> @@ -555,10 +549,7 @@ int cros_ec_query_all(struct cros_ec_device *ec_dev)
> "failed to retrieve wake mask: %d\n", ret);
> }
>
> - ret = 0;
> -
> -exit:
> - return ret;
> + return 0;
> }
> EXPORT_SYMBOL(cros_ec_query_all);
>
> diff --git a/drivers/platform/chrome/cros_ec_proto_test.c b/drivers/platform/chrome/cros_ec_proto_test.c
> index 730248be42a7..27b81a5a9880 100644
> --- a/drivers/platform/chrome/cros_ec_proto_test.c
> +++ b/drivers/platform/chrome/cros_ec_proto_test.c
> @@ -180,8 +180,7 @@ static void cros_ec_proto_test_query_all_pretest(struct kunit *test)
>
> /*
> * cros_ec_query_all() will free din and dout and allocate them again to fit the usage by
> - * calling devm_kfree() and devm_kzalloc(). Set them to NULL as they aren't managed by
> - * ec_dev->dev.
> + * calling krealloc(). Set them to NULL as they aren't allocated by kalloc().
> */
> ec_dev->din = NULL;
> ec_dev->dout = NULL;
> --
> 2.36.1.255.ge46751e96f-goog
>
^ permalink raw reply [flat|nested] 28+ messages in thread* Re: [PATCH v2 12/15] platform/chrome: use krealloc() for `din` and `dout`
2022-06-07 19:06 ` Guenter Roeck
@ 2022-06-08 2:17 ` Tzung-Bi Shih
0 siblings, 0 replies; 28+ messages in thread
From: Tzung-Bi Shih @ 2022-06-08 2:17 UTC (permalink / raw)
To: Guenter Roeck; +Cc: bleung, groeck, chrome-platform, linux-kernel
On Tue, Jun 07, 2022 at 12:06:23PM -0700, Guenter Roeck wrote:
> On Tue, Jun 7, 2022 at 7:57 AM Tzung-Bi Shih <tzungbi@kernel.org> wrote:
> >
> > Use krealloc() to re-allocate `din` and `dout`. Don't use devm variant
> > because the two buffers could be re-allocated multiple times during
> > runtime. Their life cycles aren't quite aligned to the device's.
>
> While this saves a few lines of code, it is runtime-expensive:
> krealloc() copies the old data, which is a waste of time/resources.
> Maybe it would be better to just use kfree() followed by kzalloc().
Ack. I wasn't aware of the memcpy(). Let's don't use it.
> > @@ -469,9 +469,9 @@ static int cros_ec_get_host_command_version_mask(struct cros_ec_device *ec_dev,
> > */
> > int cros_ec_query_all(struct cros_ec_device *ec_dev)
> > {
> > - struct device *dev = ec_dev->dev;
> > u32 ver_mask = 0;
> > int ret;
> > + u8 *din, *dout;
> >
> > /* First try sending with proto v3. */
> > if (!cros_ec_get_proto_info(ec_dev, CROS_EC_DEV_EC_INDEX)) {
> > @@ -492,21 +492,15 @@ int cros_ec_query_all(struct cros_ec_device *ec_dev)
> > }
> > }
> >
> > - devm_kfree(dev, ec_dev->din);
> > - devm_kfree(dev, ec_dev->dout);
> > -
> > - ec_dev->din = devm_kzalloc(dev, ec_dev->din_size, GFP_KERNEL);
> > - if (!ec_dev->din) {
> > - ret = -ENOMEM;
> > - goto exit;
> > - }
> > + din = krealloc(ec_dev->din, ec_dev->din_size, GFP_KERNEL);
> > + if (!din)
> > + return -ENOMEM;
>
> I would suggest assigning the values directly; the new variables don't
> really add value.
checkpatch.pl generated a warning for it. See [1].
Will remove the krealloc()s anyway.
[1]: https://github.com/torvalds/linux/commit/972fdea2e6ece7578915d386a5447bc78d3fb8b8
^ permalink raw reply [flat|nested] 28+ messages in thread
* [PATCH v2 13/15] platform/chrome: cros_ec_proto: don't show MKBP version if unsupported
2022-06-07 14:56 [PATCH v2 00/15] platform/chrome: Kunit tests and refactor for cros_ec_query_all() Tzung-Bi Shih
` (11 preceding siblings ...)
2022-06-07 14:56 ` [PATCH v2 12/15] platform/chrome: use krealloc() for `din` and `dout` Tzung-Bi Shih
@ 2022-06-07 14:56 ` Tzung-Bi Shih
2022-06-07 19:08 ` Guenter Roeck
2022-06-07 14:56 ` [PATCH v2 14/15] platform/chrome: cros_ec_proto: return 0 on getting version mask success Tzung-Bi Shih
2022-06-07 14:56 ` [PATCH v2 15/15] platform/chrome: cros_ec_proto: return 0 on getting wake " Tzung-Bi Shih
14 siblings, 1 reply; 28+ messages in thread
From: Tzung-Bi Shih @ 2022-06-07 14:56 UTC (permalink / raw)
To: bleung, groeck; +Cc: chrome-platform, linux-kernel, tzungbi
It wrongly showed the following message when it doesn't support MKBP:
"MKBP support version 4294967295".
Fix it.
Signed-off-by: Tzung-Bi Shih <tzungbi@kernel.org>
---
No v1. New in the series.
drivers/platform/chrome/cros_ec_proto.c | 8 ++++----
1 file changed, 4 insertions(+), 4 deletions(-)
diff --git a/drivers/platform/chrome/cros_ec_proto.c b/drivers/platform/chrome/cros_ec_proto.c
index de6bc457e496..ee15a73eee38 100644
--- a/drivers/platform/chrome/cros_ec_proto.c
+++ b/drivers/platform/chrome/cros_ec_proto.c
@@ -506,13 +506,13 @@ int cros_ec_query_all(struct cros_ec_device *ec_dev)
ret = cros_ec_get_host_command_version_mask(ec_dev,
EC_CMD_GET_NEXT_EVENT,
&ver_mask);
- if (ret < 0 || ver_mask == 0)
+ if (ret < 0 || ver_mask == 0) {
ec_dev->mkbp_event_supported = 0;
- else
+ } else {
ec_dev->mkbp_event_supported = fls(ver_mask);
- dev_dbg(ec_dev->dev, "MKBP support version %u\n",
- ec_dev->mkbp_event_supported - 1);
+ dev_dbg(ec_dev->dev, "MKBP support version %u\n", ec_dev->mkbp_event_supported - 1);
+ }
/* Probe if host sleep v1 is supported for S0ix failure detection. */
ret = cros_ec_get_host_command_version_mask(ec_dev,
--
2.36.1.255.ge46751e96f-goog
^ permalink raw reply related [flat|nested] 28+ messages in thread* Re: [PATCH v2 13/15] platform/chrome: cros_ec_proto: don't show MKBP version if unsupported
2022-06-07 14:56 ` [PATCH v2 13/15] platform/chrome: cros_ec_proto: don't show MKBP version if unsupported Tzung-Bi Shih
@ 2022-06-07 19:08 ` Guenter Roeck
0 siblings, 0 replies; 28+ messages in thread
From: Guenter Roeck @ 2022-06-07 19:08 UTC (permalink / raw)
To: Tzung-Bi Shih; +Cc: bleung, groeck, chrome-platform, linux-kernel
On Tue, Jun 7, 2022 at 7:57 AM Tzung-Bi Shih <tzungbi@kernel.org> wrote:
>
> It wrongly showed the following message when it doesn't support MKBP:
> "MKBP support version 4294967295".
>
> Fix it.
>
> Signed-off-by: Tzung-Bi Shih <tzungbi@kernel.org>
Reviewed-by: Guenter Roeck <groeck@chromium.org>
> ---
> No v1. New in the series.
>
> drivers/platform/chrome/cros_ec_proto.c | 8 ++++----
> 1 file changed, 4 insertions(+), 4 deletions(-)
>
> diff --git a/drivers/platform/chrome/cros_ec_proto.c b/drivers/platform/chrome/cros_ec_proto.c
> index de6bc457e496..ee15a73eee38 100644
> --- a/drivers/platform/chrome/cros_ec_proto.c
> +++ b/drivers/platform/chrome/cros_ec_proto.c
> @@ -506,13 +506,13 @@ int cros_ec_query_all(struct cros_ec_device *ec_dev)
> ret = cros_ec_get_host_command_version_mask(ec_dev,
> EC_CMD_GET_NEXT_EVENT,
> &ver_mask);
> - if (ret < 0 || ver_mask == 0)
> + if (ret < 0 || ver_mask == 0) {
> ec_dev->mkbp_event_supported = 0;
> - else
> + } else {
> ec_dev->mkbp_event_supported = fls(ver_mask);
>
> - dev_dbg(ec_dev->dev, "MKBP support version %u\n",
> - ec_dev->mkbp_event_supported - 1);
> + dev_dbg(ec_dev->dev, "MKBP support version %u\n", ec_dev->mkbp_event_supported - 1);
> + }
>
> /* Probe if host sleep v1 is supported for S0ix failure detection. */
> ret = cros_ec_get_host_command_version_mask(ec_dev,
> --
> 2.36.1.255.ge46751e96f-goog
>
^ permalink raw reply [flat|nested] 28+ messages in thread
* [PATCH v2 14/15] platform/chrome: cros_ec_proto: return 0 on getting version mask success
2022-06-07 14:56 [PATCH v2 00/15] platform/chrome: Kunit tests and refactor for cros_ec_query_all() Tzung-Bi Shih
` (12 preceding siblings ...)
2022-06-07 14:56 ` [PATCH v2 13/15] platform/chrome: cros_ec_proto: don't show MKBP version if unsupported Tzung-Bi Shih
@ 2022-06-07 14:56 ` Tzung-Bi Shih
2022-06-07 19:11 ` Guenter Roeck
2022-06-07 14:56 ` [PATCH v2 15/15] platform/chrome: cros_ec_proto: return 0 on getting wake " Tzung-Bi Shih
14 siblings, 1 reply; 28+ messages in thread
From: Tzung-Bi Shih @ 2022-06-07 14:56 UTC (permalink / raw)
To: bleung, groeck; +Cc: chrome-platform, linux-kernel, tzungbi
cros_ec_get_host_command_version_mask() used to return value from
send_command() which is number of bytes for input payload on success
(i.e. sizeof(struct ec_response_get_cmd_versions)).
However, the callers don't need to know how many bytes are available.
- Fix cros_ec_get_host_command_version_mask() to return 0 on success;
negative integers on error.
- Remove the unneeded `ver_mask` initialization as the callers should
take it only if cros_ec_get_host_command_version_mask() returns 0.
- Add a Kunit test: `ver_mask` has some garbage bytes from previous
EC_CMD_GET_NEXT_EVENT but there is no host sleep to make sure the
caller checks the return values correctly.
Signed-off-by: Tzung-Bi Shih <tzungbi@kernel.org>
---
Changes from v1:
- Return 0 on success; otherwise, negative intergers.
drivers/platform/chrome/cros_ec_proto.c | 37 ++-
drivers/platform/chrome/cros_ec_proto_test.c | 286 +++++++++++++++++++
2 files changed, 308 insertions(+), 15 deletions(-)
diff --git a/drivers/platform/chrome/cros_ec_proto.c b/drivers/platform/chrome/cros_ec_proto.c
index ee15a73eee38..9d96ed16244f 100644
--- a/drivers/platform/chrome/cros_ec_proto.c
+++ b/drivers/platform/chrome/cros_ec_proto.c
@@ -428,13 +428,12 @@ static int cros_ec_get_proto_info_legacy(struct cros_ec_device *ec_dev)
* the caller has ec_dev->lock mutex or the caller knows there is
* no other command in progress.
*/
-static int cros_ec_get_host_command_version_mask(struct cros_ec_device *ec_dev,
- u16 cmd, u32 *mask)
+static int cros_ec_get_host_command_version_mask(struct cros_ec_device *ec_dev, u16 cmd, u32 *mask)
{
struct ec_params_get_cmd_versions *pver;
struct ec_response_get_cmd_versions *rver;
struct cros_ec_command *msg;
- int ret;
+ int ret, mapped;
msg = kmalloc(sizeof(*msg) + max(sizeof(*rver), sizeof(*pver)),
GFP_KERNEL);
@@ -450,13 +449,25 @@ static int cros_ec_get_host_command_version_mask(struct cros_ec_device *ec_dev,
pver->cmd = cmd;
ret = send_command(ec_dev, msg);
- if (ret > 0) {
- rver = (struct ec_response_get_cmd_versions *)msg->data;
- *mask = rver->version_mask;
+ if (ret < 0)
+ goto exit;
+
+ mapped = cros_ec_map_error(msg->result);
+ if (mapped) {
+ ret = mapped;
+ goto exit;
}
- kfree(msg);
+ if (ret == 0) {
+ ret = -EPROTO;
+ goto exit;
+ }
+ rver = (struct ec_response_get_cmd_versions *)msg->data;
+ *mask = rver->version_mask;
+ ret = 0;
+exit:
+ kfree(msg);
return ret;
}
@@ -469,7 +480,7 @@ static int cros_ec_get_host_command_version_mask(struct cros_ec_device *ec_dev,
*/
int cros_ec_query_all(struct cros_ec_device *ec_dev)
{
- u32 ver_mask = 0;
+ u32 ver_mask;
int ret;
u8 *din, *dout;
@@ -503,9 +514,7 @@ int cros_ec_query_all(struct cros_ec_device *ec_dev)
ec_dev->dout = dout;
/* Probe if MKBP event is supported */
- ret = cros_ec_get_host_command_version_mask(ec_dev,
- EC_CMD_GET_NEXT_EVENT,
- &ver_mask);
+ ret = cros_ec_get_host_command_version_mask(ec_dev, EC_CMD_GET_NEXT_EVENT, &ver_mask);
if (ret < 0 || ver_mask == 0) {
ec_dev->mkbp_event_supported = 0;
} else {
@@ -515,10 +524,8 @@ int cros_ec_query_all(struct cros_ec_device *ec_dev)
}
/* Probe if host sleep v1 is supported for S0ix failure detection. */
- ret = cros_ec_get_host_command_version_mask(ec_dev,
- EC_CMD_HOST_SLEEP_EVENT,
- &ver_mask);
- ec_dev->host_sleep_v1 = (ret >= 0 && (ver_mask & EC_VER_MASK(1)));
+ ret = cros_ec_get_host_command_version_mask(ec_dev, EC_CMD_HOST_SLEEP_EVENT, &ver_mask);
+ ec_dev->host_sleep_v1 = (ret == 0 && (ver_mask & EC_VER_MASK(1)));
/* Get host event wake mask. */
ret = cros_ec_get_host_event_wake_mask(ec_dev, &ec_dev->host_event_wake_mask);
diff --git a/drivers/platform/chrome/cros_ec_proto_test.c b/drivers/platform/chrome/cros_ec_proto_test.c
index 27b81a5a9880..af69410f2978 100644
--- a/drivers/platform/chrome/cros_ec_proto_test.c
+++ b/drivers/platform/chrome/cros_ec_proto_test.c
@@ -890,6 +890,182 @@ static void cros_ec_proto_test_query_all_no_mkbp(struct kunit *test)
}
}
+static void cros_ec_proto_test_query_all_no_mkbp_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;
+
+ /* Set some garbage bytes. */
+ ec_dev->mkbp_event_supported = 0xbf;
+
+ /* For cros_ec_get_proto_info() without passthru. */
+ {
+ struct ec_response_get_protocol_info *data;
+
+ mock = cros_kunit_ec_xfer_mock_add(test, sizeof(*data));
+ KUNIT_ASSERT_PTR_NE(test, mock, NULL);
+
+ /*
+ * Although it doesn't check the value, provides valid sizes so that
+ * cros_ec_query_all() allocates din and dout correctly.
+ */
+ data = (struct ec_response_get_protocol_info *)mock->o_data;
+ data->max_request_packet_size = 0xbe;
+ data->max_response_packet_size = 0xef;
+ }
+
+ /* For cros_ec_get_proto_info() with passthru. */
+ {
+ mock = cros_kunit_ec_xfer_mock_addx(test, 0, EC_RES_INVALID_COMMAND, 0);
+ KUNIT_ASSERT_PTR_NE(test, mock, NULL);
+ }
+
+ /* For cros_ec_get_host_command_version_mask() for MKBP. */
+ {
+ mock = cros_kunit_ec_xfer_mock_addx(test, 0, EC_RES_INVALID_COMMAND, 0);
+ KUNIT_ASSERT_PTR_NE(test, mock, NULL);
+ }
+
+ cros_ec_proto_test_query_all_pretest(test);
+ ret = cros_ec_query_all(ec_dev);
+ KUNIT_EXPECT_EQ(test, ret, 0);
+
+ /* For cros_ec_get_proto_info() without passthru. */
+ {
+ 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_PROTOCOL_INFO);
+ KUNIT_EXPECT_EQ(test, mock->msg.insize,
+ sizeof(struct ec_response_get_protocol_info));
+ KUNIT_EXPECT_EQ(test, mock->msg.outsize, 0);
+ }
+
+ /* For cros_ec_get_proto_info() with passthru. */
+ {
+ 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_PASSTHRU_OFFSET(CROS_EC_DEV_PD_INDEX) |
+ EC_CMD_GET_PROTOCOL_INFO);
+ KUNIT_EXPECT_EQ(test, mock->msg.insize,
+ sizeof(struct ec_response_get_protocol_info));
+ KUNIT_EXPECT_EQ(test, mock->msg.outsize, 0);
+ }
+
+ /* For cros_ec_get_host_command_version_mask() for MKBP. */
+ {
+ struct ec_params_get_cmd_versions *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_GET_CMD_VERSIONS);
+ KUNIT_EXPECT_EQ(test, mock->msg.insize,
+ sizeof(struct ec_response_get_cmd_versions));
+ KUNIT_EXPECT_EQ(test, mock->msg.outsize, sizeof(*data));
+
+ data = (struct ec_params_get_cmd_versions *)mock->i_data;
+ KUNIT_EXPECT_EQ(test, data->cmd, EC_CMD_GET_NEXT_EVENT);
+
+ KUNIT_EXPECT_EQ(test, ec_dev->mkbp_event_supported, 0);
+ }
+}
+
+static void cros_ec_proto_test_query_all_no_mkbp_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;
+
+ /* Set some garbage bytes. */
+ ec_dev->mkbp_event_supported = 0xbf;
+
+ /* For cros_ec_get_proto_info() without passthru. */
+ {
+ struct ec_response_get_protocol_info *data;
+
+ mock = cros_kunit_ec_xfer_mock_add(test, sizeof(*data));
+ KUNIT_ASSERT_PTR_NE(test, mock, NULL);
+
+ /*
+ * Although it doesn't check the value, provides valid sizes so that
+ * cros_ec_query_all() allocates din and dout correctly.
+ */
+ data = (struct ec_response_get_protocol_info *)mock->o_data;
+ data->max_request_packet_size = 0xbe;
+ data->max_response_packet_size = 0xef;
+ }
+
+ /* For cros_ec_get_proto_info() with passthru. */
+ {
+ mock = cros_kunit_ec_xfer_mock_addx(test, 0, EC_RES_INVALID_COMMAND, 0);
+ KUNIT_ASSERT_PTR_NE(test, mock, NULL);
+ }
+
+ /* For cros_ec_get_host_command_version_mask() for MKBP. */
+ {
+ mock = cros_kunit_ec_xfer_mock_add(test, 0);
+ KUNIT_ASSERT_PTR_NE(test, mock, NULL);
+ }
+
+ cros_ec_proto_test_query_all_pretest(test);
+ ret = cros_ec_query_all(ec_dev);
+ KUNIT_EXPECT_EQ(test, ret, 0);
+
+ /* For cros_ec_get_proto_info() without passthru. */
+ {
+ 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_PROTOCOL_INFO);
+ KUNIT_EXPECT_EQ(test, mock->msg.insize,
+ sizeof(struct ec_response_get_protocol_info));
+ KUNIT_EXPECT_EQ(test, mock->msg.outsize, 0);
+ }
+
+ /* For cros_ec_get_proto_info() with passthru. */
+ {
+ 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_PASSTHRU_OFFSET(CROS_EC_DEV_PD_INDEX) |
+ EC_CMD_GET_PROTOCOL_INFO);
+ KUNIT_EXPECT_EQ(test, mock->msg.insize,
+ sizeof(struct ec_response_get_protocol_info));
+ KUNIT_EXPECT_EQ(test, mock->msg.outsize, 0);
+ }
+
+ /* For cros_ec_get_host_command_version_mask() for MKBP. */
+ {
+ struct ec_params_get_cmd_versions *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_GET_CMD_VERSIONS);
+ KUNIT_EXPECT_EQ(test, mock->msg.insize,
+ sizeof(struct ec_response_get_cmd_versions));
+ KUNIT_EXPECT_EQ(test, mock->msg.outsize, sizeof(*data));
+
+ data = (struct ec_params_get_cmd_versions *)mock->i_data;
+ KUNIT_EXPECT_EQ(test, data->cmd, EC_CMD_GET_NEXT_EVENT);
+
+ KUNIT_EXPECT_EQ(test, ec_dev->mkbp_event_supported, 0);
+ }
+}
+
static void cros_ec_proto_test_query_all_no_host_sleep(struct kunit *test)
{
struct cros_ec_proto_test_priv *priv = test->priv;
@@ -996,6 +1172,113 @@ static void cros_ec_proto_test_query_all_no_host_sleep(struct kunit *test)
}
}
+static void cros_ec_proto_test_query_all_no_host_sleep_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;
+
+ /* Set some garbage bytes. */
+ ec_dev->host_sleep_v1 = true;
+
+ /* For cros_ec_get_proto_info() without passthru. */
+ {
+ struct ec_response_get_protocol_info *data;
+
+ mock = cros_kunit_ec_xfer_mock_add(test, sizeof(*data));
+ KUNIT_ASSERT_PTR_NE(test, mock, NULL);
+
+ /*
+ * Although it doesn't check the value, provides valid sizes so that
+ * cros_ec_query_all() allocates din and dout correctly.
+ */
+ data = (struct ec_response_get_protocol_info *)mock->o_data;
+ data->max_request_packet_size = 0xbe;
+ data->max_response_packet_size = 0xef;
+ }
+
+ /* For cros_ec_get_proto_info() with passthru. */
+ {
+ mock = cros_kunit_ec_xfer_mock_addx(test, 0, EC_RES_INVALID_COMMAND, 0);
+ KUNIT_ASSERT_PTR_NE(test, mock, NULL);
+ }
+
+ /* For cros_ec_get_host_command_version_mask() for MKBP. */
+ {
+ struct ec_response_get_cmd_versions *data;
+
+ mock = cros_kunit_ec_xfer_mock_add(test, sizeof(*data));
+ KUNIT_ASSERT_PTR_NE(test, mock, NULL);
+
+ /* In order to pollute next cros_ec_get_host_command_version_mask(). */
+ data = (struct ec_response_get_cmd_versions *)mock->o_data;
+ data->version_mask = 0xbeef;
+ }
+
+ /* For cros_ec_get_host_command_version_mask() for host sleep v1. */
+ {
+ mock = cros_kunit_ec_xfer_mock_add(test, 0);
+ KUNIT_ASSERT_PTR_NE(test, mock, NULL);
+ }
+
+ cros_ec_proto_test_query_all_pretest(test);
+ ret = cros_ec_query_all(ec_dev);
+ KUNIT_EXPECT_EQ(test, ret, 0);
+
+ /* For cros_ec_get_proto_info() without passthru. */
+ {
+ 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_PROTOCOL_INFO);
+ KUNIT_EXPECT_EQ(test, mock->msg.insize,
+ sizeof(struct ec_response_get_protocol_info));
+ KUNIT_EXPECT_EQ(test, mock->msg.outsize, 0);
+ }
+
+ /* For cros_ec_get_proto_info() with passthru. */
+ {
+ 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_PASSTHRU_OFFSET(CROS_EC_DEV_PD_INDEX) |
+ EC_CMD_GET_PROTOCOL_INFO);
+ KUNIT_EXPECT_EQ(test, mock->msg.insize,
+ sizeof(struct ec_response_get_protocol_info));
+ KUNIT_EXPECT_EQ(test, mock->msg.outsize, 0);
+ }
+
+ /* For cros_ec_get_host_command_version_mask() for MKBP. */
+ {
+ 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_CMD_VERSIONS);
+ KUNIT_EXPECT_EQ(test, mock->msg.insize,
+ sizeof(struct ec_response_get_cmd_versions));
+ KUNIT_EXPECT_EQ(test, mock->msg.outsize, sizeof(struct ec_params_get_cmd_versions));
+ }
+
+ /* For cros_ec_get_host_command_version_mask() for host sleep v1. */
+ {
+ 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_CMD_VERSIONS);
+ KUNIT_EXPECT_EQ(test, mock->msg.insize,
+ sizeof(struct ec_response_get_cmd_versions));
+ KUNIT_EXPECT_EQ(test, mock->msg.outsize, sizeof(struct ec_params_get_cmd_versions));
+
+ KUNIT_EXPECT_FALSE(test, ec_dev->host_sleep_v1);
+ }
+}
+
static void cros_ec_proto_test_query_all_default_wake_mask_return_error(struct kunit *test)
{
struct cros_ec_proto_test_priv *priv = test->priv;
@@ -1183,7 +1466,10 @@ static struct kunit_case cros_ec_proto_test_cases[] = {
KUNIT_CASE(cros_ec_proto_test_query_all_legacy_data_error),
KUNIT_CASE(cros_ec_proto_test_query_all_legacy_return0),
KUNIT_CASE(cros_ec_proto_test_query_all_no_mkbp),
+ KUNIT_CASE(cros_ec_proto_test_query_all_no_mkbp_return_error),
+ KUNIT_CASE(cros_ec_proto_test_query_all_no_mkbp_return0),
KUNIT_CASE(cros_ec_proto_test_query_all_no_host_sleep),
+ 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),
{}
};
--
2.36.1.255.ge46751e96f-goog
^ permalink raw reply related [flat|nested] 28+ messages in thread* Re: [PATCH v2 14/15] platform/chrome: cros_ec_proto: return 0 on getting version mask success
2022-06-07 14:56 ` [PATCH v2 14/15] platform/chrome: cros_ec_proto: return 0 on getting version mask success Tzung-Bi Shih
@ 2022-06-07 19:11 ` Guenter Roeck
2022-06-08 2:17 ` Tzung-Bi Shih
0 siblings, 1 reply; 28+ messages in thread
From: Guenter Roeck @ 2022-06-07 19:11 UTC (permalink / raw)
To: Tzung-Bi Shih; +Cc: bleung, groeck, chrome-platform, linux-kernel
On Tue, Jun 7, 2022 at 7:57 AM Tzung-Bi Shih <tzungbi@kernel.org> wrote:
>
> cros_ec_get_host_command_version_mask() used to return value from
> send_command() which is number of bytes for input payload on success
> (i.e. sizeof(struct ec_response_get_cmd_versions)).
>
> However, the callers don't need to know how many bytes are available.
>
> - Fix cros_ec_get_host_command_version_mask() to return 0 on success;
> negative integers on error.
>
> - Remove the unneeded `ver_mask` initialization as the callers should
> take it only if cros_ec_get_host_command_version_mask() returns 0.
>
> - Add a Kunit test: `ver_mask` has some garbage bytes from previous
> EC_CMD_GET_NEXT_EVENT but there is no host sleep to make sure the
> caller checks the return values correctly.
>
This should be separate patches.
Thanks,
Guenter
> Signed-off-by: Tzung-Bi Shih <tzungbi@kernel.org>
> ---
> Changes from v1:
> - Return 0 on success; otherwise, negative intergers.
>
> drivers/platform/chrome/cros_ec_proto.c | 37 ++-
> drivers/platform/chrome/cros_ec_proto_test.c | 286 +++++++++++++++++++
> 2 files changed, 308 insertions(+), 15 deletions(-)
>
> diff --git a/drivers/platform/chrome/cros_ec_proto.c b/drivers/platform/chrome/cros_ec_proto.c
> index ee15a73eee38..9d96ed16244f 100644
> --- a/drivers/platform/chrome/cros_ec_proto.c
> +++ b/drivers/platform/chrome/cros_ec_proto.c
> @@ -428,13 +428,12 @@ static int cros_ec_get_proto_info_legacy(struct cros_ec_device *ec_dev)
> * the caller has ec_dev->lock mutex or the caller knows there is
> * no other command in progress.
> */
> -static int cros_ec_get_host_command_version_mask(struct cros_ec_device *ec_dev,
> - u16 cmd, u32 *mask)
> +static int cros_ec_get_host_command_version_mask(struct cros_ec_device *ec_dev, u16 cmd, u32 *mask)
> {
> struct ec_params_get_cmd_versions *pver;
> struct ec_response_get_cmd_versions *rver;
> struct cros_ec_command *msg;
> - int ret;
> + int ret, mapped;
>
> msg = kmalloc(sizeof(*msg) + max(sizeof(*rver), sizeof(*pver)),
> GFP_KERNEL);
> @@ -450,13 +449,25 @@ static int cros_ec_get_host_command_version_mask(struct cros_ec_device *ec_dev,
> pver->cmd = cmd;
>
> ret = send_command(ec_dev, msg);
> - if (ret > 0) {
> - rver = (struct ec_response_get_cmd_versions *)msg->data;
> - *mask = rver->version_mask;
> + if (ret < 0)
> + goto exit;
> +
> + mapped = cros_ec_map_error(msg->result);
> + if (mapped) {
> + ret = mapped;
> + goto exit;
> }
>
> - kfree(msg);
> + if (ret == 0) {
> + ret = -EPROTO;
> + goto exit;
> + }
>
> + rver = (struct ec_response_get_cmd_versions *)msg->data;
> + *mask = rver->version_mask;
> + ret = 0;
> +exit:
> + kfree(msg);
> return ret;
> }
>
> @@ -469,7 +480,7 @@ static int cros_ec_get_host_command_version_mask(struct cros_ec_device *ec_dev,
> */
> int cros_ec_query_all(struct cros_ec_device *ec_dev)
> {
> - u32 ver_mask = 0;
> + u32 ver_mask;
> int ret;
> u8 *din, *dout;
>
> @@ -503,9 +514,7 @@ int cros_ec_query_all(struct cros_ec_device *ec_dev)
> ec_dev->dout = dout;
>
> /* Probe if MKBP event is supported */
> - ret = cros_ec_get_host_command_version_mask(ec_dev,
> - EC_CMD_GET_NEXT_EVENT,
> - &ver_mask);
> + ret = cros_ec_get_host_command_version_mask(ec_dev, EC_CMD_GET_NEXT_EVENT, &ver_mask);
> if (ret < 0 || ver_mask == 0) {
> ec_dev->mkbp_event_supported = 0;
> } else {
> @@ -515,10 +524,8 @@ int cros_ec_query_all(struct cros_ec_device *ec_dev)
> }
>
> /* Probe if host sleep v1 is supported for S0ix failure detection. */
> - ret = cros_ec_get_host_command_version_mask(ec_dev,
> - EC_CMD_HOST_SLEEP_EVENT,
> - &ver_mask);
> - ec_dev->host_sleep_v1 = (ret >= 0 && (ver_mask & EC_VER_MASK(1)));
> + ret = cros_ec_get_host_command_version_mask(ec_dev, EC_CMD_HOST_SLEEP_EVENT, &ver_mask);
> + ec_dev->host_sleep_v1 = (ret == 0 && (ver_mask & EC_VER_MASK(1)));
>
> /* Get host event wake mask. */
> ret = cros_ec_get_host_event_wake_mask(ec_dev, &ec_dev->host_event_wake_mask);
> diff --git a/drivers/platform/chrome/cros_ec_proto_test.c b/drivers/platform/chrome/cros_ec_proto_test.c
> index 27b81a5a9880..af69410f2978 100644
> --- a/drivers/platform/chrome/cros_ec_proto_test.c
> +++ b/drivers/platform/chrome/cros_ec_proto_test.c
> @@ -890,6 +890,182 @@ static void cros_ec_proto_test_query_all_no_mkbp(struct kunit *test)
> }
> }
>
> +static void cros_ec_proto_test_query_all_no_mkbp_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;
> +
> + /* Set some garbage bytes. */
> + ec_dev->mkbp_event_supported = 0xbf;
> +
> + /* For cros_ec_get_proto_info() without passthru. */
> + {
> + struct ec_response_get_protocol_info *data;
> +
> + mock = cros_kunit_ec_xfer_mock_add(test, sizeof(*data));
> + KUNIT_ASSERT_PTR_NE(test, mock, NULL);
> +
> + /*
> + * Although it doesn't check the value, provides valid sizes so that
> + * cros_ec_query_all() allocates din and dout correctly.
> + */
> + data = (struct ec_response_get_protocol_info *)mock->o_data;
> + data->max_request_packet_size = 0xbe;
> + data->max_response_packet_size = 0xef;
> + }
> +
> + /* For cros_ec_get_proto_info() with passthru. */
> + {
> + mock = cros_kunit_ec_xfer_mock_addx(test, 0, EC_RES_INVALID_COMMAND, 0);
> + KUNIT_ASSERT_PTR_NE(test, mock, NULL);
> + }
> +
> + /* For cros_ec_get_host_command_version_mask() for MKBP. */
> + {
> + mock = cros_kunit_ec_xfer_mock_addx(test, 0, EC_RES_INVALID_COMMAND, 0);
> + KUNIT_ASSERT_PTR_NE(test, mock, NULL);
> + }
> +
> + cros_ec_proto_test_query_all_pretest(test);
> + ret = cros_ec_query_all(ec_dev);
> + KUNIT_EXPECT_EQ(test, ret, 0);
> +
> + /* For cros_ec_get_proto_info() without passthru. */
> + {
> + 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_PROTOCOL_INFO);
> + KUNIT_EXPECT_EQ(test, mock->msg.insize,
> + sizeof(struct ec_response_get_protocol_info));
> + KUNIT_EXPECT_EQ(test, mock->msg.outsize, 0);
> + }
> +
> + /* For cros_ec_get_proto_info() with passthru. */
> + {
> + 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_PASSTHRU_OFFSET(CROS_EC_DEV_PD_INDEX) |
> + EC_CMD_GET_PROTOCOL_INFO);
> + KUNIT_EXPECT_EQ(test, mock->msg.insize,
> + sizeof(struct ec_response_get_protocol_info));
> + KUNIT_EXPECT_EQ(test, mock->msg.outsize, 0);
> + }
> +
> + /* For cros_ec_get_host_command_version_mask() for MKBP. */
> + {
> + struct ec_params_get_cmd_versions *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_GET_CMD_VERSIONS);
> + KUNIT_EXPECT_EQ(test, mock->msg.insize,
> + sizeof(struct ec_response_get_cmd_versions));
> + KUNIT_EXPECT_EQ(test, mock->msg.outsize, sizeof(*data));
> +
> + data = (struct ec_params_get_cmd_versions *)mock->i_data;
> + KUNIT_EXPECT_EQ(test, data->cmd, EC_CMD_GET_NEXT_EVENT);
> +
> + KUNIT_EXPECT_EQ(test, ec_dev->mkbp_event_supported, 0);
> + }
> +}
> +
> +static void cros_ec_proto_test_query_all_no_mkbp_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;
> +
> + /* Set some garbage bytes. */
> + ec_dev->mkbp_event_supported = 0xbf;
> +
> + /* For cros_ec_get_proto_info() without passthru. */
> + {
> + struct ec_response_get_protocol_info *data;
> +
> + mock = cros_kunit_ec_xfer_mock_add(test, sizeof(*data));
> + KUNIT_ASSERT_PTR_NE(test, mock, NULL);
> +
> + /*
> + * Although it doesn't check the value, provides valid sizes so that
> + * cros_ec_query_all() allocates din and dout correctly.
> + */
> + data = (struct ec_response_get_protocol_info *)mock->o_data;
> + data->max_request_packet_size = 0xbe;
> + data->max_response_packet_size = 0xef;
> + }
> +
> + /* For cros_ec_get_proto_info() with passthru. */
> + {
> + mock = cros_kunit_ec_xfer_mock_addx(test, 0, EC_RES_INVALID_COMMAND, 0);
> + KUNIT_ASSERT_PTR_NE(test, mock, NULL);
> + }
> +
> + /* For cros_ec_get_host_command_version_mask() for MKBP. */
> + {
> + mock = cros_kunit_ec_xfer_mock_add(test, 0);
> + KUNIT_ASSERT_PTR_NE(test, mock, NULL);
> + }
> +
> + cros_ec_proto_test_query_all_pretest(test);
> + ret = cros_ec_query_all(ec_dev);
> + KUNIT_EXPECT_EQ(test, ret, 0);
> +
> + /* For cros_ec_get_proto_info() without passthru. */
> + {
> + 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_PROTOCOL_INFO);
> + KUNIT_EXPECT_EQ(test, mock->msg.insize,
> + sizeof(struct ec_response_get_protocol_info));
> + KUNIT_EXPECT_EQ(test, mock->msg.outsize, 0);
> + }
> +
> + /* For cros_ec_get_proto_info() with passthru. */
> + {
> + 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_PASSTHRU_OFFSET(CROS_EC_DEV_PD_INDEX) |
> + EC_CMD_GET_PROTOCOL_INFO);
> + KUNIT_EXPECT_EQ(test, mock->msg.insize,
> + sizeof(struct ec_response_get_protocol_info));
> + KUNIT_EXPECT_EQ(test, mock->msg.outsize, 0);
> + }
> +
> + /* For cros_ec_get_host_command_version_mask() for MKBP. */
> + {
> + struct ec_params_get_cmd_versions *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_GET_CMD_VERSIONS);
> + KUNIT_EXPECT_EQ(test, mock->msg.insize,
> + sizeof(struct ec_response_get_cmd_versions));
> + KUNIT_EXPECT_EQ(test, mock->msg.outsize, sizeof(*data));
> +
> + data = (struct ec_params_get_cmd_versions *)mock->i_data;
> + KUNIT_EXPECT_EQ(test, data->cmd, EC_CMD_GET_NEXT_EVENT);
> +
> + KUNIT_EXPECT_EQ(test, ec_dev->mkbp_event_supported, 0);
> + }
> +}
> +
> static void cros_ec_proto_test_query_all_no_host_sleep(struct kunit *test)
> {
> struct cros_ec_proto_test_priv *priv = test->priv;
> @@ -996,6 +1172,113 @@ static void cros_ec_proto_test_query_all_no_host_sleep(struct kunit *test)
> }
> }
>
> +static void cros_ec_proto_test_query_all_no_host_sleep_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;
> +
> + /* Set some garbage bytes. */
> + ec_dev->host_sleep_v1 = true;
> +
> + /* For cros_ec_get_proto_info() without passthru. */
> + {
> + struct ec_response_get_protocol_info *data;
> +
> + mock = cros_kunit_ec_xfer_mock_add(test, sizeof(*data));
> + KUNIT_ASSERT_PTR_NE(test, mock, NULL);
> +
> + /*
> + * Although it doesn't check the value, provides valid sizes so that
> + * cros_ec_query_all() allocates din and dout correctly.
> + */
> + data = (struct ec_response_get_protocol_info *)mock->o_data;
> + data->max_request_packet_size = 0xbe;
> + data->max_response_packet_size = 0xef;
> + }
> +
> + /* For cros_ec_get_proto_info() with passthru. */
> + {
> + mock = cros_kunit_ec_xfer_mock_addx(test, 0, EC_RES_INVALID_COMMAND, 0);
> + KUNIT_ASSERT_PTR_NE(test, mock, NULL);
> + }
> +
> + /* For cros_ec_get_host_command_version_mask() for MKBP. */
> + {
> + struct ec_response_get_cmd_versions *data;
> +
> + mock = cros_kunit_ec_xfer_mock_add(test, sizeof(*data));
> + KUNIT_ASSERT_PTR_NE(test, mock, NULL);
> +
> + /* In order to pollute next cros_ec_get_host_command_version_mask(). */
> + data = (struct ec_response_get_cmd_versions *)mock->o_data;
> + data->version_mask = 0xbeef;
> + }
> +
> + /* For cros_ec_get_host_command_version_mask() for host sleep v1. */
> + {
> + mock = cros_kunit_ec_xfer_mock_add(test, 0);
> + KUNIT_ASSERT_PTR_NE(test, mock, NULL);
> + }
> +
> + cros_ec_proto_test_query_all_pretest(test);
> + ret = cros_ec_query_all(ec_dev);
> + KUNIT_EXPECT_EQ(test, ret, 0);
> +
> + /* For cros_ec_get_proto_info() without passthru. */
> + {
> + 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_PROTOCOL_INFO);
> + KUNIT_EXPECT_EQ(test, mock->msg.insize,
> + sizeof(struct ec_response_get_protocol_info));
> + KUNIT_EXPECT_EQ(test, mock->msg.outsize, 0);
> + }
> +
> + /* For cros_ec_get_proto_info() with passthru. */
> + {
> + 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_PASSTHRU_OFFSET(CROS_EC_DEV_PD_INDEX) |
> + EC_CMD_GET_PROTOCOL_INFO);
> + KUNIT_EXPECT_EQ(test, mock->msg.insize,
> + sizeof(struct ec_response_get_protocol_info));
> + KUNIT_EXPECT_EQ(test, mock->msg.outsize, 0);
> + }
> +
> + /* For cros_ec_get_host_command_version_mask() for MKBP. */
> + {
> + 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_CMD_VERSIONS);
> + KUNIT_EXPECT_EQ(test, mock->msg.insize,
> + sizeof(struct ec_response_get_cmd_versions));
> + KUNIT_EXPECT_EQ(test, mock->msg.outsize, sizeof(struct ec_params_get_cmd_versions));
> + }
> +
> + /* For cros_ec_get_host_command_version_mask() for host sleep v1. */
> + {
> + 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_CMD_VERSIONS);
> + KUNIT_EXPECT_EQ(test, mock->msg.insize,
> + sizeof(struct ec_response_get_cmd_versions));
> + KUNIT_EXPECT_EQ(test, mock->msg.outsize, sizeof(struct ec_params_get_cmd_versions));
> +
> + KUNIT_EXPECT_FALSE(test, ec_dev->host_sleep_v1);
> + }
> +}
> +
> static void cros_ec_proto_test_query_all_default_wake_mask_return_error(struct kunit *test)
> {
> struct cros_ec_proto_test_priv *priv = test->priv;
> @@ -1183,7 +1466,10 @@ static struct kunit_case cros_ec_proto_test_cases[] = {
> KUNIT_CASE(cros_ec_proto_test_query_all_legacy_data_error),
> KUNIT_CASE(cros_ec_proto_test_query_all_legacy_return0),
> KUNIT_CASE(cros_ec_proto_test_query_all_no_mkbp),
> + KUNIT_CASE(cros_ec_proto_test_query_all_no_mkbp_return_error),
> + KUNIT_CASE(cros_ec_proto_test_query_all_no_mkbp_return0),
> KUNIT_CASE(cros_ec_proto_test_query_all_no_host_sleep),
> + 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),
> {}
> };
> --
> 2.36.1.255.ge46751e96f-goog
>
^ permalink raw reply [flat|nested] 28+ messages in thread* Re: [PATCH v2 14/15] platform/chrome: cros_ec_proto: return 0 on getting version mask success
2022-06-07 19:11 ` Guenter Roeck
@ 2022-06-08 2:17 ` Tzung-Bi Shih
0 siblings, 0 replies; 28+ messages in thread
From: Tzung-Bi Shih @ 2022-06-08 2:17 UTC (permalink / raw)
To: Guenter Roeck; +Cc: bleung, groeck, chrome-platform, linux-kernel
On Tue, Jun 07, 2022 at 12:11:23PM -0700, Guenter Roeck wrote:
> On Tue, Jun 7, 2022 at 7:57 AM Tzung-Bi Shih <tzungbi@kernel.org> wrote:
> >
> > cros_ec_get_host_command_version_mask() used to return value from
> > send_command() which is number of bytes for input payload on success
> > (i.e. sizeof(struct ec_response_get_cmd_versions)).
> >
> > However, the callers don't need to know how many bytes are available.
> >
> > - Fix cros_ec_get_host_command_version_mask() to return 0 on success;
> > negative integers on error.
> >
> > - Remove the unneeded `ver_mask` initialization as the callers should
> > take it only if cros_ec_get_host_command_version_mask() returns 0.
> >
> > - Add a Kunit test: `ver_mask` has some garbage bytes from previous
> > EC_CMD_GET_NEXT_EVENT but there is no host sleep to make sure the
> > caller checks the return values correctly.
> >
> This should be separate patches.
Ack. Will fix in next version.
^ permalink raw reply [flat|nested] 28+ messages in thread
* [PATCH v2 15/15] platform/chrome: cros_ec_proto: return 0 on getting wake mask success
2022-06-07 14:56 [PATCH v2 00/15] platform/chrome: Kunit tests and refactor for cros_ec_query_all() Tzung-Bi Shih
` (13 preceding siblings ...)
2022-06-07 14:56 ` [PATCH v2 14/15] platform/chrome: cros_ec_proto: return 0 on getting version mask success Tzung-Bi Shih
@ 2022-06-07 14:56 ` Tzung-Bi Shih
2022-06-07 19:12 ` Guenter Roeck
14 siblings, 1 reply; 28+ messages in thread
From: Tzung-Bi Shih @ 2022-06-07 14:56 UTC (permalink / raw)
To: bleung, groeck; +Cc: chrome-platform, linux-kernel, tzungbi
cros_ec_get_host_event_wake_mask() used to return value from
send_command() which is number of bytes for input payload on success
(i.e. sizeof(struct ec_response_host_event_mask)).
However, the callers don't need to know how many bytes are available.
- Fix cros_ec_get_host_event_wake_mask() to return 0 on success;
negative integers on error.
- Add a Kunit test for guarding if send_command() returns 0 in
get_host_event_wake_mask().
Signed-off-by: Tzung-Bi Shih <tzungbi@kernel.org>
---
Changes from v1:
- Return 0 on success; otherwise, negative intergers.
drivers/platform/chrome/cros_ec_proto.c | 23 ++--
drivers/platform/chrome/cros_ec_proto_test.c | 128 +++++++++++++++++++
2 files changed, 142 insertions(+), 9 deletions(-)
diff --git a/drivers/platform/chrome/cros_ec_proto.c b/drivers/platform/chrome/cros_ec_proto.c
index 9d96ed16244f..04c852aa790b 100644
--- a/drivers/platform/chrome/cros_ec_proto.c
+++ b/drivers/platform/chrome/cros_ec_proto.c
@@ -256,18 +256,23 @@ static int cros_ec_get_host_event_wake_mask(struct cros_ec_device *ec_dev, uint3
msg->insize = sizeof(*r);
ret = send_command(ec_dev, msg);
- if (ret >= 0) {
- mapped = cros_ec_map_error(msg->result);
- if (mapped) {
- ret = mapped;
- goto exit;
- }
+ if (ret < 0)
+ goto exit;
+
+ mapped = cros_ec_map_error(msg->result);
+ if (mapped) {
+ ret = mapped;
+ goto exit;
}
- if (ret > 0) {
- r = (struct ec_response_host_event_mask *)msg->data;
- *mask = r->mask;
+
+ if (ret == 0) {
+ ret = -EPROTO;
+ goto exit;
}
+ r = (struct ec_response_host_event_mask *)msg->data;
+ *mask = r->mask;
+ ret = 0;
exit:
kfree(msg);
return ret;
diff --git a/drivers/platform/chrome/cros_ec_proto_test.c b/drivers/platform/chrome/cros_ec_proto_test.c
index af69410f2978..3ee0de337d53 100644
--- a/drivers/platform/chrome/cros_ec_proto_test.c
+++ b/drivers/platform/chrome/cros_ec_proto_test.c
@@ -1406,6 +1406,133 @@ static void cros_ec_proto_test_query_all_default_wake_mask_return_error(struct k
}
}
+static void cros_ec_proto_test_query_all_default_wake_mask_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;
+
+ /* Set some garbage bytes. */
+ ec_dev->host_event_wake_mask = U32_MAX;
+
+ /* For cros_ec_get_proto_info() without passthru. */
+ {
+ struct ec_response_get_protocol_info *data;
+
+ mock = cros_kunit_ec_xfer_mock_add(test, sizeof(*data));
+ KUNIT_ASSERT_PTR_NE(test, mock, NULL);
+
+ /*
+ * Although it doesn't check the value, provides valid sizes so that
+ * cros_ec_query_all() allocates din and dout correctly.
+ */
+ data = (struct ec_response_get_protocol_info *)mock->o_data;
+ data->max_request_packet_size = 0xbe;
+ data->max_response_packet_size = 0xef;
+ }
+
+ /* For cros_ec_get_proto_info() with passthru. */
+ {
+ mock = cros_kunit_ec_xfer_mock_addx(test, 0, EC_RES_INVALID_COMMAND, 0);
+ KUNIT_ASSERT_PTR_NE(test, mock, NULL);
+ }
+
+ /* For cros_ec_get_host_command_version_mask() for MKBP. */
+ {
+ mock = cros_kunit_ec_xfer_mock_add(test, 0);
+ KUNIT_ASSERT_PTR_NE(test, mock, NULL);
+ }
+
+ /* For cros_ec_get_host_command_version_mask() for host sleep v1. */
+ {
+ mock = cros_kunit_ec_xfer_mock_add(test, 0);
+ KUNIT_ASSERT_PTR_NE(test, mock, NULL);
+ }
+
+ /* For get_host_event_wake_mask(). */
+ {
+ mock = cros_kunit_ec_xfer_mock_add(test, 0);
+ KUNIT_ASSERT_PTR_NE(test, mock, NULL);
+ }
+
+ cros_ec_proto_test_query_all_pretest(test);
+ ret = cros_ec_query_all(ec_dev);
+ KUNIT_EXPECT_EQ(test, ret, 0);
+
+ /* For cros_ec_get_proto_info() without passthru. */
+ {
+ 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_PROTOCOL_INFO);
+ KUNIT_EXPECT_EQ(test, mock->msg.insize,
+ sizeof(struct ec_response_get_protocol_info));
+ KUNIT_EXPECT_EQ(test, mock->msg.outsize, 0);
+ }
+
+ /* For cros_ec_get_proto_info() with passthru. */
+ {
+ 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_PASSTHRU_OFFSET(CROS_EC_DEV_PD_INDEX) |
+ EC_CMD_GET_PROTOCOL_INFO);
+ KUNIT_EXPECT_EQ(test, mock->msg.insize,
+ sizeof(struct ec_response_get_protocol_info));
+ KUNIT_EXPECT_EQ(test, mock->msg.outsize, 0);
+ }
+
+ /* For cros_ec_get_host_command_version_mask() for MKBP. */
+ {
+ 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_CMD_VERSIONS);
+ KUNIT_EXPECT_EQ(test, mock->msg.insize,
+ sizeof(struct ec_response_get_cmd_versions));
+ KUNIT_EXPECT_EQ(test, mock->msg.outsize, sizeof(struct ec_params_get_cmd_versions));
+ }
+
+ /* For cros_ec_get_host_command_version_mask() for host sleep v1. */
+ {
+ 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_CMD_VERSIONS);
+ KUNIT_EXPECT_EQ(test, mock->msg.insize,
+ sizeof(struct ec_response_get_cmd_versions));
+ KUNIT_EXPECT_EQ(test, mock->msg.outsize, sizeof(struct ec_params_get_cmd_versions));
+ }
+
+ /* For get_host_event_wake_mask(). */
+ {
+ u32 mask;
+
+ 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_HOST_EVENT_GET_WAKE_MASK);
+ KUNIT_EXPECT_EQ(test, mock->msg.insize, sizeof(struct ec_response_host_event_mask));
+ KUNIT_EXPECT_EQ(test, mock->msg.outsize, 0);
+
+ mask = ec_dev->host_event_wake_mask;
+ KUNIT_EXPECT_EQ(test, mask & EC_HOST_EVENT_MASK(EC_HOST_EVENT_LID_CLOSED), 0);
+ KUNIT_EXPECT_EQ(test, mask & EC_HOST_EVENT_MASK(EC_HOST_EVENT_AC_DISCONNECTED), 0);
+ KUNIT_EXPECT_EQ(test, mask & EC_HOST_EVENT_MASK(EC_HOST_EVENT_BATTERY_LOW), 0);
+ KUNIT_EXPECT_EQ(test, mask & EC_HOST_EVENT_MASK(EC_HOST_EVENT_BATTERY_CRITICAL), 0);
+ KUNIT_EXPECT_EQ(test, mask & EC_HOST_EVENT_MASK(EC_HOST_EVENT_BATTERY), 0);
+ KUNIT_EXPECT_EQ(test, mask & EC_HOST_EVENT_MASK(EC_HOST_EVENT_PD_MCU), 0);
+ KUNIT_EXPECT_EQ(test, mask & EC_HOST_EVENT_MASK(EC_HOST_EVENT_BATTERY_STATUS), 0);
+ }
+}
+
static void cros_ec_proto_test_release(struct device *dev)
{
}
@@ -1471,6 +1598,7 @@ static struct kunit_case cros_ec_proto_test_cases[] = {
KUNIT_CASE(cros_ec_proto_test_query_all_no_host_sleep),
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),
{}
};
--
2.36.1.255.ge46751e96f-goog
^ permalink raw reply related [flat|nested] 28+ messages in thread* Re: [PATCH v2 15/15] platform/chrome: cros_ec_proto: return 0 on getting wake mask success
2022-06-07 14:56 ` [PATCH v2 15/15] platform/chrome: cros_ec_proto: return 0 on getting wake " Tzung-Bi Shih
@ 2022-06-07 19:12 ` Guenter Roeck
2022-06-08 2:17 ` Tzung-Bi Shih
0 siblings, 1 reply; 28+ messages in thread
From: Guenter Roeck @ 2022-06-07 19:12 UTC (permalink / raw)
To: Tzung-Bi Shih; +Cc: bleung, groeck, chrome-platform, linux-kernel
On Tue, Jun 7, 2022 at 7:57 AM Tzung-Bi Shih <tzungbi@kernel.org> wrote:
>
> cros_ec_get_host_event_wake_mask() used to return value from
> send_command() which is number of bytes for input payload on success
> (i.e. sizeof(struct ec_response_host_event_mask)).
>
> However, the callers don't need to know how many bytes are available.
>
> - Fix cros_ec_get_host_event_wake_mask() to return 0 on success;
> negative integers on error.
>
> - Add a Kunit test for guarding if send_command() returns 0 in
> get_host_event_wake_mask().
>
Please split into two patches.
Thanks,
Guenter
> Signed-off-by: Tzung-Bi Shih <tzungbi@kernel.org>
> ---
> Changes from v1:
> - Return 0 on success; otherwise, negative intergers.
>
> drivers/platform/chrome/cros_ec_proto.c | 23 ++--
> drivers/platform/chrome/cros_ec_proto_test.c | 128 +++++++++++++++++++
> 2 files changed, 142 insertions(+), 9 deletions(-)
>
> diff --git a/drivers/platform/chrome/cros_ec_proto.c b/drivers/platform/chrome/cros_ec_proto.c
> index 9d96ed16244f..04c852aa790b 100644
> --- a/drivers/platform/chrome/cros_ec_proto.c
> +++ b/drivers/platform/chrome/cros_ec_proto.c
> @@ -256,18 +256,23 @@ static int cros_ec_get_host_event_wake_mask(struct cros_ec_device *ec_dev, uint3
> msg->insize = sizeof(*r);
>
> ret = send_command(ec_dev, msg);
> - if (ret >= 0) {
> - mapped = cros_ec_map_error(msg->result);
> - if (mapped) {
> - ret = mapped;
> - goto exit;
> - }
> + if (ret < 0)
> + goto exit;
> +
> + mapped = cros_ec_map_error(msg->result);
> + if (mapped) {
> + ret = mapped;
> + goto exit;
> }
> - if (ret > 0) {
> - r = (struct ec_response_host_event_mask *)msg->data;
> - *mask = r->mask;
> +
> + if (ret == 0) {
> + ret = -EPROTO;
> + goto exit;
> }
>
> + r = (struct ec_response_host_event_mask *)msg->data;
> + *mask = r->mask;
> + ret = 0;
> exit:
> kfree(msg);
> return ret;
> diff --git a/drivers/platform/chrome/cros_ec_proto_test.c b/drivers/platform/chrome/cros_ec_proto_test.c
> index af69410f2978..3ee0de337d53 100644
> --- a/drivers/platform/chrome/cros_ec_proto_test.c
> +++ b/drivers/platform/chrome/cros_ec_proto_test.c
> @@ -1406,6 +1406,133 @@ static void cros_ec_proto_test_query_all_default_wake_mask_return_error(struct k
> }
> }
>
> +static void cros_ec_proto_test_query_all_default_wake_mask_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;
> +
> + /* Set some garbage bytes. */
> + ec_dev->host_event_wake_mask = U32_MAX;
> +
> + /* For cros_ec_get_proto_info() without passthru. */
> + {
> + struct ec_response_get_protocol_info *data;
> +
> + mock = cros_kunit_ec_xfer_mock_add(test, sizeof(*data));
> + KUNIT_ASSERT_PTR_NE(test, mock, NULL);
> +
> + /*
> + * Although it doesn't check the value, provides valid sizes so that
> + * cros_ec_query_all() allocates din and dout correctly.
> + */
> + data = (struct ec_response_get_protocol_info *)mock->o_data;
> + data->max_request_packet_size = 0xbe;
> + data->max_response_packet_size = 0xef;
> + }
> +
> + /* For cros_ec_get_proto_info() with passthru. */
> + {
> + mock = cros_kunit_ec_xfer_mock_addx(test, 0, EC_RES_INVALID_COMMAND, 0);
> + KUNIT_ASSERT_PTR_NE(test, mock, NULL);
> + }
> +
> + /* For cros_ec_get_host_command_version_mask() for MKBP. */
> + {
> + mock = cros_kunit_ec_xfer_mock_add(test, 0);
> + KUNIT_ASSERT_PTR_NE(test, mock, NULL);
> + }
> +
> + /* For cros_ec_get_host_command_version_mask() for host sleep v1. */
> + {
> + mock = cros_kunit_ec_xfer_mock_add(test, 0);
> + KUNIT_ASSERT_PTR_NE(test, mock, NULL);
> + }
> +
> + /* For get_host_event_wake_mask(). */
> + {
> + mock = cros_kunit_ec_xfer_mock_add(test, 0);
> + KUNIT_ASSERT_PTR_NE(test, mock, NULL);
> + }
> +
> + cros_ec_proto_test_query_all_pretest(test);
> + ret = cros_ec_query_all(ec_dev);
> + KUNIT_EXPECT_EQ(test, ret, 0);
> +
> + /* For cros_ec_get_proto_info() without passthru. */
> + {
> + 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_PROTOCOL_INFO);
> + KUNIT_EXPECT_EQ(test, mock->msg.insize,
> + sizeof(struct ec_response_get_protocol_info));
> + KUNIT_EXPECT_EQ(test, mock->msg.outsize, 0);
> + }
> +
> + /* For cros_ec_get_proto_info() with passthru. */
> + {
> + 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_PASSTHRU_OFFSET(CROS_EC_DEV_PD_INDEX) |
> + EC_CMD_GET_PROTOCOL_INFO);
> + KUNIT_EXPECT_EQ(test, mock->msg.insize,
> + sizeof(struct ec_response_get_protocol_info));
> + KUNIT_EXPECT_EQ(test, mock->msg.outsize, 0);
> + }
> +
> + /* For cros_ec_get_host_command_version_mask() for MKBP. */
> + {
> + 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_CMD_VERSIONS);
> + KUNIT_EXPECT_EQ(test, mock->msg.insize,
> + sizeof(struct ec_response_get_cmd_versions));
> + KUNIT_EXPECT_EQ(test, mock->msg.outsize, sizeof(struct ec_params_get_cmd_versions));
> + }
> +
> + /* For cros_ec_get_host_command_version_mask() for host sleep v1. */
> + {
> + 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_CMD_VERSIONS);
> + KUNIT_EXPECT_EQ(test, mock->msg.insize,
> + sizeof(struct ec_response_get_cmd_versions));
> + KUNIT_EXPECT_EQ(test, mock->msg.outsize, sizeof(struct ec_params_get_cmd_versions));
> + }
> +
> + /* For get_host_event_wake_mask(). */
> + {
> + u32 mask;
> +
> + 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_HOST_EVENT_GET_WAKE_MASK);
> + KUNIT_EXPECT_EQ(test, mock->msg.insize, sizeof(struct ec_response_host_event_mask));
> + KUNIT_EXPECT_EQ(test, mock->msg.outsize, 0);
> +
> + mask = ec_dev->host_event_wake_mask;
> + KUNIT_EXPECT_EQ(test, mask & EC_HOST_EVENT_MASK(EC_HOST_EVENT_LID_CLOSED), 0);
> + KUNIT_EXPECT_EQ(test, mask & EC_HOST_EVENT_MASK(EC_HOST_EVENT_AC_DISCONNECTED), 0);
> + KUNIT_EXPECT_EQ(test, mask & EC_HOST_EVENT_MASK(EC_HOST_EVENT_BATTERY_LOW), 0);
> + KUNIT_EXPECT_EQ(test, mask & EC_HOST_EVENT_MASK(EC_HOST_EVENT_BATTERY_CRITICAL), 0);
> + KUNIT_EXPECT_EQ(test, mask & EC_HOST_EVENT_MASK(EC_HOST_EVENT_BATTERY), 0);
> + KUNIT_EXPECT_EQ(test, mask & EC_HOST_EVENT_MASK(EC_HOST_EVENT_PD_MCU), 0);
> + KUNIT_EXPECT_EQ(test, mask & EC_HOST_EVENT_MASK(EC_HOST_EVENT_BATTERY_STATUS), 0);
> + }
> +}
> +
> static void cros_ec_proto_test_release(struct device *dev)
> {
> }
> @@ -1471,6 +1598,7 @@ static struct kunit_case cros_ec_proto_test_cases[] = {
> KUNIT_CASE(cros_ec_proto_test_query_all_no_host_sleep),
> 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),
> {}
> };
>
> --
> 2.36.1.255.ge46751e96f-goog
>
^ permalink raw reply [flat|nested] 28+ messages in thread* Re: [PATCH v2 15/15] platform/chrome: cros_ec_proto: return 0 on getting wake mask success
2022-06-07 19:12 ` Guenter Roeck
@ 2022-06-08 2:17 ` Tzung-Bi Shih
0 siblings, 0 replies; 28+ messages in thread
From: Tzung-Bi Shih @ 2022-06-08 2:17 UTC (permalink / raw)
To: Guenter Roeck; +Cc: bleung, groeck, chrome-platform, linux-kernel
On Tue, Jun 07, 2022 at 12:12:25PM -0700, Guenter Roeck wrote:
> On Tue, Jun 7, 2022 at 7:57 AM Tzung-Bi Shih <tzungbi@kernel.org> wrote:
> >
> > cros_ec_get_host_event_wake_mask() used to return value from
> > send_command() which is number of bytes for input payload on success
> > (i.e. sizeof(struct ec_response_host_event_mask)).
> >
> > However, the callers don't need to know how many bytes are available.
> >
> > - Fix cros_ec_get_host_event_wake_mask() to return 0 on success;
> > negative integers on error.
> >
> > - Add a Kunit test for guarding if send_command() returns 0 in
> > get_host_event_wake_mask().
> >
> Please split into two patches.
Ack.
^ permalink raw reply [flat|nested] 28+ messages in thread