linux-kernel.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
* [PATCH 1/2] platform/chrome: cros_ec_proto: Use EC struct for features
@ 2021-10-04 17:07 Prashant Malani
  2021-10-04 17:07 ` [PATCH 2/2] platform/chrome: cros_ec_proto: Use ec_command for check_features Prashant Malani
  2021-10-31 23:01 ` [PATCH 1/2] platform/chrome: cros_ec_proto: Use EC struct for features Benson Leung
  0 siblings, 2 replies; 3+ messages in thread
From: Prashant Malani @ 2021-10-04 17:07 UTC (permalink / raw)
  To: linux-kernel
  Cc: Prashant Malani, Benson Leung, Enric Balletbo i Serra,
	Guenter Roeck, Gustavo A. R. Silva, Lee Jones

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 <pmalani@chromium.org>
---
 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


^ permalink raw reply related	[flat|nested] 3+ messages in thread

* [PATCH 2/2] platform/chrome: cros_ec_proto: Use ec_command for check_features
  2021-10-04 17:07 [PATCH 1/2] platform/chrome: cros_ec_proto: Use EC struct for features Prashant Malani
@ 2021-10-04 17:07 ` Prashant Malani
  2021-10-31 23:01 ` [PATCH 1/2] platform/chrome: cros_ec_proto: Use EC struct for features Benson Leung
  1 sibling, 0 replies; 3+ messages in thread
From: Prashant Malani @ 2021-10-04 17:07 UTC (permalink / raw)
  To: linux-kernel
  Cc: Prashant Malani, Benson Leung, Enric Balletbo i Serra,
	Guenter Roeck, Gustavo A. R. Silva, Lee Jones

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 <pmalani@chromium.org>
---
 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


^ permalink raw reply related	[flat|nested] 3+ messages in thread

* Re: [PATCH 1/2] platform/chrome: cros_ec_proto: Use EC struct for features
  2021-10-04 17:07 [PATCH 1/2] platform/chrome: cros_ec_proto: Use EC struct for features Prashant Malani
  2021-10-04 17:07 ` [PATCH 2/2] platform/chrome: cros_ec_proto: Use ec_command for check_features Prashant Malani
@ 2021-10-31 23:01 ` Benson Leung
  1 sibling, 0 replies; 3+ messages in thread
From: Benson Leung @ 2021-10-31 23:01 UTC (permalink / raw)
  To: linux-kernel, Prashant Malani
  Cc: Gustavo A. R. Silva, Enric Balletbo i Serra, Guenter Roeck, Lee Jones

[-- Attachment #1: Type: text/plain, Size: 843 bytes --]

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.
bleung@google.com
Chromium OS Project
bleung@chromium.org

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 228 bytes --]

^ permalink raw reply	[flat|nested] 3+ messages in thread

end of thread, other threads:[~2021-10-31 23:01 UTC | newest]

Thread overview: 3+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2021-10-04 17:07 [PATCH 1/2] platform/chrome: cros_ec_proto: Use EC struct for features Prashant Malani
2021-10-04 17:07 ` [PATCH 2/2] platform/chrome: cros_ec_proto: Use ec_command for check_features Prashant Malani
2021-10-31 23:01 ` [PATCH 1/2] platform/chrome: cros_ec_proto: Use EC struct for features Benson Leung

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).