CHROMIUM: cros-ec: Fix usage of cros_ec_cmd_xfer

cros_ec_cmd_xfer returns success status if the command transport
completes successfully, but the execution result is incorrectly ignored.
In many cases, the execution result is assumed to be successful, leading
to ignored errors and operating on uninitialized data.

Add a new function cros_ec_cmd_xfer_status that checks the execution result
and returns error status if it's non-success. Convert most uses in the
codebase (those which don't already decode the result) to use this new
function.

BUG=chromium:451105
TEST=Manual on Samus. Verify PD AU and PD charger drivers still function
correctly.

Change-Id: Ic35d0cd6358435f027d21a562d60165a3b90d6c7
Signed-off-by: Shawn Nematbakhsh <shawnn@chromium.org>
Reviewed-on: https://chromium-review.googlesource.com/242722
Reviewed-by: Gwendal Grignou <gwendal@chromium.org>
Reviewed-by: Sameer Nanda <snanda@chromium.org>

drivers/i2c/busses/i2c-cros-ec-tunnel.c

@@ -70,7 +70,7 @@ static int ec_i2c_forward_msg(struct ec_i2c_device *bus, int cmd,
 	msg.outsize = outmsg ? outmsg->len : 0;
 	msg.indata = inmsg ? inmsg->buf : NULL;
 	msg.insize = inmsg ? inmsg->len : 0;
-	return cros_ec_cmd_xfer(bus->ec, &msg);
+	return cros_ec_cmd_xfer_status(bus->ec, &msg);
 }
 
 static int ec_i2c_limited_xfer(struct i2c_adapter *adap, struct i2c_msg msgs[],
@@ -285,11 +285,10 @@ static int ec_i2c_xfer(struct i2c_adapter *adap, struct i2c_msg i2c_msgs[],
 	msg.indata = response;
 	msg.insize = response_len;
 
-	result = cros_ec_cmd_xfer(bus->ec, &msg);
+	result = cros_ec_cmd_xfer_status(bus->ec, &msg);
 	if (result < 0)
 		goto exit;
 
-	/* FIXME: This assumes msg.result == EC_RES_SUCCESS */
 	result = ec_i2c_parse_response(response, i2c_msgs, &num);
 	if (result < 0)
 		goto exit;

drivers/iio/accel/cros_ec_accel.c

@@ -308,7 +308,7 @@ static int send_motion_host_cmd(struct cros_ec_accel_state *st,
 		sizeof(struct ec_response_motion_sense);
 
 	/* Send host command. */
-	if (cros_ec_cmd_xfer(st->ec, &st->msg) > 0)
+	if (cros_ec_cmd_xfer_status(st->ec, &st->msg) > 0)
 		return 0;
 	else
 		return -EIO;

drivers/input/keyboard/cros_ec_keyb.c

@@ -188,8 +188,7 @@ static int cros_ec_keyb_get_state(struct cros_ec_keyb *ckdev, uint8_t *kb_state)
 		.insize = ckdev->cols,
 	};
 
-	ret = cros_ec_cmd_xfer(ckdev->ec, &msg);
-	/* FIXME: This assumes msg.result == EC_RES_SUCCESS */
+	ret = cros_ec_cmd_xfer_status(ckdev->ec, &msg);
 	return ret;
 }
 

drivers/mfd/cros_ec.c

@@ -348,6 +348,20 @@ int cros_ec_cmd_xfer(struct cros_ec_device *ec_dev,
 }
 EXPORT_SYMBOL(cros_ec_cmd_xfer);
 
+int cros_ec_cmd_xfer_status(struct cros_ec_device *ec_dev,
+			    struct cros_ec_command *msg)
+{
+	int ret = cros_ec_cmd_xfer(ec_dev, msg);
+
+	if (ret < 0)
+		dev_err(ec_dev->dev, "Command xfer error (err:%d)\n", ret);
+	else if (msg->result)
+		return -EECRESULT - msg->result;
+
+	return ret;
+}
+EXPORT_SYMBOL(cros_ec_cmd_xfer_status);
+
 static const struct mfd_cell cros_accel_devs[] = {
 	{
 		.name = "cros-ec-accel",

drivers/mfd/cros_ec_pd_update.c

@@ -102,14 +102,8 @@ static int cros_ec_pd_command(struct device *dev,
 	msg.indata = indata;
 	msg.insize = insize;
 
-	ret = cros_ec_cmd_xfer(pd_dev->ec_dev, &msg);
-	if (ret < 0) {
-		dev_err(dev, "Command xfer error (err:%d)\n", ret);
-		return ret;
-	} else if (msg.result)
-		return -EECRESULT - msg.result;
-	else
-		return EC_RES_SUCCESS;
+	ret = cros_ec_cmd_xfer_status(pd_dev->ec_dev, &msg);
+	return ret >= 0 ? EC_RES_SUCCESS : ret;
 }
 
 /**

drivers/platform/chromeos_vbc_ec.c

@@ -74,10 +74,9 @@ static int vbc_read(struct device *dev, void *buf, size_t count)
 	ec = dev_get_drvdata(dev->parent);
 
 	param.op = EC_VBNV_CONTEXT_OP_READ;
-	err = cros_ec_cmd_xfer(ec, &msg);
+	err = cros_ec_cmd_xfer_status(ec, &msg);
 	if (err < 0)
 		return err;
-	/* FIXME: This assumes msg.result == EC_RES_SUCCESS */
 	count = min(count, sizeof(resp.block));
 	memcpy(buf, resp.block, count);
 
@@ -104,10 +103,9 @@ static int vbc_write(struct device *dev, const void *buf, size_t count)
 
 	param.op = EC_VBNV_CONTEXT_OP_WRITE;
 	memcpy(param.block, buf, count);
-	err = cros_ec_cmd_xfer(ec, &msg);
+	err = cros_ec_cmd_xfer_status(ec, &msg);
 	if (err < 0)
 		return err;
-	/* FIXME: This assumes msg.result == EC_RES_SUCCESS */
 	return count;
 }
 

drivers/power/cros_ec-charger.c

@@ -113,7 +113,7 @@ static int get_ec_power_info(struct power_supply *psy,
 	msg.outsize = 0;
 	msg.indata = (u8 *)ec_info;
 	msg.insize = sizeof(struct ec_response_power_info);
-	ret = cros_ec_cmd_xfer(ec, &msg);
+	ret = cros_ec_cmd_xfer_status(ec, &msg);
 	if (ret < 0) {
 		dev_err(dev, "Unable to query EC power info (err:%d)\n",
 			ret);

drivers/power/cros_usbpd-charger.c

@@ -90,7 +90,7 @@ int ec_command(struct charger_data *charger, int command,
 	msg.outsize = outsize;
 	msg.indata = indata;
 	msg.insize = insize;
-	return cros_ec_cmd_xfer(ec_device, &msg);
+	return cros_ec_cmd_xfer_status(ec_device, &msg);
 }
 
 static int set_ec_usb_pd_override_ports(struct charger_data *charger,

drivers/regulator/cros_ec-tps65090.c

@@ -63,7 +63,7 @@ static int ec_tps65090_fet_is_enabled(struct regulator_dev *rdev)
 	msg.outsize = sizeof(struct ec_params_ldo_get);
 	msg.indata = (u8 *)&ec_info;
 	msg.insize = sizeof(struct ec_response_ldo_get);
-	ret = cros_ec_cmd_xfer(ec, &msg);
+	ret = cros_ec_cmd_xfer_status(ec, &msg);
 	if (ret < 0)
 		return ret;
 
@@ -85,7 +85,7 @@ static int ec_tps65090_fet_enable(struct regulator_dev *rdev)
 	msg.outsize = sizeof(struct ec_params_ldo_set);
 	msg.indata = NULL;
 	msg.insize = 0;
-	return cros_ec_cmd_xfer(ec, &msg);
+	return cros_ec_cmd_xfer_status(ec, &msg);
 }
 
 static int ec_tps65090_fet_disable(struct regulator_dev *rdev)
@@ -103,7 +103,7 @@ static int ec_tps65090_fet_disable(struct regulator_dev *rdev)
 	msg.outsize = sizeof(struct ec_params_ldo_set);
 	msg.indata = NULL;
 	msg.insize = 0;
-	return cros_ec_cmd_xfer(ec, &msg);
+	return cros_ec_cmd_xfer_status(ec, &msg);
 }
 
 static int ec_tps65090_set_voltage(struct regulator_dev *rdev, int min,

include/linux/mfd/cros_ec.h

@@ -163,14 +163,32 @@ int cros_ec_prepare_tx(struct cros_ec_device *ec_dev,
  * cros_ec_cmd_xfer - Send a command to the ChromeOS EC
  *
  * Call this to send a command to the ChromeOS EC.  This should be used
- * instead of calling the EC's cmd_xfer() callback directly.
+ * instead of calling the EC's cmd_xfer() callback directly. Note that
+ * msg->result should be checked before assuming that the command ran
+ * successfully on the EC.
  *
  * @ec_dev: EC device
  * @msg: Message to write
+ * @return: Num. of bytes transferred on success, <0 on failure
  */
 int cros_ec_cmd_xfer(struct cros_ec_device *ec_dev,
 		     struct cros_ec_command *msg);
 
+/**
+ * cros_ec_cmd_xfer_status - Send a command to the ChromeOS EC
+ *
+ * This function is identical to cros_ec_cmd_xfer, except it returns succes
+ * status only if both the command was transmitted successfully and the EC
+ * replied with success status. It's not necessary to check msg->result when
+ * using this function.
+ *
+ * @ec_dev: EC device
+ * @msg: Message to write
+ * @return: Num. of bytes transferred on success, <0 on failure
+ */
+int cros_ec_cmd_xfer_status(struct cros_ec_device *ec_dev,
+			    struct cros_ec_command *msg);
+
 /**
  * cros_ec_remove - Remove a ChromeOS EC
  *