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

From: Guenter Roeck
Date: Sat Jun 18 2016 - 00:14:14 EST


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
EC_RES_INVALID_PARAM
EC_RES_INVALID_VERSION
EC_RES_INVALID_HEADER


Personal preference, but yes.

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

You lost me there, sorry.

API if that's what you'd like...


That isn't what I suggested.

Guenter