Re: [v3,1/4] mfd: cros_ec: Add cros_ec_cmd_xfer_status helper

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 




On 06/17/2016 06:08 PM, Brian Norris wrote:
On Fri, Jun 17, 2016 at 02:41:51PM -0700, Guenter Roeck wrote:
On Fri, Jun 17, 2016 at 12:58:12PM -0700, Brian Norris wrote:
+int cros_ec_cmd_xfer_status(struct cros_ec_device *ec_dev,
+			    struct cros_ec_command *msg)
+{
+	int ret;
+
+	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 != EC_RES_SUCCESS)
+		return -EECRESULT - msg->result;

I have been wondering about the error return codes here, and if they should be
converted to standard Linux error codes. For example, I just hit error -1003
with a driver I am working on. This translates to EC_RES_INVALID_PARAM, or,
in Linux terms, -EINVAL. I think it would be better to use standard error
codes, especially since some of the errors are logged.

How do you propose we do that? Do all of the following become EINVAL?

         EC_RES_INVALID_COMMAND

	-EOPNOTSUPP

         EC_RES_INVALID_PARAM

	-EINVAL or -EBADMSG

         EC_RES_INVALID_VERSION

	-EPROTO or -EBADR or -EBADE or -EBADRQC or -EPROTOOPT

         EC_RES_INVALID_HEADER

	-EPROTO or -EBADR or -EBADE

Doesn't look that bad to me. Also, the raw error could still be logged,
for example with dev_dbg().

Guenter


We lose a lot of information that way. And particularly, cros_ec_num_pwms()
won't be able to count as accurately. But I can just go back to not using this
API if that's what you'd like...

Brian


--
To unsubscribe from this list: send the line "unsubscribe devicetree" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at  http://vger.kernel.org/majordomo-info.html



[Index of Archives]     [Device Tree Compilter]     [Device Tree Spec]     [Linux Driver Backports]     [Video for Linux]     [Linux USB Devel]     [Linux PCI Devel]     [Linux Audio Users]     [Linux Kernel]     [Linux SCSI]     [XFree86]     [Yosemite Backpacking]
  Powered by Linux