The Chrome EC's features are returned through an
ec_response_get_features struct, but they are stored in an independent
array. Although the two are effectively the same at present (2 unsigned
32 bit ints), there is the possibility that they could go out of sync.
Avoid this by only using the EC struct to store the features.
Signed-off-by: Prashant Malani <[email protected]>
---
drivers/mfd/cros_ec_dev.c | 4 ++--
drivers/platform/chrome/cros_ec_proto.c | 15 ++++++++-------
include/linux/platform_data/cros_ec_proto.h | 2 +-
3 files changed, 11 insertions(+), 10 deletions(-)
diff --git a/drivers/mfd/cros_ec_dev.c b/drivers/mfd/cros_ec_dev.c
index 8c08d1c55726..6ee1f410eb53 100644
--- a/drivers/mfd/cros_ec_dev.c
+++ b/drivers/mfd/cros_ec_dev.c
@@ -146,8 +146,8 @@ static int ec_device_probe(struct platform_device *pdev)
ec->ec_dev = dev_get_drvdata(dev->parent);
ec->dev = dev;
ec->cmd_offset = ec_platform->cmd_offset;
- ec->features[0] = -1U; /* Not cached yet */
- ec->features[1] = -1U; /* Not cached yet */
+ ec->features.flags[0] = -1U; /* Not cached yet */
+ ec->features.flags[1] = -1U; /* Not cached yet */
device_initialize(&ec->class_dev);
for (i = 0; i < ARRAY_SIZE(cros_mcu_devices); i++) {
diff --git a/drivers/platform/chrome/cros_ec_proto.c b/drivers/platform/chrome/cros_ec_proto.c
index a9f1867e5d8f..b908cdd680e3 100644
--- a/drivers/platform/chrome/cros_ec_proto.c
+++ b/drivers/platform/chrome/cros_ec_proto.c
@@ -812,36 +812,37 @@ EXPORT_SYMBOL(cros_ec_get_host_event);
*/
bool cros_ec_check_features(struct cros_ec_dev *ec, int feature)
{
+ struct ec_response_get_features *features = &ec->features;
struct cros_ec_command *msg;
int ret;
- if (ec->features[0] == -1U && ec->features[1] == -1U) {
+ if (features->flags[0] == -1U && features->flags[1] == -1U) {
/* features bitmap not read yet */
- msg = kzalloc(sizeof(*msg) + sizeof(ec->features), GFP_KERNEL);
+ msg = kzalloc(sizeof(*msg) + sizeof(*features), GFP_KERNEL);
if (!msg) {
dev_err(ec->dev, "failed to allocate memory to get EC features\n");
return false;
}
msg->command = EC_CMD_GET_FEATURES + ec->cmd_offset;
- msg->insize = sizeof(ec->features);
+ msg->insize = sizeof(*features);
ret = cros_ec_cmd_xfer_status(ec->ec_dev, msg);
if (ret < 0) {
dev_warn(ec->dev, "cannot get EC features: %d/%d\n",
ret, msg->result);
- memset(ec->features, 0, sizeof(ec->features));
+ memset(features, 0, sizeof(*features));
} else {
- memcpy(ec->features, msg->data, sizeof(ec->features));
+ memcpy(features, msg->data, sizeof(*features));
}
dev_dbg(ec->dev, "EC features %08x %08x\n",
- ec->features[0], ec->features[1]);
+ features->flags[0], features->flags[1]);
kfree(msg);
}
- return !!(ec->features[feature / 32] & EC_FEATURE_MASK_0(feature));
+ return !!(features->flags[feature / 32] & EC_FEATURE_MASK_0(feature));
}
EXPORT_SYMBOL_GPL(cros_ec_check_features);
diff --git a/include/linux/platform_data/cros_ec_proto.h b/include/linux/platform_data/cros_ec_proto.h
index 9d370816a419..df3c78c92ca2 100644
--- a/include/linux/platform_data/cros_ec_proto.h
+++ b/include/linux/platform_data/cros_ec_proto.h
@@ -205,7 +205,7 @@ struct cros_ec_dev {
struct cros_ec_debugfs *debug_info;
bool has_kb_wake_angle;
u16 cmd_offset;
- u32 features[2];
+ struct ec_response_get_features features;
};
#define to_cros_ec_dev(dev) container_of(dev, struct cros_ec_dev, class_dev)
--
2.33.0.800.g4c38ced690-goog
Use the existing cros_ec_command() for cros_ec_check_features(). This
eliminates an unnecessary duplication of the memory allocation/free and
memory copy code.
Signed-off-by: Prashant Malani <[email protected]>
---
drivers/platform/chrome/cros_ec_proto.c | 20 +++-----------------
1 file changed, 3 insertions(+), 17 deletions(-)
diff --git a/drivers/platform/chrome/cros_ec_proto.c b/drivers/platform/chrome/cros_ec_proto.c
index b908cdd680e3..c4caf2e2de82 100644
--- a/drivers/platform/chrome/cros_ec_proto.c
+++ b/drivers/platform/chrome/cros_ec_proto.c
@@ -813,33 +813,19 @@ EXPORT_SYMBOL(cros_ec_get_host_event);
bool cros_ec_check_features(struct cros_ec_dev *ec, int feature)
{
struct ec_response_get_features *features = &ec->features;
- struct cros_ec_command *msg;
int ret;
if (features->flags[0] == -1U && features->flags[1] == -1U) {
/* features bitmap not read yet */
- msg = kzalloc(sizeof(*msg) + sizeof(*features), GFP_KERNEL);
- if (!msg) {
- dev_err(ec->dev, "failed to allocate memory to get EC features\n");
- return false;
- }
-
- msg->command = EC_CMD_GET_FEATURES + ec->cmd_offset;
- msg->insize = sizeof(*features);
-
- ret = cros_ec_cmd_xfer_status(ec->ec_dev, msg);
+ ret = cros_ec_command(ec->ec_dev, 0, EC_CMD_GET_FEATURES + ec->cmd_offset,
+ NULL, 0, features, sizeof(*features));
if (ret < 0) {
- dev_warn(ec->dev, "cannot get EC features: %d/%d\n",
- ret, msg->result);
+ dev_warn(ec->dev, "cannot get EC features: %d\n", ret);
memset(features, 0, sizeof(*features));
- } else {
- memcpy(features, msg->data, sizeof(*features));
}
dev_dbg(ec->dev, "EC features %08x %08x\n",
features->flags[0], features->flags[1]);
-
- kfree(msg);
}
return !!(features->flags[feature / 32] & EC_FEATURE_MASK_0(feature));
--
2.33.0.800.g4c38ced690-goog
Hi Prashant,
On Mon, 4 Oct 2021 10:07:09 -0700, Prashant Malani wrote:
> The Chrome EC's features are returned through an
> ec_response_get_features struct, but they are stored in an independent
> array. Although the two are effectively the same at present (2 unsigned
> 32 bit ints), there is the possibility that they could go out of sync.
> Avoid this by only using the EC struct to store the features.
Applied, thanks!
[1/2] platform/chrome: cros_ec_proto: Use EC struct for features
commit: 7ff22787ba49c2e66dcec92f3e2b79ef6b6a0d71
[2/2] platform/chrome: cros_ec_proto: Use ec_command for check_features
commit: 297d34e73d491a3edbd6e8c31d33ec90447a908b
Best regards,
--
Benson Leung
Staff Software Engineer
Chrome OS Kernel
Google Inc.
[email protected]
Chromium OS Project
[email protected]